Skip to content

Preserve direction in trade size accessors - #764

Closed
luisleo526 wants to merge 5 commits into
mainfrom
fix/trade-accessor-signs-1126
Closed

luisleo526 wants to merge 5 commits into
mainfrom
fix/trade-accessor-signs-1126

Conversation

@luisleo526

Copy link
Copy Markdown
Collaborator

Pine trade-size accessors return positive quantities for long trades and negative quantities for short trades. Raw C report quantities retain their magnitude and separate direction. Related caller assertions and documentation rows distinguish those representations.

Refs #694 (case 4 only; the other cases remain open). Supersedes #760.

This successor branch is based on engine main after the hosted disk fix. Its tree matches the earlier signed-size candidate; the only additional source delta keeps the two signed accessor expressions on one line each so existing documentation citations remain valid.

Validation on the successor branch:

  • The binding documentation anchors, documentation lint, and both doc-reverts modes pass on the exact head.
  • The related-run evidence remains scoped to the earlier candidate: 28 executed tests passed, six private-fixture rows were skipped, and one ledger-cache Python row was an infrastructure gap because pytest was unavailable.
  • The accessor pin test remains a named gap pending the normal gate/private fixture route.
  • Hosted exact-head CI and one parity PASS remain pending.

Not run on the related candidate due to missing fixtures:

  • test_adapter_coof_relower: tests/fixtures/magnifier_intrabars/mi-ltf-f-1D/meta.json
  • test_engine_trade_accessors: tests/fixtures/daily_break_close/tg-xau1d-requests/meta.json
  • test_margin_call_1x_long_entry_fill_l4a: tests/fixtures/margin_open_rules/admission/short-refused-at-close/metrics.json
  • test_margin_rules_forward_replay: tests/fixtures/margin_call_rules/sizing/size-btc-p3u/tv_trades.csv
  • test_margin_rules_forward_replay_private: tests/fixtures/margin_ledger_rules/c1-r1-exp-l2s-reject/r1/tv_trades.csv
  • test_aggregated_entry_bar_index_tape: tests/fixtures/aggregated_entry_bar_index/bars.inc

Infrastructure gap: test_run_json_ledger_cache_policy could not run because its Python interpreter lacked pytest.

References: open trade size https://www.tradingview.com/pine-script-reference/v6/#fun_strategy.opentrades.size, closed trade size https://www.tradingview.com/pine-script-reference/v6/#fun_strategy.closedtrades.size.

Supersedes #760 (same change, history rebuilt so each commit names the documentation it removes).

🤖 Generated with Claude Code

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.
The signed trade-size accessor fix intentionally removes two obsolete public statements from docs/pages/pine-to-native.md. The sentence introduced by 7328f4d is wrong after the fix because Pine's size accessor carries direction in its sign; the sentence introduced by cced2aa is the same obsolete unsigned/raw-quantity description. The replacement rows state positive long and negative short and distinguish raw magnitude fields with is_long.

Name 7328f4d and cced2aa so the doc-reverts guard can verify why those historical sentences were removed.
The accessor bodies already return signed quantities; keep each expression on one source line so the existing documentation citations remain bound to the same header and implementation lines. This preserves the exact signed-size tree behavior while removing the anchor shift.

@graphify-labs graphify-labs Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 closed_trade_size() and open_trade_size() return signed quantities, positive for long and negative for short, matching Pine's strategy.closedtrades.size() and strategy.opentrades.size(). Raw Trade.qty and pf_trade_t::qty stay as magnitudes with a separate is_long, and the docs table now says so. Existing short-side test expectations flip sign (e.g. the margin-call and bracket-revive slices), and the fixture and tape checks apply the side to the expected quantity before comparing.

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

Update the margin-call twin assertion digest and register the pyramiding
partial-drain signed-size rewrite in the twin parity inventory. Recompute the
inventory hash so the guard covers both intentional assertion changes.
luisleo526 added a commit that referenced this pull request Nov 7, 2026
Combine six engine pull requests into one change, gated once.

Source pull requests and the heads merged here:
- #774 Add live drawing capacity to the runtime limit vocabulary (99b0bfe)
- #764 Preserve direction in trade size accessors (834333f)
- #765 ci: reduce sanitizer debug info footprint (be68cab)
- #773 Include ABI layout probe in PR smoke (4f31c10)
- #772 ci: promote the engine baseline on parity alone (349418e)
- #759 Write bounded drawing arenas in place and share their slots between checkpoints (cecedc0)

Gate on head b4c316b (tree 1a4b2ea): required CI green; one parity sweep over 7,989 probes with no lost coverage, no band movement and no hard regression; one review of the combined diff. In the post-merge native run on the same tree, every native shard and the thread-sanitizer leg pass; the address-sanitizer legs fail on runner disk space (No space left on device), the same jobs that fail on main.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@luisleo526

Copy link
Copy Markdown
Collaborator Author

Merged as part of #776 (combined change, commit 63f3978), which carries this pull request's head unchanged and passed the full gate once. Closing this one; the branch is kept.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant