Repository navigation
Size a reversal's exits from the new entry, as TradingView does - #817
Conversation
With process_orders_on_close and the default FIFO close rule, a strategy.close() of the open position followed, in the same execution, by a reversing strategy.entry() with an explicit quantity and named strategy.exit() calls for that entry reserved the exit quantity from a basis reduced by the close of the OLD position: the pending close was subtracted from the sum of the entry's book basis and the new entry's own units. A percentage exit was then sized from too little, or from nothing (it then fell back to the old position's size), an absolute-quantity exit lost its quantity, and a part of the new position could be left without an exit and stay open past its stop. The close is now subtracted from the book part of the basis only, floored at zero, before the new entry's units are added. Wherever the close does not exceed the book part the basis is unchanged. tests/test_adapter_named_exit_after_reversing_close.cpp runs twenty scenarios, each with and without the strategy.close(). Fourteen use process_orders_on_close (long and short reversals with the old position larger, smaller and equal; absolute-quantity and limit-only exits; a default-quantity entry). It checks every exit quantity against values derived from the entry's inputs, requires the old position's trade to carry the close's exit id, and compares the close run with the control field by field. Ten of those fourteen fail without the change. Six scenarios run without process_orders_on_close (the old position larger, smaller and equal, long and short, with next-bar fills): they compare the close run with the control field by field and assert the totals, times, prices and positions; they pass with and without the change, so the trades there are unchanged. The split of a reversal's exits between the take-profit and the stop is not asserted without process_orders_on_close. Not changed: reversals under the same id, close_entries_rule ANY, a same-side re-entry under another id and a partial close before a reversal; their exit sizing is separate work. Refs #803. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Without process_orders_on_close, a strategy.entry with an explicit quantity that reverses an open position is parked in the same-bar command queue until the end of the script execution. The pending-parent scan of strategy.exit looked only at the live and the pending entry lists, so the parked entry was not counted: the exit's basis was zero and its percentage fell back to the size of the position being replaced. With qty_percent below 100 and sizes that differ, the take-profit took a share of the old position (an old position of 4 reversed into 2 closed the whole 2 at the limit and left no stop; 2 reversed into 4 gave 1 and 3 instead of 2 and 2), and a take-profit given as an absolute quantity never fired, so the stop closed the whole position. The scan now also counts a parked entry that reverses the open position, so the percentage is taken from the new entry. Flat starts, same-side adds and default-quantity reversals are unchanged. tests/test_adapter_named_exit_after_reversing_close.cpp now asserts the exit split of its six next-bar scenarios (the four with different sizes fail without the change) and adds four next-bar scenarios with an absolute-quantity take-profit (they fail without the change as well). Refs #803. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Code graph out of date: Health delta baseline: last indexed commit f5e929d, 10 commit(s) behind this PR's base. Impact and hotspot numbers below may be wrong.
No verdict: the code graph is out of date. Static checks found no blocking issues, but they compared against an outdated graph. The next push after the repository re-indexes gets a full review.
Not checked: tests were not run; no formal proof of the changed code.
Graphify review — findings
Fixes sizing of named strategy.exit calls placed alongside a reversing strategy.entry with an explicit quantity: the exit percentage now comes from the new entry rather than the position being replaced. exit now also treats a parked same-bar reversing entry as a pending parent, and the live reservation basis subtracts the pending close from the book only, so the new entry's units are no longer reduced by the old position. With process_orders_on_close and FIFO, a percentage exit takes its share of the new entry and an absolute-quantity exit keeps its quantity; without it, an absolute-quantity take-profit that never fired now does (#803). Adds test_adapter_named_exit_after_reversing_close, which runs long and short reversals with and without the close and checks quantities against the order inputs. The changelog gets two entries covering both modes.
No blocking issues surfaced.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 797 functions depend on the 711 functions this change touches.
Health — this change adds coupling hotspots:
- new:
check_exits()— 1 callers, 7 callees - new:
run_case()— 1 callers, 6 callees
Verification — 797 functions in the blast radius were not formally verified this run (proofs are advisory here).
Health delta baseline: last indexed commit f5e929d, 10 commit(s) behind this PR's base.
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 749 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
1401 of 1401 test file(s) selected (100%) via static blast radius.
Escalated to a full run for safety (
noncode-change,uncovered-code) — the selection is not trustworthy on its own (see below). CI should run the whole suite.
benchmarks/tests/test_aggregate.py— full-run-safetybenchmarks/tests/test_check_provenance.py— full-run-safetybenchmarks/tests/test_compare.py— full-run-safetybenchmarks/tests/test_public_names.py— full-run-safetybenchmarks/tests/test_run_all.py— full-run-safetybenchmarks/tests/test_run_strategy_feeds.py— full-run-safetydocker/pineforge_sink_finalize_test.py— full-run-safetydocker/pineforge_sink_replay_rq_test.py— full-run-safetydocker/pineforge_sink_replay_test.py— full-run-safetydocker/pineforge_sink_to_json_test.py— full-run-safetydocker/quota_columns_test.py— full-run-safetydocker/quota_convert_test.py— full-run-safetydocker/quota_finalize_test.py— full-run-safetydocker/replay_rq_engine_test.py— full-run-safetydocker/run_json_artifact_window_test.py— full-run-safetydocker/run_json_bin_test.py— full-run-safetydocker/run_json_bounded_transport_test.py— full-run-safetydocker/run_json_bulk_equity_fuzz_test.py— full-run-safetydocker/run_json_bulk_equity_test.py— full-run-safetydocker/run_json_codes_test.py— full-run-safetydocker/run_json_converter_metadata_test.py— full-run-safetydocker/run_json_converter_session_test.py— full-run-safetydocker/run_json_diagnostics_test.py— full-run-safetydocker/run_json_equity_scan_test.py— full-run-safetydocker/run_json_fused_test.py— full-run-safetydocker/run_json_harnesses_test.py— full-run-safetydocker/run_json_metrics_only_test.py— full-run-safetydocker/run_json_misc2_test.py— full-run-safetydocker/run_json_misc_test.py— full-run-safetydocker/run_json_pfbar_test.py— full-run-safetydocker/run_json_provenance_test.py— full-run-safetydocker/run_json_quota_supervisor_test.py— full-run-safetydocker/run_json_replay_test.py— full-run-safetydocker/run_json_selected_sink_test.py— full-run-safetydocker/run_json_sink_lexical_test.py— full-run-safetydocker/run_json_sink_summary_test.py— full-run-safetydocker/run_json_sink_test.py— full-run-safetydocker/run_json_sink_vectors_test.py— full-run-safetydocker/run_json_spool_work_test.py— full-run-safetydocker/run_json_stringio_test.py— full-run-safetydocker/run_json_symbol_feeds_test.py— full-run-safetydocker/run_json_syminfo_test.py— full-run-safetydocker/run_json_trade_scan_test.py— full-run-safetydocker/run_json_window_budget_test.py— full-run-safetydocker/run_json_wrapper_work_test.py— full-run-safetyinclude/pineforge/native_run_spec.hpp— full-run-safetyscripts/derive_corpus_feeds_self_test.py— full-run-safetyscripts/fingerprint_self_test.py— full-run-safetyscripts/run_corpus_self_test.py— full-run-safetyscripts/test_abi_receipt_skips.py— full-run-safety- … and 1351 more
non-code file(s) changed (
CHANGELOG.md,tests/CMakeLists.txt) → running the full suite for safety (a code graph can't see config/fixture/data deps)
changed code file(s) with no mapped test (
CHANGELOG.md) — a coverage gap or a missing link — running the full suite rather than only the selected tests
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
· 2 grounded finding(s) anchored inline below.
| CHECK(old_rows == 1); | ||
| } | ||
|
|
||
| void check_exits(const Scenario& spec, const std::vector<Bar>& bars, const Observed& got) { |
There was a problem hiding this comment.
check_exits()
fans out to 7 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| got.final_units, got.error.c_str()); | ||
| } | ||
|
|
||
| void run_case(const Scenario& spec) { |
There was a problem hiding this comment.
run_case()
fans out to 6 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
The train now holds the changes merged into it so far, among them the fill-time drawdown rule (#804). The only textual conflict was the unreleased section of the changelog, where all three lines are kept; the adapter and the test registrations merged without edits. Refs #803. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
…#825) Include #797, #804, #817, #801 and #816: closed-trade cache placement, fill-time drawdown halt and flatten, reversal exit sizing, selected-capacity admission, and the percent default quantity cap. Document the intentional replacement of these earlier descriptions: - docs/design/native-feature-parity.md, introduced by 91d65ad: the old `update_risk_state(bar.close)` once per script bar at the close mark description is obsolete after #804. The cell now describes the fill-time peak, closing-fill threshold, halt latch and immediate flatten. Preserve MG10-a and MG10-b as HISTORICAL, measured on engine f5e929d, before the #804 change, with their values, claim status and grade unchanged. - docs/pages/memory-management.md, introduced by ba14721: "Storage is private: a global ordinal chooses one four-way set" no longer describes placement below the cap after #797 (653be41). Row n owns slot n while retained rows fit the cap; above it the existing four-way sets apply. Crossing the cap invalidates hot rows while retaining allocated pages. - docs/pages/memory-management.md, introduced by 69cddc9: "Page growth preserves entries through the 64/128/256 boundaries." is replaced by the current 256-slot page-boundary rule, which preserves entries through every page boundary within the cap. Re-anchor the remaining documentation citations and move the private-fixture registration evidence anchor from line 3470 to the actual token at line 3473. Preserve its row, predicate and reason; no new parity result or closed surface is claimed. Closes #804, closes #805. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Refs #803
Limits of this change (put under the description of the change)
What it changes: without
process_orders_on_close, astrategy.entrywith an explicit quantity that reverses an open position, followed in the same execution bystrategy.exitcalls for it (with or without astrategy.closeof the old position before the entry), now takes the percentage of those exits from the new entry instead of from the position being replaced; a take-profit given as an absoluteqtyon that entry now fires and the stop closes only the rest. One loop instrategy.exit's pending-parent scan now also counts a parked reversing entry; it is reached only when a position is open and the parked entry's side is opposite to it.What was measured against TradingView (NASDAQ:AAPL 15-minute bars of one day, fixed quantities, no commission, no
process_orders_on_close, an old long reversed into a short): an old position of 4 reversed into 2 gives 1 at the limit and 1 at the stop, and an old position of 2 reversed into 4 gives 2 and 2, plain and with astrategy.close; with an absolute take-profit (qty = 1, thenqty = 2) TradingView closes that quantity at the limit and the rest at the stop. Before this change PineForge closed the whole 2 at the limit with no stop (1 and 3 for 2 into 4) and its absolute take-profit never fired, in every release image from 1.0.0 to 1.11.0 that was run (1.6.1 and 1.6.2 were not tested); with it, the new test's expectations, which are derived from the scenario inputs by the rule TradingView showed, hold.What it does not change and does not claim:
pyramidingabove 1) and a default-quantity reversal are unchanged (the new loop's condition is false for a flat start and for a same-side add, and a default-quantity reversal keeps its zero basis); no TradingView measurement exists for them here.process_orders_on_closepaths are unchanged.Known item: this change adds lines to
src/source/pine_adapter.cpp, so the documentation-anchor check (scripts/check_doc_anchors.py) reports the anchors below them on this pull request; they are re-anchored once, by one commit on the release branch after the last member pull request.🤖 Generated with Claude Code