From 5d79187e738c2a93188be14721453f77f2b7dc69 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Thu, 24 Sep 2026 16:56:19 -0400 Subject: [PATCH 1/2] fix(agent): the cycle line counts rail vetoes and names the rail (#812) `signals=3 blocked=0 entered=0` read as a silent drop when the rails had refused every entry: `blocked` counts only entries withheld before evaluation. The line now ends with `vetoed=N (rail, ...)`, one per entry, each rail named once by its leading clause. Appended after `exited=` so the `signals=` token keel-live-run.sh greps is untouched. Co-Authored-By: Claude Opus 5.5 --- keel/commands/trading.py | 23 +++++++- tests/commands/test_service_parity.py | 80 +++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 1 deletion(-) diff --git a/keel/commands/trading.py b/keel/commands/trading.py index 6f7a8e9f..912bb323 100644 --- a/keel/commands/trading.py +++ b/keel/commands/trading.py @@ -19,6 +19,7 @@ from keel import agent from keel.data.repository import Repository from keel.execution import equity as equity_mod +from keel.execution.executor import ExecutionResult # -- the typed gates' wording, and the output lines (their ONE home) ------------------------------ # @@ -162,6 +163,26 @@ def reset_high_water_mark(repo: Repository) -> None: repo.set_state("equity_history", []) +def _vetoed_token(enter_results: list[ExecutionResult]) -> str: + """`vetoed=N (rail, ...)` -- the entries the rails refused after evaluation (#812). + + `blocked` counts only entries withheld BEFORE evaluation, so without this a vetoed cycle + printed `signals=3 blocked=0 entered=0`, which reads as a silent drop. N is one per entry, + not one per rail: an entry tripping two rails is still one setup. Each rail is named once, + by the leading clause of its violation string (`"account_dd_breaker_weekly: drawdown ..."` + -> `account_dd_breaker_weekly`); the arithmetic stays in the JSON log. Appended after + `exited=` so the `signals=[0-9]+` token the live runner greps is untouched. + """ + vetoed = [r for r in enter_results if not r.placed and r.vetoed_by] + rails: list[str] = [] + for r in vetoed: + for violation in r.vetoed_by: + rail = violation.split(":", 1)[0].strip() or violation.strip() + if rail not in rails: + rails.append(rail) + return f"vetoed={len(vetoed)} ({', '.join(rails)})" if rails else f"vetoed={len(vetoed)}" + + def render_loop_result(result: agent.LoopResult) -> list[str]: """The exact lines `keel agent` prints for one cycle -- the shared twin every front-end shows, so a cycle reads identically wherever it is rendered.""" @@ -173,7 +194,7 @@ def render_loop_result(result: agent.LoopResult) -> list[str]: f"[{result.ts}] mode={result.mode} polled={result.polled} " f"products={result.products} stale={result.stale_products} " f"signals={len(result.enter_signals)} blocked={len(result.blocked_entries)} " - f"entered={entered} exited={exited}" + f"entered={entered} exited={exited} {_vetoed_token(result.enter_results)}" ] if result.paper_equity is not None: lines.append( diff --git a/tests/commands/test_service_parity.py b/tests/commands/test_service_parity.py index 88c2b582..6efa593b 100644 --- a/tests/commands/test_service_parity.py +++ b/tests/commands/test_service_parity.py @@ -680,3 +680,83 @@ def test_agent_cycle_lines_come_from_the_shared_renderer() -> None: ts=NOW_TS, skipped=True, skip_reason="market_closed", mode="paper", polled=0 ) assert trading_service.render_loop_result(skipped) == [f"[{NOW_TS}] skipped: market_closed"] + + +def _entry(placed: bool, vetoed_by: list[str], reason: str = "") -> Any: + from keel.execution.executor import ExecutionResult + + return ExecutionResult( + placed=placed, order_id=None, vetoed_by=vetoed_by, preview=None, reason=reason + ) + + +def _cycle_line(enter_results: list[Any]) -> str: + from keel import agent + + result = agent.LoopResult( + ts=NOW_TS, + skipped=False, + skip_reason=None, + mode="paper", + polled=95, + products=["FET-USD"], + enter_results=enter_results, + ) + lines = trading_service.render_loop_result(result) + assert len(lines) == 1, lines + return lines[0] + + +_WEEKLY = "account_dd_breaker_weekly: drawdown 0.0863 >= max_weekly_dd_pct 0.08" + + +def test_cycle_line_counts_rail_vetoed_entries_and_names_the_rail() -> None: + """#812: the 2026-09-17 FET cycles printed `signals=3 blocked=0 entered=0`, indistinguishable + from a silent drop, while `account_dd_breaker_weekly` had refused all three. A vetoed entry + is now counted after `exited=`, with the rail that said no -- its leading clause only, the + arithmetic stays in the JSON log.""" + line = _cycle_line([_entry(False, [_WEEKLY], "paper: vetoed by rails")] * 3) + + assert line == ( + f"[{NOW_TS}] mode=paper polled=95 products=['FET-USD'] stale=[] " + "signals=0 blocked=0 entered=0 exited=0 vetoed=3 (account_dd_breaker_weekly)" + ) + + +def test_cycle_line_counts_one_veto_per_entry_and_lists_each_rail_once() -> None: + """One entry tripping two rails is ONE vetoed entry, not two (the same rule + `activity.summarise_cycle` applies to the PAXG cycle of 2026-08-08). Every distinct rail is + named once, in the order the entries hit them.""" + line = _cycle_line( + [ + _entry(False, ["per_asset_concentration_cap: PAXG 0.41 > 0.40", _WEEKLY]), + _entry(False, ["total_exposure_cap: 0.9 > 0.8", _WEEKLY]), + ] + ) + + assert line.endswith( + " exited=0 vetoed=2 " + "(per_asset_concentration_cap, account_dd_breaker_weekly, total_exposure_cap)" + ), line + + +def test_cycle_line_reports_vetoed_zero_without_a_rail_list_when_nothing_was_vetoed() -> None: + """A placed entry and an unplaced one with an EMPTY `vetoed_by` (a paper no-fill, a declined + confirm) are not rail vetoes. The token is still printed, so its absence never has to be + read as zero; the rail list is omitted.""" + line = _cycle_line( + [ + _entry(True, []), + _entry(False, [], "paper: no fill (position open or insufficient synthetic cash)"), + ] + ) + + assert line.endswith(" signals=0 blocked=0 entered=1 exited=0 vetoed=0"), line + + +def test_cycle_line_names_a_routing_gate_token_that_has_no_colon() -> None: + """The live entry-spread gate reports bare tokens (`max_entry_spread`), not + `rail: detail` strings; the token itself is the name.""" + line = _cycle_line([_entry(False, ["max_entry_spread"])]) + + assert line.endswith(" vetoed=1 (max_entry_spread)"), line From d4d2dd1ee79aa1f788d87ae12687831081a69254 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Thu, 24 Sep 2026 18:42:47 -0400 Subject: [PATCH 2/2] refactor(rails): one rail_name parse; say how the cycle line and overlay differ (#812) - guards.rail_name replaces the three copies of the violation parse (activity._first_clause, doctor's inline split, trading._vetoed_token). - _vetoed_token and summarise_cycle now state that the overlay's blocked = the line's blocked + vetoed + unplaced entries no rail named, and that vetoed counts entries only. - The cycle-line tests pair each entry result with its signal, as run_once does, so the FET test pins the real line: signals=3 ... vetoed=3. Co-Authored-By: Claude Opus 5.5 --- keel/commands/activity.py | 21 +++++++++---------- keel/commands/doctor.py | 4 ++-- keel/commands/trading.py | 15 ++++++++++---- keel/execution/guards.py | 13 ++++++++++++ tests/commands/test_service_parity.py | 29 ++++++++++++++++++++++++--- tests/execution/test_guards.py | 19 ++++++++++++++++++ 6 files changed, 80 insertions(+), 21 deletions(-) diff --git a/keel/commands/activity.py b/keel/commands/activity.py index 7126bebd..1bd89048 100644 --- a/keel/commands/activity.py +++ b/keel/commands/activity.py @@ -101,6 +101,8 @@ from keel_core.paths import resolve_under_state_root +from keel.execution import guards + # -- bounds (see the module docstring) ----------------------------------------------------------- #: Bytes of the log's TAIL that are ever read in one build. The whole point of the tail read is @@ -613,14 +615,6 @@ def _short_num(value: Any, places: int = 4) -> str: return f"{head}.{tail}" if tail else head -def _first_clause(text: str) -> str: - """The leading identifier of a `violation` string -- `"per_asset_concentration_cap: PAXG - exposure ... exceeds ..."` -> `"per_asset_concentration_cap"`. Which rail said no is what - belongs in a one-line summary; the arithmetic behind it belongs in the expansion.""" - head = text.split(":", 1)[0].strip() - return head or text.strip() - - def _last_exc_line(text: str) -> str: """The final line of a traceback -- the exception type and message. The frames above it are the single largest thing in this log (they are why 330 lines occupy 815 KB) and the least @@ -641,8 +635,11 @@ def _add(seq: list[str], value: Any) -> None: def summarise_cycle(cycle_id: str | None, events: Sequence[ActivityEvent]) -> ActivityCycle: """Turn one cycle's events into the row the overlay shows. PURE. - The five counts are deliberately the ones `keel agent`'s own human run-log one-liner already - taught an operator to read (`signals=0 blocked=0 entered=0 exited=0`), plus `errors`: + The five counts borrow their NAMES from `keel agent`'s human run-log one-liner (`signals=0 + blocked=0 entered=0 exited=0`), plus `errors` -- but `blocked` here is wider than that line's. + The run-log counts only entries withheld before evaluation and prints rail vetoes as a + separate `vetoed=N` (#812), so for one cycle: this `blocked` = its `blocked` + `vetoed` + + unplaced entries no rail named. See `trading._vetoed_token`. * `signals` -- setups the rules produced, summed from `agent.signals_evaluated.signal_count` (which is `len(enter_signals)`, exactly what the run-log line counts). Falls back to @@ -716,7 +713,7 @@ def summarise_cycle(cycle_id: str | None, events: Sequence[ActivityEvent]) -> Ac elif name == "guards.check_failed": violation = fields.get("violation") if isinstance(violation, str) and violation: - _add(highlights, f"rail veto: {_first_clause(violation)}") + _add(highlights, f"rail veto: {guards.rail_name(violation)}") else: _add(highlights, "rail veto") elif name == "engine.setup_rejected": @@ -1137,7 +1134,7 @@ def _generic_detail(ev: ActivityEvent) -> str: def render_event_detail(ev: ActivityEvent) -> str: """The human-readable right-hand side of one expanded event line -- the fields that MEAN something for the events this system actually emits: a `violation` in full (an operator needs - the arithmetic, which is exactly what the collapsed row's `_first_clause` drops), the `gate` + the arithmetic, which is exactly what the collapsed row's `guards.rail_name` drops), the `gate` that rejected a setup, the `reason` an entry was not placed, and a setup's entry/stop/target. Never raises on a missing or oddly-typed field -- every access is a `.get` with a visible `?` diff --git a/keel/commands/doctor.py b/keel/commands/doctor.py index fd56bbca..f1bbf6ec 100644 --- a/keel/commands/doctor.py +++ b/keel/commands/doctor.py @@ -49,7 +49,7 @@ from keel import attestations from keel.data.feed_scope import reports_consolidated_volume from keel.data.freshness import Freshness -from keel.execution import sizing +from keel.execution import guards, sizing from keel.types import Granularity from keel.version import build_info, check_install @@ -777,7 +777,7 @@ def veto_findings(lines: Iterable[str], since_ts: float) -> list[Finding]: continue total += 1 for violation in event.get("violations", []): - reason = str(violation).split(":", 1)[0].strip() + reason = guards.rail_name(str(violation)) counts[reason] = counts.get(reason, 0) + 1 if total == 0: diff --git a/keel/commands/trading.py b/keel/commands/trading.py index 912bb323..30c8ce72 100644 --- a/keel/commands/trading.py +++ b/keel/commands/trading.py @@ -19,6 +19,7 @@ from keel import agent from keel.data.repository import Repository from keel.execution import equity as equity_mod +from keel.execution import guards from keel.execution.executor import ExecutionResult # -- the typed gates' wording, and the output lines (their ONE home) ------------------------------ @@ -169,15 +170,21 @@ def _vetoed_token(enter_results: list[ExecutionResult]) -> str: `blocked` counts only entries withheld BEFORE evaluation, so without this a vetoed cycle printed `signals=3 blocked=0 entered=0`, which reads as a silent drop. N is one per entry, not one per rail: an entry tripping two rails is still one setup. Each rail is named once, - by the leading clause of its violation string (`"account_dd_breaker_weekly: drawdown ..."` - -> `account_dd_breaker_weekly`); the arithmetic stays in the JSON log. Appended after - `exited=` so the `signals=[0-9]+` token the live runner greps is untouched. + by `guards.rail_name`; the arithmetic stays in the JSON log. Appended after `exited=` so + the `signals=[0-9]+` token the live runner greps is untouched. + + ENTRIES only. A rail-vetoed exit (`base_balance` refusing a sell) is not counted here. + + **This line's `blocked` is not the activity overlay's.** `activity.summarise_cycle` counts + every unplaced entry as blocked, so for one cycle: overlay `blocked` = this `blocked` + + `vetoed` + unplaced entries no rail named (a paper no-fill, a declined confirm -- still + absent from this line). """ vetoed = [r for r in enter_results if not r.placed and r.vetoed_by] rails: list[str] = [] for r in vetoed: for violation in r.vetoed_by: - rail = violation.split(":", 1)[0].strip() or violation.strip() + rail = guards.rail_name(violation) if rail not in rails: rails.append(rail) return f"vetoed={len(vetoed)} ({', '.join(rails)})" if rails else f"vetoed={len(vetoed)}" diff --git a/keel/execution/guards.py b/keel/execution/guards.py index 561509c9..59ec3f88 100644 --- a/keel/execution/guards.py +++ b/keel/execution/guards.py @@ -274,6 +274,19 @@ class GuardResult: skipped_rails: list[str] = field(default_factory=list) +def rail_name(violation: str) -> str: + """The rail a `violations` entry names -- its leading clause: `"per_asset_concentration_cap: + PAXG exposure ... exceeds ..."` -> `"per_asset_concentration_cap"`. + + Every rail below writes `: `, and the executor's routing gate writes a bare + token (`max_entry_spread`), which is its own name. Which rail said no is what belongs in a + one-line summary (the cycle line, the activity overlay, `doctor`); the arithmetic behind it + belongs in the JSON log. The ONE parse of this format, so the summaries cannot drift (#812). + """ + head = violation.split(":", 1)[0].strip() + return head or violation.strip() + + def _asset(product_id: object) -> str: """The base leg of `product_id`: the bucket key rails 1/4/5/6/8 group and compare by. diff --git a/tests/commands/test_service_parity.py b/tests/commands/test_service_parity.py index 6efa593b..8d36627b 100644 --- a/tests/commands/test_service_parity.py +++ b/tests/commands/test_service_parity.py @@ -691,8 +691,30 @@ def _entry(placed: bool, vetoed_by: list[str], reason: str = "") -> Any: def _cycle_line(enter_results: list[Any]) -> str: + """One cycle's line, with a signal per entry result -- `run_once` appends the two together, + so a vetoed entry never comes without the signal that produced it.""" from keel import agent - + from keel.strategy.rules.base import Action, Setup, Signal + from keel.types import Side + + signal = Signal( + rule_name="turtle_breakout", + product_id="FET-USD", + action=Action.ENTER, + side=Side.BUY, + setup=Setup( + product_id="FET-USD", + direction="long", + entry=Decimal("0.16735"), + stop=Decimal("0.1625"), + target=Decimal("0.1963"), + context={}, + ts=NOW_TS, + ), + cts_score=7, + entry_technique="signal_candle", + ts=NOW_TS, + ) result = agent.LoopResult( ts=NOW_TS, skipped=False, @@ -700,6 +722,7 @@ def _cycle_line(enter_results: list[Any]) -> str: mode="paper", polled=95, products=["FET-USD"], + enter_signals=[signal] * len(enter_results), enter_results=enter_results, ) lines = trading_service.render_loop_result(result) @@ -719,7 +742,7 @@ def test_cycle_line_counts_rail_vetoed_entries_and_names_the_rail() -> None: assert line == ( f"[{NOW_TS}] mode=paper polled=95 products=['FET-USD'] stale=[] " - "signals=0 blocked=0 entered=0 exited=0 vetoed=3 (account_dd_breaker_weekly)" + "signals=3 blocked=0 entered=0 exited=0 vetoed=3 (account_dd_breaker_weekly)" ) @@ -751,7 +774,7 @@ def test_cycle_line_reports_vetoed_zero_without_a_rail_list_when_nothing_was_vet ] ) - assert line.endswith(" signals=0 blocked=0 entered=1 exited=0 vetoed=0"), line + assert line.endswith(" signals=2 blocked=0 entered=1 exited=0 vetoed=0"), line def test_cycle_line_names_a_routing_gate_token_that_has_no_colon() -> None: diff --git a/tests/execution/test_guards.py b/tests/execution/test_guards.py index 3a43507e..07fc9ad6 100644 --- a/tests/execution/test_guards.py +++ b/tests/execution/test_guards.py @@ -2269,3 +2269,22 @@ def test_an_entry_intent_still_uses_its_own_stop_for_rail9(repo): result = check(intent, repo, _config(), NOW_TS) assert _keys(result) == {"no_stop_widening"} + + +@pytest.mark.parametrize( + ("violation", "rail"), + [ + ( + "account_dd_breaker_weekly: drawdown 0.0863 >= max_weekly_dd_pct 0.08", + "account_dd_breaker_weekly", + ), + ("base_balance: held 0 -- expects 12: refusing", "base_balance"), + ("max_entry_spread", "max_entry_spread"), + (" book_unreadable ", "book_unreadable"), + ], +) +def test_rail_name_is_the_leading_clause_of_a_violation(violation: str, rail: str) -> None: + """#812: every one-line summary of a veto (the cycle line, the activity overlay, `doctor`) + names the rail, not the arithmetic. A detail with a second colon still splits once, and the + routing gate's bare tokens are their own name.""" + assert guards.rail_name(violation) == rail