feat(reports): réel-vs-réel = analyse de résultat + défaut mois précédent #255

Closed
maximus wants to merge 0 commits from issue-253-analyse-resultat into main
Owner

Resolves #253.

Le rapport « réel vs réel » était un rapport de dépenses (WHERE amount < 0) : les revenus n'apparaissaient pas, impossible de juger surplus vs déficit. Il devient une analyse de résultat (le nom du produit).

Changement 1 — analyse de résultat

REVENUS            (section, SUM signé)
DÉPENSES           (section, ABS des négatifs — inchangé)
── Résultat avant transferts = revenus − dépenses
TRANSFERTS         (section, SUM signé net — #243)
══ Résultat net (total) = avant transferts + transferts nets
  • COMPARE_DELTA_SQL : WHERE + CASE par type élargis à income en SUM signé (comme transfer). Dépenses inchangées (ABS des sorties), netting transferts #243 intact.
  • Ordre des sections : revenus → dépenses → transferts (COMPARE_TYPE_ORDER).
  • ComparePeriodTable : le grand total plat (devenu faux dès qu'on mélange revenus + dépenses ABS) est remplacé par les 2 lignes de résultat, extraites dans un module pur testé compareResults.ts. Couleurs type-aware : hausse de revenu/résultat = vert, de dépense = rouge ; les montants de résultat sont colorés par signe (surplus vert / déficit rouge).
  • ComparePeriodChart reste une vue de dépenses : filtré aux feuilles expense pour ne pas mélanger des barres de revenu.
  • La ligne « avant transferts » n'apparaît que s'il y a des transferts (sinon elle égale le net → bruit).

Changement 2 — défaut mois précédent

useCompare : la synchro initiale sur le to de useReportsPeriod (= 31 déc, année civile) écrasait le défaut mois-précédent de initialState → le compare ouvrait sur décembre (mois futur, vide). La 1ʳᵉ passe de l'effet est désormais sautée ; seules les navigations utilisateur synchronisent ensuite.

Divers

  • .gitignore : la règle reports/ (scratch autopilot) était non ancrée et ignorait silencieusement les nouveaux fichiers sous src/components/reports/. Ancrée à /reports/.

Tests

  • Nouveau compareResults.test.ts : roll-up résultat, transfert déséquilibré à une jambe, déficit, exclusion des sous-totaux.
  • reportService.test.ts : SQL élargi (sous-chaînes), règle bucket revenu, ordre de sections revenus-d'abord.
  • Build tsc+vite + 685 vitest verts. Aucun changement Rust / DB (v1→v16 inchangées).

Suivi

  • #254 (collapse/expand des sous-catégories) — backlog, hors scope.
  • Le point CPA sur le graphique (revenus non montrés) est volontaire pour l'instant.
Resolves #253. Le rapport « réel vs réel » était un rapport de dépenses (`WHERE amount < 0`) : les revenus n'apparaissaient pas, impossible de juger surplus vs déficit. Il devient une **analyse de résultat** (le nom du produit). ## Changement 1 — analyse de résultat ``` REVENUS (section, SUM signé) DÉPENSES (section, ABS des négatifs — inchangé) ── Résultat avant transferts = revenus − dépenses TRANSFERTS (section, SUM signé net — #243) ══ Résultat net (total) = avant transferts + transferts nets ``` - `COMPARE_DELTA_SQL` : `WHERE` + `CASE` par type élargis à `income` en SUM signé (comme `transfer`). Dépenses inchangées (ABS des sorties), netting transferts #243 intact. - Ordre des sections : revenus → dépenses → transferts (`COMPARE_TYPE_ORDER`). - `ComparePeriodTable` : le grand total plat (devenu faux dès qu'on mélange revenus + dépenses ABS) est remplacé par les 2 lignes de résultat, extraites dans un module pur testé `compareResults.ts`. Couleurs **type-aware** : hausse de revenu/résultat = vert, de dépense = rouge ; les montants de résultat sont colorés par signe (surplus vert / déficit rouge). - `ComparePeriodChart` reste une vue de dépenses : filtré aux feuilles `expense` pour ne pas mélanger des barres de revenu. - La ligne « avant transferts » n'apparaît que s'il y a des transferts (sinon elle égale le net → bruit). ## Changement 2 — défaut mois précédent `useCompare` : la synchro initiale sur le `to` de `useReportsPeriod` (= 31 déc, année civile) écrasait le défaut mois-précédent de `initialState` → le compare ouvrait sur décembre (mois futur, vide). La 1ʳᵉ passe de l'effet est désormais sautée ; seules les navigations utilisateur synchronisent ensuite. ## Divers - `.gitignore` : la règle `reports/` (scratch autopilot) était non ancrée et ignorait silencieusement les nouveaux fichiers sous `src/components/reports/`. Ancrée à `/reports/`. ## Tests - Nouveau `compareResults.test.ts` : roll-up résultat, transfert déséquilibré à une jambe, déficit, exclusion des sous-totaux. - `reportService.test.ts` : SQL élargi (sous-chaînes), règle bucket revenu, ordre de sections revenus-d'abord. - Build tsc+vite + **685 vitest** verts. Aucun changement Rust / DB (v1→v16 inchangées). ## Suivi - #254 (collapse/expand des sous-catégories) — backlog, hors scope. - Le point CPA sur le graphique (revenus non montrés) est volontaire pour l'instant.
maximus added 1 commit 2026-07-05 22:37:36 +00:00
feat(reports): real-vs-real compare as an income statement + previous-month default
All checks were successful
PR Check / rust (pull_request) Successful in 23m4s
PR Check / frontend (pull_request) Successful in 2m26s
f45845e408
The "actual vs actual" compare was expenses-only (WHERE amount < 0), so revenue
categories were invisible — you could not tell a surplus from a deficit. Make it
an income statement (the product's namesake "résultat"):

- COMPARE_DELTA_SQL: broaden the WHERE and per-type CASE to include `income` as
  a signed SUM (like `transfer`), so revenue credits survive the outflow filter.
  Expenses stay ABS-of-outflows, byte-identical. Transfer netting (#243) intact.
- Section order is now income -> expense -> transfer (COMPARE_TYPE_ORDER).
- ComparePeriodTable: replace the now-meaningless flat grand total with two
  result lines — "result before transfers" (revenues - expenses) and the net
  "result" (after transfers) — extracted into a pure, tested compareResults
  module. Delta colours are direction-aware (income/result up = green, spending
  up = red); result amounts are coloured by sign (surplus/deficit).
- ComparePeriodChart stays a spending view: filter to expense leaves so revenue
  bars don't mix into the same axis.
- useCompare: skip the initial period-sync so the compare opens on the previous
  (last complete) month instead of the civil-year December that useReportsPeriod
  yields by default.

Also anchor the `reports/` gitignore to `/reports/` — the unanchored rule was
silently ignoring new files under src/components/reports/.

Tests: new compareResults.test.ts (result roll-up, unbalanced transfer, deficit,
subtotal exclusion); reportService.test.ts updated for the broadened SQL, the
income bucket rule, and income-first section order. Build + 685 vitest green.

Resolves #253

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

Revue /pr-review — REQUEST_CHANGES

Résumé : le Changement 1 (analyse de résultat) est solide — SQL correct, dépenses byte-identiques, netting #243 intact, module pur compareResults.ts bien testé, i18n FR/EN complet, et le fix .gitignore (/reports/ ancré) est nécessaire et juste (sans lui les nouveaux fichiers sous src/components/reports/ étaient silencieusement ignorés). Un point bloquant sur le Changement 2 : le garde « sauter la 1ʳᵉ passe » est défait par React.StrictMode en dev et réintroduit le bug qu'il corrige.

Point bloquant

1. src/hooks/useCompare.ts:130-142 — le garde didInitPeriodSync (skip-first) est non idempotent sous StrictMode → le compare rouvre sur décembre en dev

L'app est montée sous <React.StrictMode> (src/main.tsx:13), qui double-invoque les effets au montage en dev (setup → cleanup → setup). Le ref n'est pas réinitialisé entre les deux :

  • setup #1 : didInitPeriodSync.current = false → passe à true → return (sauté).
  • setup #2 : didInitPeriodSync.current = déjà true → ne saute pas → parse to = 2026-12-31dispatch(SET_REFERENCE_PERIOD {2026, 12}).

Résultat : en dev, la référence est écrasée vers décembre (mois futur, vide) — exactement le bug que le Changement 2 vise à corriger. Se manifeste ~11 mois sur 12 (sauf en janvier, où le mois précédent = décembre). La prod (build Tauri, pas de double-invoke StrictMode) est correcte, donc l'utilisateur final n'est pas touché — mais la vérification manuelle en tauri dev le sera, et l'affirmation du corps de PR (« la 1ʳᵉ passe de l'effet est désormais sautée ») est fausse en dev.

À noter : les deux autres gardes-ref du code (duplicateCheckDone dans useCategories.ts, planLoadedRef dans CategoriesMigrationPage.tsx) sont des gardes run-once — StrictMode-safe car le garde empêche la ré-exécution. Le pattern skip-first est l'inverse et casse sous StrictMode.

Fix robuste — garder sur un changement de to plutôt que sur un booléen :

const lastSyncedTo = useRef(to); // valeur initiale = pas de sync au montage
useEffect(() => {
  if (to === lastSyncedTo.current) return; // couvre aussi le re-run StrictMode
  lastSyncedTo.current = to;
  const [y, m] = to.split("-").map(Number);
  if (!Number.isFinite(y) || !Number.isFinite(m)) return;
  if (y !== state.year || m !== state.month) {
    dispatch({ type: "SET_REFERENCE_PERIOD", payload: { year: y, month: m } });
  }
}, [to]);

Sous StrictMode : setup #1 to === lastSyncedTo → return ; setup #2 idem → return. Les navigations utilisateur (changement réel de to) déclenchent la sync ensuite. Le comportement de cet effet n'est couvert par aucun test (useCompare.test.ts ne teste que les helpers purs) — un test RTL avec MemoryRouter verrouillerait le défaut mois-précédent.

Suggestions (non bloquant)

  • Clé i18n orpheline : reports.compare.totalRow (en.json/fr.json:430) n'est plus référencée depuis la suppression de la ligne « grand total ». À retirer.
  • hasTransfers non consommé côté UI : computeResults le calcule et le teste, mais la table décide du rendu via transferSection. Champ mort côté UI (ou brancher la table dessus pour une seule source de vérité).
  • Graphique vide si période sans dépenses : ComparePeriodChart filtre aux feuilles expense ; une période 100 % revenus/transferts afficherait un graphe sans barres (axes seuls) alors que la table, elle, a des données. Cas rare, purement cosmétique.

Revue adversariale automatisée. La logique métier (SQL income-statement, roll-up des résultats, sections) est correcte et bien couverte ; le seul blocage est la robustesse StrictMode de l'effet de sync.

## Revue /pr-review — REQUEST_CHANGES **Résumé** : le Changement 1 (analyse de résultat) est solide — SQL correct, dépenses byte-identiques, netting #243 intact, module pur `compareResults.ts` bien testé, i18n FR/EN complet, et le fix `.gitignore` (`/reports/` ancré) est nécessaire et juste (sans lui les nouveaux fichiers sous `src/components/reports/` étaient silencieusement ignorés). Un point bloquant sur le Changement 2 : le garde « sauter la 1ʳᵉ passe » est défait par `React.StrictMode` en dev et réintroduit le bug qu'il corrige. ### Point bloquant **1. `src/hooks/useCompare.ts:130-142` — le garde `didInitPeriodSync` (skip-first) est non idempotent sous StrictMode → le compare rouvre sur décembre en dev** L'app est montée sous `<React.StrictMode>` (`src/main.tsx:13`), qui double-invoque les effets au montage en dev (setup → cleanup → setup). Le ref n'est pas réinitialisé entre les deux : - setup #1 : `didInitPeriodSync.current` = false → passe à true → `return` (sauté). - setup #2 : `didInitPeriodSync.current` = déjà true → **ne saute pas** → parse `to` = `2026-12-31` → `dispatch(SET_REFERENCE_PERIOD {2026, 12})`. Résultat : en dev, la référence est écrasée vers décembre (mois futur, vide) — exactement le bug que le Changement 2 vise à corriger. Se manifeste ~11 mois sur 12 (sauf en janvier, où le mois précédent = décembre). La prod (build Tauri, pas de double-invoke StrictMode) est correcte, donc l'utilisateur final n'est pas touché — mais la vérification manuelle en `tauri dev` le sera, et l'affirmation du corps de PR (« la 1ʳᵉ passe de l'effet est désormais sautée ») est fausse en dev. À noter : les deux autres gardes-ref du code (`duplicateCheckDone` dans `useCategories.ts`, `planLoadedRef` dans `CategoriesMigrationPage.tsx`) sont des gardes **run-once** — StrictMode-safe car le garde empêche la ré-exécution. Le pattern **skip-first** est l'inverse et casse sous StrictMode. Fix robuste — garder sur un *changement* de `to` plutôt que sur un booléen : ```ts const lastSyncedTo = useRef(to); // valeur initiale = pas de sync au montage useEffect(() => { if (to === lastSyncedTo.current) return; // couvre aussi le re-run StrictMode lastSyncedTo.current = to; const [y, m] = to.split("-").map(Number); if (!Number.isFinite(y) || !Number.isFinite(m)) return; if (y !== state.year || m !== state.month) { dispatch({ type: "SET_REFERENCE_PERIOD", payload: { year: y, month: m } }); } }, [to]); ``` Sous StrictMode : setup #1 `to === lastSyncedTo` → return ; setup #2 idem → return. Les navigations utilisateur (changement réel de `to`) déclenchent la sync ensuite. Le comportement de cet effet n'est couvert par aucun test (`useCompare.test.ts` ne teste que les helpers purs) — un test RTL avec `MemoryRouter` verrouillerait le défaut mois-précédent. ### Suggestions (non bloquant) - **Clé i18n orpheline** : `reports.compare.totalRow` (`en.json`/`fr.json:430`) n'est plus référencée depuis la suppression de la ligne « grand total ». À retirer. - **`hasTransfers` non consommé côté UI** : `computeResults` le calcule et le teste, mais la table décide du rendu via `transferSection`. Champ mort côté UI (ou brancher la table dessus pour une seule source de vérité). - **Graphique vide si période sans dépenses** : `ComparePeriodChart` filtre aux feuilles `expense` ; une période 100 % revenus/transferts afficherait un graphe sans barres (axes seuls) alors que la table, elle, a des données. Cas rare, purement cosmétique. --- *Revue adversariale automatisée. La logique métier (SQL income-statement, roll-up des résultats, sections) est correcte et bien couverte ; le seul blocage est la robustesse StrictMode de l'effet de sync.*
maximus added 1 commit 2026-07-05 23:37:25 +00:00
fix(reports): StrictMode-safe period sync + review polish (#253)
All checks were successful
PR Check / rust (pull_request) Successful in 22m20s
PR Check / frontend (pull_request) Successful in 2m35s
d57b2563af
Addresses /pr-review REQUEST_CHANGES on #255.

- useCompare: the "skip first sync" boolean was not StrictMode-safe — the dev
  double-invoke of effects flipped the flag on setup #1, so setup #2 re-synced
  the reference month to the civil-year December, re-introducing the very bug
  Changement 2 fixes (dev only; prod has no double-invoke). Replace it with a
  value-change guard: a ref seeded with the initial `to` plus a pure
  syncReferenceOnPeriodChange() that only dispatches when `to` actually changes.
  Idempotent across the double-invoke, and now unit-tested (5 cases) since the
  decision is a pure function (the project has no renderHook harness).
- Remove the now-orphaned reports.compare.totalRow i18n key (both locales) — the
  flat grand total it labelled was replaced by the result lines.
- ComparePeriodTable: gate the "before transfers" line on results.hasTransfers
  (previously computed/tested but unused).
- ComparePeriodChart: show the no-data empty state when the expense filter
  leaves nothing (a pure income/transfer period) instead of bare axes.

Build + 690 vitest green.

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

Point bloquant corrigé en d57b256.

StrictMode : le garde booléen « skip-first » n'était pas idempotent sous le double-invoke des effets en dev — le flag flippait au setup #1, donc le setup #2 re-synchronisait la référence sur décembre (le bug même du Changement 2). Remplacé par un garde sur changement de valeur : un ref initialisé à la valeur de montage + une fonction pure syncReferenceOnPeriodChange() qui ne dispatch que si to change réellement. Idempotent sous le double-invoke, et testé (5 cas) — la décision étant pure, pas besoin de harness de rendu.

Les 3 mineurs aussi :

  • clé i18n orpheline reports.compare.totalRow retirée (2 locales) ;
  • ligne « avant transferts » gardée sur results.hasTransfers (auparavant calculé/testé mais non consommé) ;
  • graphique : empty-state quand le filtre dépenses ne laisse rien (période 100 % revenus/transferts) au lieu d'axes nus.

Build tsc+vite + 690 vitest verts.

Point bloquant corrigé en `d57b256`. **StrictMode** : le garde booléen « skip-first » n'était pas idempotent sous le double-invoke des effets en dev — le flag flippait au setup #1, donc le setup #2 re-synchronisait la référence sur décembre (le bug même du Changement 2). Remplacé par un garde sur *changement de valeur* : un ref initialisé à la valeur de montage + une fonction pure `syncReferenceOnPeriodChange()` qui ne dispatch que si `to` change réellement. Idempotent sous le double-invoke, et testé (5 cas) — la décision étant pure, pas besoin de harness de rendu. Les 3 mineurs aussi : - clé i18n orpheline `reports.compare.totalRow` retirée (2 locales) ; - ligne « avant transferts » gardée sur `results.hasTransfers` (auparavant calculé/testé mais non consommé) ; - graphique : empty-state quand le filtre dépenses ne laisse rien (période 100 % revenus/transferts) au lieu d'axes nus. Build tsc+vite + **690 vitest** verts.
Author
Owner

Revue — PR #255 · feat(reports): réel-vs-réel = analyse de résultat + défaut mois précédent

Verdict : REQUEST_CHANGES

Résumé

La refonte du rapport Comparaison en analyse de résultat est propre, bien découpée (module pur compareResults.ts) et bien testée. Mais élargir COMPARE_DELTA_SQL pour faire remonter les revenus a un effet de bord non suivi : getCartesSnapshot partage cette requête, donc les catégories de revenu s'infiltrent désormais dans la carte « top mouvements » du tableau de bord Cartes — où le code couleur pensé pour des dépenses les représente à contresens.

Problème bloquant

1. src/services/reportService.ts:1177 — les revenus polluent la carte « top mouvements » des Cartes

getCartesSnapshot (ligne 1019) appelle getCompareMonthOverMonth(...) pour bâtir momRows, puis en dérive topMoversUp / topMoversDown (lignes 1177-1201) sans filtre de type :

const significantMovers = momRows.filter(
  (r) => !r.is_parent && r.deltaAbs !== 0 && (r.previousAmount > 0 || r.currentAmount > 0),
);

Avant cette PR, le WHERE (t.amount < 0 OR type = 'transfer') excluait entièrement les catégories de revenu de COMPARE_DELTA_SQL (transactions positives, type income) : la carte était de fait dépenses-seulement (les transferts nettent à ~0 → filtrés par deltaAbs !== 0). Après la PR, les revenus reviennent en SUM signé positif, passent le filtre significantMovers, et atterrissent dans les listes.

Or TopMoversList (src/components/reports/cards/TopMoversList.tsx:44) code la couleur pour des dépenses :

const accentClass = direction === "up" ? "text-[var(--negative)]" : "text-[var(--positive)]";

Conséquence : une hausse de revenu (ex. « Salaire +2 000 $ +25 % ») s'affiche en rouge sous « Biggest increases / Catégories en hausse » comme une mauvaise nouvelle, et une baisse de revenu passe en vert dans « en baisse ». Le sens financier est inversé pour toute ligne de revenu, sur un écran hors du périmètre annoncé de la PR.

Ni testé, ni mentionné dans la PR : le test reportService.cartes.test.ts « splits top movers by sign » mocke les réponses SQL avec des momRows sans type de catégorie (la requête COMPARE_CATEGORIES_SQL n'est pas routée → tout devient expense par défaut), donc il reste vert et ne détecte pas la fuite. La régression est silencieuse.

Correctif : filtrer les mouvements aux feuilles expense, en miroir de ce que fait déjà ComparePeriodChart dans cette même PR ((r.category_type ?? "expense") === "expense") :

const significantMovers = momRows.filter(
  (r) => !r.is_parent && (r.category_type ?? "expense") === "expense" &&
         r.deltaAbs !== 0 && (r.previousAmount > 0 || r.currentAmount > 0),
);

Ajouter un cas de test cartes qui route COMPARE_CATEGORIES_SQL avec une catégorie income et vérifie qu'elle n'apparaît pas dans topMoversUp/Down.

Points positifs

  • compareResults.ts extrait en module pur, arithmétique de roll-up cohérente (deltaAbs = current − previouscombine interne consistant), bien couvert (transfert déséquilibré, déficit, exclusion des sous-totaux, YTD).
  • Fix du défaut mois-précédent via syncReferenceOnPeriodChange : garde sur changement de to plutôt que flag fire-once → idempotent sous StrictMode, avec tests explicites (mount no-op, double-invoke, navigation, to illisible).
  • i18n symétrique (clés resultBeforeTransfers / resultNet ajoutées FR+EN, totalRow retirée des deux et plus référencée), CHANGELOG dans les deux langues.
  • .gitignore : ancrage reports//reports/ correct et nécessaire (le motif non ancré aurait silencieusement ignoré compareResults.ts sous src/components/reports/ ; le fichier est bien tracké).
  • Suite locale verte confirmée (139 tests sur les fichiers touchés).

Suggestions non bloquantes

  • renderResultRow / pct : quand un résultat traverse zéro (déficit → surplus), pct(delta, previous) divise par Math.abs(previous) et peut afficher un pourcentage sémantiquement bizarre (ex. −500 → +300 = +160 %). Pré-existant sur les sous-totaux, non bloquant — envisager un tiret quand previous et current sont de signes opposés.
  • Période pure-dépenses (revenus non encore catégorisés) : le « Résultat net » s'affiche en gros rouge = −(total dépenses). Correct pour une analyse de résultat, mais peut surprendre un utilisateur en cours de migration ; à surveiller côté guide utilisateur.
## Revue — PR #255 · `feat(reports): réel-vs-réel = analyse de résultat + défaut mois précédent` **Verdict : REQUEST_CHANGES** ### Résumé La refonte du rapport Comparaison en analyse de résultat est propre, bien découpée (module pur `compareResults.ts`) et bien testée. Mais élargir `COMPARE_DELTA_SQL` pour faire remonter les revenus a un effet de bord non suivi : `getCartesSnapshot` partage cette requête, donc les catégories de revenu s'infiltrent désormais dans la carte « top mouvements » du tableau de bord Cartes — où le code couleur pensé pour des dépenses les représente à contresens. ### Problème bloquant **1. `src/services/reportService.ts:1177` — les revenus polluent la carte « top mouvements » des Cartes** `getCartesSnapshot` (ligne 1019) appelle `getCompareMonthOverMonth(...)` pour bâtir `momRows`, puis en dérive `topMoversUp` / `topMoversDown` (lignes 1177-1201) **sans filtre de type** : ```ts const significantMovers = momRows.filter( (r) => !r.is_parent && r.deltaAbs !== 0 && (r.previousAmount > 0 || r.currentAmount > 0), ); ``` Avant cette PR, le `WHERE (t.amount < 0 OR type = 'transfer')` excluait entièrement les catégories de revenu de `COMPARE_DELTA_SQL` (transactions positives, type `income`) : la carte était de fait dépenses-seulement (les transferts nettent à ~0 → filtrés par `deltaAbs !== 0`). Après la PR, les revenus reviennent en SUM signé positif, passent le filtre `significantMovers`, et atterrissent dans les listes. Or `TopMoversList` (`src/components/reports/cards/TopMoversList.tsx:44`) code la couleur pour des dépenses : ```ts const accentClass = direction === "up" ? "text-[var(--negative)]" : "text-[var(--positive)]"; ``` Conséquence : une hausse de revenu (ex. « Salaire +2 000 $ +25 % ») s'affiche **en rouge** sous « Biggest increases / Catégories en hausse » comme une mauvaise nouvelle, et une baisse de revenu passe **en vert** dans « en baisse ». Le sens financier est inversé pour toute ligne de revenu, sur un écran hors du périmètre annoncé de la PR. Ni testé, ni mentionné dans la PR : le test `reportService.cartes.test.ts` « splits top movers by sign » mocke les réponses SQL avec des `momRows` sans type de catégorie (la requête `COMPARE_CATEGORIES_SQL` n'est pas routée → tout devient `expense` par défaut), donc il reste vert et ne détecte pas la fuite. La régression est silencieuse. **Correctif** : filtrer les mouvements aux feuilles `expense`, en miroir de ce que fait déjà `ComparePeriodChart` dans cette même PR (`(r.category_type ?? "expense") === "expense"`) : ```ts const significantMovers = momRows.filter( (r) => !r.is_parent && (r.category_type ?? "expense") === "expense" && r.deltaAbs !== 0 && (r.previousAmount > 0 || r.currentAmount > 0), ); ``` Ajouter un cas de test cartes qui route `COMPARE_CATEGORIES_SQL` avec une catégorie `income` et vérifie qu'elle n'apparaît pas dans `topMoversUp/Down`. ### Points positifs - `compareResults.ts` extrait en module pur, arithmétique de roll-up cohérente (`deltaAbs = current − previous` → `combine` interne consistant), bien couvert (transfert déséquilibré, déficit, exclusion des sous-totaux, YTD). - Fix du défaut mois-précédent via `syncReferenceOnPeriodChange` : garde sur *changement* de `to` plutôt que flag fire-once → idempotent sous StrictMode, avec tests explicites (mount no-op, double-invoke, navigation, `to` illisible). - i18n symétrique (clés `resultBeforeTransfers` / `resultNet` ajoutées FR+EN, `totalRow` retirée des deux et plus référencée), CHANGELOG dans les deux langues. - `.gitignore` : ancrage `reports/` → `/reports/` correct et nécessaire (le motif non ancré aurait silencieusement ignoré `compareResults.ts` sous `src/components/reports/` ; le fichier est bien tracké). - Suite locale verte confirmée (139 tests sur les fichiers touchés). ### Suggestions non bloquantes - `renderResultRow` / `pct` : quand un résultat traverse zéro (déficit → surplus), `pct(delta, previous)` divise par `Math.abs(previous)` et peut afficher un pourcentage sémantiquement bizarre (ex. −500 → +300 = +160 %). Pré-existant sur les sous-totaux, non bloquant — envisager un tiret `—` quand `previous` et `current` sont de signes opposés. - Période pure-dépenses (revenus non encore catégorisés) : le « Résultat net » s'affiche en gros rouge = −(total dépenses). Correct pour une analyse de résultat, mais peut surprendre un utilisateur en cours de migration ; à surveiller côté guide utilisateur.
maximus added 1 commit 2026-07-06 00:14:27 +00:00
fix(reports): exclude income from Cartes top movers (#253 regression)
All checks were successful
PR Check / rust (pull_request) Successful in 22m23s
PR Check / frontend (pull_request) Successful in 2m25s
c7914c2ca5
/pr-review on #255 caught a cross-consumer regression: broadening
COMPARE_DELTA_SQL to surface income (this PR) also feeds getCartesSnapshot,
whose "top movers" card is a spending view (up = red). A salary rise would land
under "biggest increases" coloured red — inverted meaning.

Filter significantMovers to expense leaves ((category_type ?? "expense") ===
"expense"), mirroring ComparePeriodChart. The surviving expense output is
byte-identical to pre-#253. Add a Cartes regression test (an income category
with the biggest delta must not top the movers list) — the existing test mocked
the compare SQL without category types, so it stayed silently green.

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

Régression Cartes corrigée en c7914c2.

En élargissant COMPARE_DELTA_SQL pour faire remonter les revenus, getCartesSnapshot (carte « top mouvements ») les recevait aussi — et cette carte est une vue de dépenses (up = rouge), donc un salaire en hausse serait passé en rouge sous « biggest increases ». Filtre significantMovers restreint aux feuilles expense ((r.category_type ?? "expense") === "expense"), en miroir de ComparePeriodChart. Sortie dépenses byte-identique à avant #253.

Test de régression ajouté (reportService.cartes.test.ts) : une catégorie income avec le plus gros delta ne doit PAS coiffer le top-movers — l'ancien test mockait le SQL sans type, d'où le trou silencieux.

Vérif : seuls 2 consommateurs des fonctions compare (useCompare — table montre les revenus, chart filtre ; getCartesSnapshot — maintenant filtré). Build + tests verts.

Régression Cartes corrigée en `c7914c2`. En élargissant `COMPARE_DELTA_SQL` pour faire remonter les revenus, `getCartesSnapshot` (carte « top mouvements ») les recevait aussi — et cette carte est une vue de dépenses (up = rouge), donc un salaire en hausse serait passé en rouge sous « biggest increases ». Filtre `significantMovers` restreint aux feuilles `expense` (`(r.category_type ?? "expense") === "expense"`), en miroir de `ComparePeriodChart`. Sortie dépenses byte-identique à avant #253. Test de régression ajouté (`reportService.cartes.test.ts`) : une catégorie `income` avec le plus gros delta ne doit PAS coiffer le top-movers — l'ancien test mockait le SQL sans type, d'où le trou silencieux. Vérif : seuls 2 consommateurs des fonctions compare (`useCompare` — table montre les revenus, chart filtre ; `getCartesSnapshot` — maintenant filtré). Build + tests verts.
Author
Owner

Revue — PR #255 : réel-vs-réel = analyse de résultat

Verdict : APPROVE

Refonte propre et bien testée. Le rapport « réel vs réel » passe d'un rapport de dépenses à une véritable analyse de résultat (revenus + dépenses + deux lignes de résultat), et le défaut d'ouverture bascule sur le mois précédent. Aucun changement Rust/DB (v1→v16 inchangées). tsc vert, 691 vitest verts en local, aucune régression détectée chez les autres consommateurs du service compare.

Vérifications

  • SQL : COMPARE_DELTA_SQL élargit le CASE/WHERE à IN ('transfer', 'income') — littéral, paramétrage $1..$8 intact, aucune injection. Dépenses inchangées (ABS des sorties), netting transferts #243 préservé.
  • Pas de fuite de revenus ailleurs : COMPARE_DELTA_SQL n'est consommé que par getCompareMonthOverMonth/YoY. Les deux vues qui reçoivent ces lignes filtrent maintenant category_type === 'expense' — Cartes top movers (significantMovers, régression corrigée + test dédié) et ComparePeriodChart. getHighlights a sa propre requête, non touchée.
  • Roll-up résultat : computeResults repose sur deltaAbs = current − previous (cohérent leaf / subtotal / factory de test) ; combine composante-à-composante garde monthDelta = monthCurrent − monthPrevious. sumLeaves exclut is_parent → pas de double comptage. Le pct à dénominateur abs(previous) est en fait le bon choix pour les lignes de résultat (un résultat précédent négatif ne fausse pas le signe du %).
  • Sync mois : syncReferenceOnPeriodChange gate sur un changement de to (ref seedé au to initial) → no-op au montage, idempotent sous StrictMode. to est dérivé synchroniquement des query params (useReportsPeriod, useMemo), donc pas de changement post-montage parasite : le défaut mois-précédent survit.
  • i18n : resultBeforeTransfers / resultNet ajoutés FR+EN ; totalRow retiré des deux (plus aucune référence — vérifié). Aucune chaîne en dur.
  • .gitignore : reports//reports/ — correction réelle et nécessaire : la règle non ancrée aurait ignoré silencieusement les nouveaux fichiers sous src/components/reports/.
  • CHANGELOG : entrée sous [Unreleased] / [Non publié] dans les deux fichiers, #253 référencé. Commits conventionnels, Resolves #253 présent.

Observations (non bloquantes)

  1. Effet du no-op au montage : une URL partagée/bookmarkée portant une période explicite non-défaut n'amorce plus le mois de référence du compare au premier rendu (il ouvre toujours sur le mois précédent). Cohérent avec l'intention de #253 ; le dropdown de référence et la navigation via PeriodSelector resynchronisent ensuite.
  2. Lignes de résultat : chaque cellule montant (courant et précédent) est colorée par signe surplus/déficit — un peu chargé visuellement, mais c'est le design documenté.
  3. Suivi déjà noté : les nets de transfert signés traversent une UI compare pensée pour des magnitudes positives ; ne mord que les transferts déséquilibrés à une jambe. Tracké.

Rien de bloquant.

## Revue — PR #255 : réel-vs-réel = analyse de résultat **Verdict : APPROVE** Refonte propre et bien testée. Le rapport « réel vs réel » passe d'un rapport de dépenses à une véritable analyse de résultat (revenus + dépenses + deux lignes de résultat), et le défaut d'ouverture bascule sur le mois précédent. Aucun changement Rust/DB (v1→v16 inchangées). `tsc` vert, **691 vitest verts** en local, aucune régression détectée chez les autres consommateurs du service compare. ### Vérifications - **SQL** : `COMPARE_DELTA_SQL` élargit le `CASE`/`WHERE` à `IN ('transfer', 'income')` — littéral, paramétrage `$1..$8` intact, aucune injection. Dépenses inchangées (ABS des sorties), netting transferts #243 préservé. - **Pas de fuite de revenus ailleurs** : `COMPARE_DELTA_SQL` n'est consommé que par `getCompareMonthOverMonth/YoY`. Les deux vues qui reçoivent ces lignes filtrent maintenant `category_type === 'expense'` — Cartes top movers (`significantMovers`, régression corrigée + test dédié) et `ComparePeriodChart`. `getHighlights` a sa propre requête, non touchée. - **Roll-up résultat** : `computeResults` repose sur `deltaAbs = current − previous` (cohérent leaf / subtotal / factory de test) ; `combine` composante-à-composante garde `monthDelta = monthCurrent − monthPrevious`. `sumLeaves` exclut `is_parent` → pas de double comptage. Le `pct` à dénominateur `abs(previous)` est en fait le bon choix pour les lignes de résultat (un résultat précédent négatif ne fausse pas le signe du %). - **Sync mois** : `syncReferenceOnPeriodChange` gate sur un *changement* de `to` (ref seedé au `to` initial) → no-op au montage, idempotent sous StrictMode. `to` est dérivé synchroniquement des query params (`useReportsPeriod`, `useMemo`), donc pas de changement post-montage parasite : le défaut mois-précédent survit. - **i18n** : `resultBeforeTransfers` / `resultNet` ajoutés FR+EN ; `totalRow` retiré des deux (plus aucune référence — vérifié). Aucune chaîne en dur. - **.gitignore** : `reports/` → `/reports/` — correction réelle et nécessaire : la règle non ancrée aurait ignoré silencieusement les nouveaux fichiers sous `src/components/reports/`. - **CHANGELOG** : entrée sous [Unreleased] / [Non publié] dans les deux fichiers, #253 référencé. Commits conventionnels, `Resolves #253` présent. ### Observations (non bloquantes) 1. **Effet du no-op au montage** : une URL partagée/bookmarkée portant une période explicite non-défaut n'amorce plus le mois de référence du compare au premier rendu (il ouvre toujours sur le mois précédent). Cohérent avec l'intention de #253 ; le dropdown de référence et la navigation via PeriodSelector resynchronisent ensuite. 2. **Lignes de résultat** : chaque cellule montant (courant *et* précédent) est colorée par signe surplus/déficit — un peu chargé visuellement, mais c'est le design documenté. 3. **Suivi déjà noté** : les nets de transfert signés traversent une UI compare pensée pour des magnitudes positives ; ne mord que les transferts déséquilibrés à une jambe. Tracké. Rien de bloquant.
Author
Owner

Mergée localement dans main (merge commit 1360940). Fermée manuellement — un merge local ne marque pas la PR « merged » côté Forgejo.

Mergée localement dans `main` (merge commit 1360940). Fermée manuellement — un merge local ne marque pas la PR « merged » côté Forgejo.
maximus closed this pull request 2026-07-06 00:55:57 +00:00
All checks were successful
PR Check / rust (pull_request) Successful in 22m23s
PR Check / frontend (pull_request) Successful in 2m25s

Pull request closed

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#255
No description provided.