diff --git a/keel/commands/insights.py b/keel/commands/insights.py index 38456b32..c3db0646 100644 --- a/keel/commands/insights.py +++ b/keel/commands/insights.py @@ -630,6 +630,24 @@ 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 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.""" + 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..e33c9432 100644 --- a/keel/sim/report.py +++ b/keel/sim/report.py @@ -1005,7 +1005,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 +1079,33 @@ 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) + # 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) + 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 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 23323350..72a380ba 100644 --- a/tests/sim/test_report.py +++ b/tests/sim/test_report.py @@ -8,6 +8,9 @@ from __future__ import annotations +import dataclasses +import subprocess +import sys from decimal import Decimal from keel.config import TierConfig @@ -1321,3 +1324,72 @@ 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`): 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; + 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") + + +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() == "[]"