From 61c7fa0d67f13d839472ac41fd7c334562ea94b0 Mon Sep 17 00:00:00 2001 From: RyuseiTaniguchi Date: Wed, 2 Sep 2026 21:52:18 +0900 Subject: [PATCH] fix(reports): stop the Spending tab crowning its own data-quality gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The information-design third of #147, built to the Spending tab mock. The summary bar read Total Spent · Categories · Top: X. "Categories: 18" is a number no decision turns on, and the crown went to `data[0]` blindly — so whenever Uncategorized was the largest line, which is the usual case, the page put a trophy on the spending it had failed to identify. Three tiles replace it. The headline names the range it measured ("Total spent · Jun 2 – Sep 2") and splits the figure into categorized and uncategorized, so the share that is unaccounted for is stated rather than decorated. The second says what the comparison is against and that it is the same length. The third gives spending as a share of income received, which is a number a decision does turn on. Uncategorized also drew `CHART_COLORS[0]`, the most saturated blue, making "we do not know" the visual hero of the chart. It takes `--chart-neutral` now, the treatment the synthetic "Other" slice already had, in both the chart and the table's category tile. Uncategorized is not a category — it is the absence of one — and should not look like the loudest. Bar becomes the default view. Nine categories spanning three orders of magnitude is a size comparison, which bars read directly. A blank cell in the Change column meant two different things. `prevTotal` defaulted to `0` for a category with no baseline row, and `ComparisonBadge` rendered nothing for zero — so "this category is new" and "this category did not move" were the same empty cell. `prevTotal` is `number | null` now, null meaning no baseline row (and null throughout when no comparison period was requested at all), and the badge renders "New". The MCP tool reports null rather than 0 for the same reason: a consumer should not be able to compute a change against a period the category was not in. The date chip said "Last 3 months" and the resolved dates appeared nowhere on the page. It prints them. Also added: a % of total column, so a row's share is readable without dividing by the headline. Verified against demo data with a temporary uncategorized charge and a category with no baseline: Uncategorized reads $3,200.00 · 18.8% · New with a neutral bar, Property Tax reads $450.00 · 2.6% · New, and the headline splits $17,055.33 into $13,855.33 categorized and $3,200.00 uncategorized at 73.8% of $23,100.00 received. Rows removed afterwards. Refs #147 --- src/app/(dashboard)/reports/page.tsx | 15 +++- src/components/atoms/spending-chart.tsx | 13 +-- src/components/molecules/comparison-badge.tsx | 30 ++++--- .../molecules/date-range-popover.tsx | 17 ++++ .../organisms/report-filter-bar.tsx | 10 +++ src/components/organisms/report-spending.tsx | 89 +++++++++++++++---- src/components/organisms/report-tabs.tsx | 4 + src/lib/comparison-state.test.ts | 35 ++++++++ src/lib/comparison-state.ts | 24 +++++ src/lib/mcp/tools/reports.ts | 5 +- src/queries/reports.ts | 11 ++- tests/integration/report-drill-down.test.ts | 29 ++++++ 12 files changed, 241 insertions(+), 41 deletions(-) create mode 100644 src/lib/comparison-state.test.ts create mode 100644 src/lib/comparison-state.ts diff --git a/src/app/(dashboard)/reports/page.tsx b/src/app/(dashboard)/reports/page.tsx index 536518e7..99d7ff8e 100644 --- a/src/app/(dashboard)/reports/page.tsx +++ b/src/app/(dashboard)/reports/page.tsx @@ -82,12 +82,20 @@ export default async function ReportsPage({ let sankeyData; let safeToSpendData; let cashFlowBarData; + let spendingTotalIncome; switch (tab) { - case "spending": - spendingData = await withHousehold(householdId, (tx) => - getSpendingByCategory(householdId, filters, tx, compPeriod)); + case "spending": { + // Income comes along for the share-of-income figure in the headline. + const [spending, income] = await Promise.all([ + withHousehold(householdId, (tx) => + getSpendingByCategory(householdId, filters, tx, compPeriod)), + withHousehold(householdId, (tx) => getIncomeVsExpense(householdId, filters, tx)), + ]); + spendingData = spending; + spendingTotalIncome = income.reduce((s, r) => s + r.income, 0); break; + } case "income-expense": { const [ie, ieCat] = await Promise.all([ withHousehold(householdId, (tx) => getIncomeVsExpense(householdId, filters, tx)), @@ -139,6 +147,7 @@ export default async function ReportsPage({ cashFlowBarData={cashFlowBarData} safeToSpendData={safeToSpendData} comparisonLabel={compLabel} + spendingTotalIncome={spendingTotalIncome} dateFrom={dateFrom} dateTo={dateTo} accountIds={accountIds} diff --git a/src/components/atoms/spending-chart.tsx b/src/components/atoms/spending-chart.tsx index 49782ced..ca248103 100644 --- a/src/components/atoms/spending-chart.tsx +++ b/src/components/atoms/spending-chart.tsx @@ -19,13 +19,14 @@ interface SpendingChartProps { onItemClick?: (item: { id: string | null; name: string }) => void; } -// The aggregated "Other" row (built below from categories past the top 8) is -// not a real category — give it a fixed neutral color instead of cycling back -// into CHART_COLORS, which would collide with an earlier slice's color. Real -// uncategorized spend also carries a null id, so key off the synthetic flag -// rather than the id to avoid recoloring legitimate rows. +// Two rows take the neutral rather than a palette slot. "Other" (rolled up from +// categories past the top 8) is not a real category, and cycling back into +// CHART_COLORS would collide with an earlier slice. Uncategorized is not a +// category either — it is the absence of one, and as the largest slice in most +// households it was taking CHART_COLORS[0], the loudest blue, making "we do not +// know" the visual hero of the chart. function colorAt(item: SpendingChartItem, i: number): string { - if (item.synthetic) return "var(--chart-neutral)"; + if (item.synthetic || item.id === null) return "var(--chart-neutral)"; return CHART_COLORS[i % CHART_COLORS.length]; } diff --git a/src/components/molecules/comparison-badge.tsx b/src/components/molecules/comparison-badge.tsx index 48ee5d1b..3092cf35 100644 --- a/src/components/molecules/comparison-badge.tsx +++ b/src/components/molecules/comparison-badge.tsx @@ -1,4 +1,5 @@ import { TrendingUp, TrendingDown, Minus } from "lucide-react"; +import { comparisonState } from "@/lib/comparison-state"; interface ComparisonBadgeProps { current: number; @@ -9,20 +10,25 @@ interface ComparisonBadgeProps { } export function ComparisonBadge({ current, previous, periodLabel, pill, invertColor }: ComparisonBadgeProps) { - if (previous === null || previous === 0) { - if (pill) { - return ( - - — - - ); - } - return null; + const state = comparisonState(current, previous); + + // A category with no baseline row is new. It used to render as an empty cell, + // which read exactly like "no change". + if (state.kind === "new") { + return ( + + New + + ); } - const change = ((current - previous) / previous) * 100; - const isUp = change > 0; - const isFlat = Math.abs(change) < 0.5; + const change = state.percent; + const isUp = state.kind === "up"; + const isFlat = state.kind === "flat"; return ( Date: {triggerValue} + {triggerDetail && ( + + )} ) : ( "Date" diff --git a/src/components/organisms/report-filter-bar.tsx b/src/components/organisms/report-filter-bar.tsx index 2bad4c31..5e1fbf17 100644 --- a/src/components/organisms/report-filter-bar.tsx +++ b/src/components/organisms/report-filter-bar.tsx @@ -69,6 +69,15 @@ export function ReportFilterBar({ accounts, categories }: ReportFilterBarProps) return null; })(); + // A preset names a window without saying which one. Resolve it here so the + // chip prints the dates the report below it actually used. + const dateDetail = (() => { + if (!dateActive || !effectivePreset) return null; + const { from, to } = rangeToDateBounds(effectivePreset); + if (!from) return null; + return `${formatDateShort(from)} – ${formatDateShort(to)}`; + })(); + function handleDatePreset(id: string) { const { from, to } = rangeToDateBounds(id); updateFilters({ from, to, preset: id === "all" ? null : id }); @@ -95,6 +104,7 @@ export function ReportFilterBar({ accounts, categories }: ReportFilterBarProps) selectedId={effectivePreset} active={dateActive} triggerValue={dateValue} + triggerDetail={dateDetail} from={fromParam ?? ""} to={toParam ?? ""} onSelectPreset={handleDatePreset} diff --git a/src/components/organisms/report-spending.tsx b/src/components/organisms/report-spending.tsx index 7f09ccb3..34499c53 100644 --- a/src/components/organisms/report-spending.tsx +++ b/src/components/organisms/report-spending.tsx @@ -1,11 +1,9 @@ "use client"; import { useState } from "react"; -import { Wallet, Layers, Crown } from "lucide-react"; import { CategoryIconTile } from "@/components/atoms/category-icon"; import { ChartViewToggle } from "@/components/atoms/chart-view-toggle"; import { SpendingChart } from "@/components/atoms/spending-chart"; -import { ReportSummaryBar, type SummaryItem } from "@/components/atoms/report-summary-bar"; import { ComparisonBadge } from "@/components/molecules/comparison-badge"; import { DrillDownSheet, type DrillDownFilter } from "@/components/organisms/drill-down-sheet"; import { @@ -19,6 +17,7 @@ import { import { centsToDisplay } from "@/lib/money"; import { activateOnKey } from "@/lib/a11y"; import { CHART_COLORS } from "@/lib/chart-colors"; +import { formatDateShort } from "@/lib/date-utils"; import type { SpendingRow } from "@/queries/reports"; interface ReportSpendingProps { @@ -27,6 +26,8 @@ interface ReportSpendingProps { dateFrom: string; dateTo: string; accountIds?: string[]; + /** Total income over the same range, for the share-of-income figure. */ + totalIncome?: number; } export function ReportSpending({ @@ -35,8 +36,11 @@ export function ReportSpending({ dateFrom, dateTo, accountIds, + totalIncome, }: ReportSpendingProps) { - const [view, setView] = useState<"donut" | "bar">("donut"); + // Nine categories spanning three orders of magnitude is a size comparison, + // which bars read directly and a donut does not. + const [view, setView] = useState<"donut" | "bar">("bar"); const [drillDown, setDrillDown] = useState(null); const chartData = data.map((r) => ({ @@ -46,14 +50,10 @@ export function ReportSpending({ })); const totalSpent = data.reduce((s, r) => s + r.total, 0); - const topCategory = data.length > 0 ? data[0] : null; - const summaryItems: SummaryItem[] = [ - { label: "Total Spent", value: totalSpent, color: "default", icon: Wallet }, - { label: "Categories", value: data.length, format: "number", icon: Layers }, - ...(topCategory - ? [{ label: `Top: ${topCategory.categoryName}`, value: topCategory.total, icon: Crown } as SummaryItem] - : []), - ]; + const uncategorized = data.find((r) => r.categoryId === null)?.total ?? 0; + const categorized = totalSpent - uncategorized; + const shareOfIncome = totalIncome && totalIncome > 0 ? (totalSpent / totalIncome) * 100 : null; + const rangeLabel = `${formatDateShort(dateFrom)} – ${formatDateShort(dateTo)}`; function handleDrillDown(item: { id: string | null; name: string }) { // Keep the null: it means "uncategorized", not "every category". @@ -66,7 +66,53 @@ export function ReportSpending({ return (
- + {/* The old bar read Total Spent · Categories · Top: X. "Categories: 18" is + a number no decision turns on, and the crown landed on Uncategorized + whenever it was the largest line — a trophy for a data-quality gap. + What a reader needs instead is how much of the total is unaccounted + for, and what the total is measured against. */} +
+
+
Total spent · {rangeLabel}
+
{centsToDisplay(totalSpent)}
+ {totalSpent > 0 && ( +
+ + + Categorized + {centsToDisplay(categorized)} + + + + Uncategorized + {centsToDisplay(uncategorized)} + +
+ )} +
+ +
+
Compared with
+
+ {compLabel ? compLabel.replace(/^vs\s+/, "") : "Nothing — showing all time"} +
+ {compLabel && ( +
the preceding period, same length
+ )} +
+ +
+
Share of income
+
+ {shareOfIncome === null ? "—" : `${shareOfIncome.toFixed(1)}%`} +
+
+ {totalIncome && totalIncome > 0 + ? `of ${centsToDisplay(totalIncome)} received` + : "no income recorded in this range"} +
+
+

Spending by Category

@@ -83,6 +129,7 @@ export function ReportSpending({ Category Amount + % of total {compLabel && Change} @@ -106,13 +153,20 @@ export function ReportSpending({
@@ -126,6 +180,9 @@ export function ReportSpending({ {centsToDisplay(row.total)} + + {totalSpent > 0 ? `${((row.total / totalSpent) * 100).toFixed(1)}%` : "—"} + {compLabel && ( { + test("a category absent from the baseline is new, not unchanged", () => { + // Both used to render as an empty cell, so "we have never seen this before" + // and "it did not move" looked identical in the Change column. + expect(comparisonState(5_000, null)).toEqual({ kind: "new" }); + }); + + test("a baseline of zero is also new — there was nothing to grow from", () => { + expect(comparisonState(5_000, 0)).toEqual({ kind: "new" }); + }); + + test("spending that went up", () => { + expect(comparisonState(150_00, 100_00)).toEqual({ kind: "up", percent: 50 }); + }); + + test("spending that came down", () => { + expect(comparisonState(50_00, 100_00)).toEqual({ kind: "down", percent: -50 }); + }); + + test("a move under half a percent reads as flat", () => { + expect(comparisonState(100_30, 100_00)).toEqual({ kind: "flat", percent: 0.3 }); + expect(comparisonState(100_00, 100_00)).toEqual({ kind: "flat", percent: 0 }); + }); + + test("half a percent is a move, not flat", () => { + expect(comparisonState(100_50, 100_00).kind).toBe("up"); + }); + + test("a category that spent nothing this period against a real baseline", () => { + expect(comparisonState(0, 100_00)).toEqual({ kind: "down", percent: -100 }); + }); +}); diff --git a/src/lib/comparison-state.ts b/src/lib/comparison-state.ts new file mode 100644 index 00000000..64ccc57b --- /dev/null +++ b/src/lib/comparison-state.ts @@ -0,0 +1,24 @@ +export type ComparisonState = + | { kind: "new" } + | { kind: "flat"; percent: number } + | { kind: "up"; percent: number } + | { kind: "down"; percent: number }; + +/** Below this, a move is noise rather than a trend. */ +const FLAT_THRESHOLD_PERCENT = 0.5; + +/** + * How this period compares with the baseline. + * + * A missing or zero baseline is reported as `new` rather than folded in with + * "no change": the Change column used to render an empty cell for both, so a + * category appearing for the first time was indistinguishable from one that had + * not moved. + */ +export function comparisonState(current: number, previous: number | null): ComparisonState { + if (previous === null || previous === 0) return { kind: "new" }; + + const percent = ((current - previous) / previous) * 100; + if (Math.abs(percent) < FLAT_THRESHOLD_PERCENT) return { kind: "flat", percent }; + return { kind: percent > 0 ? "up" : "down", percent }; +} diff --git a/src/lib/mcp/tools/reports.ts b/src/lib/mcp/tools/reports.ts index 29adf509..ac30ed05 100644 --- a/src/lib/mcp/tools/reports.ts +++ b/src/lib/mcp/tools/reports.ts @@ -39,8 +39,11 @@ export function registerReportTools(server: McpServer, householdId: string) { groupId: r.groupId, totalCents: r.total, totalDisplay: centsToDisplay(r.total), + // null means the category has no baseline row at all — new, rather + // than unchanged. Reporting it as 0 would let a consumer compute a + // change against a period the category was not in. prevTotalCents: r.prevTotal, - prevTotalDisplay: centsToDisplay(r.prevTotal), + prevTotalDisplay: r.prevTotal === null ? null : centsToDisplay(r.prevTotal), })), ); }, diff --git a/src/queries/reports.ts b/src/queries/reports.ts index 8f46e29b..f90771eb 100644 --- a/src/queries/reports.ts +++ b/src/queries/reports.ts @@ -37,7 +37,12 @@ export interface SpendingRow { groupId: string | null; categoryIcon: string | null; total: number; - prevTotal: number; + /** + * The same category's spending in the comparison period, or `null` when the + * category has no baseline row — it is new, which is not the same as + * unchanged. `null` also when no comparison period was requested. + */ + prevTotal: number | null; } export interface IncomeExpenseRow { @@ -66,7 +71,7 @@ export async function getSpendingByCategory( const currentSpending = await aggregateSpending(householdId, filters, db); const enriched = await enrichSpendingMap(currentSpending, db); - let prevMap = new Map(); + let prevMap: Map | null = null; if (comparisonPeriod) { prevMap = await aggregateSpending(householdId, { ...filters, ...comparisonPeriod }, db); } @@ -78,7 +83,7 @@ export async function getSpendingByCategory( groupId: row.groupId, categoryIcon: row.categoryIcon, total: row.value, - prevTotal: prevMap.get(row.id ?? "uncategorized") ?? 0, + prevTotal: prevMap?.get(row.id ?? "uncategorized") ?? null, })); } diff --git a/tests/integration/report-drill-down.test.ts b/tests/integration/report-drill-down.test.ts index b28c53de..1b2023f4 100644 --- a/tests/integration/report-drill-down.test.ts +++ b/tests/integration/report-drill-down.test.ts @@ -181,3 +181,32 @@ describe("an income drill-down explains its own figure", () => { expect(drill.rows.map((r) => r.name).sort()).toEqual(["Bonus", "Salary"]); }); }); + +describe("a category's comparison against the baseline", () => { + test("a category absent from the baseline reports no previous figure, not zero", async () => { + // Food spends in both periods; Salary's category is irrelevant here — what + // matters is that a category with no baseline row is distinguishable from + // one that spent nothing, which `?? 0` made impossible. + await insertTransaction(db, householdId, accountId, { date: "2026-03-05", normalizedAmount: -5000, amount: 5000, categoryId: foodCatId, name: "Grocery" }); + await insertTransaction(db, householdId, accountId, { date: "2026-02-10", normalizedAmount: -2000, amount: 2000, categoryId: foodCatId, name: "Grocery last period" }); + await insertTransaction(db, householdId, accountId, { date: "2026-03-06", normalizedAmount: -3000, amount: 3000, categoryId: null, name: "First ever uncategorized" }); + const { getSpendingByCategory } = await import("../../src/queries/reports"); + + const rows = await getSpendingByCategory(householdId, RANGE, db, { + dateFrom: "2026-02-01", + dateTo: "2026-02-28", + }); + + expect(rows.find((r) => r.categoryName === "Food")?.prevTotal).toBe(2000); + expect(rows.find((r) => r.categoryId === null)?.prevTotal).toBeNull(); + }); + + test("without a comparison period nothing claims a previous figure", async () => { + await insertTransaction(db, householdId, accountId, { date: "2026-03-05", normalizedAmount: -5000, amount: 5000, categoryId: foodCatId, name: "Grocery" }); + const { getSpendingByCategory } = await import("../../src/queries/reports"); + + const rows = await getSpendingByCategory(householdId, RANGE, db); + + expect(rows.every((r) => r.prevTotal === null)).toBe(true); + }); +});