fix(dashboard): correct performance display for accounts with negative start value#759
fix(dashboard): correct performance display for accounts with negative start value#759
Conversation
…dation and sync During CSV validation (check_activities_import) and broker sync (prepare_activities_for_sync / build_asset_spec), resolve_symbol_quote was called once per activity row, causing N provider hits for a CSV with N rows of the same symbol. Add fetch_provider_quote_ccy, which caches the raw Option<String> provider result keyed by (symbol, mic, instrument_type). The cache is scoped to a single validation run or sync pass, so the provider is hit at most once per unique symbol. Per-row precedence resolution (resolve_quote_ccy_precedence) runs locally with actual per-row inputs after the cached lookup, keeping terminal_fallback and existing-asset currency correct for every row.
…t dedup The review step (check_activities_import) was computing idempotency keys using symbol@mic strings, while the apply step used the resolved asset UUID. On re-import, the review showed 0 duplicates (keys mismatched the DB), then apply correctly skipped them all — confusing UX with no data corruption. Two-part fix: 1. Store the resolved asset UUID on ActivityImport.asset_id during check_activities_import when the asset already exists. Falls back to symbol@mic for new assets (first-time import), where no DB key exists yet to match against. 2. Normalize quantity, unit_price, amount (abs values) and minor-unit currencies (GBp → GBP with factor conversion) in build_import_idempotency_key, matching the normalization that prepare_activities_internal applies before the apply-step key computation.
…art value - Return period_return: None when start_value <= 0 in both summary and full performance paths, serialising as null to the frontend - Show amber warning dot (with tooltip) on accounts with null return % but non-zero value; hide gain row for empty/zero-value accounts - Add NegativeAccountBalance health check via daily_account_valuation query - Trigger portfolio recalc when tracking mode switches Holdings→Transactions - Align group/filter/view-all button styles on dashboard (ghost + hover:bg-success/10) - Fix periodReturn type to number | null in types.ts and addon-sdk
![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.
Reviewed commit: 00a53b10ba
ℹ️ 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 "@codex address that feedback".
| (acc.totalGainLossPercent ?? 0) * | ||
| (Number(acc.totalValueBaseCurrency) / totalValueBaseCurrency), |
There was a problem hiding this comment.
When a member account has totalGainLossPercent === null (the new non-computable return case), this aggregation treats it as 0 and still weights it by that account's market value. That produces a seemingly precise group return that is mathematically wrong for mixed groups and can hide the fact that some accounts have unavailable returns (for example, one null-return account plus one positive-return account gets diluted instead of marked unavailable). Group return should stay null (or be computed only from known-return accounts with reweighting), not coerced to zero.
Useful? React with 👍 / 👎.
1 participant
Summary
daily_account_valuation. This causedperiod_return = gain / negative_start = -283%and similar nonsensical values on the dashboard.period_returnnow returnsNone(→nullin JSON) whenstart_value ≤ 0, in both the summary and full-history calculation paths. Frontend type updated tonumber | null.NegativeAccountBalanceconsistency check queriesdaily_account_valuationfor accounts with negative total value and surfaces a warning in the health panel.ghost+hover:bg-success/10to match the "View All" button style.Test plan
–gain row and amber dot on dashboard; tooltip reads "Return % unavailable — activity history may be inconsistent (e.g. buys without deposits)"total_valuein historyperiodReturn: nulldoes not break addon-sdk consumers