From bdb14224154d9832401cf0048e433eb532aa5361 Mon Sep 17 00:00:00 2001 From: Don Nyambudzi Date: Tue, 18 Aug 2026 19:11:45 +0100 Subject: [PATCH 1/4] fix: scope trade netting by account 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. --- src/utils/addTrades.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/utils/addTrades.js b/src/utils/addTrades.js index c28a4d3..46acd59 100644 --- a/src/utils/addTrades.js +++ b/src/utils/addTrades.js @@ -432,7 +432,7 @@ async function createTempExecutions() { temp2.price = parseFloat(tradesData[key].Price); temp2.execTime = dayjs.tz(formatedDateTD + " " + tradesData[key]['Exec Time'], timeZoneTrade.value).unix() - let tempId = "e" + temp2.execTime + "_" + temp2.symbol.replace(".", "_") + "_" + temp2.type + "_" + temp2.side; + let tempId = "e" + temp2.execTime + "_" + temp2.account + "_" + temp2.symbol.replace(".", "_") + "_" + temp2.type + "_" + temp2.side; // It happens that two or more trades happen at the same (second) time. So we need to differentiated them if (tempId != lastId) { x = 1 @@ -938,7 +938,7 @@ async function createTrades() { var b = _ .chain(tempExecutions) .orderBy(["execTime"], ["asc"]) - .groupBy(item => `"${item.symbol}+${item.type}+${item.strategy}+${item.td}"`); + .groupBy(item => `"${item.account}+${item.symbol}+${item.type}+${item.strategy}+${item.td}"`); let objectB = JSON.parse(JSON.stringify(b)) //console.log("object b "+JSON.stringify(objectB)) @@ -987,10 +987,10 @@ async function createTrades() { /* Checking existing open position amongst open positions stored IN PARSE / DATABASE */ - const existingOpenPositionParseIndex = openPositionsParse.findIndex(x => x.symbol == tempExec.symbol && x.type == tempExec.type && x.strategy == tempExec.strategy) + const existingOpenPositionParseIndex = openPositionsParse.findIndex(x => x.account == tempExec.account && x.symbol == tempExec.symbol && x.type == tempExec.type && x.strategy == tempExec.strategy) /* Checking existing open position amongst open positions stored LOCALLT */ - const existingOpenPositionFileIndex = openPositionsFile.findIndex(x => x.symbol == tempExec.symbol && x.type == tempExec.type && x.strategy == tempExec.strategy) + const existingOpenPositionFileIndex = openPositionsFile.findIndex(x => x.account == tempExec.account && x.symbol == tempExec.symbol && x.type == tempExec.type && x.strategy == tempExec.strategy) //checking existing open positions array when importing file if (newTrade == true) { @@ -1071,7 +1071,7 @@ async function createTrades() { //console.log(" -> exec id "+tempExec.id) //tempExecIds.push(tempExec.id) - temp7.id = tempExec.side == "B" || tempExec.side == "S" ? "t" + tempExec.execTime + "_" + tempExec.symbol + "_" + tempExec.type + "_B" : "t" + tempExec.execTime + "_" + tempExec.symbol + "_" + tempExec.type + "_SS" + temp7.id = tempExec.side == "B" || tempExec.side == "S" ? "t" + tempExec.execTime + "_" + tempExec.account + "_" + tempExec.symbol + "_" + tempExec.type + "_B" : "t" + tempExec.execTime + "_" + tempExec.account + "_" + tempExec.symbol + "_" + tempExec.type + "_SS" console.log(" --> ID " + temp7.id) currentTradeId = temp7.id temp7.account = tempExec.account; From ff532b81721cce4f26635d7075dc7726baf66aec Mon Sep 17 00:00:00 2001 From: Don Nyambudzi Date: Tue, 18 Aug 2026 19:11:54 +0100 Subject: [PATCH 2/4] fix: raise the request body limit from the 100 kB default 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. --- index.mjs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/index.mjs b/index.mjs index 5c49cfd..059ea25 100644 --- a/index.mjs +++ b/index.mjs @@ -46,7 +46,11 @@ console.log(' -> Database URI ' + hiddenDatabaseURI) let tradenoteDatabase = process.env.TRADENOTE_DATABASE var app = express(); -app.use(express.json()); +// Default body-parser limit is 100 kB. One account's busy day already runs ~92 kB +// compactly encoded, and a day CANNOT be split (dedupe omits whole days that +// already hold trades), so consolidating a second account into one user pushes +// ordinary days past the ceiling and they 413. +app.use(express.json({ limit: '25mb' })); const port = process.env.TRADENOTE_PORT; const PROXY_PORT = 39482; @@ -339,7 +343,7 @@ const setupApiRoutes = (app) => { - app.use(express.json()); + app.use(express.json({ limit: '25mb' })); let allUsers const getAllUsers = async () => { @@ -498,7 +502,7 @@ const startIndex = async () => { resolve(); } else { // In production, handle API routes normally - app.use('/api/*', express.json(), (req, res, next) => { + app.use('/api/*', express.json({ limit: '25mb' }), (req, res, next) => { //console.log(`Received API request: ${req.method} ${req.url}`); next(); // Pass control to specific API handlers }); From c694691b8292d549e02848f390f7cb67b39d01be Mon Sep 17 00:00:00 2001 From: Don Nyambudzi Date: Tue, 18 Aug 2026 19:35:49 +0100 Subject: [PATCH 3/4] ci: publish the patched image to this fork's GHCR 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 Claude-Session: https://claude.ai/code/session_01Xxd7YyvYvqoKwTyxBcfdvG --- .github/workflows/publish-image.yml | 57 +++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 .github/workflows/publish-image.yml diff --git a/.github/workflows/publish-image.yml b/.github/workflows/publish-image.yml new file mode 100644 index 0000000..68c2871 --- /dev/null +++ b/.github/workflows/publish-image.yml @@ -0,0 +1,57 @@ +# Publish the patched TradeNote image to this fork's GitHub Container Registry. +# +# Uses the built-in GITHUB_TOKEN, so no registry secret needs to be configured. +# Builds docker/Dockerfile — the full build, not a derived FROM-image — so the +# Vue frontend is compiled from patched source too. That matters: the server and +# the web UI each carry their own copy of the netting logic, and a derived image +# would patch only the server, leaving a hand-import through the browser still +# merging accounts. +name: publish-image + +on: + push: + branches: [main, 'fix/**'] + tags: ['v*'] + workflow_dispatch: + +env: + REGISTRY: ghcr.io + IMAGE_NAME: ${{ github.repository }} + +jobs: + build-and-push: + runs-on: ubuntu-latest + permissions: + contents: read + packages: write + steps: + - uses: actions/checkout@v4 + + - uses: docker/setup-buildx-action@v3 + + - uses: docker/login-action@v3 + with: + registry: ${{ env.REGISTRY }} + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + - id: meta + uses: docker/metadata-action@v5 + with: + # ghcr requires a lowercase path; github.repository is already lowercase here. + images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} + tags: | + type=ref,event=branch + type=ref,event=tag + type=sha,format=long + type=raw,value=latest,enable={{is_default_branch}} + + - uses: docker/build-push-action@v6 + with: + context: . + file: docker/Dockerfile + push: true + tags: ${{ steps.meta.outputs.tags }} + labels: ${{ steps.meta.outputs.labels }} + cache-from: type=gha + cache-to: type=gha,mode=max From 5cf8322f4ed409a0a5722baca39b7a7fb7c288dd Mon Sep 17 00:00:00 2001 From: Don Nyambudzi Date: Tue, 18 Aug 2026 20:07:11 +0100 Subject: [PATCH 4/4] fix: account filter duplicated one account and omitted the others 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 Claude-Session: https://claude.ai/code/session_01Xxd7YyvYvqoKwTyxBcfdvG --- src/utils/addTrades.js | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/utils/addTrades.js b/src/utils/addTrades.js index 46acd59..7561f48 100644 --- a/src/utils/addTrades.js +++ b/src/utils/addTrades.js @@ -2101,6 +2101,16 @@ export async function useUploadTrades(param99, param0) { const results = await query.first(param99 === "api" ? { useMasterKey: true } : undefined); //console.log(" results "+JSON.stringify(results)) if (results) { + // Dedupe on write. The list is append-only and a journal that + // was imported before the fix above already carries repeats; + // without this it would keep saving them back. + const seenAccounts = new Set() + param = (param || []).filter(a => { + const v = a && a.value + if (v == null || seenAccounts.has(v)) return false + seenAccounts.add(v) + return true + }) results.set("accounts", param) //console.log("param 2" + JSON.stringify(param2)) if (param99 === "api") { @@ -2140,8 +2150,14 @@ export async function useUploadTrades(param99, param0) { if (!check) { let tempArray = currentUser.value.accounts let temp = {} - temp.value = tradeAccounts[0] - temp.label = tradeAccounts[0] + // Was tradeAccounts[0], which is the account being + // iterated ONLY when there is one of them. With two + // accounts in a journal every import re-added the first + // and never added the second, so the filter accumulated + // one duplicate per import and the second account was + // never selectable. + temp.value = element + temp.label = element tempArray.push(temp) updateTradeAccounts(tempArray, temp.value) }