From e86986d4303195457b85f2840c122eb80f6a4319 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Wed, 12 Aug 2026 14:08:15 -0400 Subject: [PATCH] fix(strategy): re-detect while flat, so an unfilled setup cannot switch the detector off MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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) --- keel/strategy/backtest.py | 37 +++++++++++--- tests/strategy/test_backtest.py | 86 +++++++++++++++++++++++++++++++-- 2 files changed, 114 insertions(+), 9 deletions(-) diff --git a/keel/strategy/backtest.py b/keel/strategy/backtest.py index b48f48ec..f51755ab 100644 --- a/keel/strategy/backtest.py +++ b/keel/strategy/backtest.py @@ -13,9 +13,16 @@ ambiguous at that resolution: entry-vs-stop ambiguity **invalidates** the trade entirely (no fill ever happened); stop-vs-target ambiguity in an open position resolves to the stop (a loss). -- **No overlap:** `detect()` is only called while flat (no pending or open - position) — one instrument, one position at a time. A rule whose condition would - fire on every bar still yields only sequential, non-overlapping trades. +- **No overlap:** `detect()` is only called while **flat** — one instrument, one position at a + time. A rule whose condition would fire on every bar still yields only sequential, + non-overlapping trades. What enforces that is the *open position* check, not the pending one: + while flat with an unfilled `Setup`, the rule is re-asked every bar and the stale setup is + replaced (#254). Carrying it instead meant a setup whose entry was never revisited pinned + `pending` for the rest of the series and `detect()` was never called again — the engine + switched its own detector off, indistinguishably from a rule that found no more setups. + Re-detecting is not a tunable ("expire after N bars"): it is what production does, since + `strategy/engine.py::evaluate` calls `detect()` once per cycle unconditionally and keeps no + pending-setup state between cycles. - **Costs:** `slippage_pct` worsens the fill price on both entry (paid) and exit (received); `fee_pct` is charged on both legs' notional. This models spread + slippage + fees (§4.2). @@ -228,9 +235,27 @@ def backtest( entry_touched = _touches(candle, pending.entry) stop_touched = _touches(candle, pending.stop) if not entry_touched: - # Not filled yet this bar (whether or not the stop alone was - # touched — the pending order never triggered, so the stop is - # irrelevant until entry is actually reached). + # Not filled this bar (whether or not the stop alone was touched — the pending + # order never triggered, so the stop is irrelevant until entry is reached). + # + # RE-DETECT rather than carry the stale setup forward (#254). Keeping it meant a + # setup whose entry was never revisited pinned `pending` for the rest of the + # series, so the `pending is None` branch above never ran again and `detect()` + # was never called again — the simulator switched its own detector off, silently, + # and the output was indistinguishable from a rule that simply found no more + # setups. Measured: `rsi_meanrev` on UNI-USD at oversold=35 stopped detecting in + # November 2021 and sat dead for ~40,000 bars, reporting 9 trades against 309 at + # the STRICTER oversold=30. + # + # Re-detecting is not a heuristic choice like "expire after N bars" — it is what + # production does. `strategy/engine.py::evaluate` calls `rule.detect()` once per + # cycle unconditionally and carries no pending-setup state between cycles, so an + # unexecuted setup is simply re-derived from fresh data. N never existed live. + # + # `candles[: i + 1]` is the same window the `pending is None` branch would use on + # this bar, and the fill attempt above already happened, so this introduces no + # lookahead: a setup derived on bar i can still only fill on bar i+1 or later. + pending = rule.detect(candles_by_tf) continue ambiguous_fill = stop_touched if ambiguous_fill: diff --git a/tests/strategy/test_backtest.py b/tests/strategy/test_backtest.py index 3328e8ab..97107d4b 100644 --- a/tests/strategy/test_backtest.py +++ b/tests/strategy/test_backtest.py @@ -76,9 +76,10 @@ def describe(self) -> dict: class _AlwaysOnRule(Rule): """Would fire a signal on EVERY bar it's asked, if the backtester let it. - Used to prove overlapping signals are not double-counted: the backtester must - only call `detect()` while flat (no pending or open position), so a persistent - "always true" condition must still yield only sequential, non-overlapping trades. + Used to prove overlapping signals are not double-counted: the backtester must only call + `detect()` while flat, so a persistent "always true" condition must still yield only + sequential, non-overlapping trades. Note it is the OPEN POSITION that blocks re-detection -- + an unfilled pending setup does not, and is replaced each bar (#254). """ name = "always_on" @@ -285,3 +286,82 @@ def test_open_position_blocks_new_detection_until_closed(self) -> None: assert len(result.trades) == 1 assert result.trades[0].outcome == "open" assert result.trades[0].exit is None + + +class _StaleThenReachableRule(Rule): + """Emits an UNREACHABLE setup first, then a fillable one from `switch_ts` onward. + + The regression fixture for #254. The first setup's entry is never touched and neither is its + stop, which is precisely the state that used to pin `pending` forever: the backtester's + `pending is None` branch never ran again, `detect()` was never called again, and the second, + fillable setup was never seen. Measured in the wild as `rsi_meanrev` on UNI-USD at + oversold=35 -- 9 trades and a detector dead from November 2021, against 309 trades at the + STRICTER oversold=30. + """ + + name = "stale_then_reachable" + params: dict = {} + + def __init__(self, switch_ts: int) -> None: + self.switch_ts = switch_ts + self.detect_calls = 0 + + def detect(self, candles_by_tf: dict[Granularity, list[Candle]]) -> Setup | None: + self.detect_calls += 1 + window = next(iter(candles_by_tf.values())) + latest = window[-1] + # Far above every price in the series, so neither entry nor stop is ever touched. + entry, stop, target = (Decimal(1000), Decimal(900), Decimal(1100)) + if latest.ts >= self.switch_ts: + entry, stop, target = (Decimal(110), Decimal(95), Decimal(130)) + return Setup( + product_id="BTC-USD", + direction="long", + entry=entry, + stop=stop, + target=target, + context={}, + ts=latest.ts, + ) + + def exit_signal(self, held: Setup, candles_by_tf: dict[Granularity, list[Candle]]) -> bool: + return False + + def describe(self) -> dict: + return {"name": self.name, "params": self.params} + + +class TestPendingSetupDoesNotFreezeDetection: + """#254: an unfilled setup must not switch the detector off for the rest of the series.""" + + def _candles(self) -> list[Candle]: + return [ + _candle(0, "100", "101", "99", "100"), # unreachable setup emitted here + _candle(60, "100", "102", "99", "101"), # entry(1000) untouched, stop(900) untouched + _candle(120, "101", "103", "100", "102"), # switch_ts: a fillable setup is now offered + _candle(180, "103", "112", "102", "111"), # touches entry(110) + _candle(240, "111", "132", "110", "131"), # touches target(130) + ] + + def test_unfilled_setup_is_replaced_so_a_later_setup_can_still_fill(self) -> None: + rule = _StaleThenReachableRule(switch_ts=120) + result = backtest(rule, self._candles()) + + # Before the fix this was 0: the ts=0 setup pinned `pending`, so the fillable setup + # offered from ts=120 was never requested, let alone filled. + assert result.n_trades == 1 + assert result.trades[0].outcome == "win" + + def test_detect_is_called_on_every_flat_bar(self) -> None: + """The contract that replaced 'detect only while pending is None'. + + A position still blocks detection (see `TestNoOverlap`); an unfilled *pending* no longer + does, which is what `strategy/engine.py::evaluate` does live -- it calls `detect()` once + per cycle unconditionally and carries no pending state between cycles. + """ + rule = _StaleThenReachableRule(switch_ts=120) + backtest(rule, self._candles()) + + # Bars 0,60,120 are flat and each must ask the rule. Bar 180 fills and bar 240 exits, + # so detection legitimately stops there. + assert rule.detect_calls == 3