feat(balance): per-security drill-down + latent gain (#216) #224

Merged
maximus merged 1 commit from issue-216-drilldown-gain into main 2026-06-10 01:07:57 +00:00
Owner

Resolves #216

Stacked on #223 (base issue-214-ui-multi-titres).

What

Surfaces unrealized (latent) gain on the EXISTING balance surfaces — no new visualization (decision 2026-06-04).

  • Service: AccountLatestSnapshot gains account kind + latest_snapshot_line_id. New getAccountLatentGainByLine(lineId) folds a line's holdings through the existing computeUnrealizedGain (reuses the book_cost=0 / NULL → N/A guard). New pure rollupLatentGain(accounts) aggregates by asset class, by envelope (vehicle_type, none bucket), and a grand total.
  • Hook (useBalanceOverview): prefetches per-detailed-account latent gain in parallel (failures isolated), exposes latentGainByAccount + latentGainRollup.
  • BalanceAccountsTable: expandable detailed rows → per-security value + latent gain %; a latent-gain column (shown only when a detailed account has one); a summary block aggregating latent gain by asset class / envelope.
  • BalanceOverviewCard: total latent gain line (hidden when no detailed accounts).

Guards

N/A rendered (never divide-by-zero) when book_cost is NULL or 0; partial-% flag when some positions lack a cost basis.

Scope

Native non-CAD currency display de-scoped this round (untestable while all securities are CAD). Modified Dietz return columns (#204) unchanged.

Tests

Focused unit tests for getAccountLatentGainByLine and rollupLatentGain (grouping, none envelope, null-% / unknown book_cost, empty). npm run build + npm test green (618 passing).

Note for #215

This PR also edits BalanceAccountsTable. It adds: a leading chevron column-cell idiom in the name cell (drill-down toggle, gated on drillable), a conditional latent-gain <th>/<td>, sub-rows after each main <tr> (rows now wrapped in a Fragment), and a colSpan computed as 5 + (hasLatentGain?1:0) + (showReturns?4:0). The per-row actions menu (where #215 adds “Détailler en titres”) is untouched and still the last <td>.

Generated autonomously by /autopilot run of 2026-06-06

Resolves #216 Stacked on #223 (base `issue-214-ui-multi-titres`). ## What Surfaces unrealized (latent) gain on the EXISTING balance surfaces — no new visualization (decision 2026-06-04). - **Service**: `AccountLatestSnapshot` gains account `kind` + `latest_snapshot_line_id`. New `getAccountLatentGainByLine(lineId)` folds a line's holdings through the existing `computeUnrealizedGain` (reuses the book_cost=0 / NULL → N/A guard). New pure `rollupLatentGain(accounts)` aggregates by asset class, by envelope (`vehicle_type`, `none` bucket), and a grand total. - **Hook** (`useBalanceOverview`): prefetches per-detailed-account latent gain in parallel (failures isolated), exposes `latentGainByAccount` + `latentGainRollup`. - **`BalanceAccountsTable`**: expandable detailed rows → per-security value + latent gain %; a latent-gain column (shown only when a detailed account has one); a summary block aggregating latent gain by asset class / envelope. - **`BalanceOverviewCard`**: total latent gain line (hidden when no detailed accounts). ## Guards N/A rendered (never divide-by-zero) when `book_cost` is NULL or 0; partial-% flag when some positions lack a cost basis. ## Scope Native non-CAD currency display **de-scoped** this round (untestable while all securities are CAD). Modified Dietz return columns (#204) unchanged. ## Tests Focused unit tests for `getAccountLatentGainByLine` and `rollupLatentGain` (grouping, `none` envelope, null-% / unknown book_cost, empty). `npm run build` + `npm test` green (618 passing). ## Note for #215 This PR also edits `BalanceAccountsTable`. It adds: a leading chevron column-cell idiom in the name cell (drill-down toggle, gated on `drillable`), a conditional latent-gain `<th>`/`<td>`, sub-rows after each main `<tr>` (rows now wrapped in a `Fragment`), and a `colSpan` computed as `5 + (hasLatentGain?1:0) + (showReturns?4:0)`. The per-row actions menu (where #215 adds “Détailler en titres”) is untouched and still the last `<td>`. Generated autonomously by /autopilot run of 2026-06-06
maximus added the
status:review
autopilot:pending-human
labels 2026-06-06 17:50:18 +00:00
maximus added 1 commit 2026-06-06 17:50:19 +00:00
Surface unrealized (latent) gain on the existing balance surfaces, no new
visualization (decision 2026-06-04).

Service:
- AccountLatestSnapshot gains account `kind` + `latest_snapshot_line_id`, so
  the table knows which rows are detailed and where their holdings live.
- getAccountLatentGainByLine(lineId) folds a line's holdings through the
  existing computeUnrealizedGain (shares the book_cost=0 / NULL -> N/A guard).
- rollupLatentGain(accounts): pure aggregation by asset class, by envelope
  (vehicle_type, 'none' bucket), and grand total; per-bucket % denominator is
  the known book_cost only (null when none), mirroring the aggregate guard.

Hook (useBalanceOverview):
- Prefetches per-detailed-account latent gain in parallel (failures isolated),
  exposes latentGainByAccount + latentGainRollup. Simple accounts skipped.

UI:
- BalanceAccountsTable: expandable detailed rows -> per-security value + latent
  gain %; a latent-gain column (shown only when a detailed account has one);
  a summary block aggregating latent gain by asset class / envelope.
- BalanceOverviewCard: total latent gain line (hidden without detailed accounts).
- N/A rendered (never divide-by-zero) when book_cost is NULL or 0; partial-%
  flag when some positions lack a cost basis.

Native non-CAD currency display de-scoped this round (untestable while all
securities are CAD). Modified Dietz return columns (#204) unchanged.

i18n: balance.latentGain.* in FR + EN. Focused unit tests for
getAccountLatentGainByLine and rollupLatentGain (grouping, 'none' envelope,
null-% / unknown book_cost, empty).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author
Owner

Verdict: APPROVE — no must-fix. Reviewed the full diff (729+/7-, 8 files) and the head-branch sources for the dependencies (computeUnrealizedGain, listHoldingsBySnapshotLine, migration v15, computeAccountReturn).

Correctness — verified sound

  • rollupLatentGain: each bucket's % denominator is SUM(known book_cost) only; total_book_cost === 0 → total_gain_pct = null (N/A) at both bucket and grand-total level, faithfully mirroring computeUnrealizedGain's aggregate guard. No double-counting within a bucket — fold() accumulates each account once per map. has_unknown_book_cost is OR-ed up. NULL vehicle_type → VEHICLE_NONE_BUCKET ('none'). The 4 rollup tests' expected math (stock 0/200=0, crypto 10/40, tfsa 30/140, grand 10/240, null-% bucket) all check out against the implementation.
  • No N+1 / scattering: detailed accounts are filtered (kind === 'detailed' && latest_snapshot_line_id != null) then fetched via Promise.all, each wrapped in try/catch → null. A single account's fetch failure is isolated — the overview still loads, that account just has no figure. Parallelism is safe (independent reads).
  • Drill-down: expansion renders the already-prefetched lg.holdings — no extra round-trip. Chevron gated on drillable = !!lg && lg.holdings.length > 0 (simple/empty rows never expand). colSpan = 5 + (hasLatentGain?1:0) + (showReturns?4:0) is exact — confirmed against the 5 unconditional columns (name, category, latestValue, periodDelta, actions). Row key correctly moved to the wrapping <Fragment>.
  • Modified Dietz (#204) untouched: computeAccountReturn and the 4 return <th>/<td> appear only as unchanged context — no +/- on the return path.
  • SQL change: the new latest_snapshot_line_id scalar subquery is identical in shape (WHERE l.account_id=a.id ORDER BY s.snapshot_date DESC LIMIT 1) to the existing latest_value/latest_snapshot_date subqueries → picks the same winning row, mutually consistent. a.kind AS kind is valid (migration v15 add kind ... CHECK (kind IN ('simple','detailed')) exists at this branch, inherited from the #223 base, not introduced here). Correlated-per-row, but matches the pre-existing pattern and the indexed columns it documents.
  • Gating: a detailed account WITH holdings always produces a latentGainByAccount entry even when the gain is 0 or all-unknown (renders +$0.00 with the partial ⚠), so a user with securities never sees an empty surface. hasLatentGain and drillable are derived from the same entry, so they can't disagree.
  • Summary-block decision: rendering the aggregate as a LatentGainSummary block below the flat table (vs grouped sections) DOES satisfy “aggregated latent gain by asset class / envelope” — it aggregates on both axes via byClass/byVehicle. The stated rationale (avoid collision with #215's grouped-sections rework) is sound. Not a gap.
  • Tests: no .skip/.only/fit/xit; mocks are real (mockSelect feeds listHoldingsBySnapshotLine, symbol carried through, has_unknown_book_cost asserted). Genuine.

Minor (non-blocking) — optional follow-ups

  1. A detailed account whose latest line carries zero holdings still yields a computeUnrealizedGain([]) all-zeros entry, which flips hasLatentGain true and emits a $0.00 bucket in byClass/byVehicle + a $0.00 grand total. Purely cosmetic and an unusual state; could be tidied by skipping holdings.length === 0 accounts from the rollup input.
  2. gain === 0 renders as green +$0.00 (treated as positive) consistently across the cell, summary, and overview total — cosmetic.

Clean, well-scoped, reuses existing surfaces and the shared book_cost guard as intended. Shipping-ready on top of #223.

## Adversarial review — PR #224 (Link 6/9, issue #216) **Verdict: APPROVE** ✅ — no must-fix. Reviewed the full diff (729+/7-, 8 files) and the head-branch sources for the dependencies (`computeUnrealizedGain`, `listHoldingsBySnapshotLine`, migration v15, `computeAccountReturn`). ### Correctness — verified sound - **`rollupLatentGain`**: each bucket's `%` denominator is `SUM(known book_cost)` only; `total_book_cost === 0 → total_gain_pct = null` (N/A) at both bucket and grand-total level, faithfully mirroring `computeUnrealizedGain`'s aggregate guard. No double-counting within a bucket — `fold()` accumulates each account once per map. `has_unknown_book_cost` is OR-ed up. NULL `vehicle_type → VEHICLE_NONE_BUCKET` ('none'). The 4 rollup tests' expected math (stock 0/200=0, crypto 10/40, tfsa 30/140, grand 10/240, null-% bucket) all check out against the implementation. - **No N+1 / scattering**: detailed accounts are filtered (`kind === 'detailed' && latest_snapshot_line_id != null`) then fetched via `Promise.all`, each wrapped in `try/catch → null`. A single account's fetch failure is isolated — the overview still loads, that account just has no figure. Parallelism is safe (independent reads). - **Drill-down**: expansion renders the already-prefetched `lg.holdings` — no extra round-trip. Chevron gated on `drillable = !!lg && lg.holdings.length > 0` (simple/empty rows never expand). `colSpan = 5 + (hasLatentGain?1:0) + (showReturns?4:0)` is **exact** — confirmed against the 5 unconditional columns (name, category, latestValue, periodDelta, actions). Row key correctly moved to the wrapping `<Fragment>`. - **Modified Dietz (#204) untouched**: `computeAccountReturn` and the 4 return `<th>`/`<td>` appear only as unchanged context — no `+`/`-` on the return path. - **SQL change**: the new `latest_snapshot_line_id` scalar subquery is identical in shape (`WHERE l.account_id=a.id ORDER BY s.snapshot_date DESC LIMIT 1`) to the existing `latest_value`/`latest_snapshot_date` subqueries → picks the same winning row, mutually consistent. `a.kind AS kind` is valid (migration v15 `add kind ... CHECK (kind IN ('simple','detailed'))` exists at this branch, inherited from the #223 base, not introduced here). Correlated-per-row, but matches the pre-existing pattern and the indexed columns it documents. - **Gating**: a detailed account WITH holdings always produces a `latentGainByAccount` entry even when the gain is 0 or all-unknown (renders `+$0.00` with the partial ⚠), so a user with securities never sees an empty surface. `hasLatentGain` and `drillable` are derived from the same entry, so they can't disagree. - **Summary-block decision**: rendering the aggregate as a `LatentGainSummary` block below the flat table (vs grouped sections) DOES satisfy *“aggregated latent gain by asset class / envelope”* — it aggregates on both axes via `byClass`/`byVehicle`. The stated rationale (avoid collision with #215's grouped-sections rework) is sound. Not a gap. - **Tests**: no `.skip`/`.only`/`fit`/`xit`; mocks are real (`mockSelect` feeds `listHoldingsBySnapshotLine`, symbol carried through, `has_unknown_book_cost` asserted). Genuine. ### Minor (non-blocking) — optional follow-ups 1. A detailed account whose latest line carries **zero holdings** still yields a `computeUnrealizedGain([])` all-zeros entry, which flips `hasLatentGain` true and emits a `$0.00` bucket in byClass/byVehicle + a `$0.00` grand total. Purely cosmetic and an unusual state; could be tidied by skipping `holdings.length === 0` accounts from the rollup input. 2. `gain === 0` renders as green `+$0.00` (treated as positive) consistently across the cell, summary, and overview total — cosmetic. Clean, well-scoped, reuses existing surfaces and the shared book_cost guard as intended. Shipping-ready on top of #223.
maximus changed target branch from issue-214-ui-multi-titres to main 2026-06-10 01:07:55 +00:00
maximus merged commit 1a4cab2e9b into main 2026-06-10 01:07:57 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: maximus/Simpl-Resultat#224
No description provided.