src/hooks/useBudget.test.ts

@@ -2,6 +2,7 @@ import { describe, it, expect } from "vitest";
 import type { BudgetYearRow, Category, BudgetEntry } from "../shared/types";
 import { buildBudgetYearRows } from "./useBudget";
 import { sumLeavesForType } from "../components/budget/budgetTableResults";
+import { reorderRows } from "../utils/reorderRows";
 import {
   type CollapseAccessors,
   visibleRows,
@@ -9,10 +10,11 @@ import {
 } from "../utils/collapsibleRows";
 
 // The Budget grid's rows are built by `useBudget` (the "3rd builder" the
-// abandoned v1 collapse plan ignored). Its final sort is LEVEL-ORDER (depth
-// ascending / BFS), not depth-first. These tests pin that invariant: the
-// shipped ancestor-walk collapse (issue #288) is order-INDEPENDENT, but the v1
-// depth-cursor algorithm assumed DFS and would have broken exactly here.
+// abandoned v1 collapse plan ignored). Since issue #424 they are emitted
+// DEPTH-FIRST: each parent row is followed directly by its whole subtree, so an
+// expanded subcategory shows under its parent and `reorderRows` (subtotals at
+// the bottom) attaches each subtotal to its own rows. The collapse (ancestor
+// walk, issue #288) does not depend on the order.
 
 function cat(
   id: number,
@@ -62,29 +64,261 @@ const ACCESSORS: CollapseAccessors<BudgetYearRow> = {
   isParent: (row) => row.is_parent,
 };
 
-describe("buildBudgetYearRows — level-order emission (issue #289 invariant)", () => {
-  const rows = buildBudgetYearRows(CATEGORIES, [], []);
+/**
+ * Depth-first invariant: every non-root row comes after its parent row, and
+ * every row between a parent row and the end of its subtree is one of its
+ * descendants. A "(direct)" leaf carries its own category as parent: it is
+ * attached to the subtotal row of that category (the `p:` key of the grid).
+ */
+function expectDepthFirst(rows: BudgetYearRow[]) {
+  const parentIndex = new Map<number, number>();
+  rows.forEach((r, i) => {
+    if (r.is_parent) parentIndex.set(r.category_id, i);
+  });
+  // Ancestor ids of a row, climbing the parent subtotal rows.
+  const ancestorsOf = (r: BudgetYearRow): number[] => {
+    const out: number[] = [];
+    let pid = r.parent_id;
+    while (pid != null && !out.includes(pid)) {
+      out.push(pid);
+      const idx = parentIndex.get(pid);
+      if (idx === undefined) break;
+      pid = rows[idx].parent_id;
+    }
+    return out;
+  };
+  rows.forEach((r, i) => {
+    if (r.parent_id != null) {
+      const p = parentIndex.get(r.parent_id);
+      expect(p, `${r.category_name} has a parent row`).toBeDefined();
+      expect(p!, `${r.category_name} comes after its parent`).toBeLessThan(i);
+    }
+  });
+  rows.forEach((r, i) => {
+    if (!r.is_parent) return;
+    const depth = r.depth ?? 0;
+    // The subtree runs until the next row of this depth or shallower.
+    let j = i + 1;
+    while (j < rows.length && (rows[j].depth ?? 0) > depth) {
+      expect(ancestorsOf(rows[j]), `${rows[j].category_name} is under ${r.category_name}`).toContain(r.category_id);
+      j++;
+    }
+    // ...and every descendant of the parent lies inside that range.
+    rows.forEach((d, k) => {
+      if (k === i) return;
+      if (ancestorsOf(d).includes(r.category_id)) {
+        expect(k, `${d.category_name} lies inside the subtree of ${r.category_name}`).toBeGreaterThan(i);
+        expect(k, `${d.category_name} lies inside the subtree of ${r.category_name}`).toBeLessThan(j);
+      }
+    });
+  });
+}
 
-  it("emits income before expense, then each group in LEVEL order (BFS), not DFS", () => {
-    // Salary (income) first; then the Housing expense group depth-ascending:
-    // root subtotal, then the depth-1 rows (parent Utilities before leaf Rent),
-    // then the depth-2 grandchildren.
-    expect(rows.map((r) => r.category_id)).toEqual([10, 1, 3, 2, 4, 5]);
+const names = (rows: BudgetYearRow[]) => rows.map((r) => r.category_name);
+
+// Shape of the "Revenus" group of a new profile (consolidated_schema.sql): one
+// depth-1 leaf ("Autres revenus") and three intermediate parents.
+const REVENUS: Category[] = [
+  cat(1000, "Revenus", "income", { is_inputable: false, sort_order: 1 }),
+  cat(1010, "Emploi", "income", { parent_id: 1000, is_inputable: false, sort_order: 1 }),
+  cat(1011, "Paie régulière", "income", { parent_id: 1010, sort_order: 1 }),
+  cat(1012, "Primes & bonus", "income", { parent_id: 1010, sort_order: 2 }),
+  cat(1013, "Travail autonome", "income", { parent_id: 1010, sort_order: 3 }),
+  cat(1020, "Gouvernemental", "income", { parent_id: 1000, is_inputable: false, sort_order: 2 }),
+  cat(1021, "Remboursement impôt", "income", { parent_id: 1020, sort_order: 1 }),
+  cat(1022, "Allocations familiales", "income", { parent_id: 1020, sort_order: 2 }),
+  cat(1023, "Crédits TPS/TVQ", "income", { parent_id: 1020, sort_order: 3 }),
+  cat(1024, "Assurance-emploi / RQAP", "income", { parent_id: 1020, sort_order: 4 }),
+  cat(1030, "Revenus de placement", "income", { parent_id: 1000, is_inputable: false, sort_order: 3 }),
+  cat(1031, "Intérêts & dividendes", "income", { parent_id: 1030, sort_order: 1 }),
+  cat(1032, "Gains en capital", "income", { parent_id: 1030, sort_order: 2 }),
+  cat(1033, "Revenus locatifs", "income", { parent_id: 1030, sort_order: 3 }),
+  cat(1090, "Autres revenus", "income", { parent_id: 1000, sort_order: 9 }),
+];
+
+// Shape of "Finances & obligations" of a new profile: leaves before AND after
+// intermediate parents ("Retrait cash" sorts after "Impôts & taxes").
+const FINANCES: Category[] = [
+  cat(1900, "Finances & obligations", "expense", { is_inputable: false, sort_order: 10 }),
+  cat(1910, "Frais bancaires", "expense", { parent_id: 1900, is_inputable: false, sort_order: 1 }),
+  cat(1911, "Frais de compte", "expense", { parent_id: 1910, sort_order: 1 }),
+  cat(1912, "Intérêts & frais de crédit", "expense", { parent_id: 1910, sort_order: 2 }),
+  cat(1913, "Frais de change", "expense", { parent_id: 1910, sort_order: 3 }),
+  cat(1920, "Impôts & taxes", "expense", { parent_id: 1900, is_inputable: false, sort_order: 2 }),
+  cat(1921, "Impôt fédéral", "expense", { parent_id: 1920, sort_order: 1 }),
+  cat(1922, "Impôt provincial", "expense", { parent_id: 1920, sort_order: 2 }),
+  cat(1923, "Acomptes provisionnels", "expense", { parent_id: 1920, sort_order: 3 }),
+  cat(1930, "Dons & cotisations", "expense", { parent_id: 1900, is_inputable: false, sort_order: 3 }),
+  cat(1931, "Dons de charité", "expense", { parent_id: 1930, sort_order: 1 }),
+  cat(1932, "Cotisations professionnelles", "expense", { parent_id: 1930, sort_order: 2 }),
+  cat(1933, "Cotisations syndicales", "expense", { parent_id: 1930, sort_order: 3 }),
+  cat(1940, "Cadeaux", "expense", { parent_id: 1900, sort_order: 4 }),
+  cat(1945, "Retrait cash", "expense", { parent_id: 1900, sort_order: 5 }),
+  cat(1946, "Achats divers non catégorisés", "expense", { parent_id: 1900, sort_order: 6 }),
+];
+
+// Shape of the older taxonomy (default profile): an inputable root, so a
+// "(direct)" row, and one intermediate parent (Assurances, migration v7).
+const RECURRENTES: Category[] = [
+  cat(2, "Dépenses récurrentes", "expense", { sort_order: 2 }),
+  cat(20, "Loyer", "expense", { parent_id: 2, sort_order: 1 }),
+  cat(21, "Électricité", "expense", { parent_id: 2, sort_order: 2 }),
+  cat(31, "Assurances", "expense", { parent_id: 2, is_inputable: false, sort_order: 12 }),
+  cat(310, "Assurance-auto", "expense", { parent_id: 31, sort_order: 1 }),
+  cat(311, "Assurance-habitation", "expense", { parent_id: 31, sort_order: 2 }),
+  cat(312, "Assurance-vie", "expense", { parent_id: 31, sort_order: 3 }),
+  cat(32, "Pharmacie", "expense", { parent_id: 2, sort_order: 13 }),
+  cat(33, "Taxes municipales", "expense", { parent_id: 2, sort_order: 14 }),
+];
+
+describe("buildBudgetYearRows — depth-first emission (issue #424)", () => {
+  it("emits income before expense, each group depth-first: Utilities is followed by its own leaves", () => {
+    const rows = buildBudgetYearRows(CATEGORIES, [], []);
+    // Salary (income) first; then Housing, its children sorted by name with
+    // leaves and sub-groups mixed: Rent, then Utilities and its leaves.
+    expect(rows.map((r) => r.category_id)).toEqual([10, 1, 2, 3, 4, 5]);
+    expectDepthFirst(rows);
   });
 
-  it("keeps depth non-decreasing within a top group — the property that would have killed the v1 (DFS-assuming) collapse algorithm", () => {
-    const expenseGroup = rows.filter((r) => r.category_type === "expense");
-    const depths = expenseGroup.map((r) => r.depth ?? 0);
-    for (let i = 1; i < depths.length; i++) {
-      expect(depths[i]).toBeGreaterThanOrEqual(depths[i - 1]);
-    }
+  it("Revenus: the depth-1 leaf sorts first by name, each sub-group carries its own leaves", () => {
+    const rows = buildBudgetYearRows(REVENUS, [], []);
+    expect(names(rows)).toEqual([
+      "Revenus",
+      "Autres revenus",
+      "Emploi",
+      "Paie régulière",
+      "Primes & bonus",
+      "Travail autonome",
+      "Gouvernemental",
+      "Allocations familiales",
+      "Assurance-emploi / RQAP",
+      "Crédits TPS/TVQ",
+      "Remboursement impôt",
+      "Revenus de placement",
+      "Gains en capital",
+      "Intérêts & dividendes",
+      "Revenus locatifs",
+    ]);
+    expectDepthFirst(rows);
+  });
 
-    // Explicit DFS-killer: the depth-2 grandchildren (Hydro/Internet) come AFTER
-    // the depth-1 leaf sibling (Rent). A depth-first emission would interleave
-    // them directly under Utilities, i.e. BEFORE Rent.
-    const leafIdx = (id: number) => rows.findIndex((r) => r.category_id === id && !r.is_parent);
-    expect(leafIdx(4)).toBeGreaterThan(leafIdx(2)); // Hydro after Rent
-    expect(leafIdx(5)).toBeGreaterThan(leafIdx(2)); // Internet after Rent
+  it("Finances & obligations: leaves before and after sub-groups, sorted by name together", () => {
+    const rows = buildBudgetYearRows(FINANCES, [], []);
+    expect(names(rows)).toEqual([
+      "Finances & obligations",
+      "Achats divers non catégorisés",
+      "Cadeaux",
+      "Dons & cotisations",
+      "Cotisations professionnelles",
+      "Cotisations syndicales",
+      "Dons de charité",
+      "Frais bancaires",
+      "Frais de change",
+      "Frais de compte",
+      "Intérêts & frais de crédit",
+      "Impôts & taxes",
+      "Acomptes provisionnels",
+      "Impôt fédéral",
+      "Impôt provincial",
+      "Retrait cash",
+    ]);
+    expectDepthFirst(rows);
+  });
+
+  it("older taxonomy: the (direct) row leads, Assurances is followed by its three leaves", () => {
+    const rows = buildBudgetYearRows(RECURRENTES, [], []);
+    expect(names(rows)).toEqual([
+      "Dépenses récurrentes",
+      "Dépenses récurrentes (direct)",
+      "Assurances",
+      "Assurance-auto",
+      "Assurance-habitation",
+      "Assurance-vie",
+      "Électricité",
+      "Loyer",
+      "Pharmacie",
+      "Taxes municipales",
+    ]);
+    expectDepthFirst(rows);
+  });
+
+  it("top groups are sorted by type, then sort_order, then name — whatever the input order", () => {
+    const all = [
+      ...FINANCES,
+      ...RECURRENTES,
+      ...REVENUS,
+      cat(9000, "Divers", "expense", { sort_order: 2 }),
+      cat(9100, "Virements", "transfer", { sort_order: 0 }),
+    ];
+    const rows = buildBudgetYearRows([...all].reverse(), [], []);
+    const roots = rows.filter((r) => (r.depth ?? 0) === 0).map((r) => r.category_name);
+    expect(roots).toEqual([
+      "Revenus", // income
+      "Dépenses récurrentes", // expense, sort_order 2, name before "Divers"
+      "Divers", // expense, sort_order 2 (lone leaf)
+      "Finances & obligations", // expense, sort_order 10
+      "Virements", // transfer, last despite sort_order 0
+    ]);
+    expectDepthFirst(rows);
+  });
+
+  it("subtotals at the bottom: each subtotal follows its own rows (reorderRows on the builder output)", () => {
+    const rows = reorderRows(buildBudgetYearRows([...FINANCES, ...RECURRENTES], [], []), false);
+    expect(names(rows)).toEqual([
+      "Dépenses récurrentes (direct)",
+      "Assurance-auto",
+      "Assurance-habitation",
+      "Assurance-vie",
+      "Assurances",
+      "Électricité",
+      "Loyer",
+      "Pharmacie",
+      "Taxes municipales",
+      "Dépenses récurrentes",
+      "Achats divers non catégorisés",
+      "Cadeaux",
+      "Cotisations professionnelles",
+      "Cotisations syndicales",
+      "Dons de charité",
+      "Dons & cotisations",
+      "Frais de change",
+      "Frais de compte",
+      "Intérêts & frais de crédit",
+      "Frais bancaires",
+      "Acomptes provisionnels",
+      "Impôt fédéral",
+      "Impôt provincial",
+      "Impôts & taxes",
+      "Retrait cash",
+      "Finances & obligations",
+    ]);
+  });
+
+  it("expanding a single intermediate parent shows its leaves right under it", () => {
+    const rows = buildBudgetYearRows(REVENUS, [], []);
+    const flipped = new Set<string>(["p:1000", "p:1010"]); // Revenus + Emploi expanded
+    const isCollapsed = (r: BudgetYearRow) => isCollapsedFor(flipped, ACCESSORS.keyOf(r), false);
+    const visible = visibleRows(rows, ACCESSORS, isCollapsed);
+    expect(names(visible)).toEqual([
+      "Revenus",
+      "Autres revenus",
+      "Emploi",
+      "Paie régulière",
+      "Primes & bonus",
+      "Travail autonome",
+      "Gouvernemental",
+      "Revenus de placement",
+    ]);
+  });
+
+  it("subtotals are unchanged by the order: each subtotal sums its own leaves", () => {
+    const ENTRIES = [entry(1011, 100), entry(1012, 20), entry(1021, 7), entry(1033, 3), entry(1090, 1000)];
+    const rows = buildBudgetYearRows(REVENUS, ENTRIES, []);
+    const annualOf = (id: number) => rows.find((r) => r.category_id === id && r.is_parent)!.annual;
+    expect(annualOf(1000)).toBe(1130);
+    expect(annualOf(1010)).toBe(120);
+    expect(annualOf(1020)).toBe(7);
+    expect(annualOf(1030)).toBe(3);
   });
 });
 
@@ -102,9 +336,9 @@ describe("buildBudgetYearRows + multi-level collapse (issue #289 end-to-end)", (
     const flipped = new Set<string>(["p:1"]); // Housing expanded
     const isCollapsed = (r: BudgetYearRow) => isCollapsedFor(flipped, ACCESSORS.keyOf(r), false);
     const visible = visibleRows(rows, ACCESSORS, isCollapsed);
-    // Housing's direct children: Utilities subtotal + Rent leaf. Hydro/Internet
-    // remain hidden under the collapsed Utilities.
-    expect(visible.map((r) => r.category_id)).toEqual([10, 1, 3, 2]);
+    // Housing's direct children, by name: Rent leaf + Utilities subtotal.
+    // Hydro/Internet remain hidden under the collapsed Utilities.
+    expect(visible.map((r) => r.category_id)).toEqual([10, 1, 2, 3]);
   });
 });
 

src/hooks/useBudget.ts

@@ -88,16 +88,22 @@ const TYPE_ORDER: Record<string, number> = { income: 0, expense: 1, transfer: 2
  * Assembles the flat, hierarchical `BudgetYearRow[]` the grid renders from the
  * raw category tree, this year's budget entries, and last year's actual totals.
  *
- * ORDERING INVARIANT (issue #289 — the multi-level-collapse consumer). The
- * final `rows.sort` below orders each top category group by DEPTH ASCENDING
- * (level-order / BFS), NOT depth-first: within a group every depth-0 row
- * precedes every depth-1 row, which precedes every depth-2 row. The abandoned
- * v1 collapse algorithm wrongly assumed a depth-first emission order and broke
- * exactly here; the shipped algorithm (ancestor walk — see `collapsibleRows.ts`)
- * is order-INDEPENDENT, so this sort needs NO change. The level-order property
- * is pinned by `useBudget.test.ts` so a refactor can't silently reintroduce the
- * v1 assumption — do not reorder into DFS without revisiting the collapse
- * consumer (`BudgetTable`).
+ * ORDERING INVARIANT (issue #424, replacing the level-order of #289). Rows are
+ * emitted DEPTH-FIRST: every parent row is followed directly by its whole
+ * subtree, so an expanded subcategory shows right under its parent and
+ * `reorderRows` (subtotals at the bottom) can attach each subtotal to its own
+ * rows. The mechanism is fixed by blocks, sorted BEFORE they are emitted:
+ * - inside a top group, one block per child: a lone leaf, or a sub-group
+ *   `[subtotal, ...its children]` (what `buildLevel2Group` returns);
+ * - the blocks of a group are sorted once: the "(direct)" block first, then by
+ *   `localeCompare` of the leaf or subtotal name, leaves and sub-groups mixed
+ *   (the sibling order of the Budget vs Actual report, `budgetService.ts`);
+ * - top groups (`[parent, ...]` block or lone leaf) are sorted by type
+ *   (`TYPE_ORDER`), then `sort_order`, then name, and concatenated.
+ * No comparator ever runs on rows inside a group: a depth sort there would
+ * bring back the level order that pushed subcategories to the end of their
+ * group. The collapse consumer (`collapsibleRows.ts`, ancestor walk) does not
+ * depend on the order; the depth-first property is pinned by `useBudget.test.ts`.
  *
  * Pure (no DB, no React) so the invariant is unit-testable without a renderer —
  * the repo has no jsdom, so hook logic that must be tested is extracted here,
@@ -136,8 +142,7 @@ export function buildBudgetYearRows(
     return { months, annual, previousYearTotal };
   };
 
-  // Index categories by id and group children by parent_id
-  const catById = new Map(allCategories.map((c) => [c.id, c]));
+  // Group children by parent_id
   const childrenByParent = new Map<number, typeof allCategories>();
   for (const cat of allCategories) {
     if (cat.parent_id) {
@@ -146,7 +151,8 @@ export function buildBudgetYearRows(
     }
   }
 
-  const rows: BudgetYearRow[] = [];
+  // One entry per top group, sorted then flattened at the end (issue #424).
+  const topGroups: Array<{ cat: Category; rows: BudgetYearRow[] }> = [];
 
   // Build rows for an intermediate parent (level 1 or 2 with children)
   function buildLevel2Group(cat: typeof allCategories[0], grandparentId: number): BudgetYearRow[] {
@@ -246,7 +252,7 @@ export function buildBudgetYearRows(
     if (inputableChildren.length === 0 && intermediateParents.length === 0 && cat.is_inputable) {
       // Standalone leaf (no children) — regular editable row
       const { months, annual, previousYearTotal } = buildMonths(cat.id);
-      rows.push({
+      topGroups.push({ cat, rows: [{
         category_id: cat.id,
         category_name: cat.name,
         category_color: cat.color || "#9ca3af",
@@ -257,14 +263,21 @@ export function buildBudgetYearRows(
         months,
         annual,
         previousYearTotal,
-      });
+      }] });
     } else if (inputableChildren.length > 0 || intermediateParents.length > 0) {
-      const allChildRows: BudgetYearRow[] = [];
+      // One block per child: a lone leaf, or a sub-group [subtotal, ...children].
+      // `name` is the leaf or subtotal name the blocks are sorted by.
+      const blocks: Array<{ isDirect: boolean; name: string; rows: BudgetYearRow[] }> = [];
+      const pushBlock = (blockRows: BudgetYearRow[], isDirect = false) => {
+        if (blockRows.length > 0) {
+          blocks.push({ isDirect, name: blockRows[0].category_name, rows: blockRows });
+        }
+      };
 
       // If parent is also inputable, create a "(direct)" fake-child row
       if (cat.is_inputable) {
         const { months, annual, previousYearTotal } = buildMonths(cat.id);
-        allChildRows.push({
+        pushBlock([{
           category_id: cat.id,
           category_name: `${cat.name} (direct)`,
           category_color: cat.color || "#9ca3af",
@@ -275,7 +288,7 @@ export function buildBudgetYearRows(
           months,
           annual,
           previousYearTotal,
-        });
+        }], true);
       }
 
       for (const child of inputableChildren) {
@@ -283,7 +296,7 @@ export function buildBudgetYearRows(
         if (grandchildren.length === 0) {
           // Simple leaf at depth 1
           const { months, annual, previousYearTotal } = buildMonths(child.id);
-          allChildRows.push({
+          pushBlock([{
             category_id: child.id,
             category_name: child.name,
             category_color: child.color || cat.color || "#9ca3af",
@@ -294,19 +307,27 @@ export function buildBudgetYearRows(
             months,
             annual,
             previousYearTotal,
-          });
+          }]);
         } else {
           // Intermediate parent at depth 1 with grandchildren
-          allChildRows.push(...buildLevel2Group(child, cat.id));
+          pushBlock(buildLevel2Group(child, cat.id));
         }
       }
 
       // Non-inputable intermediate parents
       for (const ip of intermediateParents) {
-        allChildRows.push(...buildLevel2Group(ip, cat.id));
+        pushBlock(buildLevel2Group(ip, cat.id));
       }
 
-      if (allChildRows.length === 0) continue;
+      if (blocks.length === 0) continue;
+
+      // Sort the blocks once, before emission: "(direct)" first, then by name,
+      // leaves and sub-groups mixed. Rows inside a block keep their order.
+      blocks.sort((a, b) => {
+        if (a.isDirect !== b.isDirect) return a.isDirect ? -1 : 1;
+        return a.name.localeCompare(b.name);
+      });
+      const allChildRows = blocks.flatMap((b) => b.rows);
 
       // Parent subtotal row: sum of leaf rows only (avoid double-counting)
       const leafRows = allChildRows.filter((r) => !r.is_parent);
@@ -319,67 +340,41 @@ export function buildBudgetYearRows(
         parentPrevYear += cr.previousYearTotal;
       }
 
-      rows.push({
-        category_id: cat.id,
-        category_name: cat.name,
-        category_color: cat.color || "#9ca3af",
-        category_type: cat.type,
-        parent_id: null,
-        is_parent: true,
-        depth: 0,
-        months: parentMonths,
-        annual: parentAnnual,
-        previousYearTotal: parentPrevYear,
+      topGroups.push({
+        cat,
+        rows: [
+          {
+            category_id: cat.id,
+            category_name: cat.name,
+            category_color: cat.color || "#9ca3af",
+            category_type: cat.type,
+            parent_id: null,
+            is_parent: true,
+            depth: 0,
+            months: parentMonths,
+            annual: parentAnnual,
+            previousYearTotal: parentPrevYear,
+          },
+          ...allChildRows,
+        ],
       });
-
-      // Sort children alphabetically, but keep "(direct)" first
-      allChildRows.sort((a, b) => {
-        if (a.category_id === cat.id && !a.is_parent) return -1;
-        if (b.category_id === cat.id && !b.is_parent) return 1;
-        return a.category_name.localeCompare(b.category_name);
-      });
-
-      rows.push(...allChildRows);
     }
     // else: non-inputable parent with no inputable children — skip
   }
 
-  // Sort by type, then within each type: keep hierarchy groups together
-  function getTopGroupId(r: BudgetYearRow): number {
-    if ((r.depth ?? 0) === 0) return r.category_id;
-    if (r.is_parent && r.parent_id === null) return r.category_id;
-    let pid = r.parent_id;
-    while (pid !== null) {
-      const pCat = catById.get(pid);
-      if (!pCat || !pCat.parent_id) return pid;
-      pid = pCat.parent_id;
-    }
-    return r.category_id;
-  }
-
-  rows.sort((a, b) => {
-    const typeA = TYPE_ORDER[a.category_type] ?? 9;
-    const typeB = TYPE_ORDER[b.category_type] ?? 9;
+  // Sort the top groups by type, then sort_order, then name, and concatenate
+  // them. Each group keeps its depth-first row order (issue #424).
+  topGroups.sort((a, b) => {
+    const typeA = TYPE_ORDER[a.cat.type] ?? 9;
+    const typeB = TYPE_ORDER[b.cat.type] ?? 9;
     if (typeA !== typeB) return typeA - typeB;
-    const groupA = getTopGroupId(a);
-    const groupB = getTopGroupId(b);
-    if (groupA !== groupB) {
-      const catA = catById.get(groupA);
-      const catB = catById.get(groupB);
-      const orderA = catA?.sort_order ?? 999;
-      const orderB = catB?.sort_order ?? 999;
-      if (orderA !== orderB) return orderA - orderB;
-      return (catA?.name ?? "").localeCompare(catB?.name ?? "");
-    }
-    // Same group: sort by depth, then parent before children at same depth
-    if (a.is_parent !== b.is_parent && (a.depth ?? 0) === (b.depth ?? 0)) return a.is_parent ? -1 : 1;
-    if ((a.depth ?? 0) !== (b.depth ?? 0)) return (a.depth ?? 0) - (b.depth ?? 0);
-    if (a.parent_id && a.category_id === a.parent_id) return -1;
-    if (b.parent_id && b.category_id === b.parent_id) return 1;
-    return a.category_name.localeCompare(b.category_name);
+    const orderA = a.cat.sort_order ?? 999;
+    const orderB = b.cat.sort_order ?? 999;
+    if (orderA !== orderB) return orderA - orderB;
+    return a.cat.name.localeCompare(b.cat.name);
   });
 
-  return rows;
+  return topGroups.flatMap((g) => g.rows);
 }
 
 /**

src/utils/collapsibleRows.test.ts

@@ -54,7 +54,9 @@ const dfsTree: Row[] = [
 ];
 
 // The EXACT same tree, emitted level-order (BFS) — the order the budget grid
-// produces. The ancestor-walk must mask identically to the DFS order.
+// produced before issue #424 (it is depth-first since). Kept as the proof that
+// the ancestor walk does not depend on the order: it must mask identically to
+// the DFS order.
 const bfsTree: Row[] = [
   r(1, null, true, 0), // Housing
   r(2, null, true, 0), // Salary

src/utils/collapsibleRows.ts

@@ -11,7 +11,8 @@
  * row until we reach a root (null), an ancestor outside this section (⇒ visible),
  * or a collapsed ancestor (⇒ hidden). This is independent of the order rows
  * arrive in — the fix for the v1 adjacency algorithm, which assumed a
- * depth-first order the budget grid (level-ordered) does not emit.
+ * depth-first order the budget grid did not emit at the time. The grid has been
+ * depth-first since issue #424, but the walk keeps not relying on it.
  *
  * The accessors are supplied by each consumer so every table can key + climb the
  * exact hierarchy it renders. Keys are prefixed `p:` so they are injective: a

src/utils/reorderRows.test.ts

@@ -0,0 +1,91 @@
+import { describe, it, expect } from "vitest";
+import { reorderRows } from "./reorderRows";
+
+// Minimal row: the two fields `reorderRows` reads, plus a name to assert on.
+interface Row {
+  name: string;
+  is_parent?: boolean;
+  depth?: number;
+}
+
+const leaf = (name: string, depth: number): Row => ({ name, is_parent: false, depth });
+const sub = (name: string, depth: number): Row => ({ name, is_parent: true, depth });
+const names = (rows: Row[]) => rows.map((r) => r.name);
+
+// Two levels, depth-first: a root group whose leaves sit before AND after a
+// sibling sub-group, then a lone root leaf.
+//   Finances
+//     Cadeaux
+//     Impôts (sub-group)
+//       Fédéral
+//       Provincial
+//     Retrait cash        <- follows the sibling sub-group "Impôts"
+//   Divers (root leaf)    <- follows the root group "Finances"
+const TWO_LEVELS: Row[] = [
+  sub("Finances", 0),
+  leaf("Cadeaux", 1),
+  sub("Impôts", 1),
+  leaf("Fédéral", 2),
+  leaf("Provincial", 2),
+  leaf("Retrait cash", 1),
+  leaf("Divers", 0),
+];
+
+// Three levels, as the Compare report tree allows (depth is not capped at 2).
+//   A
+//     A1 (sub-group)
+//       A1a (sub-group)
+//         x
+//       y               <- follows the sibling sub-group "A1a"
+//     z                 <- follows the sibling sub-group "A1"
+const THREE_LEVELS: Row[] = [
+  sub("A", 0),
+  sub("A1", 1),
+  sub("A1a", 2),
+  leaf("x", 3),
+  leaf("y", 2),
+  leaf("z", 1),
+];
+
+describe("reorderRows — subtotals at the bottom (issue #424)", () => {
+  it("two levels: a leaf that follows a sibling sub-group stays outside its block", () => {
+    expect(names(reorderRows(TWO_LEVELS, false))).toEqual([
+      "Cadeaux",
+      "Fédéral",
+      "Provincial",
+      "Impôts",
+      "Retrait cash",
+      "Finances",
+      "Divers",
+    ]);
+  });
+
+  it("three levels: each subtotal follows its own rows only", () => {
+    expect(names(reorderRows(THREE_LEVELS, false))).toEqual(["x", "A1a", "y", "A1", "z", "A"]);
+  });
+
+  it("consecutive sibling sub-groups each close on their own leaves", () => {
+    const rows: Row[] = [
+      sub("Revenus", 0),
+      leaf("Autres", 1),
+      sub("Emploi", 1),
+      leaf("Paie", 2),
+      sub("Gouv", 1),
+      leaf("Alloc", 2),
+    ];
+    expect(names(reorderRows(rows, false))).toEqual(["Autres", "Paie", "Emploi", "Alloc", "Gouv", "Revenus"]);
+  });
+
+  it("keeps every row exactly once", () => {
+    const out = reorderRows(TWO_LEVELS, false);
+    expect(out).toHaveLength(TWO_LEVELS.length);
+    expect(new Set(out)).toEqual(new Set(TWO_LEVELS));
+  });
+});
+
+describe("reorderRows — subtotals on top", () => {
+  it("returns the list unchanged", () => {
+    expect(reorderRows(TWO_LEVELS, true)).toBe(TWO_LEVELS);
+    expect(reorderRows(THREE_LEVELS, true)).toBe(THREE_LEVELS);
+  });
+});

src/utils/reorderRows.ts

@@ -3,6 +3,11 @@
  * Recursively moves subtotal (parent) rows below their children
  * at every depth level when "subtotals on bottom" is enabled.
  *
+ * Expects a depth-first list (each parent followed by its whole subtree), the
+ * order `buildBudgetYearRows` and the report tree builders emit. A subtotal's
+ * block ends at the next row of its own depth or shallower, so a leaf that
+ * follows a sibling sub-group stays outside that sub-group (issue #424).
+ *
  * 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`).
@@ -25,6 +30,15 @@ export function reorderRows<
           currentChildren = [];
         }
         currentParent = row;
+      } else if (currentParent && (row.depth ?? 0) <= parentDepth) {
+        // A row at this level (or above) that is not a parent of this level
+        // closes the current block: it is a sibling of the subtotal, not one of
+        // its children (issue #424 — a leaf after a sibling sub-group used to
+        // fall into that sub-group's block).
+        result.push(...reorderGroup(currentChildren, parentDepth + 1), currentParent);
+        currentChildren = [];
+        currentParent = null;
+        result.push(row);
       } else if (currentParent) {
         currentChildren.push(row);
       } else {