From 4ac368aa5ff71317e9ae231545d9677d55aa7387 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Tue, 18 Aug 2026 01:53:33 -0400 Subject: [PATCH 1/2] feat(executor): routing-time max-spread gate for live BUY entries Fixes #350. WHAT. A live BUY whose previewed book shows `(best_ask - best_bid) / mid` at or beyond `execution.max_entry_spread_pct` (new config section, default 0.005 = 50bp) is REFUSED after the preview and before the confirm gate and placement -- so a thin book cannot be entered at a moment its spread alone makes the fill economics materially worse than the cost model assumes. The refusal is recorded in `ExecutionResult.vetoed_by` (the tokens `max_entry_spread` / `book_unreadable`, the same one-legible-token shape rail violations use) and logged at WARNING as a structured event carrying the measured spread, the threshold and the product. WHY. The issue is the rail-work agreement CONTRIBUTING requires; the operator proposed it in the Phase 10 expansion review. It is the live-path half of the spread guardrail whose sizing half (#358) caps every Tier-2 addition at a 2% target weight. Design decisions: - Post-preview placement, deliberately: `guards.check` is broker-less by design, and the book exists only in the `broker.preview_order` result -- so this is a routing-time gate BESIDE the eighteen rails, not a numbered `guards.check` rail. It consumes the SAME preview #332's `_warn_if_market_routing_overrides_entry` reads: one helper (`_preview_book`, bid/ask/mid-safe: missing keys, NaN, non-finite, non-positive all read as unreadable), two consumers. The warning keeps its exact behavior and position (its tests pass unchanged); the gate runs after it, before confirm/place. - BUY-only: exits, brackets, stop rolls and scale-outs are never gated -- the same principle that makes rail 17 halt entries, not exits. - Paper mode never runs the gate: `_paper_enter` fills synthetically without a preview, so the paper-hourly profile accrues NO evidence about it -- a reason this ships before any live resumption, not after paper expansion. - Fail-closed on an unreadable book: a live BUY whose preview carries no readable bid AND ask is refused with the distinct `book_unreadable` reason and logged loudly. "Cannot know" is a different fact from "too wide"; the real venue's preview carries both sides (`cb_client.preview_order` maps best_bid/best_ask to Decimal), so an unreadable book means a degraded response -- the moment not to spend. The spread arithmetic also refuses (rather than swallows, as #336 taught the warning to do) on extreme-exponent Overflow. - 50bp default anchored to #334's `SLIPPAGE_CAP_PCT`: the backtest never assumes more than 50bp per-leg slippage on even the thinnest book, so a spread AT the cap has consumed the model's entire worst-case cost and the taker fee rides outside it -- hence the boundary is >= (fail-closed), unlike #332's strictly-greater visibility threshold. Validated on load to (0, 0.10], ConfigError naming `execution.max_entry_spread_pct`. Test-side consequence, honestly reported: the shared test fakes' bookless default previews modelled a shape the real venue does not return, and under a fail-closed gate every such "normal successful BUY" test would refuse. The fakes (test_executor, test_agent, test_cli, test_reconcile) now carry both book sides, and the one #332 test that borrowed the default preview as its degraded/bookless shape constructs that shape explicitly -- meaning unchanged, sourcing changed. Golden config fixtures regenerated via the documented script; the defaults golden now pins the 0.005. --- README.md | 6 +- config.yaml | 12 + docs/fiqh-basis.md | 10 + docs/operator-runbook.md | 27 ++ keel/execution/executor.py | 254 ++++++++++++++++-- keel/templates/config.live.yaml | 12 + keel/templates/config.yaml | 12 + packages/keel-core/keel_core/config.py | 63 +++++ tests/execution/test_executor.py | 289 ++++++++++++++++++++- tests/execution/test_reconcile.py | 6 +- tests/fixtures/config_golden_defaults.json | 3 + tests/fixtures/config_golden_full.json | 3 + tests/fixtures/config_golden_full.yaml | 4 + tests/test_agent.py | 30 +++ tests/test_cli.py | 4 + tests/test_config.py | 34 +++ 16 files changed, 735 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 967fd052..76cdded9 100644 --- a/README.md +++ b/README.md @@ -86,7 +86,11 @@ applies the confirm/autonomy gate, then places and logs. There is deliberately n quote-balance checks, venue **subscription/withdrawal attestations** — rail 17 encodes §65.4 *qabd*: an asset that cannot be withdrawn may not have been validly possessed, so withdrawal capability is attested and enforced, not assumed. A rail veto names itself and - the command that clears it. + the command that clears it. Beside the rails sits one routing-time check that needs the + venue's own book, which a broker-less rail cannot see: the **max-spread entry gate** (#350) + refuses a live BUY whose previewed `(best_ask − best_bid) / mid` is at or beyond + `execution.max_entry_spread_pct` (default 50bp) — BUY-only, live-only, and fail-closed on + an unreadable book. - **Screening** (`keel/compliance/screen.py`) — allowlist admission is split by what is knowable: market facts are computed; Shariah classifications are **attested, never inferred**, and an absent attestation is a rejection, not a default pass. diff --git a/config.yaml b/config.yaml index 9f1d1843..3525adef 100644 --- a/config.yaml +++ b/config.yaml @@ -151,3 +151,15 @@ logging: research: pbo_max: 0.05 slope_floor: -0.5 + +# Live-execution routing-time guardrails (#350). max_entry_spread_pct is the threshold of the +# entry spread gate: a live BUY whose previewed book shows (best_ask - best_bid) / mid at or +# beyond it is REFUSED before placement, so a thin book cannot be entered when its spread +# alone makes the fill materially more expensive than the cost model assumes. BUY entries +# only (SELLs/exits are never gated) and the live path only -- paper fills are synthetic and +# see no book, so the paper profiles accrue no evidence about this gate. 0.005 = 50bp, +# anchored to the backtest's worst-case per-leg slippage assumption (#334's slippage cap): +# a spread AT that line has already consumed the model's entire cost estimate. Valid range +# (0, 0.10] -- a fraction of price, not basis points. +execution: + max_entry_spread_pct: 0.005 diff --git a/docs/fiqh-basis.md b/docs/fiqh-basis.md index 8c01864d..bf4fbab8 100644 --- a/docs/fiqh-basis.md +++ b/docs/fiqh-basis.md @@ -154,6 +154,16 @@ religious claim: | 12 | stale-feed + kill-switch, fails closed | operational safety | | 13, 14 | spend only the settled quote currency; monthly allowance cap | operational safety | +Beside the rails — not among them, and not numbered — sits one routing-time check with the +same prudential character: the **max-spread entry gate** (#350, `keel/execution/executor.py`) +refuses a live BUY whose previewed book shows `(best_ask − best_bid) / mid` at or beyond +`execution.max_entry_spread_pct` (default 50bp). It is not a `guards.check` rail because the +rails are broker-less by design and the book exists only in the venue's preview response. +BUY-only (exits must execute), live-only (paper fills are synthetic and see no book), and +fails closed on an unreadable book — the same fail-closed family as rails 12/13/17, justified +by trading evidence (the backtest's worst-case per-leg cost assumption), carrying no +religious claim. + Rail 7 carries a correction this repository records prominently: §65.6 holds that "speculation per se, which means sale/purchase keeping in mind possible change in prices in the future, is not prohibited" — what makes speculation *maisir* is non-ownership, diff --git a/docs/operator-runbook.md b/docs/operator-runbook.md index 6d1078dc..15ff056a 100644 --- a/docs/operator-runbook.md +++ b/docs/operator-runbook.md @@ -295,6 +295,33 @@ spread gate. The 8 incumbents keep their relative shape rescaled to 78% total (r untouched, so their evidence stays comparable across the expansion). Paperforward — the daily profile — deliberately stays at 8 so its evidence remains a like-for-like 8-asset series. +### The spread guardrail: a sizing half and a live-path half + +Thin books cost more to trade than the cost model assumes, and the corpus's thin tail is +exactly where the expansion above added exposure. The guardrail has two halves, each doing the +half it can: + +- **Sizing (#358):** every Tier-2 addition sits at a flat 2% target weight, so a thin book can + only ever be a 2% position. +- **Live path (#350):** a **routing-time maximum-spread gate** refuses a live BUY when the + venue's own previewed book shows `(best_ask − best_bid) / mid` at or beyond + `execution.max_entry_spread_pct` — default **0.005 (50bp)**, anchored to the backtest's + worst-case per-leg slippage assumption (#334's `SLIPPAGE_CAP_PCT`): if the spread ALONE + consumes the model's entire cost estimate, the fill economics are materially worse than + anything the rule was measured on, and the entry waits for the book to tighten. + +The gate is BUY-only (exits must execute — the same principle that makes rail 17 halt entries, +not exits), **fails closed** (a live BUY whose preview carries no readable bid/ask is refused +with a distinct `book_unreadable` reason, never guessed past), and lives beside the eighteen +rails rather than among them: `guards.check` is broker-less by design, and the book exists only +in the preview the executor just fetched. + +**Paper accrues no evidence about it.** Paper fills are synthetic and see no book, so neither +paper profile ever exercises the gate — a reason it ships before any live resumption (the gate +must already be in force when live BUYs resume) rather than being validated on paper first. +A refusal is visible in the cycle log as `executor.entry_spread_refused` (with the measured +spread and the threshold) or `executor.entry_book_unreadable`. + **The rows differ from every other turtle row by one param.** `params.granularity: "ONE_HOUR"` — `TurtleBreakout`'s declared trading timeframe, persisted the way `RsiMeanReversion.timeframe` is and coerced back by `keel/agent.py`'s registry. A row with no `granularity` key (every row diff --git a/keel/execution/executor.py b/keel/execution/executor.py index 6ff51b4c..a65802c8 100644 --- a/keel/execution/executor.py +++ b/keel/execution/executor.py @@ -61,6 +61,16 @@ defers resting-order routing until a price-conditional rule earns it. What is NOT deferred is visibility: `_warn_if_market_routing_overrides_entry` logs at WARNING whenever a routed entry sits materially off the venue's own book, so the override is visible rather than silent. + +**The routing-time max-spread entry gate (#350).** A live BUY whose previewed book is too +wide to enter is REFUSED after the preview and before the confirm gate/placement: +`(best_ask - best_bid) / mid` at or beyond `execution.max_entry_spread_pct` (default 0.005, +50bp -- #334's slippage cap as the anchor) refuses the order, and a preview with no readable +bid/ask fails closed with a distinct reason. It sits BESIDE the eighteen rails, not among +them: `guards.check` is broker-less by design, and the book exists only in the preview this +module just fetched -- the same preview #332's warning reads (`_preview_book`: one helper, +two consumers). BUY-only (exits must execute, like rail 17 halting entries not exits) and +live-only (paper fills are synthetic, see no book, and accrue no evidence about it). """ from __future__ import annotations @@ -91,9 +101,13 @@ class ExecutionResult: """The outcome of `execute()` (or one of the management actions below). - `vetoed_by` is non-empty only when `guards.check` rejected the intent (the specific rail - names, verbatim from `GuardResult.violations`) -- a confirm-gate rejection or a broker-side - failure leaves it `[]` and explains itself via `reason` instead. + `vetoed_by` is non-empty when `guards.check` rejected the intent (the specific rail + names, verbatim from `GuardResult.violations`) OR when the routing-time entry spread gate + refused a live BUY (#350 -- the tokens `max_entry_spread` / `book_unreadable`, see + `_entry_spread_gate`; not a `guards.check` rail, but reported the same way so a caller + reading `vetoed_by` sees one legible shape for "this order was refused before + placement"). A confirm-gate rejection or a broker-side failure leaves it `[]` and + explains itself via `reason` instead. """ placed: bool @@ -493,6 +507,21 @@ def _run_order( # at WARNING, before the confirm gate and before placement. _warn_if_market_routing_overrides_entry(intent, preview, order_configuration) + # #350: THE ROUTING-TIME MAX-SPREAD ENTRY GATE. A live BUY whose book -- read from the + # same preview the warning above just consumed -- is too wide (or unreadable) is refused + # HERE, before the confirm gate and before placement. See `_entry_spread_gate` for the + # full rationale; the ordering (warning first, gate second) is deliberate and pinned by + # #332's tests. + spread_refusal = _entry_spread_gate(intent, preview, config.execution.max_entry_spread_pct) + if spread_refusal is not None: + return ExecutionResult( + placed=False, + order_id=None, + vetoed_by=[spread_refusal.veto], + preview=preview, + reason=spread_refusal.reason, + ) + if mode == "confirm": approved = confirm_fn(preview) if confirm_fn is not None else False if not approved: @@ -707,32 +736,55 @@ def _log_intent_divergence(order_id: int, intent: OrderIntent | None, realized: ENTRY_OVERRIDE_WARN_BP = Decimal("50") -def _preview_best_ask(preview: Preview | dict[str, Any]) -> Decimal | None: - """The venue's best ask out of a preview response, in whichever of its two shapes. - - Both shapes already cross this module (`ConfirmFn`'s docstring explains why they coexist): - the pre-port `CoinbaseClient.preview_order` dict, which maps `best_ask` to a `Decimal`, and - the port's `Preview`, whose Coinbase adapter carries the same book as a string inside - `detail`. `None` when the venue returned no usable ask -- a degraded response, not an - error, and nothing downstream may compute a deviation against a guess. +def _preview_book(preview: Preview | dict[str, Any]) -> tuple[Decimal | None, Decimal | None]: + """The venue's book out of a preview response, as `(best_bid, best_ask)`, each field read + INDEPENDENTLY and safely, in whichever of the preview's two shapes. + + One helper, two consumers (#350): #332's entry-override warning needs only the ask, and + the routing-time max-spread gate needs both sides plus their midpoint. Both shapes + already cross this module (`ConfirmFn`'s docstring explains why they coexist): the + pre-port `CoinbaseClient.preview_order` dict, which maps `best_bid`/`best_ask` to + `Decimal`s, and the port's `Preview`, whose Coinbase adapter carries the same book as + strings inside `detail`. + + Each side is `None` when THE VENUE returned no usable value for that field -- absent key, + non-numeric string, or a non-finite/non-positive number -- a degraded response, not an + error. Per-field independence is load-bearing: the warning's contract (#332) is ask-only, + so a book with a readable ask but no bid must still hand the warning its reference while + telling the spread gate (which refuses on a half-readable book) that it cannot compute. + Nothing downstream may compute against a guessed side. + + `is_finite()` is checked FIRST, deliberately outside any try: `Decimal('NaN') > 0` + RAISES InvalidOperation, and a venue string of "nan" parses into exactly that + (`cb_client` does `Decimal(value)` on venue strings with no finiteness check, so the + input is reachable). A non-finite side is a degraded preview, not a routing failure. """ - raw: Any = None + raw: dict[str, Any] = {} if isinstance(preview, Mapping): - raw = preview.get("best_ask") + raw = {"best_bid": preview.get("best_bid"), "best_ask": preview.get("best_ask")} else: detail = getattr(preview, "detail", None) - raw = detail.get("best_ask") if detail is not None else None - if raw is None: - return None - try: - ask = Decimal(str(raw)) - except (InvalidOperation, TypeError, ValueError): - return None - # `is_finite()` first, deliberately outside any try: Decimal('NaN') > 0 RAISES - # InvalidOperation, and a venue string of "NaN" parses into exactly that -- the - # sibling `_log_intent_divergence` keeps the same hazard inside its own try for the - # same reason. A non-finite ask is a degraded preview, not a routing failure. - return ask if ask.is_finite() and ask > 0 else None + raw = detail if detail is not None else {} + + def _side(key: str) -> Decimal | None: + value = raw.get(key) + if value is None: + return None + try: + parsed = Decimal(str(value)) + except (InvalidOperation, TypeError, ValueError): + return None + return parsed if parsed.is_finite() and parsed > 0 else None + + return _side("best_bid"), _side("best_ask") + + +def _preview_best_ask(preview: Preview | dict[str, Any]) -> Decimal | None: + """The venue's best ask out of a preview response, in whichever of its two shapes. + + A thin consumer of `_preview_book` (above): same shapes, same per-field safety, ask only. + """ + return _preview_book(preview)[1] def _warn_if_market_routing_overrides_entry( @@ -814,6 +866,158 @@ def _warn_if_market_routing_overrides_entry( ) +# -- #350: the routing-time max-spread entry gate ------------------------------------------------- + + +#: The `ExecutionResult.vetoed_by` token recorded when a live BUY is refused because the +#: previewed book's spread is at/beyond `execution.max_entry_spread_pct`. Deliberately the +#: same "one legible token" shape `GuardResult.violations` uses for rail vetoes, so a caller +#: (or operator) reading `vetoed_by` cannot confuse a gate refusal with a rail violation. +SPREAD_GATE_VETO = "max_entry_spread" + +#: The same, for the fail-closed case: the preview carried no readable bid/ask, so the spread +#: is not "too wide" but UNKNOWN -- a different fact, reported differently on purpose. +SPREAD_GATE_BOOK_UNREADABLE_VETO = "book_unreadable" + + +@dataclass(frozen=True) +class _SpreadGateRefusal: + """Why `_entry_spread_gate` refused a BUY: the `vetoed_by` token plus the human sentence + for `ExecutionResult.reason` -- one return value so the two can never disagree.""" + + veto: str + reason: str + + +def _entry_spread_gate( + intent: OrderIntent, + preview: Preview | dict[str, Any] | None, + max_entry_spread_pct: Decimal, +) -> _SpreadGateRefusal | None: + """Refuse a live BUY whose previewed book is too wide to enter (#350). `None` = proceed. + + **What it decides.** For a BUY on the live path, `(best_ask - best_bid) / mid` at or + beyond `execution.max_entry_spread_pct` (default 0.005 = 50bp) refuses the order BEFORE + the confirm gate and placement. The anchor is #334's backtest slippage cap + (`strategy.backtest.SLIPPAGE_CAP_PCT`): the backtest never assumes more than 50bp of + per-leg slippage even on the thinnest book, so a spread AT the cap has already consumed + the model's entire worst-case cost estimate and the taker fee rides outside the model -- + the fill economics are materially worse than anything the rule was measured on. The + comparison is `>=`, the fail-closed side of the line, UNLIKE #332's visibility-only + strictly-greater: at the threshold the spread alone equals the model's worst case, which + is already too wide to enter on this reasoning. + + **Where it sits, and why.** AFTER `guards.check` and AFTER the preview: guards are + broker-less by design (this module's docstring), so the book -- which only + `broker.preview_order` returns -- cannot reach a `guards.check` rail. The gate is a + routing-time check BESIDE the eighteen rails, not a numbered rail, and it consumes the + SAME preview #332's `_warn_if_market_routing_overrides_entry` reads (one helper, + `_preview_book`, two consumers). It runs after that warning so the warning's position -- + pinned by #332's tests -- is unchanged; on a wide book both facts are true at routing + time (the entry was market-routed, AND the book is too wide), and the refusal event below + is the terminal record. Paper mode never runs it at all: `_paper_enter` fills + synthetically without a preview, so the paper-hourly profile accrues NO evidence about + this gate -- which is why it ships before any live resumption rather than being validated + on paper first. + + **SELLs are never gated** (`intent.side != Side.BUY` returns immediately): exits, exit + brackets, stop rolls and scale-outs must execute -- the same principle that makes rail 17 + halt entries, not exits. A spread gate that trapped an exit would strand a position in + exactly the book conditions the rule said to leave. + + **Fail-closed on an unreadable book.** A preview with no readable bid AND ask (missing + keys, NaN, non-numeric, non-finite, non-positive, or a spread whose arithmetic + overflows -- the extreme-exponent hazard #336 taught the warning about, refused rather + than swallowed here because this is a money gate) is refused with the DISTINCT + `book_unreadable` token: "cannot know" is a different fact from "too wide", and a gate + that guessed a spread from half a book would be a gate that sometimes trades on fiction. + The real venue's preview carries both sides for market orders (`cb_client.preview_order` + maps `best_bid`/`best_ask` to `Decimal`), so an unreadable book on the live path means a + degraded response -- exactly the moment not to spend. + + Every BUY routes market today (#258), so "every live BUY" and "every market-routed live + BUY" are the same set; if #260's remediation ever lands resting BUY orders, revisit the + scope -- a resting limit does not cross the spread it sits inside. + """ + if intent.side != Side.BUY: + return None + if preview is None: + # Unreachable from `_run_order` (it just previewed), but the function stays honest + # standalone: no preview, no book, fail closed. + return _book_unreadable_refusal(intent, "the preview response was empty") + + bid, ask = _preview_book(preview) + if bid is None or ask is None: + return _book_unreadable_refusal( + intent, + f"no readable {'best_bid' if bid is None else 'best_ask'} in the preview response", + ) + # The arithmetic stays INSIDE a try, matching `_warn_if_market_routing_overrides_entry`: + # `is_finite()` admits extreme exponents (1E+999999999 parses and compares fine), and + # Decimal add/div on such magnitudes raises Overflow -- an ArithmeticError. Telemetry + # swallows that (#336); a money gate refuses on it: an uncomputable spread is an + # unreadable book, not a pass. + try: + mid = (bid + ask) / Decimal(2) + spread_pct = (ask - bid) / mid + except ArithmeticError: + return _book_unreadable_refusal( + intent, "spread uncomputable (extreme magnitudes in the book)" + ) + if spread_pct < max_entry_spread_pct: + return None + log_event( + logger, + logging.WARNING, + "executor.entry_spread_refused", + rule=intent.rule_kind, + product=intent.product_id, + side=intent.side.value, + best_bid=str(bid), + best_ask=str(ask), + mid=str(mid), + spread_pct=str(spread_pct), + threshold_pct=str(max_entry_spread_pct), + veto=SPREAD_GATE_VETO, + detail=( + "refused at routing: the live book's spread alone is at/beyond " + "execution.max_entry_spread_pct, so the fill would cost more than the worst " + "per-leg cost the backtest ever models (#334's slippage cap) -- entries into " + "this book wait for it to tighten (#350)" + ), + ) + return _SpreadGateRefusal( + veto=SPREAD_GATE_VETO, + reason=( + f"refused by the routing-time entry spread gate: spread {spread_pct} of mid " + f"{mid} is at/beyond execution.max_entry_spread_pct {max_entry_spread_pct}" + ), + ) + + +def _book_unreadable_refusal(intent: OrderIntent, why: str) -> _SpreadGateRefusal: + """The fail-closed arm of `_entry_spread_gate`, logged loudly: an unreadable book is a + DEGRADED venue response, and an operator seeing repeated refusals here needs to know it + is the preview shape that changed, not the market.""" + log_event( + logger, + logging.WARNING, + "executor.entry_book_unreadable", + rule=intent.rule_kind, + product=intent.product_id, + side=intent.side.value if isinstance(intent.side, Side) else str(intent.side), + veto=SPREAD_GATE_BOOK_UNREADABLE_VETO, + detail=( + f"refused at routing: {why} -- the spread is UNKNOWN, not merely wide, and a " + f"live BUY must not be sized against a book it cannot read (#350)" + ), + ) + return _SpreadGateRefusal( + veto=SPREAD_GATE_BOOK_UNREADABLE_VETO, + reason=f"refused by the routing-time entry spread gate: {why}", + ) + + def _order_row(intent: OrderIntent, mode: str, now_ts: int) -> dict[str, Any]: # Routed MARKET unconditionally (#258's faithful-engine decision): `expected_fill` below # records the rule's intended entry even though execution ignores it -- the override diff --git a/keel/templates/config.live.yaml b/keel/templates/config.live.yaml index a19b2ff4..4cf1072b 100644 --- a/keel/templates/config.live.yaml +++ b/keel/templates/config.live.yaml @@ -162,3 +162,15 @@ logging: research: pbo_max: 0.05 slope_floor: -0.5 + +# Live-execution routing-time guardrails (#350). max_entry_spread_pct is the threshold of the +# entry spread gate: a live BUY whose previewed book shows (best_ask - best_bid) / mid at or +# beyond it is REFUSED before placement, so a thin book cannot be entered when its spread +# alone makes the fill materially more expensive than the cost model assumes. BUY entries +# only (SELLs/exits are never gated) and the live path only -- paper fills are synthetic and +# see no book, so the paper profiles accrue no evidence about this gate. 0.005 = 50bp, +# anchored to the backtest's worst-case per-leg slippage assumption (#334's slippage cap): +# a spread AT that line has already consumed the model's entire cost estimate. Valid range +# (0, 0.10] -- a fraction of price, not basis points. +execution: + max_entry_spread_pct: 0.005 diff --git a/keel/templates/config.yaml b/keel/templates/config.yaml index 9f1d1843..3525adef 100644 --- a/keel/templates/config.yaml +++ b/keel/templates/config.yaml @@ -151,3 +151,15 @@ logging: research: pbo_max: 0.05 slope_floor: -0.5 + +# Live-execution routing-time guardrails (#350). max_entry_spread_pct is the threshold of the +# entry spread gate: a live BUY whose previewed book shows (best_ask - best_bid) / mid at or +# beyond it is REFUSED before placement, so a thin book cannot be entered when its spread +# alone makes the fill materially more expensive than the cost model assumes. BUY entries +# only (SELLs/exits are never gated) and the live path only -- paper fills are synthetic and +# see no book, so the paper profiles accrue no evidence about this gate. 0.005 = 50bp, +# anchored to the backtest's worst-case per-leg slippage assumption (#334's slippage cap): +# a spread AT that line has already consumed the model's entire cost estimate. Valid range +# (0, 0.10] -- a fraction of price, not basis points. +execution: + max_entry_spread_pct: 0.005 diff --git a/packages/keel-core/keel_core/config.py b/packages/keel-core/keel_core/config.py index 3c18cfa3..a5a832db 100644 --- a/packages/keel-core/keel_core/config.py +++ b/packages/keel-core/keel_core/config.py @@ -281,6 +281,29 @@ class ResearchConfig: slope_floor: Decimal = Decimal("-0.5") +@dataclass(frozen=True) +class ExecutionConfig: + """Live-execution routing-time guardrails (Issue #350). + + `max_entry_spread_pct` is the threshold of the routing-time max-spread entry gate: a live + BUY whose previewed book shows `(best_ask - best_bid) / mid` at or above it is REFUSED + before the confirm gate and placement, so a thin book cannot be entered at a moment its + spread alone makes the fill economics materially worse than the cost model assumes. + SELLs are never gated (exits must execute), and paper mode never runs the gate (paper + fills are synthetic and see no book -- which is why the paper-hourly profile accrues no + evidence about it, and the gate ships before any live resumption rather than after). + + The default 0.005 (50bp) is anchored to #334's `strategy/backtest.SLIPPAGE_CAP_PCT`: the + backtest never assumes more than 50bp of per-leg slippage on even the thinnest book, so a + spread AT the cap has already consumed the model's entire worst-case cost, leaving the + taker fee wholly outside it. Validated to (0, 0.10] at load: 0 would silently disarm the + gate, and anything past 10% is not a threshold a thin-book guardrail could meaningfully + have crossed by accident. + """ + + max_entry_spread_pct: Decimal = Decimal("0.005") + + @dataclass(frozen=True) class Config: allowlist: list[str] @@ -319,6 +342,7 @@ class Config: settlement_currencies: frozenset[str] = DEFAULT_SETTLEMENT_CURRENCIES logging: LoggingConfig = field(default_factory=LoggingConfig) research: ResearchConfig = field(default_factory=ResearchConfig) + execution: ExecutionConfig = field(default_factory=ExecutionConfig) # Only the real, binding caps are required; `max_per_order_usd`/`max_per_day_usd` are optional @@ -620,6 +644,43 @@ def _parse_research(raw: dict[str, Any]) -> ResearchConfig: return ResearchConfig(pbo_max=pbo_max, slope_floor=slope_floor) +#: The most permissive `execution.max_entry_spread_pct` a config may state. A guardrail +#: threshold above 10% cannot meaningfully be called a thin-book protection, so a value in +#: that territory is a typo (a misplaced decimal, a percentage typed where a fraction was +#: meant) and is refused at load rather than silently gutting the gate. +_MAX_ENTRY_SPREAD_PCT_CEILING = Decimal("0.10") + + +def _parse_execution(raw: dict[str, Any]) -> ExecutionConfig: + """Parse `execution:` -- optional, falls back to `ExecutionConfig`'s defaults. + + `max_entry_spread_pct` is checked for FINITENESS before the range comparison, mirroring + `_non_negative_decimal`: `Decimal('NaN')` parses cleanly from a YAML `nan`, and the + `<= 0` / `> ceiling` comparisons on it would RAISE InvalidOperation deep inside the + executor's gate instead of failing here where the operator is editing the file. + """ + execution_raw = raw.get("execution") or {} + defaults = ExecutionConfig() + + pct = _to_decimal( + execution_raw.get("max_entry_spread_pct", defaults.max_entry_spread_pct), + "execution.max_entry_spread_pct", + ) + if not pct.is_finite(): + raise ConfigError( + f"execution.max_entry_spread_pct: must be a finite number in (0, 0.10], " + f"got {pct!r}" + ) + if pct <= 0 or pct > _MAX_ENTRY_SPREAD_PCT_CEILING: + raise ConfigError( + f"execution.max_entry_spread_pct: must be a number in (0, 0.10] -- a fraction of " + f"price, not basis points (0.005 = 50bp); got {pct!r}. 0 would silently disarm " + f"the routing-time entry-spread gate (#350), and a value above 0.10 is not a " + f"thin-book threshold anyone means to set." + ) + return ExecutionConfig(max_entry_spread_pct=pct) + + def load_config(path: str | Path) -> Config: """Parse and validate `config.yaml` at `path`, returning a typed `Config`. @@ -787,6 +848,7 @@ def load_config(path: str | Path) -> Config: settlement_currencies=settlement_currencies, logging=_parse_logging(raw), research=_parse_research(raw), + execution=_parse_execution(raw), ) @@ -823,6 +885,7 @@ def load_secrets(env_path: str | Path = ".env") -> dict: "TierConfig", "LoggingConfig", "ResearchConfig", + "ExecutionConfig", "FeesConfig", "Config", "load_config", diff --git a/tests/execution/test_executor.py b/tests/execution/test_executor.py index 5a1e44e2..27a0517a 100644 --- a/tests/execution/test_executor.py +++ b/tests/execution/test_executor.py @@ -69,11 +69,19 @@ def __init__( # both USD and USDC with `usdc_balance`, so tests that only mean "the account is funded" # keep meaning that -- the mismatch tests below set the two independently on purpose. self._balances = balances + # The default preview carries BOTH sides of a TIGHT book, because that is what the + # real venue returns (`cb_client.preview_order` maps `best_bid`/`best_ask` to + # `Decimal`; see `tests/fixtures/cb_preview_order.json`) and #350's spread gate fails + # closed on a preview without them. A test that means "a degraded/bookless response" + # passes its own preview dict -- see the #332 warning tests and the gate's + # `book_unreadable` tests. self._preview = preview or { "order_total": Decimal("50.00"), "commission_total": Decimal("0.30"), "errs": [], "warning": [], + "best_bid": Decimal("49990"), + "best_ask": Decimal("50000"), } self._place_success = place_success self._place_order_id_seq = 0 @@ -972,6 +980,10 @@ def test_a_filled_order_records_the_previewed_commission_as_its_fee(repo): "commission_total": Decimal("0.30"), "errs": [], "warning": [], + # Both book sides, as the real venue returns them: #350's spread gate fails + # closed on a preview without them, and this test is about the FEE, not the book. + "best_bid": Decimal("49990"), + "best_ask": Decimal("50000"), } ) signal = _enter_signal() @@ -1719,11 +1731,13 @@ def _override_fields(caplog) -> dict: def _quoted_preview(best_ask: str) -> dict[str, Any]: - """A `CoinbaseClient.preview_order`-shaped dict carrying the venue's own book. + """A `CoinbaseClient.preview_order`-shaped dict carrying the venue's own ASK side only. - The real client maps `best_bid`/`best_ask` to `Decimal` when the venue returns them; the - default `FakeBroker` preview omits them, which is exactly the degraded shape the warning - code has to survive (a preview with no book is not an error, it is just not a reference). + The real client maps `best_bid`/`best_ask` to `Decimal` when the venue returns them. This + helper carries only the ask -- everything the #332 warning reads -- so it doubles as the + half-readable shape #350's spread gate must treat as `book_unreadable` while the warning + still reads its reference (a preview with no book is not an error, it is just not a + spread). """ return { "order_total": Decimal("50.00"), @@ -1843,12 +1857,20 @@ def test_entry_below_market_warns_with_a_negative_sign(self, caplog) -> None: def test_a_preview_without_a_book_quote_is_silent_not_fatal(self, caplog) -> None: """No `best_ask`, no honest reference -- and a warning built on a guess would be noise. - The default `FakeBroker` preview shape (no bid/ask keys) models a degraded venue - response; the cycle must proceed exactly as before this warning existed. + A preview with no bid/ask keys models a degraded venue response; the warning must be + silent and the cycle must proceed exactly as before this warning existed. (Constructed + explicitly rather than borrowed from `FakeBroker`'s default, which -- since #350's + spread gate made a bookless live BUY a REFUSAL -- models the real venue and carries a + book.) """ from keel.execution.executor import _warn_if_market_routing_overrides_entry - bookless = FakeBroker()._preview # the default shape: no best_bid/best_ask keys + bookless = { + "order_total": Decimal("50.00"), + "commission_total": Decimal("0.30"), + "errs": [], + "warning": [], + } with caplog.at_level(logging.WARNING): _warn_if_market_routing_overrides_entry(self._intent(entry="50300"), bookless) _warn_if_market_routing_overrides_entry( @@ -1937,3 +1959,256 @@ def test_an_explicitly_non_market_configuration_never_warns(self, caplog) -> Non ) assert not [r for r in caplog.records if r.getMessage() == _OVERRIDE_EVENT] + + +# -- #350: the routing-time maximum-spread entry gate ------------------------------------------- + + +#: The two stable event ids the spread gate emits -- names, never sentences, per +#: `keel_core.telemetry`'s contract, so tests (and any aggregation) key on them. +_SPREAD_REFUSED_EVENT = "executor.entry_spread_refused" +_BOOK_UNREADABLE_EVENT = "executor.entry_book_unreadable" + +#: The `vetoed_by` reason strings the gate records on `ExecutionResult` -- deliberately the +#: same "one legible token" shape `GuardResult.violations` uses for rail vetoes. +SPREAD_GATE_VETO = "max_entry_spread" +BOOK_UNREADABLE_VETO = "book_unreadable" + + +def _gate_fields(caplog, event: str) -> dict: + """The structured payload of the last `event` record -- same rationale as + `_override_fields`: `log_event` attaches fields via `extra`, so `caplog.text` shows only + the event name and asserting on it would pass for any values.""" + from keel_core.telemetry import _FIELDS_ATTR + + records = [r for r in caplog.records if r.getMessage() == event] + assert records, f"no {event} record was emitted" + return getattr(records[-1], _FIELDS_ATTR) + + +def _book_preview(best_bid: Decimal | str, best_ask: Decimal | str) -> dict[str, Any]: + """A `CoinbaseClient.preview_order`-shaped dict carrying BOTH sides of the venue's book. + + Like `_quoted_preview` above, but with `best_bid` too: the spread gate needs both sides + (the #332 warning reads only the ask). Values are passed through VERBATIM -- `Decimal` for + the good path (what the real client maps venue strings to), raw strings for the degraded + cases (`"nan"`, `"not-a-number"`), which the port's `Preview.detail` can carry un-parsed. + """ + return { + "order_total": Decimal("50.00"), + "commission_total": Decimal("0.30"), + "errs": [], + "warning": [], + "best_bid": best_bid, + "best_ask": best_ask, + } + + +class TestMaxSpreadEntryGate: + """#350: a live BUY whose previewed book is too wide is REFUSED at routing time. + + The gate runs AFTER `guards.check` and AFTER the preview (guards are broker-less by + design; the book exists only in `broker.preview_order`'s result -- the same preview + #332's warning reads), and BEFORE the confirm gate and placement. SELLs are never gated + (exits must execute -- the same principle that makes rail 17 halt entries only), and + paper mode never runs it (paper fills are synthetic and see no book, which is exactly why + the paper-hourly profile accrues NO evidence about this gate). + + Default threshold under test: `execution.max_entry_spread_pct` = 0.005 (50bp), anchored to + #334's `SLIPPAGE_CAP_PCT` -- if the spread ALONE exceeds the worst per-leg cost the + backtest ever assumes, the fill economics are materially worse than modeled. + """ + + def test_a_wide_book_refuses_the_live_buy_before_any_placement(self, repo, caplog) -> None: + """50,000 bid / 50,300 ask is a 59.8bp spread -- beyond the 50bp default -- so the + entry is refused at routing: no `place_order`, no order row, the refusal recorded in + `vetoed_by` with the gate's own reason token, and a WARNING carrying the measured + spread, the threshold and the product.""" + broker = FakeBroker(preview=_book_preview(Decimal("50000"), Decimal("50300"))) + signal = _enter_signal(_setup(entry=Decimal("50150"))) # at the mid: the #332 + # warning below must stay silent so this test isolates the GATE. + + with caplog.at_level(logging.WARNING): + result = execute(signal, broker, repo, _config(), "autonomous", now_ts=NOW_TS) + + assert result.placed is False + assert result.vetoed_by == [SPREAD_GATE_VETO] + assert result.order_id is None + assert broker.place_calls == [] + assert repo.get_orders() == [] + fields = _gate_fields(caplog, _SPREAD_REFUSED_EVENT) + assert fields["product"] == "BTC-USD" + assert fields["best_bid"] == "50000" + assert fields["best_ask"] == "50300" + assert fields["spread_pct"] == "0.005982053838484546360917248255" + assert fields["threshold_pct"] == "0.005" + assert fields["veto"] == SPREAD_GATE_VETO + + def test_a_tight_book_places_and_the_332_warning_stays_independent(self, repo, caplog) -> None: + """10bp spread passes the gate. The #332 warning is a SEPARATE consumer of the same + book: an entry materially off the ask still warns (and places), and an entry at the + market stays silent -- the gate changes neither behavior.""" + tight = FakeBroker(preview=_book_preview(Decimal("50000"), Decimal("50010"))) + + with caplog.at_level(logging.WARNING): + placed = execute( + _enter_signal(_setup(entry=Decimal("50005"))), + tight, + repo, + _config(), + "autonomous", + now_ts=NOW_TS, + ) + + assert placed.placed is True + assert placed.vetoed_by == [] + assert not [r for r in caplog.records if r.getMessage() == _SPREAD_REFUSED_EVENT] + assert not [r for r in caplog.records if r.getMessage() == _OVERRIDE_EVENT] + + # Same tight book, entry 58bp ABOVE the ask: the #332 warning fires, the gate still + # passes, and the order places -- one book, two independent consumers. + with caplog.at_level(logging.WARNING): + warned = execute( + _enter_signal(_setup(entry=Decimal("50300"))), + FakeBroker(preview=_book_preview(Decimal("50000"), Decimal("50010"))), + repo, + _config(), + "autonomous", + now_ts=NOW_TS, + ) + + assert warned.placed is True + assert _override_fields(caplog)["market_ref"] == "50010" + assert not [r for r in caplog.records if r.getMessage() == _SPREAD_REFUSED_EVENT] + + def test_a_spread_exactly_at_the_threshold_is_refused(self, repo) -> None: + """The boundary is pinned: >= refuses, a hair under passes. + + 49,000/51,000 is a 2,000-wide book on a 50,000 mid -- exactly 0.04. AT the line the + spread alone already consumes the model's entire worst-case per-leg cost, leaving the + taker fee wholly outside it, so "at" is already too wide -- the fail-closed side of + the line, unlike #332's visibility-only strictly-greater. + """ + from keel.config import ExecutionConfig + + at_the_line = _config(execution=ExecutionConfig(max_entry_spread_pct=Decimal("0.04"))) + broker = FakeBroker(preview=_book_preview(Decimal("49000"), Decimal("51000"))) + result = execute( + _enter_signal(_setup(entry=Decimal("50000"))), + broker, + repo, + at_the_line, + "autonomous", + now_ts=NOW_TS, + ) + assert result.placed is False + assert result.vetoed_by == [SPREAD_GATE_VETO] + assert broker.place_calls == [] + + a_hair_under = _config(execution=ExecutionConfig(max_entry_spread_pct=Decimal("0.0401"))) + broker = FakeBroker(preview=_book_preview(Decimal("49000"), Decimal("51000"))) + result = execute( + _enter_signal(_setup(entry=Decimal("50000"))), + broker, + repo, + a_hair_under, + "autonomous", + now_ts=NOW_TS, + ) + assert result.placed is True + assert result.vetoed_by == [] + + def test_a_sell_with_a_monstrous_spread_is_never_gated(self, repo, caplog) -> None: + """Exits must execute: a SELL through the same preview/place pipeline sees a 400bp + spread and places anyway. Trapping an exit in a wide book would strand the position + exactly when the rule says leave.""" + broker = FakeBroker(preview=_book_preview(Decimal("48000"), Decimal("50000"))) # 400bp + + with caplog.at_level(logging.WARNING): + result = scale_out( + broker, + repo, + _config(), + product_id="BTC-USD", + qty=Decimal("0.001"), + exit_price=Decimal("50000"), + rule_name="position_rule", + now_ts=NOW_TS, + ) + + assert result.placed is True + assert result.vetoed_by == [] + assert len(broker.place_calls) == 1 + assert not [r for r in caplog.records if r.getMessage() == _SPREAD_REFUSED_EVENT] + assert not [r for r in caplog.records if r.getMessage() == _BOOK_UNREADABLE_EVENT] + + def test_an_unreadable_book_refuses_the_live_buy_with_a_distinct_reason( + self, repo, caplog + ) -> None: + """Fail-closed: a live BUY whose preview carries no readable bid/ask is refused with + `book_unreadable` -- missing keys (the degraded venue shape), a NaN or non-numeric + side, or a non-positive one. The gate must not guess a spread, and must say loudly + WHY it refused, distinguishing 'too wide' from 'cannot know'.""" + bookless = FakeBroker( + preview={ + "order_total": Decimal("50.00"), + "commission_total": Decimal("0.30"), + "errs": [], + "warning": [], + # no best_bid/best_ask keys at all -- the degraded response shape + } + ) + + with caplog.at_level(logging.WARNING): + result = execute( + _enter_signal(), bookless, repo, _config(), "autonomous", now_ts=NOW_TS + ) + + assert result.placed is False + assert result.vetoed_by == [BOOK_UNREADABLE_VETO] + assert bookless.place_calls == [] + fields = _gate_fields(caplog, _BOOK_UNREADABLE_EVENT) + assert fields["product"] == "BTC-USD" + assert fields["veto"] == BOOK_UNREADABLE_VETO + + # Each individual way a side can be unreadable, through the same `execute` path. + for bad in ("nan", "not-a-number", "0", "-50000"): + broker = FakeBroker(preview=_book_preview(bad, "50000")) + with caplog.at_level(logging.WARNING): + refused = execute( + _enter_signal(), broker, repo, _config(), "autonomous", now_ts=NOW_TS + ) + assert refused.placed is False, f"bid={bad!r} must be unreadable, not traded" + assert refused.vetoed_by == [BOOK_UNREADABLE_VETO] + assert broker.place_calls == [] + + broker = FakeBroker(preview=_book_preview("50000", "nan")) + with caplog.at_level(logging.WARNING): + refused = execute(_enter_signal(), broker, repo, _config(), "autonomous", now_ts=NOW_TS) + assert refused.vetoed_by == [BOOK_UNREADABLE_VETO] + assert broker.place_calls == [] + + def test_the_port_preview_shape_is_gated_too(self, repo, caplog) -> None: + """`Preview` (Phase B's shape) carries the book as strings inside `detail`; the gate + must read it the same way #332's warning does, or the migration would silently disarm + the gate.""" + from keel_broker_api.results import Preview + + preview = Preview( + product_id="BTC-USD", + side=Side.BUY, + est_base_size=Decimal("0.001"), + est_quote_size=Decimal("50"), + est_fee=Decimal("0.30"), + synthetic=False, + detail={"best_bid": "50000", "best_ask": "50300"}, + ) + broker = FakeBroker() + broker.preview_order = lambda *a, **k: preview # type: ignore[method-assign] + + with caplog.at_level(logging.WARNING): + result = execute(_enter_signal(), broker, repo, _config(), "autonomous", now_ts=NOW_TS) + + assert result.placed is False + assert result.vetoed_by == [SPREAD_GATE_VETO] + assert broker.place_calls == [] diff --git a/tests/execution/test_reconcile.py b/tests/execution/test_reconcile.py index 920d4d73..de6cdce3 100644 --- a/tests/execution/test_reconcile.py +++ b/tests/execution/test_reconcile.py @@ -75,7 +75,11 @@ def get_accounts(self) -> list[dict[str, Any]]: def preview_order(self, product_id: str, side: Any, order_configuration: dict) -> dict: return {"order_total": Decimal("50"), "commission_total": Decimal("0"), - "errs": [], "warning": []} + "errs": [], "warning": [], + # Both book sides, as the real venue returns them: #350's spread gate + # fails closed on a preview without them (reconcile places SELLs only, + # which the gate never touches -- this keeps the fake honest anyway). + "best_bid": Decimal("49990"), "best_ask": Decimal("50000")} def place_order(self, product_id: str, side: Any, order_configuration: dict) -> dict: self.placed.append({"product_id": product_id, "side": side, diff --git a/tests/fixtures/config_golden_defaults.json b/tests/fixtures/config_golden_defaults.json index bb77964b..f70e6f6b 100644 --- a/tests/fixtures/config_golden_defaults.json +++ b/tests/fixtures/config_golden_defaults.json @@ -17,6 +17,9 @@ "budget_usd": "0", "cadence_days": 7 }, + "execution": { + "max_entry_spread_pct": "0.005" + }, "fees": { "maker_pct": "0.006", "taker_pct": "0.012" diff --git a/tests/fixtures/config_golden_full.json b/tests/fixtures/config_golden_full.json index 63f5d89e..79d1b19b 100644 --- a/tests/fixtures/config_golden_full.json +++ b/tests/fixtures/config_golden_full.json @@ -19,6 +19,9 @@ "budget_usd": "75.5", "cadence_days": 14 }, + "execution": { + "max_entry_spread_pct": "0.0075" + }, "fees": { "maker_pct": "0.0035", "taker_pct": "0.0075" diff --git a/tests/fixtures/config_golden_full.yaml b/tests/fixtures/config_golden_full.yaml index 1530064e..1a7ad782 100644 --- a/tests/fixtures/config_golden_full.yaml +++ b/tests/fixtures/config_golden_full.yaml @@ -102,3 +102,7 @@ logging: research: pbo_max: 0.10 slope_floor: -0.75 + +# Non-default on purpose (the default is 0.005 = 50bp) -- see the `research` note above. +execution: + max_entry_spread_pct: 0.0075 diff --git a/tests/test_agent.py b/tests/test_agent.py index a883c521..a77b9af9 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -85,6 +85,11 @@ def preview_order(self, product_id: str, side: Any, order_configuration: dict) - "commission_total": Decimal("0"), "errs": [], "warning": [], + # Both book sides, as the real venue returns them: #350's spread gate fails + # closed on a preview without them (tests that mean a degraded/bookless + # response pass their own preview dict). + "best_bid": Decimal("99.95"), + "best_ask": Decimal("100"), } def place_order(self, product_id: str, side: Any, order_configuration: dict) -> dict: @@ -1459,6 +1464,31 @@ def test_paper_enter_sizes_off_paper_equity(repo): assert orders[0]["qty"] != Decimal("1"), "must not fill the old fixed 1-unit qty" +def test_paper_mode_never_runs_the_entry_spread_gate(repo): + """#350's max-spread gate is live-path ONLY: paper fills are synthetic and see no book, so + `_paper_enter` never previews an order and the gate (fail-closed on an unreadable book for + a live BUY) must not refuse a paper entry. The same "no readable book" condition that + REFUSES a live BUY therefore fills here -- which is exactly why the paper-hourly profile + accrues NO evidence about the gate, and why the gate ships before any live resumption + rather than being validated on paper first. + """ + from keel.strategy.paper import PaperTrader + + trader = PaperTrader(repo) + trader.seed_cash(Decimal("30000"), now_ts=1_000) + repo.set_state("last_feed_ts", 90_000) + config = _paper_config() # the gate is armed at its default 50bp threshold + sig = _paper_enter_signal(entry=Decimal("100"), stop=Decimal("90"), target=Decimal("130")) + + result = agent._paper_enter( + trader, sig, repo, config, now_ts=90_000, paper_equity=Decimal("30000") + ) + + assert result.placed + assert result.vetoed_by == [] + assert repo.get_orders(mode="paper") and repo.get_orders(mode="live") == [] + + def test_run_once_sizes_a_paper_entry_off_the_seeded_synthetic_equity(repo, monkeypatch): """Loop-level: the `equity_now` Task 5 computes for the paper branch is what sizes the fill, not a re-derived value and not the fixed 1-unit qty `_AlwaysEnterRule` used to produce.""" diff --git a/tests/test_cli.py b/tests/test_cli.py index c81896ac..e5997e2c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -55,6 +55,10 @@ def preview_order(self, product_id: str, side: Any, order_configuration: dict) - "commission_total": Decimal("0"), "errs": [], "warning": [], + # Both book sides, as the real venue returns them: #350's spread gate fails + # closed on a preview without them. + "best_bid": Decimal("99.95"), + "best_ask": Decimal("100"), } def place_order(self, product_id: str, side: Any, order_configuration: dict) -> dict: diff --git a/tests/test_config.py b/tests/test_config.py index 3db16c07..d0e5ab6b 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -580,6 +580,40 @@ def test_load_config_logging_empty_file_raises_configerror(write_config): load_config(path) +# -- execution (the #350 routing-time max-spread entry gate) ------------------------------------ + + +def test_load_config_execution_defaults_to_a_50bp_entry_spread_cap(valid_config_path): + """Omitted -> the conservative default 0.005 (50bp), anchored to #334's + `strategy/backtest.SLIPPAGE_CAP_PCT`: if the spread ALONE exceeds the worst per-leg cost + the backtest ever assumes, the fill economics are materially worse than modeled.""" + config = load_config(valid_config_path) + + assert config.execution.max_entry_spread_pct == Decimal("0.005") + assert isinstance(config.execution.max_entry_spread_pct, Decimal) + + +def test_load_config_execution_spread_cap_overridable(write_config): + text = VALID_CONFIG_YAML + "\nexecution:\n max_entry_spread_pct: 0.0075\n" + path = write_config(text) + + config = load_config(path) + + assert config.execution.max_entry_spread_pct == Decimal("0.0075") + + +@pytest.mark.parametrize("bad", ["0", "-0.001", "0.2", "nan", "wide"]) +def test_load_config_execution_spread_cap_out_of_range_raises_configerror(write_config, bad): + """The threshold must be a finite number in (0, 0.10]: 0 disables the gate silently, a + huge value guts it, and NaN would raise InvalidOperation inside the gate's comparison + instead of failing at load where the operator is editing the file.""" + text = VALID_CONFIG_YAML + f"\nexecution:\n max_entry_spread_pct: {bad}\n" + path = write_config(text) + + with pytest.raises(ConfigError, match="execution.max_entry_spread_pct"): + load_config(path) + + def test_load_secrets_missing_env_returns_empty_dict(tmp_path): missing_path = tmp_path / "does-not-exist.env" From 6c648cc09cb0a075a02c9947847e0234174a50e7 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Tue, 18 Aug 2026 02:10:57 -0400 Subject: [PATCH 2/2] test(executor): pin the spread gate's fail-closed arm and gate-before-confirm ordering Two review findings on #350's money-path arms, both test-only, plus one docstring enumeration: - The extreme-exponent fail-closed arm of `_entry_spread_gate` (the `except ArithmeticError` whose spread is uncomputable) had no test. Pinned both ways a readable-sides book can still break the arithmetic: an `Overflow` on `bid + ask` at `1E+999999999` magnitudes, and a `DivisionByZero` from a subnormal pair whose mid half-even rounds to zero while the difference survives nonzero. #332's warning swallows these (telemetry); the gate must refuse with `book_unreadable` -- each case verified to fail under a swallow-like-telemetry mutation before landing. - Every gate test ran autonomous mode, so nothing pinned that the gate sits BEFORE the confirm gate. A wide-book BUY in `mode="confirm"` with an approving confirm_fn is still refused and the approver is never consulted -- a human cannot approve around the gate. Verified to fail under a confirm-first reordering mutation. - `summarise_cycle`'s docstring enumerated two non-placement causes (rails, confirm gate); added the third honestly -- the routing-time entry-spread gate refusing a live BUY. Docstring only, no behavior change. --- keel/commands/activity.py | 3 +- tests/execution/test_executor.py | 75 ++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/keel/commands/activity.py b/keel/commands/activity.py index 332770d9..90b0dac3 100644 --- a/keel/commands/activity.py +++ b/keel/commands/activity.py @@ -632,7 +632,8 @@ def summarise_cycle(cycle_id: str | None, events: Sequence[ActivityEvent]) -> Ac counting `engine.setup_detected` for a cycle whose `signals_evaluated` records fell outside the read window. * `blocked` -- entries that did NOT become an order: an `agent.enter_evaluated` with - `placed=false` (rails vetoed it, or the confirm gate declined), plus an + `placed=false` (rails vetoed it, the routing-time entry-spread gate refused the live + BUY's book, or the confirm gate declined), plus an `agent.entry_bar_not_ready` (withheld before it was ever evaluated). One per entry, never one per *reason* -- the PAXG cycle of 2026-08-08 trips two guards on a single signal, and reporting `blocked=2` there would imply two setups where there was one. diff --git a/tests/execution/test_executor.py b/tests/execution/test_executor.py index 27a0517a..026ed34a 100644 --- a/tests/execution/test_executor.py +++ b/tests/execution/test_executor.py @@ -2212,3 +2212,78 @@ def test_the_port_preview_shape_is_gated_too(self, repo, caplog) -> None: assert result.placed is False assert result.vetoed_by == [SPREAD_GATE_VETO] assert broker.place_calls == [] + + def test_extreme_magnitude_books_are_refused_not_swallowed(self, repo, caplog) -> None: + """The arithmetic arm of fail-closed: a book whose sides PARSE, are finite, and are + positive can still make the spread itself uncomputable. Decimal admits extreme + exponents (`1E+999999999` constructs and compares fine -- #336's lesson, mirrored by + #332's sibling test above), and `bid + ask` on such magnitudes raises Overflow; a + subnormal pair can round the mid to zero and raise DivisionByZero on the divide. + #332's warning SWALLOWS both (telemetry must never fail a routing); a money gate + must REFUSE on them -- an uncomputable spread is an unreadable book, never an abort + and never a pass.""" + huge = FakeBroker(preview=_book_preview(Decimal("1E+999999999"), Decimal("9E+999999999"))) + + with caplog.at_level(logging.WARNING): + result = execute( + _enter_signal(), huge, repo, _config(), "autonomous", now_ts=NOW_TS + ) + + assert result.placed is False + assert result.vetoed_by == [BOOK_UNREADABLE_VETO] + assert "spread uncomputable" in (result.reason or "") + assert huge.place_calls == [] + + # A subnormal pair: the sum rounds to one ulp, half of which half-even rounds the mid + # to ZERO, while the difference survives as nonzero -- so `(ask - bid) / mid` raises + # DivisionByZero. Both raises are ArithmeticErrors; both must land in the same + # fail-closed arm, refusing (not crashing) exactly like the overflow case. + subnormal = FakeBroker( + preview=_book_preview(Decimal("4E-1000028"), Decimal("1.44E-1000026")) + ) + + with caplog.at_level(logging.WARNING): + refused = execute( + _enter_signal(), subnormal, repo, _config(), "autonomous", now_ts=NOW_TS + ) + + assert refused.placed is False + assert refused.vetoed_by == [BOOK_UNREADABLE_VETO] + assert "spread uncomputable" in (refused.reason or "") + assert subnormal.place_calls == [] + fields = _gate_fields(caplog, _BOOK_UNREADABLE_EVENT) + assert fields["product"] == "BTC-USD" + assert fields["veto"] == BOOK_UNREADABLE_VETO + + def test_a_wide_book_refuses_in_confirm_mode_without_ever_consulting_the_approver( + self, repo + ) -> None: + """Gate BEFORE confirm, pinned: every other gate test runs autonomous, which never + exercises the ordering. Here a wide-book BUY runs in `mode="confirm"` with an + approver attached -- and is STILL refused, with the approver NEVER consulted: the + spread gate sits upstream of the confirm gate in `_run_order`, so a human (or any + approving `confirm_fn`) cannot approve around a book the gate judged too wide. A + gate an operator could overrule is a suggestion, not a gate.""" + consulted: list[dict] = [] + + def _approve(preview) -> bool: # would approve -- and must never get the chance + consulted.append(preview) + return True + + broker = FakeBroker(preview=_book_preview(Decimal("50000"), Decimal("50300"))) + + result = execute( + _enter_signal(_setup(entry=Decimal("50150"))), # at the mid: isolate the GATE + broker, + repo, + _config(), + mode="confirm", + confirm_fn=_approve, + now_ts=NOW_TS, + ) + + assert result.placed is False + assert result.vetoed_by == [SPREAD_GATE_VETO] + assert broker.place_calls == [] + assert repo.get_orders() == [] + assert consulted == []