Add total P&L support for single account view#45
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| if (balanceSummary.combined) { | ||
| const rates = buildCurrencyRateMapFromSummary(balanceSummary, baseCurrency); | ||
| Object.entries(balanceSummary.combined).forEach(([key, entry]) => { | ||
| const normalized = normalizeCurrencyCode(key); | ||
| if (!normalized) { | ||
| return; | ||
| } | ||
| const equity = entry && entry.totalEquity; | ||
| if (!isFiniteNumber(equity)) { | ||
| return; | ||
| } | ||
| let convertedDeposits = 0; | ||
| totalsMap.forEach((amount, sourceCurrency) => { | ||
| convertedDeposits += convertAmountToCurrency(amount, sourceCurrency, normalized, rates, baseCurrency); | ||
| }); |
There was a problem hiding this comment.
When computing total P&L the code builds a rate map only from currencies that appear in the current balance summary and then converts every historical deposit using those rates. If a user previously deposited funds in a currency they no longer hold (so that currency is absent from the balance summary), convertAmountToCurrency receives no rate for that currency and returns 0, effectively dropping those deposits from convertedDeposits. The resulting combined totalPnl will be overstated because past deposits in missing currencies are never subtracted. Consider an account that once deposited USD, converted everything to CAD, and now only holds CAD: total P&L will ignore the USD contributions entirely. The conversion should fall back to a reasonable rate (e.g., derived from the activity currency or base currency) instead of skipping the amount when the currency is absent from the current balances.
Useful? React with 👍 / 👎.
9ea89d3 to
c3b5f5f
Compare
c7f2145 to
c8636e3
Compare
1 participant
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68dfba66908c832da9e32109d1dcc46f