Revisar una pull request contra el ticket
La última puerta antes de que una pull request esté lista. Responde una pregunta: ¿este diff entrega el ticket, dentro de la arquitectura? — y la responde contra dos fuentes que están por encima del gusto del revisor: los criterios de aceptación y la arquitectura de referencia.
Esta revisión lee e informa. No edita: una revisión que arregla lo que encuentra destruye la evidencia de lo que estaba mal, y nadie aprende el patrón.
Última de tres fases, después de TICKET-ANALYSIS.md y
IMPLEMENTATION-PHASE.md. WORKFLOW.md es cómo
encajan entre sí y cómo instalarlos.
Contenido
- Establecer qué se está revisando realmente
- Primero los criterios de aceptación
- La pasada arquitectónica
- La pasada de pruebas
- Gravedad y veredicto
- Modos de fallo
- Lista de verificación
1. Establecer el diff
Haz diff de la rama contra la rama base real del repositorio, no contra una suposición sobre cuál es esa rama. Revisa cada archivo cambiado; un archivo saltado por parecer aburrido es donde se esconde el cambio de configuración.
Anota lo que el diff toca que el ticket no mencionaba. Esa lista es una entrada para las secciones §2 y §5, no un hallazgo aún.
2. Primero los criterios de aceptación
Toma los criterios de aceptación uno por uno y, para cada uno, nombra el código que lo satisface y la prueba que lo demuestra. Ambos, o no está satisfecho.
- Un criterio con código y sin prueba no está demostrado.
- Un criterio con una prueba y sin código es una prueba que verifica el marco de trabajo.
- Un criterio que no puedes localizar en absoluto es el hallazgo más importante, y supera cualquier observación de estilo en la revisión.
Luego la otra dirección: cambios sin ningún criterio detrás. Algunos son legítimos —
una corrección dentro del código que ya se está cambiando (IMPLEMENTATION-PHASE.md §2). El resto es
ampliación del alcance, y es más barato nombrarlo aquí que explicarlo después del lanzamiento.
3. La pasada arquitectónica
Recorre la lista de verificación de cumplimiento para las capas que toca el diff. Los hallazgos que más valen son aquellos que una compilación exitosa no puede mostrar:
- P2 — cualquier cosa con forma de dominio añadida al núcleo compartido: una entidad, DTO, enumeración, conjunto de datos semilla, constante de precio, cadena visible para el usuario.
- P3 — una lectura o escritura entre contextos; un segundo servicio accediendo a la base de datos del servicio propietario.
- P4 — esquema cambiado sin migración, o una migración que no es portátil entre proveedores.
- P5 — un secreto, clave o cadena de conexión en código fuente, configuración o un comentario; configuración leída desde un lugar que no sea el entorno.
- P8 — una nueva dependencia que falla al iniciar en lugar de degradarse, o una ausente del endpoint de salud y del banner de inicio.
- P9 / P10 — cableado en línea en
Program.cs; un punto de extensión añadido como clase base en lugar de una interfaz registrada en DI. - P11 — una forma externa que llega al dominio sin traducir.
- P15 — una nueva ruta sin trazas, métricas o registros.
SERVICE-API-PATTERNS.md— un endpoint fuera de los grupos de confianza, una lista sin límite, un cuerpo de error no uniforme, una llamada saliente sin un tiempo de espera explícito, una escritura indeterminada reintentada.- Seguridad — cualquier cosa recién expuesta públicamente recibe la pasada de
SECURITY-REVIEW.mdantes de ser publicada, no después.
Una desviación que la pull request registra como una decisión con una razón no es un hallazgo. La misma desviación, en silencio, sí lo es — esa distinción es P14 haciendo su trabajo.
4. La pasada de pruebas
Lee los cuerpos de las pruebas. El número de pruebas, número de archivos y estructura de carpetas no prueban nada; una auditoría del 14 de agosto de 2026 a un conjunto de 447 pruebas encontró aproximadamente el 45% eran marcadores de posición o estaban protegidos para no afirmar nunca nada, y nada de eso era visible desde el exterior (P13).
- ¿Está cada nueva prueba en la capa que contiene la lógica, o un navegador está suplantando a un validador?
- ¿Afirma un resultado, o que se hizo clic en un botón?
- ¿Hay una prueba de regresión para cada ruta existente que toca el diff, o solo pruebas para la nueva rama?
- ¿Cumple el criterio por prueba — independiente,
data-testid, sin esperas temporales, sin credenciales de producción (TESTING-STRATEGY.md§6)? - ¿Algún nuevo punto de entrada de prueba es ejecutado por un contexto de CI (§9)?
5. Gravedad y veredicto
Clasifica cada hallazgo, y clasifícalo por consecuencia en lugar de por lo fácil que sea describirlo:
| Nivel | Qué significa |
|---|---|
| Bloqueante | Un criterio de aceptación no se cumple ni se demuestra; se viola un principio sin una decisión registrada; un secreto está expuesto; una prueba fue debilitada o eliminada para que saliera verde |
| Debería corregirse | Correcto pero fuera del patrón: capa de prueba incorrecta, cobertura de regresión faltante, una suposición no registrada, documentación que el cambio volvió falsa |
| Considerar | Verdaderamente opcional. Mantén esta sección corta — una larga entierra las dos primeras |
Cierra con el veredicto que necesita la puerta del diagrama, expresado claramente: ¿esta pull request entrega el ticket y está dentro de la arquitectura? Una de tres respuestas — validada; validada con los hallazgos no bloqueantes listados; o no validada, con los hallazgos bloqueantes nombrados. "Se ve bien, algunos comentarios" no es un veredicto.
6. Modos de fallo
| Síntoma | Causa |
|---|---|
| La revisión aprueba un diff que no cumple un requisito | La pasada comenzó por el código en lugar de por los criterios de aceptación |
| La revisión es una lista de opiniones de estilo | Las pasadas arquitectónica y de pruebas se omitieron porque la compilación estaba verde — que es exactamente lo que verifican |
| Una suite verde se trata como evidencia | Los cuerpos de las pruebas nunca se leyeron; se confió en el conteo y la estructura |
| Todos los hallazgos se reportan con el mismo peso | Sin clasificación de gravedad, por lo que el hallazgo bloqueante queda debajo de un detalle de nomenclatura |
| La misma violación se repite entre pull requests | Los hallazgos fueron corregidos por el revisor en lugar de reportados, por lo que el patrón nunca se aprendió |
| Se reporta una desviación que el autor ya había justificado | Las decisiones registradas de la pull request no se leyeron antes de revisar |
7. Lista de verificación
- Diff realizado contra la rama base real del repositorio; cada archivo cambiado leído
- Cada criterio de aceptación mapeado al código que lo satisface y la prueba que lo demuestra
- Cambios sin ningún criterio detrás identificados y clasificados
- Lista de verificación de cumplimiento recorrida para cada capa que toca el diff
- Decisiones registradas leídas primero, para que las desviaciones justificadas no se reporten como hallazgos
- Cualquier cosa recién pública sometida a la pasada de revisión de seguridad
- Cuerpos de las pruebas leídos, no contados; capa, afirmaciones de resultado, cobertura de regresión y el criterio por prueba verificados
- Hallazgos clasificados como bloqueante / debería corregirse / considerar
- Veredicto explícito dado contra el ticket
- Nada editado durante la revisión
Generado desde docs/delivery/PR-REVIEW.md por scripts/build-marketplace.mjs. No edites este archivo: cambia el documento fuente, o su entrada en catalog/marketplace.catalog.json, y vuelve a ejecutar el generador.