fix(strategy): re-detect while flat, so an unfilled setup cannot switch the detector off - #256
Merged
Merged
Conversation
…ch the detector off
`backtest()` set `pending` and, if the entry was never touched, carried it forward unchanged.
The only path that cleared it without a fill was the stop being touched first. So a setup whose
entry and stop were both never revisited pinned `pending` for the rest of the series: the
`position is None and pending is None` branch never ran again, `rule.detect()` was never called
again, and the engine silently switched its own detector off.
Nothing errored and no warning fired. The output was indistinguishable from a rule that simply
found no further setups — a frozen backtest looks exactly like a selective one, which is how this
survived every experiment in docs/experiments/.
Measured: `rsi_meanrev` on UNI-USD, hourly, everything default except `oversold`.
oversold=30 -> 309 closed trades, last exit 2026-08-04 (trades throughout)
oversold=35 -> 9 closed trades, last exit 2021-11-15 (dead ~40,000 bars)
LOOSENING the entry threshold cut the trade count 34x. Also visible as non-monotonic n on
BTC-USD (181 -> 80 between oversold 30 and 35) and AAVE-USD.
The fix re-detects instead of expiring after N bars, because N never existed in production:
`strategy/engine.py::evaluate` calls `rule.detect()` once per cycle unconditionally and carries
no pending-setup state between cycles. An unexecuted setup is simply re-derived from fresh data.
This makes the simulator match the live path rather than adopting a new tunable.
No lookahead is introduced: the fill attempt still happens first, and the re-detect uses the same
`candles[: i + 1]` window the flat branch would have used, so a setup derived on bar i can still
only fill on bar i+1 or later. The open-position check — not the pending one — is what enforces
one-position-at-a-time, and it is unchanged.
The committed BTC daily baseline fixture is UNCHANGED by this, since no freeze occurs on that
corpus. Two regression tests added from the UNI-USD case, both verified to fail on the unfixed
code (detect_calls 1 vs 3).
Closes #254.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eaitbrahim
added a commit
that referenced
this pull request
Aug 12, 2026
…arket orders do (#258) The simulator held a Setup until a later bar's range TOUCHED its entry, then filled AT that level. Production does not do that. `execution/executor.py::_order_row` writes: order_type="market", limit_price=None, expected_fill=intent.entry so live never rests an order at `Setup.entry` and never waits for price to come to it -- when `engine.evaluate` emits a signal and the rails pass, the executor buys at market that cycle. The old model therefore granted the backtest two things the live box does not have: 1. Free optionality on the entry price. A setup only became a trade if the market offered the chosen level, so unfavourable entries were silently declined. Live pays whatever the market is doing, favourable or not. 2. Unbounded patience. Any later bar touching the level filled there. Both flatter results, and the bias runs OPPOSITE to #254's, which suppressed trades. Entries now fill at `candles[i+1].open` plus slippage -- the first price obtainable once the signal exists, and the earliest fill involving no lookahead. `Setup.entry` becomes informational, exactly as `expected_fill` is live; risk is measured from the achieved fill against the setup's stop, never from the quoted entry. This subsumes #254: every pending now resolves on the very next bar, so the state that froze the detector is unreachable rather than merely handled. It also removes the wrinkle in #247's fee justification -- "touching entry is marketable, therefore taker" held for a breakout entry above the market but not for a mean-reversion entry below it, which touching would make a MAKER fill. With every entry a market order, taker is unconditionally the right rate. Consequence named rather than hidden: a rule encoding a CONFIRMATION condition in its entry price no longer gets one. `pullback_continuation` sets `entry = signal_candle.high + buffer` precisely to demand follow-through, and a market fill takes trades it meant to decline. That is not introduced here -- it is what the live box already does. Making the executor honour a stop/limit entry is the alternative (#257 "Option B"); it changes money-moving code to accommodate a rule already measured as alpha-deficient, so it is deliberately not taken. Tests: entry-vs-stop intrabar resolution is deleted -- that ambiguity only arises for an entry seeking a level, and is now unreachable. `_resolve_order` is still exercised for stop-vs-target. #254's regression class is restated as `TestEntryFillsAtNextBarOpen`, asserting a setup whose entry is never touched fills anyway, and that `detect_calls` is 1 rather than #256's 3 -- the number that distinguishes the two fill models. Golden regenerated. On the committed BTC daily corpus the move is tiny (profit_factor 1.269371 -> 1.269287, n_trades 13 unchanged) because `turtle_breakout` enters at the close and 24/7 crypto barely gaps. That is this corpus being insensitive, not the fill model being unimportant -- it does not exercise an offset entry. Closes #257. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
eaitbrahim
added a commit
that referenced
this pull request
Aug 13, 2026
…ine, and record why the defects were invisible (#261) Both experiments re-run under #256 (pending setups no longer freeze the detector) and #258 (entries fill at the next bar's open, as production's market orders do). Same designs, same combinations, same fee grid and slippage pin -- only the engine differs. EVERY CONCLUSION SURVIVES EXCEPT ONE. ZEC-turtle no longer clears the maker line (1.034 -> 0.968), so #252 section 6's three-probe narrative describes a survivor the faithful engine never produces. The replacement is simpler and worse for the library: the viable quadrant is empty at every reachable fee -- 0 of 90 in #252, 0 of 82 in #255 -- with nothing needing three gates to die. #255 strengthens: the level shift across the trade floor widens from 1.1631 -> 0.8938 to 1.1251 -> 0.8396, gross-positive cells at the floor nearly halve (11/76 -> 6/82), and the 34x UNI-USD monotonicity anomaly that exposed both defects is structurally gone (3 assets -> 0). The two defects pushed in OPPOSITE directions -- #256 suppressed opportunity, #258 flattered execution -- so correcting both moved everything one way rather than adding noise: trade counts rose in 87 of 90, gross profit factors fell in 69 of 90. Arm B's transfer is restated on a single engine: 0.5770 in-sample vs 0.5427 out-of-sample, a gap of 0.034. #252's 0.6335 vs 0.6346 compared figures from one engine and was partly luck. The conclusion is unchanged -- the sweep winner is not overfit, it is stably unprofitable. Records the operational takeaway as section 5: NEITHER DEFECT WAS FOUND BY LOOKING FOR DEFECTS, and neither was findable by the means we had. 2,712 tests passed throughout. A frozen backtest and a highly selective strategy produce identical-looking output, so no summary ledger distinguishes them. The fix is invariants the engine reports about itself -- a dead-tail warning, intent-vs-fill divergence logging (#260), and cost anchored to output (#247, shipped, and the model for the rest) -- not more unit tests, which only assert behaviour someone already imagined. Annotates rather than rewrites, per the convention #247 set: the original numbers were real outputs of the code as it stood. Both documents keep their figures and carry a banner pointing here. Two ledger rows; chain verifies clean at 85. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closed
3 tasks
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
1 participant
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.
Closes #254. A correctness defect in the instrument every experiment in
docs/experiments/wasproduced with.
The defect
backtest()setpendingand, if the entry was never touched, carried it forward unchanged. Theonly path that cleared it without a fill was the stop being touched first. So a setup whose entry
and stop were both never revisited pinned
pendingfor the rest of the series: theposition is None and pending is Nonebranch never ran again,rule.detect()was never calledagain, and the engine silently switched its own detector off.
Nothing errored, nothing warned. The output was indistinguishable from a rule that simply found no
further setups — a frozen backtest looks exactly like a selective one, which is how it survived
every experiment to date.
Measured
rsi_meanrevon UNI-USD, hourly, everything at defaults exceptoversold:Loosening the entry threshold cut the trade count 34×. Also visible as non-monotonic
nonBTC-USD (181 → 80 between
oversold30 and 35) and AAVE-USD.Why re-detect rather than "expire after N bars"
Because N never existed in production.
strategy/engine.py::evaluatecallsrule.detect()once per cycle unconditionally and carries no pending-setup state between cycles — an unexecuted
setup is simply re-derived from fresh data next cycle. Expiry would have been a new tunable
invented for the simulator; re-detection makes the simulator match the live path it is supposed to
be modelling. This was checked against the live code rather than assumed.
No lookahead is introduced. The fill attempt still happens first, and the re-detect uses the
same
candles[: i + 1]window the flat branch would have used, so a setup derived on baricanstill only fill on bar
i+1or later.One-position-at-a-time is unaffected. That is enforced by the open position check, not the
pending one, and it is unchanged —
TestNoOverlappasses untouched. Two now-stale docstrings thatclaimed
detect()runs only whenpending is Nonehave been corrected.Verification
(
detect_calls1 vs 3) — reverted the fix, ran them, confirmed red, restored.usefully bounds the blast radius: this only moves series where a setup went unrevisited.
ruff check keel tests packagesclean.Blast radius on the research corpus
Non-trivial, and being re-measured now. Early rows from the #252 re-run against fixed code, Arm B
(
entry_lookback=336):Trade counts move by only ±1–4, but gross profit factors move materially on the low-n assets. The
full 90-combination re-run of #252 and the 96-cell re-run of #255 follow in a separate PR, since
restating those documents' numbers is a documentation change and this is an engine change.
🤖 Generated with Claude Code