diff --git a/docs/operator-runbook.md b/docs/operator-runbook.md index 28e94263..0fa82ba2 100644 --- a/docs/operator-runbook.md +++ b/docs/operator-runbook.md @@ -875,7 +875,11 @@ cancel` and the exit path stay at the terminal. **Rules.** Everything keel knows about its rules, on one page, and nothing on it is a control: promotion happens in the CLI. The rule ledger and each rule's parameters come first, then the track records of the rules in the promotion pipeline (candidate and paper) with each paper rule's -distance to the promotion gate -- unsorted, because a sorted record is a leaderboard -- then the +distance to the promotion gate -- unsorted, because a sorted record is a leaderboard. There is one +record per rule, named by kind, product and ledger id, holding the paper trades that rule journalled; +paper trades journalled with no rule id get one record per kind with no id and no gate. An average +win or loss with no trade on its side reads "—" (`n/a` in `keel insights summary`), not $0.00, and so +does the realized rr with no win or no loss yet, and every figure of a rule with no closed trade. Then the DCA plan proposal (`/rules?budget=500&buffer=0.1` fills it; it is applied by the `keel dca plan` command it shows, at a terminal), then the research record: every trial, the evidence matrix, the gauntlet and the slippage assumptions. Track records, promotion-gate distances and the research diff --git a/keel/commands/insights.py b/keel/commands/insights.py index 0bc8a15f..b280410c 100644 --- a/keel/commands/insights.py +++ b/keel/commands/insights.py @@ -29,7 +29,8 @@ or writes it -- it is built entirely off `trade_outcomes` (see `Repository.get_trade_outcomes`). **On `--mode`:** `paper.track_record` is inherently paper-mode/R-aware regardless of a rule's -current lifecycle `status` (it just replays `orders(mode='paper')` for a given rule name), so +current lifecycle `status` (it just replays `orders(mode='paper')` -- per rule, by +`orders.rule_id`, since #962 -- see `paper.track_records_by_rule`), so `--mode` does not change *how* stats are computed -- it selects *which* rules the report scopes to: `paper` (the default) shows rules still in the proving pipeline (`candidate`/`paper` status, i.e. "how close to promotion"); `live` shows rules already promoted and trading (`live` status, @@ -54,7 +55,7 @@ from keel.commands.status import StatusReport, _human_age, gather_status from keel.config import Config from keel.data.repository import Repository -from keel.strategy.paper import track_record +from keel.strategy.paper import track_record, track_records_by_rule from keel.strategy.promotion import ( PromotionConfig, check_floors, @@ -62,6 +63,7 @@ promotion_class_of, ) from keel.strategy.rules.base import Rule +from keel.strategy.stats import summarize # -- the pure report shapes ----------------------------------------------------------------- @@ -604,13 +606,28 @@ def min_win_rate_pct(self) -> Decimal: @dataclass(frozen=True) class RuleTrackRecord: + """One rule's paper track record (#962: one per ledger ROW, not per kind). + + `rule_name` is the rule's KIND (the `rules` table has no name column; `promotion` says why), + so it alone cannot tell two rules of one kind apart -- `rule_id` and `product_id` do. A record + with `rule_id=None` and `status="unattributed"` holds the kind's paper trades journalled with + no `orders.rule_id`: they belong to no one rule, and are shown rather than dropped or folded + into a rule that shares the kind (`build_insights_report`). + """ + rule_name: str + rule_id: int | None + product_id: str | None status: str promotion_class: str n_trades: int win_rate: float - avg_win: Decimal - avg_loss: Decimal + avg_win: Decimal | None + """`None` when the sample has no winning trade (#965). `BacktestResult` holds 0 there, a + figure `promotion.pool_stats` multiplies by a zero count; as a reading it is unknown, not + $0.00, so the report says so and the CLI prints "n/a".""" + avg_loss: Decimal | None + """`None` when the sample has no losing trade -- `avg_win`'s rule, on the loss side.""" realized_rr: Decimal | None expectancy: Decimal profit_factor: Decimal @@ -670,16 +687,20 @@ def build_account_summary(report: StatusReport) -> AccountSummary: def _realized_rr_for_display(stats: Any) -> Decimal | None: - """`avg_win / |avg_loss|`, or `None` when there are no losing trades yet. - - Deliberately diverges from `strategy.promotion._realized_rr` (which returns `Infinity` in - that case, because a promotion-gate check needs a comparable scalar). This is a display - value, not a gate decision -- `None` ("no losses yet, nothing to measure against") is the - honest reading, not a real-looking `Infinity`. + """`avg_win / |avg_loss|`, or `None` when either side is empty: no losing trades yet (nothing + to measure against) or no winning trades yet (#965: `avg_win` is then the 0 `BacktestResult` + writes for an empty side, and `0 / |avg_loss|` would read as a measured 0.00). + + Deliberately diverges from `strategy.promotion._realized_rr` (which returns `Infinity` for no + losses, because a promotion-gate check needs a comparable scalar, and works in R). This is a + display value, not a gate decision -- `check_floors` still decides the verdict. Each side is + decided by COUNTING that side's trades, like `_side_average`. """ - if stats.avg_loss == 0: + avg_win = _side_average(stats, "win", stats.avg_win) + avg_loss = _side_average(stats, "loss", stats.avg_loss) + if avg_win is None or avg_loss is None: return None - return stats.avg_win / abs(stats.avg_loss) + return avg_win / abs(avg_loss) def build_gate_distance(rule: Rule, stats: Any, default_floor: PromotionConfig) -> GateDistance: @@ -719,34 +740,54 @@ def _human_dt(ts: int) -> str: return time.strftime("%Y-%m-%d %H:%M:%S", time.localtime(ts)) +def _side_average(stats: Any, outcome: str, average: Decimal) -> Decimal | None: + """`average` (the sample's `avg_win` or `avg_loss`), or `None` when no closed trade in the + sample has `outcome` (#965). Decided by COUNTING that side's trades, not by reading the + average as zero: `BacktestResult` writes 0 for an empty side, and 0 is a value a reader + takes as measured.""" + if not any(trade.outcome == outcome for trade in stats.trades): + return None + return average + + +def _rule_and_class(row: dict[str, Any]) -> tuple[Rule | None, str]: + """`row`'s built rule and promotion class -- `(None, "default")` when its kind is no longer + in `RULE_REGISTRY` (`agent._build_rule` raises `ValueError` for that).""" + try: + rule = agent_mod._build_rule(row) + except ValueError: + return None, "default" + return rule, promotion_class_of(rule) + + def build_rule_track_record( row: dict[str, Any], stats: Any, default_floor: PromotionConfig ) -> RuleTrackRecord: """Aggregate `row` (a `repo.get_rules()` row) + its paper `stats` into a `RuleTrackRecord`. + `stats` is THIS rule's sample -- `track_records_by_rule(repo, kind)[row["id"]]` -- and the + gate is judged on it, so a gate card is one rule's distance, like its record (#962). + Degrades gracefully -- promotion_class="default", gate=None -- when `row["kind"]` is no - longer in `RULE_REGISTRY` (`agent._build_rule` raises `ValueError` for that), rather than - crashing the whole report over one stale row. + longer in `RULE_REGISTRY`, rather than crashing the whole report over one stale row. """ - try: - rule = agent_mod._build_rule(row) - promotion_class = promotion_class_of(rule) - except ValueError: - rule = None - promotion_class = "default" + rule, promotion_class = _rule_and_class(row) gate: GateDistance | None = None if row["status"] == "paper" and rule is not None: gate = build_gate_distance(rule, stats, default_floor) + product_id = (row.get("params") or {}).get("product_id") return RuleTrackRecord( rule_name=row["kind"], + rule_id=int(row["id"]), + product_id=str(product_id) if product_id is not None else None, status=row["status"], promotion_class=promotion_class, n_trades=stats.n_trades, win_rate=stats.win_rate, - avg_win=stats.avg_win, - avg_loss=stats.avg_loss, + avg_win=_side_average(stats, "win", stats.avg_win), + avg_loss=_side_average(stats, "loss", stats.avg_loss), realized_rr=_realized_rr_for_display(stats), expectancy=stats.expectancy, profit_factor=stats.profit_factor, @@ -756,6 +797,39 @@ def build_rule_track_record( ) +#: The `status` of the record that holds a kind's trades journalled with no rule id. +UNATTRIBUTED = "unattributed" + + +def build_unattributed_track_record(kind_row: dict[str, Any], stats: Any) -> RuleTrackRecord: + """The record of `kind_row`'s KIND's paper trades journalled with no `orders.rule_id`. + + They belong to no one rule: no id, no product, no gate (a gate is one rule's distance to + promotion, and these trades are no rule's evidence). The promotion class is a property of + the kind, so it is read off `kind_row`, a rule of that kind in scope. Shown rather than + dropped -- a hidden trade is a gap in the record nobody can see -- and never folded into a + rule that shares the kind, which would hand that rule another's trades (#962). + """ + _rule, promotion_class = _rule_and_class(kind_row) + return RuleTrackRecord( + rule_name=kind_row["kind"], + rule_id=None, + product_id=None, + status=UNATTRIBUTED, + promotion_class=promotion_class, + n_trades=stats.n_trades, + win_rate=stats.win_rate, + avg_win=_side_average(stats, "win", stats.avg_win), + avg_loss=_side_average(stats, "loss", stats.avg_loss), + realized_rr=_realized_rr_for_display(stats), + expectancy=stats.expectancy, + profit_factor=stats.profit_factor, + max_drawdown=stats.max_drawdown, + significant=stats.n_trades >= 30, + gate=None, + ) + + _PAPER_PIPELINE_STATUSES = {"candidate", "paper"} @@ -785,9 +859,20 @@ def build_insights_report( else: rows = [r for r in rows if r["status"] in _PAPER_PIPELINE_STATUSES] + # One read of the paper journal per KIND, split by the rule that journalled each trade (#962). + by_kind = { + kind: track_records_by_rule(repo, kind) for kind in dict.fromkeys(r["kind"] for r in rows) + } + no_trades = summarize([]) rules = [ - build_rule_track_record(row, track_record(repo, row["kind"]), default_floor) for row in rows + build_rule_track_record(row, by_kind[row["kind"]].get(row["id"], no_trades), default_floor) + for row in rows ] + # Trades journalled with no rule id, after the rules: one record per kind in scope. + for kind, partitions in by_kind.items(): + if None in partitions: + first_row = next(r for r in rows if r["kind"] == kind) + rules.append(build_unattributed_track_record(first_row, partitions[None])) return InsightsReport( now_ts=now_ts, @@ -956,6 +1041,8 @@ def build_journal_report( "proven edge" ) +_UNATTRIBUTED_NOTE = "trades journalled with no rule id -- they belong to no one rule of this kind" + _TWO_DP = Decimal("0.01") @@ -965,8 +1052,9 @@ def _quantized(x: Any) -> Any: Passes through anything that isn't a finite `Decimal` unchanged: a sentinel string ("n/a (no losses yet)", "n/a", "dca"), `None`, or a non-finite `Decimal` (`Infinity`, - which `BacktestResult.profit_factor`/a zero-loss realized-rr can legitimately be, and - which `.quantize()` itself refuses to round) all render exactly as before. + which `BacktestResult.profit_factor` can legitimately be, and which `.quantize()` itself + refuses to round) all render exactly as before. The display realized rr is never + `Infinity`: with no losing (or no winning) trades it is `None`. """ if isinstance(x, Decimal) and x.is_finite(): return x.quantize(_TWO_DP) @@ -1010,13 +1098,33 @@ def render_summary(report: InsightsReport) -> list[str]: lines.append(f"rule track record ({len(report.rules)}):") for r in report.rules: - rr = r.realized_rr if r.realized_rr is not None else "n/a (no losses yet)" + if r.n_trades == 0: + # Nothing was measured: every figure is unknown, not the zero the engine holds. + win_rate = expectancy = profit_factor = max_dd = "n/a" + rr: Any = "n/a (no trades yet)" + else: + win_rate = f"{r.win_rate:.1%}" + expectancy = _money(r.expectancy) + profit_factor = _ratio(r.profit_factor) + max_dd = _money(r.max_drawdown) + if r.realized_rr is not None: + rr = r.realized_rr + elif r.avg_win is None: + rr = "n/a (no wins yet)" + else: + rr = "n/a (no losses yet)" + rule_id = r.rule_id if r.rule_id is not None else "n/a" + avg_win = _money(r.avg_win) if r.avg_win is not None else "n/a" + avg_loss = _money(r.avg_loss) if r.avg_loss is not None else "n/a" lines.append( - f" [{r.rule_name}] status={r.status} class={r.promotion_class} n={r.n_trades} " - f"win_rate={r.win_rate:.1%} avg_win={_money(r.avg_win)} avg_loss={_money(r.avg_loss)} " - f"rr={_ratio(rr)} expectancy={_money(r.expectancy)} " - f"profit_factor={_ratio(r.profit_factor)} max_dd={_money(r.max_drawdown)}" + f" [{r.rule_name}] rule={rule_id} product={r.product_id or 'n/a'} " + f"status={r.status} class={r.promotion_class} n={r.n_trades} " + f"win_rate={win_rate} avg_win={avg_win} avg_loss={avg_loss} " + f"rr={_ratio(rr)} expectancy={expectancy} " + f"profit_factor={profit_factor} max_dd={max_dd}" ) + if r.status == UNATTRIBUTED: + lines.append(f" {_UNATTRIBUTED_NOTE}") if not r.significant: lines.append(f" {_SMALL_SAMPLE_NOTE}") if r.gate is not None: diff --git a/keel/strategy/paper.py b/keel/strategy/paper.py index d631d95b..fde5320d 100644 --- a/keel/strategy/paper.py +++ b/keel/strategy/paper.py @@ -468,18 +468,56 @@ def track_record(repo: Repository, rule_name: str) -> BacktestResult: """Aggregate `rule_name`'s paper trades (from `orders(mode='paper')`) into a `BacktestResult`-shaped summary, directly comparable to `backtest.backtest()`'s historical output. + + `rule_name` is a rule KIND (`rules.kind`), so this pools every rule of that kind -- all + products, every lifecycle status. That pool is what `insights`' journal matches a closed + trade's R against (it has the kind, not the rule). A per-RULE record is + `track_records_by_rule`'s (#962). """ - orders = repo.get_orders(mode="paper") + return summarize(_journalled_trades(_paper_payloads(repo, rule_name))) - entries: dict[int, dict] = {} - exits: list[dict] = [] - for order in orders: + +def track_records_by_rule(repo: Repository, rule_name: str) -> dict[int | None, BacktestResult]: + """`track_record(repo, rule_name)` split by the rule that journalled each trade (#962). + + Keyed by `orders.rule_id`: a real paper cycle writes it on the entry, and the exit copies + the entry's (see `_OpenPaperPosition.rule_id`), so a round trip never straddles two keys. + A fill written with NO `rule_id` (a hand-built signal, or a fill journalled before the + column was populated) is keyed `None` -- it belongs to no rule, and folding it into + whichever rule shares its kind would hand one rule another's trades. Only keys with at + least one journalled order appear; a rule that never traded has no entry here, and its + record is the empty sample, which the caller builds. + + The partitions are disjoint and cover the kind: their `n_trades` sum to + `track_record(repo, rule_name).n_trades`. Path-dependent figures (drawdown, losing streak) + are each rule's own, over its own trades in journal order. + """ + partitions: dict[int | None, list[tuple[dict, dict]]] = {} + for order, payload in _paper_payloads(repo, rule_name): + key = order.get("rule_id") + partitions.setdefault(int(key) if key is not None else None, []).append((order, payload)) + return {key: summarize(_journalled_trades(rows)) for key, rows in partitions.items()} + + +def _paper_payloads(repo: Repository, rule_name: str) -> list[tuple[dict, dict]]: + """Every journalled paper order of kind `rule_name`, with its decoded payload, oldest first.""" + found: list[tuple[dict, dict]] = [] + for order in repo.get_orders(mode="paper"): raw = order["raw_response"] if not raw: continue payload = json.loads(raw) if payload.get("rule_name") != rule_name: continue + found.append((order, payload)) + return found + + +def _journalled_trades(rows: list[tuple[dict, dict]]) -> list[Trade]: + """Pair entry and exit payloads into `Trade`s; an entry with no exit is an open trade.""" + entries: dict[int, dict] = {} + exits: list[dict] = [] + for order, payload in rows: if payload.get("role") == "entry": entries[order["id"]] = payload elif payload.get("role") == "exit": @@ -533,4 +571,4 @@ def track_record(repo: Repository, rule_name: str) -> BacktestResult: ) ) - return summarize(trades) + return trades diff --git a/keel/web/payload.py b/keel/web/payload.py index 5c1d40b6..fc7dcb74 100644 --- a/keel/web/payload.py +++ b/keel/web/payload.py @@ -1148,10 +1148,20 @@ def _track_record_payload(record: RuleTrackRecord) -> dict[str, Any]: """One rule's track record. At zero closed trades every statistic is UNKNOWN, not zero: the sample is empty, so there is no win rate to be 0% and no average to be $0.00 (the report holds 0 because `BacktestResult` does; serialising that as data would assert a measurement). The - trade count stays a real number. Win rate is in percent units (see `_gate_payload`).""" + trade count stays a real number. Win rate is in percent units (see `_gate_payload`). + + One side of a TRADED sample can be empty too -- all wins, so no average loss (#965). The + report says so itself (`avg_win`/`avg_loss` are `None`), and `money(None)` is `absent()`: + nothing here works the empty side out from the win rate and the count. + + `rule_name` is the rule's KIND; `rule_id` and `product_id` are what tell two rules of one kind + apart (#962). Identifiers, so bare strings like `_rule_row_payload`'s `id`; `None` on the + record of trades journalled with no rule id, which has neither.""" traded = record.n_trades != 0 return { "rule_name": record.rule_name, + "rule_id": str(record.rule_id) if record.rule_id is not None else None, + "product_id": record.product_id, "status": label(record.status), "promotion_class": record.promotion_class, "n_trades": count(record.n_trades), diff --git a/keel/web/static/js/render.js b/keel/web/static/js/render.js index 66a86bbb..4616f901 100644 --- a/keel/web/static/js/render.js +++ b/keel/web/static/js/render.js @@ -2460,6 +2460,8 @@ function recordsSection(data) { "h-records", [ { label: "rule", numeric: false }, + { label: "id", numeric: false }, + { label: "product", numeric: false }, { label: "status", numeric: false }, { label: "class", numeric: false }, { label: "trades", numeric: true }, @@ -2475,6 +2477,8 @@ function recordsSection(data) { records.map( /** @param {any} row */ (row) => [ plain(row.rule_name), + plain(row.rule_id) || "—", + plain(row.product_id) || "—", row.status, plain(row.promotion_class), row.n_trades, @@ -2493,7 +2497,7 @@ function recordsSection(data) { ); fragment.append( note( - "Rules in the promotion pipeline (candidate and paper). Below 30 closed trades a win rate is not yet distinguishable from random entry, which is why the sample column says so rather than leaving the number to speak for itself. A live rule's record is `keel insights summary --mode live` at a terminal.", + "Rules in the promotion pipeline (candidate and paper), one record per rule: each holds the paper trades that rule journalled. A record with no id holds the trades of that kind journalled with no rule id, which belong to no one rule. Below 30 closed trades a win rate is not yet distinguishable from random entry, which is why the sample column says so rather than leaving the number to speak for itself. A live rule's record is `keel insights summary --mode live` at a terminal.", ), ); @@ -2503,7 +2507,7 @@ function recordsSection(data) { fragment.append( note("Only rules in paper have one: a candidate has not backtested yet, and a live rule has already cleared it."), ); - for (const [index, row] of gated.entries()) fragment.append(gateCard(row, index)); + for (const row of gated) fragment.append(gateCard(row)); } return fragment; } @@ -2530,25 +2534,31 @@ function samplePill(significant) { * both and computes neither: "`passing` is the engine's verdict, copied; the floors are the * config's own values, copied. Nothing here re-runs `check_floors`." * - * The summary's id carries the card's POSITION as well as the rule's name. A record is named by - * its rule KIND, and a ledger can hold several paper rules of one kind, so a name-only id repeats - * -- and `rebuildInto` reopens disclosures by `summary[id]`, so opening one such card reopened - * every card of that kind on the next poll. + * The card is one RULE's (#962): `rule_name` is the rule's kind, and a ledger can hold several + * paper rules of one kind, so the summary names the kind, the product and the ledger id -- the + * Holdings heading's order -- and the id is built from the ledger id. A gate is only built for a + * ledger row, so every gated record carries one. * - * @param {any} rule - * @param {number} position the card's place among the gated records, for its id. + * @param {any} rule one record of `/api/rules`'s `records`, with a `gate`. * @returns {HTMLElement} */ -function gateCard(rule, position) { +function gateCard(rule) { const gate = rule.gate; const node = el("details", "gate"); const summary = el("summary"); // An id on every summary, and it is not decoration: `main.js` restores which disclosures were // open across a rebuild by matching these, so a reader who expanded one does not watch it shut - // the moment the engine writes a row. Derived from the row it describes and its place in the - // service's order, so it is stable across a re-read that did not change the ledger. - summary.id = ["h-gatedist", plain(gate.rule_name), String(position)].join("-"); - summary.append(plain(gate.rule_name), " · "); + // the moment the engine writes a row. The rule's ledger id: its own, and stable across a + // re-read and a reorder alike. (A kind-only id reopened every card of that kind, #953.) + summary.id = ["h-gatedist", plain(rule.rule_id)].join("-"); + summary.append( + [ + plain(rule.rule_name), + plain(rule.product_id) || "—", + "rule ".concat(plain(rule.rule_id)), + ].join(" · "), + " · ", + ); summary.append(field(gate.passing)); node.append(summary); diff --git a/tests/commands/test_insights.py b/tests/commands/test_insights.py index 27b0154e..373cd7af 100644 --- a/tests/commands/test_insights.py +++ b/tests/commands/test_insights.py @@ -12,6 +12,7 @@ import json from decimal import Decimal +from types import SimpleNamespace from typing import Any import pytest @@ -23,6 +24,7 @@ InsightsReport, JournalEntry, RuleTrackRecord, + _realized_rr_for_display, build_account_summary, build_equity_curve, build_gate_distance, @@ -107,6 +109,7 @@ def _seed_paper_trade( pnl: str = "10", r_multiple: str | None = "1.0", outcome: str = "win", + rule_id: int | None = None, ) -> tuple[int, int]: """Write an entry+exit paper order pair directly (matching `paper.py`'s own payload shape) so tests can control exact win/loss/r_multiple stats without simulating candles.""" @@ -134,7 +137,7 @@ def _seed_paper_trade( } ), confirmation="paper", - rule_id=None, + rule_id=rule_id, created_at=entry_ts, updated_at=entry_ts, ) @@ -169,7 +172,7 @@ def _seed_paper_trade( } ), confirmation="paper", - rule_id=None, + rule_id=rule_id, created_at=exit_ts, updated_at=exit_ts, ) @@ -305,6 +308,18 @@ def test_rule_track_record_realized_rr_none_when_no_losses(repo: Repository) -> assert record.realized_rr is None +def test_realized_rr_for_display_counts_losing_trades_not_the_average() -> None: + """Both empty sides are decided by COUNTING trades (like `_side_average`): a sample with a + win and a non-zero `avg_loss` but no trade whose outcome is "loss" has no loss side.""" + stats = SimpleNamespace( + trades=[SimpleNamespace(outcome="win")], + avg_win=Decimal("10"), + avg_loss=Decimal("-5"), + ) + + assert _realized_rr_for_display(stats) is None + + def test_rule_track_record_degrades_gracefully_for_unrecognized_kind(repo: Repository) -> None: """A stale `rules` row whose `kind` is no longer in `RULE_REGISTRY` must not crash the whole report -- `_build_rule` raises `ValueError`, which degrades to a default class and a @@ -426,9 +441,11 @@ def test_a_real_records_win_rate_is_displayed_as_the_clis_percentage(repo: Repos """`win_rate` is a 0..1 fraction (`wins / n`); `percent()` takes percent units, so the page once showed 40% as "0.4%". Drive a REAL report through `rules_payload` and compare with the CLI's own `f"{win_rate:.1%}"` -- for the record, the gate's figure and the gate's floor.""" - repo.insert_rule("turtle_breakout", {"product_id": "BTC-USD"}, status="paper") + rule_id = repo.insert_rule("turtle_breakout", {"product_id": "BTC-USD"}, status="paper") for i in range(2): - _seed_paper_trade(repo, "turtle_breakout", entry_ts=1000 + i, exit_ts=1100 + i) + _seed_paper_trade( + repo, "turtle_breakout", rule_id=rule_id, entry_ts=1000 + i, exit_ts=1100 + i + ) for i in range(3): _seed_paper_trade( repo, @@ -439,6 +456,7 @@ def test_a_real_records_win_rate_is_displayed_as_the_clis_percentage(repo: Repos outcome="loss", entry_ts=2000 + i, exit_ts=2100 + i, + rule_id=rule_id, ) config = _config() report = build_insights_report(repo, config, gather_status(repo, config, now_ts=NOW_TS), NOW_TS) @@ -455,9 +473,11 @@ def test_a_real_records_win_rate_is_displayed_as_the_clis_percentage(repo: Repos def test_the_cli_summary_still_prints_the_fraction_as_it_always_did(repo: Repository) -> None: """The percent-unit value is carried beside the fraction; the CLI line and the gate floor figures are untouched.""" - repo.insert_rule("turtle_breakout", {"product_id": "BTC-USD"}, status="paper") + rule_id = repo.insert_rule("turtle_breakout", {"product_id": "BTC-USD"}, status="paper") for i in range(2): - _seed_paper_trade(repo, "turtle_breakout", entry_ts=1000 + i, exit_ts=1100 + i) + _seed_paper_trade( + repo, "turtle_breakout", rule_id=rule_id, entry_ts=1000 + i, exit_ts=1100 + i + ) config = _config() report = build_insights_report(repo, config, gather_status(repo, config, now_ts=NOW_TS), NOW_TS) @@ -466,8 +486,9 @@ def test_the_cli_summary_still_prints_the_fraction_as_it_always_did(repo: Reposi gate = [line for line in lines if line.startswith(" gate:")] assert record == [ - " [turtle_breakout] status=paper class=trend_follow n=2 win_rate=100.0% avg_win=10.00 " - "avg_loss=0.00 rr=n/a (no losses yet) expectancy=10.00 profit_factor=Infinity max_dd=0.00" + f" [turtle_breakout] rule={rule_id} product=BTC-USD status=paper class=trend_follow " + "n=2 win_rate=100.0% avg_win=10.00 avg_loss=n/a rr=n/a (no losses yet) " + "expectancy=10.00 profit_factor=Infinity max_dd=0.00" ] assert gate == [ " gate: blocked -- trades_remaining=98 (n>=100, win_rate>=0.3, rr>=1.5, expectancy>0)" @@ -496,8 +517,8 @@ def test_a_rule_with_no_closed_trades_shows_its_statistics_as_unknown_not_zero( def test_a_rule_with_one_closed_trade_still_shows_numbers(repo: Repository) -> None: - repo.insert_rule("turtle_breakout", {"product_id": "BTC-USD"}, status="paper") - _seed_paper_trade(repo, "turtle_breakout") + rule_id = repo.insert_rule("turtle_breakout", {"product_id": "BTC-USD"}, status="paper") + _seed_paper_trade(repo, "turtle_breakout", rule_id=rule_id) shown = _rules_records(repo)[0] @@ -508,6 +529,269 @@ def test_a_rule_with_one_closed_trade_still_shows_numbers(repo: Repository) -> N assert shown["gate"]["win_rate"]["display"] == "100.0%" +# -- one record per rule, not per kind (#962) ---------------------------------------------------- + + +def _report(repo: Repository, **kwargs: Any) -> InsightsReport: + config = _config() + return build_insights_report( + repo, config, gather_status(repo, config, now_ts=NOW_TS), NOW_TS, **kwargs + ) + + +def _seed_two_turtles(repo: Repository) -> tuple[int, int]: + """Two paper rules of ONE kind: BTC with two wins, ETH with one loss.""" + btc = repo.insert_rule("turtle_breakout", {"product_id": "BTC-USD"}, status="paper") + eth = repo.insert_rule("turtle_breakout", {"product_id": "ETH-USD"}, status="paper") + for i in range(2): + _seed_paper_trade(repo, "turtle_breakout", rule_id=btc, entry_ts=1000 + i, exit_ts=1100 + i) + _seed_paper_trade( + repo, + "turtle_breakout", + rule_id=eth, + product_id="ETH-USD", + exit_price="90", + pnl="-10", + r_multiple="-1.0", + outcome="loss", + entry_ts=3000, + exit_ts=3100, + ) + return btc, eth + + +def test_rules_of_one_kind_each_get_their_own_record(repo: Repository) -> None: + """#962: the records were keyed by KIND, so two turtle rules showed the kind's pooled figures + twice with nothing to tell them apart. Each record is now its own rule's trades, named by its + ledger id and product, and so is its gate.""" + btc, eth = _seed_two_turtles(repo) + + records = _report(repo).rules + + assert [(r.rule_id, r.product_id, r.rule_name) for r in records] == [ + (btc, "BTC-USD", "turtle_breakout"), + (eth, "ETH-USD", "turtle_breakout"), + ] + assert [(r.n_trades, r.win_rate) for r in records] == [(2, 1.0), (1, 0.0)] + gates = [r.gate for r in records] + assert all(g is not None for g in gates) + assert [(g.n_trades, g.win_rate) for g in gates if g is not None] == [(2, 1.0), (1, 0.0)] + + +def test_a_rule_outside_the_scope_lends_its_trades_to_no_record(repo: Repository) -> None: + """A disabled rule's trades were pooled into the paper rule of the same kind. They are its + own, and it is not in the pipeline, so they appear nowhere in this report.""" + retired = repo.insert_rule("turtle_breakout", {"product_id": "BTC-USD"}, status="disabled") + paper = repo.insert_rule("turtle_breakout", {"product_id": "ETH-USD"}, status="paper") + _seed_paper_trade(repo, "turtle_breakout", rule_id=retired) + + records = _report(repo).rules + + assert [(r.rule_id, r.n_trades) for r in records] == [(paper, 0)] + + +def test_trades_with_no_rule_id_get_one_record_per_kind_after_the_rules( + repo: Repository, +) -> None: + """A fill journalled with no `rule_id` belongs to no rule. It is shown -- dropping it would + hide a trade -- as ONE record for its kind, after the rules, with no id, no product and no + gate, never folded into a rule that shares the kind.""" + btc, eth = _seed_two_turtles(repo) + _seed_paper_trade(repo, "turtle_breakout", entry_ts=5000, exit_ts=5100) + _seed_paper_trade(repo, "turtle_breakout", entry_ts=6000, exit_ts=6100) + + records = _report(repo).rules + + assert [(r.rule_id, r.status, r.n_trades) for r in records] == [ + (btc, "paper", 2), + (eth, "paper", 1), + (None, "unattributed", 2), + ] + unattributed = records[-1] + assert unattributed.rule_name == "turtle_breakout" + assert unattributed.product_id is None + assert unattributed.gate is None + assert unattributed.promotion_class == records[0].promotion_class + + +def test_a_rule_with_no_trades_of_its_own_is_not_lent_the_unattributed_ones( + repo: Repository, +) -> None: + """The one rule of its kind, and no trade of its own: its record is the empty sample. The + unattributed trades are their own record, however natural it would be to hand them over.""" + rule = repo.insert_rule("turtle_breakout", {"product_id": "BTC-USD"}, status="paper") + _seed_paper_trade(repo, "turtle_breakout") + + records = _report(repo).rules + + assert [(r.rule_id, r.n_trades) for r in records] == [(rule, 0), (None, 1)] + assert records[0].gate is not None and records[0].gate.n_trades == 0 + + +def test_unattributed_trades_of_a_kind_outside_the_scope_are_not_shown(repo: Repository) -> None: + """The unattributed record follows the kinds in scope: `--rule dca` shows no turtle row.""" + repo.insert_rule("dca", {"product_id": "BTC-USD"}, status="paper") + _seed_paper_trade(repo, "turtle_breakout") + + assert [r.rule_name for r in _report(repo).rules] == ["dca"] + assert _report(repo, rule_filter="turtle_breakout").rules == [] + + +def test_the_cli_names_each_records_rule_and_product(repo: Repository) -> None: + btc, eth = _seed_two_turtles(repo) + _seed_paper_trade(repo, "turtle_breakout", entry_ts=5000, exit_ts=5100) + + lines = render_summary(_report(repo)) + heads = [line.split(" status=")[0] for line in lines if line.startswith(" [")] + + assert heads == [ + f" [turtle_breakout] rule={btc} product=BTC-USD", + f" [turtle_breakout] rule={eth} product=ETH-USD", + " [turtle_breakout] rule=n/a product=n/a", + ] + assert ( + " trades journalled with no rule id -- they belong to no one rule of this kind" in lines + ) + + +def test_the_json_summary_carries_each_records_rule_id(tmp_path, valid_config_path) -> None: + """`--json` is `asdict(report)`: the records gain `rule_id` and `product_id`, and keep every + key they had.""" + db_path = tmp_path / "keel.db" + repo = _repo_at(db_path) + btc, eth = _seed_two_turtles(repo) + repo._conn.commit() + + result = CliRunner().invoke( + cli, + ["--config", str(valid_config_path), "--db", str(db_path), "insights", "summary", "--json"], + ) + + assert result.exit_code == 0, result.output + rules = json.loads(result.output)["rules"] + assert [(r["rule_id"], r["product_id"], r["n_trades"]) for r in rules] == [ + (btc, "BTC-USD", 2), + (eth, "ETH-USD", 1), + ] + assert set(rules[0]) == { + "rule_name", + "rule_id", + "product_id", + "status", + "promotion_class", + "n_trades", + "win_rate", + "avg_win", + "avg_loss", + "realized_rr", + "expectancy", + "profit_factor", + "max_drawdown", + "significant", + "gate", + } + + +# -- an empty side is unknown, not $0.00 (#965) --------------------------------------------------- + + +def test_an_all_wins_record_has_no_average_loss(repo: Repository) -> None: + btc, _eth = _seed_two_turtles(repo) + + record = next(r for r in _report(repo).rules if r.rule_id == btc) + + assert record.avg_win == Decimal("10") + assert record.avg_loss is None + + +def test_an_all_losses_record_has_no_average_win(repo: Repository) -> None: + _btc, eth = _seed_two_turtles(repo) + + record = next(r for r in _report(repo).rules if r.rule_id == eth) + + assert record.avg_win is None + assert record.avg_loss == Decimal("-10") + + +def test_an_empty_side_reads_unknown_on_the_rules_page(repo: Repository) -> None: + _seed_two_turtles(repo) + + wins_only, losses_only = _rules_records(repo) + + assert wins_only["avg_loss"]["display"] == "—" + assert wins_only["avg_loss"]["state"] == "unknown" + assert wins_only["avg_win"]["display"] != "—" + assert losses_only["avg_win"]["display"] == "—" + assert losses_only["avg_win"]["state"] == "unknown" + assert losses_only["avg_loss"]["display"] != "—" + + +def test_an_empty_side_prints_n_a_at_the_terminal(repo: Repository) -> None: + btc, eth = _seed_two_turtles(repo) + + lines = [line for line in render_summary(_report(repo)) if line.startswith(" [")] + + assert "avg_win=10.00 avg_loss=n/a rr=" in lines[0] + assert "avg_win=n/a avg_loss=-10.00 rr=" in lines[1] + + +def test_an_empty_side_leaves_the_gate_verdict_exactly_as_check_floors_gives_it( + repo: Repository, +) -> None: + """#965 is display only. The engine's sample still holds 0 for the empty side -- `promotion` + reads it (`pool_stats` weights it by a zero count; `_realized_rr` turns a zero `avg_loss_r` + into Infinity) -- and the gate's verdict and reasons are `check_floors` on that sample.""" + from keel.strategy.paper import track_records_by_rule + from keel.strategy.promotion import check_floors + + btc, eth = _seed_two_turtles(repo) + by_rule = track_records_by_rule(repo, "turtle_breakout") + assert by_rule[btc].avg_loss == Decimal(0) and by_rule[eth].avg_win == Decimal(0) + floor = floor_for_class("trend_follow", default=_default_floor(_config())) + + for record in _report(repo).rules: + assert record.rule_id is not None and record.gate is not None + assert (record.gate.passing, record.gate.blocking_reasons) == check_floors( + by_rule[record.rule_id], floor + ) + + +def test_an_all_losses_record_has_no_realized_rr_and_says_why(repo: Repository) -> None: + """No winning trade means no average win to divide: `realized_rr` is unknown (None), not the + 0.00 that `0 / |avg_loss|` gives. The reason is told apart from "no losses yet" by which side + of the record is empty. The page's gate card and record row both read "—" (`ratio(None)`).""" + _btc, eth = _seed_two_turtles(repo) + + record = next(r for r in _report(repo).rules if r.rule_id == eth) + line = next(ln for ln in render_summary(_report(repo)) if f"rule={eth} " in ln) + shown = _rules_records(repo)[1] + + assert record.realized_rr is None + assert record.gate is not None and record.gate.realized_rr is None + assert "avg_win=n/a avg_loss=-10.00 rr=n/a (no wins yet) " in line + assert "(no losses yet)" not in line + assert shown["realized_rr"]["display"] == "—" + assert shown["gate"]["realized_rr"]["display"] == "—" + + +def test_a_zero_trade_record_prints_every_figure_as_n_a_at_the_terminal(repo: Repository) -> None: + """Unknown shown as zero, in the terminal: with no trades there is no win rate, expectancy, + profit factor or drawdown to print. The page already says "—"; `--json` keeps its numbers + (n_trades=0 tells a reader they are placeholders).""" + rule_id = repo.insert_rule("turtle_breakout", {"product_id": "BTC-USD"}, status="paper") + report = _report(repo) + + lines = render_summary(report) + record = [line for line in lines if line.startswith(" [turtle_breakout]")] + + assert record == [ + f" [turtle_breakout] rule={rule_id} product=BTC-USD status=paper class=trend_follow " + "n=0 win_rate=n/a avg_win=n/a avg_loss=n/a rr=n/a (no trades yet) " + "expectancy=n/a profit_factor=n/a max_dd=n/a" + ] + assert report.rules[0].win_rate == 0 and report.rules[0].expectancy == 0 + + # -- build_journal_report ----------------------------------------------------------------------- @@ -759,6 +1043,8 @@ def test_render_summary_quantizes_ratio_to_two_decimal_places() -> None: ) rule = RuleTrackRecord( rule_name="dca", + rule_id=7, + product_id="BTC-USD", status="live", promotion_class="default", n_trades=40, diff --git a/tests/strategy/test_paper.py b/tests/strategy/test_paper.py index 008d9014..d5508d3c 100644 --- a/tests/strategy/test_paper.py +++ b/tests/strategy/test_paper.py @@ -16,7 +16,12 @@ from keel.data.db import connect, migrate from keel.data.repository import Repository from keel.strategy.backtest import BacktestResult -from keel.strategy.paper import _DEFAULT_FEE_PCT, PaperTrader, track_record +from keel.strategy.paper import ( + _DEFAULT_FEE_PCT, + PaperTrader, + track_record, + track_records_by_rule, +) from keel.strategy.rules.base import Action, Setup, Signal from keel.types import Candle, Side @@ -404,6 +409,54 @@ def test_open_position_included_in_trades_but_not_aggregate_stats( assert len(result.trades) == 1 assert result.trades[0].outcome == "open" + def _closed( + self, repo: Repository, rule_name: str, rule_id: int | None, ts: int, *, win: bool + ) -> None: + trader = PaperTrader(repo) + setup = _setup(entry="100", stop="90", target="120", ts=ts) + trader.on_signal(_enter_signal(rule_name=rule_name, setup=setup, rule_id=rule_id)) + if win: + trader.on_candle("BTC-USD", _candle(ts + 60, "115", "121", "114", "120")) + else: + trader.on_candle("BTC-USD", _candle(ts + 60, "95", "96", "89", "90")) + + def test_by_rule_splits_one_kind_into_each_rules_own_record(self, repo: Repository) -> None: + """#962: N rules of one kind are N records, each from the trades its own rule journalled + (`orders.rule_id`), not N copies of the kind's pooled record.""" + first = repo.insert_rule("turtle_breakout", {"product_id": "BTC-USD"}, status="paper") + second = repo.insert_rule("turtle_breakout", {"product_id": "ETH-USD"}, status="paper") + self._closed(repo, "turtle_breakout", first, 1_000, win=True) + self._closed(repo, "turtle_breakout", first, 2_000, win=True) + self._closed(repo, "turtle_breakout", second, 3_000, win=False) + self._closed(repo, "other_kind", second, 4_000, win=True) + + by_rule = track_records_by_rule(repo, "turtle_breakout") + + assert sorted(by_rule, key=str) == [first, second] + assert (by_rule[first].n_trades, by_rule[first].win_rate) == (2, 1.0) + assert (by_rule[second].n_trades, by_rule[second].win_rate) == (1, 0.0) + # Every trade of the kind lands in exactly one partition: nothing lost, nothing doubled. + whole = track_record(repo, "turtle_breakout") + assert sum(r.n_trades for r in by_rule.values()) == whole.n_trades == 3 + + def test_by_rule_keeps_trades_journalled_without_a_rule_id_apart( + self, repo: Repository + ) -> None: + """A paper fill written with no `rule_id` (a hand-built signal, or a fill journalled before + `orders.rule_id` was populated) belongs to no rule. It is its own partition, keyed `None`, + never folded into a rule that happens to share the kind.""" + rule = repo.insert_rule("turtle_breakout", {"product_id": "BTC-USD"}, status="paper") + self._closed(repo, "turtle_breakout", rule, 1_000, win=True) + self._closed(repo, "turtle_breakout", None, 2_000, win=False) + + by_rule = track_records_by_rule(repo, "turtle_breakout") + + assert (by_rule[rule].n_trades, by_rule[rule].win_rate) == (1, 1.0) + assert (by_rule[None].n_trades, by_rule[None].win_rate) == (1, 0.0) + + def test_by_rule_has_no_partition_for_a_kind_with_no_trades(self, repo: Repository) -> None: + assert track_records_by_rule(repo, "turtle_breakout") == {} + def test_no_paper_trades_yields_empty_result(self, repo: Repository) -> None: result = track_record(repo, "unknown_rule") diff --git a/tests/web/test_payload.py b/tests/web/test_payload.py index 2206e344..c050d670 100644 --- a/tests/web/test_payload.py +++ b/tests/web/test_payload.py @@ -156,6 +156,8 @@ def _insights_report(**overrides: Any) -> InsightsReport: rules=[ RuleTrackRecord( rule_name="turtle_breakout", + rule_id=10, + product_id="BTC-USD", status="paper", promotion_class="trend", n_trades=12, @@ -878,6 +880,34 @@ def test_the_closed_trade_count_is_read_from_the_report_not_measured_here() -> N assert len(built["records"]) == 1 +def test_each_track_record_names_its_rule_by_ledger_id_and_product() -> None: + """#962: a record's `rule_name` is its KIND, so several rules of one kind read alike; the + ledger id and product tell them apart. Identifiers, so bare strings like `_rule_row_payload`'s + `id`. A record of trades journalled with no rule id has neither, and sends `None` -- never an + empty string the client would draw as a blank id.""" + attributed = _insights_report().rules[0] + unattributed = replace(attributed, rule_id=None, product_id=None, status="unattributed") + + built = payload.rules_payload([], records=_insights_report(rules=[attributed, unattributed])) + + assert [(r["rule_name"], r["rule_id"], r["product_id"]) for r in built["records"]] == [ + ("turtle_breakout", "10", "BTC-USD"), + ("turtle_breakout", None, None), + ] + assert built["records"][1]["status"]["display"] == "unattributed" + + +def test_an_empty_side_of_the_sample_is_absent_not_zero() -> None: + """#965: the report holds `None` for an average with no trade on its side; the page draws the + absent mark, not $0.00, and the other side keeps its figure.""" + record = replace(_insights_report().rules[0], avg_loss=None) + + built = payload.rules_payload([], records=_insights_report(rules=[record]))["records"][0] + + assert built["avg_loss"] == payload.absent() + assert built["avg_win"]["display"] == "$31.20" + + def test_every_field_carries_all_three_keys_always() -> None: """Schema uniformity is itself part of the contract: a client that has to test whether `state` is present is branching on payload SHAPE, which is inference by another route. Every field diff --git a/tests/web/test_rules_view.py b/tests/web/test_rules_view.py index 32a0fe5a..8a67fb01 100644 --- a/tests/web/test_rules_view.py +++ b/tests/web/test_rules_view.py @@ -151,6 +151,8 @@ def _assert_pairs(body: str, table_id: str, expected: tuple[tuple[str, str], ... _RECORD_PAIRS = ( ("rule", "rule_name"), + ("id", "rule_id"), + ("product", "product_id"), ("status", "status"), ("class", "promotion_class"), ("trades", "n_trades"), @@ -181,6 +183,27 @@ def test_the_track_records_offer_no_sort_control() -> None: assert re.findall(r"\brecordsSection\(([^)]*)\)", _builder("rulesView")) == ["data"] +def test_the_track_records_are_drawn_in_the_order_they_arrive() -> None: + """The server puts the ledger's rules first and the unattributed record last, and promises it. + The client must not undo that: the rows argument of `table("h-records", ...)` is `records.map(` + itself, and no use of `records` in the section reorders, copies or slices it.""" + body = _builder("recordsSection") + found = re.search(r'table\(\s*"h-records"', body) + assert found is not None + columns, after = _balanced_block(body, body.index("[", found.start())) + rows_argument = body[after:].lstrip(" \n,") + + assert rows_argument.startswith("records.map(") + # Every method called on the `records` binding (not `data.records`) is `.map(` -- the table + # rows -- or `.filter(` -- the gated subset, which keeps order. None hands it to a function. + methods = re.findall(r"(? None: """`GateDistance` is `None` for every status but `paper`, so the branch is the presence of the payload's object -- never a figure read and compared here.""" @@ -192,22 +215,34 @@ def test_the_gate_cards_are_drawn_on_the_presence_of_a_gate() -> None: def test_every_gate_card_gets_an_id_of_its_own() -> None: - """A track record is named by its rule KIND, and a ledger can hold several paper rules of one - kind (the browser check found five `turtle_breakout` cards). An id built from the name alone - repeats, and `rebuildInto` restores open disclosures by `summary[id]` -- so opening one card - reopened all five on the next poll. The id carries the card's position as well.""" + """Several paper rules share a kind (the browser check found five `turtle_breakout` cards), and + `rebuildInto` restores open disclosures by `summary[id]` -- so an id built from the kind + reopened every card of that kind on the next poll. #953 added the card's position; #962 gives + each record its ledger id, which is the rule's own and survives a reorder, so the id is that. + A gate is only ever built for a ledger row, so every gated record has one.""" section = _builder("recordsSection") card = _builder("gateCard") - assert re.findall(r"function gateCard\((\w+), (\w+)\)", _code_only(_source("render.js"))) == [ - ("rule", "position") - ] - assert re.findall(r"for \(const \[(\w+), (\w+)\] of gated\.entries\(\)\)", section) == [ - ("index", "row") - ] - assert re.findall(r"gateCard\(([^)]*)\)", section) == ["row, index"] + assert re.findall(r"function gateCard\(([^)]*)\)", _code_only(_source("render.js"))) == ["rule"] + assert re.findall(r"gateCard\(([^)]*)\)", section) == ["row"] ids = re.findall(r"summary\.id = (.*);", card) - assert ids == ['["h-gatedist", plain(gate.rule_name), String(position)].join("-")'], ids + assert ids == ['["h-gatedist", plain(rule.rule_id)].join("-")'], ids + + +def test_each_gate_card_names_its_rule_by_kind_product_and_id() -> None: + """#962: "turtle_breakout · blocked" five times named no rule. The summary reads the record's + kind, product and ledger id -- in that order, the Holdings heading's -- then the verdict.""" + card = _builder("gateCard") + summary = card[card.index('el("summary")') : card.index("node.append(summary)")] + + assert re.findall(r"\brule\.(\w+)", summary) == [ + "rule_id", + "rule_name", + "product_id", + "rule_id", + ] + assert re.findall(r"\bgate\.(\w+)", summary) == ["passing"] + assert summary.index("rule.product_id") < summary.index("gate.passing") _GATE_PAIRS = (