maximus/Simpl-Resultat

Fork 0

fix(reports): net transfer-type categories in real-vs-real compare (#243) #248

Closed

maximus wants to merge 0 commits from issue-243-netting-transferts into main

maximus commented

2026-07-04 21:08:30 +00:00

Owner

Real-vs-real Compare summed only outflows (SUM(ABS(amount)) filtered on amount < 0), so a transfer category like "Paiement CC" always showed the sum of its debits instead of netting to ~0. This nets transfer-type categories via a signed SUM(t.amount) while keeping the expense behavior (ABS + amount < 0) for every other type, and broadens the WHERE so transfer credits survive the expense filter. Both MoM and YoY share one COMPARE_DELTA_SQL constant; real-vs-budget already nets and is unchanged.

Resolves #243

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

Real-vs-real Compare summed only outflows (SUM(ABS(amount)) filtered on amount < 0), so a `transfer` category like "Paiement CC" always showed the sum of its debits instead of netting to ~0. This nets `transfer`-type categories via a signed SUM(t.amount) while keeping the expense behavior (ABS + amount < 0) for every other type, and broadens the WHERE so transfer credits survive the expense filter. Both MoM and YoY share one COMPARE_DELTA_SQL constant; real-vs-budget already nets and is unchanged. Resolves #243 Generated autonomously by /autopilot run of 2026-07-04

maximus added 1 commit 2026-07-04 21:08:31 +00:00

fix(reports): net transfer-type categories in real-vs-real compare

PR Check / rust (pull_request) Successful in 23m23s

Details

PR Check / frontend (pull_request) Successful in 2m25s

Details

4b000d022f

Reports > Compare (real vs real) summed only outflows (SUM(ABS(amount))
filtered on amount < 0), so a `transfer` category like "Paiement CC" -
whose debit and matching credit should cancel - always showed the sum of
its debits instead of ~0.

Net transfer-type categories via a signed SUM(t.amount) while keeping the
expense behavior (ABS + amount < 0) for every other type, and broaden the
WHERE so transfer credits survive the expense filter. Both MoM and YoY now
share a single COMPARE_DELTA_SQL constant so they stay in sync. Real-vs-
budget already nets (signed SUM, no filter) and is unchanged.

Resolves #243

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:08:44 +00:00

maximus referenced this pull request

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

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

maximus commented

2026-07-05 00:53:00 +00:00

Author

Owner

Revue /pr-review — PR #248 (issue #243)

Verdict : APPROVE

Résumé

Le correctif implémente correctement l'intention : les catégories de type transfer s'annulent via un SUM(t.amount) signé, pendant que le comportement des autres types reste strictement identique (WHERE amount < 0 + ABS). Changelog bilingue, tests et lien d'issue présents. Quelques finitions non-bloquantes subsistent pour le cas rare d'un transfert déséquilibré sur la période.

Points validés

Correctness du SQL

Lignes non-transfer : le WHERE reste t.amount < 0 et la somme reste ABS(t.amount) → totaux bit-à-bit identiques à l'ancien comportement. Vérifié.
Lignes transfer : WHERE élargi (OR COALESCE(c.type,'expense')='transfer') pour que les crédits survivent au filtre de dépense, puis SUM(t.amount) signé → une paire débit/crédit équilibrée s'annule à ~0. Correct.
Non catégorisé (c.type NULL via LEFT JOIN) → COALESCE = 'expense' → comportement ABS. Correct.
Division par zéro protégée : rowsToDeltas renvoie deltaPct = null quand previous === 0 (transfert netté) → rendu « — », pas de NaN/Infinity.
SQL entièrement paramétré ($1..$8), aucune injection.

Refactor

Les deux requêtes (MoM et YoY) étaient caractère-pour-caractère identiques ; leur extraction dans COMPARE_DELTA_SQL est sûre et supprime la duplication. L'ordre des paramètres est préservé dans les deux appels.

Schéma & données

categories.type existe (NOT NULL DEFAULT 'expense', valeurs expense/income/transfer) ; transfer est posé via l'UI (CategoryForm) et le seed (id 71 « Paiement CC »). Aucune migration touchée.

Claim « real-vs-budget nète déjà »

Confirmé : getActualsByCategoryRange (budgetService) utilise déjà SUM(amount) signé sans ABS ni filtre de signe → les transferts s'y annulent nativement. Fichier non modifié.

Process

Commit conventionnel valide, « Resolves #243 », entrées Changelog sous [Unreleased] dans les deux langues. Aucune chaîne UI ajoutée → pas de sujet i18n. type:bug : les assertions structurelles échoueraient sur l'ancien SQL (absence du CASE transfert) → garde de régression effective au niveau chaîne, acceptable vu l'absence de SQLite runnable en tests unitaires.

Suggestions (non-bloquantes)

Toutes concernent l'arrivée de valeurs signées/négatives de transfert dans du code de rendu conçu pour des magnitudes positives. Le cas courant — transfert équilibré sur la même période (ex. « Paiement CC » avec les deux comptes importés) — nète à ~0 et s'affiche proprement ; ces points ne touchent que le transfert déséquilibré / à une seule jambe.

Ligne Total de ComparePeriodTable (l.66-76) — le total additionne currentAmount/deltaAbs sur toutes les lignes, mélangeant désormais des magnitudes de dépense positives et des nets de transfert signés. Équilibré → contribue ~0 (améliore même le total, qui n'est plus gonflé par les débits de transfert) ; déséquilibré → décale le total du montant net. Envisager d'exclure les lignes transfer du total, ou de les présenter à part.
ComparePeriodChart + variationColor — convention « dépense » (positif = rouge/mauvais, barres vers la droite). Un net de transfert négatif dessine une barre vers la gauche avec le rayon d'angle du mauvais côté, et un delta de transfert positif est peint en rouge. Cosmétique.
rowsToDeltas deltaPct (l.400) — divise par le previousAmount signé (pas Math.abs). Un transfert dont la période précédente nète négatif produit un signe de % inversé vs deltaAbs (ligne verte affichant +50 %). Le chemin des totaux utilise déjà Math.abs(previous) (l.78) — aligner le calcul par ligne.
getCartesSnapshot significantMovers (l.952-954) — le filtre previousAmount > 0 || currentAmount > 0 écarte maintenant du top-movers Cartes les transferts qui nètent négatif sur les deux périodes. Probablement souhaitable (un transfert n'est pas un mouvement de dépense), simple note de changement de comportement.
Tests — surtout des assertions structurelles sur la chaîne SQL (fragiles à tout reformatage de COMPARE_DELTA_SQL), plus une fonction de sémantique de référence ; ils n'exécutent pas le SQL. Compromis raisonnable vu la contrainte « pas de SQLite dans les tests unitaires », à garder en tête.

Revue adversariale générée par /pr-review. Cas heureux (transfert équilibré) correct et amélioré ; suggestions = polissage de suivi pour les valeurs signées, non-bloquant.

## Revue /pr-review — PR #248 (issue #243) **Verdict : APPROVE** ### Résumé Le correctif implémente correctement l'intention : les catégories de type `transfer` s'annulent via un `SUM(t.amount)` signé, pendant que le comportement des autres types reste strictement identique (WHERE `amount < 0` + `ABS`). Changelog bilingue, tests et lien d'issue présents. Quelques finitions non-bloquantes subsistent pour le cas rare d'un transfert déséquilibré sur la période. ### Points validés **Correctness du SQL** - Lignes non-`transfer` : le WHERE reste `t.amount < 0` et la somme reste `ABS(t.amount)` → totaux bit-à-bit identiques à l'ancien comportement. Vérifié. - Lignes `transfer` : WHERE élargi (`OR COALESCE(c.type,'expense')='transfer'`) pour que les crédits survivent au filtre de dépense, puis `SUM(t.amount)` signé → une paire débit/crédit équilibrée s'annule à ~0. Correct. - Non catégorisé (`c.type` NULL via LEFT JOIN) → `COALESCE` = 'expense' → comportement ABS. Correct. - Division par zéro protégée : `rowsToDeltas` renvoie `deltaPct = null` quand `previous === 0` (transfert netté) → rendu « — », pas de NaN/Infinity. - SQL entièrement paramétré (`$1..$8`), aucune injection. **Refactor** - Les deux requêtes (MoM et YoY) étaient caractère-pour-caractère identiques ; leur extraction dans `COMPARE_DELTA_SQL` est sûre et supprime la duplication. L'ordre des paramètres est préservé dans les deux appels. **Schéma & données** - `categories.type` existe (`NOT NULL DEFAULT 'expense'`, valeurs `expense`/`income`/`transfer`) ; `transfer` est posé via l'UI (`CategoryForm`) et le seed (id 71 « Paiement CC »). Aucune migration touchée. **Claim « real-vs-budget nète déjà »** - Confirmé : `getActualsByCategoryRange` (budgetService) utilise déjà `SUM(amount)` signé sans `ABS` ni filtre de signe → les transferts s'y annulent nativement. Fichier non modifié. **Process** - Commit conventionnel valide, « Resolves #243 », entrées Changelog sous `[Unreleased]` dans les deux langues. Aucune chaîne UI ajoutée → pas de sujet i18n. `type:bug` : les assertions structurelles échoueraient sur l'ancien SQL (absence du `CASE` transfert) → garde de régression effective au niveau chaîne, acceptable vu l'absence de SQLite runnable en tests unitaires. ### Suggestions (non-bloquantes) Toutes concernent l'arrivée de valeurs **signées/négatives** de transfert dans du code de rendu conçu pour des magnitudes positives. Le cas courant — transfert équilibré sur la même période (ex. « Paiement CC » avec les deux comptes importés) — nète à ~0 et s'affiche proprement ; ces points ne touchent que le transfert déséquilibré / à une seule jambe. 1. **Ligne Total de `ComparePeriodTable` (l.66-76)** — le total additionne `currentAmount`/`deltaAbs` sur toutes les lignes, mélangeant désormais des magnitudes de dépense positives et des nets de transfert signés. Équilibré → contribue ~0 (améliore même le total, qui n'est plus gonflé par les débits de transfert) ; déséquilibré → décale le total du montant net. Envisager d'exclure les lignes `transfer` du total, ou de les présenter à part. 2. **`ComparePeriodChart` + `variationColor`** — convention « dépense » (positif = rouge/mauvais, barres vers la droite). Un net de transfert négatif dessine une barre vers la gauche avec le rayon d'angle du mauvais côté, et un delta de transfert positif est peint en rouge. Cosmétique. 3. **`rowsToDeltas` deltaPct (l.400)** — divise par le `previousAmount` signé (pas `Math.abs`). Un transfert dont la période précédente nète négatif produit un signe de % inversé vs `deltaAbs` (ligne verte affichant `+50 %`). Le chemin des totaux utilise déjà `Math.abs(previous)` (l.78) — aligner le calcul par ligne. 4. **`getCartesSnapshot` significantMovers (l.952-954)** — le filtre `previousAmount > 0 || currentAmount > 0` écarte maintenant du top-movers Cartes les transferts qui nètent négatif sur les deux périodes. Probablement souhaitable (un transfert n'est pas un mouvement de dépense), simple note de changement de comportement. 5. **Tests** — surtout des assertions structurelles sur la chaîne SQL (fragiles à tout reformatage de `COMPARE_DELTA_SQL`), plus une fonction de sémantique de référence ; ils n'exécutent pas le SQL. Compromis raisonnable vu la contrainte « pas de SQLite dans les tests unitaires », à garder en tête. --- *Revue adversariale générée par `/pr-review`. Cas heureux (transfert équilibré) correct et amélioré ; suggestions = polissage de suivi pour les valeurs signées, non-bloquant.*

maximus referenced this pull request from a commit

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

Merge PR #248: fix(reports) net transfer-type categories in real-vs-real compare (#243)

maximus commented

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

Author

Owner

Merged into main locally as part of the milestone stack (merge commit a6fa89e). 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 `a6fa89e`). 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:18 +00:00

maximus referenced this pull request

2026-07-05 22:18:22 +00:00

analyse de résultat (revenus + résultat avant/après transferts) + défaut mois précédent #253

maximus referenced this pull request

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

montant inclus dans comparable #243