From 5a1ce42034831d511334a529029b17c9433a901e Mon Sep 17 00:00:00 2001 From: le king fu Date: Tue, 7 Jul 2026 19:07:20 -0400 Subject: [PATCH] refactor(reports): extract a generic leaf-driven tree builder Factor the hand-rolled hierarchy walk out of buildCompareTree into a generic buildLeafDrivenTree(leaves, opts): the shared skeleton (leaf/ orphan split, ancestor "relevant" set, DB-order adjacency, buildNode recursion with the MAX_TREE_DEPTH guard + loop-break, magnitude sibling ordering, orphan append, contiguous income->expense->transfer section sort) now lives in one place, parameterised by injection points (categoryIdOf, makeLeaf, makeSubtotal, decorateDirectLeaf, makeOrphan, sortKey, isSubtotal, sectionOf, sectionOrder, maxDepth). buildCompareTree becomes a thin specialisation supplying only the delta arithmetic; output is byte-identical (compare tests unchanged as the non-regression guard). Exported (with TreeCatMeta / TreeSectionType / LeafDrivenTreeOptions) so the trends tree reuses the exact same skeleton. Behavior-preserving refactor: no user-facing change, no CHANGELOG entry, no DB migration. Resolves #263 Co-Authored-By: Claude Opus 4.8 (1M context) --- src/services/reportService.ts | 340 +++++++++++++++++++++------------- 1 file changed, 216 insertions(+), 124 deletions(-) diff --git a/src/services/reportService.ts b/src/services/reportService.ts index 046ae5a..ec2fedd 100644 --- a/src/services/reportService.ts +++ b/src/services/reportService.ts @@ -435,71 +435,118 @@ function monthBoundaries(year: number, month: number): { start: string; end: str return { start: `${year}-${mm}-01`, end: `${year}-${mm}-${dd}` }; } -// --- Compare hierarchy (Issue #247) --- +// --- Generic leaf-driven tree builder (Issue #263) --- + +/** Section key shared by every hierarchical report (income-statement order). */ +export type TreeSectionType = "expense" | "income" | "transfer"; /** - * 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). + * Minimal category metadata every hierarchical report needs: id, display + * name/color, income-statement `type`, and `parent_id` to walk the hierarchy. + * Shared by the compare tree (Issue #247) and the trends tree (Issue #264). */ -interface CompareCatMeta { +export interface TreeCatMeta { id: number; name: string; color: string | null; - type: "expense" | "income" | "transfer" | null; + type: TreeSectionType | null; parent_id: number | null; } -const COMPARE_CATEGORIES_SQL = `SELECT id, name, color, type, parent_id FROM categories`; - -// Income statement reading order: revenues first, then expenses, then the -// (netted) transfers — the two result lines are injected around them in the UI. -const COMPARE_TYPE_ORDER: Record = { income: 0, expense: 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. + * Injection points that specialise the generic tree builder for a concrete row + * type `T`. `T` is BOTH the flat leaf payload fed in and the enriched row shape + * emitted — leaf/subtotal rows just add the hierarchy fields (`parent_id`, + * `is_parent`, `depth`, …) the caller writes in `makeLeaf` / `makeSubtotal`. */ -export function buildCompareTree( - leaves: CategoryDelta[], - categories: CompareCatMeta[], -): CategoryDelta[] { - const catById = new Map(); +export interface LeafDrivenTreeOptions { + /** Category hierarchy (fetched without an is_active filter — keeps history). */ + categories: TreeCatMeta[]; + /** Category a leaf belongs to; a null/absent id → orphan (top-level leaf). */ + categoryIdOf: (leaf: T) => number | null; + /** Enrich a leaf payload into a leaf row parented at `parentId` / `depth`. */ + makeLeaf: (cat: TreeCatMeta, leaf: T, parentId: number | null, depth: number) => T; + /** Reduce a node's descendant leaves into its subtotal (is_parent) row. */ + makeSubtotal: ( + cat: TreeCatMeta, + descendantLeaves: T[], + parentId: number | null, + depth: number, + ) => T; + /** Annotate the "(direct)" leaf of a parent that also has own transactions. */ + decorateDirectLeaf: (leafRow: T, cat: TreeCatMeta) => T; + /** Enrich an orphan (Uncategorized / hard-deleted) payload into a root leaf. */ + makeOrphan: (leaf: T) => T; + /** Magnitude used to order siblings (descending) — read off a block's head. */ + sortKey: (row: T) => number; + /** True when a row is a subtotal, so descendant *leaves* can be summed. */ + isSubtotal: (row: T) => boolean; + /** Section key of a row for the final contiguous type sort. */ + sectionOf: (row: T) => string; + /** Section order, income-first e.g. `{ income: 0, expense: 1, transfer: 2 }`. */ + sectionOrder: Record; + /** Rank for a section absent from `sectionOrder` (default 9 — sorts last). */ + unknownSectionRank?: number; + /** Cyclic-chain depth cap (default `MAX_TREE_DEPTH`). */ + maxDepth?: number; +} + +/** + * Generic leaf-driven hierarchy builder. Turns a flat list of per-category + * leaves into a parent/child tree with subtotal rows, ordered by magnitude + * within each type and grouped into contiguous income → expense → transfer + * sections. Driven by the leaves that actually exist (not the full category + * list), so netted-to-zero groups and soft-deleted categories with history are + * preserved and no empty rows appear. + * + * Behavior — including the `MAX_TREE_DEPTH` guard and buildNode loop-break + * against a corrupted parent_id cycle — is shared verbatim by every + * hierarchical report; the row arithmetic is injected via `opts`. Extracted + * from `buildCompareTree` (Issue #263) so the trends tree (Issue #264) reuses + * the exact same skeleton. + */ +export function buildLeafDrivenTree(leaves: T[], opts: LeafDrivenTreeOptions): T[] { + const { + categories, + categoryIdOf, + makeLeaf, + makeSubtotal, + decorateDirectLeaf, + makeOrphan, + sortKey, + isSubtotal, + sectionOf, + sectionOrder, + unknownSectionRank = 9, + maxDepth = MAX_TREE_DEPTH, + } = opts; + + 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); + // Flat leaf lookup by category id. Rows whose id is null (Uncategorized) or + // absent from the table (hard-deleted) become orphans: depth-0 leaves + // preserved in their incoming order. + const leafByCat = new Map(); + const orphans: T[] = []; + for (const l of leaves) { + const id = categoryIdOf(l); + if (id != null && catById.has(id)) { + leafByCat.set(id, l); } else { - orphans.push(d); + orphans.push(l); } } - // "Relevant" = every category that has a delta plus all of its ancestors. + // "Relevant" = every category that has a leaf plus all of its ancestors. const relevant = new Set(); - for (const id of deltaByCat.keys()) { + for (const id of leafByCat.keys()) { let cur: number | null | undefined = id; let guard = 0; - while (cur != null && guard <= MAX_TREE_DEPTH) { + while (cur != null && guard <= maxDepth) { if (relevant.has(cur)) break; relevant.add(cur); cur = catById.get(cur)?.parent_id ?? null; @@ -508,7 +555,7 @@ export function buildCompareTree( } // Adjacency among relevant categories only, preserving DB order. - const childrenByParent = new Map(); + 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); @@ -517,93 +564,36 @@ export function buildCompareTree( } } - 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 + rows: T[]; + sortKey: number; // magnitude 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 => { + // (recursively built) child blocks, siblings ordered by magnitude desc. + const buildNode = (cat: TreeCatMeta, 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); + const children = depth >= maxDepth ? [] : (childrenByParent.get(cat.id) ?? []); + const hasDirect = leafByCat.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 leaf = makeLeaf(cat, leafByCat.get(cat.id)!, cat.parent_id ?? null, depth); + return { rows: [leaf], sortKey: sortKey(leaf) }; } 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. + // rows. 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), - }); + const direct = decorateDirectLeaf( + makeLeaf(cat, leafByCat.get(cat.id)!, cat.id, depth + 1), + cat, + ); + childBlocks.push({ rows: [direct], sortKey: sortKey(direct) }); } for (const child of children) { const b = buildNode(child, depth + 1); @@ -612,9 +602,9 @@ export function buildCompareTree( 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) }; + const descendantLeaves = childRows.filter((r) => !isSubtotal(r)); + const subtotal = makeSubtotal(cat, descendantLeaves, cat.parent_id ?? null, depth); + return { rows: [subtotal, ...childRows], sortKey: sortKey(subtotal) }; }; // Roots = relevant categories with no relevant parent. Ordered by magnitude. @@ -629,17 +619,15 @@ export function buildCompareTree( 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" }); - } + for (const l of orphans) rows.push(makeOrphan(l)); - // Stable sort by type so sections (income → expense → transfer) are + // Stable sort by section so types (income → expense → transfer) are // contiguous; magnitude/tree order within a type is preserved via the index. - const order = new Map(); + 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; + const ta = sectionOrder[sectionOf(a)] ?? unknownSectionRank; + const tb = sectionOrder[sectionOf(b)] ?? unknownSectionRank; if (ta !== tb) return ta - tb; return order.get(a)! - order.get(b)!; }); @@ -647,6 +635,110 @@ export function buildCompareTree( return rows; } +// --- 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). + */ +type CompareCatMeta = TreeCatMeta; + +const COMPARE_CATEGORIES_SQL = `SELECT id, name, color, type, parent_id FROM categories`; + +// Income statement reading order: revenues first, then expenses, then the +// (netted) transfers — the two result lines are injected around them in the UI. +const COMPARE_TYPE_ORDER: Record = { income: 0, expense: 1, transfer: 2 }; + +/** + * 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. + * + * A thin specialisation of `buildLeafDrivenTree` (Issue #263): the generic + * skeleton drives the hierarchy walk, sibling ordering, depth guard and section + * sort; this only supplies the delta arithmetic. Behavior is byte-identical to + * the previous hand-rolled builder — the compare tests are the guard. + * + * Exported for unit testing. + */ +export function buildCompareTree( + leaves: CategoryDelta[], + categories: CompareCatMeta[], +): CategoryDelta[] { + const typeOf = (c: TreeCatMeta): TreeSectionType => c.type ?? "expense"; + + return buildLeafDrivenTree(leaves, { + categories, + categoryIdOf: (d) => d.categoryId, + makeLeaf: (cat, d, parentId, depth) => ({ + ...d, + parent_id: parentId, + is_parent: false, + depth, + category_type: typeOf(cat), + }), + makeSubtotal: (cat, descendantLeaves, parentId, depth) => { + 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), + }; + }, + // A parent with both children and its own transactions surfaces the direct + // spend as a "(direct)" leaf (mirrors getBudgetVsActualData). + decorateDirectLeaf: (row, cat) => ({ ...row, categoryName: `${cat.name} (direct)` }), + // Orphans keep their own already-computed delta; only the hierarchy fields + // are stamped. Uncategorized rows (category_type undefined) → 'expense'. + makeOrphan: (d) => ({ + ...d, + parent_id: null, + is_parent: false, + depth: 0, + category_type: d.category_type ?? "expense", + }), + sortKey: (row) => Math.abs(row.deltaAbs), + isSubtotal: (row) => row.is_parent === true, + sectionOf: (row) => row.category_type ?? "expense", + sectionOrder: COMPARE_TYPE_ORDER, + }); +} + function previousMonth(year: number, month: number): { year: number; month: number } { if (month === 1) return { year: year - 1, month: 12 }; return { year, month: month - 1 }; -- 2.45.2