Repository navigation
Preserve signed quantities in trade-size accessors - #760
luisleo526 wants to merge 5 commits into
Conversation
Assert open and closed short accessors preserve direction from a real fill.
Report signed quantities for open and closed short trades while preserving long, flat and bounds behavior.
Keep Pine trade-size assertions signed by direction while retaining raw Trade quantity magnitudes, fixture bytes and fill-derived numeric oracles. Compare native open lots directly and keep corruption checks on stored qty. Document positive long and negative short accessor sizes. Updated callers: ledger fixtures and sealed text readers; aggregated entry rows; bracket lifecycle and position cycles; short reversal affordability; margin calls and chronology; forward replay; native open lots; pyramiding; mintick sizing; and close reissue scenarios, including their native variants. The two Pine-to-native size table rows distinguish signed Pine accessors from raw C report quantities and their separate direction. References: https://www.tradingview.com/pine-script-reference/v6/#fun_strategy.opentrades.size https://www.tradingview.com/pine-script-reference/v6/#fun_strategy.closedtrades.size
…ngine into fix/signed-trade-size-1126
![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.
No issues found by static checks — no coupling regressions or blocking issues in the code graph. That is not the same as safe to merge: see what was not checked.
Not checked: tests were not run; formal verification proved 0 of 1 changed function(s) (1 not verified).
Formal verification. PR-changed functions: 0/1 verified (0 proven, 0 may-equivalent, 0 distinguished) · 1 not verified (1 unsupported).
Not verified on this run: open\_trade\_size (unsupported).
Graphify review — findings
Makes strategy.closedtrades.size() and strategy.opentrades.size() return signed quantities (positive long, negative short) to match Pine, by applying the trade's direction in closed_trade_size and open_trade_size. Raw Trade.qty and pf_trade_t::qty stay as magnitudes with a separate is_long, so only the script-facing accessors change. Existing short-side test expectations are flipped to negatives, and the docs now spell out the signed accessors versus the unsigned raw fields.
Review partial — this diff was larger than one review pass covers, so later files were not reviewed; some findings may be missing.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 3254 functions depend on the 1993 functions this change touches.
Health — grade A; 10 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
applied()— 10 callers, 10 callees (high)refused()— 4 callers, 9 callees (medium)sink_on_run()— 2 callers, 17 callees (high)exercise()— 3 callers, 11 callees (medium)engine_series()— 3 callers, 11 callees (medium)quote_is_read_only()— 5 callers, 6 callees (medium)run_twin()— 5 callers, 5 callees (medium)giant_intraday()— 2 callers, 11 callees (medium)- …and 2 more
Verification — 3254 functions in the blast radius were not formally verified this run (proofs are advisory here).
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: 3083 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify open\_trade\_size.
The verifier did not have enough to check open\_trade\_size, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: unsupported: member function — receiver/self (`this`) not synthesizable from the simple-type literal pools — abstaining rather than guessing a receiver
…ngine into fix/signed-trade-size-1126
|
Superseded by #764: the same change on a rebuilt history, where each commit names the documentation it removes, with source lines kept stable for the documentation anchors. Branch kept. |
Pine trade-size accessors now return positive quantities for long trades and negative quantities for short trades. Previously, short open and closed trades returned positive quantities. Raw C report quantities retain their magnitude and separate direction. Related caller assertions and the two documentation rows now distinguish those representations.
Refs #694 (case 4 only; the other cases remain open).
Validation: the exact candidate built successfully and selected 35 related tests: 28 passed, six were skipped for missing private fixtures, and the ledger-cache Python wrapper failed to start because pytest was unavailable. No executed size assertion failed. The earlier direct short-fill regression failed before the fix and passed after it; it is not claimed as a run of the skipped accessor test on this candidate. Exact-head CI and parity remain pending before landing.
Not run on this candidate due to missing fixtures:
Infrastructure gap: test_run_json_ledger_cache_policy could not run its assertions because its Python interpreter lacked pytest. The hosted CI result will be reported separately.
References: open trade size, closed trade size.
Refs #694 (case 4).
🤖 Generated with Claude Code