feat(balance): multi-security entry UI + SecurityPicker (#214) #223

Merged
maximus merged 1 commit from issue-214-ui-multi-titres into main 2026-06-10 01:07:54 +00:00
Owner

Resolves #214

Stacked on #213 (base branch issue-213-reducer-dispatch).

What

Turns the detailed account snapshot variant into the real per-title entry surface, building on the minimal sub-rows from #213.

  • SecurityPicker (new): autocomplete combobox over the existing balance_securities catalogue + inline creation. Accepts any normalized symbol (UPPER/TRIM), no live ticker validation (price fetch is best-effort + separate). Emits SecurityPick {symbol, asset_type, name, isNew}; a stock/crypto toggle sets the asset class when creating a new symbol (default stock). Built on the CategoryCombobox UI idiom (ARIA listbox, keyboard nav, click-outside). Pure helpers filterSecurities / decideCreateOption exported + unit-tested (no jsdom harness).
  • SnapshotLineRow detailed sub-rows: labeled columns [title (SecurityPicker), quantity, price (+ existing PriceFetchControl), value (qty x price, read-only), book_cost, live unrealized gain]. Account value = displayed SUM of positions. Simple accounts unchanged.
  • useSnapshotEditor: new SET_HOLDING_SECURITY action + setHoldingSecurity callback (atomically sets symbol + asset_type + name, drops stale fetched-price attribution). Securities catalogue loaded in loadForDate, exposed as state.securities (refreshes after a save that creates a security).
  • i18n FR + EN under balance.snapshot.detailed.* (col.*, picker.*, book cost, unrealized gain) — no hardcoded UI text.
  • CHANGELOG EN + FR under [Unreleased].

Quality gate

  • npm run build (tsc + vite): green
  • npm test: green — 613 tests (+10 new in SecurityPicker.test.ts)

Notes for downstream (#215 / #216)

  • SecurityPicker API: securities, value, assetType, onSelect(pick: SecurityPick). SecurityPick = {symbol, asset_type, name, isNew}.
  • asset_type default on inline creation = stock (matches makeEmptyHolding); user-switchable via the create-row toggle.
  • Per-row latent gain is a live editor display only; the table drill-down latent gain stays #216.

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

Resolves #214 Stacked on #213 (base branch `issue-213-reducer-dispatch`). ## What Turns the *detailed* account snapshot variant into the real per-title entry surface, building on the minimal sub-rows from #213. - **`SecurityPicker`** (new): autocomplete combobox over the existing `balance_securities` catalogue + inline creation. Accepts any normalized symbol (UPPER/TRIM), **no live ticker validation** (price fetch is best-effort + separate). Emits `SecurityPick {symbol, asset_type, name, isNew}`; a stock/crypto toggle sets the asset class when creating a new symbol (default `stock`). Built on the `CategoryCombobox` UI idiom (ARIA listbox, keyboard nav, click-outside). Pure helpers `filterSecurities` / `decideCreateOption` exported + unit-tested (no jsdom harness). - **`SnapshotLineRow`** detailed sub-rows: labeled columns [title (SecurityPicker), quantity, price (+ existing `PriceFetchControl`), value (qty x price, read-only), book_cost, live unrealized gain]. Account value = displayed SUM of positions. **Simple accounts unchanged.** - **`useSnapshotEditor`**: new `SET_HOLDING_SECURITY` action + `setHoldingSecurity` callback (atomically sets symbol + asset_type + name, drops stale fetched-price attribution). Securities catalogue loaded in `loadForDate`, exposed as `state.securities` (refreshes after a save that creates a security). - **i18n** FR + EN under `balance.snapshot.detailed.*` (`col.*`, `picker.*`, book cost, unrealized gain) — no hardcoded UI text. - **CHANGELOG** EN + FR under `[Unreleased]`. ## Quality gate - `npm run build` (tsc + vite): green - `npm test`: green — 613 tests (+10 new in `SecurityPicker.test.ts`) ## Notes for downstream (#215 / #216) - `SecurityPicker` API: `securities`, `value`, `assetType`, `onSelect(pick: SecurityPick)`. `SecurityPick = {symbol, asset_type, name, isNew}`. - `asset_type` default on inline creation = `stock` (matches `makeEmptyHolding`); user-switchable via the create-row toggle. - Per-row latent gain is a live editor display only; the table drill-down latent gain stays #216. Generated autonomously by /autopilot run of 2026-06-06
maximus added the
status:review
autopilot:pending-human
labels 2026-06-06 17:40:02 +00:00
maximus added 1 commit 2026-06-06 17:40:03 +00:00
Turn the detailed-account snapshot variant into the real per-title entry
surface (building on the minimal sub-rows from #213):

- New SecurityPicker (src/components/balance/SecurityPicker.tsx): an
  autocomplete combobox over the existing balance_securities catalogue
  (loaded via listSecurities()) with inline creation. Accepts any
  normalized symbol (UPPER/TRIM) with NO live ticker validation — the
  price fetch is best-effort and separate. On pick/create it emits a
  SecurityPick {symbol, asset_type, name, isNew}; a stock/crypto toggle
  lets the user set the asset class when creating a new symbol (default
  'stock'). Built on the CategoryCombobox UI idiom (ARIA listbox,
  keyboard nav, click-outside). Pure helpers filterSecurities /
  decideCreateOption are exported and unit-tested (no jsdom harness).

- SnapshotLineRow detailed sub-rows: labeled columns
  [title (SecurityPicker), quantity, price (+ existing PriceFetchControl),
  value (qty x price, read-only), book_cost, live unrealized gain].
  Account value = displayed SUM of positions. Simple accounts unchanged.

- useSnapshotEditor: new SET_HOLDING_SECURITY action + setHoldingSecurity
  callback (atomically sets symbol + asset_type + name and drops the
  stale fetched-price attribution since the symbol changed). The
  securities catalogue is loaded in loadForDate and exposed as
  state.securities, so it refreshes after a save that creates a security.

- i18n: extended balance.snapshot.detailed.* (col.*, picker.*, book cost,
  unrealized gain) in FR + EN — no hardcoded UI text.

- CHANGELOG (EN + FR) under [Unreleased]: first user-visible surface of
  the per-title detail chain (#210-#213 were schema/service/reducer).

Build (tsc + vite) green; npm test green (613 tests, +10 SecurityPicker).

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

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

Verdict: APPROVE. No must-fix issues. The pure helpers are correct, the stale-price drop is sound end-to-end, i18n parity is exact, and the tests genuinely cover the load-bearing logic. A few non-blocking notes below.

Scrutinized hardest — all pass

SecurityPicker logic — CORRECT. decideCreateOption normalizes both the query and each catalogue symbol via normalizeSecuritySymbol, so an exact (normalized) match returns null — no casing-duplicate path. Typing aapl when AAPL exists: filter surfaces AAPL to pick, no create option offered. filterSecurities is case-insensitive over symbol+name, preserves catalogue order, empty→whole list. The create symbol is the normalized form, so a picker-created security collapses onto the same balance_securities row as the v14/v16 migration path (UPPER(TRIM(...))).

Stale-price drop — CORRECT, verified end-to-end. SET_HOLDING_SECURITY (useSnapshotEditor.ts:355) sets symbol+asset_type+name and nulls price_source / price_fetched_at. buildDetailedLines (useSnapshotEditor.ts:522-523) propagates those nulls into the saved SnapshotHoldingInput, and the service inserts price_source ?? 'manual'. A price fetched for the old ticker is therefore not mis-attributed to the new one. Good — this was the highest-risk item.

asset_type on create/pick — CORRECT. Default 'stock' matches makeEmptyHolding. Picking an existing security emits sec.asset_type (preserved, never forced to stock). The stock/crypto toggle only governs a newly-created symbol. Note (non-blocking): findOrCreateSecurity's UPSERT does ON CONFLICT DO UPDATE SET asset_type=$4 on every save, so the holding's asset_type is always written back — but since picking carries the stored value, an untouched existing security round-trips unchanged. No corruption.

PriceFetchControl reuse — CORRECT. categoryKind="priced" is right: the control early-returns null for any non-priced kind, so passing priced is what makes the fetch button render for detailed rows (additionally gated on useIsPremium()).

i18n — COMPLETE. FR + EN have identical key sets under balance.snapshot.detailed.* (col.*, picker.*, bookCost*, latentGain*); every t(...) key referenced in SecurityPicker.tsx / SnapshotLineRow.tsx is defined in both, including the dynamic picker.assetType.{stock|crypto}. No hardcoded UI text. Extends #213's namespace without duplicating.

CHANGELOG — accurate, no collision. This is a linear stack (#219→…→#223→#224→…→#227). #218 (#227) sits downstream of #223 and inherits this [Unreleased] entry by fast-forward; its own job is ADR/guide lines, not re-adding the #214 entry. No add/add conflict.

Tests — 10 real cases, no .skip/.only. filterSecurities (6) + decideCreateOption (4) cover empty/whitespace, case-insensitive symbol+name match, order preservation, no-match, normalized create, exact-match→no-create, and partial-match→create. This is the right surface to test given no jsdom harness (mirrors CategoryCombobox.test.ts).

Non-blocking notes

  1. Display-vs-persist sub-cent drift. The account total shown in SnapshotLineRow (detailedTotal, line ~91) is round(Σ raw qty×price), while the persisted aggregate is round(Σ round(qty×price)) (buildDetailedLines). These can differ by one cent in pathological cases (round-then-sum vs sum-then-round). It's documented intent ("account value = displayed SUM"), the service is authoritative and re-validates exactly, and the magnitude is ≤1¢ — fine to ship, just flagging the known asymmetry. The per-row read-only value (computedValue, raw qty×price .toFixed(2)) shares the same harmless rounding boundary.
  2. Dead i18n key. balance.snapshot.detailed.symbolPlaceholder is no longer referenced anywhere (the plain-text symbol input it fed was replaced by picker.placeholder). Harmless; tidy up in a later pass.
  3. PR body count nit. Body says "+10 new tests" — correct (10 it()); just noting an earlier read miscounted due to describe lines.

Clean, well-scoped link in the stack. Ship it.

## Adversarial review — PR #223 (Link 5/9, issue #214) **Verdict: APPROVE.** No must-fix issues. The pure helpers are correct, the stale-price drop is sound end-to-end, i18n parity is exact, and the tests genuinely cover the load-bearing logic. A few non-blocking notes below. ### Scrutinized hardest — all pass **SecurityPicker logic — CORRECT.** `decideCreateOption` normalizes *both* the query and each catalogue symbol via `normalizeSecuritySymbol`, so an exact (normalized) match returns `null` — no casing-duplicate path. Typing `aapl` when `AAPL` exists: filter surfaces `AAPL` to pick, no create option offered. `filterSecurities` is case-insensitive over symbol+name, preserves catalogue order, empty→whole list. The create symbol is the normalized form, so a picker-created security collapses onto the same `balance_securities` row as the v14/v16 migration path (`UPPER(TRIM(...))`). **Stale-price drop — CORRECT, verified end-to-end.** `SET_HOLDING_SECURITY` (`useSnapshotEditor.ts:355`) sets symbol+asset_type+name and nulls `price_source` / `price_fetched_at`. `buildDetailedLines` (`useSnapshotEditor.ts:522-523`) propagates those nulls into the saved `SnapshotHoldingInput`, and the service inserts `price_source ?? 'manual'`. A price fetched for the *old* ticker is therefore not mis-attributed to the new one. Good — this was the highest-risk item. **asset_type on create/pick — CORRECT.** Default `'stock'` matches `makeEmptyHolding`. Picking an existing security emits `sec.asset_type` (preserved, never forced to stock). The stock/crypto toggle only governs a *newly-created* symbol. Note (non-blocking): `findOrCreateSecurity`'s UPSERT does `ON CONFLICT DO UPDATE SET asset_type=$4` on every save, so the holding's asset_type is always written back — but since picking carries the stored value, an untouched existing security round-trips unchanged. No corruption. **PriceFetchControl reuse — CORRECT.** `categoryKind="priced"` is right: the control early-returns `null` for any non-`priced` kind, so passing `priced` is what makes the fetch button render for detailed rows (additionally gated on `useIsPremium()`). **i18n — COMPLETE.** FR + EN have identical key sets under `balance.snapshot.detailed.*` (`col.*`, `picker.*`, `bookCost*`, `latentGain*`); every `t(...)` key referenced in `SecurityPicker.tsx` / `SnapshotLineRow.tsx` is defined in both, including the dynamic `picker.assetType.{stock|crypto}`. No hardcoded UI text. Extends #213's namespace without duplicating. **CHANGELOG — accurate, no collision.** This is a linear stack (#219→…→#223→#224→…→#227). #218 (#227) sits downstream of #223 and inherits this `[Unreleased]` entry by fast-forward; its own job is ADR/guide lines, not re-adding the #214 entry. No add/add conflict. **Tests — 10 real cases, no `.skip`/`.only`.** `filterSecurities` (6) + `decideCreateOption` (4) cover empty/whitespace, case-insensitive symbol+name match, order preservation, no-match, normalized create, exact-match→no-create, and partial-match→create. This is the right surface to test given no jsdom harness (mirrors `CategoryCombobox.test.ts`). ### Non-blocking notes 1. **Display-vs-persist sub-cent drift.** The account total shown in `SnapshotLineRow` (`detailedTotal`, line ~91) is `round(Σ raw qty×price)`, while the persisted aggregate is `round(Σ round(qty×price))` (`buildDetailedLines`). These can differ by one cent in pathological cases (round-then-sum vs sum-then-round). It's documented intent ("account value = displayed SUM"), the service is authoritative and re-validates exactly, and the magnitude is ≤1¢ — fine to ship, just flagging the known asymmetry. The per-row read-only value (`computedValue`, raw `qty×price` `.toFixed(2)`) shares the same harmless rounding boundary. 2. **Dead i18n key.** `balance.snapshot.detailed.symbolPlaceholder` is no longer referenced anywhere (the plain-text symbol input it fed was replaced by `picker.placeholder`). Harmless; tidy up in a later pass. 3. **PR body count nit.** Body says "+10 new tests" — correct (10 `it()`); just noting an earlier read miscounted due to `describe` lines. Clean, well-scoped link in the stack. Ship it.
maximus changed target branch from issue-213-reducer-dispatch to main 2026-06-10 01:07:53 +00:00
maximus merged commit c4edfb0a35 into main 2026-06-10 01:07:54 +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#223
No description provided.