Skip to content

fix: count cross-account flips in the account-wide totals - #115

Merged
AntonyGarand merged 1 commit into
masterfrom
fix/account-wide-cross-account-flip-totals
Oct 21, 2026
Merged

AntonyGarand merged 1 commit into
masterfrom
fix/account-wide-cross-account-flip-totals

Conversation

@AntonyGarand

Copy link
Copy Markdown
Contributor

Problem

A player with many alts funneling earth runes across accounts reported wrong account-wide totals: alts bought runes at 4 gp, a main sold them at 5 gp — 1,907,078,609 bought and 1,283,457,544 sold in total. Profit ea (1 gp) and ROI (25%) looked right, but the totals were ~10x too small:

Panel value Before After
Total Profit 126.0M gp ~1.28b gp
Quantity Flipped 125,999,310 1,283,457,544
Avg. Profit ea. / ROI / Bought / Sold / Avg prices correct unchanged

Root cause

HistoryManager.countFlipQuantity (changed in 4374bfa) grouped the merged account-wide offer list by account and summed min(bought, sold) per account. For this player the alts never sold (min = 0 each) and the main had bought only 125,999,310 runes itself, so that within-account min is exactly the 125,999,310 "Quantity Flipped" shown in the report — every rune bought on an alt and sold on the main was invisible to the totals. getValueOfMatchedOffers caps revenue/expense at the same quantity, so Total Profit collapsed with it, while per-item ratios stayed correct.

Fix

Take min(items bought, items sold) across the whole tradelist again, so surplus inventory is shared between the player's own accounts in the account-wide view. Single-account views are unaffected (all their offers belong to one account).

  • getFlips still matches offers per account, so the trade history / flips tab is unchanged — only the aggregate totals (total profit, quantity flipped, ROI, and the profit/flip-count sort options) now aggregate across accounts.
  • This also removes the per-account grouping allocation: measured at the reported player's scale (~133k offers), the per-item stats math got ~4x faster (~26 ms → ~7 ms per recompute), which also benefits the sort comparators that recompute profit per comparison.
  • HistoryTotalsTest.accountWideQuantityWidensTheSumWithoutSharingUnmatchedInventory pinned the old per-account semantics; it now expects funneled items to match across accounts (renamed to accountWideQuantityMatchesItemsFunnelledAcrossAccounts). This is a deliberate behavior change — flagging it in case the old semantics were intentional for some workflow I'm not aware of.

Tests

  • New AccountWideCrossAccountFlipsTest reproduces the reported scenario through the real account-wide pipeline (per-account hydrated FlippingItems merged with FlippingItem::merge, stats computed exactly like FlippingItemPanel.updateLabels). Both total assertions failed on master with the exact reported numbers and pass now; the label test pins bought/sold/avg prices/profit-ea/ROI.
  • Full suite: 98 tests, 0 failures.

countFlipQuantity summed min(bought, sold) per account, so items bought on
one account and sold on another were never counted as flipped. Players who
funnel items across their own accounts (alts buying to bypass the ge limit,
a main selling) saw massively undercounted account-wide totals: for a
reported earth rune player (1.9b bought at 4gp across alts, 1.28b sold at
5gp on the main) the panel showed 125,999,310 flipped and 126M gp profit
instead of 1,283,457,544 flipped and ~1.28b gp profit.

- take min(items bought, items sold) across the whole tradelist again, which
  also drops the per-account grouping allocation (measured ~4x faster per
  recompute at that player's 133k-offer scale)
- getFlips still matches offers per account, so the trade history tab is
  unchanged; only the totals (profit, quantity flipped, roi, sorting) now
  aggregate across the player's accounts
- regression test assembles the reported scenario through the real
  account-wide merge pipeline; HistoryTotalsTest now expects funneled items
  to match across accounts
@coderabbitai

coderabbitai Bot commented Oct 21, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@AntonyGarand
AntonyGarand marked this pull request as ready for review October 21, 2026 15:10
@AntonyGarand
AntonyGarand merged commit 1341697 into master Oct 21, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant