Fix percent-equity fee and margin lifecycle parity#133
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Aligns PineForge’s percent-of-equity sizing and broker margin-call math with TradingView by using a fee-net “live equity” ledger (subtracting already-paid percent entry fees on still-open lots) and by tightening the lifecycle/provenance rules around close-then-open reversals and commissioned default-sized shorts.
Changes:
- Introduce fee-net equity helpers (
surviving_open_percent_commission_account,percent_commission_live_equity) and use them for percent-of-equity sizing snapshots and adverse-margin equity checks. - Refine margin-call lifecycle/provenance handling for commissioned default-sized shorts (including close-then-short “opening then adverse retry” sequencing) and invalidate that provenance on script-driven partial reductions/adds while preserving it across broker-driven margin trims.
- Add new clean-room quantity oracles and regression tests covering fee-net sizing, FIFO fee snapshot scaling, reversals, and the new lifecycle behaviors.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_percent_equity_open_entry_fee.cpp | New regression coverage for fee-net percent-of-equity sizing and margin-call equity behavior, including clean-room quantity oracles. |
| tests/test_margin_call.cpp | Adds extensive lifecycle/provenance regression tests for commissioned default-sized close-then-open behavior and floor-zero edges. |
| tests/CMakeLists.txt | Registers the new percent-equity fee test target. |
| src/engine_strategy_commands.cpp | Freezes default-sized market order sizing equity from fee-net live equity. |
| src/engine_orders.cpp | Tracks partial-reduction cause to preserve/clear commissioned short lifecycle provenance appropriately. |
| src/engine_fills.cpp | Implements fee-net equity helpers; updates margin-call logic to use fee-net equity and adds the close-then-short “adverse retry” sequencing and lifecycle floor-zero handling. |
| include/pineforge/engine.hpp | Documents updated sizing-equity semantics; declares new helpers; adds new provenance/lifecycle flags and partial-reduction cause plumbing. |
💡 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.
Summary
Validation
No strategy-specific runtime override or target identifier is present in production code.