fix(typing): scope mypy strict to the brokers, and check keel.* for real

keel/agent.py

@@ -60,7 +60,7 @@
 from collections.abc import Callable
 from dataclasses import dataclass, field
 from decimal import Decimal
-from typing import Any
+from typing import Any, Literal
 
 from keel_core.products import quote_currency_of
 from keel_core.telemetry import bind_cycle, log_event, new_cycle_id, unbind_cycle
@@ -309,9 +309,16 @@ def _open_tranche(
     rails vetoed. Neither is an error here -- the tranche is real either way, and reconciliation
     simply has no bracket to resolve back to it until one is placed.
     """
-    entry_fill = None if order is None else order["actual_fill"]
-    qty = None if order is None else order["qty"]
-    if entry_fill is None or qty is None:
+    # `order is None` is folded into the one guard below rather than tested separately, so the
+    # checker can narrow `order` for the `order["fee"]` read after it. Testing only the two
+    # extracted values left `order` typed `dict | None` all the way down, even though a `None`
+    # order forces `entry_fill`/`qty` to `None` and returns here.
+    if order is None:
+        entry_fill = qty = None
+    else:
+        entry_fill = order["actual_fill"]
+        qty = order["qty"]
+    if order is None or entry_fill is None or qty is None:
         log_event(
             logger,
             logging.WARNING,
@@ -478,6 +485,11 @@ def _seed_paper_account_if_needed(
         repo.set_state("equity_state_mode", "paper")
     if paper_trader.get_cash() is None:
         funding = config.paper.starting_equity_usd
+        # Declared up front: inferring from the first branch pins `seed` to `Decimal`, and the
+        # mark-to-market fallback below can legitimately yield `None` (unreadable broker). The
+        # `is None` check in that branch still returns before `seed_cash`, so this only widens
+        # the declaration to match what the two branches actually produce.
+        seed: Decimal | None
         if funding > 0:
             seed = funding
         else:
@@ -593,7 +605,10 @@ def _handle_exits(
     repo: Repository,
     broker: Any,
     config: Config,
-    mode: str,
+    # Same `Literal` pair as `_effective_mode`'s return and `executor.execute`'s parameter: this
+    # only ever receives the former and only ever forwards to the latter, so a bare `str` here
+    # was the one gap that let an unchecked value through the middle of that path.
+    mode: Literal["confirm", "autonomous"],
     now_ts: int,
     confirm_fn: executor.ConfirmFn | None = None,
 ) -> list[ExecutionResult]:
@@ -769,9 +784,17 @@ class LoopResult:
     drawdown_weekly_pct: Decimal | None = None
 
 
-def _effective_mode(config: Config, repo: Repository, now_ts: int) -> str:
+def _effective_mode(
+    config: Config, repo: Repository, now_ts: int
+) -> Literal["confirm", "autonomous"]:
     """The executor mode for this cycle: `"autonomous"` or `"confirm"`.
 
+    The return type is the same `Literal` pair `executor.execute` accepts, not a bare `str`:
+    those two values are all this can return (see the fail-toward-`"confirm"` note below), and
+    the wider annotation meant the two `execute(...)` call sites in this module were passing an
+    unchecked `str` into a `Literal` parameter. `config.auto_trade.mode` stays a `str` -- it has
+    a third value, `"paper"`, which never reaches an executor mode at all.
+
     Two independent switches, deliberately not conflated into one enum:
 
     * `config.auto_trade.mode` says whether this is real money at all (`paper` never reaches an

keel/analysis/levels.py

@@ -91,15 +91,19 @@ def _cluster_pivots(
     clusters: list[list[tuple[int, Decimal]]] = [[ordered[0]]]
     for pivot in ordered[1:]:
         current = clusters[-1]
-        avg = sum(price for _, price in current) / len(current)
+        # `Decimal(0)` start: a bare `sum()` starts from an `int` 0, which types the running
+        # average as `Decimal | float` and carries that widening into `Level.price` below. Every
+        # cluster holds at least the pivot that opened it, so the start value is never actually
+        # summed -- this pins the type without changing a single computed number.
+        avg = sum((price for _, price in current), Decimal(0)) / len(current)
         if abs(pivot[1] - avg) <= avg * tolerance:
             current.append(pivot)
         else:
             clusters.append([pivot])
 
     levels = []
     for cluster in clusters:
-        avg_price = sum(price for _, price in cluster) / len(cluster)
+        avg_price = sum((price for _, price in cluster), Decimal(0)) / len(cluster)
         touches = _distinct_touches([ts for ts, _ in cluster], min_separation_sec)
         levels.append(Level(price=avg_price, kind=kind, touches=touches, angular=False))
     return levels

keel/cli.py

@@ -1816,14 +1816,18 @@ def _build_account_metrics(
         per_asset_pnl[trade.asset] = per_asset_pnl.get(trade.asset, Decimal("0")) + (
             trade.pnl or Decimal("0")
         )
+    # `exit_ts` is optional on the trade type because an OPEN trade has none; `closed_trades`
+    # has already excluded those, so every entry here carries one. Written as a filter rather
+    # than left implicit so the average is taken over exactly the trades that can contribute a
+    # duration -- matching the defensive `trade.pnl or Decimal("0")` two lines up, and keeping
+    # a stray `None` from reaching `Decimal(None - entry_ts)`.
+    hold_spans = [
+        Decimal(t.exit_ts - t.entry_ts) / Decimal(3600)
+        for t in closed_trades
+        if t.exit_ts is not None
+    ]
     avg_hold_hours = (
-        sum(
-            (Decimal(t.exit_ts - t.entry_ts) / Decimal(3600) for t in closed_trades),
-            Decimal("0"),
-        )
-        / len(closed_trades)
-        if closed_trades
-        else Decimal("0")
+        sum(hold_spans, Decimal("0")) / len(hold_spans) if hold_spans else Decimal("0")
     )
 
     return {

keel/commands/insights.py

@@ -310,23 +310,29 @@ def _match_trade(entries_for_rule: list[Any], opened_at: int, closed_at: int) ->
 def _journal_entry_from_outcome(
     row: dict[str, Any], trades_by_rule: dict[str, list[Any]], repo: Repository
 ) -> JournalEntry:
+    # `outcome` is resolved through a separate `matched_outcome` local, and declared `str` here,
+    # so that `JournalEntry.outcome` (a `str`) receives a value the checker agrees is one. Read
+    # straight off `matched` -- which `_match_trade` types `Any` -- it stayed `str | None` all
+    # the way to the constructor even though the fallback below leaves no `None` path.
+    outcome: str
     if row["is_dca"]:
         r_multiple = None
         outcome = "dca"
     else:
         r_multiple = None
-        outcome = None
+        matched_outcome: str | None = None
         rule_name = row["rule_name"]
         if rule_name:
             if rule_name not in trades_by_rule:
                 trades_by_rule[rule_name] = track_record(repo, rule_name).trades
             matched = _match_trade(trades_by_rule[rule_name], row["opened_at"], row["closed_at"])
             if matched is not None:
                 r_multiple = matched.r_multiple
-                outcome = matched.outcome
-        if outcome is None:
+                matched_outcome = matched.outcome
+        if matched_outcome is None:
             pnl = row["pnl_net"]
-            outcome = "win" if pnl > 0 else "loss" if pnl < 0 else "scratch"
+            matched_outcome = "win" if pnl > 0 else "loss" if pnl < 0 else "scratch"
+        outcome = matched_outcome
 
     return JournalEntry(
         closed_at=row["closed_at"],

keel/commands/rules.py

@@ -461,13 +461,19 @@ def rules_seed(
     seeded: list[str] = []
     skipped: list[str] = []
     for kind in kind_list:
-        rule_cls = agent.RULE_REGISTRY[kind]
         for product in product_list:
             label = f"{kind}:{product}"
             if not force and (kind, product) in existing_keys:
                 skipped.append(label)
                 continue
-            rule = rule_cls(product_id=product)
+            # Via `build_rule_from_params` rather than `RULE_REGISTRY[kind](product_id=...)`:
+            # that function is documented as THE `(kind, params)` -> `Rule` boundary, and this
+            # was the one caller reaching around it. With `product_id` as the only param none
+            # of its coercion tables apply, so it calls the very same constructor -- but a rule
+            # kind that later needs coercion for a seeded default gets it here for free. Its
+            # unknown-kind `ValueError` cannot fire: `kind_list` is checked against the registry
+            # above and exits non-zero before this loop.
+            rule = agent.build_rule_from_params(kind, {"product_id": product})
             params = _json_plain(rule.describe()["params"])
             params["product_id"] = product
             repo.insert_rule(kind, params, status=status, now_ts=now_ts)
@@ -515,7 +521,11 @@ def _declared_choices(rule_cls: type) -> dict[str, tuple[Any, ...]]:
     exception: an un-checkable param is the status quo, a crashing `rules add` is not.
     """
     try:
-        hints = get_type_hints(rule_cls.__init__)
+        # `type: ignore[misc]`: mypy rejects reading `__init__` off a value because a subclass
+        # could carry an incompatible one -- which is precisely what is being introspected here.
+        # Reaching for the constructor of whatever concrete rule class was passed IS the job of
+        # this function (see the docstring), so the unsoundness it warns about is the intent.
+        hints = get_type_hints(rule_cls.__init__)  # type: ignore[misc]
     except (NameError, TypeError):  # an unresolvable forward ref, or a slot wrapper __init__
         return {}
 

keel/commands/tui.py

@@ -251,8 +251,13 @@ def _autonomy_lines(report: StatusReport) -> list[ScreenLine]:
     else:
         lines.append(ScreenLine("autonomy: off", "muted"))
     if a.autonomous and not a.live:
-        lapsed_text = f"  (was ON but LAPSED at {_human_dt(a.autonomous_until)})"
-        lines.append(ScreenLine(lapsed_text, "muted"))
+        # An expiry is what makes this branch reachable (autonomy recorded ON, its deadline
+        # passed), so `autonomous_until` is set here in practice. Guarded anyway because the
+        # failure is silent rather than loud: `_human_dt(None)` does not raise --
+        # `time.localtime(None)` means "now" -- so a missing deadline would render as having
+        # lapsed at this very instant, which reads as fact.
+        lapsed_at = _human_dt(a.autonomous_until) if a.autonomous_until is not None else "unknown"
+        lines.append(ScreenLine(f"  (was ON but LAPSED at {lapsed_at})", "muted"))
     elif a.live and a.autonomous_until is not None:
         lines.append(ScreenLine(f"  lapses at {_human_dt(a.autonomous_until)}", "muted"))
     return lines
@@ -313,9 +318,12 @@ def _available_lines(available: AvailableBalance | None) -> list[ScreenLine]:
     if available is None:
         return []
     if available.amount is not None:
+        # `updated_ts` is a separate field from `amount` and can be absent while the amount is
+        # present; same silent-"now" hazard as the autonomy line above, and on a freshness
+        # stamp specifically, where a wrong value is worse than an admitted missing one.
+        as_of = _human_dt(available.updated_ts) if available.updated_ts is not None else "unknown"
         text = (
-            f"live account: {available.amount:,.2f} {available.quote} available  "
-            f"({_human_dt(available.updated_ts)})"
+            f"live account: {available.amount:,.2f} {available.quote} available  ({as_of})"
         )
         return [ScreenLine(text, "ok")]
     return [ScreenLine(f"live account: unavailable -- {available.error}", "warn")]

keel/sim/portfolio_sim.py

@@ -561,7 +561,11 @@ def _process_rule_signals(
     account: SimAccount,
     config: Config,
     latest_price: dict[str, Decimal],
-    held: dict[str, _Held],
+    # Keyed by (asset, rule_name) -- one slot per RULE per asset, matching `run()`'s own `held`
+    # and `_process_dca_signals`. This said `dict[str, _Held]`, which no caller ever passed and
+    # which contradicted both the `(asset, signal.rule_name)` membership test and the assignment
+    # under the same key in this function's body.
+    held: dict[tuple[str, str], _Held],
     now_ts: int,
     monthly_volume_cap: Decimal | None = None,
 ) -> bool:

keel/sim/report.py

@@ -401,7 +401,7 @@ def _would_have_traded_gaps(telemetry: SimTelemetry) -> list[GapItem]:
 
 
 def _data_coverage_gaps(coverage: dict) -> list[GapItem]:
-    gaps = []
+    gaps: list[GapItem] = []
     if not coverage:
         return gaps
 

keel/strategy/backtest.py

@@ -51,7 +51,7 @@
 from dataclasses import dataclass
 from decimal import Decimal
 
-from keel.strategy.rules.base import Rule, Setup, Trade
+from keel.strategy.rules.base import Rule, Setup, Trade, TradeOutcome
 from keel.strategy.stats import BacktestResult, summarize
 from keel.types import Candle, Granularity, Side
 
@@ -181,6 +181,10 @@ def _close_trade(
     risk = (entry_fill - position.setup.stop) * qty
     r_multiple = pnl / risk if risk != 0 else None
 
+    # Annotated rather than inferred: without it the three branches widen `outcome` to plain
+    # `str`, which `Trade.outcome` then rejects. Naming the alias also catches a typo in one of
+    # these literals here, at the assignment, instead of at the constructor call below.
+    outcome: TradeOutcome
     if pnl > 0:
         outcome = "win"
     elif pnl < 0:

keel/strategy/rules/base.py

@@ -71,6 +71,13 @@ class Signal:
     rule_id: int | None = None
 
 
+#: How a trade ended. Named rather than spelled out at each use so the producer
+#: (`backtest._closed_trade`, which picks the branch) and the consumer (`Trade.outcome`) cannot
+#: drift: an inline `Literal[...]` repeated in both places lets one side gain a value the other
+#: silently rejects, and the mismatch only shows up as an `arg-type` error at the constructor.
+type TradeOutcome = Literal["win", "loss", "open", "scratch"]
+
+
 @dataclass
 class Trade:
     """A backtest/paper fill pair (entry + optional exit)."""
@@ -85,7 +92,7 @@ class Trade:
     r_multiple: Decimal | None
     mfe: Decimal
     mae: Decimal
-    outcome: Literal["win", "loss", "open", "scratch"]
+    outcome: TradeOutcome
 
 
 class Rule(ABC):
@@ -106,6 +113,13 @@ class Rule(ABC):
 
     name: str
     params: dict
+    #: The product this rule instance trades. Declared here because it was already a de-facto
+    #: part of the interface: every concrete rule takes it as its first constructor argument and
+    #: stores it (`PullbackContinuation`, `Dca` and `TurtleBreakout` assign it; `RsiMeanReversion`
+    #: carries it as a dataclass field), and `sim.portfolio_sim`/`sim.report` read
+    #: `rule.product_id` off rules they hold only as this base type. Annotation only -- no
+    #: default, exactly like `name`/`params`, so nothing about construction changes.
+    product_id: str
     promotion_class: str = "default"
     rule_id: int | None = None
     #: Why the last `detect()` call declined, or `None` if it fired (or never recorded one).

keel/strategy/rules/turtle_breakout.py

@@ -161,12 +161,18 @@ def __init__(
     # Rule interface
     # ------------------------------------------------------------------
 
-    def _decline(self, gate: str, **numbers: object) -> None:
+    def _decline(self, gate: str, **numbers: object) -> Setup | None:
         """Record WHY this bar declined on `last_rejection`, and return `None` for `detect()`.
 
         Returning `None` (rather than setting and letting the caller `return None`) keeps each
         decline a single line, so the reason can never drift from the branch that produced it.
 
+        The return type is `Setup | None` rather than the `None` this always returns, because
+        all 8 call sites are `return self._decline(...)` and mypy rejects using the value of a
+        `-> None` call at all (`func-returns-value`) -- the annotation names what the CALLER
+        returns, which is what makes the one-line idiom above type-check. It never returns a
+        `Setup`; declining is the only thing it does.
+
         This only ever writes an attribute -- it does NOT log. `detect()` runs once per bar in
         `strategy.backtest` and `sim.portfolio_sim`, so logging here would emit millions of
         events in a sim; `engine.evaluate` reads the attribute and folds it into the one
@@ -346,11 +352,16 @@ def _prior_breakout_won(self, daily: list[Candle]) -> bool:
         warmup = max(entry_lookback, adx_period, atr_period) + 1
 
         won = False
-        pos_entry: Decimal | None = None
-        pos_stop: Decimal | None = None
+        # (entry, stop) as ONE optional pair rather than two independently-optional locals:
+        # they are set together on entry and cleared together on exit, and every read of the
+        # stop happens on a bar where the entry is also live. As two variables that coupling
+        # was invisible -- narrowing `pos_entry is None` told the checker nothing about
+        # `pos_stop`, so the protective-stop comparison below read a `Decimal | None`. Pairing
+        # them makes "stopped without an entry" unrepresentable instead of merely unreachable.
+        position: tuple[Decimal, Decimal] | None = None
         for i in range(warmup, len(tail)):
             c = tail[i]
-            if pos_entry is None:
+            if position is None:
                 # entry mirrors detect(): close > prior-entry_lookback Donchian high, ADX>thr,
                 # optional MACD>0, valid 2N stop.
                 if not float(c.close) > donchian_high(tail[:i], entry_lookback):
@@ -370,15 +381,16 @@ def _prior_breakout_won(self, daily: list[Candle]) -> bool:
                 stop_px = entry_px - stop_mult * atr_i
                 if stop_px >= entry_px:
                     continue
-                pos_entry, pos_stop = entry_px, stop_px
+                position = (entry_px, stop_px)
             else:
                 # manage: 2N stop first (protective), then the asymmetric channel-low exit.
+                pos_entry, pos_stop = position
                 if c.low <= pos_stop:
                     won = False  # stopped out below entry
-                    pos_entry = pos_stop = None
+                    position = None
                 elif float(c.close) <= donchian_low(tail[:i], exit_lookback):
                     won = c.close > pos_entry
-                    pos_entry = pos_stop = None
+                    position = None
 
         self._filter_cache = (last_ts, won)
         return won