fix(margin): close one whole contract on every floor-zero liquidation#136
Merged
Merged
Conversation
TradingView's forced-liquidation quantity rule floors the minimum-restore quantity to the instrument lot step BEFORE the documented 4x over- liquidation, and when that product truncates to zero it closes ONE WHOLE CONTRACT, capped at the open position. The engine had the 4x multiplier and the floor-before ordering right, but conditioned the one-contract fallback on a commissioned all-in market SHORT lifecycle (or a direct short-reversal lifecycle) and on position_side_ == SHORT, defaulting everything else to a one-qty_step nibble or to a dust no-op. The opening-affordability twin was gated even more narrowly, on a fee-created percent-commission provenance. Neither gate is a real broker predicate. Fitting the rule against every `Signal == "Margin call"` fragment in the campaign's TV exports (58,737 USDT-account fragments over 89 slugs) reaches 99.956% exact. On the 974 events where the fallback value is unconstrained TV closed exactly 1.0000 contracts 971 times; 950 of those lie OUTSIDE the lifecycle gate and 464 are LONG *and* commission-free — a configuration the gate cannot reach by construction. Every competing fallback scored 0/974: one qty_step, 4 qty_step, the whole residual, 1% of the position. The engine's own converted-currency carried-rollover helper already implements the unconditional form, so this makes the two self-consistent. The change is a pure relaxation of the lot rule: it deletes the lifecycle, side, and commission-model conditioning and adds no predicate. The structural guards are kept unchanged — a real sub-contract restore quantity, a lot grid no coarser than one contract, and a grid that can express one whole contract (or a full-position cap). When they do not hold the branch now fails closed instead of fabricating a one-step nibble: `min(qty_step_, qty)` was contradicted 962 times and supported 0 times, and is unreachable. The opt-in whole-residual interpretation keeps precedence. The converted-account-currency cent-rounding tolerance is preserved and moved out of the lot rule into the opening AFFORDABILITY (trigger) test where it belongs. The fit excluded every FX-converted account, so it carries no evidence about that edge; the tolerance is identically zero for same-currency strategies, so their arithmetic is unchanged. Known untested dimension: every margin-call slug on this evidence base is a 0.0001-lot instrument, so the tape cannot distinguish "one whole contract (1.0)" from "10,000 x qty_step". Tests: four RED-first fixtures for the classes the gate could not reach (a commission-free LONG at margin 100, a commission-free SHORT on the finite- price cascade, structural-guard preservation with a teeth case, and the full-position cap below one contract). Twelve existing fixtures whose expectations encoded the deleted conditioning are updated to the generic behaviour; the lifecycle provenance bits they also assert are unchanged. ctest 121/121. Corpus gate 252 ran, ok=252, excellent=247 / strong=4 / anomaly=1 — identical to the pre-change baseline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adjusts Pineforge’s margin-call forced-liquidation sizing to match TradingView’s observed behavior when the minimum-restore quantity floors to zero at the instrument lot step: close one whole contract (capped by the open position), without gating on position side, commission model, or entry-lifecycle provenance.
Changes:
- Relaxes the floor-zero forced-liquidation fallback in
process_margin_call()to apply generically (while keeping existing structural guards and the opt-in whole-residual behavior). - Updates and expands unit tests to cover previously unreachable configurations (e.g., LONG + commission-free; CASH_PER_ORDER) and to assert the one-contract outcome.
- Updates a related “frozen flat gap reject” fixture to reflect the new generic floor-zero liquidation behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/engine_fills.cpp | Removes lifecycle/commission gating for floor-zero liquidation and applies a generic one-contract fallback with structural guards. |
| tests/test_margin_call.cpp | Updates existing fixtures and adds new ones to validate the generic one-contract floor-zero behavior across multiple shapes. |
| tests/test_frozen_flat_gap_reject.cpp | Updates expected outcome for a commission-free LONG case that now triggers a one-contract margin-call trim. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+839
to
+843
| // tapes only — it excluded every FX-converted account (those score 3.9% | ||
| // because q_min needs ~1e-7 relative precision through the daily | ||
| // conversion series) — so it carries no evidence about this edge and | ||
| // must not be read as deleting it. Same-currency strategies keep the | ||
| // exact comparison: the tolerance is identically zero for them. |
Comment on lines
+957
to
+965
| // conditioning. Fitted against every `Signal == "Margin call"` | ||
| // fragment in the campaign's TV exports (58,737 USDT-account | ||
| // fragments over 89 slugs, 99.956% exact): on the 974 events where | ||
| // the fallback value is unconstrained TV closed exactly 1.0000 | ||
| // contracts 971 times. 950 of those lie OUTSIDE any short/ | ||
| // commissioned lifecycle scope and 464 of them are LONG *and* | ||
| // commission-free. Competing fallbacks scored 0/974 each: one | ||
| // qty_step, 4 qty_step, the whole residual, 1% of the position. | ||
| // |
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.
The bug
TradingView's forced-liquidation quantity rule floors the minimum-restore quantity
to the instrument lot step before the documented 4x over-liquidation, and when
that product truncates to zero it closes one whole contract, capped at the open
position.
The engine already had the 4x multiplier and the floor-before ordering right. What
it got wrong is the floor-zero fallback: it conditioned the one-contract branch on a
commissioned all-in market SHORT lifecycle (or a direct short-reversal lifecycle) and
on
position_side_ == SHORT, defaulting everything else to a one-qty_stepnibble ora dust no-op. The opening-affordability twin was gated even more narrowly, on a
fee-created percent-commission provenance.
Neither gate is a real broker predicate. This change is a pure relaxation: it
deletes lifecycle conditioning and adds none, keeping the structural guards
(
qty_step <= 1.0,0 < raw_q_min < 1.0, on-grid / full-position cap) that thealready-generic carried-rollover helper uses.
Evidence
Fitted against every
Signal == "Margin call"fragment in the validationexports — 58,711 / 58,737 = 99.956 % exact across 89 accounts.
On the 974 events where the fallback value is unconstrained, TradingView closed
exactly
1.0000contracts 971 times. 950 of those lie outside the gate thischange removes, and 464 are LONG and commission-free — a configuration the old
gate cannot reach by construction. Zero events anywhere support the gate as a
restriction.
Competing fallbacks on the same discriminating set: one
qty_step0/974,4
qty_step0/974, whole residual 0/974, 1 % of position 0/974.Competing lot shapes overall: x3/x5/x2 3.4 %, floor-after-multiply 25.9 %,
no quantisation 0.017 %, always-close-whole-position 1.7 %. So 4x,
floor-before, and one-whole-contract are each uniquely determined.
Gates
252/252 engine tapes byte-identical to the unpatched build.
417-strategy sweeps at identical pins, separate fresh build dirs.
1 UP / 0 DOWN, sweep exit 0. One strategy moved strong -> excellent
(match 99.50 -> 100.00, exitP90 0.0163 -> 0.0000, pnlP90 8.3696 -> 0.0076).
margin-call rows; 72 other margin-call strategies unchanged; nothing outside the
class moved.
Honest limits
one
countAbsDelta75 -> 96, onepnlP9025.24 -> 27.21, one match% 93.60 -> 93.40.None is a tier DOWN, but this rule is not uniformly closer to TradingView and
should not be described that way. A further sub-mechanism is still mis-modelled.
qty_step = 0.0001, so the datacannot distinguish "one whole contract (1.0)" from "10,000 x qty_step". The
implementation writes
1.0, matching the existingstd::min(1.0, qty)sites.flip $0.000006-$0.003223 on $0.42M-$29.6M accounts), 3 are genuinely unexplained.
byte-identical corpus result. Positive evidence comes from the unit tests and the
fit, not from the corpus.