sim report: word an empty-side R profit factor as insights does (#1000)

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

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():

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() == "[]"