From 20156da7e598c883c7e9b6d0b4e39fae3ab3cf26 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Mon, 17 Aug 2026 16:16:49 -0400 Subject: [PATCH] =?UTF-8?q?chore(fidelity):=20close=20Phase=209's=20loose?= =?UTF-8?q?=20ends=20=E2=80=94=20alias=20the=20flat=20slippage=20to=20the?= =?UTF-8?q?=20engine=20floor,=20refresh=20stale=20#259-era=20docstrings,?= =?UTF-8?q?=20guard=20the=20warning's=20arithmetic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- keel/cli.py | 6 +++++- keel/execution/executor.py | 30 ++++++++++++++++++++++-------- tests/execution/test_executor.py | 7 +++++++ tests/test_cli.py | 6 ++++++ 4 files changed, 40 insertions(+), 9 deletions(-) diff --git a/keel/cli.py b/keel/cli.py index 8da12758..6bf1f4df 100644 --- a/keel/cli.py +++ b/keel/cli.py @@ -1746,7 +1746,11 @@ def pnl(ctx: click.Context, asset: str | None, raw_marks: tuple[str, ...]) -> No # table and benchmark comparison was priced at half the cost of trading # (`docs/experiments/2026-08-11-hourly-backtest-turtle-breakout.md` ยง5). _SIM_FEE_PCT = backtest_mod.TAKER_FEE_PCT -_SIM_SLIPPAGE_PCT = Decimal("0.0005") +# Aliased to the engine's floor (#259's cleanup), not repeated: the simulate report asserts +# its flat-priced dollar sections cost "the flat SLIPPAGE_FLOOR_PCT per leg", and that claim +# must be structurally true, not true by numeric coincidence that a retune would silently +# break. (`portfolio_sim` and `paper` still carry their own literals -- see #335.) +_SIM_SLIPPAGE_PCT = backtest_mod.SLIPPAGE_FLOOR_PCT def _sim_fee_pct(config: Config) -> Decimal: diff --git a/keel/execution/executor.py b/keel/execution/executor.py index 30c2d6bf..6ff51b4c 100644 --- a/keel/execution/executor.py +++ b/keel/execution/executor.py @@ -645,11 +645,13 @@ def _log_intent_divergence(order_id: int, intent: OrderIntent | None, realized: follow-through) production silently takes trades the rule meant to decline (#260). Logged UNCONDITIONALLY rather than past a threshold. A basis-point threshold that is right for - BTC is wrong for a thin book, and the per-asset liquidity model that would set it does not - exist yet (#259) -- gating this on that work would block the cheap half behind the expensive - half. This follows what #247 did with the fee rate: make the number visible first, act on it - second. `divergence_bps` is signed, so direction is legible without recomputing it: positive - means the fill came in ABOVE the rule's intended entry. + BTC is wrong for a thin book -- the per-asset liquidity model that would set it now exists + (#259, `strategy/backtest.slippage_for_quote_volume`) but lives on the RESEARCH side; the + live path has no such statistic in hand at fill time, and gating on the cheap half behind + the expensive half was declined when this shipped. This follows what #247 did with the fee + rate: make the number visible first, act on it second. `divergence_bps` is signed, so + direction is legible without recomputing it: positive means the fill came in ABOVE the + rule's intended entry. Never raises. This is telemetry attached to an order that has already been placed and settled; a formatting problem here must not fail a cycle. @@ -682,8 +684,11 @@ def _log_intent_divergence(order_id: int, intent: OrderIntent | None, realized: #: #: A VISIBILITY threshold, not a correctness one: crossing it changes no order, only whether #: the operator is told. Anchored in this repo's own cost model, where a fill is priced at a -#: 1.2% taker fee per leg (`strategy/backtest.TAKER_FEE_PCT`) plus 5bp of slippage -#: (`cli._SIM_SLIPPAGE_PCT`). Against that, a deviation of a few bp is microstructure -- the +#: 1.2% taker fee per leg (`strategy/backtest.TAKER_FEE_PCT`) plus a slippage FLOOR of 5bp +#: (`strategy/backtest.SLIPPAGE_FLOOR_PCT`; #259 scales it up to a 50bp cap on thin books -- +#: so "10x the slippage assumption" below holds at the liquid end and narrows toward the cap, +#: where a firing is all the more truthful). Against that, a deviation of a few bp is +#: microstructure -- the #: drift any enter-at-close rule (`turtle_breakout`, `rsi_meanrev`) accumulates by routing one #: cycle after its signal bar -- while tens of bp means the rule's entry encodes a CONDITION: #: `pullback_continuation` enters at `signal_candle.high + buffer_ticks`, which sits above @@ -695,6 +700,8 @@ def _log_intent_divergence(order_id: int, intent: OrderIntent | None, realized: #: enter-at-close rule's signal and the next cycle's ask CAN exceed the line in volatile #: stretches; that firing is truthful (the fill really is that far off intent) and #: informative, not spurious. This is a VISIBILITY threshold, not a correctness one. +#: (Its 50bp is COINCIDENTALLY equal to #259's slippage CAP -- different constants, different +#: modules, no coupling; do not retune one on the other's reasoning.) #: The comparison is strictly greater: a deviation exactly at the line is "at", not #: "beyond", and logs nothing. ENTRY_OVERRIDE_WARN_BP = Decimal("50") @@ -778,7 +785,14 @@ def _warn_if_market_routing_overrides_entry( return if not expected.is_finite() or expected <= 0: return - deviation_bps = (expected - ref) / ref * Decimal(10_000) + # The arithmetic stays INSIDE a try, matching `_log_intent_divergence`: `is_finite()` + # admits extreme exponents (a rule bug like 1E+999999999 parses and compares fine), and + # Decimal division/multiplication on such magnitudes raises ArithmeticError -- which + # telemetry must swallow, never propagate into the routing it observes. + try: + deviation_bps = (expected - ref) / ref * Decimal(10_000) + except ArithmeticError: + return if abs(deviation_bps) <= ENTRY_OVERRIDE_WARN_BP: return log_event( diff --git a/tests/execution/test_executor.py b/tests/execution/test_executor.py index 83e2af36..5a1e44e2 100644 --- a/tests/execution/test_executor.py +++ b/tests/execution/test_executor.py @@ -1868,6 +1868,13 @@ def test_a_preview_without_a_book_quote_is_silent_not_fatal(self, caplog) -> Non _warn_if_market_routing_overrides_entry( self._intent(entry="0"), _quoted_preview("50000") ) + # An extreme-but-finite exponent (a rule bug, not venue data): parses, is_finite, + # and compares fine -- the DIVISION is what raises (Decimal Overflow, an + # ArithmeticError). Telemetry must swallow it, matching intent_divergence's + # inside-the-try arithmetic. + _warn_if_market_routing_overrides_entry( + self._intent(entry="1E+999999999"), _quoted_preview("50000") + ) assert not [r for r in caplog.records if r.getMessage() == _OVERRIDE_EVENT] diff --git a/tests/test_cli.py b/tests/test_cli.py index ad61880c..14fb240c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -26,6 +26,7 @@ from keel.commands._common import DISCLAIMER from keel.data.db import connect, migrate from keel.data.repository import Repository +from keel.strategy.backtest import SLIPPAGE_FLOOR_PCT from keel.types import Candle, Granularity from tests.conftest import VALID_CONFIG_YAML @@ -981,6 +982,11 @@ def test_simulate_reports_per_product_slippage_beside_the_results(tmp_path, monk A profit factor printed without its assumed slippage has the same problem a profit factor printed without its fee rate had (#247): the reader cannot check the number. """ + # The flat rate simulate's dollar sections use is ALIASED to the engine's floor, not a + # repeated literal: the report asserts those sections cost "the flat SLIPPAGE_FLOOR_PCT + # per leg", and that claim must be structurally true -- a retuned floor with a stray + # 0.0005 literal here would make the report's own cost statement silently false. + assert cli_module._SIM_SLIPPAGE_PCT == SLIPPAGE_FLOOR_PCT db_path = tmp_path / "sim.db" out_path = tmp_path / "report.md" repo = _repo_at(db_path)