From 70962cb057a1912ab2a92aa2830b0eb701bd82ec Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Tue, 13 Oct 2026 16:07:31 -0400 Subject: [PATCH 1/3] rules backtest: word an empty-side profit factor as insights does (#988) The `keel rules backtest` line printed the engine's profit_factor raw: `Infinity` for all wins and a measured-looking `0` for all scratches or no closed trade. It now prints `undefined_reading` over the reason #986 gave the track record -- `n/a (no losses yet)`, `n/a (no wins or losses yet)`, `n/a (no trades yet)` -- through one shared `insights.profit_factor_reason_of(stats)`, whose sides are decided by counting trades like `build_rule_track_record`. An all-losses 0 is a measurement and keeps its number. The returned BacktestResult and the gates are unchanged. Co-Authored-By: Claude Opus 5.5 --- keel/commands/insights.py | 24 +++++- keel/commands/rules.py | 13 ++- tests/commands/test_rules_services.py | 109 +++++++++++++++++++++++++- 3 files changed, 142 insertions(+), 4 deletions(-) diff --git a/keel/commands/insights.py b/keel/commands/insights.py index b9b948b8..3337303b 100644 --- a/keel/commands/insights.py +++ b/keel/commands/insights.py @@ -607,6 +607,27 @@ def _empty_side(n_trades: int, avg_win: Decimal | None, avg_loss: Decimal | None return None +def _profit_factor_reason( + n_trades: int, avg_win: Decimal | None, avg_loss: Decimal | None +) -> str | None: + """Why a profit factor is not a measurement, or `None` when it is -- see + `RuleTrackRecord.profit_factor_reason`, the record's reading of this one rule.""" + reason = _empty_side(n_trades, avg_win, avg_loss) + return None if reason == NO_WINS_YET else reason + + +def profit_factor_reason_of(stats: Any) -> str | None: + """`_profit_factor_reason` over a `BacktestResult` itself, its sides decided by COUNTING + (`_side_average`) exactly as `build_rule_track_record` decides a record's (#988). The + `rules backtest` line prints `undefined_reading` of it, so a backtest and a track record of + the same sample word an empty side the same way; the engine's value is untouched.""" + return _profit_factor_reason( + stats.n_trades, + _side_average(stats, "win", stats.avg_win), + _side_average(stats, "loss", stats.avg_loss), + ) + + @dataclass(frozen=True) class GateDistance: rule_name: str @@ -691,8 +712,7 @@ def profit_factor_reason(self) -> str | None: scratches is its 0 for 0/0 ("no wins or losses yet", unknown). All losses is a real 0 -- no gross profit over a real loss -- and keeps its number. The engine's own value is untouched; the gate never reads it.""" - reason = _empty_side(self.n_trades, self.avg_win, self.avg_loss) - return None if reason == NO_WINS_YET else reason + return _profit_factor_reason(self.n_trades, self.avg_win, self.avg_loss) @dataclass(frozen=True) diff --git a/keel/commands/rules.py b/keel/commands/rules.py index 10d09182..4d500a3c 100644 --- a/keel/commands/rules.py +++ b/keel/commands/rules.py @@ -42,6 +42,7 @@ from keel.analysis import indicators from keel.commands._common import _load_cfg, _open_repo, with_disclaimer from keel.commands._products import parse_products_option +from keel.commands.insights import profit_factor_reason_of, undefined_reading from keel.compliance import screen as screen_mod from keel.data.history import GRANULARITY_SECONDS from keel.data.repository import Repository @@ -624,6 +625,16 @@ def _r_text(value: Decimal | None) -> str: return "n/a" if value is None else str(value) +def _profit_factor_text(stats: backtest_mod.BacktestResult) -> str: + """The profit factor for the `rules backtest` line: the engine's value when it is a + measurement, else `keel insights summary`'s reading of the empty side (#988) -- `n/a (no + losses yet)` for the all-wins `Infinity`, `n/a (no wins or losses yet)` for an all-scratch + 0/0, `n/a (no trades yet)` for no closed trade. An all-losses 0 is measured and stays 0. + Display only: the returned `BacktestResult` keeps the engine's value.""" + reason = profit_factor_reason_of(stats) + return str(stats.profit_factor) if reason is None else undefined_reading(reason) + + def run_rule_backtest( repo: Repository, config: Any | None, @@ -652,7 +663,7 @@ def run_rule_backtest( f"rule {rule_id} ({resolved.row['kind']}): n_trades={stats.n_trades} " 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"profit_factor={_profit_factor_text(stats)} 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)}" ) diff --git a/tests/commands/test_rules_services.py b/tests/commands/test_rules_services.py index fbc51253..1dbe0347 100644 --- a/tests/commands/test_rules_services.py +++ b/tests/commands/test_rules_services.py @@ -20,13 +20,16 @@ from __future__ import annotations +import re from decimal import Decimal from typing import Any import pytest from keel import agent +from keel.commands import insights as insights_mod from keel.commands import rules as rules_mod +from keel.commands.insights import build_rule_track_record from keel.commands.rules import ( RulesRefused, RulesUsageError, @@ -52,8 +55,10 @@ from keel.strategy import backtest as backtest_mod from keel.strategy import promotion as promotion_mod from keel.strategy.reduction import Reduction +from keel.strategy.rules.base import Trade from keel.strategy.rules.reverse_dca import ReverseDca -from keel.types import Candle, Granularity +from keel.strategy.stats import summarize +from keel.types import Candle, Granularity, Side from tests.data.test_sell_proposals import _row as _proposal_row from tests.strategy.rule_conformance import minimal_params @@ -630,6 +635,108 @@ def test_run_rule_backtest_line_labels_the_units_of_every_figure(repo: Repositor assert "expectancy_r=n/a " in out[0] +def _closed(outcome: str, pnl: str, i: int = 0) -> Trade: + return Trade( + entry_ts=1_700_000_000 + i, + exit_ts=1_700_000_100 + i, + entry=Decimal("100"), + exit=Decimal("100") + Decimal(pnl), + qty=Decimal("1"), + side=Side.BUY, + pnl=Decimal(pnl), + r_multiple=None, + mfe=Decimal(0), + mae=Decimal(0), + outcome=outcome, # type: ignore[arg-type] + ) + + +def _backtest_line_over( + repo: Repository, monkeypatch: pytest.MonkeyPatch, trades: list[Trade] +) -> tuple[str, backtest_mod.BacktestResult]: + """The `rules backtest` line for a backtest that closed exactly `trades`, through the real + `run_rule_backtest` (resolution, rendering) with only the engine run replaced.""" + 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)) + calls: list[int] = [] + + def _stats(_resolved: Any) -> backtest_mod.BacktestResult: + calls.append(1) + return summarize(trades) + + monkeypatch.setattr(rules_mod, "backtest_resolved", _stats) + out, err = _collect() + _outcome, stats = run_rule_backtest( + repo, _config(), 1, granularity_opt="ONE_DAY", echo=out.append, echo_err=err.append + ) + assert calls == [1] and len(out) == 1 # the replaced engine run is the one rendered + return out[0], stats + + +def _profit_factor_text(line: str) -> str: + match = re.search(r" profit_factor=(.*?) expectancy_px=", line) + assert match is not None, line + return match.group(1) + + +@pytest.mark.parametrize( + ("trades", "engine_value", "shown"), + [ + # All wins: gross profit over no loss -- the engine's Infinity, a known fact (#988). + ([("win", "10"), ("win", "5")], Decimal("Infinity"), "n/a (no losses yet)"), + # All scratches: 0/0 -- the engine's 0 is not a measurement. + ([("scratch", "0"), ("scratch", "0")], Decimal(0), "n/a (no wins or losses yet)"), + # No closed trade at all: nothing was measured. + ([], Decimal(0), "n/a (no trades yet)"), + # All losses: no gross profit over a real loss IS a measured 0, and keeps its number. + ([("loss", "-10"), ("loss", "-5")], Decimal(0), "0"), + # Both sides hold a trade: the ratio, as before. + ([("win", "30"), ("loss", "-10")], Decimal(3), "3"), + ], + ids=["all-wins", "all-scratch", "no-trades", "all-losses", "mixed"], +) +def test_run_rule_backtest_words_an_empty_side_profit_factor_like_insights( + repo: Repository, + monkeypatch: pytest.MonkeyPatch, + trades: list[tuple[str, str]], + engine_value: Decimal, + shown: str, +) -> None: + """#988: the backtest line printed the engine's `profit_factor` raw -- `Infinity` for all + wins, a measured-looking `0` for all scratches or no trade. It now prints the reading + `keel insights summary` prints for the same sample (#986), while the returned + `BacktestResult` keeps the engine's own value.""" + line, stats = _backtest_line_over( + repo, monkeypatch, [_closed(o, pnl, i) for i, (o, pnl) in enumerate(trades)] + ) + + assert stats.profit_factor == engine_value # the engine's value is untouched + assert _profit_factor_text(line) == shown + assert "Infinity" not in line + + +def test_run_rule_backtest_reads_the_same_reason_insights_records_for_the_sample( + repo: Repository, monkeypatch: pytest.MonkeyPatch +) -> None: + """One rule, not two: for each empty-side shape the backtest line's profit factor IS + `undefined_reading` over the reason a `RuleTrackRecord` of the same stats carries.""" + shapes = { + "all-wins": [_closed("win", "10")], + "all-scratch": [_closed("scratch", "0")], + } + for trades in shapes.values(): + stats = summarize(trades) + record = build_rule_track_record( + {"id": 1, "kind": "dca", "status": "candidate", "params": {"product_id": "BTC-USD"}}, + stats, + promotion_mod.PromotionConfig(), + ) + assert record.profit_factor_reason is not None + assert insights_mod.profit_factor_reason_of(stats) == record.profit_factor_reason + + # -- the sleeve_sell route through attempt_promotion (P12 Task 12.2, spec §3.7, plan R21) --------- _DAY = 86_400 From 52f0f995dbd6eaa1732578e3131972e1ff969bbc Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Tue, 13 Oct 2026 16:22:27 -0400 Subject: [PATCH 2/3] fix(backtest): n/a for every figure at n_trades == 0; parity test drives the line The rules backtest line prints bare n/a for win_rate, profit_factor, expectancy_px and max_drawdown_px when no trade closed, as insights summary does. The parity test now runs all-wins, all-scratch and a zero-pnl win through the real line. Co-Authored-By: Claude Opus 5.5 --- keel/commands/rules.py | 24 ++++++-- tests/commands/test_rules_services.py | 79 ++++++++++++++++++++------- 2 files changed, 78 insertions(+), 25 deletions(-) diff --git a/keel/commands/rules.py b/keel/commands/rules.py index 4d500a3c..73b78da7 100644 --- a/keel/commands/rules.py +++ b/keel/commands/rules.py @@ -629,8 +629,11 @@ def _profit_factor_text(stats: backtest_mod.BacktestResult) -> str: """The profit factor for the `rules backtest` line: the engine's value when it is a measurement, else `keel insights summary`'s reading of the empty side (#988) -- `n/a (no losses yet)` for the all-wins `Infinity`, `n/a (no wins or losses yet)` for an all-scratch - 0/0, `n/a (no trades yet)` for no closed trade. An all-losses 0 is measured and stays 0. - Display only: the returned `BacktestResult` keeps the engine's value.""" + 0/0. An all-losses 0 is measured and stays 0. With no closed trade nothing was measured, so + the line prints bare `n/a` for it, as `render_summary` does at `n_trades == 0` (see + `run_rule_backtest`). Display only: the returned `BacktestResult` keeps the engine's value.""" + if stats.n_trades == 0: + return "n/a" reason = profit_factor_reason_of(stats) return str(stats.profit_factor) if reason is None else undefined_reading(reason) @@ -650,7 +653,12 @@ def run_rule_backtest( Returns the outcome (its single echoed line) beside the raw `BacktestResult` -- the strategy console's retry flow renders both, and a caller that only wants the CLI's line reads `outcome.lines`. Refusals (unknown id, no granularity) are the service's - `RulesRefused` shapes, already echoed to `echo_err`.""" + `RulesRefused` shapes, already echoed to `echo_err`. + + At `n_trades == 0` nothing was measured, so `win_rate`, `profit_factor`, `expectancy_px` + and `max_drawdown_px` print bare `n/a` (the R pair already does) -- what `keel insights + summary` prints for a rule with no trade (#988). Display only: the engine's zeros stay in + the returned `BacktestResult`.""" resolved = resolve_rule_backtest( repo, config, rule_id, granularity_opt=granularity_opt, echo_err=echo_err ) @@ -659,12 +667,16 @@ def run_rule_backtest( # 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. + measured = stats.n_trades > 0 + win_rate = f"{stats.win_rate:.2%}" if measured else "n/a" + expectancy_px = stats.expectancy if measured else "n/a" + max_drawdown_px = stats.max_drawdown if measured else "n/a" sink( f"rule {rule_id} ({resolved.row['kind']}): n_trades={stats.n_trades} " - f"win_rate={stats.win_rate:.2%} expectancy_r={_r_text(stats.expectancy_r)} " + f"win_rate={win_rate} expectancy_r={_r_text(stats.expectancy_r)} " f"max_drawdown_r={_r_text(stats.max_drawdown_r)} " - f"profit_factor={_profit_factor_text(stats)} expectancy_px={stats.expectancy} " - f"max_drawdown_px={stats.max_drawdown} (px = price units, 1-coin notional) " + f"profit_factor={_profit_factor_text(stats)} expectancy_px={expectancy_px} " + f"max_drawdown_px={max_drawdown_px} (px = price units, 1-coin notional) " f"{_describe_fee(resolved.fee_pct, resolved.fee_source)}" ) return ( diff --git a/tests/commands/test_rules_services.py b/tests/commands/test_rules_services.py index 1dbe0347..c7e4a8b5 100644 --- a/tests/commands/test_rules_services.py +++ b/tests/commands/test_rules_services.py @@ -629,8 +629,11 @@ def test_run_rule_backtest_line_labels_the_units_of_every_figure(repo: Repositor "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] + # This flat series closes no trade, so nothing was measured: the px figures read `n/a`, not + # the engine's 0 (#988). The legend stays either way. + assert stats.n_trades == 0 + assert "expectancy_px=n/a " in out[0] + assert "max_drawdown_px=n/a (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] @@ -689,13 +692,16 @@ def _profit_factor_text(line: str) -> str: # All scratches: 0/0 -- the engine's 0 is not a measurement. ([("scratch", "0"), ("scratch", "0")], Decimal(0), "n/a (no wins or losses yet)"), # No closed trade at all: nothing was measured. - ([], Decimal(0), "n/a (no trades yet)"), + ([], Decimal(0), "n/a"), + # A win that made nothing: counting says it has no losses, so "no losses yet" -- reading + # the zero average would say "no wins or losses". Gross 0 over gross 0 is the engine's 0. + ([("win", "0")], Decimal(0), "n/a (no losses yet)"), # All losses: no gross profit over a real loss IS a measured 0, and keeps its number. ([("loss", "-10"), ("loss", "-5")], Decimal(0), "0"), # Both sides hold a trade: the ratio, as before. ([("win", "30"), ("loss", "-10")], Decimal(3), "3"), ], - ids=["all-wins", "all-scratch", "no-trades", "all-losses", "mixed"], + ids=["all-wins", "all-scratch", "no-trades", "zero-pnl-win", "all-losses", "mixed"], ) def test_run_rule_backtest_words_an_empty_side_profit_factor_like_insights( repo: Repository, @@ -717,24 +723,59 @@ def test_run_rule_backtest_words_an_empty_side_profit_factor_like_insights( assert "Infinity" not in line -def test_run_rule_backtest_reads_the_same_reason_insights_records_for_the_sample( +_N0_LINE = re.compile( + r"^rule 1 \(dca\): n_trades=(?P\S+) win_rate=(?P\S+) " + r"expectancy_r=(?P\S+) max_drawdown_r=(?P\S+) " + r"profit_factor=(?P.+?) expectancy_px=(?P\S+) " + r"max_drawdown_px=(?P\S+) \(px = price units, 1-coin notional\) " +) + + +def test_run_rule_backtest_prints_n_a_for_every_figure_when_no_trade_closed( repo: Repository, monkeypatch: pytest.MonkeyPatch ) -> None: - """One rule, not two: for each empty-side shape the backtest line's profit factor IS - `undefined_reading` over the reason a `RuleTrackRecord` of the same stats carries.""" - shapes = { - "all-wins": [_closed("win", "10")], - "all-scratch": [_closed("scratch", "0")], + """#988: at n_trades == 0 `keel insights summary` prints bare `n/a` for every figure, + because nothing was measured -- the engine's 0.00% / 0 are not measurements. The backtest + line says the same, in the same key order, with the n and the legend untouched.""" + line, stats = _backtest_line_over(repo, monkeypatch, []) + + match = _N0_LINE.match(line) + assert match is not None, line + assert match.groupdict() == { + "n_trades": "0", + "win_rate": "n/a", + "expectancy_r": "n/a", + "max_drawdown_r": "n/a", + "profit_factor": "n/a", + "expectancy_px": "n/a", + "max_drawdown_px": "n/a", } - for trades in shapes.values(): - stats = summarize(trades) - record = build_rule_track_record( - {"id": 1, "kind": "dca", "status": "candidate", "params": {"product_id": "BTC-USD"}}, - stats, - promotion_mod.PromotionConfig(), - ) - assert record.profit_factor_reason is not None - assert insights_mod.profit_factor_reason_of(stats) == record.profit_factor_reason + assert stats.n_trades == 0 and stats.win_rate == 0 # display only: the engine value stays + + +@pytest.mark.parametrize( + "trades", + [ + [_closed("win", "10")], + [_closed("scratch", "0")], + [_closed("win", "0")], + ], + ids=["all-wins", "all-scratch", "zero-pnl-win"], +) +def test_run_rule_backtest_reads_the_same_reason_insights_records_for_the_sample( + repo: Repository, monkeypatch: pytest.MonkeyPatch, trades: list[Trade] +) -> None: + """One rule, not two: for each empty-side shape the backtest LINE's profit factor IS + `undefined_reading` over the reason a `RuleTrackRecord` of the same stats carries.""" + line, stats = _backtest_line_over(repo, monkeypatch, trades) + record = build_rule_track_record( + {"id": 1, "kind": "dca", "status": "candidate", "params": {"product_id": "BTC-USD"}}, + stats, + promotion_mod.PromotionConfig(), + ) + + assert record.profit_factor_reason is not None + assert _profit_factor_text(line) == insights_mod.undefined_reading(record.profit_factor_reason) # -- the sleeve_sell route through attempt_promotion (P12 Task 12.2, spec §3.7, plan R21) --------- From f1dbde17b5d89040368120941e29eb0c3b218df3 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Tue, 13 Oct 2026 16:30:01 -0400 Subject: [PATCH 3/3] docs(insights): profit_factor_reason_of names the n=0 bare n/a (#988) Co-Authored-By: Claude Opus 5.5 --- keel/commands/insights.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/keel/commands/insights.py b/keel/commands/insights.py index 3337303b..38456b32 100644 --- a/keel/commands/insights.py +++ b/keel/commands/insights.py @@ -618,9 +618,11 @@ def _profit_factor_reason( def profit_factor_reason_of(stats: Any) -> str | None: """`_profit_factor_reason` over a `BacktestResult` itself, its sides decided by COUNTING - (`_side_average`) exactly as `build_rule_track_record` decides a record's (#988). The - `rules backtest` line prints `undefined_reading` of it, so a backtest and a track record of - the same sample word an empty side the same way; the engine's value is untouched.""" + (`_side_average`) exactly as `build_rule_track_record` decides a record's (#988). When the + sample has trades the `rules backtest` line prints `undefined_reading` of it, so a backtest + and a track record of the same sample word an empty side the same way; at n_trades == 0 it + prints a bare `n/a`, as `render_summary` does for a rule with no trade. The engine's value + is untouched.""" return _profit_factor_reason( stats.n_trades, _side_average(stats, "win", stats.avg_win),