maximus/Simpl-Resultat
1
0
Fork 0
Code Issues 19 Pull requests 8 Projects Releases 34 Packages Wiki Activity Actions

feat(categories): type-ahead combobox for the parent and keyword category fields #428

Merged
maximus merged 3 commits from issue-423-category-combobox-parent-keyword into main 2026-10-12 22:47:34 +00:00
Conversation 5 Commits 3 Files changed 16 +664 -92
maximus commented 2026-10-12 21:23:07 +00:00
Owner
Copy link

Fixes #423

What changes

The « Catégorie parente » field of CategoryForm and the « Catégorie » field of AddKeywordDialog (right-click on a transaction) were native <select>s. Both now use CategoryCombobox, like the category cell of the Transactions page: typing filters the list, which is ordered hierarchically with indentation and shows seed categories translated through i18n_key (both selects showed the raw name before).

  • CategoryCombobox.tsx: input type widened to a structural CategoryLike (id, name, parent_id?, sort_order, i18n_key?); sortHierarchical is generic over it. CategoryTreeNode[] passes without conversion, every existing caller compiles unchanged, CategoryCombobox.test.ts untouched (option A of the issue).
  • parentOptions.ts (new, pure): roots and level-1 categories; when editing, the edited category is out, and the allowed parent depth shrinks by the height of its sub-tree (04ac2e9, after the review): with children, only roots remain; with grandchildren, no parent at all. No separate descent walk: that rule alone leaves the descent out (a descendant sits at least one level below the edited category, deeper than any depth still allowed). The parent currently stored on the form is always kept, so a legacy or inconsistent parent never leaves the field blank (keeping it changes nothing on save). This list is the only guard today: updateCategory checks neither depth nor cycle, #393 will add that (D1).
  • CategoryForm.tsx: prop editingId (passed by CategoryDetailPanel in edit mode), combobox with « Aucun parent » as the extra option that resets to null, aria-label on the field.
  • keywordCategory.ts (new): loadKeywordCategories() reads transactionService.getAllCategories() (active and inputable, same list as Transactions, D2) and reconcileInitialCategory() keeps the pre-filled category only when it is in that list. AddKeywordDialog holds categoryId at null until the list is in, which keeps the apply button disabled meanwhile (no window where an id absent from the list is applicable), also on a failed load. The redundant categoriesLoaded flag of 04ac2e9 is gone (8898213).
  • AddKeywordDialog layout: the two fields moved out of the overflow-y-auto block into a fixed block above it. The old body scrolled as a whole, so the absolute listbox was clipped by its scrollport (confirmed by the plan check; SplitAdjustmentModal has the same clipping, not fixed here). The matches block still scrolls on its own.
  • Changelogs (Changed / Modifié), user guide step and its docs.* mirror in fr.json / en.json. No new i18n key: categories.parent, categories.noParent, transactions.table.noCategory reused.

Tests

Suite Before After
vitest 1932 (derived: 1963 minus the 31 added) 1963 (115 files), npm run build green

New, 31 tests measured on the five files alone: parentOptions.test.ts (14), keywordCategory.test.ts (5), CategoryForm.render.test.tsx (6), AddKeywordDialog.render.test.tsx (4), CategoryDetailPanel.render.test.tsx (2). The mutation counts below come from a run filtered on src/components/categories, which also covers the 14 tests of categories-migration, all green under every mutation. Static renders use the k[...] fake t of TransactionTable.render.test.tsx; the seed-parent expectation is written against the exact categoriesSeed.housing[defaultValue=Logement] string (#389).

Mutations (author, one at a time, restored from scratchpad copies, git diff --stat identical before/after)

# Mutation Red
1 parentOptions.ts: <= maxDepth → <= maxDepth + 1 (level 2 enters; the issue's parent.parent_id === null literal no longer exists) 8
2 parentOptions.ts: remove the descent exclusion 0, then rewritten : the line was redundant with the depth rule (see above), so it was removed from the code. Replacement after the review: MAX_PARENT_DEPTH - height → MAX_PARENT_DEPTH (height ignored)
2b parentOptions.ts: back to the children-only rule of 4bdb004 (height > 0 ? 0 : MAX_PARENT_DEPTH), the review's blocker 1
3 keywordCategory.ts: reconcileInitialCategory returns initialId ?? null (fallback removed) 2
4 keywordCategory.ts: loadKeywordCategories calls getAllCategoriesWithCounts 1
5 AddKeywordDialog.tsx: overflow-y-auto back on the fields block 1
6 parentOptions.ts: cycle guard (seen) of the height walk removed; the step bound turns the endless loop into a wrong height 1
7 parentOptions.ts: step bound of the height walk removed (defense behind seen, expected green) 0
8 CategoryDetailPanel.tsx: editingId no longer passed 1
9 CategoryForm.tsx: editingId not handed to buildParentOptions 2

Mutations 3 and 4 are caught because the component consumes exactly the two helpers; a mutation applied inside the component's useEffect is not observable under renderToStaticMarkup (no effect runs). The render tests pin the component to the helpers only through the mocked import.

Notes for review

  • Keyword dialog list: children whose parent is not inputable are orphans for sortHierarchical and show un-indented at the end, exactly as in the Transactions combobox.
  • The dialog body no longer scrolls as a whole, only the matches list does (its own max-h-[220px]). On a window under roughly 750 px tall the footer may leave the card; acceptable for the desktop window, said here rather than hidden.
  • Not verified in the running app (no Tauri dev run in this session): the open list drawing over the matches block, and keyboard navigation inside the modal.
  • Out of scope, as the issue says: AdjustmentForm.tsx:124 and LinkTransfersModal.tsx:260 keep their <select>; service-side cycle guard in #393.

🤖 Generated with Claude Code

Fixes #423 ## What changes The « Catégorie parente » field of `CategoryForm` and the « Catégorie » field of `AddKeywordDialog` (right-click on a transaction) were native `<select>`s. Both now use `CategoryCombobox`, like the category cell of the Transactions page: typing filters the list, which is ordered hierarchically with indentation and shows seed categories translated through `i18n_key` (both selects showed the raw `name` before). - `CategoryCombobox.tsx`: input type widened to a structural `CategoryLike` (`id`, `name`, `parent_id?`, `sort_order`, `i18n_key?`); `sortHierarchical` is generic over it. `CategoryTreeNode[]` passes without conversion, every existing caller compiles unchanged, `CategoryCombobox.test.ts` untouched (option A of the issue). - `parentOptions.ts` (new, pure): roots and level-1 categories; when editing, the edited category is out, and the allowed parent depth shrinks by the height of its sub-tree (`04ac2e9`, after the review): with children, only roots remain; with grandchildren, no parent at all. **No separate descent walk**: that rule alone leaves the descent out (a descendant sits at least one level below the edited category, deeper than any depth still allowed). The parent currently stored on the form is always kept, so a legacy or inconsistent parent never leaves the field blank (keeping it changes nothing on save). This list is the only guard today: `updateCategory` checks neither depth nor cycle, #393 will add that (D1). - `CategoryForm.tsx`: prop `editingId` (passed by `CategoryDetailPanel` in edit mode), combobox with « Aucun parent » as the extra option that resets to `null`, `aria-label` on the field. - `keywordCategory.ts` (new): `loadKeywordCategories()` reads `transactionService.getAllCategories()` (active and inputable, same list as Transactions, D2) and `reconcileInitialCategory()` keeps the pre-filled category only when it is in that list. `AddKeywordDialog` holds `categoryId` at `null` until the list is in, which keeps the apply button disabled meanwhile (no window where an id absent from the list is applicable), also on a failed load. The redundant `categoriesLoaded` flag of `04ac2e9` is gone (`8898213`). - `AddKeywordDialog` layout: the two fields moved out of the `overflow-y-auto` block into a fixed block above it. The old body scrolled as a whole, so the `absolute` listbox was clipped by its scrollport (confirmed by the plan check; `SplitAdjustmentModal` has the same clipping, not fixed here). The matches block still scrolls on its own. - Changelogs (Changed / Modifié), user guide step and its `docs.*` mirror in `fr.json` / `en.json`. No new i18n key: `categories.parent`, `categories.noParent`, `transactions.table.noCategory` reused. ## Tests | Suite | Before | After | |---|---|---| | vitest | 1932 (derived: 1963 minus the 31 added) | **1963** (115 files), `npm run build` green | New, 31 tests measured on the five files alone: `parentOptions.test.ts` (14), `keywordCategory.test.ts` (5), `CategoryForm.render.test.tsx` (6), `AddKeywordDialog.render.test.tsx` (4), `CategoryDetailPanel.render.test.tsx` (2). The mutation counts below come from a run filtered on `src/components/categories`, which also covers the 14 tests of `categories-migration`, all green under every mutation. Static renders use the `k[...]` fake `t` of `TransactionTable.render.test.tsx`; the seed-parent expectation is written against the exact `categoriesSeed.housing[defaultValue=Logement]` string (#389). ## Mutations (author, one at a time, restored from scratchpad copies, `git diff --stat` identical before/after) | # | Mutation | Red | |---|---|---| | 1 | `parentOptions.ts`: `<= maxDepth` → `<= maxDepth + 1` (level 2 enters; the issue's `parent.parent_id === null` literal no longer exists) | 8 | | 2 | `parentOptions.ts`: remove the descent exclusion | **0, then rewritten** : the line was redundant with the depth rule (see above), so it was removed from the code. Replacement after the review: `MAX_PARENT_DEPTH - height` → `MAX_PARENT_DEPTH` (height ignored) | 3 | | 2b | `parentOptions.ts`: back to the children-only rule of `4bdb004` (`height > 0 ? 0 : MAX_PARENT_DEPTH`), the review's blocker | 1 | | 3 | `keywordCategory.ts`: `reconcileInitialCategory` returns `initialId ?? null` (fallback removed) | 2 | | 4 | `keywordCategory.ts`: `loadKeywordCategories` calls `getAllCategoriesWithCounts` | 1 | | 5 | `AddKeywordDialog.tsx`: `overflow-y-auto` back on the fields block | 1 | | 6 | `parentOptions.ts`: cycle guard (`seen`) of the height walk removed; the step bound turns the endless loop into a wrong height | 1 | | 7 | `parentOptions.ts`: step bound of the height walk removed (defense behind `seen`, expected green) | 0 | | 8 | `CategoryDetailPanel.tsx`: `editingId` no longer passed | 1 | | 9 | `CategoryForm.tsx`: `editingId` not handed to `buildParentOptions` | 2 | Mutations 3 and 4 are caught because the component consumes exactly the two helpers; a mutation applied inside the component's `useEffect` is not observable under `renderToStaticMarkup` (no effect runs). The render tests pin the component to the helpers only through the mocked import. ## Notes for review - Keyword dialog list: children whose parent is not inputable are orphans for `sortHierarchical` and show un-indented at the end, exactly as in the Transactions combobox. - The dialog body no longer scrolls as a whole, only the matches list does (its own `max-h-[220px]`). On a window under roughly 750 px tall the footer may leave the card; acceptable for the desktop window, said here rather than hidden. - Not verified in the running app (no Tauri dev run in this session): the open list drawing over the matches block, and keyboard navigation inside the modal. - Out of scope, as the issue says: `AdjustmentForm.tsx:124` and `LinkTransfersModal.tsx:260` keep their `<select>`; service-side cycle guard in #393. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
maximus added 1 commit 2026-10-12 21:23:08 +00:00
feat(categories): type-ahead combobox for the parent and keyword category fields
All checks were successful
PR Check — Frontend / frontend (pull_request) Successful in 1m57s
Details
4bdb00402e 
Fixes #423

The "Parent category" field of CategoryForm and the "Category" field of
AddKeywordDialog were native selects with no typing. Both now use
CategoryCombobox, like the category cell of the Transactions page: typed
letters filter the list, which is ordered hierarchically with indentation
and shows seed categories through their i18n_key.

- CategoryCombobox accepts a minimal structural type (CategoryLike), so
  CategoryTreeNode rows pass without conversion; sortHierarchical is
  generic over it. Callers are unchanged.
- buildParentOptions (pure, tested) keeps roots and level-1 categories,
  leaves out the edited category, and keeps only roots when the edited
  category has children. That rule alone also leaves out its descent (a
  descendant is never a root), so no separate subtree walk is needed. The
  parent currently stored is always kept so the field never looks blank.
  CategoryForm gets an editingId prop, passed by CategoryDetailPanel.
- AddKeywordDialog reads the Transactions list (active and inputable,
  decision D2) through loadKeywordCategories, keeps the pre-filled
  category only when it is in that list (reconcileInitialCategory), and
  holds the apply button until the list is in. Its two fields sit above
  the scrolling block of matches, so the open list is not clipped by the
  scrollport.
- Tests: parentOptions, keywordCategory, and static renders of both
  components. Changelogs and the user guide (with its i18n mirror).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
maximus added the
status:review
type:feature
source:human
 labels 2026-10-12 21:23:09 +00:00
maximus commented 2026-10-12 22:13:38 +00:00
Author
Owner
Copy link

Revue adversariale — REQUEST_CHANGES

Exécuté : suite ciblée rejouée sur une copie hors dépôt de 4bdb004 (git archive, 46 tests categories + CategoryCombobox verts), sept mutations une à la fois, copie restaurée et comparée à HEAD (diff -rq vide), puis supprimée. CI run 526 lue dans le log : build 11 s, 114 fichiers, 1957 tests. Le stat main..HEAD qui montre docs/adr/0024-serveur-mcp.md supprimé est un artefact : main a avancé de 8b5b3b1 après le point de branche, le diff contre la base de fusion fait 15 fichiers.

Mutations

Mutation Rouge
Sonde relecteur : édition d'une racine avec petits-enfants ([1, 10→1, 100→10, 2], editingId: 1) doit rendre [] obtenu [2]
Retirer categoriesLoaded && de canApply 0
useState(initialCategoryId) au lieu de null 0
Auteur 5 : overflow-y-auto remis sur le bloc des champs 1
Auteur 1 : <= maxDepth + 1 8
Retirer le maintien de currentParentId 1
Auteur 3 : reconcile rend initialId ?? null 2

Issue bloquante

  1. src/components/categories/parentOptions.ts:721-723 — la règle D1 ne compte que les enfants directs, pas la descendance. editedHasChildren passe maxDepth à 0 : les racines restent offertes. Éditer une racine 1 qui porte 10 → 100 (trois niveaux) offre la racine 2 comme parent ; sauver donne 2 → 1 → 10 → 100, quatre niveaux, que le CHANGELOG de cette PR dit désormais impossible (« ni un parent qui créerait un quatrième niveau »). D1 (#423) : « les parents qui porteraient la hiérarchie au-delà de 3 niveaux en comptant la descendance de la catégorie éditée ». Rien ne rattrape : updateCategory (categoryService.ts:80-96) n'a aucune garde de profondeur (#393 pas livré), moveCategory (useCategories.ts) non plus ; seul createCategory en a une. Le test leaves out the whole descent (parentOptions.test.ts:617, toEqual([2])) consacre le mauvais résultat.
    • Correctif : hauteur du sous-arbre édité (0 sans enfant, 1 avec enfants, 2 avec petits-enfants) → maxDepth = MAX_PARENT_DEPTH - hauteur ; négatif, seul currentParentId survit au filtre (la liste ne montre plus que « Aucun parent », plus le parent déjà enregistré s'il y en a un). Le test de parentOptions.test.ts:617 passe à [].
    • Preuve attendue : la sonde ci-dessus verte ; mutation « hauteur plafonnée à 1 » rouge.

Suggestions (non bloquantes)

  • AddKeywordDialog.tsx : les gardes « categoryId tenu à null avant la liste » et categoriesLoaded se couvrent l'une l'autre, aucune ne rougit un test prise seule (deux mutations à 0). Le corps de la PR annonce « no window where an id absent from the list is applicable » sans test qui l'épingle. Extraire canApply en fonction pure testée, ou dire dans la PR que l'invariant tient par lecture.
  • AddKeywordDialog.tsx:69 : [initialCategoryId] en dépendance de l'effet recharge la liste et écrase le choix de l'utilisateur si la prop change pendant que le dialogue est ouvert. Stable chez les trois appelants aujourd'hui ; [] avec la valeur lue une fois serait plus sûr, ou un commentaire pour le dire.
  • Édition d'une catégorie dont le parent est inactif : categories vient de getAllCategoriesWithCounts (actives seulement), le parent n'est pas dans la liste, le champ s'affiche vide alors que parent_id est posé. Préexistant (l'ancien <select> montrait « Aucun parent », plus trompeur), à verser dans #393.

Vérifié sans remarque

Pas de secret ni de .env ; aucune chaîne en dur (transactions.table.noCategory, categories.noParent, categories.parent réutilisées) ; CategoryLike compatible avec Category (parent_id?: number) et CategoryTreeNode ; activeExtra="" → libellé « Aucun parent » ("" != null) ; cancelled sur l'effet de chargement ; rognage réglé (carte sans overflow-hidden, liste absolute z-50 hors du bloc défilant, bloc des champs à min-height: auto donc jamais écrasé) ; Enter sur le combobox fermé ouvre la liste au lieu de soumettre le formulaire (voulu) ; timeline de #423 : pull_ref closes depuis #428 ; commit au format conventionnel ; aucune migration.

Label de #423 : status:review → status:needs-fix.

## Revue adversariale — REQUEST_CHANGES **Exécuté** : suite ciblée rejouée sur une copie hors dépôt de `4bdb004` (`git archive`, 46 tests `categories` + `CategoryCombobox` verts), sept mutations une à la fois, copie restaurée et comparée à `HEAD` (`diff -rq` vide), puis supprimée. CI run 526 lue dans le log : build 11 s, 114 fichiers, 1957 tests. Le stat `main..HEAD` qui montre `docs/adr/0024-serveur-mcp.md` supprimé est un artefact : `main` a avancé de `8b5b3b1` après le point de branche, le diff contre la base de fusion fait 15 fichiers. ### Mutations | Mutation | Rouge | |---|---| | Sonde relecteur : édition d'une racine avec petits-enfants (`[1, 10→1, 100→10, 2]`, `editingId: 1`) doit rendre `[]` | obtenu `[2]` | | Retirer `categoriesLoaded &&` de `canApply` | 0 | | `useState(initialCategoryId)` au lieu de `null` | 0 | | Auteur 5 : `overflow-y-auto` remis sur le bloc des champs | 1 | | Auteur 1 : `<= maxDepth + 1` | 8 | | Retirer le maintien de `currentParentId` | 1 | | Auteur 3 : `reconcile` rend `initialId ?? null` | 2 | ### Issue bloquante 1. **`src/components/categories/parentOptions.ts:721-723` — la règle D1 ne compte que les enfants directs, pas la descendance.** `editedHasChildren` passe `maxDepth` à 0 : les racines restent offertes. Éditer une racine `1` qui porte `10 → 100` (trois niveaux) offre la racine `2` comme parent ; sauver donne `2 → 1 → 10 → 100`, quatre niveaux, que le CHANGELOG de cette PR dit désormais impossible (« ni un parent qui créerait un quatrième niveau »). D1 (#423) : « les parents qui porteraient la hiérarchie au-delà de 3 niveaux **en comptant la descendance** de la catégorie éditée ». Rien ne rattrape : `updateCategory` (`categoryService.ts:80-96`) n'a aucune garde de profondeur (#393 pas livré), `moveCategory` (`useCategories.ts`) non plus ; seul `createCategory` en a une. Le test `leaves out the whole descent` (`parentOptions.test.ts:617`, `toEqual([2])`) consacre le mauvais résultat. - Correctif : hauteur du sous-arbre édité (0 sans enfant, 1 avec enfants, 2 avec petits-enfants) → `maxDepth = MAX_PARENT_DEPTH - hauteur` ; négatif, seul `currentParentId` survit au filtre (la liste ne montre plus que « Aucun parent », plus le parent déjà enregistré s'il y en a un). Le test de `parentOptions.test.ts:617` passe à `[]`. - Preuve attendue : la sonde ci-dessus verte ; mutation « hauteur plafonnée à 1 » rouge. ### Suggestions (non bloquantes) - `AddKeywordDialog.tsx` : les gardes « `categoryId` tenu à `null` avant la liste » et `categoriesLoaded` se couvrent l'une l'autre, aucune ne rougit un test prise seule (deux mutations à 0). Le corps de la PR annonce « no window where an id absent from the list is applicable » sans test qui l'épingle. Extraire `canApply` en fonction pure testée, ou dire dans la PR que l'invariant tient par lecture. - `AddKeywordDialog.tsx:69` : `[initialCategoryId]` en dépendance de l'effet recharge la liste et écrase le choix de l'utilisateur si la prop change pendant que le dialogue est ouvert. Stable chez les trois appelants aujourd'hui ; `[]` avec la valeur lue une fois serait plus sûr, ou un commentaire pour le dire. - Édition d'une catégorie dont le parent est **inactif** : `categories` vient de `getAllCategoriesWithCounts` (actives seulement), le parent n'est pas dans la liste, le champ s'affiche vide alors que `parent_id` est posé. Préexistant (l'ancien `<select>` montrait « Aucun parent », plus trompeur), à verser dans #393. ### Vérifié sans remarque Pas de secret ni de `.env` ; aucune chaîne en dur (`transactions.table.noCategory`, `categories.noParent`, `categories.parent` réutilisées) ; `CategoryLike` compatible avec `Category` (`parent_id?: number`) et `CategoryTreeNode` ; `activeExtra=""` → libellé « Aucun parent » (`"" != null`) ; `cancelled` sur l'effet de chargement ; rognage réglé (carte sans `overflow-hidden`, liste `absolute z-50` hors du bloc défilant, bloc des champs à `min-height: auto` donc jamais écrasé) ; `Enter` sur le combobox fermé ouvre la liste au lieu de soumettre le formulaire (voulu) ; timeline de #423 : `pull_ref closes` depuis #428 ; commit au format conventionnel ; aucune migration. Label de #423 : `status:review` → `status:needs-fix`.
maximus added 1 commit 2026-10-12 22:15:56 +00:00
fix(categories): shrink the allowed parent depth by the height of the edited sub-tree
All checks were successful
PR Check — Frontend / frontend (pull_request) Successful in 1m54s
Details
04ac2e933e 
Refs #423

The review of PR #428 showed that the rule only looked at direct children:
editing a root that carries grandchildren still offered the other roots as
a parent, and saving created a fourth level, which nothing guards against
in updateCategory or moveCategory. The allowed parent depth is now
MAX_PARENT_DEPTH minus the height of the edited sub-tree (0, 1 or 2), so a
category with grandchildren gets no parent at all, only its current one.

Test written first on the reviewer's probe ([1, 10->1, 100->10, 2],
editing 1: [] expected, [2] obtained before the fix).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
maximus commented 2026-10-12 22:16:25 +00:00
Author
Owner
Copy link

Correctif du bloquant poussé en 04ac2e9 (tête non relue).

  • Règle : la profondeur de parent autorisée vaut MAX_PARENT_DEPTH - hauteur(sous-arbre édité) ; hauteur 0 (feuille) → racines et niveau 1, hauteur 1 → racines seules, hauteur 2 → aucun parent, seul le parent courant survit.
  • Rouge d'abord : la sonde de la revue ([1, 10→1, 100→10, 2], editingId: 1) écrite en test avant le correctif : [2] obtenu, [] attendu, 1 rouge sur 13.
  • Mutations sur l'arbre corrigé (copie de sauvegarde hors dépôt, diff vide après restauration) : retour à la règle « enfants directs seulement » de 4bdb004 → 1 rouge ; hauteur ignorée (MAX_PARENT_DEPTH constant) → 3 rouges.
  • Suite : 1958 vitest verts (1957 + 1), tsc propre.
  • Suggestions non bloquantes laissées telles quelles : les deux gardes du dialogue se couvrent (la seconde protège le cas où quelqu'un remettrait initialCategoryId comme valeur initiale) ; [initialCategoryId] en dépendance reste, les trois appelants remontent le dialogue à chaque ouverture ; parent inactif absent de la liste → #393.
Correctif du bloquant poussé en `04ac2e9` (tête non relue). - **Règle** : la profondeur de parent autorisée vaut `MAX_PARENT_DEPTH - hauteur(sous-arbre édité)` ; hauteur 0 (feuille) → racines et niveau 1, hauteur 1 → racines seules, hauteur 2 → aucun parent, seul le parent courant survit. - **Rouge d'abord** : la sonde de la revue (`[1, 10→1, 100→10, 2]`, `editingId: 1`) écrite en test avant le correctif : `[2]` obtenu, `[]` attendu, 1 rouge sur 13. - **Mutations sur l'arbre corrigé** (copie de sauvegarde hors dépôt, `diff` vide après restauration) : retour à la règle « enfants directs seulement » de `4bdb004` → 1 rouge ; hauteur ignorée (`MAX_PARENT_DEPTH` constant) → 3 rouges. - Suite : 1958 vitest verts (1957 + 1), `tsc` propre. - Suggestions non bloquantes laissées telles quelles : les deux gardes du dialogue se couvrent (la seconde protège le cas où quelqu'un remettrait `initialCategoryId` comme valeur initiale) ; `[initialCategoryId]` en dépendance reste, les trois appelants remontent le dialogue à chaque ouverture ; parent inactif absent de la liste → #393.
maximus commented 2026-10-12 22:39:53 +00:00
Author
Owner
Copy link

Revue — APPROVE

Résumé. Les deux <select> passent au CategoryCombobox sans toucher aux appelants existants (généricité structurelle CategoryLike), la règle des parents est isolée dans une fonction pure bien testée, et le dialogue mot-clé ne peut plus appliquer un identifiant absent de sa liste. Suite rejouée sur une copie hors dépôt (supprimée en fin de revue) : 1958 verts (114 fichiers), CI check-frontend verte sur 04ac2e9 (run 527). Timeline de #423 : pull_ref closes depuis cette PR. Commits au format conventionnel. Aucune migration, aucun secret, i18n par clés existantes.

Mutations du relecteur (une à la fois, copie restaurée après chacune, arbre réel inchangé)

# Mutation Rouges
R1 parentOptions.ts : la catégorie éditée n'est plus exclue 6
R2 parentOptions.ts : le parent courant n'est plus conservé 1
R3 parentOptions.ts : garde seen de subtreeHeight retirée 0
R4 parentOptions.ts : orphelin compté en profondeur 1 8
R5 parentOptions.ts : hauteur ignorée (MAX_PARENT_DEPTH seul) 3
R6 CategoryForm.tsx : activeExtra={null} 1
R7 CategoryForm.tsx : editingId: null passé à buildParentOptions 0
R8 AddKeywordDialog.tsx : categoriesLoaded retiré de canApply 0
R9 AddKeywordDialog.tsx : categoryId démarre à initialCategoryId 0
R10 keywordCategory.ts : reconcileInitialCategory rend toujours initialId 2

Aucune survivante ne cache un défaut de comportement ; elles marquent des épingles manquantes.

Suggestions (non bloquantes)

  1. Le commentaire sur #393 décrit du code qui n'existe pas. parentOptions.ts (« Saving an invalid parent is refused by the service-side validation of #393 ») et le corps de la PR renvoient à une validation côté service. #393 est ouverte (status:ready) et updateCategory (categoryService.ts) fait un UPDATE sans contrôle de profondeur ni de cycle ; seul createCategory vérifie la profondeur. Ce qui protège aujourd'hui, c'est que la combobox n'offre jamais un parent nouveau invalide (le parent conservé est toujours celui déjà enregistré). Reformuler en « serait le rôle de #393, ouverte », pour ne pas laisser croire à un filet qui n'est pas posé.
  2. R3 : la garde seen de subtreeHeight n'est pas épinglée. Le test de cycle édite 3, hors du cycle. Sans la garde, éditer un membre du cycle boucle sans fin : sonde [1→2, 2→1, 3] avec editingId: 1, 1 vert avec la garde (rend [3]), timeout à 40 s sans. Ajouter ce cas au test existant.
  3. R7 : le câblage editingId → buildParentOptions n'est pas épinglé. Le rendu statique ne montre pas la liste fermée ; un vi.mock("./parentOptions") avec assertion sur les arguments d'appel suffirait. Même angle mort pour CategoryDetailPanel.tsx (editingId={selectedCategory.id}), sans test.
  4. R8/R9 : deux gardes redondantes, chacune non épinglée seule. categoriesLoaded et le null initial se couvrent mutuellement ; le test « keeps the apply button disabled » est vert parce que la prévisualisation mockée ne se résout jamais, pas grâce à categoriesLoaded. Acceptable en défense en profondeur ; à savoir si l'une des deux est retirée un jour.
  5. Conséquence de D2 : dans ReportsCategoryPage, une catégorie zoomée non saisissable (parent) perd son pré-remplissage. Couvert par la phrase du changelog, rien à changer.

Actions faites

  • Verdict posté, #423 passe status:review → status:approved.
  • Copie hors dépôt scratchpad/pr428 et mut.sh supprimées ; git status du dépôt vide.
## Revue — APPROVE **Résumé.** Les deux `<select>` passent au `CategoryCombobox` sans toucher aux appelants existants (généricité structurelle `CategoryLike`), la règle des parents est isolée dans une fonction pure bien testée, et le dialogue mot-clé ne peut plus appliquer un identifiant absent de sa liste. Suite rejouée sur une copie hors dépôt (supprimée en fin de revue) : **1958 verts** (114 fichiers), CI `check-frontend` verte sur `04ac2e9` (run 527). Timeline de #423 : `pull_ref closes` depuis cette PR. Commits au format conventionnel. Aucune migration, aucun secret, i18n par clés existantes. ### Mutations du relecteur (une à la fois, copie restaurée après chacune, arbre réel inchangé) | # | Mutation | Rouges | |---|---|---| | R1 | `parentOptions.ts` : la catégorie éditée n'est plus exclue | 6 | | R2 | `parentOptions.ts` : le parent courant n'est plus conservé | 1 | | R3 | `parentOptions.ts` : garde `seen` de `subtreeHeight` retirée | **0** | | R4 | `parentOptions.ts` : orphelin compté en profondeur 1 | 8 | | R5 | `parentOptions.ts` : hauteur ignorée (`MAX_PARENT_DEPTH` seul) | 3 | | R6 | `CategoryForm.tsx` : `activeExtra={null}` | 1 | | R7 | `CategoryForm.tsx` : `editingId: null` passé à `buildParentOptions` | **0** | | R8 | `AddKeywordDialog.tsx` : `categoriesLoaded` retiré de `canApply` | **0** | | R9 | `AddKeywordDialog.tsx` : `categoryId` démarre à `initialCategoryId` | **0** | | R10 | `keywordCategory.ts` : `reconcileInitialCategory` rend toujours `initialId` | 2 | Aucune survivante ne cache un défaut de comportement ; elles marquent des épingles manquantes. ### Suggestions (non bloquantes) 1. **Le commentaire sur #393 décrit du code qui n'existe pas.** `parentOptions.ts` (« Saving an invalid parent is refused by the service-side validation of #393 ») et le corps de la PR renvoient à une validation côté service. #393 est **ouverte** (`status:ready`) et `updateCategory` (`categoryService.ts`) fait un `UPDATE` sans contrôle de profondeur ni de cycle ; seul `createCategory` vérifie la profondeur. Ce qui protège aujourd'hui, c'est que la combobox n'offre jamais un parent nouveau invalide (le parent conservé est toujours celui déjà enregistré). Reformuler en « serait le rôle de #393, ouverte », pour ne pas laisser croire à un filet qui n'est pas posé. 2. **R3 : la garde `seen` de `subtreeHeight` n'est pas épinglée.** Le test de cycle édite `3`, hors du cycle. Sans la garde, éditer un membre du cycle boucle sans fin : sonde `[1→2, 2→1, 3]` avec `editingId: 1`, 1 vert avec la garde (rend `[3]`), `timeout` à 40 s sans. Ajouter ce cas au test existant. 3. **R7 : le câblage `editingId` → `buildParentOptions` n'est pas épinglé.** Le rendu statique ne montre pas la liste fermée ; un `vi.mock("./parentOptions")` avec assertion sur les arguments d'appel suffirait. Même angle mort pour `CategoryDetailPanel.tsx` (`editingId={selectedCategory.id}`), sans test. 4. **R8/R9 : deux gardes redondantes, chacune non épinglée seule.** `categoriesLoaded` et le `null` initial se couvrent mutuellement ; le test « keeps the apply button disabled » est vert parce que la prévisualisation mockée ne se résout jamais, pas grâce à `categoriesLoaded`. Acceptable en défense en profondeur ; à savoir si l'une des deux est retirée un jour. 5. Conséquence de D2 : dans `ReportsCategoryPage`, une catégorie zoomée non saisissable (parent) perd son pré-remplissage. Couvert par la phrase du changelog, rien à changer. ### Actions faites - Verdict posté, #423 passe `status:review` → `status:approved`. - Copie hors dépôt `scratchpad/pr428` et `mut.sh` supprimées ; `git status` du dépôt vide.
maximus added 1 commit 2026-10-12 22:44:57 +00:00
refactor(categories): follow-ups of the review of PR #428
All checks were successful
PR Check — Frontend / frontend (pull_request) Successful in 1m55s
Details
8898213e02 
Refs #423

- parentOptions.ts: say that this list is the only guard today (updateCategory
  checks neither depth nor cycle, #393 will), instead of pointing to a
  service-side validation that does not exist yet. The height walk is also
  bounded by the number of categories, so a missing cycle guard gives a wrong
  height instead of an endless loop; a test edits a member of a cycle.
- AddKeywordDialog.tsx: drop `categoriesLoaded`, which only repeated what the
  null initial category already guarantees.
- Tests: the form and the detail panel are pinned to what they hand to
  buildParentOptions (editingId, currentParentId), through a pass-through spy.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
maximus commented 2026-10-12 22:45:29 +00:00
Author
Owner
Copy link

Suites de la revue 23121 appliquées en 8898213 (tête non relue, merge demandé par Max : « corrige les 4 points et merge »).

  1. #393 mal cité : parentOptions.ts et le corps de la PR disent maintenant que cette liste est la seule garde (updateCategory ne vérifie ni profondeur ni cycle), au lieu d'une validation côté service qui n'existe pas.
  2. Garde de cycle non épinglée : test « éditer un membre d'un cycle » ([1→2, 2→1, 3], editingId: 1 → [3]). La marche de hauteur est aussi bornée par le nombre de catégories, ce qui rend la mutation observable : sans seen, hauteur fausse et 1 rouge au lieu d'une boucle sans fin.
  3. Câblage editingId : espion passe-plat sur buildParentOptions dans CategoryForm.render.test.tsx et un nouveau CategoryDetailPanel.render.test.tsx ; retirer editingId du panneau → 1 rouge, ne pas le transmettre depuis le formulaire → 2 rouges.
  4. Gardes redondantes du dialogue : categoriesLoaded retiré, le null initial suffit (le bouton reste désactivé tant que la liste n'est pas là, y compris sur un chargement en échec).

Suite : 1963 vitest verts (115 fichiers), tsc propre. Mutations reportées dans le tableau du corps (lignes 6 à 9).

Suites de la revue 23121 appliquées en `8898213` (**tête non relue**, merge demandé par Max : « corrige les 4 points et merge »). 1. **#393 mal cité** : `parentOptions.ts` et le corps de la PR disent maintenant que cette liste est la seule garde (`updateCategory` ne vérifie ni profondeur ni cycle), au lieu d'une validation côté service qui n'existe pas. 2. **Garde de cycle non épinglée** : test « éditer un membre d'un cycle » (`[1→2, 2→1, 3]`, `editingId: 1` → `[3]`). La marche de hauteur est aussi bornée par le nombre de catégories, ce qui rend la mutation observable : sans `seen`, hauteur fausse et 1 rouge au lieu d'une boucle sans fin. 3. **Câblage `editingId`** : espion passe-plat sur `buildParentOptions` dans `CategoryForm.render.test.tsx` et un nouveau `CategoryDetailPanel.render.test.tsx` ; retirer `editingId` du panneau → 1 rouge, ne pas le transmettre depuis le formulaire → 2 rouges. 4. **Gardes redondantes du dialogue** : `categoriesLoaded` retiré, le `null` initial suffit (le bouton reste désactivé tant que la liste n'est pas là, y compris sur un chargement en échec). Suite : 1963 vitest verts (115 fichiers), `tsc` propre. Mutations reportées dans le tableau du corps (lignes 6 à 9).
maximus merged commit 8898213e02 into main 2026-10-12 22:47:34 +00:00
maximus deleted branch issue-423-category-combobox-parent-keyword 2026-10-12 22:47:34 +00:00
maximus removed the
status:review
 label 2026-10-12 22:48:24 +00:00
maximus commented 2026-10-12 22:48:24 +00:00
Author
Owner
Copy link

Mergée en avance rapide par l'API sur 8898213 (main = tête de la PR, branche supprimée), sur la demande de Max « corrige les 4 points et merge ».

  • Revues : REQUEST_CHANGES sur 4bdb004 (23095), APPROVE sur 04ac2e9 (23121).
  • 8898213 ajouté après la revue, non relu : les quatre suivis de 23121 (commentaires sur #393, test et borne de la marche de hauteur, espions sur buildParentOptions, retrait de categoriesLoaded). Seul changement de comportement : la garde redondante retirée du dialogue, le null initial garde le bouton désactivé.
  • Preuves sur la tête : CI run 528 verte ; worktree npm ci propre : sonde rouge rc=1, build, 1963 vitest (115 fichiers).
  • Suites non bloquantes rouvertes en issue : les deux autres <select> de catégories et le rognage identique de SplitAdjustmentModal.
Mergée en avance rapide par l'API sur `8898213` (`main` = tête de la PR, branche supprimée), sur la demande de Max « corrige les 4 points et merge ». - Revues : REQUEST_CHANGES sur `4bdb004` (23095), APPROVE sur `04ac2e9` (23121). - **`8898213` ajouté après la revue, non relu** : les quatre suivis de 23121 (commentaires sur #393, test et borne de la marche de hauteur, espions sur `buildParentOptions`, retrait de `categoriesLoaded`). Seul changement de comportement : la garde redondante retirée du dialogue, le `null` initial garde le bouton désactivé. - Preuves sur la tête : CI run 528 verte ; worktree `npm ci` propre : sonde rouge rc=1, build, 1963 vitest (115 fichiers). - Suites non bloquantes rouvertes en issue : les deux autres `<select>` de catégories et le rognage identique de `SplitAdjustmentModal`.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
autopilot:pending-human

PR produced by /autopilot — awaits human review/merge

  
source:analyste

Créée par le Super-Analyste

  
source:defenseur

Créée par l Escouade Défenseur

  
source:human

Créée par un humain

  
source:medic

Créée par le Medic (auto-détection)

  
status:approved

PR approuvée, prête à merger

  
status:blocked

Bloquée (question, dépendance, clarification)

  
status:in-progress

Un agent (Medic/Dev) travaille dessus

  
status:needs-clarification

Issue blocked by ambiguity — Max needs to clarify before re-running autopilot

  
status:needs-fix

Reviewer demande des corrections

  
status:ready

Analysée et documentée, prête à prendre

  
status:review

PR créée, en attente du Reviewer

  
status:triage

Nouvelle issue, pas encore analysée

  
type:bug

Correction de bug → Medic

  
type:feature

Nouvelle fonctionnalité → Dev

  
type:infra

Config, CI/CD, déploiement → Dev

  
type:refactor

Refactoring / amélioration technique → Dev

  
type:schema

Modèle de données / migrations → Dev

  
type:security

Issue de sécurité → Medic ou Dev

No labels
autopilot:pending-human
source:analyste
source:defenseur
source:human
source:medic
status:approved
status:blocked
status:in-progress
status:needs-clarification
status:needs-fix
status:ready
status:review
status:triage
type:bug
type:feature
type:infra
type:refactor
type:schema
type:security
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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#428
No description provided.
Delete branch "issue-423-category-combobox-parent-keyword"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?