fix(soxl): preserve holdings and include research trading costs - #486
Merged
Conversation
Co-Authored-By: Codex <noreply@openai.com>
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.
SOXL research could report gains from an untraded price round trip, omit trading fees from net returns, and lose fees entirely when targets invested all available cash. Preserve the carried portfolio between signals, reserve cash for cash-inclusive one-way turnover costs, and calculate interval returns from pre-trade equity. Summary turnover now uses executed trades and drawdown includes initial NAV; the existing intraday consumer converts holdings to the same pre-trade return basis.
Normal indicator warmup still carries the portfolio. A strategy exception now fails with a fixed message instead of being reported as a successful cash hold. The legacy same-close research assumption is documented; this does not change the frozen V3/V7 runner, inputs, strategy versions, or prior evidence.
Validation: the six original accounting regressions failed on the base implementation. All 28 related SOXL, archive, shared-summary, and intraday tests now pass, including hand-calculated fee and holding examples; Ruff 0.15.9 on the three changed files and git diff --check pass. No real-data replay, model call, deployment, or trading was performed. Previously generated metrics are not recomputed by this change.