From 25816dbbdbed5ce32e3f5f66788e211583253979 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Tue, 13 Oct 2026 17:43:31 -0400 Subject: [PATCH 1/4] sim report: word an empty-side R profit factor as insights does (#1000) The edge table's `Profit factor (R)` printed the engine's raw `inf` for an R sample with no losing trade and `0.000` for one of scratches only (or a zero-R win). It now prints `n/a (no losses yet)` / `n/a (no wins or losses yet)` through `undefined_reading` and a new shared `insights.profit_factor_r_reason_of`, whose sides are decided by counting the R sample (`n_wins_r` / `n_losses_r`), not by reading a zero. An all-losses 0 keeps its number; a row with no R stays a bare `n/a`. `BacktestResult` is unchanged; `_r_cell`'s docstring now says the `inf` it keeps is only the degenerate zero-R-loss case. Co-Authored-By: Claude Opus 5.5 --- keel/commands/insights.py | 17 ++++++++++++ keel/sim/report.py | 26 +++++++++++++++++-- tests/sim/test_report.py | 54 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 2 deletions(-) diff --git a/keel/commands/insights.py b/keel/commands/insights.py index 38456b32..ddaa4536 100644 --- a/keel/commands/insights.py +++ b/keel/commands/insights.py @@ -630,6 +630,23 @@ def profit_factor_reason_of(stats: Any) -> str | None: ) +def profit_factor_r_reason_of(stats: Any) -> str | None: + """`_profit_factor_reason` over a `BacktestResult`'s R SAMPLE (#1000) -- the closed trades + that carry an initial risk, `n_trades - n_excluded_no_risk` of them -- for the `keel sim` + report's `Profit factor (R)` cell. Its sides are decided by COUNTING that sample's trades + (`n_wins_r` / `n_losses_r`), never by reading `profit_factor_r`: the engine writes `Infinity` + for no losing R and `0` both for all scratches and for a zero-R win, and only the count tells + those apart. So an R profit factor with an empty side is worded as the track record and + `rules backtest` word a money one. An empty R sample returns `NO_TRADES_YET`; the edge table + prints a bare `n/a` there instead, as it does for every R figure of a row with no R. The + engine's value is untouched.""" + return _profit_factor_reason( + stats.n_trades - stats.n_excluded_no_risk, + stats.avg_win_r if stats.n_wins_r else None, + stats.avg_loss_r if stats.n_losses_r else None, + ) + + @dataclass(frozen=True) class GateDistance: rule_name: str diff --git a/keel/sim/report.py b/keel/sim/report.py index f47bbf40..ad5c4b33 100644 --- a/keel/sim/report.py +++ b/keel/sim/report.py @@ -62,6 +62,7 @@ from decimal import Decimal from typing import TYPE_CHECKING +from keel.commands.insights import profit_factor_r_reason_of, undefined_reading from keel.execution import sizing from keel.execution.guards import _asset from keel.sim.benchmark import BenchmarkResult @@ -1005,7 +1006,7 @@ def _render_edge_section( lines.append( 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.avg_loss_r)} | {_profit_factor_r_cell(result)} | " f"{_r_cell(result.max_drawdown_r)} | {result.max_losing_streak} | " f"{_r_cell(result.avg_mfe_r)} | {_r_cell(result.avg_mae_r)} |" ) @@ -1079,9 +1080,30 @@ def _render_accumulation_section(accumulation: dict[str, DcaSleeve]) -> list[str ] +def _profit_factor_r_cell(result: BacktestResult) -> str: + """The edge table's `Profit factor (R)` cell (#1000): a bare `n/a` when the row has no R + (as `_r_cell` gives every R figure), `undefined_reading` of the reason when a side of the R + sample is empty -- `n/a (no losses yet)`, `n/a (no wins or losses yet)`, the words + `insights summary`, the Rules page and `rules backtest` print (#986/#998), through the one + shared `insights.profit_factor_r_reason_of` -- and the measured ratio otherwise. An + all-losses `0.000` is a measurement and keeps its number. + + The engine's `inf` / `0` were printed raw here until #1000. Nothing parses this Markdown + table (it is read by a person, and the run's figures are kept on `BacktestResult`, which + is unchanged), so the cell carries the reading rather than the sentinel.""" + if result.profit_factor_r is None: + return _r_cell(None) + reason = profit_factor_r_reason_of(result) + if reason is not None: + return undefined_reading(reason) + return _r_cell(result.profit_factor_r) + + 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.""" + reads as a measured flat edge). `inf` is left only for the degenerate profit factor whose + two sides both hold a trade but whose losing R sums to zero (a zero-R loss beside a win); + an EMPTY side never reaches here -- `_profit_factor_r_cell` words it (#1000).""" if value is None: return "n/a" if value.is_infinite(): diff --git a/tests/sim/test_report.py b/tests/sim/test_report.py index 23323350..3912b9ea 100644 --- a/tests/sim/test_report.py +++ b/tests/sim/test_report.py @@ -8,6 +8,7 @@ from __future__ import annotations +import dataclasses from decimal import Decimal from keel.config import TierConfig @@ -1321,3 +1322,56 @@ def test_edge_section_shows_r_as_na_and_counts_trades_excluded_for_no_risk(): assert notes == [ "Excluded from R (no initial risk recorded): a:A 1 of 2 trades, b:B 1 of 1 trades." ] + + +def _scratch_r_trade_at(exit_ts: int) -> Trade: + """A closed trade WITH R that is neither a win nor a loss (the engine's `scratch`).""" + return dataclasses.replace(_r_trade_at(exit_ts, "0"), outcome="scratch") + + +_PF_R = 6 # the "Profit factor (R)" column + + +def test_edge_section_words_an_empty_side_profit_factor_as_insights_does(): + """#1000: an empty side of the R sample reads as the track record words it (#986/#998), + not as the engine's `Infinity` / `0`. Sides are decided by COUNTING the R sample + (`n_wins_r` / `n_losses_r`): a zero-R win with no loss has an engine profit factor of 0, + yet it is a sample with no losses. An all-losses 0 is a measurement and keeps its number; + a row with no R at all stays a bare `n/a`.""" + all_wins = summarize([_r_trade_at(_HOUR, "20"), _r_trade_at(2 * _HOUR, "10")]) + all_scratch = summarize([_scratch_r_trade_at(_HOUR), _scratch_r_trade_at(2 * _HOUR)]) + all_losses = summarize([_r_trade_at(_HOUR, "-10")]) + zero_r_win = summarize([dataclasses.replace(_r_trade_at(_HOUR, "0"), outcome="win")]) + both_sides = summarize([_r_trade_at(_HOUR, "20"), _r_trade_at(2 * _HOUR, "-10")]) + no_trades = summarize([]) + # The fixtures reach the cases they name: the engine's raw values. + assert all_wins.profit_factor_r == Decimal("Infinity") + assert all_scratch.profit_factor_r == 0 and all_scratch.n_wins_r == 0 + assert all_losses.profit_factor_r == 0 and all_losses.n_losses_r == 1 + assert zero_r_win.profit_factor_r == 0 and zero_r_win.n_wins_r == 1 + assert no_trades.profit_factor_r is None + + edge = { + "wins:A": all_wins, + "scratch:B": all_scratch, + "losses:C": all_losses, + "zero:D": zero_r_win, + "both:E": both_sides, + "none:F": no_trades, + POOLED_KEY: all_wins, + } + table = _table(_render_edge_section(edge, Decimal("0.012"))) + + assert [(row[0], row[_PF_R]) for row in table[1:]] == [ + ("wins:A", "n/a (no losses yet)"), + ("scratch:B", "n/a (no wins or losses yet)"), + ("losses:C", "0.000"), + ("zero:D", "n/a (no losses yet)"), + ("both:E", "2.000"), + ("none:F", "n/a"), + (f"**{POOLED_KEY}**", "n/a (no losses yet)"), + ] + # Every row still has the header's eleven cells. + assert {len(row) for row in table} == {11} + # The engine's value is untouched by the rendering. + assert all_wins.profit_factor_r == Decimal("Infinity") From a58e0cd905897b8eff37ef4e7595876db0d6b0fb Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Tue, 13 Oct 2026 17:57:31 -0400 Subject: [PATCH 2/4] fix(sim): lazy insights import in report, correct unreachable-state docstrings (#1000) Co-Authored-By: Claude Opus 5.5 --- keel/commands/insights.py | 4 ++-- keel/sim/report.py | 10 ++++++---- tests/sim/test_report.py | 23 +++++++++++++++++++++-- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/keel/commands/insights.py b/keel/commands/insights.py index ddaa4536..2ed24548 100644 --- a/keel/commands/insights.py +++ b/keel/commands/insights.py @@ -635,8 +635,8 @@ def profit_factor_r_reason_of(stats: Any) -> str | None: that carry an initial risk, `n_trades - n_excluded_no_risk` of them -- for the `keel sim` report's `Profit factor (R)` cell. Its sides are decided by COUNTING that sample's trades (`n_wins_r` / `n_losses_r`), never by reading `profit_factor_r`: the engine writes `Infinity` - for no losing R and `0` both for all scratches and for a zero-R win, and only the count tells - those apart. So an R profit factor with an empty side is worded as the track record and + for no losing R, and `0` both for an all-losses sample (a measured 0) and for an all-scratch + sample (no reading); only the counts tell those two apart. So an R profit factor with an empty side is worded as the track record and `rules backtest` word a money one. An empty R sample returns `NO_TRADES_YET`; the edge table prints a bare `n/a` there instead, as it does for every R figure of a row with no R. The engine's value is untouched.""" diff --git a/keel/sim/report.py b/keel/sim/report.py index ad5c4b33..e33c9432 100644 --- a/keel/sim/report.py +++ b/keel/sim/report.py @@ -62,7 +62,6 @@ from decimal import Decimal from typing import TYPE_CHECKING -from keel.commands.insights import profit_factor_r_reason_of, undefined_reading from keel.execution import sizing from keel.execution.guards import _asset from keel.sim.benchmark import BenchmarkResult @@ -1093,6 +1092,9 @@ def _profit_factor_r_cell(result: BacktestResult) -> str: is unchanged), so the cell carries the reading rather than the sentinel.""" if result.profit_factor_r is None: return _r_cell(None) + # Lazy: keeps click and the commands layer out of `import keel.sim.report`. + from keel.commands.insights import profit_factor_r_reason_of, undefined_reading + reason = profit_factor_r_reason_of(result) if reason is not None: return undefined_reading(reason) @@ -1101,9 +1103,9 @@ def _profit_factor_r_cell(result: BacktestResult) -> 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` is left only for the degenerate profit factor whose - two sides both hold a trade but whose losing R sums to zero (a zero-R loss beside a win); - an EMPTY side never reaches here -- `_profit_factor_r_cell` words it (#1000).""" + reads as a measured flat edge). `inf` is unreachable from engine output: an EMPTY + side is worded by `_profit_factor_r_cell` (#1000), and a loss always has negative R (the + engine classifies by pnl sign). It is kept as a guard for a hand-built result.""" if value is None: return "n/a" if value.is_infinite(): diff --git a/tests/sim/test_report.py b/tests/sim/test_report.py index 3912b9ea..6b132b5b 100644 --- a/tests/sim/test_report.py +++ b/tests/sim/test_report.py @@ -9,6 +9,8 @@ from __future__ import annotations import dataclasses +import subprocess +import sys from decimal import Decimal from keel.config import TierConfig @@ -1335,8 +1337,9 @@ def _scratch_r_trade_at(exit_ts: int) -> Trade: def test_edge_section_words_an_empty_side_profit_factor_as_insights_does(): """#1000: an empty side of the R sample reads as the track record words it (#986/#998), not as the engine's `Infinity` / `0`. Sides are decided by COUNTING the R sample - (`n_wins_r` / `n_losses_r`): a zero-R win with no loss has an engine profit factor of 0, - yet it is a sample with no losses. An all-losses 0 is a measurement and keeps its number; + (`n_wins_r` / `n_losses_r`): an all-scratch sample and an all-losses sample both + have an engine profit factor of 0, yet only the latter is a measurement. (The hand-built + `zero:D` row, a zero-R win, is not engine output; it guards counting over reading.) An all-losses 0 is a measurement and keeps its number; a row with no R at all stays a bare `n/a`.""" all_wins = summarize([_r_trade_at(_HOUR, "20"), _r_trade_at(2 * _HOUR, "10")]) all_scratch = summarize([_scratch_r_trade_at(_HOUR), _scratch_r_trade_at(2 * _HOUR)]) @@ -1375,3 +1378,19 @@ def test_edge_section_words_an_empty_side_profit_factor_as_insights_does(): assert {len(row) for row in table} == {11} # The engine's value is untouched by the rendering. assert all_wins.profit_factor_r == Decimal("Infinity") + + +def test_importing_sim_report_does_not_load_click_or_the_commands_layer(): + """#1000: `keel.sim.report` words the profit-factor cell through `keel.commands.insights`, + but only lazily -- a bare `import keel.sim.report` must not drag click, `keel.agent` or the + commands package into the interpreter (origin/main loads none of them). A fresh interpreter, + since this process has long since imported them.""" + code = ( + "import sys; import keel.sim.report; " + "banned = ('click', 'keel.agent', 'keel.commands.insights'); " + "print(sorted(m for m in banned if m in sys.modules))" + ) + done = subprocess.run( + [sys.executable, "-c", code], capture_output=True, text=True, check=True + ) + assert done.stdout.strip() == "[]" From 7c3bdf9df4960fc30cc76fbe6294ff99adcb3f14 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Tue, 13 Oct 2026 17:58:17 -0400 Subject: [PATCH 3/4] style: wrap docstring lines to ruff limit (#1000) Co-Authored-By: Claude Opus 5.5 --- keel/commands/insights.py | 3 ++- tests/sim/test_report.py | 6 ++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/keel/commands/insights.py b/keel/commands/insights.py index 2ed24548..c3db0646 100644 --- a/keel/commands/insights.py +++ b/keel/commands/insights.py @@ -636,7 +636,8 @@ def profit_factor_r_reason_of(stats: Any) -> str | None: report's `Profit factor (R)` cell. Its sides are decided by COUNTING that sample's trades (`n_wins_r` / `n_losses_r`), never by reading `profit_factor_r`: the engine writes `Infinity` for no losing R, and `0` both for an all-losses sample (a measured 0) and for an all-scratch - sample (no reading); only the counts tell those two apart. So an R profit factor with an empty side is worded as the track record and + sample (no reading); only the counts tell those two apart. + So an R profit factor with an empty side is worded as the track record and `rules backtest` word a money one. An empty R sample returns `NO_TRADES_YET`; the edge table prints a bare `n/a` there instead, as it does for every R figure of a row with no R. The engine's value is untouched.""" diff --git a/tests/sim/test_report.py b/tests/sim/test_report.py index 6b132b5b..00294714 100644 --- a/tests/sim/test_report.py +++ b/tests/sim/test_report.py @@ -1339,7 +1339,7 @@ def test_edge_section_words_an_empty_side_profit_factor_as_insights_does(): not as the engine's `Infinity` / `0`. Sides are decided by COUNTING the R sample (`n_wins_r` / `n_losses_r`): an all-scratch sample and an all-losses sample both have an engine profit factor of 0, yet only the latter is a measurement. (The hand-built - `zero:D` row, a zero-R win, is not engine output; it guards counting over reading.) An all-losses 0 is a measurement and keeps its number; + `zero:D` row, a zero-R win, is not engine output; it guards counting over reading.) An all-losses 0 keeps its number; a row with no R at all stays a bare `n/a`.""" all_wins = summarize([_r_trade_at(_HOUR, "20"), _r_trade_at(2 * _HOUR, "10")]) all_scratch = summarize([_scratch_r_trade_at(_HOUR), _scratch_r_trade_at(2 * _HOUR)]) @@ -1390,7 +1390,5 @@ def test_importing_sim_report_does_not_load_click_or_the_commands_layer(): "banned = ('click', 'keel.agent', 'keel.commands.insights'); " "print(sorted(m for m in banned if m in sys.modules))" ) - done = subprocess.run( - [sys.executable, "-c", code], capture_output=True, text=True, check=True - ) + done = subprocess.run([sys.executable, "-c", code], capture_output=True, text=True, check=True) assert done.stdout.strip() == "[]" From c5323d8acea82f9f5331bec67b0c71837622398f Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Tue, 13 Oct 2026 17:58:28 -0400 Subject: [PATCH 4/4] style: wrap test docstring line (#1000) Co-Authored-By: Claude Opus 5.5 --- tests/sim/test_report.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/sim/test_report.py b/tests/sim/test_report.py index 00294714..72a380ba 100644 --- a/tests/sim/test_report.py +++ b/tests/sim/test_report.py @@ -1339,7 +1339,8 @@ def test_edge_section_words_an_empty_side_profit_factor_as_insights_does(): not as the engine's `Infinity` / `0`. Sides are decided by COUNTING the R sample (`n_wins_r` / `n_losses_r`): an all-scratch sample and an all-losses sample both have an engine profit factor of 0, yet only the latter is a measurement. (The hand-built - `zero:D` row, a zero-R win, is not engine output; it guards counting over reading.) An all-losses 0 keeps its number; + `zero:D` row, a zero-R win, is not engine output; it guards counting over reading.) + An all-losses 0 keeps its number; a row with no R at all stays a bare `n/a`.""" all_wins = summarize([_r_trade_at(_HOUR, "20"), _r_trade_at(2 * _HOUR, "10")]) all_scratch = summarize([_scratch_r_trade_at(_HOUR), _scratch_r_trade_at(2 * _HOUR)])