From 162b985f9b75a0375db06fb07e9629891ef2f880 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Wed, 12 Aug 2026 16:00:42 -0400 Subject: [PATCH] fix(strategy): fill entries at the next bar's open, as production's market orders do 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) --- keel/strategy/backtest.py | 140 ++++++++++++----------- tests/baseline/test_backtest_baseline.py | 34 ++++-- tests/fixtures/baseline_backtest.json | 104 ++++++++--------- tests/strategy/test_backtest.py | 112 ++++++------------ 4 files changed, 189 insertions(+), 201 deletions(-) diff --git a/keel/strategy/backtest.py b/keel/strategy/backtest.py index f51755ab..d3ce1174 100644 --- a/keel/strategy/backtest.py +++ b/keel/strategy/backtest.py @@ -4,25 +4,36 @@ `BacktestResult`. Per spec §12 (and the supporting knowledge-base sources §2.3/§4.2/ §20.2/§20.5): -- **Intrabar resolution:** a rule's `Setup` (entry/stop/target) is a *pending* order; - when a single bar's range spans two of these levels at once (e.g. both entry and - stop, or both stop and target), the order they were touched in is ambiguous from - that bar alone. We resolve it using `finer_candles` covering that bar's time span, - falling back to the conservative outcome ("backtesting exists to lower - expectations, not raise them") when finer data isn't available or is still - 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). +- **Entries fill at the next bar's open (#257).** Production places MARKET orders — + `execution/executor.py::_order_row` writes `order_type="market"`, `limit_price=None`, and keeps + the setup's own price only as `expected_fill`. So a `Setup` detected on bar `i` is filled at + `candles[i+1].open` plus slippage, the first price obtainable once the signal exists. + **`Setup.entry` is informational here**, exactly as `expected_fill` is live; risk is measured + from the achieved fill against the setup's stop, never from the quoted entry. + + The previous model waited for a later bar's range to *touch* `entry` and filled at that level. + That gave the simulator free optionality on the entry price (unfavourable entries were silently + declined, since a setup only became a trade if the market offered the chosen level) and + unbounded patience — neither of which production has, and both of which flattered results. + + A consequence worth naming: a rule that encodes 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, and modelling it faithfully is the + point. Making the executor honour a stop/limit entry is the alternative, and belongs in the + executor rather than in this module (#257, "Option B"). +- **Intrabar resolution:** when a single bar's range spans both the stop and the target, the order + they were touched in is ambiguous from that bar alone. We resolve it using `finer_candles` + covering that bar's time span, falling back to the conservative outcome ("backtesting exists to + lower expectations, not raise them") when finer data isn't available or is still ambiguous at + that resolution: stop-vs-target ambiguity resolves to the **stop** (a loss). This applies on the + fill bar too — filling at the open means the rest of that bar can reach either level. - **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. + non-overlapping trades. It is the *open position* that enforces this. (Under the touch-fill + model an unfilled setup could pin `pending` forever and silently switch the detector off; #254 + fixed that by re-detecting each bar, and #257 makes the situation unreachable — every pending + now fills on the very next bar.) - **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). @@ -48,9 +59,17 @@ # The default rate `backtest` charges per leg, and the reason it is the TAKER rate. # -# This module fills market-style: a pending `Setup` fills the moment a later bar's range -# touches its entry, at that level plus slippage. That is a marketable order crossing the -# spread -- taker behaviour -- so the taker rate is the one that prices it. Until #247 this +# This module fills market-style: a `Setup` detected on bar i fills at bar i+1's OPEN plus +# slippage (#257), mirroring the market order `execution/executor.py` actually places. That is a +# marketable order crossing the spread -- taker behaviour -- so the taker rate prices it. +# +# #257 also removed the one wrinkle in this justification. Under the previous touch-fill model the +# claim held only for a rule whose entry sat ABOVE the market (a breakout, i.e. a stop order, +# genuinely marketable) and was wrong for one whose entry sat BELOW it (`rsi_meanrev` buying near +# support, which touching would make a resting MAKER fill). Now that every entry is a market +# order at the open, the taker rate is the right one for all of them, unconditionally. +# +# Until #247 this # default was the MAKER rate (0.006) while the fill model was unchanged, and the two halves of # the project disagreed **in writing**: `config.yaml`'s own `fees:` comment says "taker_pct is # the sim's default -- it fills market-style at next-bar open", and `keel_core.config @@ -232,61 +251,46 @@ def backtest( continue if position is None and pending is not None: - entry_touched = _touches(candle, pending.entry) - stop_touched = _touches(candle, pending.stop) - if not entry_touched: - # 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: - order = _resolve_order( - i, candles, finer_candles, {"entry": pending.entry, "stop": pending.stop} - ) - if order != "entry": - # Stop breached before (or indistinguishably from) entry: - # invalidate — no fill ever happened. - pending = None - continue + # PRODUCTION PLACES MARKET ORDERS (#257). `execution/executor.py::_order_row` writes + # `order_type="market"`, `limit_price=None`, and records the setup's own price only as + # `expected_fill`. 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 on that cycle. + # + # This branch runs on the bar AFTER detection, so `candle.open` is the first price + # obtainable once the signal exists — the faithful analogue of that market order, and + # the earliest fill that involves no lookahead. + # + # What this replaces, and why it had to go: the previous model held the setup until a + # later bar's range TOUCHED `entry`, then filled AT that level. That granted the + # simulator two things production does not have — 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) and unbounded patience. Both flatter results, and + # the bias runs opposite to #254's, which suppressed trades. + # + # `Setup.entry` is therefore now INFORMATIONAL in the backtest, exactly as + # `expected_fill` is live. Risk is measured from the achieved fill against the setup's + # stop (`_close_trade`), never from the quoted entry, so nothing downstream reads it. + # + # Consequence worth stating: a rule whose entry encodes a CONFIRMATION condition no + # longer gets it. `pullback_continuation` sets `entry = signal_candle.high + buffer` + # precisely to require follow-through, and under a market fill it takes trades it + # meant to decline. That is not introduced here — it is what the live box already + # does, and modelling it is the point. Fixing it belongs in the executor, not here + # (the Option B discussion on #257). position = _OpenPosition( setup=pending, - entry_fill=pending.entry * (Decimal(1) + slippage_pct), + entry_fill=candle.open * (Decimal(1) + slippage_pct), entry_ts=candle.ts, mfe=Decimal(0), mae=Decimal(0), ) pending = None - if ambiguous_fill: - # Finer data was already spent to prove entry hit before stop. - # Re-checking this same bar's raw (coarse) range for stop/target - # would spuriously re-trigger the very stop level finer data just - # showed was touched *before* entry. Defer stop/target resolution - # for the remainder of this bar to the next bar's coarse data. - position.mfe = max(position.mfe, candle.high - position.entry_fill) - position.mae = max(position.mae, position.entry_fill - candle.low) - continue - # Stop is known clear this bar (simple, unambiguous fill); fall - # through to the shared stop/target/exit-signal check below, which - # for this bar only needs to consider the target. + # Fall through to the shared stop/target block for the REMAINDER of this bar. Unlike + # the old unambiguous-fill path, the stop is NOT known clear here — we filled at the + # open, so this bar's range can reach either level, and the shared block's + # stop-vs-target `_resolve_order` is exactly the right adjudicator. + assert position is not None # noqa: S101 - narrows type for the checks below position.mfe = max(position.mfe, candle.high - position.entry_fill) diff --git a/tests/baseline/test_backtest_baseline.py b/tests/baseline/test_backtest_baseline.py index d32eaa83..2678e29b 100644 --- a/tests/baseline/test_backtest_baseline.py +++ b/tests/baseline/test_backtest_baseline.py @@ -4,20 +4,40 @@ uv run python tests/baseline/regenerate_golden.py -**Regenerated once, for #247** -- the fee correction (maker 0.006 -> taker 0.012, the rate that -matches this engine's market-style fill model). That is a deliberate strategy-output change, the -one case the regeneration script exists for, so the golden was rebuilt rather than pinned to the -superseded rate. What moved, on the committed BTC daily corpus: +**Regenerated twice**, both times for a deliberate strategy-output change -- the one case the +regeneration script exists for. + +**#247** -- the fee correction (maker 0.006 -> taker 0.012, the rate that matches this engine's +market-style fill model): profit_factor 1.6143 -> 1.2694 expectancy 1368.48 -> 692.08 max_drawdown 12177.59 -> 14220.67 n_trades 13 win_rate 0.4615 (both UNCHANGED) -`n_trades` and `win_rate` holding still is the check that this was a costing change and not an +`n_trades` and `win_rate` holding still was the check that this was a costing change and not an accidental change to fill logic: fees are charged on a filled trade, they never decide whether a -level was touched. The old numbers are not restated anywhere -- they were real outputs of the -code as it stood, and `docs/experiments/` keeps them as printed. +level was touched. + +**#257** -- entries now fill at the next bar's OPEN rather than seeking the setup's quoted entry +level, mirroring the market orders `execution/executor.py` actually places: + + profit_factor 1.269371 -> 1.269287 + expectancy 692.0773 -> 691.9480 + max_drawdown 14220.67 -> 14222.05 + n_trades 13 win_rate 0.4615 (both UNCHANGED) + +That #247 reasoning does NOT carry over here: this *is* a fill-logic change, so `n_trades` was +free to move and simply didn't. It held because `turtle_breakout` sets `entry = current.close`, +and on a 24/7 crypto series the next bar's open sits a hair from the prior close -- so on this +corpus the two models pick nearly the same price and decline nearly the same trades. The change +is materially larger for a rule whose entry is offset from the close (`pullback_continuation` +uses `signal_candle.high + buffer_ticks`), which this daily BTC baseline does not exercise. Read +the tiny deltas above as "this corpus is insensitive to the fill model", not as "the fill model +barely matters". + +Old numbers are not restated anywhere -- they were real outputs of the code as it stood, and +`docs/experiments/` keeps them as printed. """ from __future__ import annotations diff --git a/tests/fixtures/baseline_backtest.json b/tests/fixtures/baseline_backtest.json index ddd03689..feef3dd0 100644 --- a/tests/fixtures/baseline_backtest.json +++ b/tests/fixtures/baseline_backtest.json @@ -1,14 +1,14 @@ { "n_trades": 13, "win_rate": 0.46153846153846156, - "avg_win": "7066.176474054761090958266667", - "avg_loss": "-4771.43629189135292830000", - "expectancy": "692.0772923914689267422769231", - "profit_factor": "1.269370965371542157908549674", - "max_drawdown": "14220.67206938411411157960", + "avg_win": "7066.600039064761090958266667", + "avg_loss": "-4772.039456179924356871428571", + "expectancy": "691.9480031637766190499692308", + "profit_factor": "1.269286602382488692734350576", + "max_drawdown": "14222.04907754411411157960", "max_losing_streak": 3, - "avg_mfe": "8373.161808461538461538461538", - "avg_mae": "3154.080499230769230769230769", + "avg_mfe": "8373.034052307692307692307692", + "avg_mae": "3154.208255384615384615384615", "trades": [ { "entry_ts": 1630886400, @@ -26,92 +26,92 @@ { "entry_ts": 1633564800, "exit_ts": 1637193600, - "entry": "55367.149740", + "entry": "55374.623475", "exit": "56869.5510", "qty": "1", "side": "BUY", - "pnl": "155.560851120", - "r_multiple": "0.02858695567882248141793699127", - "mfe": "13632.850260", - "mae": "1988.149740", + "pnl": "147.997431300", + "r_multiple": "0.02715974657327545316323549808", + "mfe": "13625.376525", + "mae": "1995.623475", "outcome": "win" }, { "entry_ts": 1673654400, "exit_ts": 1675987200, - "entry": "19944.847440", + "entry": "19944.037035", "exit": "21618.155515", "qty": "1", "side": "BUY", - "pnl": "1174.552039540", - "r_multiple": "1.290097101259778564353488280", - "mfe": "4317.332560", - "mae": "51.597440", + "pnl": "1175.372169400", + "r_multiple": "1.292148086974900141034908216", + "mfe": "4318.142965", + "mae": "50.787035", "outcome": "win" }, { "entry_ts": 1676505600, "exit_ts": 1677196800, - "entry": "24341.204520", + "entry": "24339.113475", "exit": "22795.222774540627790", "qty": "1", "side": "BUY", - "pnl": "-2111.618872993859743480", - "r_multiple": "-1.376025381557044531310422740", - "mfe": "947.675480", - "mae": "1821.204520", + "pnl": "-2109.502735453859743480", + "r_multiple": "-1.376522086104324129707675681", + "mfe": "949.766525", + "mae": "1819.113475", "outcome": "loss" }, { "entry_ts": 1679097600, "exit_ts": 1685923200, - "entry": "27467.346810", + "entry": "27467.476875", "exit": "25728.62925", "qty": "1", "side": "BUY", - "pnl": "-2377.069272720", - "r_multiple": "-1.004370501762226745106656646", - "mfe": "3582.653190", - "mae": "2075.916810", + "pnl": "-2377.200898500", + "r_multiple": "-1.004370921023420863167429861", + "mfe": "3582.523125", + "mae": "2076.046875", "outcome": "loss" }, { "entry_ts": 1687392000, "exit_ts": 1690156800, - "entry": "30010.077540", + "entry": "30014.849925", "exit": "29162.391510", "qty": "1", "side": "BUY", - "pnl": "-1557.755658600", - "r_multiple": "-0.7713039485138699642697348065", - "mfe": "1852.132460", - "mae": "1160.077540", + "pnl": "-1562.585312220", + "r_multiple": "-0.7718713698114655561104454677", + "mfe": "1847.360075", + "mae": "1164.849925", "outcome": "loss" }, { "entry_ts": 1698105600, "exit_ts": 1701734400, - "entry": "33104.123790", + "entry": "33096.259860", "exit": "44943.53362983121245620", "qty": "1", "side": "BUY", - "pnl": "10902.83795079323790672560", - "r_multiple": "5.461568673537852537024361160", - "mfe": "11895.876210", - "mae": "252.673790", + "pnl": "10910.79624795323790672560", + "r_multiple": "5.487170764056078383764899962", + "mfe": "11903.740140", + "mae": "244.809860", "outcome": "win" }, { "entry_ts": 1707955200, "exit_ts": 1710115200, - "entry": "51884.519295", + "entry": "51884.439255", "exit": "72033.8676788414257480", "qty": "1", "side": "BUY", - "pnl": "18662.3277401553286390240", - "r_multiple": "5.497843570452889471520541203", - "mfe": "21059.460705", - "mae": "1371.509295", + "pnl": "18662.4087406353286390240", + "r_multiple": "5.497997072594832711056430988", + "mfe": "21059.540745", + "mae": "1371.429255", "outcome": "win" }, { @@ -130,14 +130,14 @@ { "entry_ts": 1731283200, "exit_ts": 1740355200, - "entry": "80469.134460", + "entry": "80467.903845", "exit": "91465.064590", "qty": "1", "side": "BUY", - "pnl": "8932.719741400", - "r_multiple": "1.785059642810650958730592204", - "mfe": "28888.875540", - "mae": "191.794460", + "pnl": "8933.965123780", + "r_multiple": "1.785747660760297998360903885", + "mfe": "28890.106155", + "mae": "190.563845", "outcome": "win" }, { @@ -156,14 +156,14 @@ { "entry_ts": 1768348800, "exit_ts": 1768867200, - "entry": "95431.922115", + "entry": "95433.282795", "exit": "89934.67586916798590230", "qty": "1", "side": "BUY", - "pnl": "-7721.64542164202992852760", - "r_multiple": "-1.416229325395986612241383396", - "mfe": "2531.697885", - "mae": "7664.392115", + "pnl": "-7723.02242980202992852760", + "r_multiple": "-1.416128469991380426250705656", + "mfe": "2530.337205", + "mae": "7665.752795", "outcome": "loss" }, { diff --git a/tests/strategy/test_backtest.py b/tests/strategy/test_backtest.py index 97107d4b..dca95f8b 100644 --- a/tests/strategy/test_backtest.py +++ b/tests/strategy/test_backtest.py @@ -134,7 +134,7 @@ def _candles(self) -> list[Candle]: return [ _candle(0, "100", "101", "99", "100"), # baseline; rule doesn't fire yet _candle(60, "104", "106", "103", "105"), # trigger bar: rule fires here - _candle(120, "105", "112", "104", "108"), # fill bar: touches entry(110) only + _candle(120, "105", "112", "104", "108"), # fill bar: fills at its open (105) _candle(180, "111", "135", "109", "132"), # exit bar: touches target(130) only ] @@ -156,7 +156,8 @@ def test_mfe_and_mae_recorded_correctly(self) -> None: result = backtest(self._rule(), self._candles()) trade = result.trades[0] - entry_fill = Decimal(110) * Decimal("1.0005") + # #257: filled at the FILL BAR's open (105), not at the setup's quoted entry (110). + entry_fill = Decimal(105) * Decimal("1.0005") expected_mfe = Decimal("135") - entry_fill # highest high (exit bar) minus entry fill expected_mae = entry_fill - Decimal("104") # lowest low (fill bar) minus entry fill assert trade.mfe == expected_mfe @@ -191,7 +192,8 @@ def test_fees_and_slippage_applied_on_entry_and_exit(self) -> None: ) trade = result.trades[0] - entry_fill = Decimal(110) * Decimal("1.0005") + # #257: entry is the fill bar's open plus slippage; the setup's 110 is informational. + entry_fill = Decimal(105) * Decimal("1.0005") exit_fill = Decimal(130) * (Decimal(1) - Decimal("0.0005")) entry_fee = entry_fill * Decimal("0.006") exit_fee = exit_fill * Decimal("0.006") @@ -204,54 +206,11 @@ def test_fees_and_slippage_applied_on_entry_and_exit(self) -> None: assert result.expectancy == expected_pnl -class TestIntrabarResolutionEntryVsStop: - """A fill-seeking bar's range spans both the entry and the stop level: ambiguous - without finer data. `finer_candles` must resolve the true order of events. - """ - - def _candles(self) -> list[Candle]: - return [ - _candle(0, "100", "101", "99", "100"), - _candle(60, "104", "106", "103", "105"), # trigger bar - # fill-seeking bar: spans BOTH entry(110) and stop(100) -> ambiguous - _candle(120, "105", "115", "95", "112"), - _candle(180, "112", "135", "111", "132"), - ] - - def _rule(self) -> _ScriptedRule: - return _ScriptedRule( - trigger_ts=60, entry=Decimal(110), stop=Decimal(100), target=Decimal(130) - ) - - def test_finer_candles_showing_stop_first_invalidates_the_trade(self) -> None: - finer = [ - # within [120, 180): price dips to touch stop(100) before ever reaching entry(110) - _candle(125, "105", "106", "99", "100"), - _candle(140, "100", "112", "99", "111"), - ] - result = backtest(self._rule(), self._candles(), finer_candles=finer) - - # invalidated: the setup never resulted in a real fill, so no trade is recorded - assert result.n_trades == 0 - assert result.trades == [] - assert result.expectancy == Decimal(0) - - def test_finer_candles_showing_entry_first_fills_and_continues(self) -> None: - finer = [ - # within [120, 180): price rises to touch entry(110) before ever touching stop(100) - _candle(125, "105", "111", "104", "110"), - _candle(140, "110", "116", "109", "112"), - ] - result = backtest(self._rule(), self._candles(), finer_candles=finer) - - assert result.n_trades == 1 - assert result.trades[0].outcome == "win" - - def test_ambiguous_bar_without_finer_candles_is_conservatively_invalidated(self) -> None: - result = backtest(self._rule(), self._candles(), finer_candles=None) - - assert result.n_trades == 0 - assert result.trades == [] +# NOTE: `TestIntrabarResolutionEntryVsStop` was deleted by #257. It exercised the resolution of +# a bar whose range spanned both the entry and the stop -- a question that only arises when an +# entry is a resting order seeking a level. Entries now fill at the next bar's open, so that +# ambiguity is unreachable and the code path it covered is gone. `_resolve_order` is still +# exercised for the stop-vs-target case, which remains real (see the shared exit block). class TestNoOverlap: @@ -331,37 +290,42 @@ 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.""" +class TestEntryFillsAtNextBarOpen: + """#257: production places market orders, so an entry fills at the next bar's open. + + This also subsumes #254. That defect was a setup whose entry level was never revisited + pinning `pending` forever, which switched the detector off for the rest of the series + (`rsi_meanrev` on UNI-USD at oversold=35: dead from November 2021, 9 trades against 309 at + the STRICTER oversold=30). Under a market fill every pending resolves on the very next bar, + so the state that caused it is now unreachable rather than merely handled. + """ 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) + _candle(0, "100", "101", "99", "100"), # rule fires here + _candle(60, "102", "103", "101", "102"), # FILL bar: opens 102, never nears 1000 + _candle(120, "102", "103", "101", "102"), ] - 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()) + def test_setup_fills_at_the_next_bars_open_not_its_quoted_entry(self) -> None: + """The entry level is never touched, and the trade happens anyway.""" + rule = _StaleThenReachableRule(switch_ts=10**9) # always the unreachable setup + result = backtest(rule, self._candles(), slippage_pct=Decimal("0.0005")) - # 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" + # Before #257 this was zero trades: entry(1000) was never touched, so nothing filled. + assert len(result.trades) == 1 + assert result.trades[0].entry == Decimal(102) * Decimal("1.0005") - def test_detect_is_called_on_every_flat_bar(self) -> None: - """The contract that replaced 'detect only while pending is None'. + def test_detector_cannot_be_frozen_by_an_unfilled_setup(self) -> None: + """The #254 regression, restated for the model that makes it impossible. - 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. + Bar 0 detects; bar 60 fills. Detection stops only because a position is open -- never + because a stale setup is pinned. """ - rule = _StaleThenReachableRule(switch_ts=120) + rule = _StaleThenReachableRule(switch_ts=10**9) 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 + # Exactly one: bar 0 detects, bar 60 fills, bar 120 holds. Under #256's re-detect + # model this was 3 (every flat bar re-asked, because nothing ever filled) -- so this + # number is also what distinguishes the two fill models. + assert rule.detect_calls == 1