Cuando el revisor de codigo con AI esta equivocado con total confianza

La semana pasada hice merge de 3 PRs. El revisor de AI genero 7 hilos de revision entre los tres.

5 fueron mejoras validas. Checks de null safety que se me habian pasado. Un patron de assertion mas limpio. Formato consistente en una utilidad de pruebas. Acepte los 5 sin dudarlo.

2 habrian roto todas las pruebas en CI. Rechace ambos.

La proporcion — 5 buenos, 2 peligrosos — resume toda la historia del AI code review en infraestructura de pruebas. La mayoria de las sugerencias estan bien. Las que no, son catastroficas. Y el AI presenta ambas con la misma confianza.

El patron: eliminar guardas “innecesarias”

Ambas sugerencias rechazadas hacian lo mismo. Identificaban guardas defensivas en el codigo de pruebas y las marcaban como condicionales innecesarios que podian simplificarse.

Aqui hay una version simplificada de lo que el AI vio:

// 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 sugerencia del 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 logica del AI es limpia: si la prueba navega a una pagina de perfil, el badge premium deberia existir. El condicional es complejidad innecesaria. Hay que hacer el assertion mas estricto.

Correcto — en produccion. Completamente equivocado en staging, donde el badge premium esta detras de un feature flag que solo esta habilitado en el ambiente de produccion. Completamente equivocado en dev, donde los datos de prueba no incluyen cuentas premium. Completamente equivocado durante las ventanas de deploy, donde el servicio de feature flags tarda hasta 30 segundos en propagarse despues del despliegue.

Por que el AI se equivoca en esto

El AI revisa el codigo de forma aislada. Ve el archivo de pruebas. Tal vez ve el componente que se esta probando. No ve — y no puede ver — lo siguiente:

1. Ejecucion en multiples ambientes. Esta prueba se ejecuta en 3 ambientes (dev, staging, produccion) con diferentes estados de feature flags, diferente disponibilidad de datos y diferente timing de despliegue.

2. Transitorios durante el deploy. Durante un rolling deployment, el servicio de feature flags puede tardar 30 segundos en sincronizarse. Una prueba que se ejecuta durante esa ventana ve un estado diferente al de una prueba que se ejecuta 60 segundos despues.

3. Variabilidad en los datos de prueba. El ambiente de staging usa snapshots anonimizados de datos de produccion que se refrescan semanalmente. No todos los usuarios en el snapshot son usuarios premium.

4. Diseno defensivo intencional. La guarda no esta ahi porque el desarrollador fue flojo. Esta ahi porque la prueba necesita pasar en los tres ambientes. La guarda es la funcionalidad.

El AI ve un condicional y piensa “code smell”. Un ingeniero que ha sido despertado a las 2 AM por una falla de CI en staging ve un condicional y piensa “alguien aprendio por las malas”.

La segunda sugerencia rechazada

La otra sugerencia peligrosa era el mismo patron aplicado a un valor 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();

El AI tiene razon en que 15 segundos es inusual para un check de visibilidad. Lo que el AI no sabe es que el ambiente de staging corre en infraestructura mas pequena, comparte base de datos con el ambiente de QA y regularmente tarda de 8 a 12 segundos en renderizar el dashboard despues de un despliegue reciente.

Reducir el timeout a los 5 segundos por defecto haria que la prueba falle en staging en cada deploy. Pasaria localmente. Pasaria en produccion. Fallaria en el unico ambiente donde mas importa detectar regresiones antes de que lleguen a los usuarios.

El arbol de decision: Aceptar / Validar / Rechazar

Despues de encontrarme con este patron varias veces, construi un framework de decision para clasificar las sugerencias de AI code review en infraestructura de pruebas:

Aceptar de inmediato

Estas sugerencias casi siempre son seguras de tomar sin validacion adicional:

• Mejoras de null safety. Agregar optional chaining, null checks o type narrowing. Estas hacen las pruebas mas robustas en todos los ambientes, no menos.
• Mejoras de tipos. Hacer los tipos mas estrictos, eliminar any, agregar return types explicitos. Mejoras de correctitud pura sin impacto en el comportamiento.
• Formato y nombres. Convenciones de nombres consistentes, orden de imports, claridad en las descripciones de pruebas.
• Claridad en assertions. Cambiar de toBeTruthy() a toBeVisible(), usar matchers mas especificos. Son mejores assertions que siguen pasando en todos los ambientes.

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

Validar antes de aceptar

Estas sugerencias podrian ser correctas, pero necesitan verificacion manual contra el comportamiento en multiples ambientes:

• Assertions mas estrictos. Cambiar matchers flexibles por matchers estrictos. Correcto en teoria, pero podria fallar en ambientes con datos diferentes.
• Eliminacion de guardas. Remover checks condicionales, optional chaining en assertions o rutas de respaldo. La guarda podria existir por una razon que el AI no puede ver.
• Cambios de timeout. Reducir timeouts, cambiar intervalos de polling, eliminar waits explicitos. Estos son sensibles al ambiente por naturaleza.
• Cambios de locators. Cambiar de getByTestId a getByRole. Mejor practica en general, pero el locator basado en roles podria coincidir con elementos diferentes en ambientes con distintos estados de feature flags.

// 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.

Rechazar

Estas sugerencias se basan en suposiciones que no se cumplen en infraestructura de pruebas con multiples ambientes:

• Cualquier cosa que asuma ejecucion en un solo ambiente. Si la sugerencia solo funciona cuando imaginas la prueba corriendo localmente, rechazala.
• Eliminar logica condicional que reconoce el ambiente. Guardas que verifican la existencia de elementos antes de hacer assertions sobre funcionalidades especificas del ambiente.
• Eliminar logica de reintentos para operaciones sensibles al deploy. Si una prueba reintenta porque el despliegue podria estar en progreso, ese reintento es fundamental.

// 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 regla del 5 de 7

Esta es la heuristica practica: en un lote de sugerencias de AI code review sobre infraestructura de pruebas, aproximadamente 5 de cada 7 son mejoras validas. Las 2 restantes estan equivocadas con total confianza sobre algo que el AI no puede observar.

La proporcion no es fija. Varia segun la complejidad del codebase y cuantos ambientes cubren tus pruebas. Pero el patron es consistente: la mayoria de las sugerencias son seguras, y una minoria romperia tu pipeline de maneras que solo se manifiestan fuera del contexto observable del AI.

Esta proporcion es lo que hace al AI code review tanto valioso como peligroso. Si la mayoria de las sugerencias estuvieran mal, las ignorarias por completo. Si todas estuvieran bien, les harias auto-merge. La division de 5 de 7 significa que debes evaluar cada una individualmente — y el costo de aceptar las dos incorrectas es desproporcionadamente alto.

Checklist para clasificar revisiones de AI

Cuando un revisor de AI sugiere cambios en tu infraestructura de pruebas, revisa lo siguiente:

1. Esta sugerencia cambia el comportamiento entre ambientes? Si la prueba se ejecuta en multiples ambientes con datos, flags o estados de deploy diferentes, cualquier sugerencia que haga los assertions mas estrictos o elimine guardas necesita validacion manual.

2. Es un cambio de timeout? Los valores de timeout en infraestructura de pruebas casi nunca son arbitrarios. Codifican conocimiento sobre las caracteristicas de rendimiento especificas de cada ambiente. Pregunta por que existe el valor actual antes de cambiarlo.

3. Esto elimina un condicional? Los condicionales en codigo de pruebas que verifican la existencia de elementos antes de hacer assertions frecuentemente son guardas que reconocen el ambiente, no code smells. Verifica que el elemento exista en todos los ambientes antes de eliminar el check.

4. Esta sugerencia sobreviviria un deploy en staging? Reproduce mentalmente la sugerencia contra tu ambiente mas lento y con menos recursos durante un rolling deployment. Si fallaria, rechazala.

5. Puedo verificar esto sin correr en produccion? Si la unica forma de validar la sugerencia es hacer merge y ver si CI falla en staging, eso es una senal fuerte para rechazar.

La leccion de fondo

El AI code review es una herramienta. Una buena. Las 5 sugerencias que acepte la semana pasada hicieron mejoras genuinas a la calidad del codigo — mejoras que podria haber pasado por alto en una revision manual. No estoy argumentando en contra del AI code review.

Estoy argumentando en contra de tratar al AI code review como una autoridad confiable sobre infraestructura que nunca ha observado. El AI analiza codigo. No analiza los ambientes en los que ese codigo se ejecuta, los estados de deploy en los que esos ambientes pueden estar, ni el contexto historico de por que existe una guarda en particular.

El trabajo del revisor humano en un proceso de revision asistido por AI no es aprobar las sugerencias sin pensar. Es ser el puente de contexto — la persona que sabe que staging tarda 12 segundos en renderizar, que el feature flag se propaga lentamente y que el check condicional en la linea 47 esta ahi porque alguien aprendio por las malas.

No apagues el AI code review. Solo deja de tratarlo como infalible. Cinco de siete. Acepta los cinco. Atrapa los dos. Esa es la disciplina que mantiene tu pipeline en verde.