När AI-kodgranskaren har självsäkert fel

Erik Treviño avatar
Erik Treviño
Cover for När AI-kodgranskaren har självsäkert fel

Jag mergade 3 PR:er förra veckan. AI-granskaren genererade 7 granskningstrådar över alla tre.

5 var giltiga förbättringar. Null safety-kontroller jag hade missat. Ett renare assertion-mönster. Konsekvent formatering av ett testverktyg. Jag accepterade alla 5 utan att tveka.

2 hade sönder alla tester i CI. Jag avvisade båda.

Förhållandet — 5 bra, 2 farliga — är hela historien om AI code review i testinfrastruktur. De flesta förslagen är bra. De som inte är det är katastrofala. Och AI:n presenterar båda med identisk självsäkerhet.

Mönstret: Att ta bort “onödiga” skyddsmekanismer

Båda avvisade förslagen gjorde samma sak. De identifierade defensiva skyddsmekanismer i testkod och flaggade dem som onödiga villkor som kunde stramas åt.

Här är en anonymiserad version av vad AI:n såg:

// 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();
});

AI:ns förslag:

// 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();
});

AI:ns logik är ren: om testet navigerar till en profilsida borde premium-märket finnas. Villkoret är onödig komplexitet. Strama åt assertionen.

Korrekt — i produktion. Helt fel i staging, där premium-märket ligger bakom en feature flag som bara är aktiverad i produktionsmiljön. Helt fel i dev, där testdatan inte innehåller premiumkonton. Helt fel under deploy-fönster, där feature flag-tjänsten tar upp till 30 sekunder att propagera efter deployment.

Varför AI:n har fel

AI:n granskar kod isolerat. Den ser testfilen. Den kanske ser komponenten som testas. Den ser inte — och kan inte se — följande:

  1. Exekvering i flera miljöer. Det här testet körs i 3 miljöer (dev, staging, produktion) med olika feature flag-tillstånd, olika datatillgänglighet och olika deployment-timing.

  2. Transienter vid deployment. Under en rullande deployment kan feature flag-tjänsten ta 30 sekunder att synkronisera. Ett test som körs under det fönstret ser ett annat tillstånd än ett test som körs 60 sekunder senare.

  3. Variation i testdata. Staging-miljön använder anonymiserade ögonblicksbilder av produktionsdata som uppdateras veckovis. Inte varje användare i ögonblicksbilden är en premiumanvändare.

  4. Medveten defensiv design. Skyddsmekanismen finns inte där för att utvecklaren var lat. Den finns där för att testet måste passera i alla tre miljöer. Skyddsmekanismen är funktionen.

AI:n ser ett villkor och tänker “code smell.” En ingenjör som blivit väckt klockan 2 på natten av ett CI-fel i staging ser ett villkor och tänker “någon lärde sig den hårda vägen.”

Det andra avvisade förslaget

Det andra farliga förslaget var samma mönster applicerat på ett timeout-värde:

// 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();

AI:n har rätt i att 15 sekunder är ovanligt för en synlighetskontroll. AI:n vet inte att staging-miljön körs på mindre infrastruktur, delar databas med QA-miljön och regelbundet tar 8-12 sekunder att rendera dashboarden efter en ny deployment.

Att sänka timeouten till standardvärdet 5 sekunder skulle få testet att misslyckas i staging vid varje deploy. Det skulle passera lokalt. Det skulle passera i produktion. Det skulle misslyckas i den enda miljön där det är viktigast att fånga regressioner innan de når användarna.

Beslutsträdet Acceptera / Validera / Avvisa

Efter att ha stött på det här mönstret flera gånger byggde jag ett beslutsramverk för att triagera AI code review-förslag på testinfrastruktur:

Acceptera omedelbart

Dessa förslag är nästan alltid säkra att ta utan vidare validering:

  • Null safety-förbättringar. Att lägga till optional chaining, null-kontroller eller typförfining. Dessa gör tester mer robusta över miljöer, inte mindre.
  • Typförbättringar. Att strama åt typer, ta bort any, lägga till explicita returtyper. Rena korrekthetsförbättringar utan beteendepåverkan.
  • Formatering och namngivning. Konsekventa namnkonventioner, importordning, tydligare testbeskrivningar.
  • Tydligare assertions. Att byta från toBeTruthy() till toBeVisible(), använda mer specifika matchers. Dessa är bättre assertions som fortfarande passerar i alla miljöer.
// AI suggestion: use more specific matcher ✅ Accept
// Before
await expect(submitButton).toBeTruthy();
// After
await expect(submitButton).toBeEnabled();

Validera före acceptans

Dessa förslag kan vara korrekta, men kräver manuell verifiering mot beteende i flera miljöer:

  • Åtstramning av assertions. Att byta från lösa matchers till strikta matchers. Korrekt i teorin, men kan misslyckas i miljöer med annan data.
  • Borttagning av skyddsmekanismer. Att ta bort villkorskontroller, optional chaining på assertions eller reservvägar. Skyddsmekanismen kan finnas av en anledning som AI:n inte kan se.
  • Timeout-ändringar. Att sänka timeouts, ändra polling-intervall, ta bort explicita väntan. Dessa är miljökänsliga till sin natur.
  • Locator-ändringar. Att byta från getByTestId till getByRole. Bättre praxis generellt, men den rollbaserade locatorn kan matcha olika element i miljöer med olika feature flag-tillstånd.
// 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.

Avvisa

Dessa förslag bygger på antaganden som inte stämmer i testinfrastruktur med flera miljöer:

  • Allt som antar exekvering i en enda miljö. Om förslaget bara fungerar om man tänker sig att testet körs lokalt, avvisa det.
  • Borttagning av miljömedveten villkorslogik. Skyddsmekanismer som kontrollerar om element finns innan de gör assertions på miljöspecifika funktioner.
  • Borttagning av retry-logik för deployment-känsliga operationer. Om ett test gör retry för att en deployment kan pågå, är den retryn bärande.
// 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.

5-av-7-regeln

Här är den praktiska tumregeln: i en batch med AI code review-förslag på testinfrastruktur är ungefär 5 av 7 giltiga förbättringar. De återstående 2 har självsäkert fel om något AI:n inte kan observera.

Förhållandet är inte fast. Det varierar med kodbaskomplexitet och hur många miljöer dina tester riktar sig mot. Men mönstret är konsekvent: majoriteten av förslagen är säkra, och en minoritet skulle knäcka din pipeline på sätt som bara manifesterar sig utanför AI:ns observerbara kontext.

Det här förhållandet är det som gör AI code review både värdefullt och farligt. Om de flesta förslagen var felaktiga skulle du ignorera dem helt. Om alla förslag var korrekta skulle du auto-mergea dem. 5-av-7-uppdelningen innebär att du måste utvärdera varje förslag individuellt — och kostnaden för att acceptera de felaktiga två är oproportionerligt hög.

En checklista för triagering av AI-granskningar

När en AI-granskare föreslår ändringar i din testinfrastruktur, gå igenom det här:

  1. Ändrar det här förslaget beteendet mellan miljöer? Om testet körs i flera miljöer med olika data, flaggor eller deployment-tillstånd behöver varje förslag som stramar åt assertions eller tar bort skyddsmekanismer manuell validering.

  2. Är det här en timeout-ändring? Timeout-värden i testinfrastruktur är nästan aldrig godtyckliga. De kodar kunskap om miljöspecifika prestandaegenskaper. Fråga varför det nuvarande värdet finns innan du ändrar det.

  3. Tar det här bort ett villkor? Villkor i testkod som kontrollerar om element finns innan de gör assertions är ofta miljömedvetna skyddsmekanismer, inte code smells. Verifiera att elementet finns i alla miljöer innan du tar bort kontrollen.

  4. Skulle det här förslaget överleva en staging-deploy? Spela mentalt upp förslaget mot din långsammaste, mest resursbegränsade miljö under en rullande deployment. Om det skulle misslyckas, avvisa det.

  5. Kan jag verifiera det här utan att köra i produktion? Om det enda sättet att validera förslaget är att mergea det och se om CI kraschar i staging, är det en stark signal att avvisa.

Den djupare lärdomen

AI code review är ett verktyg. Ett bra sådant. De 5 förslagen jag accepterade förra veckan förbättrade kodkvaliteten genuint — förbättringar jag kanske hade missat i manuell granskning. Jag argumenterar inte emot AI code review.

Jag argumenterar emot att behandla AI code review som en betrodd auktoritet på infrastruktur den aldrig har observerat. AI:n analyserar kod. Den analyserar inte miljöerna koden körs i, de deployment-tillstånd miljöerna kan befinna sig i, eller den historiska kontexten för varför en viss skyddsmekanism finns.

Den mänskliga granskarens uppgift i en AI-assisterad granskningsprocess är inte att bara godkänna förslag. Det är att vara kontextbryggan — den som vet att staging tar 12 sekunder att rendera, att feature flaggen propagerar långsamt, och att villkorskontrollen på rad 47 finns där för att någon lärde sig den hårda vägen.

Stäng inte av AI code review. Sluta bara behandla det som ofelbart. Fem av sju. Acceptera de fem. Fånga de två. Det är disciplinen som håller din pipeline grön.