MPDX-10016 Build balance table for MPGA

[kegrimes]

Description

In MPGA, we need to build a balance table that shows the end balance for every month in the last 12 months.

✅ Design approved by Joanna!

Jira ticket: MPDX-10016

Testing

• Impersonate: Courtney Campbell
• Go to /reports/mpgaIncomeExpenses
• Scroll down to the Balance table
• (1) Check the end balances are correct (can compare with staff expenses report)
• (2) Check overall total row, pagination, and total column are not present
• (3) Check negative balances are in red text
• (4) Click "Print" and check the balance table exists
• (5) Click "Export" and check the balance table is correct
• (6) Filter "Year To Date" and check all the above works

Checklist:

• I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
• I have applied the appropriate labels (Add the label "Preview" to automatically create a preview environment)
• I have run the Claude Code /quality:agent-review command locally and fixed any relevant suggestions
• I have requested a review from another person on the project
• I have tested my changes in preview or in staging
• I have cleaned up my commit history

[github-actions]

Preview branch generated at https://mpdx-10016-build-end-balance-table.d3dytjb8adxkk5.amplifyapp.com

[github-actions]

Bundle sizes [mpdx-react]

Compared against df619f6

No significant changes found

[kegrimes]

🤖 agent-review · ✅ no blockers open · risk HIGH
Rollout mode shadow — this report cannot approve or block the PR. · 8 agents run (Architecture, Data Integrity, Testing & Quality, Standards Compliance, Financial Accuracy, API Contracts, Localization, Accessibility) · no debate rounds

BLOCKERS — fix or dismiss to pass

• #1 · 7/10 · src/components/Reports/MPGAIncomeExpensesReport/Tables/useBalanceTableData.ts:121 — Two new user-visible strings, t('Ending Balance') and t('Balance Report'), have no key in public/locales/en/translation.json. — 🚫 dismissed by @kegrimes [false-positive]: the nightly .github/workflows/crowdin.yml job runs yarn extract + crowdin download and maintains standing bot PR [no-Jira] Update translations #1758 (updated 2026-09-18), so new keys reach Crowdin and the en catalog automatically; ci.yml also uploads sources on push to main. Committing the catalog diff per-PR is a convention here, not a correctness requirement.

⚠️ Rule correction pending. This finding came from .claude/review/rules/i18n.md, which asserts that a missing catalog entry "silently never reaches Crowdin". That is wrong — the rule was written from ci.yml alone and missed crowdin.yml. The rule doc needs correcting or the Localization lane will re-raise this on every PR.

OTHER FINDINGS (17)

• #2 · 6/10 · MPGAIncomeExpensesContext/MPGAIncomeExpensesContext.tsx:167 — startBalance sums the unfiltered reportsStaffExpenses.funds, but useBalanceTableData walks it forward with monthly nets derived from useFilteredFunds(…, filters?.categories). With a category filter active, the Ending Balance row mixes an unfiltered opening balance with filtered deltas and matches neither view. (architecture)
• #3 · 5/10 · MPGAIncomeExpensesContext/MPGAIncomeExpensesContext.tsx:167 — useMpgaTransactionsQuery still discards error, so a failed query leaves startBalance null and the UI renders "No Balance data available" plus a disabled Balance CSV item. This diff adds a third surface that reports absence when the backend actually failed. (data-integrity)
• #4 · 5/10 · Tables/TableCard.test.tsx:253 — "does not offer sorting on a single balance row" asserts the absence of sort icons without ever sorting or hovering, so it passes whether or not disableColumnSorting={isBalance} is set. MUI only renders those icons on active sort or hover. (testing)
• #5 · 5/10 · Tables/useBalanceTableData.ts:25 — Ending balance is re-derived client-side by walking startBalance forward with category-rollup net, though Fund.endBalance is a non-null server-computed field the query does not select. Any amount the server counts but does not surface as a category row makes the last month silently disagree. (data-integrity, financial)
• #6 · 4/10 · Helper/formatBalance.ts:4 — New pure helper formatBalance has no colocated formatBalance.test.ts covering its null/undefined-vs-zero branch; it is only exercised indirectly through PrintTables/TableCard tests, against this repo's Helper/*.ts convention. (testing)
• #7 · 4/10 · Helper/formatBalance.ts:9 — Checklist "Null vs zero distinguished" fails at report level: a hyphen now means no data in the Balance table but exactly zero in the adjacent Income/Expenses tables (zeroAmountFormat renders 0 as a hyphen). A reader comparing stacked tables cannot tell an empty month from a zero month. (financial)
• #8 · 4/10 · Tables/TableCard.tsx:109 — A third report type is expressed as an isBalance boolean threaded through three shared renderers: TableCard (9 branches), PrintTables (4), and CustomExport. The balance table shares almost no behaviour with income/expenses, so every future change must be reasoned about in both modes. (architecture)
• #9 · 3/10 · Helper/formatBalance.ts:1 — formatBalance forks zeroAmountFormat (src/lib/intlFormat.ts:88) into a feature helper and hand-rolls new Intl.NumberFormat(locale).format(0), giving the report a second number-formatting path with no shared try/catch or fraction-digit settings. (architecture)
• #10 · 3/10 · Helper/formatBalance.ts:12 — Checklist "Formatting via src/lib/intlFormat.ts" fails: the bare Intl.NumberFormat for zero uses different fraction-digit options and skips the module's non-finite guard, so a NaN balance falls into the amountFormat falsy branch and renders as a blank cell rather than a dash. (financial)
• #11 · 3/10 · MPGATransactions.graphql:16 — startBalance is period-scoped but Fund normalizes by id, so the cache holds one startBalance per fund regardless of the startMonth/endMonth that produced it. Two live queries for different periods would read each other's balance. (data-integrity)
• #12 · 3/10 · MPGATransactions.graphql:16 — The selection takes only Fund.startBalance and derives every ending balance from category net, though Fund.endBalance exists in the production schema. Nothing reconciles the derived value, so roundTwoDecimals drift in filterFunds or an omitted transaction shows as a wrong balance with no detectable error. (api-contracts)
• #13 · 3/10 · Tables/PrintTables.tsx:61 — The new balance column colors negative amounts with theme.palette.error.main as text color; the repo rule requires a stated measured contrast ratio for a semantic-token change on text, and this diff states none. (accessibility)
• #14 · 3/10 · Tables/TableCard.tsx:108 — const overallTotal = type === ReportTypeEnum.Income ? income : expenses; now silently maps the new Balance type to the expenses total in both TableCard and PrintTables. Dead today only because the total row is wrapped in !isBalance — latent, not fixed. (architecture)
• #15 · 3/10 · Tables/TableCard.tsx:150 — formatBalance renders a zero balance as 0, but the cell's tooltip still calls amountFormat(params.value, locale), which returns "" for 0. A real zero balance shows a value with no tooltip while every other month has one. (data-integrity)
• #16 · 3/10 · Tables/TableCard.tsx:183 — The Balance row reuses the shared Average column header, but its value is the mean of month-end balances over elapsed months, not an average monthly flow like the Income/Expenses rows above it. Two different quantities under one header. (financial)
• #17 · 3/10 · Tables/useBalanceTableData.ts:1 — useBalanceTableData is report-wide derived state (three consumers) but sits under Tables/ and is imported from two sibling folders, while allData and totals are owned by the context. It also reaches into Charts/MonthlySummaryChart/useMonthlySummaryChartData, so a Tables module depends on a chart-local hook. (architecture)
• #18 · 2/10 · Tables/TableCard.tsx:305 — The DataGrid negative-balance CSS class also relies on error.main as the state cue, mitigated by the minus sign already present in the formatted text. (accessibility)

🔧 Fix suggestions (1)

#10 — Route formatBalance's zero case through src/lib/intlFormat and guard non-finite values: src/components/Reports/MPGAIncomeExpensesReport/Helper/formatBalance.ts

-import { amountFormat } from 'src/lib/intlFormat';
+import { amountFormat, numberFormat } from 'src/lib/intlFormat';
@@
-  if (value === null || value === undefined) {
+  if (value === null || value === undefined || !Number.isFinite(value)) {
@@
-    return new Intl.NumberFormat(locale).format(0);
+    return numberFormat(0, locale);

Apply: bash /tmp/automated_fixes/fix_3_financial.sh (run bash /tmp/automated_fixes/apply_all.sh first for a dry run)

📦 Dependency impact

Blast radius: 21 transitive dependents. Not truncated.

Changed file	Direct dependents
MPGAIncomeExpensesContext.tsx	12 — the report page, BreakdownModal, CardSkeleton, ExpensesPieChart, MonthlySummaryChart, useMonthlySummaryChartData, SummaryBarChart, TotalRow, + tests
MPGAIncomeExpensesReportTestWrapper.tsx	7 — test-only
Helper/MPGAReportEnum.tsx	3 — including src/hooks/useFilteredFunds.ts, outside this feature folder
Tables/TableCard.tsx	2 — Helper/createRows.tsx, Tables/TotalRow.tsx
DisplayModes/PrintOnlyReport.tsx · ScreenOnlyReport.tsx · ExportCsvButton.tsx	1 each — MPGAIncomeExpensesReport.tsx
Helper/filterFunds.ts	1 — src/hooks/useFilteredFunds.ts

The context change is the one with real reach: 12 dependents including the route page. Findings #2 and #3 both land there.

📊 Review detail & stats

Generated: 2026-09-18 · Day: Friday · Files changed: 20 (+802 −106 lines)
Risk score: 43 — HIGH · Required reviewer: experienced
⚠️ Friday: HIGH-risk change. One open blocker and two correctness findings around filter interaction — worth landing Monday unless it is needed before the weekend.

Risk factors detected: pattern score 40 (src/components/Reports/** at +2 across 20 files) · volume score 3 · no special patterns fired · no unmatched risk paths — the risk map covers every changed file.

Deterministic evidence:

• AST/static rules: none — static_analysis.ast_grep is disabled in this repo (ESLint already gates its syntactic invariants in CI)
• CI snapshot: unavailable — the local run does not fetch check state
• Cross-repo context: none — context is disabled

Agent summary (bands: Critical 9-10 · High 7-8 · Important 5-6 · Suggestions 3-4):

Agent	Critical	High	Important	Suggestions	Confidence
Architecture Review Agent	0	0	1	4	Medium
Data Integrity Review Agent	0	0	2	2	High
Testing & Quality Review Agent	0	0	1	1	High
Standards Compliance Review Agent	0	0	0	0	High
Financial Accuracy Review Agent	0	0	0	4	Medium
API Contracts Review Agent	0	0	0	1	High
Localization Review Agent	0	1	0	0	High
Accessibility Review Agent	0	0	0	2	Medium
Total	0	1	4	14	–

(Counts are per-lane raw findings; #5 was corroborated by two lanes and is counted once in the ledger.)

Per-agent perspectives on blockers:

• #1 — Localization (7/10): the only lane that flagged it. No other lane contested it; standards returned zero findings and did not raise the catalog gap independently.

Review quality:

• Average agent confidence: Medium-High · Consensus rate: 1 of 18 findings corroborated across lanes (#5) · Average severity 3.8
• Findings suppressed by approved learnings: 0 (the learning store is empty — this is the first review)
• 9 near-miss pairs were adjudicated by hand; 1 merged (#5), 8 kept as distinct

💬 How to act on this review

Every finding above is numbered. Severity ≥ 7 findings carry a checkbox and must each be fixed or dismissed before this review counts as passed; lower severities are advisory. Interact from a PR comment — @claude fix 1, 3 · @claude dismiss 2 [false-positive]: <one-line reason> (a reason code and explanation are required) — or locally with /agent-review:address. Valid codes: false-positive, intentional, pre-existing, deferred, duplicate, insufficient-evidence, other.

• @claude fix 1, 3, 5 — AI applies those fixes on this branch and checks them off
• @claude dismiss 2 [intentional]: matches legacy import behavior — checks it off with your reason; repeated dismissals of the same finding class teach the review to stop raising it
• @claude fix 1, 3; dismiss 2 [false-positive]: guarded by the caller — mixed operations use a semicolon between clauses

Or locally: /agent-review:address pulls this ledger into a Claude Code session.