es-kimo/claude-skills

Revue de maintenabilité

Le critère est unique. Combien d'endroits la prochaine personne (humaine ou agent) devra-t-elle ouvrir pour modifier ce code ? Les bugs de correction ne sont pas traités. Cela relève des revues de code ou de PR.

0. Mode

Mode	Cible	Base de comparaison	Cible bloquant
Auto-vérification	Mon diff avant commit	L'intégralité du dépôt	Corriger (sans rapport, immédiatement)
Revue de modification	Diff de la PR	L'intégralité du dépôt	Problèmes créés par cette modification uniquement
Inspection	Module ou répertoire entier	L'intégralité du dépôt	Tout, par ordre d'impact décroissant

En revue de modification, les problèmes préexistants ne sont pas remontés comme bloquants. Ils sont notés séparément comme "dette existante".

1. Lecture des critères du projet

1. Chercher docs/review/architecture.md. Sinon, utiliser le document de structure pointé par CLAUDE.md · AGENTS.md.
2. Les critères du projet priment sur les critères généraux de cette compétence. En particulier, extraire du document de référence :
   • La carte des modules et la direction des dépendances
   • La liste des abstractions à réutiliser (client HTTP, types de résultat, helpers de test, etc.)
   • La liste des dettes connues. Les motifs figurant dans cette liste ne sont pas des précédents que le nouveau code doit suivre.
3. S'il n'y a pas de document de référence, dessiner directement la carte des modules à partir de la structure des répertoires et des export des index de chaque module, et noter en première ligne "Absence de document de référence".

2. Ce qu'il faut examiner

Commencer par le haut. Plus c'est haut, plus le coût futur est élevé, et plus les agents le manquent souvent.

2-1. Contournement des abstractions existantes ?

L'agent travaille sur un seul ticket, ignore les modules créés par les tickets précédents et recrée la même chose. C'est l'échec le plus fréquent et le plus coûteux.

Pour chaque nouvelle fonction, type, classe d'erreur, constante, appel externe, chercher dans le dépôt du code qui fait la même chose.

git grep -nE "class \w+Error extends"            # classes d'erreur
git grep -nE "https?://[a-z.]+\\.(com|io|dev)"    # adresses API externes et en-têtes
git grep -nE "^(export )?type \w+ = \{"          # types de nom similaire (ex. XxxEntry)
git grep -nF "<chaîne ou regex caractéristique du nouveau code>"

• S'il existe plus d'un wrapper appelant le même système externe, les appelants sont obligés de tester les erreurs avec plusieurs classes. Chercher aussi ces traces (a instanceof X || b instanceof Y).
• Si la même constante de liste (extensions, valeurs d'état) se trouve à deux endroits, un seul sera mis à jour.

2-2. Direction des dépendances

• La modification viole-t-elle les règles de couches du document de référence ? (ex. module domaine qui importe une UI ou une API externe).
• Un module inférieur importe-t-il un module supérieur, ou deux modules s'importent-ils mutuellement ?

2-3. Unités

Découper par raison de changement, pas par taille.

• Si un fichier contient deux groupes de fonctions qui ne s'appellent pas mutuellement, c'est le signe de plus d'une raison de changer (ex. mappage de formulaire + accès à des clés imbriquées + formatage de date).
• Si plusieurs fichiers d'écran répètent la même séquence de préparation (authentification → chargement de configuration → branchement d'erreur), cette séquence devrait être une fonction nommée.

2-4. Qualité de l'abstraction

• API manquante : si deux appelants ou plus répètent le même prétraitement ou post-traitement, cela signifie que la fonction correspondante est absente du module. Exemple : si tous les appelants font doc.items.find((s) => s.kind === "x") puis appellent parseX(...), il manque readX(doc).
• Abstraction excessive : interface, factory, option, générique utilisé à un seul endroit. Ce sont des points d'extension inutiles pour l'instant.
• Type de frontière : si l'échec doit être traité par l'appelant, cela doit apparaître comme un type de résultat (union discriminée). L'appelant ne doit pas bifurquer en comparant des chaînes de message.

2-5. Patterns

• La modification suit-elle les patterns déjà utilisés dans le dépôt (type de résultat, injection de dépendance, gestion d'erreur, mode d'export des modules) ?
• Un nouveau pattern n'est accepté que s'il existe une raison pour laquelle le pattern existant ne convient pas. Si la raison n'est pas dans la PR ou un commentaire, le signaler.

2-6. Commentaires et noms

• Le commentaire est-il attaché à la mauvaise cible ? Il arrive fréquemment qu'un commit d'édition insère une nouvelle fonction entre deux fonctions, laissant le JSDoc sur une fonction différente.
• Le nom correspond-il à ce qu'il fait ? Si le nom laisse attendre un comportement différent, le signaler.

3. Jugement

Bloquant (ne pas fusionner)

• Du code ayant la même responsabilité existe déjà et a été recréé. Sauf si la PR ou un commentaire explique pourquoi l'existant ne peut pas être utilisé.
• Violation des règles de couches du document de référence.
• La nouvelle API force deux appelants ou plus à répéter le même traitement.
• Un commentaire est attaché à une cible différente, induisant en erreur.

Suggestion (ne bloque pas la fusion, à traiter en travail ultérieur)

• Séparation de fichier, amélioration de nom, doublon préexistant.

Ce qu'il ne faut pas noter : les préférences (fonctionnel/impératif, longueur de ligne), ce que le linter et le formateur attrapent, l'introduction de nouvelles bibliothèques hors du périmètre du ticket.

4. Vérification

Pour chaque remarque, vérifier avec du code. Une remarque erronée fait perdre du temps à l'agent d'implémentation.

• "Doublon" : ouvrir les deux emplacements et vérifier qu'ils font vraiment la même chose. Si l'apparence est similaire mais que les spécifications respectives sont différentes, ne pas compter.
• "N endroits" : compter avec grep.
• "Créé par cette modification" : vérifier avec git diff <base>...HEAD que la ligne concernée est présente.

5. Rapport

Première ligne : périmètre examiné (mode, chemin cible, présence du document de référence) et nombre (N bloquant, N suggestion, N dette existante).

Pour chaque élément :

• Titre : quel est le coût, sous forme de résultat. ("L'appelant teste l'erreur 401 GitHub avec 3 classes d'erreur" ✓ / "Amélioration de la gestion d'erreur" ✗)
• Emplacement : tous les emplacements pertinents en fichier:ligne. Pour un doublon, les deux côtés.
• Coût : combien d'endroits devront être modifiés lors du prochain changement. ("Changer l'en-tête de version API nécessite de modifier 3 fichiers")
• Direction : une ou deux phrases indiquant dans quelle direction fusionner.
• Niveau : bloquant / suggestion / dette existante.

S'il n'y a rien à signaler, écrire "Rien". Ne pas signaler pour remplir.

Ce qu'il ne faut pas faire

• Demander de découper uniquement sur le nombre de lignes ou la taille du fichier.
• Signaler sans donner de direction ("Il serait bon d'abstraire davantage").
• Considérer comme doublon du code d'apparence similaire uniquement.
• Remonter un problème préexistant comme bloquant pour cette PR.