maximus/Simpl-Resultat

Fork 0

refactor(reports): extraire un constructeur d'arbre feuille-driven generique (#263) #267

Closed

maximus wants to merge 0 commits from issue-263-generic-tree-builder into main

maximus commented

2026-07-07 23:07:53 +00:00

Owner

Resolves #263

Generated autonomously by /autopilot run of 2026-07-07

Objectif

Factoriser le tree-builder de buildCompareTree en un constructeur d'arbre feuille-driven generique buildLeafDrivenTree<T>(leaves, opts), reutilisable par #264 (arbre des tendances). Refactor behavior-preserving : getCompareMonthOverMonth / getCompareYearOverYear produisent un output strictement identique.

Ce qui change (`src/services/reportService.ts` uniquement)

Nouveau buildLeafDrivenTree<T>(leaves, opts) exporte + types TreeCatMeta, TreeSectionType, LeafDrivenTreeOptions<T>. Le squelette partage : split feuille/orphelin, set « relevant » (feuille + ancetres), adjacence en ordre DB, recursion buildNode avec garde MAX_TREE_DEPTH + loop-break conservee, tri des freres par magnitude, append des orphelins, tri stable par section (income -> expense -> transfer).
buildCompareTree devient une fine specialisation qui n'injecte que l'arithmetique des deltas (makeLeaf / makeSubtotal / decorateDirectLeaf / makeOrphan / sortKey / isSubtotal / sectionOf / sectionOrder). CompareCatMeta = alias de TreeCatMeta.

Preuve behavior-preserving

Tests compare inchanges (aucune assertion modifiee) : git diff --name-only ne touche que reportService.ts, zero fichier de test.
reportService.test.ts (cas compare + buildCompareTree hierarchie) + compareResults.test.ts = 44 tests verts.
Suite complete : 713 vitest verts. Build (tsc + vite) vert.
Mapping ligne-a-ligne verifie : makeLeaf == ancien leafRow, makeSubtotal == ancien subtotalRow, sortKey=Math.abs(deltaAbs), sectionOf=category_type ?? "expense", sectionOrder=COMPARE_TYPE_ORDER, unknownSectionRank=9 (defaut) == ?? 9, maxDepth=MAX_TREE_DEPTH (defaut).

Decisions

[MEDIUM] Builder place dans reportService.ts (a cote de buildCompareTree), pas un nouveau fichier — suit le pattern existant (buildCompareTree y vit deja) et minimise le rayon d'explosion. Alternative ecartee : src/services/leafDrivenTree.ts dedie (plus isole mais plus de churn).
[MEDIUM] T sert a la fois de payload de feuille et de ligne de sortie (les champs hierarchie sont optionnels sur CategoryDelta) — colle au compare et a ce dont #264 (sidecar id-keyed) a besoin. Deux type-params <TLeaf, TRow> ecartes (sur-ingenierie).
[MEDIUM] CompareCatMeta converti en alias de TreeCatMeta (shape identique) pour eviter la derive ; le commentaire compare-specifique (fetch sans filtre is_active) est conserve.

Signature pour #264

export function buildLeafDrivenTree<T>(leaves: T[], opts: LeafDrivenTreeOptions<T>): T[]

Aucune migration DB (v1->v16 inchangees). Refactor interne pur -> pas d'entree CHANGELOG, pas de cle i18n.

Resolves #263 Generated autonomously by /autopilot run of 2026-07-07 ## Objectif Factoriser le tree-builder de `buildCompareTree` en un constructeur d'arbre feuille-driven **generique** `buildLeafDrivenTree<T>(leaves, opts)`, reutilisable par #264 (arbre des tendances). **Refactor behavior-preserving** : `getCompareMonthOverMonth` / `getCompareYearOverYear` produisent un output strictement identique. ## Ce qui change (`src/services/reportService.ts` uniquement) - Nouveau `buildLeafDrivenTree<T>(leaves, opts)` exporte + types `TreeCatMeta`, `TreeSectionType`, `LeafDrivenTreeOptions<T>`. Le squelette partage : split feuille/orphelin, set « relevant » (feuille + ancetres), adjacence en ordre DB, recursion `buildNode` avec **garde `MAX_TREE_DEPTH` + loop-break** conservee, tri des freres par magnitude, append des orphelins, tri stable par section (income -> expense -> transfer). - `buildCompareTree` devient une **fine specialisation** qui n'injecte que l'arithmetique des deltas (`makeLeaf` / `makeSubtotal` / `decorateDirectLeaf` / `makeOrphan` / `sortKey` / `isSubtotal` / `sectionOf` / `sectionOrder`). `CompareCatMeta` = alias de `TreeCatMeta`. ## Preuve behavior-preserving - **Tests compare inchanges** (aucune assertion modifiee) : `git diff --name-only` ne touche que `reportService.ts`, zero fichier de test. - `reportService.test.ts` (cas compare + `buildCompareTree` hierarchie) + `compareResults.test.ts` = **44 tests verts**. - Suite complete : **713 vitest verts**. Build (tsc + vite) vert. - Mapping ligne-a-ligne verifie : `makeLeaf` == ancien `leafRow`, `makeSubtotal` == ancien `subtotalRow`, `sortKey`=`Math.abs(deltaAbs)`, `sectionOf`=`category_type ?? "expense"`, `sectionOrder`=`COMPARE_TYPE_ORDER`, `unknownSectionRank`=9 (defaut) == `?? 9`, `maxDepth`=`MAX_TREE_DEPTH` (defaut). ## Decisions - [MEDIUM] Builder place dans `reportService.ts` (a cote de `buildCompareTree`), pas un nouveau fichier — suit le pattern existant (buildCompareTree y vit deja) et minimise le rayon d'explosion. Alternative ecartee : `src/services/leafDrivenTree.ts` dedie (plus isole mais plus de churn). - [MEDIUM] `T` sert a la fois de payload de feuille et de ligne de sortie (les champs hierarchie sont optionnels sur `CategoryDelta`) — colle au compare et a ce dont #264 (sidecar id-keyed) a besoin. Deux type-params `<TLeaf, TRow>` ecartes (sur-ingenierie). - [MEDIUM] `CompareCatMeta` converti en alias de `TreeCatMeta` (shape identique) pour eviter la derive ; le commentaire compare-specifique (fetch sans filtre is_active) est conserve. ## Signature pour #264 `export function buildLeafDrivenTree<T>(leaves: T[], opts: LeafDrivenTreeOptions<T>): T[]` Aucune migration DB (v1->v16 inchangees). Refactor interne pur -> pas d'entree CHANGELOG, pas de cle i18n.

maximus added 1 commit 2026-07-07 23:07:54 +00:00

refactor(reports): extract a generic leaf-driven tree builder

PR Check / rust (pull_request) Successful in 23m41s

Details

PR Check / frontend (pull_request) Successful in 2m27s

Details

5a1ce42034

Factor the hand-rolled hierarchy walk out of buildCompareTree into a
generic buildLeafDrivenTree<T>(leaves, opts): the shared skeleton (leaf/
orphan split, ancestor "relevant" set, DB-order adjacency, buildNode
recursion with the MAX_TREE_DEPTH guard + loop-break, magnitude sibling
ordering, orphan append, contiguous income->expense->transfer section
sort) now lives in one place, parameterised by injection points
(categoryIdOf, makeLeaf, makeSubtotal, decorateDirectLeaf, makeOrphan,
sortKey, isSubtotal, sectionOf, sectionOrder, maxDepth).

buildCompareTree becomes a thin specialisation supplying only the delta
arithmetic; output is byte-identical (compare tests unchanged as the
non-regression guard). Exported (with TreeCatMeta / TreeSectionType /
LeafDrivenTreeOptions) so the trends tree reuses the exact same skeleton.

Behavior-preserving refactor: no user-facing change, no CHANGELOG entry,
no DB migration.

Resolves #263

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

maximus added the

autopilot:pending-human

type:refactor

labels 2026-07-07 23:08:03 +00:00

maximus commented

2026-07-08 00:11:51 +00:00

Author

Owner

Revue /pr-review — APPROVE

Verdict : APPROVE ✅

Résumé

Extraction propre et behavior-preserving du tree-builder de buildCompareTree vers un constructeur générique réutilisable buildLeafDrivenTree<T>. buildCompareTree devient une fine spécialisation qui n'injecte que l'arithmétique des deltas. La preuve la plus forte est là : seul src/services/reportService.ts change (git diff --name-only main..HEAD), le fichier de test reportService.test.ts est byte-identique — les 44 tests compare/hiérarchie sont le garde-fou de non-régression.

Vérification behavior-preserving (mapping ligne-à-ligne)

J'ai retracé chaque point d'injection contre l'ancien code inline, tous exacts :

Ancien	Nouveau	OK
orphan check `d.categoryId != null && catById.has(...)`	`categoryIdOf` + même garde	✅
boucle relevant sur `MAX_TREE_DEPTH`	`maxDepth` (défaut `MAX_TREE_DEPTH`)	✅
`leafRow` (spread + stamp)	`makeLeaf` idem	✅
`subtotalRow` (somme + `deltaPct` dénominateur signé)	`makeSubtotal` arithmétique identique	✅
direct leaf `{ ...direct, categoryName: '(direct)' }`	`decorateDirectLeaf(makeLeaf(...))`	✅
`subtotalRow` orphelins	`makeOrphan` idem	✅
tri final `COMPARE_TYPE_ORDER[..] ?? 9`	`sectionOrder[sectionOf] ?? unknownSectionRank(=9)`	✅

Points scrutés de plus près (les deux seuls endroits où la forme change) :

descendantLeaves filter : !r.is_parent → !isSubtotal(r) = !(r.is_parent === true). Équivalent : toute ligne construite dans buildNode porte un is_parent strictement booléen (makeLeaf→false, makeSubtotal→true), jamais undefined. Aucune divergence.
sortKey du direct leaf : désormais calculé sur la ligne décorée (post-categoryName), mais deltaAbs n'est pas touché par le renommage → ordre des frères identique au byte.

Autres contrôles

Ordre de déclaration sain : MAX_TREE_DEPTH avant le builder générique ; COMPARE_TYPE_ORDER (const) lu seulement au runtime par buildCompareTree (pas de TDZ) ; buildLeafDrivenTree est une function declaration hoistée.
Types : les champs hiérarchie de CategoryDelta (parent_id/is_parent/depth/category_type) sont optionnels → T = CategoryDelta typecheck sans souci.
Pas de casse externe : buildCompareTree n'est consommé qu'en interne (getCompareMonthOverMonth/YoY) + tests ; COMPARE_TYPE_ORDER/COMPARE_CATEGORIES_SQL restent module-private (seule mention = un commentaire doc dans overTimeResults.ts).
Commit conventionnel (refactor(reports): …), Resolves #263, aucun artefact debug / .only / .skip. Refactor interne pur → pas de migration, pas d'i18n, pas de CHANGELOG (correct).
PR cible main → couverte par check.yml. C'est le maillon de base de la pile reports (#267 → #268/#269) : le builder générique atterrit d'abord pour que #264 le consomme.

Suggestions (non bloquantes)

buildLeafDrivenTree n'a pas de test unitaire direct — il est couvert à 100 % transitivement par la suite compare, et #264 apportera la couverture du second consommateur. OK de différer.

Beau refactor, bien commenté, rayon d'explosion minimal.

## Revue /pr-review — APPROVE **Verdict : APPROVE** ✅ ### Résumé Extraction propre et *behavior-preserving* du tree-builder de `buildCompareTree` vers un constructeur générique réutilisable `buildLeafDrivenTree<T>`. `buildCompareTree` devient une fine spécialisation qui n'injecte que l'arithmétique des deltas. La preuve la plus forte est là : **seul `src/services/reportService.ts` change** (`git diff --name-only main..HEAD`), le fichier de test `reportService.test.ts` est **byte-identique** — les 44 tests compare/hiérarchie sont le garde-fou de non-régression. ### Vérification behavior-preserving (mapping ligne-à-ligne) J'ai retracé chaque point d'injection contre l'ancien code inline, tous exacts : | Ancien | Nouveau | OK | |---|---|---| | orphan check `d.categoryId != null && catById.has(...)` | `categoryIdOf` + même garde | ✅ | | boucle relevant sur `MAX_TREE_DEPTH` | `maxDepth` (défaut `MAX_TREE_DEPTH`) | ✅ | | `leafRow` (spread + stamp) | `makeLeaf` idem | ✅ | | `subtotalRow` (somme + `deltaPct` dénominateur signé) | `makeSubtotal` arithmétique identique | ✅ | | direct leaf `{ ...direct, categoryName: '(direct)' }` | `decorateDirectLeaf(makeLeaf(...))` | ✅ | | `subtotalRow` orphelins | `makeOrphan` idem | ✅ | | tri final `COMPARE_TYPE_ORDER[..] ?? 9` | `sectionOrder[sectionOf] ?? unknownSectionRank(=9)` | ✅ | Points scrutés de plus près (les deux seuls endroits où la *forme* change) : - **`descendantLeaves` filter** : `!r.is_parent` → `!isSubtotal(r)` = `!(r.is_parent === true)`. Équivalent : toute ligne construite dans `buildNode` porte un `is_parent` strictement booléen (`makeLeaf`→`false`, `makeSubtotal`→`true`), jamais `undefined`. Aucune divergence. - **`sortKey` du direct leaf** : désormais calculé sur la ligne *décorée* (post-`categoryName`), mais `deltaAbs` n'est pas touché par le renommage → ordre des frères identique au byte. ### Autres contrôles - **Ordre de déclaration** sain : `MAX_TREE_DEPTH` avant le builder générique ; `COMPARE_TYPE_ORDER` (const) lu seulement au runtime par `buildCompareTree` (pas de TDZ) ; `buildLeafDrivenTree` est une function declaration hoistée. - **Types** : les champs hiérarchie de `CategoryDelta` (`parent_id`/`is_parent`/`depth`/`category_type`) sont optionnels → `T = CategoryDelta` typecheck sans souci. - **Pas de casse externe** : `buildCompareTree` n'est consommé qu'en interne (`getCompareMonthOverMonth/YoY`) + tests ; `COMPARE_TYPE_ORDER`/`COMPARE_CATEGORIES_SQL` restent module-private (seule mention = un commentaire doc dans `overTimeResults.ts`). - Commit conventionnel (`refactor(reports): …`), `Resolves #263`, aucun artefact debug / `.only` / `.skip`. Refactor interne pur → pas de migration, pas d'i18n, pas de CHANGELOG (correct). - PR cible `main` → couverte par `check.yml`. C'est le maillon de base de la pile reports (#267 → #268/#269) : le builder générique atterrit d'abord pour que #264 le consomme. ### Suggestions (non bloquantes) 1. `buildLeafDrivenTree` n'a pas de test unitaire *direct* — il est couvert à 100 % transitivement par la suite compare, et #264 apportera la couverture du second consommateur. OK de différer. Beau refactor, bien commenté, rayon d'explosion minimal.

maximus referenced this pull request

2026-07-08 00:18:49 +00:00

feat(reports): getCategoryOverTime -> arbre tendances (sidecar id-keyed) + resultats (#264) #268

maximus commented

2026-07-08 00:34:03 +00:00

Author

Owner

Mergée localement dans main (pile #266/#267/#268/#269, tip fe87313). Fermée manuellement — merge local d'une pile stackée (l'API merge Forgejo échoue sur les bases non-main), tip cumulé validé build + 728 vitest verts.

Mergée localement dans `main` (pile #266/#267/#268/#269, tip `fe87313`). Fermée manuellement — merge local d'une pile stackée (l'API merge Forgejo échoue sur les bases non-`main`), tip cumulé validé build + 728 vitest verts.

maximus closed this pull request

2026-07-08 00:34:03 +00:00

PR Check / rust (pull_request) Successful in 23m41s

Details

PR Check / frontend (pull_request) Successful in 2m27s

Details