CHANGELOG.fr.md

@@ -12,6 +12,7 @@
 - La page d'erreur du démarrage ne reste plus affichée quand le profil finit de s'ouvrir après le délai de 10 secondes : l'application s'affiche dès que le profil est ouvert (#395).
 - Le petit badge à côté du bouton « Récupérer le prix » d'un titre boursier, dans le Bilan, affiche désormais « non garanti » au lieu de l'anglais « best-effort » (#389).
 - Les installations faites depuis le paquet `.rpm` reçoivent désormais les mises à jour automatiques. Le manifeste de mise à jour ne listait que le `.deb`, qu'une installation `.rpm` téléchargeait puis refusait ; il liste maintenant chaque paquet Linux sous sa propre clé. Les installations `.rpm` existantes n'ont rien à réinstaller : elles recevront la prochaine version (vérifié sur le paquet v0.17.1 publié). Les instructions de téléchargement de la page de release ne mentionnent plus d'AppImage, qui n'est pas construite, et nomment désormais le `.rpm`. Le manifeste n'a plus d'entrée Linux générique : les AppImage des versions 0.4.3 à 0.4.5, qui ne savaient pas installer le `.deb` qu'elle désignait et en étaient cassées, affichent désormais une erreur à la vérification des mises à jour ; installez plutôt le paquet `.deb` ou `.rpm` (#320).
+- Budget : une sous-catégorie dépliée s'affiche sous sa catégorie parente, et non plus en fin de groupe. Avec les sous-totaux en bas, chaque sous-total suit ses propres lignes, dans la grille Budget comme dans les rapports Budget vs réel et Comparables (#424).
 
 ### Sécurité
 

CHANGELOG.md

@@ -12,6 +12,7 @@
 - The startup error page no longer stays on screen when the profile finishes opening after the 10-second delay: the app now shows as soon as the profile is open (#395).
 - In French, the small badge next to the "Fetch price" button of a stock in the Balance sheet now reads "non garanti" instead of the English "best-effort" (#389).
 - Installs from the `.rpm` package now receive automatic updates. The update manifest only listed the `.deb`, which an `.rpm` install downloaded and then refused; it now lists each Linux package under its own key. Existing `.rpm` installs need no reinstall: they will receive the next version (checked on the published v0.17.1 package). The download instructions of the release page no longer mention an AppImage, which is not built, and now name the `.rpm`. The manifest no longer has a generic Linux entry: the AppImage builds of 0.4.3 to 0.4.5, which could not install the `.deb` it pointed to and were broken by it, now report an error when checking for updates instead; install the `.deb` or `.rpm` package (#320).
+- Budget: an expanded subcategory now shows under its parent category instead of at the end of the group. With subtotals at the bottom, each subtotal follows its own rows, in the Budget grid as in the Budget vs Actual and Compare reports (#424).
 
 ### Security
 

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
+// abandoned v1 collapse plan ignored). Since issue #424 they are emitted
-// ascending / BFS), not depth-first. These tests pin that invariant: the
+// DEPTH-FIRST: each parent row is followed directly by its whole subtree, so an
-// shipped ancestor-walk collapse (issue #288) is order-INDEPENDENT, but the v1
+// expanded subcategory shows under its parent and `reorderRows` (subtotals at
-// depth-cursor algorithm assumed DFS and would have broken exactly here.
+// 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", () => {
+const names = (rows: BudgetYearRow[]) => rows.map((r) => r.category_name);
-    // Salary (income) first; then the Housing expense group depth-ascending:
+
-    // root subtotal, then the depth-1 rows (parent Utilities before leaf Rent),
+// Shape of the "Revenus" group of a new profile (consolidated_schema.sql): one
-    // then the depth-2 grandchildren.
+// depth-1 leaf ("Autres revenus") and three intermediate parents.
-    expect(rows.map((r) => r.category_id)).toEqual([10, 1, 3, 2, 4, 5]);
+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", () => {
+  it("Revenus: the depth-1 leaf sorts first by name, each sub-group carries its own leaves", () => {
-    const expenseGroup = rows.filter((r) => r.category_type === "expense");
+    const rows = buildBudgetYearRows(REVENUS, [], []);
-    const depths = expenseGroup.map((r) => r.depth ?? 0);
+    expect(names(rows)).toEqual([
-    for (let i = 1; i < depths.length; i++) {
+      "Revenus",
-      expect(depths[i]).toBeGreaterThanOrEqual(depths[i - 1]);
+      "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
+  it("Finances & obligations: leaves before and after sub-groups, sorted by name together", () => {
-    // the depth-1 leaf sibling (Rent). A depth-first emission would interleave
+    const rows = buildBudgetYearRows(FINANCES, [], []);
-    // them directly under Utilities, i.e. BEFORE Rent.
+    expect(names(rows)).toEqual([
-    const leafIdx = (id: number) => rows.findIndex((r) => r.category_id === id && !r.is_parent);
+      "Finances & obligations",
-    expect(leafIdx(4)).toBeGreaterThan(leafIdx(2)); // Hydro after Rent
+      "Achats divers non catégorisés",
-    expect(leafIdx(5)).toBeGreaterThan(leafIdx(2)); // Internet after Rent
+      "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
+    // Housing's direct children, by name: Rent leaf + Utilities subtotal.
-    // remain hidden under the collapsed Utilities.
+    // Hydro/Internet remain hidden under the collapsed Utilities.
-    expect(visible.map((r) => r.category_id)).toEqual([10, 1, 3, 2]);
+    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
+ * ORDERING INVARIANT (issue #424, replacing the level-order of #289). Rows are
- * final `rows.sort` below orders each top category group by DEPTH ASCENDING
+ * emitted DEPTH-FIRST: every parent row is followed directly by its whole
- * (level-order / BFS), NOT depth-first: within a group every depth-0 row
+ * subtree, so an expanded subcategory shows right under its parent and
- * precedes every depth-1 row, which precedes every depth-2 row. The abandoned
+ * `reorderRows` (subtotals at the bottom) can attach each subtotal to its own
- * v1 collapse algorithm wrongly assumed a depth-first emission order and broke
+ * rows. The mechanism is fixed by blocks, sorted BEFORE they are emitted:
- * exactly here; the shipped algorithm (ancestor walk — see `collapsibleRows.ts`)
+ * - inside a top group, one block per child: a lone leaf, or a sub-group
- * is order-INDEPENDENT, so this sort needs NO change. The level-order property
+ *   `[subtotal, ...its children]` (what `buildLevel2Group` returns);
- * is pinned by `useBudget.test.ts` so a refactor can't silently reintroduce the
+ * - the blocks of a group are sorted once: the "(direct)" block first, then by
- * v1 assumption — do not reorder into DFS without revisiting the collapse
+ *   `localeCompare` of the leaf or subtotal name, leaves and sub-groups mixed
- * consumer (`BudgetTable`).
+ *   (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
+  // Group children by parent_id
-  const catById = new Map(allCategories.map((c) => [c.id, c]));
   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({
+      topGroups.push({
-        category_id: cat.id,
+        cat,
-        category_name: cat.name,
+        rows: [
-        category_color: cat.color || "#9ca3af",
+          {
-        category_type: cat.type,
+            category_id: cat.id,
-        parent_id: null,
+            category_name: cat.name,
-        is_parent: true,
+            category_color: cat.color || "#9ca3af",
-        depth: 0,
+            category_type: cat.type,
-        months: parentMonths,
+            parent_id: null,
-        annual: parentAnnual,
+            is_parent: true,
-        previousYearTotal: parentPrevYear,
+            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
+  // Sort the top groups by type, then sort_order, then name, and concatenate
-  function getTopGroupId(r: BudgetYearRow): number {
+  // them. Each group keeps its depth-first row order (issue #424).
-    if ((r.depth ?? 0) === 0) return r.category_id;
+  topGroups.sort((a, b) => {
-    if (r.is_parent && r.parent_id === null) return r.category_id;
+    const typeA = TYPE_ORDER[a.cat.type] ?? 9;
-    let pid = r.parent_id;
+    const typeB = TYPE_ORDER[b.cat.type] ?? 9;
-    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;
     if (typeA !== typeB) return typeA - typeB;
-    const groupA = getTopGroupId(a);
+    const orderA = a.cat.sort_order ?? 999;
-    const groupB = getTopGroupId(b);
+    const orderB = b.cat.sort_order ?? 999;
-    if (groupA !== groupB) {
+    if (orderA !== orderB) return orderA - orderB;
-      const catA = catById.get(groupA);
+    return a.cat.name.localeCompare(b.cat.name);
-      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);
   });
 
-  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 {