feat(budget): add expandable cost breakdown table to Budget Overview#466
Conversation
Adds a new GET /api/budget/breakdown endpoint and CostBreakdownTable React component that displays a 4-level expandable hierarchy (categories → work items → vendors → budget lines) with confidence labels, margin ranges, and invoice tracking. Integrates into the Budget Overview page with 85 unit and integration tests across 3 test files. Fixes #464 Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) <noreply@anthropic.com> Co-Authored-By: Claude backend-developer (Haiku 4.5) <noreply@anthropic.com> Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com> Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
steilerDev
left a comment
There was a problem hiding this comment.
[security-engineer]
Security Review — PR #466: Budget Breakdown Table (GET /api/budget/breakdown)
Reviewed files:
server/src/services/budgetBreakdownService.ts— SQL queries, subsidy computationserver/src/routes/budgetOverview.ts— route handler, auth checkclient/src/components/CostBreakdownTable/CostBreakdownTable.tsx— rendering financial dataclient/src/lib/budgetOverviewApi.ts— API clientshared/src/types/budgetBreakdown.ts— shared types
Authentication & Authorization
The new GET /api/budget/breakdown endpoint correctly enforces authentication:
fastify.get('/breakdown', async (request, reply) => {
if (!request.user) {
throw new UnauthorizedError();
}
const breakdown = getBudgetBreakdown(fastify.db);
return reply.status(200).send({ breakdown });
});- Auth check:
request.userguard present — consistent with the existing/budget/overviewpattern. - Both
adminandmemberroles can access this endpoint, which is appropriate (the endpoint is read-only and single-tenant). - Integration tests in
budgetOverview.breakdown.test.tsconfirm 401 on unauthenticated requests and 200 for both roles.
No authorization gaps found.
Injection Analysis (A03)
All SQL in budgetBreakdownService.ts uses Drizzle's sql\`` tagged template literals. No string concatenation or raw user input flows into any query. All six queries (A–F) follow the established safe pattern:
sql`SELECT ... FROM work_items wi INNER JOIN ... WHERE ...`No injection vectors found.
XSS / Frontend Rendering (A03)
CostBreakdownTable.tsx renders server-sourced data (work item titles, category names, budget line descriptions, household item names) exclusively as React JSX text nodes:
<span>{item.title}</span>
<span>{category.categoryName}</span>
<span>{line.description || 'Untitled'}</span>- No
dangerouslySetInnerHTML,innerHTML, oreval()anywhere in the component tree. ConfidenceBadgeprocesses server enum values with.replace(/_/g, ' ').toLowerCase()— this is a string transformation on a server-validated enum, not user-supplied input. Safe.formatCurrency()receives numeric values from the API — no string injection path.- CSS class selection uses CSS Modules (
styles.valuePositive/styles.valueNegative) based on arithmetic comparison, not string interpolation from server data.
No XSS vectors found.
Sensitive Data Exposure (A02)
The response shape exposes financial data (planned amounts, actual costs, subsidy payback rates, projected budgets) to any authenticated user. This is appropriate and intentional for the budget overview page in a single-tenant home-building application.
BreakdownBudgetLine includes plannedAmount and actualCost. These are the appropriate fields for this feature and consistent with the existing budget endpoints.
No inappropriate data exposure found.
Informational: No Route-Level Response Schema
The /breakdown handler returns data without a Fastify JSON schema declaration on the route. This mirrors the existing /overview handler's pattern (both are inline handlers in budgetOverviewRoutes). In this single-tenant model the impact is negligible — Fastify serializes the full object — but a response schema would provide response validation and auto-generated API docs at no security cost.
This is consistent with the existing /budget/overview endpoint and is documented in prior reviews as an accepted pattern.
Informational: Unbounded SELECT on budget breakdown data
getBudgetBreakdown performs four unbounded SELECT queries (all work item budget lines, all HI budget lines, all invoices, all subsidies). At single-tenant construction project scale this is acceptable and matches the pattern used by the existing getBudgetOverview service. No DoS concern at target scale.
Checklist
- No SQL/command/XSS injection vectors in new code
- Authentication enforced on the new endpoint (401 on unauthenticated)
- Both roles (admin, member) correctly granted access
- No sensitive data (secrets, tokens, PII) exposed in logs, errors, or client responses
- User input is not used in any query parameter
- No new dependencies introduced
- No hardcoded credentials or secrets
- CORS configuration unchanged
- Error responses (401) do not leak internal details
- No
dangerouslySetInnerHTMLor unsafe DOM patterns in frontend
No blocking security findings. This PR is clear to merge from a security perspective.
steilerDev
left a comment
There was a problem hiding this comment.
[product-architect]
Architecture Review — PR #466: Budget Breakdown Table
Verdict: Comment (1 medium finding)
I reviewed the full diff (12 files, ~2900 lines added) against the Wiki Architecture, API Contract, and Schema pages.
What looks good
- Shared types: Clean type definitions in
shared/src/types/budgetBreakdown.tswith properCostDisplay,BreakdownBudgetLine,BreakdownWorkItem, andBreakdownHouseholdIteminterfaces. All correctly exported fromshared/src/index.ts. - Service layer:
budgetBreakdownService.tsfollows established patterns — raw SQL queries via Drizzlesqltemplate, Map-based aggregation, proper subsidy payback computation consistent withbudgetOverviewService. UsesCONFIDENCE_MARGINSfrom shared correctly. - Route handler: Minimal, auth-guarded, follows the existing
budgetOverview.tspattern exactly. Response wrapped in{ breakdown }matching theBudgetBreakdownResponsetype. - Test coverage: 85 tests across 3 files — service unit tests, route integration tests, and component tests cover empty state, auth, data accuracy, subsidy payback, costDisplay modes, and accessibility (aria-expanded). Solid coverage.
- CSS: Uses design tokens exclusively (
var(--color-*),var(--spacing-*),var(--font-size-*)), responsive breakpoints, sticky column on mobile,prefers-reduced-motionsupport. - No schema changes: Read-only endpoint querying existing tables — no migration needed.
Findings
[MEDIUM] Wiki API Contract not updated for GET /api/budget/breakdown
The new endpoint GET /api/budget/breakdown is not documented in wiki/API-Contract.md. Per project conventions, every new endpoint must be documented in the API Contract wiki in the same PR. This includes:
- Path, method, auth requirements
- Response shape (the
BudgetBreakdownResponsewrapper with nestedBudgetBreakdown) - Error responses (401 for unauthenticated)
This is a documentation gap only — the implementation itself is correct and consistent.
Minor observations (informational, non-blocking)
-
Sequential fetch:
fetchBudgetBreakdown()is called afterfetchBudgetOverview()completes (BudgetOverviewPage.tsx line ~324). Both are independent reads and could be parallelized withPromise.allfor faster page load. -
Mixed mode color: In
CostDisplay, themixedbranch always appliesstyles.valuePositiveto both the Actual and Projected spans, even when values could theoretically be negative. TheisPositivecheck is not used in the mixed path. -
Test helper complexity:
getButtonByControls(~140 lines) reverse-engineers DOM structure with hardcoded category name lists. Addingdata-testidoraria-controlsattributes to expand buttons would make tests more resilient to UI changes. -
Empty Remaining column: The "Remaining" column header exists but all item/category rows render empty cells — only the top-level summary row uses it. Consider whether the column should be removed from inner rows or populated with per-item remaining values.
steilerDev
left a comment
There was a problem hiding this comment.
[product-owner]
PR Review — #466: feat(budget): add expandable cost breakdown table to Budget Overview
Story: #464 — 5.13: Detailed expandable cost breakdown table on Budget Overview
AC Verification
| AC | Description | Verdict | Notes |
|---|---|---|---|
| 1 | Summary table with Sources, WI Budget, HI Budget, Remaining | PASS | Implementation uses a row-based layout (Name/Budget/Payback/Remaining columns) with summary-level rows for Available Funds, Work Item Budget, Household Item Budget, and Remaining. This is a reasonable interpretation -- the column-based layout described in the AC would not support the expandable hierarchy. Values match overview totals. Minor: label says "Available Funds" instead of "Sources" -- acceptable, arguably clearer. |
| 2 | Work Item Budget expandable to category rows with name, cost, subsidy, sum row | PASS | WorkItemCategorySection renders per-category rows with projected/actual cost via CostDisplay, subsidy payback, and sum row at bottom. |
| 3 | Household Item Budget expandable to HI category rows | PASS | HouseholdItemCategorySection renders per HI category (furniture, appliances, etc.) with aggregated cost and subsidy payback. Sum row included. |
| 4 | Category rows expandable to individual item rows | PASS | Both WI and HI category rows expand to show individual ItemRow components with item name, cost, and subsidy. |
| 5 | Cost display: actual/projected/mixed logic | PASS | CostDisplay component correctly renders "Actual: [amount]" for actual, "Expected: [min] - [max]" for projected, and both for mixed. Backend computeCostDisplay determines mode based on invoice status of budget lines. Tested in unit tests (scenarios 4, 5, 22-24). |
| 6 | Item rows expandable to budget line rows | PASS | BudgetLineRow shows description (or "Untitled"), planned amount, confidence badge, and actual cost when invoiced. |
| 7 | Subsidy payback column with dash for zero, sum at category level | PASS | Item row renders item.subsidyPayback === 0 ? '---' : formatCurrency(item.subsidyPayback). Category rows aggregate payback from child items. Backend computeEntitySubsidyPayback handles percentage and fixed reduction types, excludes rejected subsidies. |
| 8 | Button elements with aria-expanded, chevron rotation | PASS | All expand/collapse uses <button> with aria-expanded attribute. CSS .chevronOpen applies rotate(90deg). prefers-reduced-motion: reduce disables transition. |
| 9 | Responsive design on mobile | PASS | @media (max-width: 767px) media query with horizontal scroll (overflow-x: auto), sticky name column (position: sticky; left: 0), touch scrolling (-webkit-overflow-scrolling: touch). |
| 10 | Category filter integration | PASS | selectedCategories prop filters visibleWICategories via useMemo. WI totals recalculated from visible categories only. Remaining updates accordingly. HI categories correctly excluded from filtering. |
| 11 | Light/dark mode with hierarchy differentiation | PASS | All styles use CSS custom properties (design tokens). Four hierarchy levels use distinct backgrounds (--color-bg-primary, --color-bg-secondary, --color-bg-primary, --color-bg-tertiary) and increasing indentation. |
| 12 | Loading and empty states | PASS | BudgetOverviewPage shows loading indicator with role="status" and aria-label="Loading cost breakdown" while breakdown is fetching. Component shows "No budget data to display" when both WI and HI categories are empty. Heading still renders in empty state. |
| 13 | Backend endpoint for breakdown data | PASS | New GET /api/budget/breakdown endpoint in budgetOverview.ts route file. Backed by budgetBreakdownService.ts with 6 SQL queries (WI lines, WI invoices, HI lines, HI invoices, subsidies, entity-subsidy links). Auth required. |
Result: All 13 ACs PASS. APPROVED from product-owner perspective.
Scope Check
The PR stays within the story's scope. No undocumented features or scope creep detected. The new endpoint, component, types, and tests are all directly related to Story 5.13.
Test Coverage
- 85 tests across 3 files:
budgetBreakdownService.test.ts(service unit),budgetOverview.breakdown.test.ts(route integration),CostBreakdownTable.test.tsx(component) - Tests cover: empty state, projected/actual/mixed cost display, confidence margins, subsidy payback (percentage, fixed, rejected), category grouping, HI categories, section totals, auth checks, response shape, category filter, aria-expanded, expand/collapse behavior
- Test authorship:
qa-integration-testerlisted in Co-Authored-By trailers
Observations (non-blocking)
-
"Available Funds" vs "Sources" label: AC 1 says "Sources" but the implementation uses "Available Funds". This is actually clearer for end users and consistent with the overview data field name (
availableFunds). No change needed. -
Remaining column shows dash on non-summary rows: The Remaining column is only meaningful on the summary-level Remaining row. All category/item/line rows show an empty cell. This is clean behavior.
-
CI still in progress: Quality Gates and Docker checks are running at time of review. Merge should wait for CI green.
-
Other agent reviews: Security review is present (comment). Architecture review is not yet posted.
steilerDev
left a comment
There was a problem hiding this comment.
[ux-designer]
PR #466 — feat/464-budget-breakdown-table — Design Review
Reviewed against the visual spec posted on Issue #464. The implementation is structurally solid and token adherence is very good throughout the CSS Module. The component structure, chevron pattern, confidence badges, and dark mode reliance on semantic tokens are all correct. Several spec deviations need to be addressed before merge.
Issues Found
HIGH — Touch target below minimum on mobile (Accessibility)
File: CostBreakdownTable.module.css, lines 289–293
/* Current — WRONG */
@media (max-width: 767px) {
.expandBtn {
width: 20px;
height: 20px;
min-width: 20px;
}
}The spec requires expand buttons to meet the 44×44px minimum touch target on mobile. The mobile override reduces them to 20×20px — less than half the minimum. Fix: use min-height: 44px and min-width: 44px while keeping the visual icon size at 20px via inner sizing, or use a transparent padding approach:
@media (max-width: 767px) {
.expandBtn {
width: 44px;
height: 44px;
min-width: 44px;
}
}MEDIUM — Level 0 font size is --font-size-lg instead of --font-size-xl
File: CostBreakdownTable.module.css, line 75
/* Current — WRONG */
.rowLevel0 td {
font-size: var(--font-size-lg);The spec table (Section 4) defines Level 0 summary values as --font-size-xl (20px). Using --font-size-lg (18px) reduces the visual prominence of the top-level totals. Fix:
.rowLevel0 td {
font-size: var(--font-size-xl);MEDIUM — Chevron transition uses hardcoded duration instead of token
File: CostBreakdownTable.module.css, line 153
/* Current — WRONG */
.chevron {
transition: transform 150ms ease;All transitions must use design tokens. The spec calls for transition: transform var(--transition-normal). Fix:
.chevron {
transition: transform var(--transition-normal);MEDIUM — Missing row hover state for Levels 1–3
File: CostBreakdownTable.module.css — not present
The spec (Section 7) requires Level 1, 2, and 3 rows to show a hover background:
.rowLevel1:hover td,
.rowLevel2:hover td,
.rowLevel3:hover td {
background: var(--color-bg-hover);
transition: background-color var(--transition-fast);
}
@media (prefers-reduced-motion: reduce) {
.rowLevel1:hover td,
.rowLevel2:hover td,
.rowLevel3:hover td {
transition: none;
}
}The Level 0 summary rows should NOT have hover backgrounds per the spec — that distinction is correctly absent.
MEDIUM — Missing aria-controls on expand buttons (Accessibility)
File: CostBreakdownTable.tsx — all <button> elements
The spec defines aria-controls pairing between each expand button and its disclosed row group (e.g., aria-controls="wi-section-categories"). The test file comments explicitly note this was removed. aria-controls is a required accessibility attribute for disclosure patterns — it allows screen readers to locate and announce the associated content region. Without it, AT users cannot navigate from the button to the expanded content.
Each section and category expand button needs an aria-controls attribute pointing to the id of the rendered row group.
MEDIUM — Missing aria-label on expand buttons (Accessibility)
File: CostBreakdownTable.tsx — all <button> elements
The expand buttons contain only an aria-hidden="true" SVG icon. Without aria-label, each button is announced as "button" with no descriptive name. Screen readers cannot distinguish "Expand Work Item Budget" from "Expand Materials category" from "Expand Foundation Work item". Fix: add descriptive aria-label attributes:
- Section buttons:
aria-label="Expand Work Item Budget breakdown" - Category buttons:
aria-label={\Expand ${category.categoryName} category`}` - Item buttons:
aria-label={\Expand ${item.title} budget lines`}`
MEDIUM — Missing <caption> on table (Accessibility)
File: CostBreakdownTable.tsx, line 481 (<table className={styles.table}>)
The spec requires a visually-hidden <caption> to describe the table to screen readers:
<table className={styles.table}>
<caption className={sharedStyles.srOnly}>
Budget cost breakdown by category and item
</caption>
…Without a caption, screen readers announce only "table" with no context.
MEDIUM — Projected range applies green success color incorrectly
File: CostBreakdownTable.tsx, lines 69–84 (CostDisplay component)
// Current — WRONG for 'projected' and 'mixed' projected line
if (costDisplay === 'projected') {
return (
<span className={isPositive ? styles.valuePositive : styles.valueNegative}>
{formatCurrency(projectedMin)} – {formatCurrency(projectedMax)}
</span>
);
}
// mixed — also wrong: projected line gets valuePositive (green)
<span className={styles.valuePositive}>
Projected: {formatCurrency(projectedMin)} – {formatCurrency(projectedMax)}
</span>styles.valuePositive applies color: var(--color-success-text-on-light) — the green payback color. Per the spec (Section 2c): projected ranges should use --color-text-secondary (neutral), not green. Green is reserved for subsidy payback values. The Actual: line in actual mode correctly uses green. Fix:
if (costDisplay === 'projected') {
return (
<span> {/* no valuePositive class — inherits row color */}
{formatCurrency(projectedMin)} – {formatCurrency(projectedMax)}
</span>
);
}
// In mixed, projected line:
<span> {/* neutral color, no valuePositive */}
Projected: {formatCurrency(projectedMin)} – {formatCurrency(projectedMax)}
</span>MEDIUM — Loading state uses text instead of skeleton shimmer
File: BudgetOverviewPage.tsx, lines 2079–2086 and BudgetOverviewPage.module.css
The spec (Section 10) requires 3 skeleton shimmer rows in the breakdown card during loading, matching the DocumentSkeleton shimmer pattern. The implementation renders <p>Loading cost breakdown…</p> with a text-style .breakdownLoading card.
The .breakdownLoading card itself is fine as a shell, but it needs shimmer rows inside it rather than a text loading message:
/* Required in CostBreakdownTable.module.css */
.skeletonRow {
height: 44px;
border-radius: var(--radius-md);
background: linear-gradient(
90deg,
var(--color-bg-tertiary) 25%,
var(--color-bg-hover) 50%,
var(--color-bg-tertiary) 75%
);
background-size: 200% 100%;
animation: shimmer 1.5s infinite;
margin-bottom: var(--spacing-2);
}
@keyframes shimmer {
0% { background-position: 200% 0; }
100% { background-position: -200% 0; }
}
@media (prefers-reduced-motion: reduce) {
.skeletonRow {
animation: none;
background: var(--color-bg-tertiary);
}
}LOW — Inline style for mixed cost display layout
File: CostBreakdownTable.tsx, line 79
<div style={{ display: 'flex', flexDirection: 'column', gap: 'var(--spacing-0-5)' }}>Inline styles bypass CSS Modules and cannot be overridden at breakpoints. Move to a CSS Module class:
/* CostBreakdownTable.module.css */
.mixedCostDisplay {
display: flex;
flex-direction: column;
gap: var(--spacing-0-5);
}LOW — Level 3 padding deviates from spec
File: CostBreakdownTable.module.css, line 120
/* Current */
.rowLevel3 td {
padding: var(--spacing-2) var(--spacing-3);The spec (Section 6) specifies var(--spacing-1-5) var(--spacing-3) for Level 3 cells to visually compact the deepest nesting level. The implementation uses --spacing-2 which is slightly more generous but loses the visual compaction that distinguishes Level 3 from Level 2. Fix:
.rowLevel3 td {
padding: var(--spacing-1-5) var(--spacing-3);INFORMATIONAL — Missing aria-live announcement region
File: CostBreakdownTable.tsx — not present
The spec (Section 11) calls for an aria-live="polite" region that announces expand/collapse state changes (e.g., "Work Item Budget breakdown expanded. 4 categories shown."). This is helpful for screen reader users who cannot see the chevron rotation. Not a blocker but improves the experience.
INFORMATIONAL — Category payback shows €0.00 instead of — for zero values
File: CostBreakdownTable.tsx, lines 214/250 (category payback cells)
<td className={styles.colPayback}>{formatCurrency(category.subsidyPayback)}</td>When subsidyPayback === 0, this renders €0.00 rather than —. Item rows correctly show — for zero payback (item.subsidyPayback === 0 ? '—' : formatCurrency(...)). Category rows should match the same pattern for visual consistency.
What Was Verified Correctly
- All background color tokens per nesting level match the spec exactly (
--color-bg-primary,--color-bg-secondary,--color-bg-tertiaryfor Levels 0/2, 1, and 3 respectively) - Sum row tokens (
--color-bg-secondary,border-top: 2px solid var(--color-border-strong)) match spec - Left border accent on Level 1 name cell (
border-left: 3px solid var(--color-border-strong)) matches spec - Indent levels correct:
--spacing-8(L1),--spacing-14(L2),--spacing-20(L3) - Confidence badge tokens are exact spec matches (
--color-bg-secondary,--color-border,--radius-full) valuePositive/valueNegativetokens on the Remaining row are correctaria-expandedcorrectly toggles on all expand buttonsprefers-reduced-motionguard on chevron transition is present (though the duration is hardcoded — see finding above)- Mobile sticky first-column with per-level background replication is correctly implemented
- Dark mode requires no component-level overrides — all tokens are semantic (correct)
section[aria-labelledby]+h2[id]structure is correctth[scope="col"]on all header cells is correct- Empty state message and heading presence in empty state are correct
formatCurrencyis used consistently (no inlineIntl.NumberFormat)- No hardcoded hex colors anywhere in the CSS Module
font-variant-numeric: tabular-numsis applied at table level (correct)- Responsive breakpoint at
max-width: 767pxmatches the spec's< 768pxboundary
Fixes reviewed issues from CostBreakdownTable component: 1. Mobile expand buttons now have 44x44px min touch target (issue #1) 2. Summary row font size upgraded to --font-size-xl for prominence (issue #2) 3. Chevron transition uses --transition-normal token instead of hardcoded value (issue #3) 4. Added hover states for Level 1-3 rows with --color-bg-hover (issue #4) 5. Added descriptive aria-labels to all expand buttons: - Section buttons: "Expand work item/household item budget categories" - Category buttons: "Expand {categoryName}" - Item buttons: "Expand {itemName}" 6. Added table caption with screen-reader-only CSS class (issue #6) 7. Projected cost ranges now use neutral text color (not valuePositive green) (issue #7) 8. Category-level subsidyPayback displays "—" when zero, not "€0.00" (issue #8) Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…Page tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
🎉 This PR is included in version 1.12.0-beta.40 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
…h tests for inline date fields Add three tests missing from the inline-edit test suite (flagged in PR #466 review): - Error alert shown when updateHouseholdItem rejects on latestDeliveryDate blur - Error alert shown when updateHouseholdItem rejects on earliestDeliveryDate blur - Clear latestDeliveryDate calls PATCH with null and does NOT trigger a re-fetch Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) <noreply@anthropic.com>
|
🎉 This PR is included in version 1.12.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
GET /api/budget/breakdownendpoint backed bybudgetBreakdownServicethat aggregates work items, vendors, and budget lines into a 4-level cost hierarchy per budget categoryCostBreakdownTableReact component with expandable rows (category → work item → vendor → budget line), confidence labels, margin range columns, and invoice trackingbudgetOverviewApi.tswith the newfetchBudgetBreakdowncall; 85 tests across 3 test files cover the backend service, route integration, and frontend componentFixes #464
Test plan
budgetBreakdownService.test.ts,budgetOverview.breakdown.test.ts,CostBreakdownTable.test.tsxCo-Authored-By: Claude dev-team-lead (Sonnet 4.6) noreply@anthropic.com
Co-Authored-By: Claude backend-developer (Haiku 4.5) noreply@anthropic.com
Co-Authored-By: Claude frontend-developer (Haiku 4.5) noreply@anthropic.com
Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) noreply@anthropic.com