feat(budget): add source attribution badges and per-source filter to cost breakdown overview#1355
Conversation
…ges and per-source filter (#1354) - New: client/src/lib/budgetSourceColors.test.ts — 15 tests for getSourceColorIndex (always [1,9], deterministic, never 0) and getSourceBadgeStyleKey (null→sourceUnassigned, consistency with colorIndex) - New: client/src/components/BudgetSourceChip/BudgetSourceChip.test.tsx — 18 tests for rendering, 24-char truncation, aria-pressed, onToggle callback, disabled state, CSS custom properties (--chip-dot/bg/text), aria-label, keyboard interaction - Extended: client/src/components/Badge/Badge.test.tsx — 2 tests for new title prop (forwards to span, absent when omitted) - Extended: client/src/components/CostBreakdownTable/CostBreakdownTable.test.tsx — updated all prop usages (selectedSourceIds/onSourceToggle/onClearSources replacing budgetSources=[]), added budgetSourceId to fixtures, added 20+ tests for source badge rendering, chip strip, filter active state, clear button, empty state - Extended: client/src/pages/BudgetOverviewPage/BudgetOverviewPage.test.tsx — added budgetSources:[] to breakdown fixtures - Extended: server/src/services/budgetBreakdownService.test.ts — 3 helpers + 2 describe blocks (~20 tests) for budgetSourceId attribution on WI/HI lines and budgetSources aggregate (id/name/totalAmount, projectedMin/Max by confidence, multi-line accumulation, multi-source, empty cases) - Extended: server/src/routes/budgetOverview.breakdown.test.ts — 2 helpers + 2 tests for budgetSources array in HTTP response and empty array when no sources assigned Fixes #1354 Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) <noreply@anthropic.com>
…cost breakdown overview
- Display a deterministic-color source badge next to each line's confidence/quoted/invoiced badge in the cost breakdown table
- Mobile collapses the source badge to a color dot with full source name in aria-label/title
- Available Funds row redesigned as a multi-select chip toolbar showing per-source allocated cost and remaining balance
- Multi-select OR semantics filter the breakdown rows; subtotals and grand totals recompute against the filtered set
- URL state ?sources=<id>,<id> persists the filter across reloads
- Empty state when no lines match the active filter
- Keyboard: Tab through chips, Space/Enter toggles, Escape clears filter and refocuses the Available Funds expand button
- Backend: GET /api/budget/breakdown extended with budgetSourceId per line and a budgetSources aggregate map (id, name, totalAmount, projectedMin, projectedMax)
- Tokens: new --color-source-N-{bg,text,dot} family (10 slots, light + dark)
- New shared component: BudgetSourceChip
- New helper: client/src/lib/budgetSourceColors.ts (deterministic id->slot mapping)
Fixes #1354
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 translator (Sonnet 4.5) <noreply@anthropic.com>
Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) <noreply@anthropic.com>
Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.5) <noreply@anthropic.com>
Co-Authored-By: Claude ux-designer (Sonnet 4.5) <noreply@anthropic.com>
Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) <noreply@anthropic.com>
… tests Source names now appear in both the chip filter strip and the expanded sub-rows. Replace getByText assertions with getAllByText length checks to disambiguate. Fixes #1354 Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) <noreply@anthropic.com> Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) <noreply@anthropic.com>
…lso gated by expand)
The Available Funds chip filter strip is rendered inside the
{availFundsExpanded && ...} block, so on collapse both the chip strip
and the sub-rows unmount. Replace getAllByText.toHaveLength(1) with
queryByText.not.toBeInTheDocument().
Fixes #1354
Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) <noreply@anthropic.com>
- CostBreakdownTable defensively coalesces breakdown.budgetSources to [] so existing fixtures and forward-compatible payloads do not crash the cost breakdown rendering. - E2E sourceDetailRow page-object now scopes to tr[class*="rowSourceDetail"] to avoid matching the chip-toolbar row that also contains the source name. - Dark-mode badge color test uses toBeAttached() instead of toBeVisible() so it works on mobile where the badge label is CSS-hidden. Fixes #1354 Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com> Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.5) <noreply@anthropic.com>
…ions - Source badge tests on Level 3 rows use toBeAttached() instead of toBeVisible() because mobile hides the badge label via CSS while keeping the element in the DOM for screen-reader access. - Selected-source detail row class assertion uses toHaveClass() so it auto-retries through the chip-click → URL-state → React-render round-trip. Fixes #1354 Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.5) <noreply@anthropic.com>
steilerDev
left a comment
There was a problem hiding this comment.
[product-architect] Architectural review: APPROVE (posted as comment because the API does not allow self-approval on the orchestrator account).
Verified
- Wiki API Contract update (
GET /api/budget/breakdown) matches the server response:budgetSourceIdperBreakdownBudgetLine,budgetSourcesaggregate array withid/name/totalAmount/projectedMin/projectedMax. Note explaining client-side filtering and per-source semantics is appropriate; no new ADR needed (the decision is small, additive, and documented inline). - No DB schema change — the source_id columns on
work_item_budgets/household_item_budgetsalready exist; the service simply selects them. - Shared types (
BreakdownBudgetLine.budgetSourceId,BudgetSourceSummaryBreakdown,BudgetBreakdown.budgetSources) match the wiki shape and are exported throughshared/src/index.ts. - Component reuse:
Badgeis extended via the existingvariantsprop (no parallel implementation).BudgetSourceChipis built as a new shared component underclient/src/components/— complies with the "every new component must be a reusable shared component" rule. - Conventions:
import typeused consistently, ESM.jsextensions preserved, snake_case in SQL, camelCase in TS,/api/prefix unchanged. - Design tokens: 10 source slots (
--color-source-0..9) defined for both light and dark themes intokens.css. Chip/badge consume them via CSS custom properties — no hardcoded colors leak into components. - Performance: client filter is
O(N)tree walks memoized on(selectedSourceIds, wiAreas, hiAreas, perspective). Backend per-source aggregation is a single pass over already-builtwiEntityData/hiEntityData. No O(n²). - Test parity: every new/modified production file has a corresponding test file (
budgetBreakdownService.test.ts,budgetOverview.breakdown.test.ts,BudgetSourceChip.test.tsx,budgetSourceColors.test.ts, plus expandedCostBreakdownTable.test.tsx).
Medium findings (not blocking — defer to dev-team-lead / refinement)
server/src/services/budgetBreakdownService.tsdeclareswiBudgetLineSourceMap(lines 570–574) andhiBudgetLineSourceMap(lines 734–738) and populates them but never reads them. The per-source aggregation later iterateswiEntityData/hiEntityDatadirectly vialine.budgetSourceId. Remove the dead maps.client/src/lib/budgetSourceColors.ts:SourceBadgeStyleKeyincludes the literalsource0, butgetSourceBadgeStyleKey(null)returns'sourceUnassigned', sosource0is unreachable through that function. The.source0class still exists inBadge.module.cssas a defensive fallback. Either narrow the type to'source1' | … | 'source9' | 'sourceUnassigned'or drop.source0from CSS — pick one.
Palette nit (informational)
- Light-mode
--color-source-7reuses--color-blue-100/--color-blue-800(identical to source 1) — only the dot color differs. With 10 deterministic slots and a small house-build user base this is unlikely to bite, but if two simultaneously-used sources hash to slots 1 and 7 the chips will look near-identical. Worth a future palette pass.
Architecture compliant. No changes required from the architect role.
steilerDev
left a comment
There was a problem hiding this comment.
[security-engineer]
No blocking findings. Approving this PR from a security perspective.
SQL parameterization — The two new column additions (wib.budget_source_id, hib.budget_source_id) use Drizzle's sql tagged template literals, consistent with the established safe query pattern throughout the service. The new budgetSourceRows aggregate query (SELECT id, name, total_amount FROM budget_sources ORDER BY name ASC) contains no user-controlled input and no interpolated values — clean.
Authorization scope — The GET /api/budget/breakdown endpoint is registered under the global preValidation session hook (auth.ts:84), which applies to all routes. The new budgetSources aggregate query reads from the single-tenant SQLite database with no cross-user data boundary to enforce — consistent with the existing breakdown queries. No elevation or bypass introduced.
Data leakage — budgetSourceId is a project-scoped UUID (same sensitivity as work item IDs already in the response). The BudgetSourceSummaryBreakdown shape exposed is { id, name, totalAmount, projectedMin, projectedMax } — no internal fields, no secrets.
Frontend XSS — Source name and chip label are rendered via React JSX text nodes throughout (no dangerouslySetInnerHTML, no innerHTML, no eval). The title prop added to Badge is forwarded as a standard HTML attribute — React escapes it automatically.
URL filter state — ?sources=2,5,8 query param is parsed as a numeric Set client-side; it drives selectedSourceIds used for local filtering only, never sent back to the server unvalidated. No SSRF or injection vector.
No findings.
|
[product-owner] Product Owner review: APPROVE (posted as comment because the API does not allow self-approval on the orchestrator account). All 8 acceptance criteria from #1354 are met. Mapping below. AC-1: Source badge on every Level 3 row — PASS. AC-2: Stable, token-driven color identity — PASS. AC-3: Per-source impact under Available Funds — PASS. CostBreakdownTable.tsx:1303-1338 renders AC-4: Filter chips with multi-select OR semantics — PASS. AC-5: URL persistence via AC-6: Empty state when filter matches nothing — PASS. CostBreakdownTable.tsx:1378-1389 renders AC-7: Accessibility — PASS. Native AC-8: Responsive + dark mode — PASS. Chip strip wraps with CI: All gates green (Quality Gates + E2E Gates + 16 E2E shards × 3 viewports + Coverage Report). No blocking gaps. Ready to merge. |
steilerDev
left a comment
There was a problem hiding this comment.
[ux-designer] Design review for PR #1355 — Source Attribution Badges & Per-Source Filter.
What was verified
- Token family
--color-source-N-{bg,text,dot}(N=0–9), Layer 2 + Layer 3 dark mode overrides Badgeextended withsource0–source9+sourceUnassignedclasses (no parallel implementation)BudgetSourceChipas a new shared component with scoped--chip-bg/text/dotpattern- Filter toolbar
role="toolbar", chips witharia-pressed, Escape key + focus refocus - Mobile dot-only badge (
.sourceBadgeLabel/.sourceBadgeDotswap via media query) prefers-reduced-motionguard on chip transitions and row animation- Live region (
role="status") for filter change announcements
Findings
Medium — Hardcoded hex/rgba in tokens.css (slots 5–9 and some dark-mode overrides)
Slots 5, 6, 8, 9 in Layer 2 use raw hex values instead of Layer 1 palette token references:
/* ACTUAL (Layer 2) */
--color-source-5-bg: #e9d5ff; /* should be var(--color-purple-100) */
--color-source-5-text: #6b21a8; /* should be var(--color-purple-800) */
--color-source-5-dot: #a855f7; /* should be var(--color-purple-500) */
--color-source-6-bg: #cffafe; /* var(--color-cyan-100) */
--color-source-6-text: #0c4a6e; /* var(--color-cyan-900) */
--color-source-6-dot: #06b6d4; /* var(--color-cyan-500) */
/* … slots 8, 9, and several dark-mode overrides similarly */If purple and cyan palette tokens are not yet in Layer 1, they should be added there first, then referenced via var() — consistent with how slots 1–4 use var(--color-blue-N), var(--color-green-N), etc. Slot 7 dark-mode dot (#ec4899) and slots 8/9 dark text/dot are also raw hex. Fix: add missing Layer 1 palette tokens and update all Layer 2/3 source-slot values to reference them.
Medium — Slot 7 (Pink) shares the same bg/text tokens as Slot 1 (Blue) in light mode
--color-source-7-bg: var(--color-blue-100); /* identical to slot 1 */
--color-source-7-text: var(--color-blue-800); /* identical to slot 1 */
--color-source-7-dot: #ec4899; /* only the dot differs */Slot 7 is named "Pink" but its background and label look identical to Slot 1 (Blue). The dot color is different, but in the badge the dot is 8px and the background+text is the primary visual. Users with 7+ sources will see two slots that look the same. Fix: use a pink/rose palette for bg and text (e.g., --color-rose-100 / --color-rose-800) so the badge is visually distinct.
Medium — .sourceFilterStrip missing flex-wrap: nowrap on mobile
The spec requires the chip strip to scroll horizontally on mobile (flex-wrap: nowrap; overflow-x: auto). The implemented base rule sets flex-wrap: wrap, with no @media (max-width: 767px) override to switch to nowrap. As a result, chips wrap onto multiple lines on small screens instead of scrolling. Fix:
@media (max-width: 767px) {
.sourceFilterStrip {
flex-wrap: nowrap;
}
}Low — animation: fadeIn 0.15s ease uses a hardcoded duration
The spec called for var(--transition-normal) here. The chip transitions correctly use the token; the row fade-in does not:
/* ACTUAL */
animation: fadeIn 0.15s ease;
/* SHOULD BE */
animation: fadeIn var(--transition-normal) ease;Checklist
| Item | Status |
|---|---|
Token family --color-source-N-{bg,text,dot} added to Layer 2 + Layer 3 |
PASS (but slots 5/6/8/9 hardcoded — medium) |
All source badge classes in Badge.module.css use var() references |
PASS |
Badge extended (not duplicated); title prop added |
PASS |
BudgetSourceChip shared component, scoped --chip-* props |
PASS |
role="toolbar", aria-pressed chips |
PASS |
| Escape key clears filter + refocuses Available Funds button | PASS |
Live region role="status" announcements |
PASS |
Mobile dot-only badge (.sourceBadgeLabel hidden) |
PASS |
| 44px touch targets on mobile | PASS |
prefers-reduced-motion guard on transitions and animation |
PASS |
Dark mode token overrides in [data-theme="dark"] |
PASS (but raw hex in several slots — medium) |
| Mobile chip strip horizontal scroll (nowrap) | FAIL (wraps instead — medium) |
Row fadeIn uses var(--transition-normal) |
FAIL (hardcoded 0.15s — low) |
Verdict
Non-blocking. The three medium findings (hardcoded palette values in source token slots, slot 7 pink/blue collision, missing mobile flex-wrap: nowrap on the strip) and one low finding (literal 0.15s animation duration) should be addressed before merge or in a follow-up. No accessibility or dark-mode usability blockers.
|
🎉 This PR is included in version 2.4.0-beta.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
…with Cost/Payback/Net columns (#1357) * feat(budget): rework per-source filter to use source rows as toggles with Cost/Payback/Net columns - Source detail rows under Available Funds become the filter affordance: each <tr role="button" aria-pressed> toggles its source on click, Space, or Enter; Escape selects all - All sources start selected by default; URL state stores deselections via ?deselectedSources=<id1>,<id2> - Deselected rows are visually dimmed (text muted, dot opacity 0.4, no left-border accent); aria-pressed conveys state - Items, areas, and any container with no surviving budget lines render null (cascade beyond just lines) - Available Funds total recomputes to the sum of currently-selected sources; "(X of Y selected)" caption shown when filter is active - New per-source columns: Cost (perspective-resolved sum), Payback (entity-level pro-rata, computed client-side), Net = totalAmount + payback - cost - Remove BudgetSourceChip component, chip filter strip, and obsolete English/German i18n keys - Live region moved outside table wrapper so announcements survive empty-state toggles Closes #1356 This supersedes the chip-toolbar UX shipped in #1354/PR #1355 per user feedback. Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) <noreply@anthropic.com> Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com> Co-Authored-By: Claude translator (Sonnet 4.6) <noreply@anthropic.com> Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) <noreply@anthropic.com> Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.5) <noreply@anthropic.com> Co-Authored-By: Claude ux-designer (Sonnet 4.6) <noreply@anthropic.com> Co-Authored-By: Claude product-architect (Opus 4.6) <noreply@anthropic.com> Co-Authored-By: Claude product-owner (Opus 4.6) <noreply@anthropic.com> * fix(budget): resolve i18n key collision and stale test assertions - Rename i18n key overview.costBreakdown.availableFunds (object) to availableFundsFilter to avoid colliding with the same-named string ("Available funds"). JSON last-write-wins meant the label was being overwritten by the caption object, breaking 3 unit tests and the rendered Available Funds row label. - Scope getByText('€200,000.00') to the Available Funds row via within() to disambiguate from the source detail row's Net column showing the same currency value. - Relax the "1 of 2" caption regex to "<digit> of <digit>" — the fixture includes an unassigned line so the total is N+1 named sources. - Replace the className-comparison dark-mode smoke check with an aria-pressed attribute assertion since deselected rows are styled via attribute selectors, not class toggles. Fixes #1356 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 e2e-test-engineer (Sonnet 4.5) <noreply@anthropic.com> Co-Authored-By: Claude translator (Sonnet 4.5) <noreply@anthropic.com> * test(budget): fix two stale CostBreakdownTable scenarios for #1356 - Remove the second fireEvent.click in the work-item cascade test — the 'No Area' container is also cascade-hidden, so its expand button is never rendered. Asserting 'Sourced Work Item' is absent after the WI section expands is sufficient. - Update the "expand shows sub-rows with name and Net value" test to set projectedMin/projectedMax to 0 on the source summaries so the Net column equals totalAmount; previously the default 5000/8000 values produced a non-zero Cost making Net != totalAmount. Fixes #1356 Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) <noreply@anthropic.com> * test(budget): use toHaveText auto-retry for live region + bump mobile row padding - Replace `textContent()` + `toMatch` with `toHaveText` regex assertion on the filter live region. The previous synchronous read could land before React re-rendered the announcement after the chip-row click. - Bump mobile row vertical padding from spacing-3 (12px) to spacing-4 (16px) so the source detail row's bounding box meets the 44px touch target on mobile viewports. Fixes #1356 Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.5) <noreply@anthropic.com> Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com> * fix(budget): include Unassigned in selected count for caption + live region Both the "(X of Y selected)" caption and the role="status" live region computed selected count without considering the Unassigned pseudo-source. When the user deselects the Unassigned chip, the previous expression (budgetSources.length - deselectedSourceIds.size) drifted off-by-one because deselectedSourceIds may contain the literal 'unassigned' key that isn't in budgetSources. Both expressions now compute selected as named-selected + (hasUnassignedLines && !deselectedSourceIds.has('unassigned') ? 1 : 0) matching the existing total formula budgetSources.length + (hasUnassignedLines ? 1 : 0). Fixes #1356 Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com> * test(budget): update caption test to use locale-agnostic regex The off-by-one fix correctly counts the Unassigned pseudo-source as a virtual source. The fixture (2 named + 1 unassigned = 3 virtual) now shows "(2 of 3 selected)" after deselecting one source — the previous "/1\D+\d+/" regex no longer matches. Use toHaveText with a generic "<digit> of <digit>" regex so the assertion is locale-agnostic and robust to count semantics. Fixes #1356 Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.5) <noreply@anthropic.com> --------- Co-authored-by: Frank Steiler <frank@steiler.de> Co-authored-by: Claude dev-team-lead (Sonnet 4.6) <noreply@anthropic.com>
|
🎉 This PR is included in version 2.4.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
?sources=<id>,<id>); subtotals and grand totals recompute against the filtered set.budgetSourceIdand a per-responsebudgetSourcesaggregate.Closes #1354
Test plan
/api/budget/breakdowncarries new fields)Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) noreply@anthropic.com