From 0b2fd40b5d41931d8089de111b6dedcbef60c306 Mon Sep 17 00:00:00 2001 From: luisleo526 Date: Sat, 1 Aug 2026 16:05:28 +0800 Subject: [PATCH] fix(engine): make dead strategy.close semantically inert --- docs/cheatsheet-runtime-and-execution.md | 8 ++- include/pineforge/engine.hpp | 11 ++-- src/engine_orders.cpp | 22 +++---- tests/test_close_all_coqueued_entry.cpp | 18 ++++-- tests/test_same_id_stop_replace.cpp | 11 +--- tests/test_strategy_pyramiding.cpp | 82 ++++++++++++++++++++++-- 6 files changed, 112 insertions(+), 40 deletions(-) diff --git a/docs/cheatsheet-runtime-and-execution.md b/docs/cheatsheet-runtime-and-execution.md index 7a0892c..60e6603 100644 --- a/docs/cheatsheet-runtime-and-execution.md +++ b/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. diff --git a/include/pineforge/engine.hpp b/include/pineforge/engine.hpp index 7782008..10d4f75 100644 --- a/include/pineforge/engine.hpp +++ b/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::min(); diff --git a/src/engine_orders.cpp b/src/engine_orders.cpp index 578a3f1..ea9ac1c 100644 --- a/src/engine_orders.cpp +++ b/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 diff --git a/tests/test_close_all_coqueued_entry.cpp b/tests/test_close_all_coqueued_entry.cpp index ef6da4f..78df30a 100644 --- a/tests/test_close_all_coqueued_entry.cpp +++ b/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) } // ───────────────────────────────────────────────────────────────────── diff --git a/tests/test_same_id_stop_replace.cpp b/tests/test_same_id_stop_replace.cpp index 47eed2b..d49cef2 100644 --- a/tests/test_same_id_stop_replace.cpp +++ b/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 diff --git a/tests/test_strategy_pyramiding.cpp b/tests/test_strategy_pyramiding.cpp index bb75996..33c5cb9 100644 --- a/tests/test_strategy_pyramiding.cpp +++ b/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::quiet_NaN(), + std::numeric_limits::quiet_NaN(), 1.0); + } + if (bar_index_ == 1 && position_side_ == PositionSide::LONG) { + strategy_entry("S", false, + /*limit=*/105.0, + std::numeric_limits::quiet_NaN(), 1.0); + strategy_exit("XL", "L", + std::numeric_limits::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();