Fix/account scoped netting - #172
Open
pasipa2 wants to merge 4 commits into
Open
Conversation
The netting key was symbol+type+strategy+td with no account, so two accounts
trading the same symbol in the same direction on the same day netted against
each other and produced merged trades belonging to neither. Measured on two live
MT5 accounts over the same window: 5,327 real trades collapsed to 2,736, with
69.3% of them mixing both accounts and holding 85.7% of all executions. Totals
survived (proceeds are summed per execution) but trade count, depth, duration and
win rate were fiction.
Five sites decided identity and none included the account:
* the groupBy key -- the merge itself
* the open-position lookups in Parse and in the current file, so one account
could adopt another's open position and append its executions to it
* the trade id and the execution id, which collide when two accounts open the
same symbol in the same second (routine when several agents seed on similar
cadences, and disambiguated today only by an order-dependent counter)
temp2.account was already carried on every execution, so this only threads it
into the keys. Parsers that do not set Account group under 'undefined' exactly as
before, so single-account imports are unchanged.
Verified against a running server: two MT5 accounts imported into ONE user over
five days, 1,572 executions, 686 trades. Each account reconciles to its own
broker total independently -- 457237 at -171.32 and 7959503 at +189.08, both
delta 0.0000 -- with 0 trades left unattributed.
express.json() was mounted with no limit, so body-parser's 100 kB default applied and /api/trades 413'd on ordinary days. This is not a large-payload edge case: a day CANNOT be split, because dedupe omits whole days that already hold trades, so posting half a day silently strands the other half. The whole day has to fit or it cannot be imported at all. One account's busy day already runs ~92 kB compactly encoded. Consolidating a second account into the same user -- now that netting is account-scoped -- takes three of five measured days past the ceiling (110 kB, 137 kB, 140 kB). Without this the account fix is unusable for the case it exists to serve. Raised to 25mb on all three mounts. Verified: those same three days now import cleanly and reconcile exactly.
Builds docker/Dockerfile -- the FULL build, not a derived FROM-image -- and pushes to ghcr.io/pasipa2/tradenote using the built-in GITHUB_TOKEN, so no registry secret is needed. The full build matters. The server and the web UI each carry their own copy of the netting logic: index.mjs imports src/utils/addTrades.js as source, while the UI runs the Vite bundle. The image currently deployed is a derived one that COPYs only the server file, so a hand-import through the browser would still merge accounts. A full build compiles the frontend from patched source too -- verified locally: the account key appears both in /app/src/utils/addTrades.js and, minified, in /app/dist/assets/index-*.js. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xxd7YyvYvqoKwTyxBcfdvG
The accounts list stored on the user is append-only, and the append used tradeAccounts[0] instead of the element being iterated. With a single-account journal those are the same value, so it went unnoticed. With two accounts, every import re-added the FIRST account -- because the missing-account check kept failing for the second -- so the filter accumulated one duplicate per import and the second account never became selectable at all. Observed after consolidating two live accounts: 63 identical entries for 7959503 and no 457237. Also dedupe on write: the list is append-only, so a journal imported before this fix already carries repeats and would otherwise keep saving them back. Trade data was never affected -- only the filter list on the user record. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xxd7YyvYvqoKwTyxBcfdvG
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
1 participant
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.
No description provided.