fix(engine): make dead strategy.close semantically inert - #143
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes the deferred-flip carry behavior so that adding an unreachable strategy.close in Pine source cannot change runtime backtest fills. It removes the runtime dependency on the compile-time script_has_strategy_close_ AST bit, keeps that bit only for generated-code compatibility, and adds regression/characterization coverage plus runtime documentation updates.
Changes:
- Remove
script_has_strategy_close_from the deferred-flip predicate inBacktestEngine::enter_market_from_flat. - Reframe
script_has_strategy_close_as a legacy, semantically inert compatibility member in engine headers and runtime docs. - Update/add tests to prove unreachable
strategy.closeis inert and adjust characterization expectations where carry behavior is now pinned correctly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/engine_orders.cpp |
Drops the compile-time flag from the deferred-flip carry condition so behavior depends only on order-captured runtime state. |
include/pineforge/engine.hpp |
Documents script_has_strategy_close_ as legacy compatibility only (no runtime semantic effect). |
tests/test_strategy_pyramiding.cpp |
Removes direct flag forcing and adds a metamorphic regression test for issue #141. |
tests/test_same_id_stop_replace.cpp |
Updates commentary to reflect the new deferred-flip predicate (no compile-time gate). |
tests/test_close_all_coqueued_entry.cpp |
Updates characterization to match generated-script behavior and pins expected carry outcome. |
docs/cheatsheet-runtime-and-execution.md |
Updates runtime cheat sheet to reflect that the flag no longer gates deferred-flip behavior and that brackets can arm carry behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+218
to
+221
| 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 |
luisleo526
added a commit
that referenced
this pull request
Aug 12, 2026
…efault sizing (#146) * fix(engine): same-bar close+reverse sequencing under frozen default sizing Generalize the TV-pinned SHORT-seed default-FIFO close collision oracle from its fixed-qty exact book to frozen PERCENT_OF_EQUITY / CASH default sizing with unequal seed/entry quantities (finding 272; derivation evidence/mc-timing-and-sequencing-derivation-20260812.json, closeReverseSeq, fitted 25/25 exact on alpha-forge-liquidity-matrix-v2). TV rule: with a SHORT seed qty S (entered on an earlier bar) and the exact same-bar book entry(Long); entry(Short); close(Long)[no-op]; close(Short)[frozen target S], all filling at the next open P, TV emits (1) the old short S exiting via order 'Long', (2) a zero-PnL dur-0 LONG round trip qty L (frozen default qty) 'Long'->'Short', (3) a second zero-PnL dur-0 LONG round trip qty min(S, L) 'Close entry(s) order Short'->'Short', and (4) an end-of-bar SHORT of exactly max(0, L - S) under the final entry's id — the real opposite entry is not queued; ordinary signal processing resumes from that position. Engine changes (all five touch points in src/engine_fills.cpp): - exact-book tagging: qty-type-aware seed gate (FIXED keeps the constant default==seed equality; PERCENT/CASH require the complete placement-frozen snapshot and matching frozen qtys on both entries); the projected final-short admission generalizes 2*seed.qty to L + min(S, L) and, for PERCENT (pct<=100), additionally mirrors the KI-54 frozen reversal re-check without its fill-time epsilon so a tagged book can never be half-declined mid-transaction. - materialization live check + fill: the close order materializes its placement-frozen target CAPPED at the live long book, min(S, L). - final-short live check: accepts lot2 == min(S, lot1) and position == lot1 + lot2. - final-short kernel: closes both LONG lots and re-opens SHORT with the residual lot1 - lot2 = max(0, L - S) at the same fill price under the final short's id/incarnation/comment; flat when L <= S. The FIXED cohort's pinned L == S collapses every relaxed gate to the old equalities and residual 0 (byte-identical; test_short_seed_close_collision unchanged). #143 dead-strategy.close, the declined-reversal close-leg suppression, gap-reject, and pyramiding paths are untouched; non-collision stale closes are still Removed. New unit coverage: tests/test_short_seed_collision_percent.cpp (percent remnant L>S with follow-up close of the remnant, percent flat L<=S, CASH remnant, gap-up KI-54 decline non-trigger control, partial-close non-trigger control). Full ctest: 125/125. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * corpus: advance to the re-minted same-bar sequencing run Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
script_has_strategy_close_AST bitstrategy.closecannot change bracket-exit fillsFixes #141.
Root cause
Codegen set
script_has_strategy_close_from textual AST presence, without reachability analysis. The runtime used that global bit to gate placement-time reversal carry, so adding an unreachablestrategy.closechanged later priced-entry quantities even though the executed broker commands were identical. Astrategy.exitbracket can close the carry source on its own, so the runtime decision must use the order's captured state rather than unrelated source text.Validation
countAbsDeltaregressions, 0 Excellent-to-below-Strong regressions, 0 unresolved failures