fix(engine): tighten reversal admission to float tolerance#139
Merged
Conversation
The fill-time margin admission gate declines an order when required_margin > free_funds + epsilon. The epsilon was widened by one whole lot of notional (qty_step * admit_price * pointvalue * fx * margin/100) on every arm of that gate. The widening was justified by an empirical claim — that every decline TradingView's exports confirm has a margin exceeding one lot — which was true only while no sub-lot ground truth existed for the reversal arm. On an all-in reversal the order spends the entire equity by construction, so the whole admission decision lives inside [0, qty_step * admit_price). The widening therefore did not blunt that arm's gate, it made it inert: a 13-month ETHUSDT.P tape carrying 2,419 reversal decisions (94 declines, 2,325 admits, percent_of_equity=100 at margin 100) puts 92 of the 94 confirmed declines BELOW one lot, and the shipped epsilon catches 2 of 94 (balanced accuracy 51.1%). Reducing the reversal arm to a genuine float guard catches 86 of 94 with 6 wrongly cancelled admits (95.6%). Scope: reversal arm only. The flat-open and same-direction-add arms keep the one-lot term — each is separately TV-pinned, nothing falsifies their premise, and the "lot-floor luck" argument genuinely applies to them because they price at the same sizing price the quantity was floored against. A reversal floors against the previous bar's close and fills at this bar's open, so its overshoot is an observable gap. Tests: new test_reversal_admission_float_guard pins the tightened arm (sub-lot adverse gap declined, exact required==free tie still admitted, favourable gap admitted, above-lot gap unchanged) plus scope controls proving the same-direction add still carries the one-lot slack, the flat open on the identical gap still fills, and qty_step==0 is untouched. test_margin_admission_gate pin H is corrected to the new ground truth. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR tightens the fill-time margin admission logic for reversals by removing the “one whole lot” epsilon widening on the reversal arm only, leaving the flat-open and same-direction-add behavior unchanged. It also adds targeted tests to pin the corrected boundary behavior and updates an existing test to reflect the new reversal rule.
Changes:
- Adjust reversal admission epsilon in
BacktestEngine::apply_filled_order_to_state()to use only the floating-point guard (no one-lot slack) whenreversal == true. - Add a new focused regression test suite covering reversal boundary cases plus scope controls for flat opens and same-direction adds.
- Update the existing margin admission gate test to assert the corrected reversal behavior for a sub-lot adverse gap.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/engine_fills.cpp | Removes one-lot epsilon widening for reversals while preserving it for non-reversal arms; expands rationale comment. |
| tests/test_reversal_admission_float_guard.cpp | New pin set covering reversal boundary cases and scope controls to ensure only the reversal arm changes. |
| tests/test_margin_admission_gate.cpp | Updates the “H” reversal slack pin to match the new reversal decline behavior and expands documentation. |
| tests/CMakeLists.txt | Adds the new test target to the test build. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
What changed
Root cause
Percent-of-equity reversal quantity is frozen against the prior close, while the order fills at the next open. The previous one-lot epsilon treated this observable gap shortfall like lot-floor remainder and made the reversal affordability gate effectively inert for sub-lot declines.
Validation
Latest pinned comparison (
engine c31996c,codegen 4b1af93) produced:Fresh local integration gates on this commit:
-j16)The behavioral change is generic and limited to reversal admission. No reservation, FIFO, partial-close, FX, codegen, or strategy-specific branch is included.