fix(budget): grid rows depth-first, subtotals at the bottom on their own rows (#424) #438
No reviewers
Labels
No labels
autopilot:pending-human
source:analyste
source:defenseur
source:human
source:medic
status:approved
status:blocked
status:in-progress
status:needs-clarification
status:needs-fix
status:ready
status:review
status:triage
type:bug
type:feature
type:infra
type:refactor
type:schema
type:security
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: maximus/Simpl-Resultat#438
Loading…
Reference in a new issue
No description provided.
Delete branch "issue-424-budget-row-order"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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](whatbuildLevel2Groupalready returns). The blocks are sorted once, before emission: the "(direct)" block first, thenlocaleCompareof 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 byTYPE_ORDER,sort_order, then name. The old row comparator (depth sort,getTopGroupId) is gone.reorderRows(src/utils/reorderRows.ts): when a row has depth <=parentDepthand 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.useBudget.ts, the "level-ordered" wording incollapsibleRows.ts, and thebfsTreecomment incollapsibleRows.test.ts. ThebfsTreecase stays, as the proof that collapse does not depend on row order.Visible change, as Max decided at planning: in Revenus, "Autres revenus" now comes first.
Files
src/hooks/useBudget.ts,src/hooks/useBudget.test.tssrc/utils/reorderRows.ts,src/utils/reorderRows.test.ts(new)src/utils/collapsibleRows.ts,src/utils/collapsibleRows.test.ts(comments only)CHANGELOG.md,CHANGELOG.fr.mdBudgetTable.render.test.tsxandbudgetOnboardingTarget.test.tsare unchanged and still green: their rows are built by hand, two levels deep.Tests
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.consolidated_schema.sql): a depth-1 leaf comes first, then three sub-groups;sort_order, then name, with the input given in reverse.reorderRows(buildBudgetYearRows(...), false)on two groups: each subtotal follows its own rows.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.[10, 1, 3, 2]→[10, 1, 2, 3], Rent now sorts before Utilities by name).Mutation proofs
Each mutation was applied alone, then
vitest runonuseBudget.test.ts,reorderRows.test.tsandcollapsibleRows.test.ts, then the file was restored.reorderRowsValidation in the worktree:
npm ci,npm run buildgreen,npm test120 files / 2076 tests green. No Rust touched, no migration (v1→v17 unchanged), no i18n string, no network call.Decisions (autopilot)
buildLevel2Groupkeeps 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.BudgetTablegroups rows bycategory_typeanyway. Rejected alternative: a per-row type sort, which is the forbidden comparator on rows inside a group.Resolves #424: the issue has no post-merge action.For the reviewer
src/hooks/useBudget.ts: the top-level loop now fillstopGroups, withpushBlockdropping empty blocks (buildLevel2Groupcan return[]).catByIdwas 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
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>Self-review (autopilot worker, not independent): APPROVE
Generated autonomously by /autopilot run of 2026-10-13. I reviewed the full diff of
22dcad7againstmain.Acceptance criteria
expectDepthFirstand by exact orders on Revenus, Finances & obligations and Dépenses récurrentes → Assurances, plusreorderRows(…, false)run on the builder output.reorderRows.test.ts(two and three levels). Budget vs Actual and Compare call the same utility and their rows carrydepth(CategoryDelta.depth,BudgetVsActualRow.depth).allChildRows.filter(!is_parent), and the "subtotals are unchanged" test passes.BudgetTable.render.test.tsxandbudgetOnboardingTarget.test.tsare untouched and green.npm run buildgreen;npm testgreen, 120 files and 2076 tests.Other checks
BudgetTablegroups rows bycategory_typeanyway (decision MEDIUM in the PR body).bfsTreecollapse case is kept.docs/), soarchitecture.mdand the guide are not affected.For the independent reviewer: confirm the block-name sort reproduces
budgetService.ts(D6). The fixtures lock it on real seed names.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 fichiersrc-tauri/**).Generated autonomously by /autopilot run of 2026-10-13
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.
reorderRowsferme 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 avecdepth + 1pour 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 cipropre)npm run buildvert,vitest run: 120 fichiers, 2076 tests verts. CI de la tête :PR Check — Frontendensuccess(statut agrégé).git checkout(arbre propre vérifié) :reorderRows:<= parentDepthremplacé par< parentDepth(la feuille sœur de même profondeur est de nouveau absorbée)pushBlocktrie un sous-groupe sur le nom de sa dernière ligne au lieu de son sous-totaluseBudget.tsdemainremis en entier,reorderRowscorrigé gardé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
Suggestions (non bloquantes)
pull_refenref_action: "none": le seulResolves #424du corps est entre backticks dans la liste des décisions, et Forgejo ne le lit pas. La fermeture repose sur le message du commit1c427a1(Resolves #424), ce qui suffit pour un merge ff (/merge-stackou API). Ajouter une ligneResolves #424en clair dans le corps couvrirait aussi un merge par commit de merge.reorderRows.test.tscouvre 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.buildLevel2Group, qui la rend en feuilledepth: 2directement sous la racine. La PR ne change pas ce cas et l'ordre reste correct, mais l'indentation saute un niveau.Actions faites
git worktree remove). La sondesrc/zz_tmp_probe.test.tsa été effacée avant le build.status:review→status:approved.