Repository navigation
fix(sync): import broker trade history without phantom positions - #29
Draft
anthony-som wants to merge 1 commit into
Draft
anthony-som wants to merge 1 commit into
anthony-som wants to merge 1 commit into
Conversation
Broker trade history has a floor (Webull keeps roughly the last year), so a sync can see fills that close positions opened before it. Those showed up as positions that never existed, e.g. a phantom short after selling shares bought earlier. - Skip fills that close a position opened before the history floor: broker-labelled closes with nothing open, and, for Webull (whose position list is complete), unlabeled equity fills that leave the replay disagreeing with the broker's position. - Close expired Webull options at 0 at the 4pm New York expiry, since Webull records no order for an expiry. No default fee rules apply. - Save broker-stated option contract multipliers, keeping user overrides. - Pass the SDK's historySince (last sync minus a 30-day overlap) so repeat syncs fetch only recent orders; content-hash dedupe absorbs refetches. - Carry broker-stated asset classes onto synced executions. Requires @luxalgo/broker-sdk with Trade.assetClass/multiplier/ positionEffect and historySince (LuxAlgo/broker-sdk#19). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.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.
Fixes #28. Draft: blocked on LuxAlgo/broker-sdk#19. This PR uses fields that don't exist in
@luxalgo/broker-sdk0.5.1:Trade.assetClass,Trade.multiplier,Trade.positionEffectandConnectOptions.historySince. Typecheck fails until the SDK release with #19 lands and the dependency is bumped. I'll push the version bump once that release is out.Summary
Per CONTRIBUTING, broker connectivity stays in the SDK: the Webull route, equity and history fixes are in broker-sdk#19. This PR is only the journal side, meaning what to do with a broker's trade history once it arrives.
apps/web/src/server/sync-history.ts(new, pure)orphanFills(known, incoming, brokerQuantity)finds incoming fills that close a position opened before the broker's history floor, which would otherwise show up as a phantom position:positionEffect: "close") with nothing open on the other side is dropped.brokerQuantityreturnsundefined) keep every unlabeled fill. Behavior is unchanged for every broker except Webull.expiredOptionCloses(optionFills, now)closes expired option positions (OCC-styleXYZ 260925P50symbols) at 4pm New York on the expiry date. The DST-aware time is 20:00Z in summer and 21:00Z in winter, and it stays fixed so the synthetic fill hashes the same on every sync.apps/web/src/server/sync.tshistorySince, the last sync minus a 30-day overlap, once an account has synced fills. The overlap catches orders placed earlier that filled since, in case the broker filters by placement time. Existing content-hash dedupe absorbs the refetches.assetClassonto executions. The SDK's"cash"has no journal equivalent, so it's left off.multiplierssettings without overwriting a value the user set, so option P&L is ×100 instead of ×1.skippedReasons. Duplicates still pass through, so dedupe counts stay accurate; an earlier draft broke four IBKR tests on exactly that. It also inserts expired-option closes at $0 withpreserveFees, so default fee rules don't charge commission on an expiry.apps/web/tests/webull-sync.test.ts(new)Product statements, following CONTRIBUTING:
Test plan
pnpm typecheckpasses, with a local build of broker-sdk#19 linked throughpnpm patch. That patch isn't part of this PR.pnpm test: 54 files / 540 tests pass, including the IBKR timezone, broker-connect and sync-resilience suites.prettier --checkon the changed files.@luxalgo/broker-sdkand re-runpnpm typecheckin CI.Known limits (called out in code with
ponytail:notes)Review
A second Claude Code session reviewed an earlier draft. It found one high issue (expiry closes on truncated history booking fake P&L) and four medium issues (default fees on synthetic closes, partial-fill double counting, the since-overlap window, and first-sync failure on empty history). All are addressed here or in broker-sdk#19.
🤖 Generated with Claude Code