feat(reports): hierarchical real-vs-real compare with subtotals (#247)

maximus commented

2026-07-04 21:32:54 +00:00

Owner

Renders the real-vs-real comparable report as a parent/child category tree with subtotal rows (net per group), mirroring the budget report — expense/income/transfer sections, per-section + grand net totals, subtotals-on-top/bottom toggle. The service builds the tree on top of the flat per-category deltas; leaf values are unchanged and the #243 transfer netting is preserved (balanced transfer groups subtotal to ~0).

Stacked on #248 (issue #243). Review vs base issue-243-netting-transferts.

Resolves #247

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

Renders the real-vs-real comparable report as a parent/child category tree with subtotal rows (net per group), mirroring the budget report — expense/income/transfer sections, per-section + grand net totals, subtotals-on-top/bottom toggle. The service builds the tree on top of the flat per-category deltas; leaf values are unchanged and the #243 transfer netting is preserved (balanced transfer groups subtotal to ~0). Stacked on #248 (issue #243). Review vs base issue-243-netting-transferts. Resolves #247 Generated autonomously by /autopilot run of 2026-07-04

maximus added 1 commit 2026-07-04 21:32:55 +00:00

feat(reports): hierarchical real-vs-real compare with subtotals f3e8e94b16

Render the real-vs-real comparable report as a parent/child category tree
with subtotal rows, mirroring the budget report — rows grouped into
expense/income/transfer sections with per-section and grand totals and a
subtotals-on-top/bottom toggle. The compare service now builds the tree on
top of the flat per-category deltas, so leaf-category figures are unchanged
and the #243 transfer netting is preserved (a balanced transfer group
subtotals to ~0, i.e. the group's net).

- reportService: buildCompareTree() synthesizes subtotal rows from the flat
  leaves + category metadata; getCompareMonthOverMonth/YoY return the tree
  (COMPARE_DELTA_SQL and rowsToDeltas untouched).
- CategoryDelta gains optional parent_id/is_parent/depth/category_type.
- ComparePeriodTable: sections, depth indentation, reorderRows toggle,
  section/grand net totals.
- Cartes top-movers and ComparePeriodChart filter to leaf rows only.
- reorderRows constraint loosened to the two fields it reads.
- i18n (FR+EN) section labels; CHANGELOG entries.

Resolves #247

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

maximus added the

status:approved

autopilot:pending-human

labels 2026-07-04 21:33:02 +00:00

maximus commented

2026-07-05 01:00:50 +00:00

Author

Owner

Revue /pr-review — APPROVE

Résumé. L'implémentation transforme le rapport comparable réel-vs-réel en arbre parent/enfant avec sous-totaux, en miroir fidèle du rapport de budget déjà en production (BudgetVsActualTable). Le netting des transferts (#243) est préservé : les valeurs des feuilles sont inchangées et un sous-total = somme arithmétique des feuilles descendantes = net du groupe. Bonne couverture de tests, i18n complet FR/EN, CHANGELOG mis à jour dans les deux fichiers, aucune migration ni schéma touché.

Points vérifiés

buildCompareTree : sous-total = somme des feuilles descendantes (pas de double-comptage multi-niveaux car descendantLeaves filtre is_parent), garde anti-cycle via MAX_TREE_DEPTH, tri final stable par type (sections contiguës).
Invariance du grand total vs feuilles plates (testée).
ComparePeriodChart et getCartesSnapshot filtrent is_parent → pas de double-comptage des sous-totaux contre leurs propres feuilles.
reorderRows : contrainte générique assouplie aux 2 seuls champs lus (is_parent, depth) — compatible budget (champs requis) et compare (optionnels) ; l'algorithme ne lit ni parent_id ni category_id, l'assouplissement est sûr.
L'hypothèse « un sous-arbre partage un seul type » est préexistante et déjà en prod dans BudgetVsActualTable (même sectionnement contigu par type) — #247 n'introduit aucun risque neuf.
Clés i18n réutilisées (reports.subtotalsOnTop/Bottom, reports.compare.totalRow, reports.bva.*) présentes ; nouvelles clés ajoutées FR + EN.
Tests cohérents avec l'implémentation (6 cas buildCompareTree + test de câblage + assertion call-count passée à 2 requêtes).

Suggestions (non bloquantes)

buildCompareTree — feuille à parent_id orphelin. Une feuille dont le parent_id (non-null) pointe vers un id absent du jeu de catégories n'est ni une racine (son parent manquant est marqué « relevant » pendant la remontée d'ancêtres) ni rattachée à un parent réel (aucune entrée d'adjacence) → elle est silencieusement omise du rapport. Quasi impossible en pratique (ON DELETE SET NULL + soft-delete : COMPARE_CATEGORIES_SQL récupère aussi les inactives), donc seulement sous corruption FK-off + hard-delete. Un filet défensif (traiter ce cas comme un orphelin, à côté du cas categoryId == null) fermerait le trou.
Dénominateur du % — incohérence cosmétique. Les lignes sous-total (is_parent) calculent deltaPct avec un dénominateur signé (pour coller à la formule feuille de rowsToDeltas), alors que les totaux de section et le grand total utilisent Math.abs(previous) via le helper pct(). Divergence uniquement quand le net précédent d'un groupe est négatif — en pratique les groupes de transferts, qui s'annulent à ~0 (donc % quasi nul et souvent null). Pattern préexistant : l'ancien grand total utilisait déjà abs.
Commentaire périmé dans getCartesSnapshot (l.950). « momRows are sorted by absolute delta already » : après #247, momRows est trié par type, plus par delta absolu. Sans impact fonctionnel (le code des top movers re-trie explicitement via .sort((a,b) => b.deltaAbs - a.deltaAbs)), mais le commentaire induit en erreur. Par ailleurs l'affirmation « byte-identical to the previous flat output » est légèrement inexacte : un parent ayant à la fois des enfants ET ses propres transactions directes fait apparaître sa dépense directe en feuille renommée « X (direct) » → un top-mover Cartes pourrait passer du libellé « X » à « X (direct) » (valeur inchangée). Rare (parent avec dépense directe) et purement cosmétique.

Note process (PR stackée). La PR cible issue-243-netting-transferts, pas main → check.yml n'a jamais tourné dessus (les maillons intermédiaires d'une pile affichent no-status). Avant de merger la pile, valider le tip cumulé en local (tsc && vite build, vitest run, cargo check, cargo test). Revue statique : les tests sont cohérents avec l'implémentation, mais leur exécution reste à confirmer au merge.

Revue adversariale automatisée via /pr-review.

## Revue /pr-review — APPROVE **Résumé.** L'implémentation transforme le rapport comparable réel-vs-réel en arbre parent/enfant avec sous-totaux, en miroir fidèle du rapport de budget déjà en production (`BudgetVsActualTable`). Le netting des transferts (#243) est préservé : les valeurs des feuilles sont inchangées et un sous-total = somme arithmétique des feuilles descendantes = net du groupe. Bonne couverture de tests, i18n complet FR/EN, CHANGELOG mis à jour dans les deux fichiers, aucune migration ni schéma touché. **Points vérifiés** - `buildCompareTree` : sous-total = somme des feuilles descendantes (pas de double-comptage multi-niveaux car `descendantLeaves` filtre `is_parent`), garde anti-cycle via `MAX_TREE_DEPTH`, tri final stable par type (sections contiguës). - Invariance du grand total vs feuilles plates (testée). - `ComparePeriodChart` et `getCartesSnapshot` filtrent `is_parent` → pas de double-comptage des sous-totaux contre leurs propres feuilles. - `reorderRows` : contrainte générique assouplie aux 2 seuls champs lus (`is_parent`, `depth`) — compatible budget (champs requis) et compare (optionnels) ; l'algorithme ne lit ni `parent_id` ni `category_id`, l'assouplissement est sûr. - L'hypothèse « un sous-arbre partage un seul `type` » est préexistante et déjà en prod dans `BudgetVsActualTable` (même sectionnement contigu par type) — #247 n'introduit aucun risque neuf. - Clés i18n réutilisées (`reports.subtotalsOnTop/Bottom`, `reports.compare.totalRow`, `reports.bva.*`) présentes ; nouvelles clés ajoutées FR + EN. - Tests cohérents avec l'implémentation (6 cas `buildCompareTree` + test de câblage + assertion call-count passée à 2 requêtes). **Suggestions (non bloquantes)** 1. **`buildCompareTree` — feuille à `parent_id` orphelin.** Une feuille dont le `parent_id` (non-null) pointe vers un id absent du jeu de catégories n'est ni une racine (son parent manquant est marqué « relevant » pendant la remontée d'ancêtres) ni rattachée à un parent réel (aucune entrée d'adjacence) → elle est silencieusement omise du rapport. Quasi impossible en pratique (`ON DELETE SET NULL` + soft-delete : `COMPARE_CATEGORIES_SQL` récupère aussi les inactives), donc seulement sous corruption FK-off + hard-delete. Un filet défensif (traiter ce cas comme un orphelin, à côté du cas `categoryId == null`) fermerait le trou. 2. **Dénominateur du % — incohérence cosmétique.** Les lignes sous-total (`is_parent`) calculent `deltaPct` avec un dénominateur *signé* (pour coller à la formule feuille de `rowsToDeltas`), alors que les totaux de section et le grand total utilisent `Math.abs(previous)` via le helper `pct()`. Divergence uniquement quand le net précédent d'un groupe est négatif — en pratique les groupes de transferts, qui s'annulent à ~0 (donc % quasi nul et souvent `null`). Pattern préexistant : l'ancien grand total utilisait déjà `abs`. 3. **Commentaire périmé dans `getCartesSnapshot` (l.950).** « `momRows` are sorted by absolute delta already » : après #247, `momRows` est trié par type, plus par delta absolu. Sans impact fonctionnel (le code des top movers re-trie explicitement via `.sort((a,b) => b.deltaAbs - a.deltaAbs)`), mais le commentaire induit en erreur. Par ailleurs l'affirmation « byte-identical to the previous flat output » est légèrement inexacte : un parent ayant à la fois des enfants ET ses propres transactions directes fait apparaître sa dépense directe en feuille renommée « X (direct) » → un top-mover Cartes pourrait passer du libellé « X » à « X (direct) » (valeur inchangée). Rare (parent avec dépense directe) et purement cosmétique. **Note process (PR stackée).** La PR cible `issue-243-netting-transferts`, pas `main` → `check.yml` n'a jamais tourné dessus (les maillons intermédiaires d'une pile affichent `no-status`). Avant de merger la pile, valider le tip cumulé en local (`tsc && vite build`, `vitest run`, `cargo check`, `cargo test`). Revue statique : les tests sont cohérents avec l'implémentation, mais leur exécution reste à confirmer au merge. --- *Revue adversariale automatisée via `/pr-review`.*

maximus referenced this pull request from a commit

2026-07-05 21:05:23 +00:00

Merge PR #249: feat(reports) hierarchical real-vs-real compare with subtotals (#247)

maximus commented

2026-07-05 21:06:19 +00:00

Author

Owner

Merged into main locally as part of the milestone stack (merge commit 8bd5121). Closing manually — the merge was done outside Forgejo's API, so it can't flag this PR as merged. The change is on main.

Merged into `main` locally as part of the milestone stack (merge commit `8bd5121`). Closing manually — the merge was done outside Forgejo's API, so it can't flag this PR as *merged*. The change is on `main`.

maximus closed this pull request

2026-07-05 21:06:19 +00:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.

feat(reports): hierarchical real-vs-real compare with subtotals (#247) #249

Revue /pr-review — APPROVE

Pull request closed