From 049c0edd1c2be2d0e6358b5cb0db36c04ef0467f Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Sat, 26 Sep 2026 15:00:00 -0400 Subject: [PATCH] fix(research): the edge table, G2 and promotion judge R, not price units (#820) Per-trade R was stored but never aggregated: `stats.summarize` built expectancy, avg win/loss, profit factor, max drawdown and MFE/MAE from pnl in price units (1-coin notional), and the edge table printed them as "unit-less R-multiples". Pools across assets were decided by the highest-priced one, and G2 and #338's pooled promotion read the same numbers. The R denominator was also signed, so a fill that gapped below its stop turned a loss into a positive R. - Trade/SimTrade carry `initial_risk` (per unit, |entry_fill - stop| against the ORIGINAL stop); one shared helper computes R at all three close paths (backtest, paper, portfolio_sim). - BacktestResult gains R twins (expectancy_r, avg_win_r, avg_loss_r, profit_factor_r, max_drawdown_r, avg_mfe_r, avg_mae_r) plus the excluded-for-no-risk count; money fields are unchanged. - The edge table's pooled and per-class samples are sorted by exit time; the table renders R and says so. - check_floors, _realized_rr, pool_stats, _pooled_floors and should_demote judge R; no R at all refuses (demotes, for should_demote). - paper.track_record recovers initial_risk for pre-#820 exits from the entry payload and recomputes their R. - `rules backtest` labels its units. Closes #820 Co-Authored-By: Claude Opus 5.5 --- keel/commands/rules.py | 14 +- keel/sim/portfolio_sim.py | 11 +- keel/sim/report.py | 62 ++++-- keel/strategy/backtest.py | 17 +- keel/strategy/paper.py | 44 ++++- keel/strategy/promotion.py | 154 ++++++++++++--- keel/strategy/rules/base.py | 31 +++ keel/strategy/stats.py | 101 +++++++++- tests/commands/test_insights.py | 58 +++++- tests/commands/test_rules_services.py | 31 +++ tests/sim/test_portfolio_sim.py | 64 +++++++ tests/sim/test_report.py | 260 +++++++++++++++++++++++++- tests/strategy/test_backtest.py | 86 +++++++++ tests/strategy/test_paper.py | 104 +++++++++++ tests/strategy/test_promotion.py | 194 +++++++++++++++++++ tests/strategy/test_stats.py | 174 +++++++++++++++++ tests/test_cli.py | 17 +- 17 files changed, 1361 insertions(+), 61 deletions(-) diff --git a/keel/commands/rules.py b/keel/commands/rules.py index 3b55b2ff..93b38ce0 100644 --- a/keel/commands/rules.py +++ b/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 ( diff --git a/keel/sim/portfolio_sim.py b/keel/sim/portfolio_sim.py index bb58ffe8..4b94a581 100644 --- a/keel/sim/portfolio_sim.py +++ b/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, ) ) diff --git a/keel/sim/report.py b/keel/sim/report.py index 58a60bdd..dea72229 100644 --- a/keel/sim/report.py +++ b/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: diff --git a/keel/strategy/backtest.py b/keel/strategy/backtest.py index 51be3415..c7769681 100644 --- a/keel/strategy/backtest.py +++ b/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), ) diff --git a/keel/strategy/paper.py b/keel/strategy/paper.py index 971310c1..d631d95b 100644 --- a/keel/strategy/paper.py +++ b/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), ) ) diff --git a/keel/strategy/promotion.py b/keel/strategy/promotion.py index f74ee782..8c6cdeb6 100644 --- a/keel/strategy/promotion.py +++ b/keel/strategy/promotion.py @@ -33,6 +33,14 @@ judges the parameter SELECTION (the trial matrix behind `--pbo-session`), which is per-parameter-set evidence already, and this change does not alter its scope. +**The floors are judged in R (#820).** Expectancy and realized R:R are read off the +`BacktestResult`'s `*_r` fields -- each trade's net P&L over the risk it carried -- never off +its price-unit fields. In price units a pool of assets at 100000 and at 0.1 is decided by the +first, so a single high-priced product set the sign of G2 and of the #338 pooled reading. +`min_expectancy` is therefore an R threshold. A sample with no R at all (no trade recorded an +initial risk) cannot be judged on those axes and is REFUSED, the same fail-closed rule an +unrun G4 gets below; `min_trades` and `min_win_rate` are counts and are judged as before. + **Rules-table access:** `data/repository.py`'s `Repository` exposes typed `rules`-table methods (`insert_rule`/`get_rules`/`update_rule_status`, P3 Task 1); this module drives the lifecycle transitions purely through that surface. The `rules` table (see @@ -44,7 +52,7 @@ from __future__ import annotations from collections.abc import Sequence -from dataclasses import dataclass +from dataclasses import dataclass, replace from decimal import Decimal from typing import Any @@ -76,7 +84,11 @@ def next_status(status: str) -> str | None: @dataclass class PromotionConfig: - """Performance floors a rule's stats must clear to promote (spec §11/§4.5).""" + """Performance floors a rule's stats must clear to promote (spec §11/§4.5). + + `min_expectancy` is in R (per trade) and `min_rr` is a ratio of mean R -- see the + module docstring's "judged in R" note (#820). + """ min_trades: int = 100 min_expectancy: Decimal = Decimal("0") @@ -138,16 +150,36 @@ def promotion_class_of(rule: object) -> str: return getattr(rule, "promotion_class", DEFAULT_CLASS) -def _realized_rr(stats: BacktestResult) -> Decimal: - """Realized reward:risk = avg win / avg loss magnitude. +def _realized_rr(stats: BacktestResult) -> Decimal | None: + """Realized reward:risk = avg win / avg loss magnitude, IN R (#820). - `avg_loss` is stored as a non-positive `Decimal` (it's the mean pnl of losing + `avg_loss_r` is stored as a non-positive `Decimal` (it's the mean R of losing trades); a rule with no losing trades yet has no realized risk to measure against, - so it is treated as clearing any rr floor. + so it is treated as clearing any rr floor. `None` when the sample has no R at all -- + the caller refuses rather than guessing. + + In R, not price units: a price-unit ratio mixes a BTC win with an XLM loss, and one + asset's price then decides the ratio. (`insights._realized_rr_for_display` is the money + ratio, deliberately separate -- it is a display value, not a gate decision.) """ - if stats.avg_loss == 0: + if stats.avg_win_r is None or stats.avg_loss_r is None: + return None + if stats.avg_loss_r == 0: return Decimal("Infinity") - return stats.avg_win / abs(stats.avg_loss) + return stats.avg_win_r / abs(stats.avg_loss_r) + + +def _no_r_reason(n_trades: int, *, pooled: bool = False) -> str: + """The refusal a sample with no R gets on the expectancy and R:R axes (#820).""" + if pooled: + return ( + f"pooled no R: none of the {n_trades} pooled trades carries an initial risk, so " + "expectancy and R:R cannot be judged in R -- refusing rather than passing" + ) + return ( + f"no R: none of the {n_trades} closed trades carries an initial risk, so expectancy " + "and R:R cannot be judged in R -- refusing rather than passing" + ) def check_floors(stats: BacktestResult, cfg: PromotionConfig) -> tuple[bool, list[str]]: @@ -169,12 +201,18 @@ def check_floors(stats: BacktestResult, cfg: PromotionConfig) -> tuple[bool, lis if stats.n_trades < cfg.min_trades: reasons.append(f"n_trades {stats.n_trades} < min_trades {cfg.min_trades}") - if stats.expectancy <= cfg.min_expectancy: - reasons.append(f"expectancy {stats.expectancy} <= min_expectancy {cfg.min_expectancy}") - + # Expectancy and R:R are judged in R (#820). No R at all is a REFUSAL, not a pass -- + # "nobody measured" must never read as "measured and fine" (see `can_promote`). rr = _realized_rr(stats) - if rr < cfg.min_rr: - reasons.append(f"rr {rr} < min_rr {cfg.min_rr}") + if stats.expectancy_r is None or rr is None: + reasons.append(_no_r_reason(stats.n_trades)) + else: + if stats.expectancy_r <= cfg.min_expectancy: + reasons.append( + f"expectancy_r {stats.expectancy_r} <= min_expectancy {cfg.min_expectancy} (in R)" + ) + if rr < cfg.min_rr: + reasons.append(f"rr_r {rr} < min_rr {cfg.min_rr} (in R)") if stats.win_rate < cfg.min_win_rate: reasons.append(f"win_rate {stats.win_rate} < min_win_rate {cfg.min_win_rate}") @@ -263,12 +301,24 @@ def pool_stats(samples: Sequence[ProductSample]) -> tuple[BacktestResult, Pooled - pooled ``profit_factor`` = ``Σ(wins_i * avg_win_i) / |Σ(losses_i * avg_loss_i)|``, with `stats.summarize`'s Infinity / 0 conventions. - pooled ``avg_mfe``/``avg_mae`` = trade-weighted means, same as expectancy. + - the ``*_r`` fields (#820) pool by the SAME arithmetic over each result's R sample, + and exactly, because R carries its own counts: with + ``m_i = n_i - n_excluded_no_risk_i`` (the trades with R), + ``expectancy_r``/``avg_mfe_r``/``avg_mae_r`` are + ``m_i``-weighted means, ``avg_win_r``/``avg_loss_r`` are + weighted by ``n_wins_r_i``/``n_losses_r_i`` (so the money + side's scratch inexactness does not arise), and + ``profit_factor_r`` is the gross-R ratio. A result with no R + (``m_i = 0``) contributes nothing but its exclusion count; a + pool with no R at all reports every R field as ``None``. + These are what the pooled floors judge. - ``trades``/``max_drawdown``/``max_losing_streak`` are NOT pooled: drawdown and streak are path-dependent -- they depend on the ORDER of trades across the union, which no per-result aggregate carries -- so any value would be fabricated. They are set - to empty/0, and the promotion gate reads none of them; a - caller wanting real cross-product drawdown must pool + to empty/0 (``max_drawdown_r`` to ``None``: an unknown, + not a flat 0R), and the promotion gate reads none of them; + a caller wanting real cross-product drawdown must pool equity curves, not stats. """ n_total = sum(s.stats.n_trades for s in samples) @@ -317,6 +367,8 @@ def pool_stats(samples: Sequence[ProductSample]) -> tuple[BacktestResult, Pooled ), ) + pooled_stats = replace(pooled_stats, **_pool_r(samples)) + reading = PooledReading( n_pooled=n_total, per_product=tuple(sorted(counts.items())), @@ -326,6 +378,47 @@ def pool_stats(samples: Sequence[ProductSample]) -> tuple[BacktestResult, Pooled return pooled_stats, reading +def _pool_r(samples: Sequence[ProductSample]) -> dict[str, Any]: + """The pooled `*_r` fields -- `pool_stats`' docstring states the arithmetic.""" + excluded = sum(s.stats.n_excluded_no_risk for s in samples) + with_r = [s.stats for s in samples if s.stats.expectancy_r is not None] + weights = [st.n_trades - st.n_excluded_no_risk for st in with_r] + m_total = sum(weights) + if m_total == 0: + return {"n_excluded_no_risk": excluded} + + def weighted_mean(values: list[Decimal | None]) -> Decimal: + pairs = zip(weights, values, strict=True) + return sum((w * v for w, v in pairs if v is not None), Decimal(0)) / m_total + + wins_total = sum(st.n_wins_r for st in with_r) + losses_total = sum(st.n_losses_r for st in with_r) + gross_win = sum( + (st.n_wins_r * st.avg_win_r for st in with_r if st.avg_win_r is not None), Decimal(0) + ) + gross_loss = sum( + (st.n_losses_r * st.avg_loss_r for st in with_r if st.avg_loss_r is not None), + Decimal(0), + ) + return { + "expectancy_r": weighted_mean([st.expectancy_r for st in with_r]), + "avg_win_r": (gross_win / wins_total) if wins_total else Decimal(0), + "avg_loss_r": (gross_loss / losses_total) if losses_total else Decimal(0), + "profit_factor_r": ( + (gross_win / abs(gross_loss)) + if gross_loss != 0 + else (Decimal("Infinity") if gross_win > 0 else Decimal(0)) + ), + # path-dependent: not pooled (see `pool_stats`), and unknown rather than 0R + "max_drawdown_r": None, + "avg_mfe_r": weighted_mean([st.avg_mfe_r for st in with_r]), + "avg_mae_r": weighted_mean([st.avg_mae_r for st in with_r]), + "n_excluded_no_risk": excluded, + "n_wins_r": wins_total, + "n_losses_r": losses_total, + } + + def _pooled_floors( pooled: BacktestResult, reading: PooledReading, cfg: PromotionConfig ) -> tuple[bool, list[str]]: @@ -355,14 +448,18 @@ def _pooled_floors( "discount pooled evidence pays for its larger n)" ) - if pooled.expectancy <= cfg.min_expectancy: - reasons.append( - f"pooled expectancy {pooled.expectancy} <= min_expectancy {cfg.min_expectancy}" - ) - + # In R, and refused without R -- exactly `check_floors`' rule (#820). rr = _realized_rr(pooled) - if rr < cfg.min_rr: - reasons.append(f"pooled rr {rr} < min_rr {cfg.min_rr}") + if pooled.expectancy_r is None or rr is None: + reasons.append(_no_r_reason(reading.n_pooled, pooled=True)) + else: + if pooled.expectancy_r <= cfg.min_expectancy: + reasons.append( + f"pooled expectancy_r {pooled.expectancy_r} <= min_expectancy " + f"{cfg.min_expectancy} (in R)" + ) + if rr < cfg.min_rr: + reasons.append(f"pooled rr_r {rr} < min_rr {cfg.min_rr} (in R)") if pooled.win_rate < cfg.min_win_rate: reasons.append(f"pooled win_rate {pooled.win_rate} < min_win_rate {cfg.min_win_rate}") @@ -590,10 +687,19 @@ def should_demote(rolling_stats: BacktestResult, cfg: PromotionConfig) -> bool: deliberately excludes `min_trades`: a live rule's rolling window is sized for timely decay detection (spec §6.3/§20.7), not for re-proving the original sample size, so requiring `min_trades` here would make demotion undetectable in practice. + + Mirrors them in R too (#820), including the no-R case: `check_floors` refuses a sample + with no R, so this DEMOTES one -- missing evidence must never block pulling a rule back + from real money (`transition`'s asymmetry). An empty rolling window already demoted + before #820 (its price-unit expectancy was 0, not above the floor), so this adds no + new demotion path for it. """ - if rolling_stats.expectancy <= cfg.min_expectancy: + rr = _realized_rr(rolling_stats) + if rolling_stats.expectancy_r is None or rr is None: + return True + if rolling_stats.expectancy_r <= cfg.min_expectancy: return True - if _realized_rr(rolling_stats) < cfg.min_rr: + if rr < cfg.min_rr: return True if rolling_stats.win_rate < cfg.min_win_rate: return True diff --git a/keel/strategy/rules/base.py b/keel/strategy/rules/base.py index 961cd838..cff3589b 100644 --- a/keel/strategy/rules/base.py +++ b/keel/strategy/rules/base.py @@ -177,6 +177,37 @@ class Trade: mfe: Decimal mae: Decimal outcome: TradeOutcome + #: The risk the trade carried, PER UNIT: `|entry_fill - setup.stop|` -- the ACHIEVED fill + #: against the setup's ORIGINAL stop, never a managed (ratcheted) level (#820). `r_multiple` + #: divides by `initial_risk * qty`, and MFE/MAE (per unit) convert to R by dividing by it. + #: `None` when no risk was recorded (a trade built before #820, or one with no stop); such a + #: trade has no R and is excluded from `stats.summarize`'s R aggregates. Last, and + #: defaulted, so every pre-#820 constructor keeps working. + initial_risk: Decimal | None = None + + +def initial_risk_of(entry_fill: Decimal, stop: Decimal) -> Decimal: + """A trade's per-unit initial risk: the distance from the ACHIEVED fill to the setup's + stop, as a magnitude (#820). + + Unsigned on purpose. The signed form `(entry_fill - stop)` goes negative when the fill + gaps below the stop, and dividing a LOSS by a negative risk printed it as a positive R. + Risk is a distance; the sign of R comes from the P&L alone. + """ + return abs(entry_fill - stop) + + +def r_multiple_of(pnl: Decimal, initial_risk: Decimal | None, qty: Decimal) -> Decimal | None: + """`pnl` in R: net P&L over the whole position's initial risk, `initial_risk * qty`. + + The ONE formula every close path uses (`backtest._close_trade`, `paper._close`, + `portfolio_sim._process_held`) and every aggregate reads (`stats.summarize`), so the R a + trade records and the R its summary pools cannot drift apart (#820). `None` -- no R, not + 0R -- when the risk is unknown or zero: a trade that risked nothing measured nothing. + """ + if initial_risk is None or initial_risk == 0: + return None + return pnl / (initial_risk * qty) #: The arithmetic a declared dimension carries: `"int"` means the legitimate values are diff --git a/keel/strategy/stats.py b/keel/strategy/stats.py index 46007d76..a4a4a51f 100644 --- a/keel/strategy/stats.py +++ b/keel/strategy/stats.py @@ -11,14 +11,28 @@ dependency on `backtest.py`, only on the shared `Trade` type from `strategy.rules.base` -- `backtest.py` imports `BacktestResult` back from here (and re-exports it, so existing `from keel.strategy.backtest import BacktestResult` call sites are unaffected). + +**Two units, side by side (#820).** The original aggregates (`expectancy`, `avg_win`, ..., +`avg_mae`) are built from `Trade.pnl`/`mfe`/`mae`: PRICE UNITS -- for the backtester, dollars +per one coin of the base asset. They are money, and `insights`, the web payload, tuning and +walk-forward present or optimise them as money, so they stay exactly as they were. They are +NOT comparable across assets: a BTC trade and an XLM trade differ by five orders of magnitude +in price, so any pool of them is decided by the highest-priced asset. + +The `*_r` twins are the same aggregates in R -- each trade's net P&L over the risk it carried +(`rules.base.r_multiple_of`) -- which is what the edge table, G2 and the promotion floors +judge (spec §5.1: "expectancy (R) ... max_drawdown (R)"). A trade with no recorded initial +risk has no R: it is EXCLUDED from every R aggregate (never counted as 0R) and counted in +`n_excluded_no_risk`, and a sample with no R at all reports every R aggregate as `None`. """ from __future__ import annotations from dataclasses import dataclass from decimal import Decimal +from typing import Any -from keel.strategy.rules.base import Trade +from keel.strategy.rules.base import Trade, r_multiple_of @dataclass @@ -28,6 +42,11 @@ class BacktestResult: All aggregate metrics (everything but `trades`/`n_trades`) are computed over *closed* trades only -- a still-open trade at the end of the series is included in `trades` for visibility but excluded from win/loss/expectancy/etc. + + The fields through `avg_mae` are in PRICE UNITS; the `*_r` fields are the same + aggregates in R over the closed trades that carry an initial risk (see the module + docstring). The R fields are defaulted so a hand-built result (tests, `pool_stats`) + that predates them still constructs -- and reads as "no R", which the floors refuse. """ trades: list[Trade] @@ -41,6 +60,25 @@ class BacktestResult: max_losing_streak: int avg_mfe: Decimal avg_mae: Decimal + #: Mean R over the closed trades with R; `None` when none has R. + expectancy_r: Decimal | None = None + #: Mean R of the winning / losing trades with R (0 when the R sample has none of that + #: side, the money fields' convention); `None` when no trade has R. + avg_win_r: Decimal | None = None + avg_loss_r: Decimal | None = None + #: Gross winning R over gross losing R, with the money field's Infinity / 0 conventions. + profit_factor_r: Decimal | None = None + #: The largest peak-to-trough fall of CUMULATIVE R, in the order the trades are given. + max_drawdown_r: Decimal | None = None + #: Mean per-unit MFE / MAE over per-unit initial risk. + avg_mfe_r: Decimal | None = None + avg_mae_r: Decimal | None = None + #: Closed trades left out of every R aggregate for carrying no (or a zero) initial risk. + n_excluded_no_risk: int = 0 + #: The R sample's winning / losing trade counts -- what `promotion.pool_stats` weights + #: the per-product R means by, so a pooled R mean is the union's exact mean. + n_wins_r: int = 0 + n_losses_r: int = 0 def _closed_pnl(trade: Trade) -> Decimal: @@ -65,6 +103,64 @@ def _closed_pnl(trade: Trade) -> Decimal: return trade.pnl +def trade_r(trade: Trade) -> Decimal | None: + """`trade`'s R, from the ONE shared formula (`rules.base.r_multiple_of`) over its recorded + initial risk -- `None` when the trade carries none. Aggregates read R through here rather + than `Trade.r_multiple`, so whether a trade is in the R sample is decided by one fact (its + initial risk) for R, MFE and MAE alike.""" + if trade.initial_risk is None or trade.initial_risk == 0: + return None + return r_multiple_of(_closed_pnl(trade), trade.initial_risk, trade.qty) + + +def _r_aggregates(closed: list[Trade]) -> dict[str, Any]: + """The `*_r` fields of `BacktestResult` over `closed` (see its field comments), as + keyword arguments. A sample with no R returns only the exclusion count, leaving every R + aggregate at its `None` default.""" + # (trade, its R, its per-unit risk) for every closed trade that has R. + sample: list[tuple[Trade, Decimal, Decimal]] = [] + for t in closed: + r = trade_r(t) + if r is not None and t.initial_risk is not None: + sample.append((t, r, t.initial_risk)) + excluded = len(closed) - len(sample) + if not sample: + return {"n_excluded_no_risk": excluded} + + win_rs = [r for t, r, _risk in sample if t.outcome == "win"] + loss_rs = [r for t, r, _risk in sample if t.outcome == "loss"] + gross_win = sum(win_rs, Decimal(0)) + gross_loss = abs(sum(loss_rs, Decimal(0))) + if gross_loss > 0: + profit_factor_r = gross_win / gross_loss + elif gross_win > 0: + profit_factor_r = Decimal("Infinity") + else: + profit_factor_r = Decimal(0) + + running = Decimal(0) + peak = Decimal(0) + max_drawdown_r = Decimal(0) + for _t, r, _risk in sample: + running += r + peak = max(peak, running) + max_drawdown_r = max(max_drawdown_r, peak - running) + + n = len(sample) + return { + "expectancy_r": sum((r for _t, r, _risk in sample), Decimal(0)) / n, + "avg_win_r": (gross_win / len(win_rs)) if win_rs else Decimal(0), + "avg_loss_r": (sum(loss_rs, Decimal(0)) / len(loss_rs)) if loss_rs else Decimal(0), + "profit_factor_r": profit_factor_r, + "max_drawdown_r": max_drawdown_r, + "avg_mfe_r": sum((t.mfe / risk for t, _r, risk in sample), Decimal(0)) / n, + "avg_mae_r": sum((t.mae / risk for t, _r, risk in sample), Decimal(0)) / n, + "n_excluded_no_risk": excluded, + "n_wins_r": len(win_rs), + "n_losses_r": len(loss_rs), + } + + def summarize(trades: list[Trade]) -> BacktestResult: """Aggregate `trades` into a `BacktestResult`. @@ -87,7 +183,7 @@ def summarize(trades: list[Trade]) -> BacktestResult: max_losing_streak=0, avg_mfe=Decimal(0), avg_mae=Decimal(0), - ) + ) # every R field defaults to None: an empty sample has no R wins = [t for t in closed if t.outcome == "win"] losses = [t for t in closed if t.outcome == "loss"] @@ -138,4 +234,5 @@ def summarize(trades: list[Trade]) -> BacktestResult: max_losing_streak=max_losing_streak, avg_mfe=avg_mfe, avg_mae=avg_mae, + **_r_aggregates(closed), ) diff --git a/tests/commands/test_insights.py b/tests/commands/test_insights.py index c27b6873..6d5cbd3a 100644 --- a/tests/commands/test_insights.py +++ b/tests/commands/test_insights.py @@ -516,9 +516,21 @@ def test_journal_report_since_until_window(repo: Repository) -> None: def test_journal_report_enriches_r_multiple_from_paper_track_record(repo: Repository) -> None: """A `trade_outcomes` row that matches a paper trade's (entry_ts, exit_ts) gets that - trade's real `r_multiple`/`outcome` rather than a pnl-sign guess.""" + trade's real `r_multiple`/`outcome` rather than a pnl-sign guess. + + The paper trade is self-consistent: 25 made on a risk of |100 - 90| = 10 is 2.5R. Since + #820 `track_record` recomputes R from the journalled fill and stop rather than trusting + a stored value the old signed formula may have written, so a fixture whose stored R + disagreed with its own pnl and stop would no longer be "the real r_multiple".""" _seed_paper_trade( - repo, "dca", entry_ts=1000, exit_ts=2000, pnl="10", r_multiple="2.5", outcome="win" + repo, + "dca", + entry_ts=1000, + exit_ts=2000, + exit_price="125", + pnl="25", + r_multiple="2.5", + outcome="win", ) _seed_trade_outcome( repo, rule_name="dca", opened_at=1000, closed_at=2000, pnl_net="9.5", is_dca=False @@ -871,3 +883,45 @@ def test_the_horizontal_axis_is_trade_order_not_calendar_time() -> None: xs = [point.x for point in curve.points] assert xs == [Decimal("0.00"), (curve.width / 2).quantize(Decimal("0.01")), curve.width] + + +def test_rule_track_record_and_gate_distance_still_present_money_not_r(repo: Repository) -> None: + """(#820 regression) `insights` and the web payload label these fields as MONEY + (`_money(...)`, `money(...)`), so they must keep reading the pnl-based aggregates after + `BacktestResult` grew R twins. Built on a sample where R and pnl disagree in size and + sign-balance, so pointing any of them at R would fail here.""" + from keel.strategy.rules.base import Trade + from keel.strategy.stats import summarize + from keel.types import Side + + def trade(pnl: str, risk: str) -> Trade: + value = Decimal(pnl) + return Trade( + entry_ts=0, + exit_ts=1, + entry=Decimal("100000"), + exit=Decimal("100000") + value, + qty=Decimal("1"), + side=Side.BUY, + pnl=value, + r_multiple=value / Decimal(risk), + mfe=Decimal("0"), + mae=Decimal("0"), + outcome="win" if value > 0 else "loss", + initial_risk=Decimal(risk), + ) + + stats = summarize([trade("6000", "2000"), trade("-2000", "2000"), trade("-2000", "2000")]) + assert stats.expectancy_r == Decimal("1") / 3 + repo.insert_rule("dca", {"product_id": "BTC-USD"}, status="paper") + row = next(r for r in repo.get_rules() if r["kind"] == "dca") + + record = build_rule_track_record(row, stats, _default_floor(_config())) + + assert record.expectancy == Decimal("2000") / 3 + assert record.avg_win == Decimal("6000") + assert record.avg_loss == Decimal("-2000") + assert record.max_drawdown == Decimal("4000") + assert record.realized_rr == Decimal("3") + assert record.gate is not None + assert record.gate.expectancy == Decimal("2000") / 3 diff --git a/tests/commands/test_rules_services.py b/tests/commands/test_rules_services.py index 3f3fa831..73cb3fce 100644 --- a/tests/commands/test_rules_services.py +++ b/tests/commands/test_rules_services.py @@ -569,3 +569,34 @@ def test_the_resolved_backtest_path_passes_the_per_product_rate(repo, monkeypatc assert seen == [backtest_mod.SLIPPAGE_CAP_PCT], ( f"backtest_resolved priced fills at {seen} — the resolved rate is not reaching the engine" ) + + +def test_run_rule_backtest_line_labels_the_units_of_every_figure(repo: Repository) -> None: + """#820: the line used to print `expectancy=` and `max_drawdown=` bare -- price units for a + one-coin position, which read as R beside the gate that (since #820) judges R. The R + figures lead, and the price-unit ones carry a `_px` suffix and a legend saying what px is.""" + import re + + repo.insert_rule( + "dca", {"product_id": "BTC-USD", "cadence_days": 7}, status="candidate", now_ts=NOW_TS + ) + repo.upsert_candles("BTC-USD", Granularity.ONE_DAY, _daily_candles(30)) + out, err = _collect() + _outcome, stats = run_rule_backtest( + repo, _config(), 1, granularity_opt="ONE_DAY", echo=out.append, echo_err=err.append + ) + + keys = re.findall(r"(\w+)=", out[0]) + assert keys[:7] == [ + "n_trades", + "win_rate", + "expectancy_r", + "max_drawdown_r", + "profit_factor", + "expectancy_px", + "max_drawdown_px", + ] + assert f"expectancy_px={stats.expectancy} " in out[0] + assert f"max_drawdown_px={stats.max_drawdown} (px = price units, 1-coin notional)" in out[0] + assert stats.expectancy_r is None # this flat series closes no trade that carries R + assert "expectancy_r=n/a " in out[0] diff --git a/tests/sim/test_portfolio_sim.py b/tests/sim/test_portfolio_sim.py index 402a809d..309a2b29 100644 --- a/tests/sim/test_portfolio_sim.py +++ b/tests/sim/test_portfolio_sim.py @@ -1110,3 +1110,67 @@ def test_a_trailing_stop_stranded_by_a_gap_down_bar_exits_instead(): assert trailed.trades[0].exit == Decimal("97") # the gap bar's OPEN assert trailed.trades[0].exit_ts == 4 * _HOUR assert trailed.trades[0].outcome == "loss" + + +# --------------------------------------------------------------------------- +# #820: the closed SimTrade carries its initial risk, and R is signed by the outcome +# --------------------------------------------------------------------------- + + +def test_a_sim_entry_that_gaps_below_its_stop_and_loses_has_negative_r(): + """(f) portfolio_sim's close path. The setup's stop is 94; the fill bar OPENS at 90, + below it. The trade loses; the signed denominator `(90 - 94) * qty` made that loss a + positive R. R divides by `|entry_fill - setup.stop| * qty`.""" + hourly = [ + _candle(0, "100", "101", "99", "100"), + _candle(_HOUR, "90", "92", "88", "89"), + _candle(2 * _HOUR, "89", "90", "88", "89"), + ] + candles_by_asset = {"BTC": {Granularity.ONE_HOUR: hourly, Granularity.ONE_DAY: []}} + result = run( + [_FirstBarRule("BTC-USD", Decimal("100"), Decimal("94"), Decimal("130"))], + candles_by_asset, + _config(), + start_ts=hourly[0].ts, + end_ts=hourly[-1].ts, + monthly_contribution=Decimal("100000"), + fee_pct=Decimal("0.01"), + slippage_pct=Decimal("0"), + ) + + assert len(result.trades) == 1 + trade = result.trades[0] + assert trade.outcome == "loss" + assert trade.entry < Decimal("94") # the fill really is below the stop + risk = Decimal("94") - trade.entry + assert trade.pnl is not None + assert trade.r_multiple == trade.pnl / (risk * trade.qty) + assert trade.r_multiple < 0 + assert trade.initial_risk == risk + + +def test_a_sim_trade_under_a_managed_stop_keeps_the_original_risk(): + """The break-even roll moves the working stop to the entry; the trade's recorded risk is + still `|100 - 90| = 10` per unit, the ORIGINAL stop (`_Held.stop`'s docstring).""" + hourly = [ + _candle(0, "100", "101", "99", "100"), + _candle(_HOUR, "100", "101", "99", "100"), + _candle(2 * _HOUR, "100", "111", "100", "108"), + _candle(3 * _HOUR, "106", "107", "99", "100"), + ] + result = _run_exit_policy_sim( + _ExitPolicyFirstBarRule( + "BTC-USD", + Decimal("100"), + Decimal("90"), + Decimal("130"), + {"be_roll_rr": Decimal("1"), "atr_period": 2}, + ), + hourly, + ) + + assert len(result.trades) == 1 + trade = result.trades[0] + assert trade.exit == Decimal("100") + assert trade.initial_risk == Decimal("10") + assert trade.r_multiple == Decimal("0") diff --git a/tests/sim/test_report.py b/tests/sim/test_report.py index 840e2b2d..23323350 100644 --- a/tests/sim/test_report.py +++ b/tests/sim/test_report.py @@ -32,7 +32,7 @@ from keel.strategy.backtest import SLIPPAGE_CAP_PCT, SLIPPAGE_FLOOR_PCT, SlippageAssumption from keel.strategy.promotion import TREND_FOLLOW, PromotionConfig, floor_for_class from keel.strategy.rules.base import Rule, Setup, Trade -from keel.strategy.stats import BacktestResult +from keel.strategy.stats import BacktestResult, summarize from keel.types import Candle, Granularity, Side _HOUR = 3600 @@ -367,6 +367,9 @@ def _pooled_result( avg_loss: Decimal = Decimal("-90"), expectancy: Decimal = Decimal("30"), ) -> BacktestResult: + """Since #820 G2 judges expectancy and R:R in R, so the fixture carries the same numbers + on its R fields (a risk of 1 per unit, where R and price units coincide).""" + wins = round(n_trades * win_rate) return BacktestResult( trades=[], n_trades=n_trades, @@ -379,9 +382,51 @@ def _pooled_result( max_losing_streak=3, avg_mfe=Decimal("2"), avg_mae=Decimal("1"), + expectancy_r=expectancy, + avg_win_r=avg_win, + avg_loss_r=avg_loss, + profit_factor_r=Decimal("2"), + max_drawdown_r=Decimal("5"), + avg_mfe_r=Decimal("2"), + avg_mae_r=Decimal("1"), + n_wins_r=wins, + n_losses_r=n_trades - wins, ) +def test_g2_refuses_a_pooled_sample_with_no_r(): + """#820: a pool whose trades carry no initial risk cannot be judged on expectancy or R:R, + and G2 refuses it -- it does not fall back to the price-unit fields.""" + no_r = BacktestResult( + trades=[], + n_trades=150, + win_rate=0.6, + avg_win=Decimal("150"), + avg_loss=Decimal("-90"), + expectancy=Decimal("30"), + profit_factor=Decimal("2"), + max_drawdown=Decimal("5"), + max_losing_streak=3, + avg_mfe=Decimal("2"), + avg_mae=Decimal("1"), + n_excluded_no_risk=150, + ) + + v = build_verdict( + pooled=no_r, + account_metrics=_PASSING_ACCOUNT_METRICS, + benchmark=_benchmark(), + coverage={}, + promotion_cfg=CANONICAL, + ) + + assert v.g2_pass is False + assert v.reasons == [ + "no R: none of the 150 closed trades carries an initial risk, so expectancy and " + "R:R cannot be judged in R -- refusing rather than passing" + ] + + def _benchmark( total_return_pct: Decimal = Decimal("0.5"), max_drawdown_pct: Decimal = Decimal("0.3"), @@ -1063,3 +1108,216 @@ def test_render_markdown_includes_the_pbo_section_when_a_run_is_supplied(): assert "Overfitting diagnostics (PBO / CSCV)" in md # Placed before the gaps backlog and the caveats, which must both still be last. assert md.index("Overfitting diagnostics") < md.index("Knowledge & data gaps") + + +# --------------------------------------------------------------------------- +# #820: the edge table pools in R, in exit-time order, and says so truthfully +# --------------------------------------------------------------------------- + + +def _one_trade_series(price: Decimal, exit_bar: tuple[str, str, str, str]) -> list[Candle]: + """bar0 flat, bar1 the trigger, bar2 the fill bar at `price` whose range is `exit_bar` + (o, h, l, c as multiples of `price`) -- the trade opens at bar2's open and resolves + within it.""" + o, h, low, c = (price * Decimal(x) for x in exit_bar) + flat = str(price) + return [ + _candle(0, flat, flat, flat, flat), + _candle(_HOUR, flat, flat, flat, flat), + _candle(2 * _HOUR, str(o), str(h), str(low), str(c)), + ] + + +_WIN_2R = ("1", "1.045", "0.995", "1.04") # target +4% touched, stop -2% not +_LOSS_1R = ("1", "1.005", "0.975", "0.98") # stop -2% touched, target not + + +def _scaled_rule(product_id: str, price: Decimal) -> _OneShotRule: + """Entry at `price`, stop 2% below, target 4% above: a win is +2R, a loss -1R, at ANY + price.""" + return _OneShotRule( + product_id, + _HOUR, + entry=price, + stop=price * Decimal("0.98"), + target=price * Decimal("1.04"), + ) + + +def test_edge_table_pooled_expectancy_r_is_the_common_r_across_price_scales(): + """(g) A BTC-scale rule and an XLM-scale rule that each made exactly +2R pool to +2R. In + price units the pool is BTC's 2000 averaged with XLM's 0.002 -- the number the table used + to print under the "R-multiples" label.""" + btc, xlm = Decimal("100000"), Decimal("0.1") + rules = [_scaled_rule("BTC-USD", btc), _scaled_rule("XLM-USD", xlm)] + candles_by_asset = { + "BTC": {Granularity.ONE_HOUR: _one_trade_series(btc, _WIN_2R)}, + "XLM": {Granularity.ONE_HOUR: _one_trade_series(xlm, _WIN_2R)}, + } + + edge = edge_table(rules, candles_by_asset, fee_pct=Decimal("0"), slippage_pct=Decimal("0")) + + assert edge["one_shot:BTC"].expectancy_r == Decimal("2") + assert edge["one_shot:XLM"].expectancy_r == Decimal("2") + assert edge[POOLED_KEY].n_trades == 2 + assert edge[POOLED_KEY].expectancy_r == Decimal("2") + assert edge[POOLED_KEY].expectancy == (Decimal("4000") + Decimal("0.004")) / 2 + + +def test_edge_table_pooled_sign_is_not_decided_by_the_highest_priced_asset(): + """BTC loses 1R, XLM wins 2R: the pool made +0.5R per trade. In price units BTC's -2000 + swamps XLM's +0.004 and the pool reads as a loser -- G2 used to see that number.""" + btc, xlm = Decimal("100000"), Decimal("0.1") + rules = [_scaled_rule("BTC-USD", btc), _scaled_rule("XLM-USD", xlm)] + candles_by_asset = { + "BTC": {Granularity.ONE_HOUR: _one_trade_series(btc, _LOSS_1R)}, + "XLM": {Granularity.ONE_HOUR: _one_trade_series(xlm, _WIN_2R)}, + } + + edge = edge_table(rules, candles_by_asset, fee_pct=Decimal("0"), slippage_pct=Decimal("0")) + + assert edge[POOLED_KEY].expectancy < 0 + assert edge[POOLED_KEY].expectancy_r == Decimal("0.5") + + +def _timed_series(trigger_bar: int, win: bool) -> list[Candle]: + """Flat at 100 up to the trigger, a quiet fill bar, then a bar that hits the 110 target + (win) or the 90 stop (loss). The trade exits at `(trigger_bar + 2) * _HOUR`.""" + bars = [_candle(i * _HOUR, "100", "100", "100", "100") for i in range(trigger_bar + 1)] + bars.append(_candle((trigger_bar + 1) * _HOUR, "100", "101", "99", "100")) + exit_bar = ("100", "111", "99", "110") if win else ("100", "101", "89", "90") + bars.append(_candle((trigger_bar + 2) * _HOUR, *exit_bar)) + return bars + + +def test_edge_table_pools_in_exit_time_order_so_max_drawdown_is_chronological(): + """Rules listed A (loses, exits 2nd), B (wins, exits 1st), C (loses, exits 3rd). + Chronologically the pool is +1R, -1R, -1R: a 2R drawdown. Concatenated in RULE order it + is -1R, +1R, -1R, whose drawdown is 1R -- a path that never happened.""" + rules = [ + _OneShotRule("A-USD", 3 * _HOUR, Decimal("100"), Decimal("90"), Decimal("110")), + _OneShotRule("B-USD", 1 * _HOUR, Decimal("100"), Decimal("90"), Decimal("110")), + _OneShotRule("C-USD", 5 * _HOUR, Decimal("100"), Decimal("90"), Decimal("110")), + ] + candles_by_asset = { + "A": {Granularity.ONE_HOUR: _timed_series(3, win=False)}, + "B": {Granularity.ONE_HOUR: _timed_series(1, win=True)}, + "C": {Granularity.ONE_HOUR: _timed_series(5, win=False)}, + } + + edge = edge_table(rules, candles_by_asset, fee_pct=Decimal("0"), slippage_pct=Decimal("0")) + + pooled = edge[POOLED_KEY] + assert [t.exit_ts for t in pooled.trades] == [3 * _HOUR, 5 * _HOUR, 7 * _HOUR] + assert pooled.max_drawdown_r == Decimal("2") + assert pooled.max_drawdown == Decimal("20") + + +def _r_trade_at(exit_ts: int, pnl: str) -> Trade: + return Trade( + entry_ts=0, + exit_ts=exit_ts, + entry=Decimal("100"), + exit=Decimal("100") + Decimal(pnl), + qty=Decimal("1"), + side=Side.BUY, + pnl=Decimal(pnl), + r_multiple=Decimal(pnl) / 10, + mfe=Decimal("0"), + mae=Decimal("0"), + outcome="win" if Decimal(pnl) > 0 else "loss", + initial_risk=Decimal("10"), + ) + + +def test_group_trades_by_class_pools_in_exit_time_order(): + rules = [ + _ClassedRule("a", "A-USD", TREND_FOLLOW), + _ClassedRule("b", "B-USD", TREND_FOLLOW), + _ClassedRule("c", "C-USD", TREND_FOLLOW), + ] + edge = { + "a:A": summarize([_r_trade_at(2 * _HOUR, "-10")]), + "b:B": summarize([_r_trade_at(1 * _HOUR, "10")]), + "c:C": summarize([_r_trade_at(3 * _HOUR, "-10")]), + } + + by_class = group_trades_by_class(edge, rules) + + pooled = by_class[TREND_FOLLOW] + assert [t.exit_ts for t in pooled.trades] == [1 * _HOUR, 2 * _HOUR, 3 * _HOUR] + assert pooled.max_drawdown_r == Decimal("2") + + +def _table(md: list[str]) -> list[list[str]]: + """The edge table's rows as cell lists (header first, the |---| rule dropped).""" + rows = [line for line in md if line.startswith("|") and not line.startswith("|---")] + return [[cell.strip() for cell in row.strip("|").split("|")] for row in rows] + + +def test_edge_section_renders_r_columns_under_a_label_that_is_true(): + result = summarize([_r_trade_at(_HOUR, "20"), _r_trade_at(2 * _HOUR, "-10")]) + + md = _render_edge_section({"rule:BTC": result, POOLED_KEY: result}, Decimal("0.012")) + table = _table(md) + + assert table[0] == [ + "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)", + ] + assert len(table) == 3 + assert table[1] == [ + "rule:BTC", + "2", + "50.0%", + "0.500", + "2.000", + "-1.000", + "2.000", + "1.000", + "1", + "0.000", + "0.000", + ] + assert table[2][0] == f"**{POOLED_KEY}**" + assert sum("R-multiple" in line for line in md) == 1 + # No trade lacked a risk, so there is no exclusion note. + assert not any(line.startswith("Excluded from R") for line in md) + + +def test_edge_section_shows_r_as_na_and_counts_trades_excluded_for_no_risk(): + no_risk = Trade( + entry_ts=0, + exit_ts=_HOUR, + entry=Decimal("100"), + exit=Decimal("90"), + qty=Decimal("1"), + side=Side.BUY, + pnl=Decimal("-10"), + r_multiple=None, + mfe=Decimal("0"), + mae=Decimal("0"), + outcome="loss", + initial_risk=None, + ) + with_r = summarize([_r_trade_at(_HOUR, "20"), no_risk]) + none_r = summarize([no_risk]) + + md = _render_edge_section({"a:A": with_r, "b:B": none_r}, Decimal("0.012")) + table = _table(md) + + assert table[1][3] == "2.000" + assert table[2][3:8] == ["n/a", "n/a", "n/a", "n/a", "n/a"] + notes = [line for line in md if line.startswith("Excluded from R")] + assert notes == [ + "Excluded from R (no initial risk recorded): a:A 1 of 2 trades, b:B 1 of 1 trades." + ] diff --git a/tests/strategy/test_backtest.py b/tests/strategy/test_backtest.py index 6f098e86..d7f7410b 100644 --- a/tests/strategy/test_backtest.py +++ b/tests/strategy/test_backtest.py @@ -995,3 +995,89 @@ def test_a_trailing_stop_stranded_by_a_gap_down_bar_exits_instead() -> None: assert result.trades[0].exit == Decimal("97") # the gap bar's OPEN assert result.trades[0].exit_ts == trigger_ts + 4 * 3600 assert result.trades[0].outcome == "loss" + + +# --------------------------------------------------------------------------- +# #820: the trade carries its initial risk, and R is signed by the OUTCOME +# --------------------------------------------------------------------------- + + +def test_initial_risk_is_measured_from_the_achieved_fill_not_the_quoted_entry() -> None: + """(d) The setup quotes 100 but the market fills at the next bar's open, 102: the risk + the trade actually carried is `|102 - 94| = 8` per unit, and R divides by that.""" + trigger_ts = 14 * 3600 + after = [ + ("102", "103", "101", "102"), # fill bar: open 102 is the achieved fill + ("102", "118", "101", "117"), # target 118 touched -> exit 118 + ] + candles = _flat_then_run_bars(trigger_ts, after) + rule = _ScriptedParamRule(trigger_ts, Decimal("100"), Decimal("94"), Decimal("118"), {}) + + result = backtest(rule, candles, fee_pct=Decimal(0), slippage_pct=Decimal(0)) + + assert result.n_trades == 1 + trade = result.trades[0] + assert trade.entry == Decimal("102") + assert trade.initial_risk == Decimal("8") + assert trade.r_multiple == Decimal("2") # (118 - 102) / 8 + + +def test_a_managed_stop_still_measures_r_against_the_original_stop() -> None: + """(d) The break-even roll moves the working stop to 100, but the trade's risk -- and so + its R -- stays the ORIGINAL 10 (`_OpenPosition.stop`'s docstring). A scratch at the + rolled stop is a fee-only loss on a risk of 10, and the trade records that 10.""" + trigger_ts = 14 * 3600 + after = [ + ("100", "101", "99", "100"), + ("100", "111", "100", "108"), + ("106", "107", "99", "100"), + ] + candles = _flat_then_run_bars(trigger_ts, after) + rule = _ScriptedParamRule( + trigger_ts, Decimal("100"), Decimal("90"), Decimal("130"), {"be_roll_rr": Decimal("1")} + ) + + result = backtest(rule, candles, fee_pct=Decimal("0.01"), slippage_pct=Decimal(0)) + + trade = result.trades[0] + assert trade.exit == Decimal("100") # the ROLLED stop + assert trade.initial_risk == Decimal("10") # |100 - 90|, not |100 - 100| + assert trade.pnl == Decimal("-2") # fees only: 1 + 1 + assert trade.r_multiple == Decimal("-0.2") + + +def test_an_entry_that_gaps_below_its_stop_and_loses_has_negative_r() -> None: + """(e) The negative-risk flip. The setup's stop is 94; the next bar OPENS at 90, below it, + so the market fill is 90 and the same bar exits at its open (the gap-through fill). Net + of fees the trade loses 1.8. The signed denominator `(90 - 94) = -4` made that loss +0.45R; + a loss is negative R, full stop.""" + trigger_ts = 14 * 3600 + after = [ + ("90", "92", "88", "89"), # fill at 90 -- already below the 94 stop + ] + candles = _flat_then_run_bars(trigger_ts, after) + rule = _ScriptedParamRule(trigger_ts, Decimal("100"), Decimal("94"), Decimal("130"), {}) + + result = backtest(rule, candles, fee_pct=Decimal("0.01"), slippage_pct=Decimal(0)) + + assert result.n_trades == 1 + trade = result.trades[0] + assert trade.entry == Decimal("90") + assert trade.outcome == "loss" + assert trade.pnl == Decimal("-1.8") + assert trade.r_multiple == Decimal("-0.45") + assert trade.initial_risk == Decimal("4") + assert result.expectancy_r == Decimal("-0.45") + + +def test_an_open_trade_carries_its_initial_risk_too() -> None: + trigger_ts = 14 * 3600 + after = [("100", "101", "99", "100")] + candles = _flat_then_run_bars(trigger_ts, after) + rule = _ScriptedParamRule(trigger_ts, Decimal("100"), Decimal("94"), Decimal("130"), {}) + + result = backtest(rule, candles, fee_pct=Decimal(0), slippage_pct=Decimal(0)) + + assert [t.outcome for t in result.trades] == ["open"] + assert result.trades[0].initial_risk == Decimal("6") + assert result.trades[0].r_multiple is None diff --git a/tests/strategy/test_paper.py b/tests/strategy/test_paper.py index 70dea9c2..008d9014 100644 --- a/tests/strategy/test_paper.py +++ b/tests/strategy/test_paper.py @@ -724,3 +724,107 @@ def test_funding_check_rejects_at_boundary_between_notional_and_actual_fill_cost assert trader.get_cash() == seed # unchanged assert not trader.has_open_position("BTC-USD") assert repo.get_orders(mode="paper") == [] + + +# -- #820: R is signed by the outcome, and the trade carries its initial risk ---------------- + + +def test_a_paper_entry_filled_above_its_stop_that_loses_has_negative_r(repo): + """(f) paper's close path. The fill (100.05) sits BELOW nothing -- the setup's stop, 102, + is above it, so the signed denominator `(100.05 - 102) * qty` is negative and turned + this loss into a positive R. The risk is `|100.05 - 102| = 1.95` per unit; a loss on it + is negative R, and the exit payload records the risk it was measured against.""" + trader = PaperTrader(repo) + setup = _setup(entry="100", stop="102", target="120") + trader.on_signal(_enter_signal(setup=setup), qty=Decimal("2")) + + exit_id = trader.on_candle("BTC-USD", _candle(1_060, "100", "101", "99", "100")) + + payload = json.loads(repo.get_order(exit_id)["raw_response"]) + entry_fill = Decimal("100") * (Decimal(1) + SLIPPAGE_PCT) + risk = Decimal("102") - entry_fill + pnl = Decimal(payload["pnl"]) + assert payload["outcome"] == "loss" + assert Decimal(payload["r_multiple"]) == pnl / (risk * Decimal("2")) + assert Decimal(payload["r_multiple"]) < 0 + assert Decimal(payload["initial_risk"]) == risk + + record = track_record(repo, "pullback_continuation") + assert [t.initial_risk for t in record.trades] == [risk] + assert record.trades[0].r_multiple == pnl / (risk * Decimal("2")) + assert record.expectancy_r == pnl / (risk * Decimal("2")) + + +def test_track_record_recovers_initial_risk_for_a_legacy_exit_from_its_entry(repo): + """An exit journalled before #820 carries no `initial_risk`, and its stored `r_multiple` + may be the flipped, signed one. Its ENTRY payload still records the fill and the stop, + so the risk is recoverable -- and R is recomputed from it with the one shared formula + rather than trusted from a payload the old formula wrote.""" + entry_payload = { + "role": "entry", + "rule_name": "legacy_rule", + "entry": "100.05", + "stop": "102", + "target": "120", + "qty": "1", + "ts": 1_000, + } + entry_id = repo.insert_order( + { + "mode": "paper", + "product_id": "BTC-USD", + "side": "BUY", + "order_type": "market", + "qty": Decimal("1"), + "limit_price": Decimal("100"), + "status": "filled", + "fee": Decimal("0"), + "expected_fill": Decimal("100"), + "actual_fill": Decimal("100.05"), + "raw_response": json.dumps(entry_payload), + "confirmation": "paper", + "rule_id": None, + "created_at": 1_000, + "updated_at": 1_000, + } + ) + exit_payload = { + "role": "exit", + "rule_name": "legacy_rule", + "entry_order_id": entry_id, + "entry": "100.05", + "exit": "99.95", + "qty": "1", + "pnl": "-0.39", + "r_multiple": "0.2", # the pre-#820 signed value: -0.39 / (100.05 - 102) + "mfe": "0.95", + "mae": "1.05", + "outcome": "loss", + "entry_ts": 1_000, + "exit_ts": 1_060, + } + repo.insert_order( + { + "mode": "paper", + "product_id": "BTC-USD", + "side": "SELL", + "order_type": "market", + "qty": Decimal("1"), + "limit_price": Decimal("100"), + "status": "filled", + "fee": Decimal("0"), + "expected_fill": Decimal("100"), + "actual_fill": Decimal("99.95"), + "raw_response": json.dumps(exit_payload), + "confirmation": "paper", + "rule_id": None, + "created_at": 1_060, + "updated_at": 1_060, + } + ) + + record = track_record(repo, "legacy_rule") + + assert [t.initial_risk for t in record.trades] == [Decimal("1.95")] + assert record.trades[0].r_multiple == Decimal("-0.2") + assert record.expectancy_r == Decimal("-0.2") diff --git a/tests/strategy/test_promotion.py b/tests/strategy/test_promotion.py index 19af8039..52d6e4f4 100644 --- a/tests/strategy/test_promotion.py +++ b/tests/strategy/test_promotion.py @@ -41,6 +41,9 @@ should_demote, transition, ) +from keel.strategy.rules.base import Trade +from keel.strategy.stats import summarize +from keel.types import Side def _stats( @@ -54,7 +57,13 @@ def _stats( Defaults comfortably clear the default `PromotionConfig` floors (rr = 30/10 = 3.0 >= 1.5, win_rate 0.6 >= 0.55, expectancy 14 > 0, n_trades 150 >= 100). + + Since #820 the floors judge expectancy and R:R in R, so the fixture carries the same + numbers on its R fields (every trade carrying a risk of 1 per unit, where R and price + units coincide). A test that needs the two to DISAGREE builds its sample from trades + through `summarize` instead. """ + wins = round(n_trades * win_rate) return BacktestResult( trades=[], n_trades=n_trades, @@ -67,6 +76,16 @@ def _stats( max_losing_streak=4, avg_mfe=Decimal("20"), avg_mae=Decimal("8"), + expectancy_r=expectancy if n_trades else None, + avg_win_r=avg_win if n_trades else None, + avg_loss_r=avg_loss if n_trades else None, + profit_factor_r=Decimal("2") if n_trades else None, + max_drawdown_r=Decimal("50") if n_trades else None, + avg_mfe_r=Decimal("20") if n_trades else None, + avg_mae_r=Decimal("8") if n_trades else None, + n_excluded_no_risk=0, + n_wins_r=wins, + n_losses_r=n_trades - wins, ) @@ -958,3 +977,178 @@ def test_transition_without_rule_id_keeps_the_kind_level_lookup(repo: Repository assert status == "live" # the newest row (paper) promoted, not the older candidate assert _rule_status(repo, newest) == "live" + + +# -- #820: the floors judge expectancy and R:R in R, never in price units --------------------- + + +def _trade_r(r: str, risk: str) -> Trade: + """A closed trade that made exactly `r` R on a per-unit risk of `risk` (qty 1).""" + pnl = Decimal(r) * Decimal(risk) + return Trade( + entry_ts=0, + exit_ts=1, + entry=Decimal("100"), + exit=Decimal("100") + pnl, + qty=Decimal("1"), + side=Side.BUY, + pnl=pnl, + r_multiple=Decimal(r), + mfe=Decimal("0"), + mae=Decimal("0"), + outcome="win" if pnl > 0 else "loss", + initial_risk=Decimal(risk), + ) + + +def _btc_loser_and_small_cap_winners() -> list[ProductSample]: + """Five products, ten trades each. BTC (risk 2000/unit) loses 1R on all ten; four + small-cap products (risk 0.002/unit) each win 2R six times and lose 1R four times. + + In R the pool made (-10 + 4 * 8) / 50 = +0.44R per trade at an R:R of 2. In price units + BTC's -20000 swamps everything else and the pool reads as a loser with R:R ~0 -- one + high-priced asset decides the sign. + """ + samples = [ProductSample("BTC-USD", summarize([_trade_r("-1", "2000") for _ in range(10)]))] + for i in range(1, 5): + trades = [_trade_r("2", "0.002") for _ in range(6)] + [ + _trade_r("-1", "0.002") for _ in range(4) + ] + samples.append(ProductSample(f"SMALL-{i}-USD", summarize(trades))) + return samples + + +def test_check_floors_judges_expectancy_and_rr_in_r() -> None: + """(h) The price-unit fields say this rule loses; its R says it wins 0.44R at 2:1.""" + samples = _btc_loser_and_small_cap_winners() + stats = summarize([t for s in samples for t in s.stats.trades]) + cfg = PromotionConfig(min_trades=50, min_win_rate=0.3) + + assert stats.expectancy < 0 # price units: BTC decides the sign + assert stats.expectancy_r == Decimal("0.44") + ok, reasons = check_floors(stats, cfg) + + assert reasons == [] + assert ok is True + + +def test_check_floors_fails_a_rule_whose_r_is_negative_whatever_its_price_units_say() -> None: + """The mirror image: BTC wins 1R ten times, the small caps lose 1R forty times. In price + units the BTC wins swamp the pool into a winner; in R it lost 30R over 50 trades.""" + trades = [_trade_r("1", "2000") for _ in range(10)] + [ + _trade_r("-1", "0.002") for _ in range(40) + ] + stats = summarize(trades) + cfg = PromotionConfig(min_trades=50, min_win_rate=0.1) + + assert stats.expectancy > 0 + ok, reasons = check_floors(stats, cfg) + + assert ok is False + assert reasons == [ + f"expectancy_r {stats.expectancy_r} <= min_expectancy {cfg.min_expectancy} (in R)", + f"rr_r 1 < min_rr {cfg.min_rr} (in R)", + ] + + +def test_check_floors_refuses_a_sample_with_no_r_at_all() -> None: + """No trade carries an initial risk: expectancy and R:R cannot be judged in R, and a + floor that cannot be judged REFUSES -- the same fail-closed rule `can_promote` applies + to an unrun G4. The sample-size and win-rate axes are judged as before.""" + stats = _stats() + stats.expectancy_r = None + stats.avg_win_r = None + stats.avg_loss_r = None + stats.n_excluded_no_risk = stats.n_trades + stats.n_wins_r = 0 + stats.n_losses_r = 0 + + ok, reasons = check_floors(stats, PromotionConfig()) + + assert ok is False + assert reasons == [ + "no R: none of the 150 closed trades carries an initial risk, so expectancy and " + "R:R cannot be judged in R -- refusing rather than passing" + ] + + +def test_min_trades_and_win_rate_are_unaffected_by_r() -> None: + stats = _stats(n_trades=10, win_rate=0.1) + + ok, reasons = check_floors(stats, PromotionConfig()) + + assert ok is False + assert reasons == [ + "n_trades 10 < min_trades 100", + "win_rate 0.1 < min_win_rate 0.55", + ] + + +def test_pool_stats_pools_r_trade_weighted() -> None: + """(h) pool_stats' R fields are recomputed from the per-product R aggregates, exactly as + the money fields are -- weighted by each product's R-sample counts.""" + pooled, reading = pool_stats(_btc_loser_and_small_cap_winners()) + + assert reading.n_pooled == 50 + assert pooled.expectancy < 0 + assert pooled.expectancy_r == Decimal("0.44") + assert pooled.avg_win_r == Decimal("2") + assert pooled.avg_loss_r == Decimal("-1") + assert pooled.profit_factor_r == Decimal("48") / Decimal("26") + assert pooled.n_wins_r == 24 + assert pooled.n_losses_r == 26 + assert pooled.n_excluded_no_risk == 0 + # path-dependent: not pooled, and None rather than a fabricated 0R + assert pooled.max_drawdown_r is None + + +def test_the_pooled_path_judges_the_pool_in_r() -> None: + """(h) #338's pooled promotion. The candidate is the BTC reading (short on trades, and a + loser); the pool's price-unit expectancy is negative only because of BTC's price. Judged + in R the pool clears, and the pooled path carries the decision.""" + samples = _btc_loser_and_small_cap_winners() + own = samples[0].stats + cfg = PromotionConfig(min_trades=50, min_win_rate=0.3) + + decision = can_promote(own, cfg, pbo=_pbo(), pooled_samples=samples) + + assert decision.reasons == [] + assert decision.floors_pass is True + assert decision.promotable is True + + +def test_the_pooled_path_refuses_a_pool_with_no_r() -> None: + no_r = _stats(n_trades=10, win_rate=0.6) + no_r.expectancy_r = None + no_r.avg_win_r = None + no_r.avg_loss_r = None + no_r.n_excluded_no_risk = 10 + no_r.n_wins_r = 0 + no_r.n_losses_r = 0 + samples = [ProductSample(f"P{i}-USD", no_r) for i in range(5)] + cfg = PromotionConfig(min_trades=50) + + decision = can_promote(no_r, cfg, pbo=_pbo(), pooled_samples=samples) + + assert decision.floors_pass is False + assert ( + "pooled no R: none of the 50 pooled trades carries an initial risk, so expectancy " + "and R:R cannot be judged in R -- refusing rather than passing" + ) in decision.reasons + + +def test_should_demote_mirrors_the_floors_in_r() -> None: + """`should_demote` mirrors `check_floors`' performance checks, so it judges R too; and a + rolling sample with no R is DEMOTED -- missing evidence must never block pulling a rule + back from real money (`transition`'s asymmetry).""" + samples = _btc_loser_and_small_cap_winners() + winner_in_r = summarize([t for s in samples for t in s.stats.trades]) + cfg = PromotionConfig(min_win_rate=0.3) + + assert should_demote(winner_in_r, cfg) is False + + no_r = _stats() + no_r.expectancy_r = None + no_r.avg_win_r = None + no_r.avg_loss_r = None + assert should_demote(no_r, cfg) is True diff --git a/tests/strategy/test_stats.py b/tests/strategy/test_stats.py index 1e740a65..299f01c8 100644 --- a/tests/strategy/test_stats.py +++ b/tests/strategy/test_stats.py @@ -150,3 +150,177 @@ def test_summarize_rejects_a_closed_trade_carrying_no_pnl() -> None: summarize([_trade("win", pnl=None)]) assert "outcome='win'" in str(excinfo.value) + + +# --------------------------------------------------------------------------- +# #820: R aggregates -- per-trade R pooled in R, never in price units +# --------------------------------------------------------------------------- + + +def _r_trade( + r: str, + risk: str | None, + *, + mfe_r: str = "0", + mae_r: str = "0", + exit_ts: int = 1, + qty: str = "1", +) -> Trade: + """A closed trade that made exactly `r` R on a per-unit initial risk of `risk`, at + `qty` units. `pnl = r * risk * qty`, so the trade's price scale is set by `risk` alone; + MFE/MAE are per unit, as every close path records them. `risk=None` builds a trade + with no recorded initial risk (pnl then comes from `r` read as price units).""" + risk_d = Decimal(risk) if risk is not None else None + qty_d = Decimal(qty) + unit = risk_d if risk_d is not None else Decimal(1) + pnl = Decimal(r) * unit * qty_d + outcome = "win" if pnl > 0 else "loss" if pnl < 0 else "scratch" + return Trade( + entry_ts=0, + exit_ts=exit_ts, + entry=Decimal("100"), + exit=Decimal("100"), + qty=qty_d, + side=Side.BUY, + pnl=pnl, + r_multiple=None, + mfe=Decimal(mfe_r) * unit, + mae=Decimal(mae_r) * unit, + outcome=outcome, # type: ignore[arg-type] + initial_risk=risk_d, + ) + + +def _r_fields(result: BacktestResult) -> tuple: + return ( + result.expectancy_r, + result.avg_win_r, + result.avg_loss_r, + result.profit_factor_r, + result.max_drawdown_r, + result.avg_mfe_r, + result.avg_mae_r, + result.n_excluded_no_risk, + result.n_wins_r, + result.n_losses_r, + ) + + +def test_r_aggregates_are_identical_across_a_btc_scale_and_an_xlm_scale_sample() -> None: + """(a) The same R path at price 100000 and at price 0.1 gives the SAME R aggregates -- + the unit the edge table and the floors claim. The money fields are wildly different, + which is the whole point: they are price units, not R.""" + btc = summarize([_r_trade("2", "2000"), _r_trade("-1", "2000"), _r_trade("0.5", "2000")]) + xlm = summarize([_r_trade("2", "0.002"), _r_trade("-1", "0.002"), _r_trade("0.5", "0.002")]) + + assert _r_fields(btc) == _r_fields(xlm) + assert btc.expectancy_r == Decimal("1.5") / 3 + assert btc.avg_win_r == Decimal("1.25") + assert btc.avg_loss_r == Decimal("-1") + assert btc.profit_factor_r == Decimal("2.5") + # ... while the price-unit expectancy differs by the price ratio. + assert btc.expectancy == Decimal("1000") + assert xlm.expectancy == Decimal("0.001") + + +def test_r_is_qty_invariant() -> None: + """R divides by the WHOLE position's risk (`initial_risk * qty`): a paper trade at + qty 0.5 that made 2R is 2R, not 1R.""" + result = summarize([_r_trade("2", "10", qty="0.5")]) + + assert result.expectancy_r == Decimal("2") + + +def test_r_drawdown_mfe_and_mae_are_in_r() -> None: + """(b) `max_drawdown_r` is the drawdown of CUMULATIVE R in list order; MFE/MAE are each + trade's per-unit excursion over its per-unit initial risk, averaged over the R sample.""" + trades = [ + _r_trade("1", "10", mfe_r="2", mae_r="0.5"), + _r_trade("-1", "0.01", mfe_r="0.5", mae_r="1"), + _r_trade("-2", "500", mfe_r="0", mae_r="2"), + _r_trade("3", "4", mfe_r="3.5", mae_r="0.5"), + ] + + result = summarize(trades) + + # cumulative R: 1, 0, -2, 1 -> peak 1, trough -2 -> drawdown 3R + assert result.max_drawdown_r == Decimal("3") + assert result.avg_mfe_r == Decimal("6") / 4 + assert result.avg_mae_r == Decimal("4") / 4 + assert result.n_wins_r == 2 + assert result.n_losses_r == 2 + + +def test_trades_with_no_or_zero_risk_are_excluded_from_r_and_counted() -> None: + """(c) A trade with no initial risk, or a zero one, has no R. It is EXCLUDED from every R + aggregate (not counted as 0R) and the exclusion is counted -- while it still counts in + the money aggregates and in `n_trades`.""" + trades = [ + _r_trade("2", "10"), + _r_trade("-1", "10"), + _r_trade("-50", None), + _r_trade("-50", "0"), + ] + + result = summarize(trades) + + assert result.n_trades == 4 + assert result.n_excluded_no_risk == 2 + assert result.expectancy_r == Decimal("0.5") + assert result.avg_loss_r == Decimal("-1") + assert result.max_drawdown_r == Decimal("1") + assert result.n_losses_r == 1 + + +def test_an_all_no_risk_sample_gives_none_r_fields_never_zero() -> None: + """(c) With no R at all, every R aggregate is None -- 0R would read as a flat, measured + edge, which is a claim nobody measured.""" + result = summarize([_r_trade("1", None), _r_trade("-1", "0")]) + + assert result.n_trades == 2 + assert result.n_excluded_no_risk == 2 + assert _r_fields(result) == (None, None, None, None, None, None, None, 2, 0, 0) + + +def test_an_empty_sample_gives_none_r_fields() -> None: + result = summarize([]) + + assert _r_fields(result) == (None, None, None, None, None, None, None, 0, 0, 0) + + +def test_r_profit_factor_keeps_the_money_conventions() -> None: + """No losses -> Infinity; an R sample of wins only reports avg_loss_r 0, like avg_loss.""" + result = summarize([_r_trade("2", "10")]) + + assert result.profit_factor_r == Decimal("Infinity") + assert result.avg_loss_r == Decimal(0) + + +def test_money_fields_still_read_pnl_not_r() -> None: + """(i) The money consumers (`insights`, the web payload, tuning, walk-forward) read + `expectancy`/`avg_win`/`avg_loss`/`max_drawdown`/`avg_mfe` as price units. Pinned here + on a sample where R and pnl disagree in size, so a regression that pointed the money + fields at R cannot pass.""" + result = summarize( + [_r_trade("2", "1000", mfe_r="3", mae_r="1"), _r_trade("-1", "1000", mfe_r="1")] + ) + + assert result.expectancy == Decimal("500") + assert result.avg_win == Decimal("2000") + assert result.avg_loss == Decimal("-1000") + assert result.max_drawdown == Decimal("1000") + assert result.avg_mfe == Decimal("2000") + assert result.avg_mae == Decimal("500") + assert result.expectancy_r == Decimal("0.5") + + +def test_the_shared_r_helper() -> None: + """One formula for every close path: `pnl / (initial_risk * qty)`, None without a risk.""" + from keel.strategy.rules.base import initial_risk_of, r_multiple_of + + assert initial_risk_of(Decimal("90"), Decimal("94")) == Decimal("4") + assert initial_risk_of(Decimal("100"), Decimal("94")) == Decimal("6") + assert r_multiple_of(Decimal("-2"), Decimal("4"), Decimal("1")) == Decimal("-0.5") + assert r_multiple_of(Decimal("6"), Decimal("4"), Decimal("0.5")) == Decimal("3") + assert r_multiple_of(Decimal("6"), None, Decimal("1")) is None + assert r_multiple_of(Decimal("6"), Decimal("0"), Decimal("1")) is None diff --git a/tests/test_cli.py b/tests/test_cli.py index 9ea4335a..7284b3f1 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1009,18 +1009,31 @@ def test_rules_promote_reports_both_readings_and_promotes_via_the_pooled_path( repo.insert_rule("pullback_continuation", {"product_id": product}, status="paper") def fake_backtest(rule, candles, **kwargs): + # The same figures in R (#820: the floors judge R): 4 or 10 wins of 16. + btc = rule.product_id == "BTC-USD" + wins = 4 if btc else 10 + expectancy = Decimal("-2") if btc else Decimal("14") return BacktestResult( trades=[], n_trades=16, - win_rate=0.25 if rule.product_id == "BTC-USD" else 0.625, + win_rate=0.25 if btc else 0.625, avg_win=Decimal("30"), avg_loss=Decimal("-10"), - expectancy=Decimal("-2") if rule.product_id == "BTC-USD" else Decimal("14"), + expectancy=expectancy, profit_factor=Decimal("2"), max_drawdown=Decimal("50"), max_losing_streak=4, avg_mfe=Decimal("20"), avg_mae=Decimal("8"), + expectancy_r=expectancy, + avg_win_r=Decimal("30"), + avg_loss_r=Decimal("-10"), + profit_factor_r=Decimal("2"), + max_drawdown_r=Decimal("50"), + avg_mfe_r=Decimal("20"), + avg_mae_r=Decimal("8"), + n_wins_r=wins, + n_losses_r=16 - wins, ) monkeypatch.setattr(rules_cmd.backtest_mod, "backtest", fake_backtest)