Skip to content

chore(fidelity): close Phase 9's loose ends — alias, stale docstrings, telemetry overflow guard - #336

Merged
eaitbrahim merged 1 commit into
mainfrom
chore/phase9-fidelity-cleanup
Aug 17, 2026
Merged

chore(fidelity): close Phase 9's loose ends — alias, stale docstrings, telemetry overflow guard#336
eaitbrahim merged 1 commit into
mainfrom
chore/phase9-fidelity-cleanup

Conversation

@eaitbrahim

Copy link
Copy Markdown
Contributor

What & why

Closes Phase 9's loose ends, from the independent whole-phase review (all findings non-blocking; this PR is bookkeeping and robustness, not behavior):

Tests-first evidence

The overflow guard was written failing-first conceptually and verified directly (Decimal('1E+999999999') against a quoted preview raises through the old code, returns silently on the new); the regression case is now in test_a_preview_without_a_book_quote_is_silent_not_fatal. The alias pin lives in test_simulate_reports_per_product_slippage_beside_the_results, where the claim it guards is rendered.

Gates

  • uv run ruff check keel tests packages — All checks passed!
  • uv run mypy — Success: no issues found in 237 source files
  • uv run pytest -q — 2887 passed, 1 skipped

Scope check

  • No rail, rule, or classification touched. No behavior change except one: the entry-override telemetry can no longer raise on pathological exponents (strictly toward "never raises").

… to the engine floor, refresh stale #259-era docstrings, guard the warning's arithmetic

From the independent phase review: _SIM_SLIPPAGE_PCT is now an alias of
SLIPPAGE_FLOOR_PCT (the report's flat-cost claim becomes structurally true, pinned
by test, instead of true by numeric coincidence); the executor's 50bp-threshold
rationale cites the floor constant rather than the CLI literal and names the
coincidence with #259's 50bp cap; the intent-divergence docstring no longer claims
the liquidity model does not exist; the entry-override arithmetic moved inside a
try (an extreme-but-finite exponent raises ArithmeticError on division, and
telemetry must never fail a routing). Promotion-gate opt-in now tracked in #335.
@eaitbrahim
eaitbrahim merged commit 6732dcc into main Aug 17, 2026
5 checks passed
@eaitbrahim
eaitbrahim deleted the chore/phase9-fidelity-cleanup branch August 17, 2026 20:18
eaitbrahim added a commit that referenced this pull request Aug 18, 2026
* 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.

* 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant