fix(research): the edge table, G2 and promotion judge R, not price units (#820)

keel/commands/rules.py

@@ -425,6 +425,11 @@ def backtest_resolved(resolved: ResolvedBacktest) -> backtest_mod.BacktestResult
     )
 
 
+def _r_text(value: Decimal | None) -> str:
+    """An R aggregate for the `rules backtest` line: `n/a` when the sample has no R."""
+    return "n/a" if value is None else str(value)
+
+
 def run_rule_backtest(
     repo: Repository,
     config: Any | None,
@@ -446,10 +451,15 @@ def run_rule_backtest(
     )
     stats = backtest_resolved(resolved)
     sink, recorded = _line_sink(echo)
+    # Every figure names its unit (#820): the R pair is what the promotion gate judges; the
+    # `_px` pair is price units for a one-coin position, which is money, not R, and not
+    # comparable across assets.
     sink(
         f"rule {rule_id} ({resolved.row['kind']}): n_trades={stats.n_trades} "
-        f"win_rate={stats.win_rate:.2%} expectancy={stats.expectancy} "
-        f"profit_factor={stats.profit_factor} max_drawdown={stats.max_drawdown} "
+        f"win_rate={stats.win_rate:.2%} expectancy_r={_r_text(stats.expectancy_r)} "
+        f"max_drawdown_r={_r_text(stats.max_drawdown_r)} "
+        f"profit_factor={stats.profit_factor} expectancy_px={stats.expectancy} "
+        f"max_drawdown_px={stats.max_drawdown} (px = price units, 1-coin notional) "
         f"{_describe_fee(resolved.fee_pct, resolved.fee_source)}"
     )
     return (

keel/sim/portfolio_sim.py

@@ -96,7 +96,7 @@
     _touches,
 )
 from keel.strategy.exit_policy import ExitPolicy, next_stop, policy_for, trailing_atr
-from keel.strategy.rules.base import Rule, Setup, Signal
+from keel.strategy.rules.base import Rule, Setup, Signal, initial_risk_of, r_multiple_of
 from keel.types import Candle, Granularity
 
 __all__ = [
@@ -157,6 +157,10 @@ class SimTrade:
     rule_kind: str
     cts_score: int
     entry_technique: str
+    #: Per-unit `|entry_fill - setup.stop|` against the ORIGINAL stop (#820), the risk
+    #: `r_multiple` divides by (times `qty`). `None` on a record with none; last and
+    #: defaulted so existing constructors keep working.
+    initial_risk: Decimal | None = None
 
 
 @dataclass
@@ -598,8 +602,8 @@ def _process_held(
     # keep the call site honest against `execution.streak.record_closed_trade`'s signature.
     account.record_trade_outcome(pnl, config, current.ts, is_dca=False)
 
-    risk = (h.entry_fill - setup.stop) * h.qty
-    r_multiple = pnl / risk if risk != 0 else None
+    initial_risk = initial_risk_of(h.entry_fill, setup.stop)
+    r_multiple = r_multiple_of(pnl, initial_risk, h.qty)
     outcome = "win" if pnl > 0 else "loss" if pnl < 0 else "scratch"
 
     trades.append(
@@ -618,6 +622,7 @@ def _process_held(
             rule_kind=h.rule.name,
             cts_score=h.cts_score,
             entry_technique=h.entry_technique,
+            initial_risk=initial_risk,
         )
     )
 

keel/sim/report.py

@@ -77,7 +77,7 @@
 )
 from keel.strategy.indicators_cts import DEFAULT_WEIGHTS
 from keel.strategy.promotion import PromotionConfig, check_floors, promotion_class_of
-from keel.strategy.rules.base import Rule
+from keel.strategy.rules.base import Rule, Trade
 from keel.strategy.stats import BacktestResult, summarize
 from keel.types import Candle, Granularity
 
@@ -169,8 +169,9 @@ def edge_table(
     stop/target resolution when the rule's own timeframe is coarser. Results are keyed
     `"{rule.name}:{asset}"` (not bare `rule.name`) so two rules of the same kind bound to
     different assets don't collide. `POOLED_KEY` (`"__pooled__"`) holds
-    `strategy.stats.summarize()` over every rule's trades concatenated -- the pooled sample
-    `build_verdict`'s G2 gate is checked against.
+    `strategy.stats.summarize()` over every rule's trades in EXIT-TIME order
+    (`_chronological`) -- the pooled sample `build_verdict`'s G2 gate is checked against, and
+    whose drawdown must be a path that happened, not one rule's history followed by the next's.
 
     `slippage_by_product` (#259) is passed through to each rule's `backtest()` unchanged; `None`
     (the default) keeps the flat `slippage_pct` for every rule, exactly as before #259. A caller
@@ -209,7 +210,7 @@ def edge_table(
         results[f"{rule.name}:{asset}"] = result
         pooled_trades.extend(result.trades)
 
-    results[POOLED_KEY] = summarize(pooled_trades)
+    results[POOLED_KEY] = summarize(_chronological(pooled_trades))
     return results
 
 
@@ -274,6 +275,18 @@ def accumulation_table(
     return rows
 
 
+def _chronological(trades: list[Trade]) -> list[Trade]:
+    """`trades` in exit-time order, for a pool drawn from several rules (#820).
+
+    A single rule's backtest is already chronological, but a pool concatenated rule by rule is
+    not, and `summarize`'s drawdown and losing streak walk the list in order -- so a
+    concatenated pool reports the drawdown of a sequence that never happened. The sort is
+    stable, and a still-open trade (no exit) sorts last: it is excluded from every aggregate
+    anyway.
+    """
+    return sorted(trades, key=lambda t: (t.exit_ts is None, t.exit_ts or 0))
+
+
 def group_trades_by_class(
     edge: dict[str, BacktestResult], rules: list[Rule]
 ) -> dict[str, BacktestResult]:
@@ -286,14 +299,15 @@ def group_trades_by_class(
     per-rule keys are `"{rule.name}:{asset}"` (the `POOLED_KEY` entry is ignored -- it pools
     across *all* classes and so isn't meaningful per-class). A rule whose edge entry is
     missing is skipped (absent data is a coverage gap, not a crash -- mirrors `edge_table`).
+    Each class's pool is summarised in exit-time order, like `edge_table`'s (`_chronological`).
     """
-    trades_by_class: dict[str, list] = {}
+    trades_by_class: dict[str, list[Trade]] = {}
     for rule in rules:
         result = edge.get(f"{rule.name}:{_asset(rule.product_id)}")
         if result is None:
             continue
         trades_by_class.setdefault(promotion_class_of(rule), []).extend(result.trades)
-    return {cls: summarize(trades) for cls, trades in trades_by_class.items()}
+    return {cls: summarize(_chronological(trades)) for cls, trades in trades_by_class.items()}
 
 
 # ---------------------------------------------------------------------------
@@ -773,13 +787,15 @@ def _render_edge_section(
     lines = [
         "## Edge table",
         "",
-        "Per-rule and pooled backtest stats (unit-less R-multiples). "
-        f"`{POOLED_KEY}` is the pooled sample G2 is checked against.",
+        "Per-rule and pooled backtest stats in R-multiples: each trade's net P&L over the risk "
+        "it carried (|entry fill - stop| x qty), so trades at any price pool on one scale. "
+        f"`{POOLED_KEY}` is every rule's trades in exit-time order -- the pooled sample G2 is "
+        "checked against, in R.",
         "",
         *cost_lines,
         "",
-        "| Rule | N | Win% | Expectancy | Avg win | Avg loss | Profit factor | Max DD | "
-        "Losing streak | Avg MFE | Avg MAE |",
+        "| Rule | N | Win% | Expectancy (R) | Avg win (R) | Avg loss (R) | Profit factor (R) | "
+        "Max DD (R) | Losing streak | Avg MFE (R) | Avg MAE (R) |",
         "|---|---|---|---|---|---|---|---|---|---|---|",
     ]
     ordered_keys = [key for key in edge if key != POOLED_KEY]
@@ -789,11 +805,19 @@ def _render_edge_section(
         result = edge[key]
         label = f"**{key}**" if key == POOLED_KEY else key
         lines.append(
-            f"| {label} | {result.n_trades} | {result.win_rate:.1%} | {result.expectancy} | "
-            f"{result.avg_win} | {result.avg_loss} | {result.profit_factor} | "
-            f"{result.max_drawdown} | {result.max_losing_streak} | {result.avg_mfe} | "
-            f"{result.avg_mae} |"
+            f"| {label} | {result.n_trades} | {result.win_rate:.1%} | "
+            f"{_r_cell(result.expectancy_r)} | {_r_cell(result.avg_win_r)} | "
+            f"{_r_cell(result.avg_loss_r)} | {_r_cell(result.profit_factor_r)} | "
+            f"{_r_cell(result.max_drawdown_r)} | {result.max_losing_streak} | "
+            f"{_r_cell(result.avg_mfe_r)} | {_r_cell(result.avg_mae_r)} |"
         )
+    excluded = [
+        f"{key} {edge[key].n_excluded_no_risk} of {edge[key].n_trades} trades"
+        for key in ordered_keys
+        if edge[key].n_excluded_no_risk
+    ]
+    if excluded:
+        lines.extend(["", f"Excluded from R (no initial risk recorded): {', '.join(excluded)}."])
     return lines
 
 
@@ -826,6 +850,16 @@ def _render_accumulation_section(accumulation: dict[str, DcaSleeve]) -> list[str
     ]
 
 
+def _r_cell(value: Decimal | None) -> str:
+    """An R aggregate for the edge table: `n/a` when the sample had no R (never a 0 that
+    reads as a measured flat edge), `inf` for a profit factor with no losing R."""
+    if value is None:
+        return "n/a"
+    if value.is_infinite():
+        return "inf"
+    return f"{value:.3f}"
+
+
 def _render_account_section(account_metrics: dict, slippage_rows=None) -> list[str]:
     lines = ["## Account results", "", "| Metric | Value |", "|---|---|"]
     for key, label in _ACCOUNT_METRIC_LABELS:

keel/strategy/backtest.py

@@ -72,7 +72,14 @@
 from decimal import Decimal
 
 from keel.strategy.exit_policy import ExitPolicy, next_stop, policy_for, trailing_atr
-from keel.strategy.rules.base import Rule, Setup, Trade, TradeOutcome
+from keel.strategy.rules.base import (
+    Rule,
+    Setup,
+    Trade,
+    TradeOutcome,
+    initial_risk_of,
+    r_multiple_of,
+)
 from keel.strategy.stats import BacktestResult, summarize
 from keel.types import Candle, Granularity, Side
 
@@ -394,8 +401,10 @@ def _close_trade(
     exit_fee = exit_fill * qty * fee_pct
     pnl = (exit_fill - entry_fill) * qty - entry_fee - exit_fee
 
-    risk = (entry_fill - position.setup.stop) * qty
-    r_multiple = pnl / risk if risk != 0 else None
+    # The ORIGINAL stop (`setup.stop`), never the managed `position.stop`, and as a magnitude:
+    # a fill that gapped below the stop still risked the distance to it (#820).
+    initial_risk = initial_risk_of(entry_fill, position.setup.stop)
+    r_multiple = r_multiple_of(pnl, initial_risk, qty)
 
     # 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
@@ -420,6 +429,7 @@ def _close_trade(
         mfe=position.mfe,
         mae=position.mae,
         outcome=outcome,
+        initial_risk=initial_risk,
     )
 
 
@@ -436,6 +446,7 @@ def _open_trade(position: _OpenPosition) -> Trade:
         mfe=position.mfe,
         mae=position.mae,
         outcome="open",
+        initial_risk=initial_risk_of(position.entry_fill, position.setup.stop),
     )
 
 

keel/strategy/paper.py

@@ -40,7 +40,7 @@
 
 from keel.data.repository import Repository
 from keel.strategy.backtest import TAKER_FEE_PCT, _stop_exit_price, _touches
-from keel.strategy.rules.base import Action, Setup, Signal, Trade
+from keel.strategy.rules.base import Action, Setup, Signal, Trade, initial_risk_of, r_multiple_of
 from keel.strategy.stats import BacktestResult, summarize
 from keel.types import Candle, Side
 
@@ -396,8 +396,8 @@ def _close(self, position: _OpenPaperPosition, exit_price: Decimal, exit_ts: int
         exit_fee = exit_fill * position.qty * self._fee_pct
         pnl = (exit_fill - position.entry_fill) * position.qty - entry_fee - exit_fee
 
-        risk = (position.entry_fill - position.setup.stop) * position.qty
-        r_multiple = pnl / risk if risk != 0 else None
+        initial_risk = initial_risk_of(position.entry_fill, position.setup.stop)
+        r_multiple = r_multiple_of(pnl, initial_risk, position.qty)
 
         if pnl > 0:
             outcome = "win"
@@ -415,6 +415,7 @@ def _close(self, position: _OpenPaperPosition, exit_price: Decimal, exit_ts: int
             "qty": str(position.qty),
             "pnl": str(pnl),
             "r_multiple": str(r_multiple) if r_multiple is not None else None,
+            "initial_risk": str(initial_risk),
             "mfe": str(position.mfe),
             "mae": str(position.mae),
             "outcome": outcome,
@@ -447,6 +448,22 @@ def _close(self, position: _OpenPaperPosition, exit_price: Decimal, exit_ts: int
         return order_id
 
 
+def _journalled_initial_risk(exit_payload: dict, entry_payload: dict | None) -> Decimal | None:
+    """The per-unit initial risk a journalled paper trade carried (#820).
+
+    An exit written since #820 records it (`"initial_risk"`). One written before does not --
+    but its ENTRY payload has always recorded the achieved fill (`"entry"`) and the setup's
+    stop (`"stop"`), which is all the risk is, so an older track record keeps its R rather
+    than silently dropping out of every R aggregate. `None` only when neither is available.
+    """
+    recorded = exit_payload.get("initial_risk")
+    if recorded is not None:
+        return Decimal(recorded)
+    if entry_payload is not None and entry_payload.get("stop") is not None:
+        return initial_risk_of(Decimal(entry_payload["entry"]), Decimal(entry_payload["stop"]))
+    return None
+
+
 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
@@ -470,23 +487,33 @@ def track_record(repo: Repository, rule_name: str) -> BacktestResult:
 
     trades: list[Trade] = []
     for payload in exits:
-        r_multiple = payload["r_multiple"]
+        entry_payload = entries.pop(payload["entry_order_id"], None)
+        initial_risk = _journalled_initial_risk(payload, entry_payload)
+        pnl = Decimal(payload["pnl"])
+        qty = Decimal(payload["qty"])
+        stored_r = payload["r_multiple"]
         trades.append(
             Trade(
                 entry_ts=payload["entry_ts"],
                 exit_ts=payload["exit_ts"],
                 entry=Decimal(payload["entry"]),
                 exit=Decimal(payload["exit"]),
-                qty=Decimal(payload["qty"]),
+                qty=qty,
                 side=Side.BUY,
-                pnl=Decimal(payload["pnl"]),
-                r_multiple=Decimal(r_multiple) if r_multiple is not None else None,
+                pnl=pnl,
+                # Recomputed whenever the risk is known: a pre-#820 exit stored the SIGNED
+                # formula's value, which is wrong for any fill that gapped below its stop.
+                r_multiple=(
+                    r_multiple_of(pnl, initial_risk, qty)
+                    if initial_risk is not None
+                    else (Decimal(stored_r) if stored_r is not None else None)
+                ),
                 mfe=Decimal(payload["mfe"]),
                 mae=Decimal(payload["mae"]),
                 outcome=payload["outcome"],
+                initial_risk=initial_risk,
             )
         )
-        entries.pop(payload["entry_order_id"], None)
 
     for payload in entries.values():
         trades.append(
@@ -502,6 +529,7 @@ def track_record(repo: Repository, rule_name: str) -> BacktestResult:
                 mfe=Decimal(0),
                 mae=Decimal(0),
                 outcome="open",
+                initial_risk=_journalled_initial_risk({}, payload),
             )
         )