rules backtest: word an empty-side profit factor as insights does (#988)

keel/commands/insights.py

@@ -607,6 +607,29 @@ 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). 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),
+        _side_average(stats, "loss", stats.avg_loss),
+    )
+
+
 @dataclass(frozen=True)
 class GateDistance:
     rule_name: str
@@ -691,8 +714,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)

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,19 @@ 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. 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)
+
+
 def run_rule_backtest(
     repo: Repository,
     config: Any | None,
@@ -639,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
     )
@@ -648,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={stats.profit_factor} 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 (

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
 
@@ -624,12 +629,155 @@ 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]
 
 
+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"),
+        # 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", "zero-pnl-win", "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
+
+
+_N0_LINE = re.compile(
+    r"^rule 1 \(dca\): n_trades=(?P<n_trades>\S+) win_rate=(?P<win_rate>\S+) "
+    r"expectancy_r=(?P<expectancy_r>\S+) max_drawdown_r=(?P<max_drawdown_r>\S+) "
+    r"profit_factor=(?P<profit_factor>.+?) expectancy_px=(?P<expectancy_px>\S+) "
+    r"max_drawdown_px=(?P<max_drawdown_px>\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:
+    """#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",
+    }
+    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) ---------
 
 _DAY = 86_400