fix(budget): grid rows depth-first, subtotals at the bottom on their own rows (#424) #438

Merged
maximus merged 2 commits from issue-424-budget-row-order into main 2026-10-16 00:36:49 +00:00
Owner

Generated autonomously by /autopilot run of 2026-10-13

Milestone planned-2026-10-12-retours-onboarding-v0171, link 1 of 8 (T1). Base: main.

What changes

  • buildBudgetYearRows (src/hooks/useBudget.ts) emits depth-first. Inside a top group it builds one block per child: a lone leaf, or a sub-group [subtotal, ...children] (what buildLevel2Group already returns). The blocks are sorted once, before emission: the "(direct)" block first, then localeCompare of the leaf or subtotal name, leaves and sub-groups mixed (the sibling order of the Budget vs Actual report). Then they are concatenated. Top groups are sorted by TYPE_ORDER, sort_order, then name. The old row comparator (depth sort, getTopGroupId) is gone.
  • reorderRows (src/utils/reorderRows.ts): when a row has depth <= parentDepth and is not a parent of that level, it now closes the current block before it is emitted. Before, a leaf that followed a sibling sub-group fell into that sub-group's block. "Retrait cash" showed above the "Impôts & taxes" subtotal, and a root leaf after a root group was absorbed the same way. The same fix applies to "subtotals at the bottom" in Budget vs Actual and Compare, which use this utility.
  • Rewrote the comments: the ordering invariant in useBudget.ts, the "level-ordered" wording in collapsibleRows.ts, and the bfsTree comment in collapsibleRows.test.ts. The bfsTree case stays, as the proof that collapse does not depend on row order.
  • CHANGELOG Fixed / Corrigé, in both files, using the issue's wording.

Visible change, as Max decided at planning: in Revenus, "Autres revenus" now comes first.

Files

  • src/hooks/useBudget.ts, src/hooks/useBudget.test.ts
  • src/utils/reorderRows.ts, src/utils/reorderRows.test.ts (new)
  • src/utils/collapsibleRows.ts, src/utils/collapsibleRows.test.ts (comments only)
  • CHANGELOG.md, CHANGELOG.fr.md

BudgetTable.render.test.tsx and budgetOnboardingTarget.test.ts are unchanged and still green: their rows are built by hand, two levels deep.

Tests

  • The two level-order tests are replaced by expectDepthFirst. It checks that every row comes after its parent row, and that the rows between a parent and the end of its subtree are exactly its descendants.
  • Exact order on four fixtures, which copy real names and ids:
    • the Revenus shape (consolidated_schema.sql): a depth-1 leaf comes first, then three sub-groups;
    • the Finances & obligations shape: leaves before and after sub-groups, with "Retrait cash" after "Impôts & taxes";
    • the older taxonomy, Dépenses récurrentes → Assurances (v2 seed + migration v7): an inputable root with a "(direct)" row;
    • the existing Housing fixture.
  • Top-group order: type, then sort_order, then name, with the input given in reverse.
  • reorderRows(buildBudgetYearRows(...), false) on two groups: each subtotal follows its own rows.
  • Expanding only Revenus + Emploi shows Emploi's three leaves right under it.
  • Subtotals are unchanged: each one sums its own leaves.
  • reorderRows.test.ts (new): subtotals at the bottom at two and three levels, a leaf after a sibling sub-group stays out of that sub-group's block, consecutive sibling sub-groups, every row kept exactly once; subtotals on top: the same array is returned.
  • Recalibrated: "expanding a root reveals its direct children" ([10, 1, 3, 2] → [10, 1, 2, 3], Rent now sorts before Utilities by name).

Mutation proofs

Each mutation was applied alone, then vitest run on useBudget.test.ts, reorderRows.test.ts and collapsibleRows.test.ts, then the file was restored.

# Mutation rc Red tests
M1 depth sort put back on the group's rows in the builder 1 8 (exact orders, depth-first invariant, collapse, bottom subtotals)
M2 block close removed in reorderRows 1 3 (two levels, three levels, builder + reorder)
M3 "(direct)" no longer forced first 1 2 (older taxonomy, builder + reorder)
M4 blocks no longer sorted by name 1 3 (Finances, older taxonomy, builder + reorder)
M5 top groups ignore the type 1 1 (top-group order)
M6 top groups lose the name tie-break 1 1 (top-group order)

Validation in the worktree: npm ci, npm run build green, npm test 120 files / 2076 tests green. No Rust touched, no migration (v1→v17 unchanged), no i18n string, no network call.

Decisions (autopilot)

  • [HIGH] The mechanism follows the issue exactly: blocks, one sort, concatenation, and no comparator on rows inside a group.
  • [HIGH] buildLevel2Group keeps its sort of the sub-group's leaves ("(direct)" first, then name). It is the same block rule one level down, since every block there is a lone leaf.
  • [MEDIUM] The top-group sort reads the type of the top category; the old comparator read each row's own type. The result is the same for both seeded taxonomies, where children carry their root's type, and BudgetTable groups rows by category_type anyway. Rejected alternative: a per-row type sort, which is the forbidden comparator on rows inside a group.
  • [HIGH] Resolves #424: the issue has no post-merge action.

For the reviewer

  • src/hooks/useBudget.ts: the top-level loop now fills topGroups, with pushBlock dropping empty blocks (buildLevel2Group can return []). catById was only used by the removed comparator.
  • src/utils/reorderRows.ts: the new branch sits between "parent of this level" and "child of the current parent". In a recursive call, rows are always deeper than the caller's level, so the < part only matters at the top.

🤖 Generated with Claude Code

Generated autonomously by /autopilot run of 2026-10-13 Milestone `planned-2026-10-12-retours-onboarding-v0171`, link 1 of 8 (T1). Base: `main`. ## What changes - **`buildBudgetYearRows`** (`src/hooks/useBudget.ts`) emits depth-first. Inside a top group it builds one block per child: a lone leaf, or a sub-group `[subtotal, ...children]` (what `buildLevel2Group` already returns). The blocks are sorted once, before emission: the "(direct)" block first, then `localeCompare` of the leaf or subtotal name, leaves and sub-groups mixed (the sibling order of the Budget vs Actual report). Then they are concatenated. Top groups are sorted by `TYPE_ORDER`, `sort_order`, then name. The old row comparator (depth sort, `getTopGroupId`) is gone. - **`reorderRows`** (`src/utils/reorderRows.ts`): when a row has depth <= `parentDepth` and is not a parent of that level, it now closes the current block before it is emitted. Before, a leaf that followed a sibling sub-group fell into that sub-group's block. "Retrait cash" showed above the "Impôts & taxes" subtotal, and a root leaf after a root group was absorbed the same way. The same fix applies to "subtotals at the bottom" in Budget vs Actual and Compare, which use this utility. - Rewrote the comments: the ordering invariant in `useBudget.ts`, the "level-ordered" wording in `collapsibleRows.ts`, and the `bfsTree` comment in `collapsibleRows.test.ts`. The `bfsTree` case stays, as the proof that collapse does not depend on row order. - CHANGELOG Fixed / Corrigé, in both files, using the issue's wording. Visible change, as Max decided at planning: in Revenus, "Autres revenus" now comes first. ## Files - `src/hooks/useBudget.ts`, `src/hooks/useBudget.test.ts` - `src/utils/reorderRows.ts`, `src/utils/reorderRows.test.ts` (new) - `src/utils/collapsibleRows.ts`, `src/utils/collapsibleRows.test.ts` (comments only) - `CHANGELOG.md`, `CHANGELOG.fr.md` `BudgetTable.render.test.tsx` and `budgetOnboardingTarget.test.ts` are unchanged and still green: their rows are built by hand, two levels deep. ## Tests - The two level-order tests are replaced by `expectDepthFirst`. It checks that every row comes after its parent row, and that the rows between a parent and the end of its subtree are exactly its descendants. - Exact order on four fixtures, which copy real names and ids: - the Revenus shape (`consolidated_schema.sql`): a depth-1 leaf comes first, then three sub-groups; - the Finances & obligations shape: leaves before and after sub-groups, with "Retrait cash" after "Impôts & taxes"; - the older taxonomy, Dépenses récurrentes → Assurances (v2 seed + migration v7): an inputable root with a "(direct)" row; - the existing Housing fixture. - Top-group order: type, then `sort_order`, then name, with the input given in reverse. - `reorderRows(buildBudgetYearRows(...), false)` on two groups: each subtotal follows its own rows. - Expanding only Revenus + Emploi shows Emploi's three leaves right under it. - Subtotals are unchanged: each one sums its own leaves. - `reorderRows.test.ts` (new): subtotals at the bottom at two and three levels, a leaf after a sibling sub-group stays out of that sub-group's block, consecutive sibling sub-groups, every row kept exactly once; subtotals on top: the same array is returned. - Recalibrated: "expanding a root reveals its direct children" (`[10, 1, 3, 2]` → `[10, 1, 2, 3]`, Rent now sorts before Utilities by name). ## Mutation proofs Each mutation was applied alone, then `vitest run` on `useBudget.test.ts`, `reorderRows.test.ts` and `collapsibleRows.test.ts`, then the file was restored. | # | Mutation | rc | Red tests | |---|---|---|---| | M1 | depth sort put back on the group's rows in the builder | 1 | 8 (exact orders, depth-first invariant, collapse, bottom subtotals) | | M2 | block close removed in `reorderRows` | 1 | 3 (two levels, three levels, builder + reorder) | | M3 | "(direct)" no longer forced first | 1 | 2 (older taxonomy, builder + reorder) | | M4 | blocks no longer sorted by name | 1 | 3 (Finances, older taxonomy, builder + reorder) | | M5 | top groups ignore the type | 1 | 1 (top-group order) | | M6 | top groups lose the name tie-break | 1 | 1 (top-group order) | Validation in the worktree: `npm ci`, `npm run build` green, `npm test` 120 files / 2076 tests green. No Rust touched, no migration (v1→v17 unchanged), no i18n string, no network call. ## Decisions (autopilot) - [HIGH] The mechanism follows the issue exactly: blocks, one sort, concatenation, and no comparator on rows inside a group. - [HIGH] `buildLevel2Group` keeps its sort of the sub-group's leaves ("(direct)" first, then name). It is the same block rule one level down, since every block there is a lone leaf. - [MEDIUM] The top-group sort reads the type of the top category; the old comparator read each row's own type. The result is the same for both seeded taxonomies, where children carry their root's type, and `BudgetTable` groups rows by `category_type` anyway. Rejected alternative: a per-row type sort, which is the forbidden comparator on rows inside a group. - [HIGH] `Resolves #424`: the issue has no post-merge action. ## For the reviewer - `src/hooks/useBudget.ts`: the top-level loop now fills `topGroups`, with `pushBlock` dropping empty blocks (`buildLevel2Group` can return `[]`). `catById` was only used by the removed comparator. - `src/utils/reorderRows.ts`: the new branch sits between "parent of this level" and "child of the current parent". In a recursive call, rows are always deeper than the caller's level, so the `<` part only matters at the top. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
maximus added 2 commits 2026-10-14 00:44:20 +00:00
buildBudgetYearRows sorted each top group by depth, so an expanded
subcategory showed at the end of its group instead of under its parent.
It now builds one block per child (lone leaf, or [subtotal, ...children]),
sorts the blocks once ("(direct)" first, then by name, leaves and
sub-groups mixed, as the Budget vs Actual report) and concatenates them;
top groups are sorted by type, sort_order and name.

reorderRows put a leaf that follows a sibling sub-group into that
sub-group's block. A row at the subtotal's depth or shallower now closes
the block, which also fixes "subtotals at the bottom" in the Budget vs
Actual and Compare reports.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
docs(changelog): budget rows under their parent, subtotals on their own rows
All checks were successful
PR Check — Frontend / frontend (pull_request) Successful in 1m59s
22dcad7a29
Resolves #424

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
maximus added the
autopilot:pending-human
label 2026-10-14 00:44:27 +00:00
Author
Owner

Self-review (autopilot worker, not independent): APPROVE

Generated autonomously by /autopilot run of 2026-10-13. I reviewed the full diff of 22dcad7 against main.

Acceptance criteria

  • Fully expanded, every subcategory sits directly under its parent, in both taxonomies and with subtotals on top or at the bottom. Proved by expectDepthFirst and by exact orders on Revenus, Finances & obligations and Dépenses récurrentes → Assurances, plus reorderRows(…, false) run on the builder output.
  • Expanding a single intermediate parent shows its children right under it. Proved by the Revenus + Emploi test.
  • Subtotals at the bottom: each subtotal follows its own rows. Proved by the new reorderRows.test.ts (two and three levels). Budget vs Actual and Compare call the same utility and their rows carry depth (CategoryDelta.depth, BudgetVsActualRow.depth).
  • Subtotals, section totals and Result rows are unchanged. Sums still come from allChildRows.filter(!is_parent), and the "subtotals are unchanged" test passes. BudgetTable.render.test.tsx and budgetOnboardingTarget.test.ts are untouched and green.
  • npm run build green; npm test green, 120 files and 2076 tests.

Other checks

  • Security: pure functions only. No I/O, no SQL, no network call.
  • Regressions:
    • A root leaf that follows a root group used to be absorbed into that group's block in bottom mode. It is now fixed by the same rule.
    • The top-group sort now reads the root's type instead of each row's type. Nothing changes for the seeded taxonomies, and BudgetTable groups rows by category_type anyway (decision MEDIUM in the PR body).
    • The bfsTree collapse case is kept.
  • i18n: no UI string added.
  • CHANGELOG: the issue's wording is under Fixed / Corrigé in both files.
  • Migrations: none touched.
  • Docs: no doc describes the row order (grep on docs/), so architecture.md and the guide are not affected.
  • Mutations: six targeted mutations, all red (table in the PR body).

For the independent reviewer: confirm the block-name sort reproduces budgetService.ts (D6). The fixtures lock it on real seed names.

## Self-review (autopilot worker, not independent): APPROVE Generated autonomously by /autopilot run of 2026-10-13. I reviewed the full diff of `22dcad7` against `main`. **Acceptance criteria** - [x] Fully expanded, every subcategory sits directly under its parent, in both taxonomies and with subtotals on top or at the bottom. Proved by `expectDepthFirst` and by exact orders on Revenus, Finances & obligations and Dépenses récurrentes → Assurances, plus `reorderRows(…, false)` run on the builder output. - [x] Expanding a single intermediate parent shows its children right under it. Proved by the Revenus + Emploi test. - [x] Subtotals at the bottom: each subtotal follows its own rows. Proved by the new `reorderRows.test.ts` (two and three levels). Budget vs Actual and Compare call the same utility and their rows carry `depth` (`CategoryDelta.depth`, `BudgetVsActualRow.depth`). - [x] Subtotals, section totals and Result rows are unchanged. Sums still come from `allChildRows.filter(!is_parent)`, and the "subtotals are unchanged" test passes. `BudgetTable.render.test.tsx` and `budgetOnboardingTarget.test.ts` are untouched and green. - [x] `npm run build` green; `npm test` green, 120 files and 2076 tests. **Other checks** - Security: pure functions only. No I/O, no SQL, no network call. - Regressions: - A root leaf that follows a root group used to be absorbed into that group's block in bottom mode. It is now fixed by the same rule. - The top-group sort now reads the root's type instead of each row's type. Nothing changes for the seeded taxonomies, and `BudgetTable` groups rows by `category_type` anyway (decision MEDIUM in the PR body). - The `bfsTree` collapse case is kept. - i18n: no UI string added. - CHANGELOG: the issue's wording is under Fixed / Corrigé in both files. - Migrations: none touched. - Docs: no doc describes the row order (grep on `docs/`), so `architecture.md` and the guide are not affected. - Mutations: six targeted mutations, all red (table in the PR body). **For the independent reviewer:** confirm the block-name sort reproduces `budgetService.ts` (D6). The fixtures lock it on real seed names.
Author
Owner

CI lue par l'orchestrateur sur la tête 22dcad7 : PR Check — Frontend run 543 vert (build OK, vitest 120 fichiers / 2076 tests, comptes égaux à ceux du worker). Pas de job Rust attendu (aucun fichier src-tauri/**).

Generated autonomously by /autopilot run of 2026-10-13

CI lue par l'orchestrateur sur la tête `22dcad7` : **PR Check — Frontend** run 543 vert (build OK, vitest 120 fichiers / 2076 tests, comptes égaux à ceux du worker). Pas de job Rust attendu (aucun fichier `src-tauri/**`). Generated autonomously by /autopilot run of 2026-10-13
Author
Owner

Revue adversariale — PR #438 (#424)

Verdict : APPROVE

Le correctif fait ce que l'issue demande : les lignes du budget sortent en profondeur d'abord, par blocs triés une seule fois, sans comparateur sur les lignes d'un groupe. reorderRows ferme le bloc d'un sous-total dès qu'une ligne de même profondeur ou moins profonde arrive. Les deux consommateurs de rapports (budgetService.buildSubGroup, reportService.buildNode) émettent déjà en profondeur d'abord avec depth + 1 pour leur ligne « (direct) ». La nouvelle branche ne touche donc que le cas fautif là aussi.

Ce que j'ai exécuté (worktree détaché sur 22dcad7, npm ci propre)

  • npm run build vert, vitest run : 120 fichiers, 2076 tests verts. CI de la tête : PR Check — Frontend en success (statut agrégé).
  • Mutations du relecteur, une à la fois, avec restauration par git checkout (arbre propre vérifié) :
# Mutation Tests rouges
R1 reorderRows : <= parentDepth remplacé par < parentDepth (la feuille sœur de même profondeur est de nouveau absorbée) 3
R2 pushBlock trie un sous-groupe sur le nom de sa dernière ligne au lieu de son sous-total 6
R3 useBudget.ts de main remis en entier, reorderRows corrigé gardé 8
R4 « (direct) » envoyé en dernier au lieu du premier 2
  • Sonde jetable (supprimée) sur un cas que les tests n'épinglent pas : sous-groupe replié (sous-total seul, enfants masqués par groups.visible) suivi d'une feuille sœur, puis d'un sous-groupe déplié et d'une racine feuille. Résultat attendu [A, B, c1, C, T, L] obtenu. Avant le correctif, B aurait glissé sous le sous-total replié A.

Checklist

  • Sécurité : rien (pur calcul d'ordre, aucune entrée externe, aucun secret).
  • Données : aucune migration, aucun SQL.
  • i18n : aucune chaîne nouvelle. CHANGELOG Corrigé présent dans les deux fichiers.
  • Commits au format conventionnel.

Suggestions (non bloquantes)

  1. Fermeture de #424. La timeline de l'issue porte un pull_ref en ref_action: "none" : le seul Resolves #424 du corps est entre backticks dans la liste des décisions, et Forgejo ne le lit pas. La fermeture repose sur le message du commit 1c427a1 (Resolves #424), ce qui suffit pour un merge ff (/merge-stack ou API). Ajouter une ligne Resolves #424 en clair dans le corps couvrirait aussi un merge par commit de merge.
  2. Cas replié non épinglé. reorderRows.test.ts couvre la feuille après un sous-groupe déplié, pas après un sous-groupe replié (sous-total sans enfants). C'est la même branche de code, ma sonde passe ; un test de plus éviterait qu'un refactor ne traite le « bloc vide » à part.
  3. Hors périmètre, préexistant : une catégorie de niveau 1 saisissable dont tous les enfants sont non saisissables passe par buildLevel2Group, qui la rend en feuille depth: 2 directement sous la racine. La PR ne change pas ce cas et l'ordre reste correct, mais l'indentation saute un niveau.

Actions faites

  • Worktree de revue supprimé (git worktree remove). La sonde src/zz_tmp_probe.test.ts a été effacée avant le build.
  • Label de #424 : status:review → status:approved.
## Revue adversariale — PR #438 (#424) **Verdict : APPROVE** Le correctif fait ce que l'issue demande : les lignes du budget sortent en profondeur d'abord, par blocs triés une seule fois, sans comparateur sur les lignes d'un groupe. `reorderRows` ferme le bloc d'un sous-total dès qu'une ligne de même profondeur ou moins profonde arrive. Les deux consommateurs de rapports (`budgetService.buildSubGroup`, `reportService.buildNode`) émettent déjà en profondeur d'abord avec `depth + 1` pour leur ligne « (direct) ». La nouvelle branche ne touche donc que le cas fautif là aussi. ### Ce que j'ai exécuté (worktree détaché sur `22dcad7`, `npm ci` propre) - `npm run build` vert, `vitest run` : 120 fichiers, 2076 tests verts. CI de la tête : `PR Check — Frontend` en `success` (statut agrégé). - Mutations du relecteur, une à la fois, avec restauration par `git checkout` (arbre propre vérifié) : | # | Mutation | Tests rouges | |---|---|---| | R1 | `reorderRows` : `<= parentDepth` remplacé par `< parentDepth` (la feuille sœur de même profondeur est de nouveau absorbée) | 3 | | R2 | `pushBlock` trie un sous-groupe sur le nom de sa **dernière** ligne au lieu de son sous-total | 6 | | R3 | `useBudget.ts` de `main` remis en entier, `reorderRows` corrigé gardé | 8 | | R4 | « (direct) » envoyé en dernier au lieu du premier | 2 | - Sonde jetable (supprimée) sur un cas que les tests n'épinglent pas : sous-groupe **replié** (sous-total seul, enfants masqués par `groups.visible`) suivi d'une feuille sœur, puis d'un sous-groupe déplié et d'une racine feuille. Résultat attendu `[A, B, c1, C, T, L]` obtenu. Avant le correctif, B aurait glissé sous le sous-total replié A. ### Checklist - Sécurité : rien (pur calcul d'ordre, aucune entrée externe, aucun secret). - Données : aucune migration, aucun SQL. - i18n : aucune chaîne nouvelle. CHANGELOG Corrigé présent dans les deux fichiers. - Commits au format conventionnel. ### Suggestions (non bloquantes) 1. **Fermeture de #424.** La timeline de l'issue porte un `pull_ref` en `ref_action: "none"` : le seul `Resolves #424` du corps est entre backticks dans la liste des décisions, et Forgejo ne le lit pas. La fermeture repose sur le message du commit `1c427a1` (`Resolves #424`), ce qui suffit pour un merge ff (`/merge-stack` ou API). Ajouter une ligne `Resolves #424` en clair dans le corps couvrirait aussi un merge par commit de merge. 2. **Cas replié non épinglé.** `reorderRows.test.ts` couvre la feuille après un sous-groupe déplié, pas après un sous-groupe replié (sous-total sans enfants). C'est la même branche de code, ma sonde passe ; un test de plus éviterait qu'un refactor ne traite le « bloc vide » à part. 3. Hors périmètre, préexistant : une catégorie de niveau 1 saisissable dont tous les enfants sont non saisissables passe par `buildLevel2Group`, qui la rend en feuille `depth: 2` directement sous la racine. La PR ne change pas ce cas et l'ordre reste correct, mais l'indentation saute un niveau. ### Actions faites - Worktree de revue supprimé (`git worktree remove`). La sonde `src/zz_tmp_probe.test.ts` a été effacée avant le build. - Label de #424 : `status:review` → `status:approved`.
maximus merged commit 22dcad7a29 into main 2026-10-16 00:36:49 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: maximus/Simpl-Resultat#438
No description provided.