feat(engine): two observable invariants, because neither past defect was findable by testing - #262
Merged
Merged
Conversation
…was findable by testing #254 and #257 both survived the entire project with 2,712 tests passing. Neither was found by looking for defects: #254 surfaced from a 34x non-monotonic trade count, #257 from asking why #254's fix REDUCED trade counts. The reason tests could not catch either generalises -- a frozen backtest and a highly selective strategy produce identical output, and so do a patient limit fill and a lucky one. Adding tests does not help, because a test asserts a behaviour someone already imagined. These two make each defect class announce itself in the ordinary output of an ordinary run. 1. PENDING LIFESPAN INVARIANT (keel/strategy/backtest.py) Asserts a pending setup is never carried across more than one bar. Since #257 the fill is unconditional, so the correct value is exactly 1 for every rule on every series -- checked, not thresholded, with no per-asset tuning and no false-positive mode. On UNI-USD under #254 this counter would have read ~40,000. The obvious alternative -- warn when trades stop long before the series ends -- was considered and REJECTED. #257 made the freeze structurally impossible, so a dead tail now only ever means the rule genuinely stopped firing (regime change, threshold too strict). That warning would fire exclusively on legitimate runs and burn the attention budget a real alert needs. If resting entry orders return (#260 Option B) the bound stops being 1, and the value it becomes IS the cancel/replace policy, stated in one place. 2. INTENT DIVERGENCE LOG (keel/execution/executor.py) The executor already persisted the rule's intended entry (`expected_fill`) and the achieved fill (`actual_fill`) on the same order row, and nothing compared them. That missing subtraction is exactly how #257 stayed invisible. `_log_intent_divergence` now reports signed basis points on every fill. Logged UNCONDITIONALLY, not past a threshold: a cutoff right for BTC is wrong for a thin book, and the per-asset liquidity model that would set one does not exist (#259). Gating this on that work would block the cheap half behind the expensive half. Same principle as #247 printing the fee rate -- make the number visible first, act on it second. Signed rather than absolute, because "we paid up" and "we got filled cheaper" are opposite failures for a rule whose entry encodes a condition. Never raises: telemetry on a settled order must not be able to fail a cycle. VERIFICATION. The lifespan invariant cannot be violated through the public API, so there is no red-then-green test and pretending otherwise would be theatre. Instead it was proven live: the fill path was deliberately broken and the assertion fired immediately, naming the rule and bar index. It also now sits in the hot loop of every backtest, so all 2,717 tests and the baseline golden execute it on every run. The divergence log has four real tests asserting on the STRUCTURED payload rather than caplog.text, which would have passed vacuously for any values. Neither touches order routing. 2717 passed, ruff clean. Refs #254, #257, #259, #260. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The operational takeaway from #261 §5, implemented. Neither #254 nor #257 was found by looking for
defects — #254 surfaced from a 34× non-monotonic trade count, #257 from asking why #254's fix
reduced trade counts — and 2,712 tests passed throughout both.
The reason tests couldn't catch either generalises: a frozen backtest and a highly selective
strategy produce identical output. So do a patient limit fill and a lucky one. Adding tests
doesn't help, because a test asserts a behaviour someone already imagined. These two make each
defect class announce itself in the ordinary output of an ordinary run. Neither touches order
routing.
1. Pending lifespan invariant —
keel/strategy/backtest.pyAsserts a pending setup is never carried across more than one bar. Since #257 the fill is
unconditional, so the correct value is exactly 1 for every rule on every series — checked
rather than thresholded, no per-asset tuning, no false-positive mode. On UNI-USD under #254
this counter would have read ~40,000.
The dead-tail heuristic was considered and rejected. #257 made the freeze structurally
impossible, so a dead tail now only ever means the rule genuinely stopped firing — a regime change,
a threshold too strict for recent volatility. A warning on "trades stopped >15% before the corpus
ends" would fire exclusively on legitimate runs, and the first three false alarms teach an
operator to ignore the fourth. That costs more than no alert.
If resting entry orders return (#260 Option B), the bound stops being 1 — and the value it
becomes IS the cancel/replace policy, stated in one place.
2. Intent divergence log —
keel/execution/executor.pyThe executor already persisted the rule's intended entry (
expected_fill) and the achieved fill(
actual_fill) on the same order row, and nothing compared them. That missing subtraction isexactly how #257 stayed invisible.
_log_intent_divergencenow reports signed basis points onevery fill.
per-asset liquidity model that would set one doesn't exist yet (slippage is one global 5bp constant applied from BTC to TON #259). Gating this on that work
would block the cheap half behind the expensive half. Same principle as fix(strategy): price fills at the taker rate, and wire the dormant PBO gate into promotion #247 printing the fee
rate — make the number visible first, act on it second.
rule whose entry encodes a condition.
Verification, including what could not be tested
The lifespan invariant cannot be violated through the public API — since #257 there is no input
that makes the engine carry a setup. So there is no red-then-green test, and writing one that
passes trivially would be theatre. The test class says this in its docstring rather than implying
coverage it doesn't have.
What was done instead:
execute it on every run. A reintroduced carry fails the suite loudly instead of silently.
The divergence log has four real tests, asserting on the structured payload (
log_eventattaches fields via
extra) rather thancaplog.text— which shows only the event name and wouldhave passed vacuously for any values at all. I hit that exact false-pass while writing them.
2717 passed, 1 skipped;
ruff check keel tests packagesclean.Refs #254, #257, #259, #260.
🤖 Generated with Claude Code