fix(engine): make dead strategy.close semantically inert

docs/cheatsheet-runtime-and-execution.md

@@ -61,7 +61,8 @@ Sentinels for "not overridden": `NaN` (doubles), `-1` (ints).
 | `close_entries_rule` | FIFO | `ANY`/`any`/`1` = ANY, else FIFO |
 
 **Compile-time only (no override path):** `margin_long` / `margin_short`
-(default 100 = 1x), `script_has_strategy_close_` (gates the deferred-flip rule).
+(default 100 = 1x). `script_has_strategy_close_` is retained only as a legacy
+generated-code compatibility bit and does not affect runtime behavior.
 
 ## 1.3 `strategy.risk.*` — compile-time only
 
@@ -210,8 +211,9 @@ mintick directionally (buys ceil, sells floor).
 
 **Deferred-flip carry (`tv_carry_qty`, `|old|+qty`):** a **priced** entry firing
 **from flat** opposite to its `created_position_side` opens at `carry + base_qty`
-**iff** all hold: `script_has_strategy_close_`, priced entry, `carry > 0`,
-opposite direction. Carry snapshot taken **at `strategy.entry` call time** =
+**iff** all hold: priced entry, `carry > 0`, opposite direction. This also
+applies when a `strategy.exit` bracket closed the source position. Carry
+snapshot taken **at `strategy.entry` call time** =
 `max(0, position_qty_ − pending_close_qty_in_bar_)`. Same-id replacement
 re-snaps carry; siblings in the same cycle get carry zeroed on fire.
 

include/pineforge/engine.hpp

@@ -902,11 +902,12 @@ class BacktestEngine {
     // callers that want the legacy hold-to-infinity behaviour.
     bool margin_call_enabled_ = true;
 
-    // True iff the user's strategy.pine contains at least one
-    // ``strategy.close`` or ``strategy.close_all`` call (compile-time
-    // determined by the codegen, set in the generated class's
-    // constructor). Gates the TradingView deferred-flip growth rule
-    // applied in ``execute_market_entry``'s FLAT branch.
+    // Legacy codegen compatibility bit. Older and current generated classes
+    // set this when the Pine source contains ``strategy.close`` or
+    // ``strategy.close_all``. Runtime behavior must not depend on it: a
+    // bracket exit can also close a carry source, and unreachable source code
+    // cannot be allowed to change fills. Keep the member until generated
+    // classes no longer write it.
     bool script_has_strategy_close_ = false;
     int64_t trade_start_time_ = std::numeric_limits<int64_t>::min();
 

src/engine_orders.cpp

@@ -745,17 +745,17 @@ void BacktestEngine::consume_tv_carry_from_siblings(const std::string& id,
 // transient state.
 //
 // Conditions:
-//   (a) script_has_strategy_close_   -- compile-time AST scan (gates the
-//                                       rule to strategies that can
-//                                       actually trigger the deferred-flip
-//                                       pattern)
-//   (b) is_priced_entry              -- stop/limit, not market
-//   (c) tv_carry_qty > 0             -- order was placed while a position
+//   (a) is_priced_entry              -- stop/limit, not market
+//   (b) tv_carry_qty > 0             -- order was placed while a position
 //                                       was open
-//   (d) requested != created direction
+//   (c) requested != created direction
 //                                    -- new entry is opposite to the carry
 //                                       position
 //
+// Do not gate this on the compile-time ``script_has_strategy_close_`` AST
+// scan. A bracket from ``strategy.exit`` can close the source position too,
+// and adding an unreachable ``strategy.close`` must be semantically inert.
+//
 // After applying the carry, ``consume_tv_carry_from_siblings`` zeroes the
 // same-source-cycle siblings so probe 93 doesn't double-grow.
 //
@@ -776,10 +776,10 @@ void BacktestEngine::enter_market_from_flat(const std::string& id, bool is_long,
                                             int created_bar,
                                             bool explicit_qty_prequantized,
                                             uint64_t entry_incarnation) {
-    bool carry_was_long = (created_position_side == PositionSide::LONG);
-    bool tv_deferred_flip =
-        script_has_strategy_close_
-        && is_priced_entry
+    const bool carry_was_long =
+        created_position_side == PositionSide::LONG;
+    const bool tv_deferred_flip =
+        is_priced_entry
         && tv_carry_qty > 0.0
         && (carry_was_long ? !is_long : is_long);
     double base_qty = explicit_qty_prequantized

tests/test_close_all_coqueued_entry.cpp

@@ -409,22 +409,28 @@ static void test_G_carry_priorbar_still_cancelled() {
 // G-opposite (KI-64 untouched — characterization). An opposite-direction entry
 // co-queued with close_all is never a target of the same-direction wipe
 // (is_long != exit_closed_was_long); its behavior is IDENTICAL before and after
-// this fix. Pins the current engine behavior: the opposite short stop survives
-// and fills when touched.
+// this fix. Pins the generated-script behavior: the opposite short stop
+// survives and fills with its placement-time reversal transaction. Because
+// this source contains close_all, legacy codegen set
+// script_has_strategy_close_=true even before issue #141 removed that AST bit
+// from the runtime predicate.
 //
 //   bar0: entry("L0", mkt)
 //   bar1: L0 fills @100 → LONG 1. arm OPPOSITE short stop "SOPP"@90 + close_all()
 //   bar2: close_all fills @100 → FLAT (1 trade). SOPP opposite dir → untouched;
 //         low 99 > 90 stays pending.
-//   bar3: low 88 ≤ 90 → SOPP fires → SHORT 1.
+//   bar3: low 88 ≤ 90 → SOPP fires from flat with carry 1 + own 1 → SHORT 2.
 //
-// EXPECTED (pre- and post-fix, characterization): position ends SHORT 1.
+// EXPECTED (pre- and post-fix, generated-source characterization): SHORT 2.
 // ─────────────────────────────────────────────────────────────────────
 static void test_G_opposite_unchanged_ki64() {
     std::printf("G-opposite (KI-64 opposite-direction unchanged)\n");
     class Probe : public ProbeBase {
     public:
-        Probe() : ProbeBase(2) {}
+        Probe() : ProbeBase(2) {
+            // Match what codegen emits for the reachable close_all below.
+            script_has_strategy_close_ = true;
+        }
         void on_bar(const Bar&) override {
             if (bar_index_ == 0) strategy_entry("L0", true);
             if (bar_index_ == 1) {
@@ -443,7 +449,7 @@ static void test_G_opposite_unchanged_ki64() {
     };
     p.run(bars, 5);
     CHECK(p.trade_count() == 1);
-    CHECK(near(p.pos_size(), -1.0));          // opposite entry survives (unchanged pre/post)
+    CHECK(near(p.pos_size(), -2.0));          // carry + own qty (unchanged pre/post)
 }
 
 // ─────────────────────────────────────────────────────────────────────

tests/test_same_id_stop_replace.cpp

@@ -81,11 +81,6 @@ class StopReplaceProbe : public BacktestEngine {
         slippage_ = 0;
         commission_value_ = 0;
         pyramiding_ = 1;
-        // The deferred-flip carry rule is gated on the script having a
-        // ``strategy.close`` call — set it true so the carry-leak
-        // regression (default position_qty_ before any fill) would
-        // surface as qty=2 rather than being silently masked.
-        script_has_strategy_close_ = true;
     }
 
     void snapshot() {
@@ -307,9 +302,9 @@ static void test_new_entry_after_same_bar_fill_defers_to_next_bar() {
 // strategy.entry placed BEFORE the first fill of any session must
 // capture tv_carry_qty=0, not the engine's default
 // ``position_qty_=1.0`` value. Pre-fix, the LE order's tv_carry_qty
-// was 1, which combined with ``script_has_strategy_close_=true`` and
-// the order firing from FLAT in the LONG direction produced
-// tv_deferred_flip = (true && true && carry=1>0 && (false?:true=true))
+// was 1, which combined with the priced order firing from FLAT in the
+// LONG direction produced
+// tv_deferred_flip = (priced && carry=1>0 && (false?:true=true))
 // → qty = 1 + 1 = 2 instead of 1. Probe 62's first in-window trade
 // fired qty=2 with double the expected PnL (-18.22 vs TV's -9.11),
 // breaking parity even before the warmup-gate buffer fix exposed the

tests/test_strategy_pyramiding.cpp

@@ -76,10 +76,6 @@ class DeferredFlipProbe : public BacktestEngine {
         slippage_ = 0;
         commission_value_ = 0;
         pyramiding_ = 1;
-        // The deferred-flip rule is gated on the script having a
-        // ``strategy.close`` call at compile time. The codegen would
-        // set this flag from an AST scan; here we set it directly.
-        script_has_strategy_close_ = true;
     }
 
     void on_bar(const Bar& bar) override {
@@ -126,6 +122,50 @@ class DeferredFlipProbe : public BacktestEngine {
     int last_bar_index_seen = -1;
 };
 
+// Metamorphic probe for issue #141. Both instances execute exactly the same
+// broker commands: a bracket closes the source long, then a pre-armed priced
+// short entry fires from flat. The only difference is the legacy codegen AST
+// bit that says whether the Pine source contains any strategy.close call.
+// An unreachable call can change that bit but cannot change runtime behavior.
+class BracketExitDeferredFlipProbe : public BacktestEngine {
+public:
+    explicit BracketExitDeferredFlipProbe(bool ast_has_strategy_close) {
+        initial_capital_ = 1'000'000;
+        default_qty_type_ = QtyType::FIXED;
+        default_qty_value_ = 1.0;
+        slippage_ = 0;
+        commission_value_ = 0;
+        pyramiding_ = 1;
+        syminfo_mintick_ = 0.01;
+        script_has_strategy_close_ = ast_has_strategy_close;
+    }
+
+    void on_bar(const Bar&) override {
+        if (bar_index_ == 0) {
+            strategy_entry("L", true,
+                           std::numeric_limits<double>::quiet_NaN(),
+                           std::numeric_limits<double>::quiet_NaN(), 1.0);
+        }
+        if (bar_index_ == 1 && position_side_ == PositionSide::LONG) {
+            strategy_entry("S", false,
+                           /*limit=*/105.0,
+                           std::numeric_limits<double>::quiet_NaN(), 1.0);
+            strategy_exit("XL", "L",
+                          std::numeric_limits<double>::quiet_NaN(),
+                          /*stop=*/95.0);
+        }
+        if (bar_index_ == 3) {
+            final_side = position_side_;
+            final_qty = position_qty_;
+            closed_trade_count = trade_count();
+        }
+    }
+
+    PositionSide final_side = PositionSide::FLAT;
+    double final_qty = 0.0;
+    int closed_trade_count = -1;
+};
+
 }  // namespace
 
 // Scenario 1: deferred-flip oracle (probe 95-style). Cycle qty grows
@@ -170,6 +210,36 @@ static void test_deferred_flip_chain_grows() {
     CHECK(max_qty >= 2);
 }
 
+// Scenario 1b: a strategy.exit bracket is itself sufficient to close the
+// source position before a priced opposite entry fires. The global AST scan
+// for strategy.close must not decide whether the captured carry applies.
+static void test_unreachable_strategy_close_is_semantically_inert() {
+    std::printf("test_unreachable_strategy_close_is_semantically_inert\n");
+    Bar bars[4] = {
+        {100, 101,  99, 100, 1000,  60'000},
+        {100, 101,  99, 100, 1000, 120'000},  // L fills; arm S + XL
+        {100, 101,  94,  95, 1000, 180'000},  // XL closes L; S untouched
+        {100, 106,  99, 105, 1000, 240'000},  // S limit fires from flat
+    };
+
+    BracketExitDeferredFlipProbe without_dead_close(false);
+    BracketExitDeferredFlipProbe with_dead_close(true);
+    without_dead_close.run(bars, 4);
+    with_dead_close.run(bars, 4);
+
+    CHECK(without_dead_close.last_error().empty());
+    CHECK(with_dead_close.last_error().empty());
+    CHECK(without_dead_close.final_side == PositionSide::SHORT);
+    CHECK(with_dead_close.final_side == PositionSide::SHORT);
+    CHECK(near(without_dead_close.final_qty, 2.0));
+    CHECK(near(with_dead_close.final_qty, 2.0));
+    CHECK(near(without_dead_close.final_qty, with_dead_close.final_qty));
+    CHECK(without_dead_close.closed_trade_count == 1);
+    CHECK(with_dead_close.closed_trade_count == 1);
+    CHECK(without_dead_close.closed_trade_count ==
+          with_dead_close.closed_trade_count);
+}
+
 // Scenario 2: deferred-flip is gated on opposite direction. A
 // pre-armed SAME-direction priced entry (long stop placed during a
 // long, position closes, long stop later fires from flat) should NOT
@@ -191,7 +261,6 @@ static void test_same_direction_no_carry() {
             slippage_ = 0;
             commission_value_ = 0;
             pyramiding_ = 1;
-            script_has_strategy_close_ = true;
         }
         void on_bar(const Bar& bar) override {
             if (bar_index_ == 0) {
@@ -323,7 +392,6 @@ static void test_two_cycle_siblings_independent_carry() {
             slippage_ = 0;
             commission_value_ = 0;
             pyramiding_ = 5;
-            script_has_strategy_close_ = true;
         }
         void on_bar(const Bar& bar) override {
             (void)bar;
@@ -487,7 +555,6 @@ static void test_per_leg_fifo_pnl_three_legs() {
             slippage_ = 0;
             commission_value_ = 0;
             pyramiding_ = 3;
-            script_has_strategy_close_ = true;
         }
         void on_bar(const Bar& bar) override {
             (void)bar;
@@ -689,6 +756,7 @@ static void test_overcap_same_id_reissue_removes_old_pending_order() {
 
 int main() {
     test_deferred_flip_chain_grows();
+    test_unreachable_strategy_close_is_semantically_inert();
     test_same_direction_no_carry();
     test_two_cycle_siblings_independent_carry();
     test_per_bar_pending_close_resets_in_script_tf_run();