insights: one track record per rule; empty side reads unknown (#962, #965)

docs/operator-runbook.md

@@ -875,7 +875,11 @@ cancel` and the exit path stay at the terminal.
 **Rules.** Everything keel knows about its rules, on one page, and nothing on it is a control:
 promotion happens in the CLI. The rule ledger and each rule's parameters come first, then the
 track records of the rules in the promotion pipeline (candidate and paper) with each paper rule's
-distance to the promotion gate -- unsorted, because a sorted record is a leaderboard -- then the
+distance to the promotion gate -- unsorted, because a sorted record is a leaderboard. There is one
+record per rule, named by kind, product and ledger id, holding the paper trades that rule journalled;
+paper trades journalled with no rule id get one record per kind with no id and no gate. An average
+win or loss with no trade on its side reads "—" (`n/a` in `keel insights summary`), not $0.00, and so
+does the realized rr with no win or no loss yet, and every figure of a rule with no closed trade. Then the
 DCA plan proposal (`/rules?budget=500&buffer=0.1` fills it; it is applied by the `keel dca plan`
 command it shows, at a terminal), then the research record: every trial, the evidence matrix, the
 gauntlet and the slippage assumptions. Track records, promotion-gate distances and the research

keel/commands/insights.py

@@ -29,7 +29,8 @@
 or writes it -- it is built entirely off `trade_outcomes` (see `Repository.get_trade_outcomes`).
 
 **On `--mode`:** `paper.track_record` is inherently paper-mode/R-aware regardless of a rule's
-current lifecycle `status` (it just replays `orders(mode='paper')` for a given rule name), so
+current lifecycle `status` (it just replays `orders(mode='paper')` -- per rule, by
+`orders.rule_id`, since #962 -- see `paper.track_records_by_rule`), so
 `--mode` does not change *how* stats are computed -- it selects *which* rules the report scopes
 to: `paper` (the default) shows rules still in the proving pipeline (`candidate`/`paper` status,
 i.e. "how close to promotion"); `live` shows rules already promoted and trading (`live` status,
@@ -54,14 +55,15 @@
 from keel.commands.status import StatusReport, _human_age, gather_status
 from keel.config import Config
 from keel.data.repository import Repository
-from keel.strategy.paper import track_record
+from keel.strategy.paper import track_record, track_records_by_rule
 from keel.strategy.promotion import (
     PromotionConfig,
     check_floors,
     floor_for_class,
     promotion_class_of,
 )
 from keel.strategy.rules.base import Rule
+from keel.strategy.stats import summarize
 
 # -- the pure report shapes -----------------------------------------------------------------
 
@@ -604,13 +606,28 @@ def min_win_rate_pct(self) -> Decimal:
 
 @dataclass(frozen=True)
 class RuleTrackRecord:
+    """One rule's paper track record (#962: one per ledger ROW, not per kind).
+
+    `rule_name` is the rule's KIND (the `rules` table has no name column; `promotion` says why),
+    so it alone cannot tell two rules of one kind apart -- `rule_id` and `product_id` do. A record
+    with `rule_id=None` and `status="unattributed"` holds the kind's paper trades journalled with
+    no `orders.rule_id`: they belong to no one rule, and are shown rather than dropped or folded
+    into a rule that shares the kind (`build_insights_report`).
+    """
+
     rule_name: str
+    rule_id: int | None
+    product_id: str | None
     status: str
     promotion_class: str
     n_trades: int
     win_rate: float
-    avg_win: Decimal
-    avg_loss: Decimal
+    avg_win: Decimal | None
+    """`None` when the sample has no winning trade (#965). `BacktestResult` holds 0 there, a
+    figure `promotion.pool_stats` multiplies by a zero count; as a reading it is unknown, not
+    $0.00, so the report says so and the CLI prints "n/a"."""
+    avg_loss: Decimal | None
+    """`None` when the sample has no losing trade -- `avg_win`'s rule, on the loss side."""
     realized_rr: Decimal | None
     expectancy: Decimal
     profit_factor: Decimal
@@ -670,16 +687,20 @@ def build_account_summary(report: StatusReport) -> AccountSummary:
 
 
 def _realized_rr_for_display(stats: Any) -> Decimal | None:
-    """`avg_win / |avg_loss|`, or `None` when there are no losing trades yet.
-
-    Deliberately diverges from `strategy.promotion._realized_rr` (which returns `Infinity` in
-    that case, because a promotion-gate check needs a comparable scalar). This is a display
-    value, not a gate decision -- `None` ("no losses yet, nothing to measure against") is the
-    honest reading, not a real-looking `Infinity`.
+    """`avg_win / |avg_loss|`, or `None` when either side is empty: no losing trades yet (nothing
+    to measure against) or no winning trades yet (#965: `avg_win` is then the 0 `BacktestResult`
+    writes for an empty side, and `0 / |avg_loss|` would read as a measured 0.00).
+
+    Deliberately diverges from `strategy.promotion._realized_rr` (which returns `Infinity` for no
+    losses, because a promotion-gate check needs a comparable scalar, and works in R). This is a
+    display value, not a gate decision -- `check_floors` still decides the verdict. Each side is
+    decided by COUNTING that side's trades, like `_side_average`.
     """
-    if stats.avg_loss == 0:
+    avg_win = _side_average(stats, "win", stats.avg_win)
+    avg_loss = _side_average(stats, "loss", stats.avg_loss)
+    if avg_win is None or avg_loss is None:
         return None
-    return stats.avg_win / abs(stats.avg_loss)
+    return avg_win / abs(avg_loss)
 
 
 def build_gate_distance(rule: Rule, stats: Any, default_floor: PromotionConfig) -> GateDistance:
@@ -719,34 +740,54 @@ def _human_dt(ts: int) -> str:
     return time.strftime("%Y-%m-%d %H:%M:%S", time.localtime(ts))
 
 
+def _side_average(stats: Any, outcome: str, average: Decimal) -> Decimal | None:
+    """`average` (the sample's `avg_win` or `avg_loss`), or `None` when no closed trade in the
+    sample has `outcome` (#965). Decided by COUNTING that side's trades, not by reading the
+    average as zero: `BacktestResult` writes 0 for an empty side, and 0 is a value a reader
+    takes as measured."""
+    if not any(trade.outcome == outcome for trade in stats.trades):
+        return None
+    return average
+
+
+def _rule_and_class(row: dict[str, Any]) -> tuple[Rule | None, str]:
+    """`row`'s built rule and promotion class -- `(None, "default")` when its kind is no longer
+    in `RULE_REGISTRY` (`agent._build_rule` raises `ValueError` for that)."""
+    try:
+        rule = agent_mod._build_rule(row)
+    except ValueError:
+        return None, "default"
+    return rule, promotion_class_of(rule)
+
+
 def build_rule_track_record(
     row: dict[str, Any], stats: Any, default_floor: PromotionConfig
 ) -> RuleTrackRecord:
     """Aggregate `row` (a `repo.get_rules()` row) + its paper `stats` into a `RuleTrackRecord`.
 
+    `stats` is THIS rule's sample -- `track_records_by_rule(repo, kind)[row["id"]]` -- and the
+    gate is judged on it, so a gate card is one rule's distance, like its record (#962).
+
     Degrades gracefully -- promotion_class="default", gate=None -- when `row["kind"]` is no
-    longer in `RULE_REGISTRY` (`agent._build_rule` raises `ValueError` for that), rather than
-    crashing the whole report over one stale row.
+    longer in `RULE_REGISTRY`, rather than crashing the whole report over one stale row.
     """
-    try:
-        rule = agent_mod._build_rule(row)
-        promotion_class = promotion_class_of(rule)
-    except ValueError:
-        rule = None
-        promotion_class = "default"
+    rule, promotion_class = _rule_and_class(row)
 
     gate: GateDistance | None = None
     if row["status"] == "paper" and rule is not None:
         gate = build_gate_distance(rule, stats, default_floor)
 
+    product_id = (row.get("params") or {}).get("product_id")
     return RuleTrackRecord(
         rule_name=row["kind"],
+        rule_id=int(row["id"]),
+        product_id=str(product_id) if product_id is not None else None,
         status=row["status"],
         promotion_class=promotion_class,
         n_trades=stats.n_trades,
         win_rate=stats.win_rate,
-        avg_win=stats.avg_win,
-        avg_loss=stats.avg_loss,
+        avg_win=_side_average(stats, "win", stats.avg_win),
+        avg_loss=_side_average(stats, "loss", stats.avg_loss),
         realized_rr=_realized_rr_for_display(stats),
         expectancy=stats.expectancy,
         profit_factor=stats.profit_factor,
@@ -756,6 +797,39 @@ def build_rule_track_record(
     )
 
 
+#: The `status` of the record that holds a kind's trades journalled with no rule id.
+UNATTRIBUTED = "unattributed"
+
+
+def build_unattributed_track_record(kind_row: dict[str, Any], stats: Any) -> RuleTrackRecord:
+    """The record of `kind_row`'s KIND's paper trades journalled with no `orders.rule_id`.
+
+    They belong to no one rule: no id, no product, no gate (a gate is one rule's distance to
+    promotion, and these trades are no rule's evidence). The promotion class is a property of
+    the kind, so it is read off `kind_row`, a rule of that kind in scope. Shown rather than
+    dropped -- a hidden trade is a gap in the record nobody can see -- and never folded into a
+    rule that shares the kind, which would hand that rule another's trades (#962).
+    """
+    _rule, promotion_class = _rule_and_class(kind_row)
+    return RuleTrackRecord(
+        rule_name=kind_row["kind"],
+        rule_id=None,
+        product_id=None,
+        status=UNATTRIBUTED,
+        promotion_class=promotion_class,
+        n_trades=stats.n_trades,
+        win_rate=stats.win_rate,
+        avg_win=_side_average(stats, "win", stats.avg_win),
+        avg_loss=_side_average(stats, "loss", stats.avg_loss),
+        realized_rr=_realized_rr_for_display(stats),
+        expectancy=stats.expectancy,
+        profit_factor=stats.profit_factor,
+        max_drawdown=stats.max_drawdown,
+        significant=stats.n_trades >= 30,
+        gate=None,
+    )
+
+
 _PAPER_PIPELINE_STATUSES = {"candidate", "paper"}
 
 
@@ -785,9 +859,20 @@ def build_insights_report(
     else:
         rows = [r for r in rows if r["status"] in _PAPER_PIPELINE_STATUSES]
 
+    # One read of the paper journal per KIND, split by the rule that journalled each trade (#962).
+    by_kind = {
+        kind: track_records_by_rule(repo, kind) for kind in dict.fromkeys(r["kind"] for r in rows)
+    }
+    no_trades = summarize([])
     rules = [
-        build_rule_track_record(row, track_record(repo, row["kind"]), default_floor) for row in rows
+        build_rule_track_record(row, by_kind[row["kind"]].get(row["id"], no_trades), default_floor)
+        for row in rows
     ]
+    # Trades journalled with no rule id, after the rules: one record per kind in scope.
+    for kind, partitions in by_kind.items():
+        if None in partitions:
+            first_row = next(r for r in rows if r["kind"] == kind)
+            rules.append(build_unattributed_track_record(first_row, partitions[None]))
 
     return InsightsReport(
         now_ts=now_ts,
@@ -956,6 +1041,8 @@ def build_journal_report(
     "proven edge"
 )
 
+_UNATTRIBUTED_NOTE = "trades journalled with no rule id -- they belong to no one rule of this kind"
+
 _TWO_DP = Decimal("0.01")
 
 
@@ -965,8 +1052,9 @@ def _quantized(x: Any) -> Any:
 
     Passes through anything that isn't a finite `Decimal` unchanged: a sentinel string
     ("n/a (no losses yet)", "n/a", "dca"), `None`, or a non-finite `Decimal` (`Infinity`,
-    which `BacktestResult.profit_factor`/a zero-loss realized-rr can legitimately be, and
-    which `.quantize()` itself refuses to round) all render exactly as before.
+    which `BacktestResult.profit_factor` can legitimately be, and which `.quantize()` itself
+    refuses to round) all render exactly as before. The display realized rr is never
+    `Infinity`: with no losing (or no winning) trades it is `None`.
     """
     if isinstance(x, Decimal) and x.is_finite():
         return x.quantize(_TWO_DP)
@@ -1010,13 +1098,33 @@ def render_summary(report: InsightsReport) -> list[str]:
 
     lines.append(f"rule track record ({len(report.rules)}):")
     for r in report.rules:
-        rr = r.realized_rr if r.realized_rr is not None else "n/a (no losses yet)"
+        if r.n_trades == 0:
+            # Nothing was measured: every figure is unknown, not the zero the engine holds.
+            win_rate = expectancy = profit_factor = max_dd = "n/a"
+            rr: Any = "n/a (no trades yet)"
+        else:
+            win_rate = f"{r.win_rate:.1%}"
+            expectancy = _money(r.expectancy)
+            profit_factor = _ratio(r.profit_factor)
+            max_dd = _money(r.max_drawdown)
+            if r.realized_rr is not None:
+                rr = r.realized_rr
+            elif r.avg_win is None:
+                rr = "n/a (no wins yet)"
+            else:
+                rr = "n/a (no losses yet)"
+        rule_id = r.rule_id if r.rule_id is not None else "n/a"
+        avg_win = _money(r.avg_win) if r.avg_win is not None else "n/a"
+        avg_loss = _money(r.avg_loss) if r.avg_loss is not None else "n/a"
         lines.append(
-            f"  [{r.rule_name}] status={r.status} class={r.promotion_class} n={r.n_trades} "
-            f"win_rate={r.win_rate:.1%} avg_win={_money(r.avg_win)} avg_loss={_money(r.avg_loss)} "
-            f"rr={_ratio(rr)} expectancy={_money(r.expectancy)} "
-            f"profit_factor={_ratio(r.profit_factor)} max_dd={_money(r.max_drawdown)}"
+            f"  [{r.rule_name}] rule={rule_id} product={r.product_id or 'n/a'} "
+            f"status={r.status} class={r.promotion_class} n={r.n_trades} "
+            f"win_rate={win_rate} avg_win={avg_win} avg_loss={avg_loss} "
+            f"rr={_ratio(rr)} expectancy={expectancy} "
+            f"profit_factor={profit_factor} max_dd={max_dd}"
         )
+        if r.status == UNATTRIBUTED:
+            lines.append(f"    {_UNATTRIBUTED_NOTE}")
         if not r.significant:
             lines.append(f"    {_SMALL_SAMPLE_NOTE}")
         if r.gate is not None:

keel/strategy/paper.py

@@ -468,18 +468,56 @@ def track_record(repo: Repository, rule_name: str) -> BacktestResult:
     """Aggregate `rule_name`'s paper trades (from `orders(mode='paper')`) into a
     `BacktestResult`-shaped summary, directly comparable to `backtest.backtest()`'s
     historical output.
+
+    `rule_name` is a rule KIND (`rules.kind`), so this pools every rule of that kind -- all
+    products, every lifecycle status. That pool is what `insights`' journal matches a closed
+    trade's R against (it has the kind, not the rule). A per-RULE record is
+    `track_records_by_rule`'s (#962).
     """
-    orders = repo.get_orders(mode="paper")
+    return summarize(_journalled_trades(_paper_payloads(repo, rule_name)))
 
-    entries: dict[int, dict] = {}
-    exits: list[dict] = []
-    for order in orders:
+
+def track_records_by_rule(repo: Repository, rule_name: str) -> dict[int | None, BacktestResult]:
+    """`track_record(repo, rule_name)` split by the rule that journalled each trade (#962).
+
+    Keyed by `orders.rule_id`: a real paper cycle writes it on the entry, and the exit copies
+    the entry's (see `_OpenPaperPosition.rule_id`), so a round trip never straddles two keys.
+    A fill written with NO `rule_id` (a hand-built signal, or a fill journalled before the
+    column was populated) is keyed `None` -- it belongs to no rule, and folding it into
+    whichever rule shares its kind would hand one rule another's trades. Only keys with at
+    least one journalled order appear; a rule that never traded has no entry here, and its
+    record is the empty sample, which the caller builds.
+
+    The partitions are disjoint and cover the kind: their `n_trades` sum to
+    `track_record(repo, rule_name).n_trades`. Path-dependent figures (drawdown, losing streak)
+    are each rule's own, over its own trades in journal order.
+    """
+    partitions: dict[int | None, list[tuple[dict, dict]]] = {}
+    for order, payload in _paper_payloads(repo, rule_name):
+        key = order.get("rule_id")
+        partitions.setdefault(int(key) if key is not None else None, []).append((order, payload))
+    return {key: summarize(_journalled_trades(rows)) for key, rows in partitions.items()}
+
+
+def _paper_payloads(repo: Repository, rule_name: str) -> list[tuple[dict, dict]]:
+    """Every journalled paper order of kind `rule_name`, with its decoded payload, oldest first."""
+    found: list[tuple[dict, dict]] = []
+    for order in repo.get_orders(mode="paper"):
         raw = order["raw_response"]
         if not raw:
             continue
         payload = json.loads(raw)
         if payload.get("rule_name") != rule_name:
             continue
+        found.append((order, payload))
+    return found
+
+
+def _journalled_trades(rows: list[tuple[dict, dict]]) -> list[Trade]:
+    """Pair entry and exit payloads into `Trade`s; an entry with no exit is an open trade."""
+    entries: dict[int, dict] = {}
+    exits: list[dict] = []
+    for order, payload in rows:
         if payload.get("role") == "entry":
             entries[order["id"]] = payload
         elif payload.get("role") == "exit":
@@ -533,4 +571,4 @@ def track_record(repo: Repository, rule_name: str) -> BacktestResult:
             )
         )
 
-    return summarize(trades)
+    return trades

keel/web/payload.py

@@ -1148,10 +1148,20 @@ def _track_record_payload(record: RuleTrackRecord) -> dict[str, Any]:
     """One rule's track record. At zero closed trades every statistic is UNKNOWN, not zero: the
     sample is empty, so there is no win rate to be 0% and no average to be $0.00 (the report holds
     0 because `BacktestResult` does; serialising that as data would assert a measurement). The
-    trade count stays a real number. Win rate is in percent units (see `_gate_payload`)."""
+    trade count stays a real number. Win rate is in percent units (see `_gate_payload`).
+
+    One side of a TRADED sample can be empty too -- all wins, so no average loss (#965). The
+    report says so itself (`avg_win`/`avg_loss` are `None`), and `money(None)` is `absent()`:
+    nothing here works the empty side out from the win rate and the count.
+
+    `rule_name` is the rule's KIND; `rule_id` and `product_id` are what tell two rules of one kind
+    apart (#962). Identifiers, so bare strings like `_rule_row_payload`'s `id`; `None` on the
+    record of trades journalled with no rule id, which has neither."""
     traded = record.n_trades != 0
     return {
         "rule_name": record.rule_name,
+        "rule_id": str(record.rule_id) if record.rule_id is not None else None,
+        "product_id": record.product_id,
         "status": label(record.status),
         "promotion_class": record.promotion_class,
         "n_trades": count(record.n_trades),