Revue de code par IA : un barème pour sortir de la boucle infinie

Demandez à un assistant de code de relire une Merge Request : il trouvera toujours quelque chose. Un nom de variable perfectible, un commentaire à reformuler, une extraction de méthode possible. L'auteur corrige, pousse, la CI repart, et la passe suivante trouve trois nouvelles remarques, tout aussi plausibles, tout aussi peu décisives. La revue automatique ne converge pas : elle a été conçue pour produire des remarques, pas pour dire quand il n'y en a plus qui comptent.

J'ai rencontré ce problème en construisant un plugin Claude Code d'équipe, utilisé au quotidien sur une plateforme SaaS B2B multi-tenant que j'ai opérée. La réponse n'a pas été un meilleur prompt, mais un barème : quatre niveaux de sévérité numérotés, un test simple qui décide si un point mérite d'être écrit, une règle qui dit où chaque niveau est publié, et une mémoire d'une passe à l'autre. Cet article détaille ce barème, la preuve exigée d'un test de non-régression, et ce que j'appelle le droit de ne pas livrer.

La revue IA qui ne finit jamais

Une revue humaine s'arrête d'elle-même : le relecteur a autre chose à faire, et il sait, par expérience, qu'une remarque de goût ne vaut pas un aller-retour. Un modèle n'a ni cette fatigue ni ce jugement implicite. Chaque passe repart de zéro et relit le code comme s'il était neuf. Sans consigne contraire, elle écrit tout ce qui pourrait être amélioré, et presque tout peut l'être.

Le coût, lui, est bien réel. Chaque remarque coûte une lecture à l'auteur. Chaque remarque traitée coûte un commit, un push et un pipeline CI complet. Et chaque remarque écartée laisse un doute : était-ce important ? Trente points dont vingt-sept sont à ignorer ne font pas une revue sévère, mais une revue inutilisable, parce que l'auteur doit faire lui-même le tri que la revue aurait dû faire.

Un barème, et surtout un test d'écriture

Le barème tient en quatre niveaux, chacun préfixé d'une lettre :

• B — bloquant — la MR ne peut pas être fusionnée : bug, faille, régression, perte de données
• M — majeur — à corriger avant la fusion, ce qui vaut un commit de plus et un cycle de CI
• m — mineur — à corriger seulement si l'auteur repasse sur la ligne, sans retour exigé
• n — nit — préférence de style ou de nommage, que l'auteur reste libre d'ignorer

Le classement se décide sur la conséquence en production, pas sur l'effort de correction : une condition inversée qui laisse fuiter des données d'un tenant à l'autre reste bloquante, même si le correctif tient en un caractère.

Mais l'essentiel n'est pas l'échelle. C'est la question que la revue doit se poser avant d'écrire chaque point :

Ce point, à lui seul, justifie-t-il que l'auteur pousse un commit de plus, donc qu'un pipeline CI complet reparte ?

Si oui, c'est un B ou un M, écrit précisément, avec la correction attendue et son chemin:ligne. Si non, mais que la remarque servira à celui qui éditera cette ligne plus tard, c'est un m ou un n, en une ligne. Et si non, et qu'elle ne sert à personne, elle ne s'écrit pas. C'est ce troisième cas qui arrête la boucle : la revue a enfin le droit, et même l'obligation, de se taire.

Certaines remarques échouent à ce test par construction, et la consigne les exclut nommément : ce qu'un formateur ou un linter du dépôt corrige déjà, une préférence personnelle sans règle du dépôt derrière elle, une réécriture à comportement identique proposée pour le goût, et un point déjà tranché dans un thread de la MR.

Des références citables

Chaque point est numéroté dans sa sévérité, dans l'ordre d'apparition : B1, M1, M2, m1… La référence ouvre le point partout où il apparaît, dans le rapport comme dans le commentaire publié sur la MR :

**M2 —** Export lancé sans vérifier le droit du rôle courant

`src/Export/ExportController.php:42`

Le contrôleur appelle le service d'export avant le contrôle d'accès :
un rôle sans le privilège d'export obtient le fichier en appelant la route directement.
Déplacer le `denyAccessUnlessGranted()` avant l'appel au service.

**m1 —** `$res` à renommer en `$exportedRows` (`src/Export/ExportService.php:88`).

Synthèse : 0 B, 2 M, 1 m, 0 n — réserves, la MR repasse par la CI.

La numérotation transforme la discussion. L'auteur répond « M2 et m1 corrigés, n3 laissé tel quel » sans recopier l'énoncé. Et l'outil de traitement des retours accepte une liste de références : on lui demande de traiter M1 m2, il ne touche qu'à ces deux threads et dit lesquels il a laissés de côté. Un détail compte plus qu'il n'y paraît : la casse porte le sens. M1 est un majeur, m1 un mineur, et la consigne interdit expressément de normaliser la casse d'une référence reçue.

Dernière règle du rapport : quand aucun B ni M n'est sorti, la revue le dit franchement. La MR n'a pas à repasser par la CI.

Où va chaque niveau

Un thread inline sur une MR n'est pas gratuit : il oblige l'auteur à y répondre puis à le résoudre. C'est le coût que seuls les points qui valent un cycle justifient. La répartition par défaut en découle :

• B et M — un thread inline par point, ancré sur la ligne concernée
• m et n — regroupés dans une seule note en fin de MR, sous forme de liste, jamais un thread chacun

La même logique vaut du côté de l'auteur. Quand l'outil traite les retours, il suit l'ordre des sévérités : les B, puis les M, puis le reste. Un m ou un n ne justifie pas à lui seul un commit : il se groupe avec les corrections majeures du même passage. S'il ne reste que des mineurs et des nits, l'outil le dit et demande s'il faut vraiment repousser. C'est souvent la réponse « non » qui fait gagner le plus de temps.

Une revue qui se souvient de ses décisions

Relire plusieurs fois une fonctionnalité sensible est utile : chaque passe peut trouver ce que la précédente a manqué. Mais une passe repart de zéro, et sans mémoire, elle rouvre aussi ce qui a déjà été tranché.

Cette mémoire existe déjà : ce sont les threads de la MR et la note groupée des mineurs. La revue les lit avant le code, et range chaque point déjà soulevé dans l'un de trois cas :

• corrigé — la revue vérifie seulement que la correction tient
• écarté — l'auteur a répondu « ne pas corriger » (défaut assumé) ou « normal » (ce n'est pas un défaut : choix produit, configuration, droit d'accès) ; le point ne se réécrit pas, il se rappelle en une ligne avec sa raison
• ouvert — sans réponse, il se signale comme une répétition, jamais comme un point neuf

Un point écarté sans raison sera redemandé à la passe suivante : en fin de passe, la revue réclame donc cette raison à l'auteur.

Reste qu'une décision ne vaut que pour le code sur lequel elle a été prise. GitLab retient le commit sur lequel chaque thread a été posé (position.head_sha) et le dernier commit de la MR (diff_refs.head_sha). Il suffit de les comparer sur le seul fichier concerné :

git diff <commit-du-thread>..<commit-de-la-mr> -- src/Export/ExportController.php

Sortie vide : la décision tient. Sinon, le point redevient « à revérifier » : c'est le seul cas où un point tranché a le droit de revenir.

Juger sur le ticket et le code, pas sur la CI

Une revue qui ignore la demande ne peut juger que le style. Avant le diff, elle lit donc le ticket lié, retrouvé par la branche, les commits ou la description de la MR, et résume en une phrase ce que le changement doit livrer. Le rapport conclut sur ce point : objectif rempli, partiellement rempli ou non rempli, avec les cas oubliés et ce qui déborde du périmètre. Sans ticket, le rapport le dit.

Le statut du pipeline, lui, ne prouve rien : un pipeline vert laisse passer une fuite de données entre tenants, un pipeline rouge peut venir d'un test instable. Chaque point s'appuie sur une ligne lue dans le fichier lui-même et citée en chemin:ligne, jamais sur les numéros d'une sortie de diff, qui ne sont pas ceux du fichier.

Même exigence pour les réserves de la revue sur sa propre méthode. Trois revues d'une même MR ont répété « cette version de PHP ne connaît pas array_any », alors qu'un polyfill du dépôt fournissait la fonction : une seule commande suffisait à le vérifier. Une réserve fausse décrédibilise tout le rapport. Désormais, une réserve se vérifie avant de s'écrire ; si la vérification est impossible, la revue écrit ce qu'elle a lancé, ce qu'elle a obtenu, et ce qu'elle ne peut donc pas conclure.

Prouver qu'un test protège vraiment

Le barème règle le bruit de la revue. Il ne règle pas un autre défaut du code généré : le test de non-régression qui passe mais ne protège rien. Vert, il ressemble exactement à un vrai garde-fou. L'exemple suivant, tiré des règles de test du plugin, semble correct à la relecture :

$proven = null;
$repository->method('findLimit')->willReturnCallback(static fn () => $proven ?? self::DEFAULT_LIMIT);
$listener = function () use (&$proven): void { $proven = 10; };

Une fonction fléchée capture $proven par valeur, au moment où elle est définie, donc à null. Le stub renvoie toujours la limite par défaut, quoi que fasse la seconde closure : le test est vert pour une mauvaise raison. Pour partager l'état entre les deux closures, il faut un objet ($state = new stdClass()).

D'où la règle inscrite dans le plugin : un test qui verrouille un correctif doit avoir été vu rouge sans le correctif, puis vert avec. On sauvegarde le fichier, on défait le correctif à la main, on lance le test, puis on restaure :

cp src/Quota/QuotaChecker.php src/Quota/QuotaChecker.php.bak
# Undo the fix by hand, then check the mutated block is really there before running anything.
grep -c 'return $used > $limit;' src/Quota/QuotaChecker.php
vendor/bin/phpunit --filter testQuotaIsReachedAtLimit
cp src/Quota/QuotaChecker.php.bak src/Quota/QuotaChecker.php && rm src/Quota/QuotaChecker.php.bak
vendor/bin/phpunit --filter testQuotaIsReachedAtLimit

Trois pièges guettent cette manipulation :

• une mutation qui n'a pas pris — un sed qui ne trouve rien laisse le fichier intact et le test vert, ce qui « prouve » l'inverse de la réalité : d'où le grep avant de lancer le test
• un échec pour une autre raison — le test doit tomber sur l'assertion qui garde le correctif, pas sur une erreur fatale ou une préparation qui plante
• une restauration par Git — git checkout -- fichier ramène la version commitée et efface au passage le correctif pas encore commité ; on restaure depuis la sauvegarde

Un test resté vert sous la mutation ne protège rien : il se retravaille avant d'être livré. C'est une forme ciblée de ce que les approches « test d'abord » obtiennent par construction, et que j'ai comparées dans un article sur TDD, BDD et tests a posteriori.

Le droit de ne pas livrer

Le même plugin enchaîne tout le chemin d'un ticket de correction : lecture du ticket et de ses pièces jointes, investigation dans le code, correctif, test, contrôles qualité, commit, MR et commentaire sur le ticket. Fait à la main, ce circuit était estimé à 20 à 40 minutes d'allers-retours entre outils ; sur un ticket simple, la commande le parcourt en quelques minutes. Mais le gain de temps n'est pas l'essentiel.

Ce qui compte, c'est la décision prise à mi-parcours. La commande énonce la cause racine la plus probable, puis se déclare confiante ou non. Elle n'est confiante que si la cause est prouvée par le code, citée en chemin:ligne, et que le correctif est circonscrit. Dans ce cas seulement, elle ouvre une MR, toujours en Draft : une proposition, jamais fusionnable sans relecture. Dans tous les autres cas, preuves minces, reproduction manquante, problème de droits d'accès ou de configuration, arbitrage produit nécessaire, correctif trop large, elle commente ses pistes sur le ticket, classées par probabilité, avec les raisons pour lesquelles elle préfère ne pas ouvrir de MR. Et elle ne modifie aucun fichier.

Un exemple typique : un bouton disparaît pour un rôle donné, chez un seul client. Le code partagé est correct ; c'est un privilège scindé en deux lors d'une refonte des droits, sans que la surcharge de configuration propre à ce client suive. Le diagnostic, adossé à la ligne de configuration et au commit fautif, justifie un correctif d'une ligne. Le même symptôme, sans repro et sans ligne fautive identifiée, aboutit à un commentaire de pistes, et c'est la bonne issue.

J'appelle cela une honnêteté câblée. Un outil qu'on évalue au nombre de MR ouvertes finit par inventer un bug pour avoir quelque chose à livrer. Un outil qui a le droit explicite de s'arrêter, et une définition précise de ce qui l'autorise à continuer, produit moins de MR, mais des MR qu'on peut relire en confiance. C'est la même discipline que le barème appliqué à la revue : écrire moins, mais seulement ce qui tient. La construction de ce type de plugin, versionné et partagé par toute une équipe, fait l'objet d'un article dédié : industrialiser les conventions d'équipe avec un plugin Claude Code.

Conclusion

Une revue de code par IA n'est pas mauvaise parce qu'elle se trompe, mais parce qu'elle ne sait pas s'arrêter. Le remède tient en peu de règles : un test qui décide si un point vaut un cycle de CI, des références numérotées et citables, une publication proportionnée à la sévérité, une mémoire des décisions portée par la MR elle-même, et des preuves exigées là où le code généré a tendance à se contenter des apparences : le ticket lu, le test vu rouge, la cause citée à la ligne près. Le modèle reste le même ; c'est le cadre qui rend sa revue actionnable.