Skip to content

Commit f269f81

Browse files
eaitbrahimclaude
andauthored
rules backtest: word an empty-side profit factor as insights does (#988) (#998)
* 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * docs(insights): profit_factor_reason_of names the n=0 bare n/a (#988) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 1edf697 commit f269f81

3 files changed

Lines changed: 202 additions & 9 deletions

File tree

‎keel/commands/insights.py‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -607,6 +607,29 @@ def _empty_side(n_trades: int, avg_win: Decimal | None, avg_loss: Decimal | None
607607
return None
608608

609609

610+
def _profit_factor_reason(
611+
n_trades: int, avg_win: Decimal | None, avg_loss: Decimal | None
612+
) -> str | None:
613+
"""Why a profit factor is not a measurement, or `None` when it is -- see
614+
`RuleTrackRecord.profit_factor_reason`, the record's reading of this one rule."""
615+
reason = _empty_side(n_trades, avg_win, avg_loss)
616+
return None if reason == NO_WINS_YET else reason
617+
618+
619+
def profit_factor_reason_of(stats: Any) -> str | None:
620+
"""`_profit_factor_reason` over a `BacktestResult` itself, its sides decided by COUNTING
621+
(`_side_average`) exactly as `build_rule_track_record` decides a record's (#988). When the
622+
sample has trades the `rules backtest` line prints `undefined_reading` of it, so a backtest
623+
and a track record of the same sample word an empty side the same way; at n_trades == 0 it
624+
prints a bare `n/a`, as `render_summary` does for a rule with no trade. The engine's value
625+
is untouched."""
626+
return _profit_factor_reason(
627+
stats.n_trades,
628+
_side_average(stats, "win", stats.avg_win),
629+
_side_average(stats, "loss", stats.avg_loss),
630+
)
631+
632+
610633
@dataclass(frozen=True)
611634
class GateDistance:
612635
rule_name: str
@@ -691,8 +714,7 @@ def profit_factor_reason(self) -> str | None:
691714
scratches is its 0 for 0/0 ("no wins or losses yet", unknown). All losses is a real 0 --
692715
no gross profit over a real loss -- and keeps its number. The engine's own value is
693716
untouched; the gate never reads it."""
694-
reason = _empty_side(self.n_trades, self.avg_win, self.avg_loss)
695-
return None if reason == NO_WINS_YET else reason
717+
return _profit_factor_reason(self.n_trades, self.avg_win, self.avg_loss)
696718

697719

698720
@dataclass(frozen=True)

‎keel/commands/rules.py‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
from keel.analysis import indicators
4343
from keel.commands._common import _load_cfg, _open_repo, with_disclaimer
4444
from keel.commands._products import parse_products_option
45+
from keel.commands.insights import profit_factor_reason_of, undefined_reading
4546
from keel.compliance import screen as screen_mod
4647
from keel.data.history import GRANULARITY_SECONDS
4748
from keel.data.repository import Repository
@@ -624,6 +625,19 @@ def _r_text(value: Decimal | None) -> str:
624625
return "n/a" if value is None else str(value)
625626

626627

628+
def _profit_factor_text(stats: backtest_mod.BacktestResult) -> str:
629+
"""The profit factor for the `rules backtest` line: the engine's value when it is a
630+
measurement, else `keel insights summary`'s reading of the empty side (#988) -- `n/a (no
631+
losses yet)` for the all-wins `Infinity`, `n/a (no wins or losses yet)` for an all-scratch
632+
0/0. An all-losses 0 is measured and stays 0. With no closed trade nothing was measured, so
633+
the line prints bare `n/a` for it, as `render_summary` does at `n_trades == 0` (see
634+
`run_rule_backtest`). Display only: the returned `BacktestResult` keeps the engine's value."""
635+
if stats.n_trades == 0:
636+
return "n/a"
637+
reason = profit_factor_reason_of(stats)
638+
return str(stats.profit_factor) if reason is None else undefined_reading(reason)
639+
640+
627641
def run_rule_backtest(
628642
repo: Repository,
629643
config: Any | None,
@@ -639,7 +653,12 @@ def run_rule_backtest(
639653
Returns the outcome (its single echoed line) beside the raw `BacktestResult` -- the
640654
strategy console's retry flow renders both, and a caller that only wants the CLI's
641655
line reads `outcome.lines`. Refusals (unknown id, no granularity) are the service's
642-
`RulesRefused` shapes, already echoed to `echo_err`."""
656+
`RulesRefused` shapes, already echoed to `echo_err`.
657+
658+
At `n_trades == 0` nothing was measured, so `win_rate`, `profit_factor`, `expectancy_px`
659+
and `max_drawdown_px` print bare `n/a` (the R pair already does) -- what `keel insights
660+
summary` prints for a rule with no trade (#988). Display only: the engine's zeros stay in
661+
the returned `BacktestResult`."""
643662
resolved = resolve_rule_backtest(
644663
repo, config, rule_id, granularity_opt=granularity_opt, echo_err=echo_err
645664
)
@@ -648,12 +667,16 @@ def run_rule_backtest(
648667
# Every figure names its unit (#820): the R pair is what the promotion gate judges; the
649668
# `_px` pair is price units for a one-coin position, which is money, not R, and not
650669
# comparable across assets.
670+
measured = stats.n_trades > 0
671+
win_rate = f"{stats.win_rate:.2%}" if measured else "n/a"
672+
expectancy_px = stats.expectancy if measured else "n/a"
673+
max_drawdown_px = stats.max_drawdown if measured else "n/a"
651674
sink(
652675
f"rule {rule_id} ({resolved.row['kind']}): n_trades={stats.n_trades} "
653-
f"win_rate={stats.win_rate:.2%} expectancy_r={_r_text(stats.expectancy_r)} "
676+
f"win_rate={win_rate} expectancy_r={_r_text(stats.expectancy_r)} "
654677
f"max_drawdown_r={_r_text(stats.max_drawdown_r)} "
655-
f"profit_factor={stats.profit_factor} expectancy_px={stats.expectancy} "
656-
f"max_drawdown_px={stats.max_drawdown} (px = price units, 1-coin notional) "
678+
f"profit_factor={_profit_factor_text(stats)} expectancy_px={expectancy_px} "
679+
f"max_drawdown_px={max_drawdown_px} (px = price units, 1-coin notional) "
657680
f"{_describe_fee(resolved.fee_pct, resolved.fee_source)}"
658681
)
659682
return (

‎tests/commands/test_rules_services.py‎

Lines changed: 151 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,16 @@
2020

2121
from __future__ import annotations
2222

23+
import re
2324
from decimal import Decimal
2425
from typing import Any
2526

2627
import pytest
2728

2829
from keel import agent
30+
from keel.commands import insights as insights_mod
2931
from keel.commands import rules as rules_mod
32+
from keel.commands.insights import build_rule_track_record
3033
from keel.commands.rules import (
3134
RulesRefused,
3235
RulesUsageError,
@@ -52,8 +55,10 @@
5255
from keel.strategy import backtest as backtest_mod
5356
from keel.strategy import promotion as promotion_mod
5457
from keel.strategy.reduction import Reduction
58+
from keel.strategy.rules.base import Trade
5559
from keel.strategy.rules.reverse_dca import ReverseDca
56-
from keel.types import Candle, Granularity
60+
from keel.strategy.stats import summarize
61+
from keel.types import Candle, Granularity, Side
5762
from tests.data.test_sell_proposals import _row as _proposal_row
5863
from tests.strategy.rule_conformance import minimal_params
5964

@@ -624,12 +629,155 @@ def test_run_rule_backtest_line_labels_the_units_of_every_figure(repo: Repositor
624629
"expectancy_px",
625630
"max_drawdown_px",
626631
]
627-
assert f"expectancy_px={stats.expectancy} " in out[0]
628-
assert f"max_drawdown_px={stats.max_drawdown} (px = price units, 1-coin notional)" in out[0]
632+
# This flat series closes no trade, so nothing was measured: the px figures read `n/a`, not
633+
# the engine's 0 (#988). The legend stays either way.
634+
assert stats.n_trades == 0
635+
assert "expectancy_px=n/a " in out[0]
636+
assert "max_drawdown_px=n/a (px = price units, 1-coin notional)" in out[0]
629637
assert stats.expectancy_r is None # this flat series closes no trade that carries R
630638
assert "expectancy_r=n/a " in out[0]
631639

632640

641+
def _closed(outcome: str, pnl: str, i: int = 0) -> Trade:
642+
return Trade(
643+
entry_ts=1_700_000_000 + i,
644+
exit_ts=1_700_000_100 + i,
645+
entry=Decimal("100"),
646+
exit=Decimal("100") + Decimal(pnl),
647+
qty=Decimal("1"),
648+
side=Side.BUY,
649+
pnl=Decimal(pnl),
650+
r_multiple=None,
651+
mfe=Decimal(0),
652+
mae=Decimal(0),
653+
outcome=outcome, # type: ignore[arg-type]
654+
)
655+
656+
657+
def _backtest_line_over(
658+
repo: Repository, monkeypatch: pytest.MonkeyPatch, trades: list[Trade]
659+
) -> tuple[str, backtest_mod.BacktestResult]:
660+
"""The `rules backtest` line for a backtest that closed exactly `trades`, through the real
661+
`run_rule_backtest` (resolution, rendering) with only the engine run replaced."""
662+
repo.insert_rule(
663+
"dca", {"product_id": "BTC-USD", "cadence_days": 7}, status="candidate", now_ts=NOW_TS
664+
)
665+
repo.upsert_candles("BTC-USD", Granularity.ONE_DAY, _daily_candles(30))
666+
calls: list[int] = []
667+
668+
def _stats(_resolved: Any) -> backtest_mod.BacktestResult:
669+
calls.append(1)
670+
return summarize(trades)
671+
672+
monkeypatch.setattr(rules_mod, "backtest_resolved", _stats)
673+
out, err = _collect()
674+
_outcome, stats = run_rule_backtest(
675+
repo, _config(), 1, granularity_opt="ONE_DAY", echo=out.append, echo_err=err.append
676+
)
677+
assert calls == [1] and len(out) == 1 # the replaced engine run is the one rendered
678+
return out[0], stats
679+
680+
681+
def _profit_factor_text(line: str) -> str:
682+
match = re.search(r" profit_factor=(.*?) expectancy_px=", line)
683+
assert match is not None, line
684+
return match.group(1)
685+
686+
687+
@pytest.mark.parametrize(
688+
("trades", "engine_value", "shown"),
689+
[
690+
# All wins: gross profit over no loss -- the engine's Infinity, a known fact (#988).
691+
([("win", "10"), ("win", "5")], Decimal("Infinity"), "n/a (no losses yet)"),
692+
# All scratches: 0/0 -- the engine's 0 is not a measurement.
693+
([("scratch", "0"), ("scratch", "0")], Decimal(0), "n/a (no wins or losses yet)"),
694+
# No closed trade at all: nothing was measured.
695+
([], Decimal(0), "n/a"),
696+
# A win that made nothing: counting says it has no losses, so "no losses yet" -- reading
697+
# the zero average would say "no wins or losses". Gross 0 over gross 0 is the engine's 0.
698+
([("win", "0")], Decimal(0), "n/a (no losses yet)"),
699+
# All losses: no gross profit over a real loss IS a measured 0, and keeps its number.
700+
([("loss", "-10"), ("loss", "-5")], Decimal(0), "0"),
701+
# Both sides hold a trade: the ratio, as before.
702+
([("win", "30"), ("loss", "-10")], Decimal(3), "3"),
703+
],
704+
ids=["all-wins", "all-scratch", "no-trades", "zero-pnl-win", "all-losses", "mixed"],
705+
)
706+
def test_run_rule_backtest_words_an_empty_side_profit_factor_like_insights(
707+
repo: Repository,
708+
monkeypatch: pytest.MonkeyPatch,
709+
trades: list[tuple[str, str]],
710+
engine_value: Decimal,
711+
shown: str,
712+
) -> None:
713+
"""#988: the backtest line printed the engine's `profit_factor` raw -- `Infinity` for all
714+
wins, a measured-looking `0` for all scratches or no trade. It now prints the reading
715+
`keel insights summary` prints for the same sample (#986), while the returned
716+
`BacktestResult` keeps the engine's own value."""
717+
line, stats = _backtest_line_over(
718+
repo, monkeypatch, [_closed(o, pnl, i) for i, (o, pnl) in enumerate(trades)]
719+
)
720+
721+
assert stats.profit_factor == engine_value # the engine's value is untouched
722+
assert _profit_factor_text(line) == shown
723+
assert "Infinity" not in line
724+
725+
726+
_N0_LINE = re.compile(
727+
r"^rule 1 \(dca\): n_trades=(?P<n_trades>\S+) win_rate=(?P<win_rate>\S+) "
728+
r"expectancy_r=(?P<expectancy_r>\S+) max_drawdown_r=(?P<max_drawdown_r>\S+) "
729+
r"profit_factor=(?P<profit_factor>.+?) expectancy_px=(?P<expectancy_px>\S+) "
730+
r"max_drawdown_px=(?P<max_drawdown_px>\S+) \(px = price units, 1-coin notional\) "
731+
)
732+
733+
734+
def test_run_rule_backtest_prints_n_a_for_every_figure_when_no_trade_closed(
735+
repo: Repository, monkeypatch: pytest.MonkeyPatch
736+
) -> None:
737+
"""#988: at n_trades == 0 `keel insights summary` prints bare `n/a` for every figure,
738+
because nothing was measured -- the engine's 0.00% / 0 are not measurements. The backtest
739+
line says the same, in the same key order, with the n and the legend untouched."""
740+
line, stats = _backtest_line_over(repo, monkeypatch, [])
741+
742+
match = _N0_LINE.match(line)
743+
assert match is not None, line
744+
assert match.groupdict() == {
745+
"n_trades": "0",
746+
"win_rate": "n/a",
747+
"expectancy_r": "n/a",
748+
"max_drawdown_r": "n/a",
749+
"profit_factor": "n/a",
750+
"expectancy_px": "n/a",
751+
"max_drawdown_px": "n/a",
752+
}
753+
assert stats.n_trades == 0 and stats.win_rate == 0 # display only: the engine value stays
754+
755+
756+
@pytest.mark.parametrize(
757+
"trades",
758+
[
759+
[_closed("win", "10")],
760+
[_closed("scratch", "0")],
761+
[_closed("win", "0")],
762+
],
763+
ids=["all-wins", "all-scratch", "zero-pnl-win"],
764+
)
765+
def test_run_rule_backtest_reads_the_same_reason_insights_records_for_the_sample(
766+
repo: Repository, monkeypatch: pytest.MonkeyPatch, trades: list[Trade]
767+
) -> None:
768+
"""One rule, not two: for each empty-side shape the backtest LINE's profit factor IS
769+
`undefined_reading` over the reason a `RuleTrackRecord` of the same stats carries."""
770+
line, stats = _backtest_line_over(repo, monkeypatch, trades)
771+
record = build_rule_track_record(
772+
{"id": 1, "kind": "dca", "status": "candidate", "params": {"product_id": "BTC-USD"}},
773+
stats,
774+
promotion_mod.PromotionConfig(),
775+
)
776+
777+
assert record.profit_factor_reason is not None
778+
assert _profit_factor_text(line) == insights_mod.undefined_reading(record.profit_factor_reason)
779+
780+
633781
# -- the sleeve_sell route through attempt_promotion (P12 Task 12.2, spec §3.7, plan R21) ---------
634782

635783
_DAY = 86_400

0 commit comments

Comments
 (0)