feat(reports): BVA (actual vs budget) income-first + result lines #286

Closed

maximus wants to merge 0 commits from issue-277-bva-income-first into main

maximus commented

2026-07-11 20:59:39 +00:00

Owner

Aligns BudgetVsActualTable (actual vs budget) on the income-statement gold standard already shipped for the real-vs-real compare report (#253/#256).

Changes

src/services/budgetService.ts: TYPE_ORDER flipped from expense-first to income-first (income → expense → transfer), matching COMPARE_TYPE_ORDER/OVER_TIME_TYPE_ORDER.
New pure, tested module src/components/reports/budgetVsActualResults.ts (mirrors compareResults.ts): sumLeaves, pct, computeResults producing resultBefore (income + expense) and resultNet (+ transfers).
src/components/reports/BudgetVsActualTable.tsx: extracted the inline section-totals reduce into renderSection (now backed by the pure module), added renderResultRow, and replaced the old flat "Total" row with interleaved Result before transfers (shown only when transfers exist) and Net result lines — same structural placement as ComparePeriodTable.
i18n: no new strings — reused the existing reports.compare.resultBeforeTransfers / reports.compare.resultNet keys (already FR+EN) per the issue's guidance.
CHANGELOG (CHANGELOG.md + CHANGELOG.fr.md) under [Unreleased] / ### Changed.

Sign-convention note (why `computeResults` differs from `compareResults`)

Real-vs-real compare ABS's expense magnitudes (positive), so its operating result is income − expense. BVA's getBudgetVsActualData does not ABS actual expenses — monthActual/ytdActual are the raw SUM(transactions.amount) (negative for expense categories, per this app's amount < 0 = expense convention used throughout reportService.ts/dashboardService.ts), and the budget figure is negated to match via the existing signFor. So expense totals here are already ≤ 0, and resultBefore is a straight add of income + expense (not a subtraction) — verified against the old flat grand-total, which (because of this same sign convention) was already numerically equal to what is now resultNet; this PR only changes its position/label/section order, not its value. Leaf and section amounts are byte-for-byte unchanged.

Testing

npm test baseline on fresh branch: 791 passed. After: 797 passed (48 files) — 6 new tests in budgetVsActualResults.test.ts, zero regressions.
npm run build (tsc + vite): green.

Resolves #277

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

Aligns `BudgetVsActualTable` (actual vs budget) on the income-statement gold standard already shipped for the real-vs-real compare report (#253/#256). ## Changes - `src/services/budgetService.ts`: `TYPE_ORDER` flipped from expense-first to **income-first** (income → expense → transfer), matching `COMPARE_TYPE_ORDER`/`OVER_TIME_TYPE_ORDER`. - New pure, tested module `src/components/reports/budgetVsActualResults.ts` (mirrors `compareResults.ts`): `sumLeaves`, `pct`, `computeResults` producing `resultBefore` (income + expense) and `resultNet` (+ transfers). - `src/components/reports/BudgetVsActualTable.tsx`: extracted the inline section-totals reduce into `renderSection` (now backed by the pure module), added `renderResultRow`, and replaced the old flat "Total" row with interleaved **Result before transfers** (shown only when transfers exist) and **Net result** lines — same structural placement as `ComparePeriodTable`. - i18n: no new strings — reused the existing `reports.compare.resultBeforeTransfers` / `reports.compare.resultNet` keys (already FR+EN) per the issue's guidance. - CHANGELOG (`CHANGELOG.md` + `CHANGELOG.fr.md`) under `[Unreleased]` / `### Changed`. ## Sign-convention note (why `computeResults` differs from `compareResults`) Real-vs-real compare ABS's expense magnitudes (positive), so its operating result is `income − expense`. BVA's `getBudgetVsActualData` does **not** ABS actual expenses — `monthActual`/`ytdActual` are the raw `SUM(transactions.amount)` (negative for expense categories, per this app's `amount < 0` = expense convention used throughout `reportService.ts`/`dashboardService.ts`), and the budget figure is negated to match via the existing `signFor`. So expense totals here are already ≤ 0, and `resultBefore` is a straight **add** of income + expense (not a subtraction) — verified against the old flat grand-total, which (because of this same sign convention) was already numerically equal to what is now `resultNet`; this PR only changes its position/label/section order, not its value. Leaf and section amounts are byte-for-byte unchanged. ## Testing - `npm test` baseline on fresh branch: 791 passed. After: **797 passed** (48 files) — 6 new tests in `budgetVsActualResults.test.ts`, zero regressions. - `npm run build` (tsc + vite): green. Resolves #277 Generated autonomously by /autopilot run of 2026-07-11

maximus added 1 commit 2026-07-11 20:59:40 +00:00

feat(reports): BVA (actual vs budget) income-first + result lines

PR Check / rust (pull_request) Successful in 20m38s

Details

PR Check / frontend (pull_request) Successful in 2m20s

Details

14755eb155

Align BudgetVsActualTable on the income-statement gold standard already
shipped for the real-vs-real compare report (#253/#256): flip TYPE_ORDER
to income-first (revenue -> expense -> transfer) and replace the flat
"Total" row with interleaved Result before transfers / Net result lines.

Roll-up logic extracted into a new pure, tested module
(budgetVsActualResults.ts) mirroring compareResults.ts, with one sign
difference: BVA amounts already arrive signed the accounting way
(expense actual/budget are negative), so the operating result is a
straight add of income + expense rather than a subtraction.

Resolves #277

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

maximus added the

autopilot:pending-human

label 2026-07-11 20:59:54 +00:00

maximus commented

2026-07-12 00:38:15 +00:00

Author

Owner

/pr-review — PR #286 · `feat(reports): BVA income-first + result lines`

Verdict : APPROVE

Résumé

Aligne BudgetVsActualTable (réel vs budget) sur le standard « analyse de résultat » déjà livré pour le compare réel-vs-réel (#253/#256). Extraction propre d'un module pur testé, TYPE_ORDER passé income-first, lignes de résultat interleavées. Logique correcte, régression nulle vérifiée, CI verte.

Vérifications

Équivalence numérique : l'ancien « Total » plat = nouveau resultNet byte-for-byte. Le total plat sommait toutes les feuilles (income + expense + transfer, dépense déjà signée négative via signFor) ; resultNet = add(add(income, expense), transfer) produit exactement la même valeur. Seuls la position, le libellé et le tri de section changent — confirmé.
Convention de signe : contrairement à compareResults (dépenses ABS'd positives → income − expense), BVA garde le SUM brut signé (dépense ≤ 0) → resultBefore est un ajout income + expense. Le doc-comment et le test l'expliquent correctement.
renderSection / totaux de section : sumLeaves(section.rows) et pct(variation, budget) reproduisent à l'identique le reduce inline et le calcul de pct de l'ancien code (pas de ×100, appliqué par pctFormatter). Aucun changement de comportement sur les feuilles/sections.
Cohérence hasTransfers ↔ transferSection : getBudgetVsActualData ne pousse une section transfert que si elle a ≥1 feuille non-nulle → les deux gardes sont toujours cohérentes (ligne « Résultat avant transferts » affichée ssi une section transfert suit).
Type-safety : BudgetVsActualRow.category_type est l'union "expense" | "income" | "transfer" (non-optionnelle), donc pas besoin du fallback ?? "expense" de compareResults — correct.
Mirroring : renderResultRow / structure nonTransferSections → resultBefore → transferSection → resultNet calquée fidèlement sur ComparePeriodTable.
i18n : reports.compare.resultBeforeTransfers / resultNet présentes FR+EN (aucune string en dur). CHANGELOG sous [Unreleased] / Changed dans les deux fichiers.
Tests : 6 tests neufs dans budgetVsActualResults.test.ts (roll-up, signe dépense, transfert non nul, déficit, exclusion is_parent, YTD) — assertions vérifiées à la main, toutes exactes. Aucun .skip/.only. Suite 797 verte + CI success.
Sécurité / migrations : aucun secret, aucune requête, aucune migration touchée.

Suggestions (non bloquantes)

Deux helpers pct homonymes coexistent désormais (compareResults.pct avec ×100, budgetVsActualResults.pct sans). Chacun est correct pour son formateur respectif (formatPct divise / pctFormatter multiplie), mais l'homonymie à signature identique est un piège au copier-coller. Le doc-comment de budgetVsActualResults.pct mitige déjà — rien à corriger.
Coloration des colonnes « budget/planifié » des lignes de résultat par signe : cohérent avec ComparePeriodTable, mais colorer un montant budgété en vert/rouge peut surprendre. Cosmétique, aligné sur le sibling — OK de garder.

Beau travail, fusionnable en l'état.

## /pr-review — PR #286 · `feat(reports): BVA income-first + result lines` **Verdict : APPROVE** ### Résumé Aligne `BudgetVsActualTable` (réel vs budget) sur le standard « analyse de résultat » déjà livré pour le compare réel-vs-réel (#253/#256). Extraction propre d'un module pur testé, `TYPE_ORDER` passé income-first, lignes de résultat interleavées. Logique correcte, régression nulle vérifiée, CI verte. ### Vérifications - **Équivalence numérique** : l'ancien « Total » plat = nouveau `resultNet` byte-for-byte. Le total plat sommait toutes les feuilles (`income + expense + transfer`, dépense déjà signée négative via `signFor`) ; `resultNet = add(add(income, expense), transfer)` produit exactement la même valeur. Seuls la position, le libellé et le tri de section changent — confirmé. - **Convention de signe** : contrairement à `compareResults` (dépenses ABS'd positives → `income − expense`), BVA garde le SUM brut signé (dépense ≤ 0) → `resultBefore` est un **ajout** `income + expense`. Le doc-comment et le test l'expliquent correctement. - **`renderSection` / totaux de section** : `sumLeaves(section.rows)` et `pct(variation, budget)` reproduisent à l'identique le `reduce` inline et le calcul de pct de l'ancien code (pas de `×100`, appliqué par `pctFormatter`). Aucun changement de comportement sur les feuilles/sections. - **Cohérence `hasTransfers` ↔ `transferSection`** : `getBudgetVsActualData` ne pousse une section transfert que si elle a ≥1 feuille non-nulle → les deux gardes sont toujours cohérentes (ligne « Résultat avant transferts » affichée ssi une section transfert suit). - **Type-safety** : `BudgetVsActualRow.category_type` est l'union `"expense" | "income" | "transfer"` (non-optionnelle), donc pas besoin du fallback `?? "expense"` de `compareResults` — correct. - **Mirroring** : `renderResultRow` / structure `nonTransferSections → resultBefore → transferSection → resultNet` calquée fidèlement sur `ComparePeriodTable`. - **i18n** : `reports.compare.resultBeforeTransfers` / `resultNet` présentes FR+EN (aucune string en dur). CHANGELOG sous `[Unreleased] / Changed` dans les deux fichiers. - **Tests** : 6 tests neufs dans `budgetVsActualResults.test.ts` (roll-up, signe dépense, transfert non nul, déficit, exclusion `is_parent`, YTD) — assertions vérifiées à la main, toutes exactes. Aucun `.skip`/`.only`. Suite 797 verte + CI `success`. - **Sécurité / migrations** : aucun secret, aucune requête, aucune migration touchée. ### Suggestions (non bloquantes) 1. Deux helpers `pct` homonymes coexistent désormais (`compareResults.pct` avec `×100`, `budgetVsActualResults.pct` sans). Chacun est correct pour son formateur respectif (`formatPct` divise / `pctFormatter` multiplie), mais l'homonymie à signature identique est un piège au copier-coller. Le doc-comment de `budgetVsActualResults.pct` mitige déjà — rien à corriger. 2. Coloration des colonnes « budget/planifié » des lignes de résultat par signe : cohérent avec `ComparePeriodTable`, mais colorer un montant *budgété* en vert/rouge peut surprendre. Cosmétique, aligné sur le sibling — OK de garder. Beau travail, fusionnable en l'état.

maximus closed this pull request

2026-07-12 01:25:43 +00:00

maximus commented

2026-07-12 01:25:58 +00:00

Author

Owner

Mergée localement dans main dans le cadre de la pile M2 (a982f9e) — merge --no-ff + résolution des conflits CHANGELOG (union #277/#278/#279). Forgejo ne marque pas les merges locaux comme merged ; fermeture manuelle. Issue liée auto-fermée via Resolves #N. Tip cumulé validé : tsc + vite build, 811 vitest, cargo check + 98 tests Rust.

Mergée localement dans `main` dans le cadre de la pile M2 (`a982f9e`) — merge `--no-ff` + résolution des conflits CHANGELOG (union #277/#278/#279). Forgejo ne marque pas les merges locaux comme *merged* ; fermeture manuelle. Issue liée auto-fermée via `Resolves #N`. Tip cumulé validé : tsc + vite build, 811 vitest, cargo check + 98 tests Rust.