Skip to content

Commit 53f42c1

Browse files
eaitbrahimclaude
andauthored
sim report: word an empty-side R profit factor as insights does (#1000) (#1003)
* 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 <noreply@anthropic.com> * fix(sim): lazy insights import in report, correct unreachable-state docstrings (#1000) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * style: wrap docstring lines to ruff limit (#1000) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * style: wrap test docstring line (#1000) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 5d4f811 commit 53f42c1

3 files changed

Lines changed: 116 additions & 2 deletions

File tree

‎keel/commands/insights.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,24 @@ def profit_factor_reason_of(stats: Any) -> str | None:
630630
)
631631

632632

633+
def profit_factor_r_reason_of(stats: Any) -> str | None:
634+
"""`_profit_factor_reason` over a `BacktestResult`'s R SAMPLE (#1000) -- the closed trades
635+
that carry an initial risk, `n_trades - n_excluded_no_risk` of them -- for the `keel sim`
636+
report's `Profit factor (R)` cell. Its sides are decided by COUNTING that sample's trades
637+
(`n_wins_r` / `n_losses_r`), never by reading `profit_factor_r`: the engine writes `Infinity`
638+
for no losing R, and `0` both for an all-losses sample (a measured 0) and for an all-scratch
639+
sample (no reading); only the counts tell those two apart.
640+
So an R profit factor with an empty side is worded as the track record and
641+
`rules backtest` word a money one. An empty R sample returns `NO_TRADES_YET`; the edge table
642+
prints a bare `n/a` there instead, as it does for every R figure of a row with no R. The
643+
engine's value is untouched."""
644+
return _profit_factor_reason(
645+
stats.n_trades - stats.n_excluded_no_risk,
646+
stats.avg_win_r if stats.n_wins_r else None,
647+
stats.avg_loss_r if stats.n_losses_r else None,
648+
)
649+
650+
633651
@dataclass(frozen=True)
634652
class GateDistance:
635653
rule_name: str

‎keel/sim/report.py‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1005,7 +1005,7 @@ def _render_edge_section(
10051005
lines.append(
10061006
f"| {label} | {result.n_trades} | {result.win_rate:.1%} | "
10071007
f"{_r_cell(result.expectancy_r)} | {_r_cell(result.avg_win_r)} | "
1008-
f"{_r_cell(result.avg_loss_r)} | {_r_cell(result.profit_factor_r)} | "
1008+
f"{_r_cell(result.avg_loss_r)} | {_profit_factor_r_cell(result)} | "
10091009
f"{_r_cell(result.max_drawdown_r)} | {result.max_losing_streak} | "
10101010
f"{_r_cell(result.avg_mfe_r)} | {_r_cell(result.avg_mae_r)} |"
10111011
)
@@ -1079,9 +1079,33 @@ def _render_accumulation_section(accumulation: dict[str, DcaSleeve]) -> list[str
10791079
]
10801080

10811081

1082+
def _profit_factor_r_cell(result: BacktestResult) -> str:
1083+
"""The edge table's `Profit factor (R)` cell (#1000): a bare `n/a` when the row has no R
1084+
(as `_r_cell` gives every R figure), `undefined_reading` of the reason when a side of the R
1085+
sample is empty -- `n/a (no losses yet)`, `n/a (no wins or losses yet)`, the words
1086+
`insights summary`, the Rules page and `rules backtest` print (#986/#998), through the one
1087+
shared `insights.profit_factor_r_reason_of` -- and the measured ratio otherwise. An
1088+
all-losses `0.000` is a measurement and keeps its number.
1089+
1090+
The engine's `inf` / `0` were printed raw here until #1000. Nothing parses this Markdown
1091+
table (it is read by a person, and the run's figures are kept on `BacktestResult`, which
1092+
is unchanged), so the cell carries the reading rather than the sentinel."""
1093+
if result.profit_factor_r is None:
1094+
return _r_cell(None)
1095+
# Lazy: keeps click and the commands layer out of `import keel.sim.report`.
1096+
from keel.commands.insights import profit_factor_r_reason_of, undefined_reading
1097+
1098+
reason = profit_factor_r_reason_of(result)
1099+
if reason is not None:
1100+
return undefined_reading(reason)
1101+
return _r_cell(result.profit_factor_r)
1102+
1103+
10821104
def _r_cell(value: Decimal | None) -> str:
10831105
"""An R aggregate for the edge table: `n/a` when the sample had no R (never a 0 that
1084-
reads as a measured flat edge), `inf` for a profit factor with no losing R."""
1106+
reads as a measured flat edge). `inf` is unreachable from engine output: an EMPTY
1107+
side is worded by `_profit_factor_r_cell` (#1000), and a loss always has negative R (the
1108+
engine classifies by pnl sign). It is kept as a guard for a hand-built result."""
10851109
if value is None:
10861110
return "n/a"
10871111
if value.is_infinite():

‎tests/sim/test_report.py‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88

99
from __future__ import annotations
1010

11+
import dataclasses
12+
import subprocess
13+
import sys
1114
from decimal import Decimal
1215

1316
from keel.config import TierConfig
@@ -1321,3 +1324,72 @@ def test_edge_section_shows_r_as_na_and_counts_trades_excluded_for_no_risk():
13211324
assert notes == [
13221325
"Excluded from R (no initial risk recorded): a:A 1 of 2 trades, b:B 1 of 1 trades."
13231326
]
1327+
1328+
1329+
def _scratch_r_trade_at(exit_ts: int) -> Trade:
1330+
"""A closed trade WITH R that is neither a win nor a loss (the engine's `scratch`)."""
1331+
return dataclasses.replace(_r_trade_at(exit_ts, "0"), outcome="scratch")
1332+
1333+
1334+
_PF_R = 6 # the "Profit factor (R)" column
1335+
1336+
1337+
def test_edge_section_words_an_empty_side_profit_factor_as_insights_does():
1338+
"""#1000: an empty side of the R sample reads as the track record words it (#986/#998),
1339+
not as the engine's `Infinity` / `0`. Sides are decided by COUNTING the R sample
1340+
(`n_wins_r` / `n_losses_r`): an all-scratch sample and an all-losses sample both
1341+
have an engine profit factor of 0, yet only the latter is a measurement. (The hand-built
1342+
`zero:D` row, a zero-R win, is not engine output; it guards counting over reading.)
1343+
An all-losses 0 keeps its number;
1344+
a row with no R at all stays a bare `n/a`."""
1345+
all_wins = summarize([_r_trade_at(_HOUR, "20"), _r_trade_at(2 * _HOUR, "10")])
1346+
all_scratch = summarize([_scratch_r_trade_at(_HOUR), _scratch_r_trade_at(2 * _HOUR)])
1347+
all_losses = summarize([_r_trade_at(_HOUR, "-10")])
1348+
zero_r_win = summarize([dataclasses.replace(_r_trade_at(_HOUR, "0"), outcome="win")])
1349+
both_sides = summarize([_r_trade_at(_HOUR, "20"), _r_trade_at(2 * _HOUR, "-10")])
1350+
no_trades = summarize([])
1351+
# The fixtures reach the cases they name: the engine's raw values.
1352+
assert all_wins.profit_factor_r == Decimal("Infinity")
1353+
assert all_scratch.profit_factor_r == 0 and all_scratch.n_wins_r == 0
1354+
assert all_losses.profit_factor_r == 0 and all_losses.n_losses_r == 1
1355+
assert zero_r_win.profit_factor_r == 0 and zero_r_win.n_wins_r == 1
1356+
assert no_trades.profit_factor_r is None
1357+
1358+
edge = {
1359+
"wins:A": all_wins,
1360+
"scratch:B": all_scratch,
1361+
"losses:C": all_losses,
1362+
"zero:D": zero_r_win,
1363+
"both:E": both_sides,
1364+
"none:F": no_trades,
1365+
POOLED_KEY: all_wins,
1366+
}
1367+
table = _table(_render_edge_section(edge, Decimal("0.012")))
1368+
1369+
assert [(row[0], row[_PF_R]) for row in table[1:]] == [
1370+
("wins:A", "n/a (no losses yet)"),
1371+
("scratch:B", "n/a (no wins or losses yet)"),
1372+
("losses:C", "0.000"),
1373+
("zero:D", "n/a (no losses yet)"),
1374+
("both:E", "2.000"),
1375+
("none:F", "n/a"),
1376+
(f"**{POOLED_KEY}**", "n/a (no losses yet)"),
1377+
]
1378+
# Every row still has the header's eleven cells.
1379+
assert {len(row) for row in table} == {11}
1380+
# The engine's value is untouched by the rendering.
1381+
assert all_wins.profit_factor_r == Decimal("Infinity")
1382+
1383+
1384+
def test_importing_sim_report_does_not_load_click_or_the_commands_layer():
1385+
"""#1000: `keel.sim.report` words the profit-factor cell through `keel.commands.insights`,
1386+
but only lazily -- a bare `import keel.sim.report` must not drag click, `keel.agent` or the
1387+
commands package into the interpreter (origin/main loads none of them). A fresh interpreter,
1388+
since this process has long since imported them."""
1389+
code = (
1390+
"import sys; import keel.sim.report; "
1391+
"banned = ('click', 'keel.agent', 'keel.commands.insights'); "
1392+
"print(sorted(m for m in banned if m in sys.modules))"
1393+
)
1394+
done = subprocess.run([sys.executable, "-c", code], capture_output=True, text=True, check=True)
1395+
assert done.stdout.strip() == "[]"

0 commit comments

Comments
 (0)