Merge pull request #51 from KhizarImran/fix/8-gap-aware-sl-tp

Fix gap-aware stop-loss and take-profit fills.

Codex 🤖, On behalf of Khizar

CHANGELOG.md

@@ -4,11 +4,15 @@
 
 ### Added
 - Documented and regression-tested the stop-first intrabar SL/TP policy,
-  inclusive touches, next-bar checks for new entries, and current gap-fill limits.
+  inclusive touches and next-bar checks for new entries.
 - Contributor setup and validation guidance, bug/feature issue templates, an
   API/platform compatibility policy, and a prioritized FX roadmap.
 
 ### Fixed
+- SL/TP processing checks the midpoint open first: crossed stops fill at the
+  adverse open, while crossed targets receive the favorable open, before any
+  later range touches. Exit spread, commission, and conversion still apply.
+  Bars that open inside both levels retain stop-first intrabar behavior.
 - Batch stop-loss/take-profit processing uses a single linear pass, preserving
   survivor and trade order, fills, and accounting when many positions exit together.
 - Native engine runs reset cash, positions, trade history, and position IDs before
@@ -20,6 +24,10 @@
   the exit-side spread (future exit commission is still charged only at closing).
 
 ### Migration
+- Gap-bar SL/TP fills now use the opening price instead of the configured level.
+  An opening target also takes precedence over a later stop touch. Rerun affected
+  backtests: PnL, equity, and strategy decisions can change; there is no legacy
+  gap-fill option. See README execution policy for long/short examples.
 - Double the previous `spread` input to preserve pre-change fills and completed
   trade PnL. Equity curves can still differ because marking now includes exit
   spread. SL/TP levels remain midpoint levels. See README price conventions.

README.md

@@ -222,15 +222,27 @@ Supply midpoint data: convert bid-only or ask-only data before running a backtes
 
 ### Intrabar stop-loss and take-profit policy
 
-OHLC bars do not reveal the order in which prices were visited. If a bar touches
-both a position's stop-loss and take-profit, the broker deterministically chooses
-the **stop-loss first**, for both long and short positions. Exact touches count
-as hits. This conservative assumption can differ from the outcome on tick data;
-it does not reconstruct the intrabar price path.
+The broker checks the midpoint **open first** for existing positions:
+
+- A long stop triggers when open <= SL; a short stop triggers when open >= SL.
+  It fills at the open, accepting the worse price if the market gaps past the stop.
+- A long target triggers when open >= TP; a short target triggers when open <= TP.
+  It fills at the open, giving price improvement if the market gaps past the target.
+- An opening fill takes precedence over any later high/low touch of the other
+  level. Exact touches at the open count as hits.
+
+If neither level triggers at the open, stops and targets fill at their configured
+midpoint levels when touched by the bar's range. OHLC bars do not reveal the order
+of those later touches. If both are touched, the broker chooses the
+**stop-loss first**, for both long and short positions. Exact touches count as
+hits. This conservative assumption can differ from the outcome on tick data;
+it does not reconstruct the intrabar price path. All fills apply exit-side spread
+(bid for longs, ask for shorts) and exit commission using the usual cost model.
 
 For example, a long entered at midpoint `1.1000`, with SL `1.0900` and TP
-`1.1100`, exits at the stop midpoint when the next bar has low `1.0800` and
-high `1.1200`. Bid/ask spread and exit commission still apply to the fill.
+`1.1100`, exits at the stop midpoint when the next bar opens at `1.1000` and
+has low `1.0800` and high `1.1200`. Bid/ask spread and exit commission still
+apply to the fill.
 
 The engine checks existing positions' SL/TP **before** calling the strategy for
 each bar. Orders opened by `next()` fill at that bar's close; their SL/TP levels
@@ -238,11 +250,17 @@ are first checked on the following bar. The entry bar's earlier high/low is not
 replayed against the new position. A position opened on the final bar instead
 undergoes the usual final liquidation at that bar's close.
 
-**Current gap limitation:** SL/TP fills use the configured midpoint level, even
-if the bar opens beyond it. If both levels are touched, stop-first still applies
-regardless of the open. Such fills may be unattainable after a gap; gap-aware
-execution is tracked in [#8](https://github.com/KhizarImran/backtestingfx/issues/8).
-This policy is fixed, with no configurable intrabar ordering option.
+For a gap example, a long with SL `1.0950` exits at midpoint `1.0900` if the
+next bar opens there. A long with TP `1.1100` exits at midpoint `1.1200` if the
+next bar opens there, even if that bar later falls through the stop. A short
+uses the opposite inequalities. This policy assumes execution at the opening
+quote without additional slippage or liquidity constraints; it has no
+configurable intrabar ordering option.
+
+**Migration:** previous versions always used the configured SL/TP level and
+chose the stop whenever the range touched both levels, regardless of the open.
+Gap-bar fills, PnL, equity, and later strategy decisions can therefore change.
+Rerun affected backtests; there is no setting to restore the old gap behavior.
 
 ### Trading non-USD pairs
 

ROADMAP.md

@@ -13,7 +13,8 @@ priorities or delivered capabilities change.
 
 ## 1. Correctness foundation (P0)
 
-- Gap-aware stop/target fills: [#8](https://github.com/KhizarImran/backtestingfx/issues/8).
+- Implemented (unreleased): gap-aware stop/target fills
+  ([#8](https://github.com/KhizarImran/backtestingfx/issues/8)); see README execution policy.
 - Data and timestamp validation: [#15](https://github.com/KhizarImran/backtestingfx/issues/15).
 - Broker, size, and signal validation: [#16](https://github.com/KhizarImran/backtestingfx/issues/16).
 - Independent native engine runs: [#18](https://github.com/KhizarImran/backtestingfx/issues/18).

src/broker.rs

@@ -59,7 +59,8 @@ impl Broker {
         });
     }
 
-    /// Midpoint OHLC triggers are inclusive; if both levels are touched, stop wins.
+    /// Inclusive midpoint triggers: process the open first, filling crossed stops
+    /// and targets there. Otherwise use the level, with stop-first range ordering.
     /// The engine calls this before the strategy, so new entries wait until the next bar.
     pub fn check_sl_tp(&mut self, bar: &Bar) {
         // Move the Vec out so settlement can borrow self while retain compacts
@@ -67,8 +68,19 @@ impl Broker {
         // without shifting the entire suffix after every triggered position.
         let mut positions = std::mem::take(&mut self.positions);
         positions.retain(|p| {
+            // The open is observed before the unknown intrabar path. A stop gap
+            // takes the adverse opening price; a target gap gets price improvement,
+            // even if the range later reaches the other level. settle applies spread.
+            let stop_at_open = p.stop_loss.is_some_and(|sl| {
+                if p.is_long { bar.open <= sl } else { bar.open >= sl }
+            });
+            let target_at_open = p.take_profit.is_some_and(|tp| {
+                if p.is_long { bar.open >= tp } else { bar.open <= tp }
+            });
             let fill = {
-                if p.is_long {
+                if stop_at_open || target_at_open {
+                    Some(bar.open)
+                } else if p.is_long {
                     let sl_hit = p.stop_loss.is_some_and(|sl| bar.low <= sl);
                     let tp_hit = p.take_profit.is_some_and(|tp| bar.high >= tp);
                     if sl_hit {
@@ -665,19 +677,104 @@ mod tests {
     }
 
     #[test]
-    fn gap_beyond_stop_currently_fills_at_level_not_open() {
-        // This records the current limitation; gap-aware fills are tracked in #8.
+    fn stops_at_or_beyond_open_fill_at_open_even_if_price_recovers() {
         for is_long in [true, false] {
-            let mut broker = test_broker(0.0, 0.0);
-            let (stop, open) = if is_long { (1.09, 1.08) } else { (1.11, 1.12) };
-            if is_long {
-                broker.buy(1.1, 1.0, 0, Some(stop), None);
-            } else {
-                broker.sell(1.1, 1.0, 0, Some(stop), None);
+            for exact_touch in [true, false] {
+                for recovers in [true, false] {
+                    for spread in [0.0, 0.0001] {
+                        let mut broker = Broker::new(10_000.0, 7.0, spread, 100_000.0, 1.27);
+                        let (stop, target, gap) = if is_long {
+                            (1.09, 1.11, 1.08)
+                        } else {
+                            (1.11, 1.09, 1.12)
+                        };
+                        let open = if exact_touch { stop } else { gap };
+                        if is_long {
+                            broker.buy(1.1, 1.0, 0, Some(stop), Some(target));
+                        } else {
+                            broker.sell(1.1, 1.0, 0, Some(stop), Some(target));
+                        }
+                        // Recovery can touch the target later, after the opening stop fill.
+                        let bar = if recovers {
+                            Bar::new(1, open, 1.12, 1.08, 1.1, 0.0)
+                        } else {
+                            Bar::new(1, open, open, open, open, 0.0)
+                        };
+                        broker.check_sl_tp(&bar);
+                        assert!(broker.positions.is_empty());
+                        assert_eq!(broker.trade_history.len(), 1);
+                        let trade = &broker.trade_history[0];
+                        let exit = open + if is_long { -spread / 2.0 } else { spread / 2.0 };
+                        let loss = if exact_touch { 1000.0 } else { 2000.0 };
+                        assert_close(trade.exit_price, exit);
+                        assert_close(trade.pnl, (-loss - spread * 100_000.0) * 1.27 - 14.0);
+                        assert_close(broker.cash - 10_000.0, trade.pnl);
+                        assert_eq!(trade.exit_timestamp, 1);
+                    }
+                }
+            }
+        }
+    }
+
+    #[test]
+    fn targets_at_or_beyond_open_fill_before_a_later_stop() {
+        for is_long in [true, false] {
+            for exact_touch in [true, false] {
+                for reverses in [true, false] {
+                    for spread in [0.0, 0.0001] {
+                        let mut broker = Broker::new(10_000.0, 7.0, spread, 100_000.0, 1.27);
+                        let (stop, target, gap) = if is_long {
+                            (1.09, 1.11, 1.12)
+                        } else {
+                            (1.11, 1.09, 1.08)
+                        };
+                        let open = if exact_touch { target } else { gap };
+                        if is_long {
+                            broker.buy(1.1, 1.0, 0, Some(stop), Some(target));
+                        } else {
+                            broker.sell(1.1, 1.0, 0, Some(stop), Some(target));
+                        }
+                        let bar = if reverses {
+                            Bar::new(1, open, 1.12, 1.08, 1.1, 0.0)
+                        } else {
+                            Bar::new(1, open, open, open, open, 0.0)
+                        };
+                        broker.check_sl_tp(&bar);
+                        assert!(broker.positions.is_empty());
+                        assert_eq!(broker.trade_history.len(), 1);
+                        let trade = &broker.trade_history[0];
+                        let exit = open + if is_long { -spread / 2.0 } else { spread / 2.0 };
+                        let profit = if exact_touch { 1000.0 } else { 2000.0 };
+                        assert_close(trade.exit_price, exit);
+                        assert_close(trade.pnl, (profit - spread * 100_000.0) * 1.27 - 14.0);
+                        assert_close(broker.cash - 10_000.0, trade.pnl);
+                        assert_eq!(trade.exit_timestamp, 1);
+                    }
+                }
+            }
+        }
+    }
+
+    #[test]
+    fn opening_bid_or_ask_crossing_a_level_does_not_trigger_a_midpoint_exit() {
+        for is_long in [true, false] {
+            for use_stop in [true, false] {
+                let mut broker = test_broker(7.0, 0.0001);
+                let level = if is_long == use_stop { 1.09 } else { 1.11 };
+                // A bid/ask crosses the level while the midpoint remains inside it.
+                let open = level + if is_long == use_stop { 0.00001 } else { -0.00001 };
+                let sl = use_stop.then_some(level);
+                let tp = (!use_stop).then_some(level);
+                if is_long {
+                    broker.buy(1.1, 1.0, 0, sl, tp);
+                } else {
+                    broker.sell(1.1, 1.0, 0, sl, tp);
+                }
+                broker.check_sl_tp(&Bar::new(1, open, open, open, open, 0.0));
+                assert_eq!(broker.positions.len(), 1);
+                assert!(broker.trade_history.is_empty());
+                assert_close(broker.cash, 9993.0);
             }
-            broker.check_sl_tp(&Bar::new(1, open, open, open, open, 0.0));
-            assert!(broker.positions.is_empty());
-            assert_close(broker.trade_history[0].exit_price, stop);
         }
     }
 }

src/engine.rs

@@ -158,6 +158,45 @@ mod tests {
         }
     }
 
+    #[test]
+    fn native_gap_fills_settle_before_strategy_and_reconcile_final_equity() {
+        struct EnterOnce(bool);
+        impl Strategy for EnterOnce {
+            fn next(&mut self, bar: &Bar, broker: &mut Broker) {
+                assert!(broker.positions.is_empty());
+                if bar.timestamp == 0 {
+                    if self.0 {
+                        broker.buy(bar.close, 0.4, 0, Some(1.09), Some(1.11));
+                    } else {
+                        broker.sell(bar.close, 0.4, 0, Some(1.11), Some(1.09));
+                    }
+                } else {
+                    assert_eq!(broker.trade_history.len(), 1);
+                    assert_eq!(broker.trade_history[0].exit_timestamp, bar.timestamp);
+                }
+            }
+        }
+        for is_long in [true, false] {
+            for open in [1.08, 1.12] {
+                let data = vec![
+                    Bar::new(0, 1.1, 1.1, 1.1, 1.1, 0.0),
+                    Bar::new(1, open, 1.12, 1.08, 1.1, 0.0),
+                ];
+                let mut engine = Engine::new(data, 10_000.0, 7.0, 0.0001, 100_000.0, 1.27);
+                let stats = engine.run(&mut EnterOnce(is_long));
+                let favorable = is_long == (open > 1.1);
+                let pnl = (if favorable { 1990.0 } else { -2010.0 }) * 0.4 * 1.27 - 5.6;
+                assert_eq!(stats.num_trades, 1);
+                let expected_exit = open + if is_long { -0.00005 } else { 0.00005 };
+                assert!((stats.trades[0].exit_price - expected_exit).abs() < 1e-8);
+                assert!((stats.trades[0].pnl - pnl).abs() < 1e-8);
+                assert!((stats.final_cash - 10_000.0 - pnl).abs() < 1e-8);
+                assert_eq!(stats.equity_curve.len(), 3);
+                assert_eq!(stats.equity_curve.last(), Some(&stats.final_cash));
+            }
+        }
+    }
+
     #[test]
     fn equity_curve_includes_initial_cash_and_final_liquidation() {
         let data = vec![Bar::new(0, 1.1000, 1.1000, 1.1000, 1.1000, 0.0)];

tests/test_intrabar_execution.py

@@ -6,6 +6,67 @@ from backtestingfx import Backtest, Strategy
 
 
 class IntrabarExecutionTest(unittest.TestCase):
+    def test_gap_fills_and_intrabar_precedence_through_python_strategy(self):
+        # Offsets describe a long; reflect prices for a short. Include gaps whose
+        # ranges never reach the old level, and gaps that reverse through both levels.
+        cases = [
+            ("stop gap", -0.02, -0.02, -0.02, -0.02, -0.02),
+            ("stop gap recovers", -0.02, -0.02, 0.02, 0.0, -0.02),
+            ("target gap", 0.02, 0.02, 0.02, 0.02, 0.02),
+            ("target gap reverses", 0.02, -0.02, 0.02, 0.0, 0.02),
+            ("stop at open", -0.01, -0.02, 0.02, 0.0, -0.01),
+            ("target at open", 0.01, -0.02, 0.02, 0.0, 0.01),
+            ("intrabar stop", 0.0, -0.015, 0.005, 0.002, -0.01),
+            ("intrabar target", 0.0, -0.005, 0.015, 0.002, 0.01),
+            ("ambiguous range", 0.0, -0.02, 0.02, 0.0, -0.01),
+            ("no trigger", 0.0, -0.005, 0.005, 0.002, 0.002),
+        ]
+        index = pd.date_range("2026-01-01", periods=2, freq="h", tz="UTC")
+        for is_long in (True, False):
+            direction = 1 if is_long else -1
+            for name, opening, low, high, close, expected_exit in cases:
+                with self.subTest(is_long=is_long, case=name):
+                    seen = []
+                    stop = 1.1 - direction * 0.01
+                    target = 1.1 + direction * 0.01
+
+                    class EnterOnce(Strategy):
+                        def next(self):
+                            seen.append(len(self.positions))
+                            if self.index == 0:
+                                order = self.buy if is_long else self.sell
+                                order(0.4, stop_loss=stop, take_profit=target)
+
+                    extremes = [1.1 + direction * low, 1.1 + direction * high]
+                    data = pd.DataFrame(
+                        {
+                            "open": [1.1, 1.1 + direction * opening],
+                            "high": [1.1, max(extremes)],
+                            "low": [1.1, min(extremes)],
+                            "close": [1.1, 1.1 + direction * close],
+                        },
+                        index=index,
+                    )
+                    stats = Backtest(
+                        data, EnterOnce, commission=7.0, spread=0.0001,
+                        quote_to_account=1.27,
+                    ).run()
+                    self.assertEqual(stats.num_trades, 1)
+                    trade = stats.trades[0]
+                    self.assertEqual(trade.is_long, is_long)
+                    self.assertAlmostEqual(trade.lot_size, 0.4)
+                    self.assertAlmostEqual(trade.entry_price, 1.1 + direction * 0.00005)
+                    self.assertAlmostEqual(
+                        trade.exit_price, 1.1 + direction * (expected_exit - 0.00005)
+                    )
+                    pnl = (expected_exit - 0.0001) * 100_000 * 0.4 * 1.27 - 5.6
+                    self.assertAlmostEqual(trade.pnl, pnl)
+                    self.assertAlmostEqual(stats.final_cash, 10_000 + pnl)
+                    self.assertAlmostEqual(stats.equity_curve[-1], stats.final_cash)
+                    self.assertEqual(trade.entry_timestamp, int(index[0].timestamp()))
+                    self.assertEqual(trade.exit_timestamp, int(index[1].timestamp()))
+                    self.assertEqual(seen, [0, 1 if name == "no trigger" else 0])
+
     def test_new_entries_are_checked_from_the_following_bar(self):
         for is_long in (True, False):
             for count in (1, 2):