Merge first: Wrap report tables on phones and finish the mobile-table follow-ups - #1340
Merged
Merged
Conversation
The Category sort compared the raw group enum (NEED/WANT/SAVING/null), so a localized reader saw rows ordered by the English enum rather than by the labels on screen, and the null group sorted to the bottom instead of beside its Uncategorized label. Sort on getGroupLabel, memoized so the order re-derives on a locale change. A regression test asserts the Uncategorized row precedes Want when sorting Group ascending. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
PHONE_HEADER_CLASS and CAPTION_CLASS were centralized into components/ui/Table.tsx, but nothing stopped a report re-declaring them locally and drifting again (three copies had already lost a tracking token before they were shared). Add a source scan that fails on a local const re-declaration of either, outside Table.tsx. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
Convert the table view of SpendingByCategoryReport to the phone-card layout (mechanism A): below `sm` the table, its row groups and each row become `block`/`grid` while staying an ordinary table from `sm` up, so nothing scrolls sideways on a phone. Layout: each row wraps into a two-track, two-line grid card. The category name (unbounded, so it wraps unclamped with `break-words sm:break-normal`) takes the whole of line 1; the amount and the % of Total share line 2, each captioned with `CellLabel` reusing the existing header key. Money cells carry `whitespace-nowrap` so a space-grouping locale cannot break a figure. Explicit `col-start`/`row-start` and restated `role` table/rowgroup/row/cell/columnheader put back the semantics a `display` restyle strips. The sortable header becomes a phone-only strip of the same SortableHeader chips, rendered (with the desktop header row) from one exhaustive mapped-type record so the two can never list different fields. The footer wraps the same way; every column has a total, so no cell leaves the DOM and none owes an `aria-colindex`. Rows stay clickable, as today. The `sm`-and-up output is unchanged apart from the phone-only additions and `whitespace-nowrap` on the figure cells. No new i18n keys. Adds SpendingByCategoryReport.mobileWrapped.test.tsx (pins placement, roles, captions, header control count, the phone sort strip, the footer, and row clickability; fails on the pre-conversion markup) and updates the existing test's header-click to address the column header row by position. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
Convert the Income by Source report table to the phone-card layout used across the other reports (mechanism A, CSS single tree). Below `sm` the table becomes a block, each row a two-column grid: the source name beside its amount on line 1, the % of Total beneath the amount on line 2; from `sm` up the output is the ordinary table, identical to today (the one added property is `whitespace-nowrap` on the figure cells). - `thead`/`tbody`/`tfoot` go `block` below `sm` (`sm:table-*-group`); every `<tr>` a `grid grid-cols-2 ... sm:table-row`; every `<td>`/`<th>` carries an explicit `col-start`/`row-start` and restated ARIA roles (`table`/`rowgroup`/`row`/`cell`), since restyling `display` strips the implicit table semantics. - The three sortable columns are one exhaustive mapped-type record whose `Object.values` render BOTH the desktop column header row and a phone-only sort strip, so neither can strand a field. - Each figure cell carries a phone-only `CellLabel` caption reusing the existing column header key (colAmount, colPercentOfTotal); the source name and its colour dot are self-describing and carry none. The name wraps unclamped (`break-words sm:break-normal`) in a `min-w-0` track. - Rows stay clickable for real categories (opens that category's transactions) and inert for the uncategorised row, exactly as today. - Footer keeps all three columns at every width, so no `aria-colindex` is owed. Follows the worktree's existing pattern: local `SortColumn` / `SortColumnsByField` / `HEADER_CLASS` / `PHONE_HEADER_CLASS` / `CAPTION_CLASS` / `FIGURE_CELL` declarations, importing only `CellLabel` from Table.tsx (Table.tsx here exports only that of the wrapped-table chrome). No new i18n keys. Adds IncomeBySourceReport.mobileWrapped.test.tsx pinning cell placement, restated roles, caption-to-column association, header control count, the phone sort strip listing every field, footer placement, and the clickable/inert row contract. It fails on the pre-conversion component. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
The Income by Source and Spending by Category conversions were written against the pre-consolidation base and re-declared PHONE_HEADER_CLASS and CAPTION_CLASS locally (identical values). Import them from components/ui/Table.tsx instead, as every other converted report does, so the consolidation guard passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
The per-security yield table is the report's one wide table (five columns:
Security, 12M Dividends, Market Value, Yield, Frequency), so below `sm` it
wraps each row into a two-line, three-column grid card (mechanism A, one CSS
tree) instead of scrolling sideways. Line 1 carries the security identity
(spanning two tracks) and the headline yield; line 2 carries the trailing-12m
dividends, market value and payout frequency, one track each. Every cell states
its own `col-start`/`row-start` and restates its ARIA role (restyling `display`
strips the implicit table semantics), each bare figure carries a `CellLabel`
reusing the column's existing header key, and money cells keep
`whitespace-nowrap`. From `sm` up the resolved output is today's table, the one
deliberate difference being that nowrap. The sort controls return below `sm` as
a phone strip of the same `SortableHeader` chips, rendered -- with the column
header row -- from one exhaustive `{ [K in YieldSortField]: ... }` record so the
two rows can never list different fields. Shared chrome (`PHONE_HEADER_CLASS`,
`CAPTION_CLASS`, `CellLabel`, the `SortColumn`/`SortColumnsByField` types) is
imported from `@/components/ui/Table`; `MONEY_CELL`/`HEADER_CLASS` stay local.
The year-over-year and frequency views have three columns each, fit a 320px
phone already, and are left as ordinary tables.
Adds DividendYieldGrowthReport.mobileWrapped.test.tsx pinning cell placement,
the restated roles, the caption-to-column association, the two header rows'
control counts and equality, phone-strip sorting, and the rows staying inert.
Two row-count assertions in the existing DividendYieldGrowthReport.test.tsx now
count body rows, because the wrapped table's header is legitimately two rows.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
Convert both history tables in SecurityPerformanceReport to the phone-card
layout (mechanism A, one CSS tree), mirroring the converted sibling reports.
Below `sm` each table becomes a block and each row wraps into a grid card so
every column fits a phone with no horizontal scroll; from `sm` up the resolved
output is identical to today (each cell restores its own `sm:px-4 sm:py-3
sm:text-sm`).
- Transaction History (6 columns): a three-track, two-line grid -- line 1 is
the date (identity), the action pill and the total (headline); line 2 is the
account, the share count and the price.
- Dividend History (4 columns): a two-track, two-line grid -- date + amount on
line 1, account + type pill on line 2 -- plus a footer that wraps the same
way, "Total Dividends" beside the total. The footer keeps its desktop
`colSpan={3}`, so its cells state `aria-colindex`.
Both sort-control sets come back below `sm` as a phone-only strip of the same
`SortableHeader` chips, rendered from one exhaustive mapped-type record per
table whose `Object.values` both header rows render. Every bare figure carries a
`CellLabel` reusing the column's existing header key (no new i18n keys); the
date and the self-describing pills carry none. Money and share cells keep
`whitespace-nowrap`; the account name wraps. Restyling `display` strips the
implicit table semantics below `sm`, so the ARIA roles are restated.
Adds SecurityPerformanceReport.mobileWrapped.test.tsx pinning cell placement,
roles, caption-to-column association, the header control counts, sorting from
the phone strip, the footer's colspan/aria-colindex, and the rows staying inert.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
WMP
marked this pull request as draft
September 9, 2026 18:45
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
Contributor
Author
|
@kenlasko : Feel free to merge the code as it is. I won’t have more Claude tokens available for another 12 hours. That said, it would be good to finish these three remaining items:
|
WMP
marked this pull request as ready for review
September 9, 2026 19:53
Convert the fixed-column Income by Security table to the below-`sm` phone-card layout (mechanism A, CSS single tree), mirroring RealizedGainsReport's security summary: a two-column, three-line grid card with the identity wrapping unclamped, every figure carrying a `CellLabel`, and the sort controls returned as a phone-only strip built from one exhaustive `SortColumnsByField` record. The Monthly and Daily tables are left as fixed tables: their column count varies at runtime with the `visibleSeries` toggles (four to seven columns), so they are not fixed-column and are out of scope for mechanism A's static grid. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
Convert the three fixed-column Monte Carlo tables to the below-`sm`
phone-card layout (mechanism A, single CSS tree resolving identically at
`sm`+): the year-by-year results table (7 columns), the per-account
holding-stats table (5 columns), and the performance-summary statistics
table (6 columns). Each row wraps into a two-track grid card with every
bare figure captioned by its existing column key, money never wrapping,
and the ARIA table roles restated since restyling `display` strips them.
The holding-stats card now shows the security name that today hides below
`sm`. All three keep their `divide-y` chrome (still recorded in the
ui-conventions baseline) and their injected-formatter / `useTranslations`
patterns; no new i18n keys.
Adds a `*.mobileWrapped.test.tsx` beside each and fixes the now-ambiguous
`getByText('50th Percentile')` in the existing performance-summary test
(the key is reused as a per-row caption).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
…t throw
B4: the branch wrote the same Enter/Space handler and the same four
focus-ring utilities out by hand in three places (`ui/SortableHeader`,
`SecurityTypeAllocationReport`, `InvestmentPerformanceReport`), and
near-copies predate them. Both halves now live in
`ui/interactive-row.ts` as `INTERACTIVE_ROW_FOCUS_CLASS` and
`activateOnKey(handler)`; the three call sites compose them. The ring
string is byte-identical to what they spelled out, so nothing renders
differently at any width, and each of the three already had Enter, Space
and non-activating-key cases -- which is why the suite staying green is
evidence of an identical refactor rather than an untested one.
`interactive-row.guard.test.ts` scans production sources for a
hand-rolled Enter+Space block in either the negated-early-return or the
positive-`if` form, and for the inset ring's `outline-offset-[-2px]`
fingerprint. Proven to fail on the original mistake (both halves, with
the right line numbers). Three un-converted near-copies outside this
change are recorded with a reason in a shrink-only list whose staleness
check fails an entry once its file is fixed; the ring scan needs no list.
B2: `formatMonth` ran `padStart` on the split result before any format
branch, so `formatMonth('undefined')` -- what a recharts tooltip's
optional `label` becomes through `String(label)` -- threw a TypeError
inside a tooltip's render and blanked the report subtree beneath it. It
now parses a leading `YYYY-MM` (a real 1-12 month, optionally followed by
the rest of an ISO date, as the payoff-date callers pass) and hands back
anything else unchanged, `''` for a non-string. Guarding the helper fixes
all its call sites at once. The new cases each threw before the guard.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
…gures in the reader's locale B1: `SecurityPerformanceReport`'s dividend total was accumulated with `dividendTx.reduce((sum, tx) => sum + Math.abs(tx.totalAmount), 0)` -- root `CLAUDE.md`'s own WRONG example -- in the table footer and again, independently, in the PDF export. Five plausible dividend amounts reach 389.46000000000004, so the footer disagreed with the sum of the figures printed above it. Both surfaces now call one `sumDividends`, which coerces each row with `Number(...)` (a `decimal(20,4)` crosses the wire as a string; the old expression coerced only because `Math.abs` does) and sums through `sumMoney` from `@/lib/format`, the repo's integer ten-thousandths helper. Four new cases fail on the original expression -- the test's money formatter prints a value money cannot hold instead of rounding the drift away, so `$389.46000000000004` is visible in the DOM. B3: `SecurityPerformanceReport` (both tables and the PDF) and `InvestmentTransactionHistoryReport` were left behind by the migration that moved three sibling reports onto `useDateFormat().formatDate` and `formatShareQuantity`, so a `de` reader saw `Jan 5, 2026` on one report and their own arrangement on the next, and a share count printed as the raw wire string `10.0000`. Both now go through those seams; the CSV date stays ISO deliberately (a machine reads it, and every unconverted sibling writes ISO), while the PDF -- read by a person -- gets the formatted date and count. `report-locale.guard.test.ts` is the durable half: it scans `src/components/reports/` for a date-fns `format(...)` with a month-NAME pattern (leaving `formatChartDate`'s identical token and every ISO pattern alone, with positive and negative controls for both) and for a `toFixed` rendering a share count in either the receiver form or the 4dp form. Proven to fail on the original mistakes. Three reports outside this change are recorded in a shrink-only baseline whose staleness check fails an entry once its file is fixed. B5: `KNOWN_SECURITY_TYPES` hand-wrote the five codes `TYPE_COLOURS` already keys, so a sixth type added to one gave either a slice captioned with the raw enum name or a missing-message key. The colour record is now the single declaration (`keyof typeof`), and the i18n guard checks the third party no type can reach -- the `dashboard` catalog's `types.*` keys, in both directions. B6: the spanning dividend-footer cell restates its `colSpan` as `aria-colspan`, for the same reason it restates `role="cell"`: below `sm` it is not a `table-cell`, so the span the table layout carried is gone. No `aria-colcount`: no converted sibling declares one. B7: deleted the orphaned `CAPTION_CLASS` JSDoc left over `TYPE_COLOURS`, and corrected two width-measurement comments in `InvestmentTransactionHistoryReportParts` that still described the `toFixed(4)` and English-date renderings this change replaced. Also added the PDF/CSV quantity case the suite had no coverage for at all -- formatted for the reader, a plain number for the spreadsheet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
`outline-offset-[-2px]` now appears nowhere else in the tree, and Tailwind v4 emits a utility only for a class it finds in a source file, so the constant's move depends on automatic source detection covering `.ts`. Verified by compiling `globals.css` through `@tailwindcss/postcss` against the new module rather than assumed, and recorded so a future reader does not move the constant back into a `.tsx` out of doubt. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
…them The B3 date scan's first draft matched the call with a regex, and its own positive control caught what that costs: `'MMM d, yyyy'` contains a COMMA, so splitting the argument list without knowing about string literals cut the pattern in half, read `yyyy'` as the last argument, and reported the directory clean while `DuplicateTransactionReport` held the exact call the guard was written for. A paren inside a literal misleads a depth count the same way, and a bounded-wildcard alternative had the opposite failure -- running past the closing paren to borrow the NEXT call's token, so `format(d, 'yyyy-MM-dd')` beside `formatChartDate(e, 'MMM')` read as one offender. It now reads the balanced argument list the way a parser does, skipping string literals in both the depth count and the argument split. Ten control cases pin it: the two shapes this change removed, a call nested two deep, one split across lines with a trailing comma, the localizing chart helper with the identical token, an `Intl` formatter's own `.format(`, two ISO patterns, and the borrow-the-next-token false positive. Re-verified against a scratch offender holding all three date shapes: all three are reported, at the right lines. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
Package A of the reviewed follow-ups to the phone-wrapping conversion. Nine items over six report components; the CSV one is a returning defect and the keyboard one is a WCAG failure the whole suite was green over. A1 -- The Uncategorized CSV amount column is a number again. The conversion exported `formatCurrency(tx.amount, tx.currencyCode)`, and `csv-export.ts`'s injection guard tab-prefixes any value opening with `-` that its `NUMERIC_VALUE` test cannot read as a number. That test admits digits, separators and currency SYMBOLS, so a currency whose narrow symbol is written in LETTERS falls outside it: `-zl 1,234.56`, `-CHF 1,234.56`, `-kr 1,234.56`, `-R$1,234.56` and pl-PL's `-1234,56 zl` are all tab-prefixed, Excel stores them as text and the column stops adding up. That is issue kenlasko#1134 returning. The value is now `Number(tx.amount)` -- the coercion because a `decimal(20,4)` crosses the wire as a string whatever the type says, and a string never takes the guard's `typeof value === 'number'` path. A2 -- The three converted CSV date columns are ISO again (`yyyy-MM-dd`, via `format(parseLocalDate(...))` as every unconverted sibling export writes): Uncategorized, Recurring Expenses and Bill Payment History had been switched to the reader's locale format. A CSV is machine-read, so two readers exporting the same rows must get one file, and a localized date is ambiguous and sorts lexicographically wrong. A1 and A2 go through one mechanism: `getExportData` takes an `ExportSurface` (`'csv' | 'pdf'`) and only the cells whose rendering depends on their reader branch, so the two exports cannot come to hold different columns. The PDF is a reading surface and keeps the reader's own number and date formats. In Recurring Expenses the surface reaches the cell through the column record's `csvValue`, which already owned the export's order. A3 -- Six clickable rows are keyboard-operable. Each was a `<tr>` with `cursor-pointer` and an `onClick` and no `tabIndex` and no `onKeyDown` (WCAG 2.1.1). They now take `tabIndex` and `activateOnKey` from the one shared `ui/interactive-row` module rather than a per-report copy of the handler. Where the click is conditional (Income by Source, Spending by Category, Recurring Expenses) the tab stop, the focus ring and the key handler appear only when the click does something: a focus stop that does nothing on Enter is one the reader has to escape. A4 -- Income by Source and Spending by Category import `SortColumn` / `SortColumnsByField` from `ui/Table` as eleven siblings do, instead of re-declaring them with ~25 lines of duplicated prose. A5 -- Income vs Expenses imports `CAPTION_CLASS` instead of inlining `"sm:hidden"` at its eight `CellLabel` caption sites. A6 -- Income vs Expenses' column record is `TableSortColumnsByField` rather than a plain `Record<>`: a `Record` lets an entry name a DIFFERENT field, which compiles into a duplicate React key, a header sorting by the wrong column and one unsortable column, none of which a label-comparing test can see. A7 -- The Frequency column sorts ordinally. It compared the LOCALIZED label, so ascending gave "Every 2 Weeks, Irregular, Monthly, Occasional, Weekly" -- alphabetical, and a different order in every language. It now compares position in `RECURRING_EXPENSE_FREQUENCIES`, which is declared in true frequency order for this. Both readers of a frequency also became defensive, because during a rolling deploy an older backend sends a code this build has no entry for: the badge takes IRREGULAR's neutral classes rather than putting the literal string `undefined` in the class attribute, and the label falls back to the raw code rather than rendering the `reports.recurringExpenses.frequency.<code>` key path on screen. An unknown code sorts after every known one rather than taking `indexOf`'s `-1` and claiming to be more frequent than WEEKLY. A8 -- Spending by Category keeps a `CELL_PLACEMENT` record, as its siblings do, instead of writing `col-start-1 row-start-2` and `col-start-2 row-start-2` out twice each; the footer now takes a data row's placement by construction. A9 -- The orphaned doc comments left behind when `CAPTION_CLASS` and `PHONE_HEADER_CLASS` moved into `ui/Table.tsx` no longer make false claims about the declaration under them. The stray `/** Every caption in a wrapped cell is phone-only. */` above the exported component in Uncategorized, Bill Payment History and (over `FREQUENCY_BADGE_CLASS`) Recurring Expenses is gone, and the phone-strip essays that had merged into the next constant's comment -- in Bill Payment History, Uncategorized and Income vs Expenses -- are re-attached to the phone sort strip they describe. Tests. Five expectations asserted the defects A1 and A2 fix (the CSV amount as `EUR -50.00`, the CSV date as the reader's format) and are corrected, each with the reason on the record; the sibling PDF assertions keep the localized strings and pin the other half of the split. A2 and A1 gain the assertions the suite lacked: the CSV amount is `typeof number`, the CSV date is ISO, and a new Recurring Expenses case holds the PDF's localized date against the CSV's ISO one from the same record. A3 and A7 changed behaviour with the suite staying green, which meant the suite had no case for either. Added: a keyboard-activation case per report (`tabIndex`, Enter, Space, an unclaimed key ignored, and for the three conditional rows that a non-clickable row is not a tab stop), an ordinal-sort case whose fixture is in neither frequency nor alphabetical order, and an unknown-frequency case. Each was checked to fail against the pre-fix code. No new i18n keys (the unknown-frequency label is the raw code). `ui/interactive-row` is Package B's module and is not on this base yet, so the six imports resolve only once B lands. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
The phone-wrapping conversion replaced the server-formatted `MonthlyBillTotal.label` with `useDateFormat().formatMonth(entry.month)`. That is the wrong helper for a chart month axis: `formatMonth` renders the user's month-and-year PREFERENCE (`2026-01`, `01/2026`, `Jan-2026`), so a numeric tick lands where the axis previously read `Jan 2026`. The convention for a chart month marker is `useChartDateFormat()` with `'MMM yyyy'`, which localizes the month NAME -- `MonthlySpendingTrendWidget` and `DividendIncomeReport` are the worked examples. This is item C4 of the reviewed list. C4 belongs to the package that owns the budget charts, but this one call site lives in a Package A file, so that package was told to leave it -- which left it owned by nobody. Committed separately for that reason: drop this commit if the month-axis convention should land as one change with the other five call sites instead. The existing test asserted `preferred-month:2025-01` on the chart's labels, so it moves onto a `useChartDateFormat` mock. The suite still mocks `formatMonth`, because `formatDate` beside it is what the Last Payment column reads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
Four surfaces printed a figure whose inputs had just been marked partial beside them, so one screen reported the same accumulation as both whole and holed: - Credit Utilization's overall ratio -- the table footer under a header reading "Total" and the donut centre had no marker at all, and the summary card's was a hand-rolled `aria-hidden` asterisk with no `sr-only` twin, so no surface gave a screen reader any signal. `lib/credit-utilization.ts` already states the contract: a non-empty `missingCurrencies` means the ratio describes only the included accounts. - Geographic Allocation's region and exchange footers printed the same `totalValue` the summary card marks, bare. - Dividend Yield Growth's portfolio yield carried the same hand-rolled marker, beside two `PartialTotal`-wrapped figures. Not on the fix list and owned by no package; left in place it would have been the one offender the new guard had to grandfather. All of them now go through `PartialTotal`, which renders the symbol, the `sr-only` suffix and the currency-naming tooltip together -- that is the point of not writing the marker by hand. The donut's overlay stays click-through, so only that one figure takes pointer events back, or the explanation would be unreachable by mouse. Geographic's COUNTRY footer is deliberately left unmarked, against the letter of the fix list: it prints `countryResp.totalPortfolioValue`, a server-side look-through aggregate, while `missingCurrencies` describes the client-side conversion of the holdings. Attaching those gaps to that number is the "flag from another aggregate" mistake, and `CountryWeightingResult` reports no completeness of its own. The component says so where the figure is rendered, and a test pins it. Guarded two ways. `src/test/partial-total-marker.guard.test.ts` scans production sources (comments stripped) and fails ANY hand-drawn `aria-hidden` asterisk outside `PartialTotal.tsx`, because a marker is four tokens of JSX and writing them is quicker than the import -- it fires on all three shipped offenders, and its own prose is proven not to trip it. The per-report assertions cover what a scan cannot see: a figure with no marker where its inputs were partial. Both extended tests fail on the original code (verified by reverting each site). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
…he chart formatter Two halves of one defect, which is why they are one commit: the Budgets area had two reports still receiving a server-rendered English month label, and every month AXIS in the area was rendering its key with the formatter meant for a table column. C3 -- the migration `budget-trend-reports.service.ts` completed on this branch stopped short of `budget-health-reports.service.ts`, so `SavingsRatePoint.month` and `HealthScoreHistoryPoint.month` were still `Mmm YYYY` built from a local `MONTH_NAMES` array. Consequences on two reports in the same area as the migrated ones: English months in all 22 locales, and -- because a label is text -- a client sort that ordered Apr, Aug, Dec, Feb, Jan. Both services now emit `monthKey` through `formatMonthKey`; the field is renamed on the DTO, in `frontend/src/types/budget.ts`, and at every consumer; the month columns render through `useDateFormat().formatMonth` and SORT on the key. API shape change, both layers, no alias (the fix list records emit-both-for-one-release as a rejected maintainer decision): SavingsRatePoint.month: string -> monthKey: string (YYYY-MM) HealthScoreHistoryPoint.month: string -> monthKey: string (YYYY-MM) The interfaces live in `budget-reports.service.ts`, which the package did not list; nothing else declares them, and no other package owns it. C4 -- `formatMonth` follows the user's date-format preference, so on an axis it renders `2026-01` / `01/2026` / `Jan-2026`: a numeric tick where the label used to read `Jan 2026`. The convention for a chart month is `useChartDateFormat` with `MMM yyyy`, which localizes the month NAME. The step between a `YYYY-MM` key and that formatter is a parse the callers kept getting to make, so it is one hook -- `useChartMonthFormat` -- and it returns an unparseable key unchanged rather than throwing, because `Intl.DateTimeFormat.format(Invalid Date)` throws a RangeError and recharts types a tooltip `label` as optional, so a throw inside a tick or tooltip render blanks the report. That is this helper's own safety, not a second copy of the `formatMonth` guard Package B is adding. Five charts moved onto it (BudgetTrendChart, BudgetCategoryTrend, BudgetTrendReport, BudgetVsActual's bar and variance charts, plus the two C3 reports' charts), tooltips included: a tooltip that named the month differently from the tick it hovers is the same defect one pixel away. Two green-suite findings, both closed here rather than noted: - the savings-rate spec never asserted the month at all, so shipping English to 22 locales was invisible to it. Its new case is clock-free: it checks the key's shape, that the keys sort chronologically as strings, that they are consecutive calendar months, and that `month` is gone rather than aliased. - `BudgetVsActualReport.test.tsx` asserted only the axis COUNT, so changing the formatter left it green. It now asserts what each axis and tooltip renders, against a mock deliberately distinguishable from the table formatter's. Where a label sort had to be disproved, the fixture labels are anti-chronological in alphabetical order (Zulu for January, Yankee for February), following `BudgetVsActualReport.test.tsx`'s own pattern: the row order is then only correct if the sort reads the key. The contract test is extended from two shapes to four and now strips comments before matching -- these interfaces are where the rule has to be explained, and the explanation must be free to name the field it replaced. It also fails if either service regrows a `MONTH_NAMES` table or stops calling `formatMonthKey`. Also in these files: three orphaned JSDoc comments left by the `CAPTION_CLASS` move, deleted (they documented the declaration that happened to follow them), and `SavingsRateReport`'s plain `Record<>` column map switched to the key-tied `TableSortColumnsByField`. Backend changes are UNVERIFIED: `backend/node_modules` does not exist in this environment, so neither Jest nor tsc can be run there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
`MonthlyBillTotal.label` was computed on the server with
`toLocaleDateString("en-US", { month: "short", year: "2-digit" })`. The
only consumer, `BillPaymentHistoryReport`, spreads the entry and
overwrites `label` with its own localized rendering of `month`, so the
field was dead payload -- and a dead field called `label` is worse than
no field at all: it is exactly what the next consumer reaches for,
shipping English months to 22 locales without touching the server.
Removed from the DTO, from `frontend/src/types/built-in-reports.ts`, and
from the service, which now accumulates a plain `Map<string, number>`
because the label was the only reason the value was an object.
API shape change, both layers:
MonthlyBillTotal.label: string -> removed (month: string stays; the
client formats it)
Two assertions rather than one, because a removed field comes back by
being re-added, not by being read: the service spec pins the exact key
set of a monthly total, and the cross-layer contract test checks both
declarations for `month` + `total` and refuses `label` or any renamed
display string (`monthLabel`, `displayMonth`, ...), plus any
`toLocaleDateString` in the service that builds them. That test now
strips comments before matching, since the two declarations are where
the rule is documented and the documentation has to name the field it
removed.
`BillPaymentHistoryReport`'s own suite is unaffected (23 passing): it
never read the server's label. That report's `chartData` label still
uses the table formatter where the chart formatter belongs, which is
Package A's file to change.
Backend changes are UNVERIFIED: `backend/node_modules` does not exist in
this environment, so neither Jest nor tsc can be run there.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
`src/test/intl-harness.guard.test.ts` fails a NEW test importing `render` or `renderHook` from `@testing-library/react`: RTL's own are unwrapped, so the tree gets no intl provider and every translated string resolves to its key. The new `useChartMonthFormat` test was on the raw import. Same call, wrapped providers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
…FX-001 actually enforces
D1. The guard named as INV-FX-001's mechanism was written from the shape of the
two defects it shipped with -- `?? amount` at the end of a line, `: 1` beside a
rate lookup -- so `report-currency.service.ts`'s
if (result == null) { this.logger.warn(...); return amount; }
was never a candidate for it, and `data-quality-reports.service.ts` then labels
that unconverted figure `currencyCode: defaultCurrency`. That is the second
clause of the invariant's own statement, reachable, and the catalog read
`enforced` over it.
Three things, none of them a change to `convertAmount`:
- A second scan, written from the rule: a statement returning the function's own
input from inside a null-check of a conversion result, whatever punctuation it
wears. It carries a self-test in both directions, because a scanning pattern
that quietly matches nothing is worse than no scan.
- The one call site that does this is recorded in
`RETURNS_ITS_INPUT_UNCONVERTED` with its reason and with what closes it, and
the list is asserted shrink-only -- so the violation is reviewed rather than
invisible. `convertAmount` keeps its `number` signature: ten report services
share it, and each needs its own missing-data policy and a completeness field
on its DTO, which is a specified change rather than a guard's business.
- INV-FX-001 goes from `enforced` to `partial`, with a Known gap naming the call
site, the label, and the shape of the fix, per the catalog's own convention
that a status is stated honestly. Its Required tests now record what is owed:
a per-report-family test that the response is withheld for a pair with no
rate. There is none, which is why the defect survived a guard written for it.
The reviewed-reciprocal assertions are unchanged and still hold.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
…own"
D2. Thirteen cells across the monthly, daily and by-security tables rendered
`x !== 0 ? fmtValue(x) : '-'`. `fmtValue` already draws the em dash for `null`,
so the ternary could only ever fire for a KNOWN zero -- and drew it as the
unknown marker. In the one report whose subject is which periods paid something,
a month that opened at nothing and a month nobody could value read the same.
Every one of them is now `{fmtValue(x)}`: the decision lives in `fmtValue`
alone, where the test is `null` and every other number, `0` included, is
formatted. `!== 0` is not a null check and `Number(null)` is `0`, which is how
the two facts get folded together.
Three cases, because a green suite after a behaviour change is a finding:
- a June row with `startValue: 0` renders `$0.00` and no dash (this fails on the
original expression);
- a June row with `startValue: null` renders the em dash and no `$` (this fails
if the branch is ever restored the other way round);
- 'shows dash for zero values in monthly table rows' was asserting the defect,
so it is now the inactive-month case: every figure in a quiet month reads
`$0.00` and no cell in the table reads as unknown. Its old second assertion
was `dashCells.some(() => true)`.
Both tables address their row by month label: the table lists every month in
range, so `tbody tr:first-child` is an empty month whose zeros are real, and
either assertion would have passed for the wrong reason.
Also records why these two tables are deliberately not phone-wrapped: the
column count is decided at runtime by the `visibleSeries` toggles (four to seven
cells), and the phone card is a `grid grid-cols-N` with per-cell placements,
which is a fixed N by construction. Written down so the next reader does not
"finish" the conversion.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
…h never ran
D3. 'exercises every sortable column on region and exchange tables' guarded its
whole exchange half on
const exchangeBtns = screen.queryAllByRole('button', { name: 'Exchange' });
if (exchangeBtns.length > 0) { ... }
and the toggle's accessible name is "By Exchange"
(`geographicAllocation.viewByExchange`), so the array was always empty and the
loop never executed once.
Fixed with `getByRole`, not a longer `queryAllByRole` list: a locator that stops
matching has to fail the test rather than skip it, which is the whole defect
here. The wait is now for "Exchange Allocation" as well -- the region table is
already mounted, so waiting for `querySelector('table')` resolves whether the
view switched or not, the second way this branch could pass without running.
It reveals no real failure: the exchange table sorts on all of its columns in
both directions. Verified the branch is load-bearing by putting the wrong name
back, which fails with "Unable to find an accessible element ... name
'Exchange'".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
…an for the next one
D4. Two tooltips in `ScheduledTransactionList` were still English for every
reader: the transfer chip's `` `Transfer to ${...|| 'account'}` `` and the split
chip's `s.category?.name || 'Uncategorized'`.
Both reuse a string another surface already owns rather than adding a key --
`common.transferPayee`, the same "Transfer to <account>" the register resolves
for a blank transfer payee, and `transactions.list.row.uncategorized`, what a
register row calls a line filed nowhere. No English catalog changes, so no
pseudo-locale regeneration and nothing owed to the other locales.
Two decisions worth their comments:
- The direction is the named `SCHEDULED_TRANSFER_DIRECTION`, not
`transferDirection(amount)`. `transferAccount` is the schedule's DESTINATION,
and that helper answers a different question -- which way a posted leg moved,
asked with that leg's own amount. Deriving it from the sign would be a
behaviour change wearing a localization's clothes.
- No counterpart name now means no tooltip, as `transferPayeeParams` already
decides for the register. The filler word ("Transfer to account") named
nothing a reader could act on.
`ScheduledTransactionList.i18n.test.ts` is rewritten from the rule. It listed
the four literals that had just been translated and asserted they were gone,
which certifies a diff: the two literals above sat in the same component
throughout, and a test naming four strings can only find those four. It now
scans every attribute a reader reads (`title`, `aria-label`, `placeholder`,
`alt`), blanks the translator calls and their keys, and fails on any prose
literal left over, reporting `path:line`. Verified against the original
expressions -- restoring both fails it at lines 126 and 133. Two pinning
assertions keep the reused keys reused, since the scan is satisfied by any
translator call. What the scan cannot see is written down: a literal passed in
by name, copy in a text child, and the ICU select arm it would misreport (which
is the other reason the direction is a constant). Its subject comes from
`import.meta.glob`, like its siblings; the previous `process.cwd()` join throws
ENOENT when vitest runs from the repository root.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
… one true reason
D5. `PortfolioValueReport`'s breakdown identity cell was converted from an
inherited 16px to `sm:text-sm`, shrinking the row identity on every desktop. It
was corrected by hand to `sm:text-base` and nothing was added, so mutating it
back left the whole suite green -- of the 27 `*.mobileWrapped` specs only seven
assert any `sm:text-*` at all.
The new scan reads the rule: a cell that hands its padding back at `sm` claims
to resolve identically to the unwrapped table from `sm` up, and its font size is
part of that claim. Which size is correct is a fact about the table BEFORE the
conversion and is not in the class string -- every wrapped table in the tree
gave its cells `text-sm`, and exactly one cell inherited the page's 16px -- so
the convention is scanned and the single exception is declared in
`INHERITED_DESKTOP_SIZE` with the pre-conversion class list as its reason. Two
tests, so it fails in both directions: a restoration that is neither the
convention nor declared, and a declared exception that is no longer in the
source at its own size. The second is the regression: putting `sm:text-sm` back
on that cell fails with `count: 1` becoming `count: 0`. Verified by doing it.
What it cannot see is in the comment, not left to be discovered: a size set once
on the `<table>` (which is how the three MonteCarlo tables legitimately hold
`text-xs`), a size composed through a constant or helper on another line, and
the rendering itself -- jsdom applies no Tailwind stylesheet, so no test here can
assert a computed font size.
D6. The chrome-constant scan justified itself with drift that never happened
("three copies had already drifted, a lost tracking token"): all fourteen
`PHONE_HEADER_CLASS` and all fourteen `CAPTION_CLASS` declarations on
`origin/main` were byte-identical. The reason is reach -- one home so the next
change to the caption breakpoint lands in every table instead of the files
somebody remembered -- and the constant that genuinely differs per report is
`HEADER_CLASS`, which this guard deliberately does not police. A future reader
must not reason from a fabricated precedent, so the old claim is named as wrong
rather than quietly dropped.
D7. `ui/Table.test.tsx` re-implemented that scan with its own `readdirSync` walk
over `src/components` only (missing `src/app` and `src/lib`), including
`*.test.tsx`, and matching unstripped bytes -- which is the shape the repo bans,
because a scan that reads its own explanation as a violation invites a weaker
explanation. Removed; its `PHONE_HEADER_CLASS` / `CAPTION_CLASS` value
assertions stay, next to the component that exports them.
The surviving scan now also catches the second way to break the rule: inlining
the constant's value at a call site. `<CellLabel className="sm:hidden">` IS
`CAPTION_CLASS` spelled out, imports nothing, and so silently skips any change
to it. Nineteen such sites exist in three files, and none of those files belongs
to this change, so they are a shrink-only baseline with a reason each --
`IncomeVsExpensesReport` is fixed by hand in the report-components change (A5)
and its line goes with that fix; the two loan-detail files are outstanding and
say so. A listed file that stops offending fails the baseline test, so the
register cannot outlive its subjects.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
D9. Moving `CAPTION_CLASS` into `components/ui/Table.tsx` left its JSDoc behind in five reports, where `/** Every caption in a wrapped cell is phone-only. */` now documents `interface CurrencyAllocation`, `function CustomTooltip` and three exported components. The claim is true of the constant and false of every one of those, and the constant carries the same sentence at its new home, so the five copies are deleted rather than reworded. The phone-strip essay in the same five files had merged into the next constant's own paragraph with no blank line between them, so a block that opens by describing the sort strip reads as documentation of `FIGURE_CELL` / `MONEY_CELL` / `CELL_PLACEMENT`. Each essay now names its subject in its first line -- `PHONE_HEADER_CLASS` in `components/ui/Table.tsx`, where that class and its doc live -- and a blank line separates it from the paragraph that really does document the declaration below. Comments only; no behaviour, and the ten suites over these five reports pass unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
…t it is D8, comment-only. Every phone-wrapped figure-cell constant adds `whitespace-nowrap` UNPREFIXED, so it applies at 640px and above as well, where the base cells carried no `white-space` class -- 82 cells across 12 reports. The behaviour is right and each cell constant documents it precisely. The JSX comment a reviewer reads first did not: it claimed the `sm`-and-up output "resolves identically to today", full stop. Root `CLAUDE.md`: an "identically", "cannot" or "complete" has to name the mechanism that makes it true, or the wording is wrong. The wording was wrong, so the wording is what changed -- `GeographicAllocationReport.tsx` and `IncomeBySourceReport.tsx` already scope their claims and are the model. File by file, one comment each unless noted: - `MonteCarloResultsTable.tsx` (the table-level `text-xs` claim, ~:63) - `MonteCarloPerformanceSummary.tsx` (~:144) - `MonthlyComparisonReport.tsx` (~:516 and ~:716) - `PortfolioValueReport.tsx` (~:1242) - `SecurityPerformanceReport.tsx` (~:909 and ~:1014) Two more carry the same overclaim and were not on the review list. Both are fixed here rather than left as the only two instances of a defect this commit exists to remove: - `MonteCarloHoldingStatsTable.tsx` (~:85) -- `FIGURE_CELL` and the symbol cell both carry the unprefixed class. - `DividendIncomeReport.tsx` (~:1714) -- already scoped its identity cell's font size honestly, and still claimed identity overall. `PortfolioValueReport.tsx`'s comment gets one more correction while its wording is being fixed: it said "each cell restores its own `sm:px-4 sm:py-3 sm:text-sm`", and the account cell restores `sm:text-base` -- it carried no size class before the conversion, so 16px inherited is what it has to hand back. That is the fact the new font-size scan in `ui-conventions.test.ts` declares as its one exception, and the comment beside the cell now agrees with it. Kept to comments and to one commit on purpose: these files belong to other changes in this review round, so a conflict here resolves by taking either side of a sentence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
Two seams the packages could not close from inside their own file sets. The inline-caption baseline entry for IncomeVsExpensesReport is deleted: that file now imports CAPTION_CLASS at all eight caption sites, and the register is shrink-only, so its staleness check was correctly failing. The two remaining entries (the amortization schedule's row and its header/footer) are unowned debt and stay. Bill Payment History's month axis moves onto useChartMonthFormat, the hook the other five month axes use. Concatenating `-01` onto the key and letting formatChartDate parse it worked, but the hook owns two decisions the call site was making badly: a malformed key became an Invalid Date, which Intl throws a RangeError on from inside a tick formatter and takes the report subtree down with it, and month 13 would have normalised to January of the next year rather than being recognised as malformed. Its test's stand-in recorded the argument by interpolation, which was fine for a string and carries the runner's zone and offset name for a Date. It now reads the day from local getters, so the expectation holds under any TZ (verified under UTC and Pacific/Auckland). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad
WMP
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linked discussion / issue
Approved in: #1309
Summary
Follow-up series to the mobile-table rollout. It extends the phone-card row layout to the report tables that were still scrolling sideways below
sm, and lands the localization and accessibility fixes that came out of the review.Phone-card wrapping (mechanism A — one CSS tree, identical
sm+ output):Each converted table below
smbecomes a two-to-four-line grid card so every column keeps half (or a third) of the width; money cells never wrap or truncate; the identity column wraps unclamped; sort controls return as a phone-only strip of the sameSortableHeaderchips, driven from one exhaustive mapped-type record shared with the desktop header. Every conversion ships a*.mobileWrapped.test.tsx.Shared chrome + guards (so the pattern cannot drift):
CellLabel,PHONE_HEADER_CLASS,CAPTION_CLASSand theSortColumn/SortColumnsByFieldtypes centralized incomponents/ui/Table.tsx; a source scan insrc/test/ui-conventions.test.tsfails a local re-declaration.Localization / correctness fixes surfaced by the review:
Accessibility:
No new user-facing strings were invented for the wrapping itself — each cell caption reuses its column's existing header key. The localization commits add keys that are translated for every locale in this same branch (i18n parity holds).
Checklist
Table.tsx; changes are additive and guarded)main.AI assistance disclosure
Implemented with Claude Code (Opus). Every table conversion was validated on the branch with the full frontend suite plus the mobile-table guards (
ui-conventions,number-locale.guard), i18n parity and pseudo-locale checks, ESLint andtscbefore landing. The author reviewed and owns the result; the layout is presentation-only and thesm+ desktop output is unchanged.🤖 Generated with Claude Code
https://claude.ai/code/session_01P4ukv9x4ZA53UT4tQtChad