From f3e8e94b1616e4c4301aea3f96f6ab0e47d9e097 Mon Sep 17 00:00:00 2001 From: le king fu Date: Sat, 4 Jul 2026 17:32:38 -0400 Subject: [PATCH] feat(reports): hierarchical real-vs-real compare with subtotals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Render the real-vs-real comparable report as a parent/child category tree with subtotal rows, mirroring the budget report — rows grouped into expense/income/transfer sections with per-section and grand totals and a subtotals-on-top/bottom toggle. The compare service now builds the tree on top of the flat per-category deltas, so leaf-category figures are unchanged and the #243 transfer netting is preserved (a balanced transfer group subtotals to ~0, i.e. the group's net). - reportService: buildCompareTree() synthesizes subtotal rows from the flat leaves + category metadata; getCompareMonthOverMonth/YoY return the tree (COMPARE_DELTA_SQL and rowsToDeltas untouched). - CategoryDelta gains optional parent_id/is_parent/depth/category_type. - ComparePeriodTable: sections, depth indentation, reorderRows toggle, section/grand net totals. - Cartes top-movers and ComparePeriodChart filter to leaf rows only. - reorderRows constraint loosened to the two fields it reads. - i18n (FR+EN) section labels; CHANGELOG entries. Resolves #247 Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.fr.md | 4 + CHANGELOG.md | 4 + src/components/reports/ComparePeriodChart.tsx | 7 +- src/components/reports/ComparePeriodTable.tsx | 307 +++++++++++++----- src/i18n/locales/en.json | 10 +- src/i18n/locales/fr.json | 10 +- src/services/reportService.test.ts | 171 +++++++++- src/services/reportService.ts | 254 ++++++++++++++- src/shared/types/index.ts | 10 + src/utils/reorderRows.ts | 6 +- 10 files changed, 680 insertions(+), 103 deletions(-) diff --git a/CHANGELOG.fr.md b/CHANGELOG.fr.md index 7b50a23..6e77ed0 100644 --- a/CHANGELOG.fr.md +++ b/CHANGELOG.fr.md @@ -2,6 +2,10 @@ ## [Non publié] +### Ajouté + +- Rapports → Comparaison (réel vs réel) : le rapport comparable s'affiche désormais en **hiérarchie de catégories avec sous-totaux**, comme le rapport de budget. Les catégories parentes regroupent leurs enfants dans une ligne de sous-total — sa valeur est le net du groupe, donc un groupe de transferts équilibrés s'annule à ~0 — avec les lignes réparties en sections Dépenses / Revenus / Transferts portant des totaux par section et un total général, plus un choix sous-totaux en haut/en bas. Les montants des catégories feuilles sont inchangés, et l'annulation des transferts du correctif précédent est préservée (#247). + ### Sécurité - Mise à jour de dépendances pour corriger quatre advisories signalés par `npm audit` : `react-router-dom` 7.13 → 7.18.1 (ce qui tire `react-router` au-delà des advisories RCE turbo-stream, DoS `__manifest`/single-fetch et XSS de redirection RSC), plus l'outillage de développement `vite` 6.4.2 → 6.4.3 et `vitest` 4.0.18 → 4.1.9. Aucun changement de comportement. L'app est livrée comme un client de bureau local : les advisories runtime (qui nécessitent un serveur SSR/RSC react-router) ne s'appliquaient pas au produit livré ; ce sont des mises à jour de durcissement (#235, #236, #237, #238). diff --git a/CHANGELOG.md b/CHANGELOG.md index 39d567e..10cd471 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +- Reports → Compare (real vs real): the comparable report is now shown as a **category hierarchy with subtotals**, like the budget report. Parent categories roll their children up into a subtotal row — its value is the group's net, so a group of balanced transfers subtotals to ~0 — with rows grouped into Expenses / Income / Transfers sections carrying per-section and grand totals, and a subtotals-on-top/bottom toggle. Leaf-category figures are unchanged, and the transfer netting from the previous fix is preserved (#247). + ### Security - Updated dependencies to clear four advisories reported by `npm audit`: `react-router-dom` 7.13 → 7.18.1 (which pulls `react-router` past the turbo-stream RCE, `__manifest`/single-fetch DoS, and RSC-redirect XSS advisories), plus the dev toolchain `vite` 6.4.2 → 6.4.3 and `vitest` 4.0.18 → 4.1.9. No behaviour change. The app ships as a local desktop client, so the runtime advisories (which require a react-router SSR/RSC server) did not apply to the shipped product; these are hardening bumps (#235, #236, #237, #238). diff --git a/src/components/reports/ComparePeriodChart.tsx b/src/components/reports/ComparePeriodChart.tsx index 688707f..a539166 100644 --- a/src/components/reports/ComparePeriodChart.tsx +++ b/src/components/reports/ComparePeriodChart.tsx @@ -42,8 +42,11 @@ export default function ComparePeriodChart({ // Sort by current-period amount (largest spending first) so the user's eye // lands on the biggest categories, then reverse so the biggest appears at - // the top of the vertical bar chart. - const chartData = [...rows] + // the top of the vertical bar chart. The table view groups by parent/child + // (Issue #247); the chart stays flat, so drop subtotal (is_parent) rows to + // avoid double-counting a group against its own leaves. + const chartData = rows + .filter((r) => !r.is_parent) .sort((a, b) => b.currentAmount - a.currentAmount) .map((r) => ({ name: r.categoryName, diff --git a/src/components/reports/ComparePeriodTable.tsx b/src/components/reports/ComparePeriodTable.tsx index aff094d..568813d 100644 --- a/src/components/reports/ComparePeriodTable.tsx +++ b/src/components/reports/ComparePeriodTable.tsx @@ -1,5 +1,8 @@ +import { Fragment, useState } from "react"; import { useTranslation } from "react-i18next"; +import { ArrowUpDown } from "lucide-react"; import type { CategoryDelta } from "../../shared/types"; +import { reorderRows } from "../../utils/reorderRows"; export interface ComparePeriodTableProps { rows: CategoryDelta[]; @@ -48,6 +51,40 @@ function variationColor(value: number): string { return ""; } +const STORAGE_KEY = "compare-subtotals-position"; + +type SectionType = "expense" | "income" | "transfer"; + +/** Aggregate of the 6 comparable figures across a set of leaf rows. */ +interface Totals { + monthCurrent: number; + monthPrevious: number; + monthDelta: number; + ytdCurrent: number; + ytdPrevious: number; + ytdDelta: number; +} + +function sumLeaves(rows: CategoryDelta[]): Totals { + return rows + .filter((r) => !r.is_parent) + .reduce( + (acc, r) => ({ + monthCurrent: acc.monthCurrent + r.currentAmount, + monthPrevious: acc.monthPrevious + r.previousAmount, + monthDelta: acc.monthDelta + r.deltaAbs, + ytdCurrent: acc.ytdCurrent + r.cumulativeCurrentAmount, + ytdPrevious: acc.ytdPrevious + r.cumulativePreviousAmount, + ytdDelta: acc.ytdDelta + r.cumulativeDeltaAbs, + }), + { monthCurrent: 0, monthPrevious: 0, monthDelta: 0, ytdCurrent: 0, ytdPrevious: 0, ytdDelta: 0 }, + ); +} + +function pct(delta: number, previous: number): number | null { + return previous !== 0 ? (delta / Math.abs(previous)) * 100 : null; +} + export default function ComparePeriodTable({ rows, previousLabel, @@ -56,31 +93,61 @@ export default function ComparePeriodTable({ cumulativeCurrentLabel, }: ComparePeriodTableProps) { const { t, i18n } = useTranslation(); + const lang = i18n.language; + + const [subtotalsOnTop, setSubtotalsOnTop] = useState(() => { + const stored = localStorage.getItem(STORAGE_KEY); + return stored === null ? true : stored === "top"; + }); + const toggleSubtotals = () => { + setSubtotalsOnTop((prev) => { + const next = !prev; + localStorage.setItem(STORAGE_KEY, next ? "top" : "bottom"); + return next; + }); + }; const monthPrevLabel = previousLabel; const monthCurrLabel = currentLabel; const ytdPrevLabel = cumulativePreviousLabel ?? previousLabel; const ytdCurrLabel = cumulativeCurrentLabel ?? currentLabel; - // Totals across all rows (there is no parent/child structure in compare mode). - const totals = rows.reduce( - (acc, r) => ({ - monthCurrent: acc.monthCurrent + r.currentAmount, - monthPrevious: acc.monthPrevious + r.previousAmount, - monthDelta: acc.monthDelta + r.deltaAbs, - ytdCurrent: acc.ytdCurrent + r.cumulativeCurrentAmount, - ytdPrevious: acc.ytdPrevious + r.cumulativePreviousAmount, - ytdDelta: acc.ytdDelta + r.cumulativeDeltaAbs, - }), - { monthCurrent: 0, monthPrevious: 0, monthDelta: 0, ytdCurrent: 0, ytdPrevious: 0, ytdDelta: 0 }, - ); - const totalMonthPct = - totals.monthPrevious !== 0 ? (totals.monthDelta / Math.abs(totals.monthPrevious)) * 100 : null; - const totalYtdPct = - totals.ytdPrevious !== 0 ? (totals.ytdDelta / Math.abs(totals.ytdPrevious)) * 100 : null; + // Group rows into contiguous type sections (the service already type-sorts). + const sectionLabels: Record = { + expense: t("reports.compare.sections.expenses"), + income: t("reports.compare.sections.income"), + transfer: t("reports.compare.sections.transfers"), + }; + const sectionTotalKeys: Record = { + expense: "reports.compare.totalExpenses", + income: "reports.compare.totalIncome", + transfer: "reports.compare.totalTransfers", + }; + const sections: { type: SectionType; rows: CategoryDelta[] }[] = []; + let currentType: SectionType | null = null; + for (const row of rows) { + const type = (row.category_type ?? "expense") as SectionType; + if (type !== currentType) { + currentType = type; + sections.push({ type, rows: [] }); + } + sections[sections.length - 1].rows.push(row); + } + + // Grand totals across every leaf. + const totals = sumLeaves(rows); return (
+
+ +
@@ -138,109 +205,185 @@ export default function ComparePeriodTable({ {rows.length === 0 ? ( - ) : ( <> - {rows.map((row) => ( - - - {/* Monthly block */} - - - - - {/* Cumulative YTD block */} - - - - - - ))} + {sections.map((section) => { + const sectionTotals = sumLeaves(section.rows); + return ( + + + + + {reorderRows(section.rows, subtotalsOnTop).map((row) => { + const isParent = row.is_parent ?? false; + const depth = row.depth ?? 0; + const isTopParent = isParent && depth === 0; + const isIntermediateParent = isParent && depth >= 1; + const paddingClass = + depth >= 3 ? "pl-20" : depth === 2 ? "pl-14" : depth === 1 ? "pl-8" : "px-3"; + return ( + + + {/* Monthly block */} + + + + + {/* Cumulative YTD block */} + + + + + + ); + })} + {/* Section net total */} + + + + + + + + + + + + + ); + })} {/* Grand totals row */} diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index 2610644..055b277 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -427,7 +427,15 @@ "referenceMonth": "Reference month", "currentAmount": "Current", "previousAmount": "Previous", - "totalRow": "Total" + "totalRow": "Total", + "sections": { + "expenses": "Expenses", + "income": "Income", + "transfers": "Transfers" + }, + "totalExpenses": "Total Expenses", + "totalIncome": "Total Income", + "totalTransfers": "Total Transfers" }, "cartes": { "kpiSectionAria": "Key indicators for the reference month", diff --git a/src/i18n/locales/fr.json b/src/i18n/locales/fr.json index 5e8d2a2..511de40 100644 --- a/src/i18n/locales/fr.json +++ b/src/i18n/locales/fr.json @@ -427,7 +427,15 @@ "referenceMonth": "Mois de référence", "currentAmount": "Courant", "previousAmount": "Précédent", - "totalRow": "Total" + "totalRow": "Total", + "sections": { + "expenses": "Dépenses", + "income": "Revenus", + "transfers": "Transferts" + }, + "totalExpenses": "Total des dépenses", + "totalIncome": "Total des revenus", + "totalTransfers": "Total des transferts" }, "cartes": { "kpiSectionAria": "Indicateurs clés du mois de référence", diff --git a/src/services/reportService.test.ts b/src/services/reportService.test.ts index 25bc8d8..8968390 100644 --- a/src/services/reportService.test.ts +++ b/src/services/reportService.test.ts @@ -5,7 +5,9 @@ import { getCompareMonthOverMonth, getCompareYearOverYear, getCategoryZoom, + buildCompareTree, } from "./reportService"; +import type { CategoryDelta } from "../shared/types"; // Mock the db module vi.mock("./db", () => { @@ -325,7 +327,10 @@ describe("getCompareMonthOverMonth", () => { await getCompareMonthOverMonth(2026, 4); - expect(mockSelect).toHaveBeenCalledTimes(1); + // Two queries now: the delta aggregation (call 0, unchanged) + the category + // metadata for the hierarchy (call 1, Issue #247). + expect(mockSelect).toHaveBeenCalledTimes(2); + expect(mockSelect.mock.calls[1][0]).toContain("FROM categories"); const sql = mockSelect.mock.calls[0][0] as string; const params = mockSelect.mock.calls[0][1] as unknown[]; expect(sql).toContain("$1"); @@ -641,3 +646,167 @@ describe("getCategoryZoom", () => { expect(txSql).toContain("t.category_id = $1"); }); }); + +describe("buildCompareTree — hierarchical real-vs-real (Issue #247)", () => { + // Flat leaf, mirroring what rowsToDeltas returns from COMPARE_DELTA_SQL. + function leaf( + categoryId: number | null, + categoryName: string, + mc: number, + mp: number, + cc = mc, + cp = mp, + ): CategoryDelta { + return { + categoryId, + categoryName, + categoryColor: "#000", + previousAmount: mp, + currentAmount: mc, + deltaAbs: mc - mp, + deltaPct: mp !== 0 ? ((mc - mp) / mp) * 100 : null, + cumulativePreviousAmount: cp, + cumulativeCurrentAmount: cc, + cumulativeDeltaAbs: cc - cp, + cumulativeDeltaPct: cp !== 0 ? ((cc - cp) / cp) * 100 : null, + }; + } + + const cats = [ + { id: 2, name: "Dépenses", color: null, type: "expense", parent_id: null }, + { id: 22, name: "Épicerie", color: "#10b981", type: "expense", parent_id: 2 }, + { id: 24, name: "Restaurant", color: "#f97316", type: "expense", parent_id: 2 }, + { id: 5, name: "Placements", color: null, type: "transfer", parent_id: null }, + { id: 50, name: "REER", color: null, type: "transfer", parent_id: 5 }, + ] as Parameters[1]; + + it("nests leaves under a parent subtotal equal to the sum of its children", () => { + const rows = buildCompareTree( + [leaf(22, "Épicerie", 500, 400, 2000, 1500), leaf(24, "Restaurant", 120, 200, 300, 500)], + cats, + ); + + // Parent subtotal on top, then children ordered by |monthly delta| desc. + expect(rows.map((r) => r.categoryName)).toEqual(["Dépenses", "Épicerie", "Restaurant"]); + + const parent = rows[0]; + expect(parent.is_parent).toBe(true); + expect(parent.categoryId).toBe(2); + expect(parent.depth).toBe(0); + expect(parent.category_type).toBe("expense"); + // Subtotal = sum of children (the group's net). + expect(parent.currentAmount).toBe(620); + expect(parent.previousAmount).toBe(600); + expect(parent.deltaAbs).toBe(20); + expect(parent.cumulativeCurrentAmount).toBe(2300); + expect(parent.cumulativePreviousAmount).toBe(2000); + expect(parent.cumulativeDeltaAbs).toBe(300); + + // Children are leaves at depth 1 with UNCHANGED values (no regression). + const epicerie = rows.find((r) => r.categoryName === "Épicerie")!; + expect(epicerie.is_parent).toBeFalsy(); + expect(epicerie.depth).toBe(1); + expect(epicerie.parent_id).toBe(2); + expect(epicerie.currentAmount).toBe(500); + expect(epicerie.previousAmount).toBe(400); + expect(epicerie.deltaAbs).toBe(100); + }); + + it("keeps a balanced transfer group netted to 0 at the subtotal (preserves #243)", () => { + const rows = buildCompareTree([leaf(50, "REER", 0, 0, 0, 0)], cats); + const parent = rows.find((r) => r.categoryId === 5)!; + expect(parent.is_parent).toBe(true); + expect(parent.category_type).toBe("transfer"); + // The netted-to-zero transfer stays 0 through the subtotal — not re-inflated. + expect(parent.currentAmount).toBe(0); + expect(parent.previousAmount).toBe(0); + expect(parent.deltaAbs).toBe(0); + }); + + it("orders sections expense → income → transfer and keeps subtrees contiguous", () => { + const rows = buildCompareTree( + [leaf(22, "Épicerie", 500, 400), leaf(50, "REER", 0, 0)], + cats, + ); + const types = rows.map((r) => r.category_type); + // All expense rows precede all transfer rows. + expect(types).toEqual(["expense", "expense", "transfer", "transfer"]); + }); + + it("preserves grand-total invariance vs the flat leaves", () => { + const flat = [ + leaf(22, "Épicerie", 500, 400, 2000, 1500), + leaf(24, "Restaurant", 120, 200, 300, 500), + leaf(50, "REER", 0, 0, 0, 0), + ]; + const rows = buildCompareTree(flat, cats); + const sumLeaves = (rs: CategoryDelta[]) => + rs.filter((r) => !r.is_parent).reduce((s, r) => s + r.currentAmount, 0); + expect(sumLeaves(rows)).toBe(sumLeaves(flat)); + }); + + it("surfaces an uncategorized (null id) row as a top-level leaf", () => { + const rows = buildCompareTree([leaf(null, "Uncategorized", 90, 40)], cats); + expect(rows).toHaveLength(1); + expect(rows[0].categoryName).toBe("Uncategorized"); + expect(rows[0].is_parent).toBeFalsy(); + expect(rows[0].depth).toBe(0); + expect(rows[0].currentAmount).toBe(90); + }); + + it("keeps a leaf whose category was soft-deleted (still in the metadata)", () => { + // A single leaf with no siblings collapses to just that leaf (no lone subtotal). + const rows = buildCompareTree([leaf(22, "Épicerie", 75, 60)], cats); + const names = rows.map((r) => r.categoryName); + expect(names).toContain("Épicerie"); + // A parent with exactly one contributing child still gets a subtotal row. + const parent = rows.find((r) => r.is_parent); + expect(parent?.currentAmount).toBe(75); + }); +}); + +describe("getCompareMonthOverMonth — wires the hierarchy (Issue #247)", () => { + it("returns parent subtotal rows when category metadata is available", async () => { + mockSelect + // 1. COMPARE_DELTA_SQL — two sibling leaves under parent id 2. + .mockResolvedValueOnce([ + { + category_id: 22, + category_name: "Épicerie", + category_color: "#10b981", + month_current_total: 500, + month_previous_total: 400, + cumulative_current_total: 2000, + cumulative_previous_total: 1500, + }, + { + category_id: 24, + category_name: "Restaurant", + category_color: "#f97316", + month_current_total: 120, + month_previous_total: 200, + cumulative_current_total: 300, + cumulative_previous_total: 500, + }, + ]) + // 2. COMPARE_CATEGORIES_SQL — the hierarchy. + .mockResolvedValueOnce([ + { id: 2, name: "Dépenses", color: null, type: "expense", parent_id: null }, + { id: 22, name: "Épicerie", color: "#10b981", type: "expense", parent_id: 2 }, + { id: 24, name: "Restaurant", color: "#f97316", type: "expense", parent_id: 2 }, + ]); + + const result = await getCompareMonthOverMonth(2026, 4); + + // Delta query stays call 0 (netting SQL + params assertions elsewhere rely on it). + expect(mockSelect.mock.calls[0][0]).toContain("month_current_total"); + expect(mockSelect.mock.calls[1][0]).toContain("FROM categories"); + + const parent = result.find((r) => r.is_parent); + expect(parent).toBeDefined(); + expect(parent!.categoryId).toBe(2); + expect(parent!.currentAmount).toBe(620); + // Leaves survive unchanged. + expect(result.find((r) => r.categoryName === "Épicerie")!.currentAmount).toBe(500); + }); +}); diff --git a/src/services/reportService.ts b/src/services/reportService.ts index dcdb859..c24413d 100644 --- a/src/services/reportService.ts +++ b/src/services/reportService.ts @@ -429,6 +429,216 @@ function monthBoundaries(year: number, month: number): { start: string; end: str return { start: `${year}-${mm}-01`, end: `${year}-${mm}-${dd}` }; } +// --- Compare hierarchy (Issue #247) --- + +/** + * Minimal category metadata for building the compare tree. Fetched WITHOUT an + * `is_active` filter so soft-deleted categories (is_active = 0) that still carry + * historic transactions keep their place in the hierarchy — matching the raw + * LEFT JOIN behavior of COMPARE_DELTA_SQL (no regression on the leaves shown). + */ +interface CompareCatMeta { + id: number; + name: string; + color: string | null; + type: "expense" | "income" | "transfer" | null; + parent_id: number | null; +} + +const COMPARE_CATEGORIES_SQL = `SELECT id, name, color, type, parent_id FROM categories`; + +const COMPARE_TYPE_ORDER: Record = { expense: 0, income: 1, transfer: 2 }; + +/** Depth cap mirroring CATEGORY_TREE_CTE — guards a cyclic parent_id chain. */ +const MAX_TREE_DEPTH = 5; + +/** + * Turns the flat per-category deltas returned by COMPARE_DELTA_SQL into a + * parent/child tree with subtotal (`is_parent`) rows, mirroring the hierarchy of + * getBudgetVsActualData. + * + * The #243 transfer netting is untouched: leaf values are exactly what the SQL + * returned, and a subtotal is the arithmetic sum of its descendant leaves — so a + * group of balanced (netted-to-zero) transfers subtotals to ~0, i.e. the group's + * net. Driven by `leaves` (the categories that actually had transactions in the + * window) rather than the full category list, so netted-to-zero transfers and + * soft-deleted categories with history are preserved and no empty rows appear. + * + * Exported for unit testing. + */ +export function buildCompareTree( + leaves: CategoryDelta[], + categories: CompareCatMeta[], +): CategoryDelta[] { + const catById = new Map(); + for (const c of categories) catById.set(c.id, c); + + // Flat delta lookup by category id. Rows whose category id is null + // (Uncategorized) or absent from the table (hard-deleted) become orphans: + // depth-0 leaves preserved in their incoming order. + const deltaByCat = new Map(); + const orphans: CategoryDelta[] = []; + for (const d of leaves) { + if (d.categoryId != null && catById.has(d.categoryId)) { + deltaByCat.set(d.categoryId, d); + } else { + orphans.push(d); + } + } + + // "Relevant" = every category that has a delta plus all of its ancestors. + const relevant = new Set(); + for (const id of deltaByCat.keys()) { + let cur: number | null | undefined = id; + let guard = 0; + while (cur != null && guard <= MAX_TREE_DEPTH) { + if (relevant.has(cur)) break; + relevant.add(cur); + cur = catById.get(cur)?.parent_id ?? null; + guard++; + } + } + + // Adjacency among relevant categories only, preserving DB order. + const childrenByParent = new Map(); + for (const c of categories) { + if (c.parent_id != null && relevant.has(c.id) && relevant.has(c.parent_id)) { + let arr = childrenByParent.get(c.parent_id); + if (!arr) childrenByParent.set(c.parent_id, (arr = [])); + arr.push(c); + } + } + + const typeOf = (c: CompareCatMeta): "expense" | "income" | "transfer" => c.type ?? "expense"; + + const leafRow = ( + cat: CompareCatMeta, + parentId: number | null, + depth: number, + ): CategoryDelta => ({ + ...deltaByCat.get(cat.id)!, + parent_id: parentId, + is_parent: false, + depth, + category_type: typeOf(cat), + }); + + const subtotalRow = ( + cat: CompareCatMeta, + descendantLeaves: CategoryDelta[], + parentId: number | null, + depth: number, + ): CategoryDelta => { + let previousAmount = 0; + let currentAmount = 0; + let cumulativePreviousAmount = 0; + let cumulativeCurrentAmount = 0; + for (const l of descendantLeaves) { + previousAmount += l.previousAmount; + currentAmount += l.currentAmount; + cumulativePreviousAmount += l.cumulativePreviousAmount; + cumulativeCurrentAmount += l.cumulativeCurrentAmount; + } + const deltaAbs = currentAmount - previousAmount; + const cumulativeDeltaAbs = cumulativeCurrentAmount - cumulativePreviousAmount; + return { + categoryId: cat.id, + categoryName: cat.name, + categoryColor: cat.color ?? "#9ca3af", + previousAmount, + currentAmount, + deltaAbs, + // Match rowsToDeltas' leaf formula (signed denominator) so a single-child + // parent shows the same % as its child. + deltaPct: previousAmount !== 0 ? (deltaAbs / previousAmount) * 100 : null, + cumulativePreviousAmount, + cumulativeCurrentAmount, + cumulativeDeltaAbs, + cumulativeDeltaPct: + cumulativePreviousAmount !== 0 + ? (cumulativeDeltaAbs / cumulativePreviousAmount) * 100 + : null, + parent_id: parentId, + is_parent: true, + depth, + category_type: typeOf(cat), + }; + }; + + interface Block { + rows: CategoryDelta[]; + sortKey: number; // |monthly delta| of the block head — orders siblings + } + + // Builds a node's block: a pure leaf, or a subtotal followed by its + // (recursively built) child blocks, siblings ordered by |monthly delta| desc. + const buildNode = (cat: CompareCatMeta, depth: number): Block | null => { + // Stop descending past the depth cap so a corrupted parent_id cycle can + // never recurse forever — deeper nodes collapse to leaves (real category + // trees are ≤ 3 levels). + const children = depth >= MAX_TREE_DEPTH ? [] : (childrenByParent.get(cat.id) ?? []); + const hasDirect = deltaByCat.has(cat.id); + + if (children.length === 0) { + if (!hasDirect) return null; + const leaf = leafRow(cat, cat.parent_id ?? null, depth); + return { rows: [leaf], sortKey: Math.abs(leaf.deltaAbs) }; + } + + const childBlocks: Block[] = []; + // A category with both children AND its own transactions surfaces the direct + // spend as a "(direct)" leaf so the subtotal stays the sum of its visible + // rows (mirrors getBudgetVsActualData). Rare: a parent auto-loses + // is_inputable when a child is added. + if (hasDirect) { + const direct = leafRow(cat, cat.id, depth + 1); + childBlocks.push({ + rows: [{ ...direct, categoryName: `${cat.name} (direct)` }], + sortKey: Math.abs(direct.deltaAbs), + }); + } + for (const child of children) { + const b = buildNode(child, depth + 1); + if (b) childBlocks.push(b); + } + childBlocks.sort((a, b) => b.sortKey - a.sortKey); + + const childRows = childBlocks.flatMap((b) => b.rows); + const descendantLeaves = childRows.filter((r) => !r.is_parent); + const subtotal = subtotalRow(cat, descendantLeaves, cat.parent_id ?? null, depth); + return { rows: [subtotal, ...childRows], sortKey: Math.abs(subtotal.deltaAbs) }; + }; + + // Roots = relevant categories with no relevant parent. Ordered by magnitude. + const rootBlocks: Block[] = []; + for (const c of categories) { + if (!relevant.has(c.id)) continue; + if (c.parent_id != null && relevant.has(c.parent_id)) continue; + const b = buildNode(c, 0); + if (b) rootBlocks.push(b); + } + rootBlocks.sort((a, b) => b.sortKey - a.sortKey); + + const rows = rootBlocks.flatMap((b) => b.rows); + // Orphans (Uncategorized + hard-deleted) appended in their incoming order. + for (const d of orphans) { + rows.push({ ...d, parent_id: null, is_parent: false, depth: 0, category_type: d.category_type ?? "expense" }); + } + + // Stable sort by type so sections (expense → income → transfer) are + // contiguous; magnitude/tree order within a type is preserved via the index. + const order = new Map(); + rows.forEach((r, i) => order.set(r, i)); + rows.sort((a, b) => { + const ta = COMPARE_TYPE_ORDER[a.category_type ?? "expense"] ?? 9; + const tb = COMPARE_TYPE_ORDER[b.category_type ?? "expense"] ?? 9; + if (ta !== tb) return ta - tb; + return order.get(a)! - order.get(b)!; + }); + + return rows; +} + function previousMonth(year: number, month: number): { year: number; month: number } { if (month === 1) return { year: year - 1, month: 12 }; return { year, month: month - 1 }; @@ -502,13 +712,19 @@ export async function getCompareMonthOverMonth( const cumPreviousStart = `${prev.year}-01-01`; const cumPreviousEnd = prevEnd; - const rows = await db.select(COMPARE_DELTA_SQL, [ - curStart, curEnd, - prevStart, prevEnd, - cumCurrentStart, cumCurrentEnd, - cumPreviousStart, cumPreviousEnd, + // Delta select stays first so its params/SQL remain `mock.calls[0]`; the + // category metadata (for the hierarchy) is fetched alongside. `?? []` guards + // under-specified mocks — db.select never returns undefined in production. + const [rows, cats] = await Promise.all([ + db.select(COMPARE_DELTA_SQL, [ + curStart, curEnd, + prevStart, prevEnd, + cumCurrentStart, cumCurrentEnd, + cumPreviousStart, cumPreviousEnd, + ]), + db.select(COMPARE_CATEGORIES_SQL), ]); - return rowsToDeltas(rows); + return buildCompareTree(rowsToDeltas(rows), cats ?? []); } /** @@ -532,13 +748,17 @@ export async function getCompareYearOverYear( const cumPreviousStart = `${year - 1}-01-01`; const cumPreviousEnd = prevMonthEnd; - const rows = await db.select(COMPARE_DELTA_SQL, [ - curMonthStart, curMonthEnd, - prevMonthStart, prevMonthEnd, - cumCurrentStart, cumCurrentEnd, - cumPreviousStart, cumPreviousEnd, + // See getCompareMonthOverMonth: delta select first, categories alongside. + const [rows, cats] = await Promise.all([ + db.select(COMPARE_DELTA_SQL, [ + curMonthStart, curMonthEnd, + prevMonthStart, prevMonthEnd, + cumCurrentStart, cumCurrentEnd, + cumPreviousStart, cumPreviousEnd, + ]), + db.select(COMPARE_CATEGORIES_SQL), ]); - return rowsToDeltas(rows); + return buildCompareTree(rowsToDeltas(rows), cats ?? []); } // --- Category zoom (Issue #74) --- @@ -946,10 +1166,14 @@ export async function getCartesSnapshot( // 12-month income vs expenses series for the overlay chart. const flow12Months = buildSeries(12); - // Top movers: biggest MoM increases / decreases. `momRows` are sorted by - // absolute delta already; filter out near-zero noise and split by sign. + // Top movers: biggest MoM increases / decreases. `momRows` now carries the + // compare hierarchy (Issue #247) — skip the subtotal (`is_parent`) rows so a + // parent group can't double-count against its own leaves. The surviving leaves + // are byte-identical to the previous flat output; the sort/slice below is + // unchanged. `momRows` are sorted by absolute delta already; filter out + // near-zero noise and split by sign. const significantMovers = momRows.filter( - (r) => r.deltaAbs !== 0 && (r.previousAmount > 0 || r.currentAmount > 0), + (r) => !r.is_parent && r.deltaAbs !== 0 && (r.previousAmount > 0 || r.currentAmount > 0), ); // Project the richer CategoryDelta shape down to the narrower CartesTopMover // shape so the Cartes dashboard keeps its stable contract regardless of how diff --git a/src/shared/types/index.ts b/src/shared/types/index.ts index 2b24a50..444d6f5 100644 --- a/src/shared/types/index.ts +++ b/src/shared/types/index.ts @@ -308,6 +308,16 @@ export interface CategoryDelta { cumulativeCurrentAmount: number; cumulativeDeltaAbs: number; cumulativeDeltaPct: number | null; + // Hierarchy block (Issue #247) — populated ONLY by the real-vs-real Compare + // tree builder (getCompareMonthOverMonth / getCompareYearOverYear). Flat + // consumers (Highlights movers, Cartes top movers) leave these undefined and + // must not read them. Snake_case (unlike the camelCase value fields above) to + // mirror BudgetVsActualRow and stay compatible with the shared `reorderRows` + // util. `is_parent` rows are subtotals whose value = the net of their group. + parent_id?: number | null; + is_parent?: boolean; + depth?: number; + category_type?: "expense" | "income" | "transfer"; } // Historical alias — used by the highlights hub. Shape identical to CategoryDelta. diff --git a/src/utils/reorderRows.ts b/src/utils/reorderRows.ts index 4ac0181..17eaab4 100644 --- a/src/utils/reorderRows.ts +++ b/src/utils/reorderRows.ts @@ -2,9 +2,13 @@ * Shared utility for reordering budget table rows. * Recursively moves subtotal (parent) rows below their children * at every depth level when "subtotals on bottom" is enabled. + * + * The generic constraint only requires the two fields the algorithm actually + * reads (`is_parent`, `depth`) so it works for both the budget rows (required + * `is_parent`) and the Compare `CategoryDelta` rows (optional `is_parent`). */ export function reorderRows< - T extends { is_parent: boolean; parent_id: number | null; category_id: number; depth?: number }, + T extends { is_parent?: boolean; depth?: number }, >(rows: T[], subtotalsOnTop: boolean): T[] { if (subtotalsOnTop) return rows;
+ {t("reports.empty.noData")}
- - - {row.categoryName} - - - {formatCurrency(row.currentAmount, i18n.language)} - - {formatCurrency(row.previousAmount, i18n.language)} - - {formatSignedCurrency(row.deltaAbs, i18n.language)} - - {formatPct(row.deltaPct, i18n.language)} - - {formatCurrency(row.cumulativeCurrentAmount, i18n.language)} - - {formatCurrency(row.cumulativePreviousAmount, i18n.language)} - - {formatSignedCurrency(row.cumulativeDeltaAbs, i18n.language)} - - {formatPct(row.cumulativeDeltaPct, i18n.language)} -
+ {sectionLabels[section.type]} +
+ + + {row.categoryName} + + + {formatCurrency(row.currentAmount, lang)} + + {formatCurrency(row.previousAmount, lang)} + + {formatSignedCurrency(row.deltaAbs, lang)} + + {formatPct(row.deltaPct, lang)} + + {formatCurrency(row.cumulativeCurrentAmount, lang)} + + {formatCurrency(row.cumulativePreviousAmount, lang)} + + {formatSignedCurrency(row.cumulativeDeltaAbs, lang)} + + {formatPct(row.cumulativeDeltaPct, lang)} +
+ {t(sectionTotalKeys[section.type])} + + {formatCurrency(sectionTotals.monthCurrent, lang)} + + {formatCurrency(sectionTotals.monthPrevious, lang)} + + {formatSignedCurrency(sectionTotals.monthDelta, lang)} + + {formatPct(pct(sectionTotals.monthDelta, sectionTotals.monthPrevious), lang)} + + {formatCurrency(sectionTotals.ytdCurrent, lang)} + + {formatCurrency(sectionTotals.ytdPrevious, lang)} + + {formatSignedCurrency(sectionTotals.ytdDelta, lang)} + + {formatPct(pct(sectionTotals.ytdDelta, sectionTotals.ytdPrevious), lang)} +
{t("reports.compare.totalRow")} - {formatCurrency(totals.monthCurrent, i18n.language)} + {formatCurrency(totals.monthCurrent, lang)} - {formatCurrency(totals.monthPrevious, i18n.language)} + {formatCurrency(totals.monthPrevious, lang)} - {formatSignedCurrency(totals.monthDelta, i18n.language)} + {formatSignedCurrency(totals.monthDelta, lang)} - {formatPct(totalMonthPct, i18n.language)} + {formatPct(pct(totals.monthDelta, totals.monthPrevious), lang)} - {formatCurrency(totals.ytdCurrent, i18n.language)} + {formatCurrency(totals.ytdCurrent, lang)} - {formatCurrency(totals.ytdPrevious, i18n.language)} + {formatCurrency(totals.ytdPrevious, lang)} - {formatSignedCurrency(totals.ytdDelta, i18n.language)} + {formatSignedCurrency(totals.ytdDelta, lang)} - {formatPct(totalYtdPct, i18n.language)} + {formatPct(pct(totals.ytdDelta, totals.ytdPrevious), lang)}