Quand le réviseur de code AI se trompe avec assurance

J’ai mergé 3 PRs la semaine dernière. Le réviseur AI a généré 7 fils de discussion sur l’ensemble des trois.

5 étaient des améliorations valides. Des vérifications de nullité que j’avais manquées. Un pattern d’assertion plus propre. Un formatage cohérent sur un utilitaire de test. J’ai accepté les 5 sans hésiter.

2 auraient cassé tous les tests en CI. J’ai rejeté les deux.

Le ratio — 5 bons, 2 dangereux — résume toute l’histoire de la revue de code AI dans l’infrastructure de test. La plupart des suggestions sont correctes. Celles qui ne le sont pas sont catastrophiques. Et l’AI présente les deux avec une confiance identique.

Le pattern : supprimer les gardes « inutiles »

Les deux suggestions rejetées faisaient la même chose. Elles identifiaient des gardes défensives dans le code de test et les signalaient comme des conditions inutiles pouvant être resserrées.

Voici une version anonymisée de ce que l’AI voyait :

// The original test code
test('verify user profile displays correctly', async ({ page }) => {
  await page.goto('/profile');

  // Check if the premium badge exists before asserting on it
  const premiumBadge = page.getByTestId('premium-badge');
  if (await premiumBadge.isVisible({ timeout: 3000 })) {
    await expect(premiumBadge).toHaveText(/Premium|Enterprise/);
  }

  // Always verify the core profile elements
  await expect(page.getByRole('heading', { name: /profile/i })).toBeVisible();
  await expect(page.getByTestId('user-email')).toBeVisible();
});

La suggestion de l’AI :

// AI suggestion: "Unnecessary conditional — the premium badge should
// always exist on the profile page. Remove the conditional check
// and assert directly."
test('verify user profile displays correctly', async ({ page }) => {
  await page.goto('/profile');

  // ❌ AI removed the guard — this breaks in staging and dev environments
  const premiumBadge = page.getByTestId('premium-badge');
  await expect(premiumBadge).toBeVisible();
  await expect(premiumBadge).toHaveText(/Premium|Enterprise/);

  await expect(page.getByRole('heading', { name: /profile/i })).toBeVisible();
  await expect(page.getByTestId('user-email')).toBeVisible();
});

La logique de l’AI est propre : si le test navigue vers une page de profil, le badge premium devrait exister. La condition est une complexité inutile. Resserrez l’assertion.

Correct — en production. Complètement faux en staging, où le badge premium est derrière un feature flag activé uniquement dans l’environnement de production. Complètement faux en dev, où les données de test n’incluent pas de comptes premium. Complètement faux pendant les fenêtres de déploiement, où le service de feature flag met jusqu’à 30 secondes à se propager après un déploiement.

Pourquoi l’AI se trompe

L’AI examine le code de manière isolée. Elle voit le fichier de test. Elle voit peut-être le composant testé. Elle ne voit pas — et ne peut pas voir — les éléments suivants :

1. L’exécution multi-environnement. Ce test s’exécute dans 3 environnements (dev, staging, production) avec des états de feature flag différents, une disponibilité des données différente et un timing de déploiement différent.

2. Les transitoires de déploiement. Pendant un déploiement progressif, le service de feature flag peut mettre 30 secondes à se synchroniser. Un test qui s’exécute pendant cette fenêtre voit un état différent d’un test qui s’exécute 60 secondes plus tard.

3. La variabilité des données de test. L’environnement de staging utilise des snapshots anonymisés de données de production, rafraîchis chaque semaine. Tous les utilisateurs du snapshot ne sont pas des utilisateurs premium.

4. La conception défensive intentionnelle. La garde n’est pas là parce que le développeur était paresseux. Elle est là parce que le test doit passer dans les trois environnements. La garde est la fonctionnalité.

L’AI voit une condition et pense « code smell ». Un ingénieur qui a été réveillé à 2 heures du matin par un échec CI en staging voit une condition et pense « quelqu’un l’a appris à ses dépens ».

La deuxième suggestion rejetée

L’autre suggestion dangereuse était le même pattern appliqué à une valeur de timeout :

// Original: defensive timeout for slow staging environment
await expect(page.getByTestId('dashboard-loaded')).toBeVisible({
  timeout: 15000, // Staging can take up to 12s after deploy
});

// AI suggestion: "Timeout of 15000ms is excessive. The default 5000ms
// should be sufficient for a visibility check. Reduce to default."
await expect(page.getByTestId('dashboard-loaded')).toBeVisible();

L’AI a raison de dire que 15 secondes est inhabituel pour une vérification de visibilité. L’AI ne sait pas que l’environnement de staging tourne sur une infrastructure plus petite, partage une base de données avec l’environnement QA, et met régulièrement 8 à 12 secondes pour afficher le tableau de bord après un nouveau déploiement.

Réduire le timeout à la valeur par défaut de 5 secondes ferait échouer le test en staging à chaque déploiement. Il passerait en local. Il passerait en production. Il échouerait dans le seul environnement où il est le plus important pour détecter les régressions avant qu’elles n’atteignent les utilisateurs.

L’arbre de décision Accepter / Valider / Rejeter

Après avoir rencontré ce pattern plusieurs fois, j’ai construit un cadre de décision pour trier les suggestions de revue de code AI sur l’infrastructure de test :

Accepter immédiatement

Ces suggestions sont presque toujours sûres à prendre sans validation supplémentaire :

• Améliorations de la sécurité null. Ajout de chaînage optionnel, de vérifications null ou de narrowing de type. Cela rend les tests plus robustes dans tous les environnements, pas moins.
• Améliorations de typage. Resserrer les types, supprimer les any, ajouter des types de retour explicites. Améliorations pures de correction sans impact comportemental.
• Formatage et nommage. Conventions de nommage cohérentes, ordre des imports, clarté des descriptions de test.
• Clarté des assertions. Passer de toBeTruthy() à toBeVisible(), utiliser des matchers plus spécifiques. Ce sont de meilleures assertions qui passent quand même dans tous les environnements.

// AI suggestion: use more specific matcher ✅ Accept
// Before
await expect(submitButton).toBeTruthy();
// After
await expect(submitButton).toBeEnabled();

Valider avant d’accepter

Ces suggestions pourraient être correctes, mais nécessitent une vérification manuelle par rapport au comportement multi-environnement :

• Resserrement des assertions. Changer des matchers lâches en matchers stricts. Correct en théorie, mais pourrait échouer dans des environnements avec des données différentes.
• Suppression de gardes. Supprimer des vérifications conditionnelles, du chaînage optionnel sur des assertions, ou des chemins de repli. La garde existe peut-être pour une raison que l’AI ne peut pas voir.
• Modifications de timeout. Réduire les timeouts, changer les intervalles de polling, supprimer les attentes explicites. Ceux-ci sont sensibles à l’environnement par nature.
• Modifications de locators. Passer de getByTestId à getByRole. Meilleure pratique en général, mais le locator basé sur le rôle pourrait correspondre à des éléments différents selon les environnements avec des états de feature flag différents.

// AI suggestion: tighten assertion ⚠️ Validate first
// Before: loose text matching
await expect(page.getByTestId('status')).toContainText('complete');
// After: exact text matching
await expect(page.getByTestId('status')).toHaveText('Complete');

// Is the text exactly "Complete" in all environments?
// Or does staging show "complete" (lowercase) or "COMPLETE" (uppercase)?
// Check before accepting.

Rejeter

Ces suggestions reposent sur des hypothèses qui ne tiennent pas dans une infrastructure de test multi-environnement :

• Tout ce qui suppose une exécution dans un seul environnement. Si la suggestion ne fonctionne que quand vous imaginez le test tournant en local, rejetez-la.
• Suppression de logique conditionnelle tenant compte de l’environnement. Les gardes qui vérifient l’existence d’un élément avant d’asserter sur des fonctionnalités spécifiques à l’environnement.
• Suppression de logique de retry pour les opérations sensibles au déploiement. Si un test fait un retry parce que le déploiement pourrait être en cours, ce retry est structurant.

// AI suggestion: remove retry ❌ Reject
// Before: retry loop for post-deploy state
await expect(async () => {
  await page.reload();
  await expect(page.getByTestId('new-feature')).toBeVisible();
}).toPass({ intervals: [1000, 2000, 5000], timeout: 30000 });

// After (AI): "Unnecessary retry — just assert directly"
await expect(page.getByTestId('new-feature')).toBeVisible();

// The retry exists because this test runs against a deployment that
// takes up to 30 seconds to propagate. The AI has never seen a deploy.

La règle du 5-sur-7

Voici l’heuristique pratique : dans un lot de suggestions de revue de code AI sur l’infrastructure de test, environ 5 sur 7 sont des améliorations valides. Les 2 restantes se trompent avec assurance sur quelque chose que l’AI ne peut pas observer.

Le ratio n’est pas fixe. Il varie selon la complexité de la codebase et le nombre d’environnements ciblés par vos tests. Mais le pattern est constant : la majorité des suggestions sont sûres, et une minorité casserait votre pipeline de manières qui ne se manifestent qu’en dehors du contexte observable par l’AI.

Ce ratio est ce qui rend la revue de code AI à la fois précieuse et dangereuse. Si la plupart des suggestions étaient fausses, vous les ignoreriez entièrement. Si toutes les suggestions étaient justes, vous les mergeriez automatiquement. Le ratio 5-sur-7 signifie que vous devez évaluer chacune individuellement — et le coût d’accepter les deux mauvaises est disproportionnellement élevé.

Une checklist de triage pour la revue AI

Quand un réviseur AI suggère des modifications à votre infrastructure de test, passez en revue ces points :

1. Cette suggestion change-t-elle le comportement selon les environnements ? Si le test s’exécute dans plusieurs environnements avec des données, des flags ou des états de déploiement différents, toute suggestion qui resserre les assertions ou supprime des gardes nécessite une validation manuelle.

2. Est-ce une modification de timeout ? Les valeurs de timeout dans l’infrastructure de test ne sont presque jamais arbitraires. Elles encodent une connaissance des caractéristiques de performance spécifiques à l’environnement. Demandez-vous pourquoi la valeur actuelle existe avant de la modifier.

3. Est-ce que cela supprime une condition ? Les conditions dans le code de test qui vérifient l’existence d’un élément avant d’asserter sont souvent des gardes tenant compte de l’environnement, pas des code smells. Vérifiez que l’élément existe dans tous les environnements avant de supprimer la vérification.

4. Cette suggestion survivrait-elle à un déploiement en staging ? Rejouez mentalement la suggestion dans votre environnement le plus lent, le plus limité en ressources, pendant un déploiement progressif. Si elle échouerait, rejetez-la.

5. Puis-je vérifier cela sans passer par la production ? Si la seule façon de valider la suggestion est de la merger et de voir si la CI casse en staging, c’est un signal fort pour la rejeter.

La leçon fondamentale

La revue de code AI est un outil. Un bon outil. Les 5 suggestions que j’ai acceptées la semaine dernière ont apporté de véritables améliorations à la qualité du code — des améliorations que j’aurais pu manquer en revue manuelle. Je ne plaide pas contre la revue de code AI.

Je plaide contre le fait de traiter la revue de code AI comme une autorité de confiance sur une infrastructure qu’elle n’a jamais observée. L’AI analyse le code. Elle n’analyse pas les environnements dans lesquels ce code s’exécute, les états de déploiement dans lesquels ces environnements peuvent se trouver, ni le contexte historique expliquant pourquoi une garde particulière existe.

Le rôle du réviseur humain dans un processus de revue assisté par l’AI n’est pas de valider les suggestions les yeux fermés. C’est d’être le pont de contexte — celui qui sait que le staging met 12 secondes à s’afficher, que le feature flag se propage lentement, et que la vérification conditionnelle à la ligne 47 est là parce que quelqu’un l’a appris à ses dépens.

Ne désactivez pas la revue de code AI. Arrêtez simplement de la traiter comme infaillible. Cinq sur sept. Acceptez les cinq. Attrapez les deux. C’est la discipline qui maintient votre pipeline au vert.