Módulo 4: Code Review de Output AI
Ejercicio: Code Review de un PR Generado por AI
Ejercicio: Code Review de un PR Generado por AI
Descripción de la cápsula
Este es el ejercicio integrador del módulo. Has aprendido la pirámide de prioridades (cápsula 02), construido un checklist de 20 items (cápsula 03), dominado los 8 red flags de AI (cápsula 04), y practicado verificación de lógica de negocio (cápsula 05). Ahora aplicas todo en un code review real.
El escenario: un compañero de equipo usó Claude Code para generar un sistema de gestión de eventos para una plataforma de conferencias. El PR tiene ~130 líneas de código FastAPI con modelos, endpoints, y lógica de negocio. Tu trabajo es hacer code review profesional usando las herramientas de este módulo.
El código tiene una mezcla intencional de código bueno y 8 problemas distribuidos en las categorías de la pirámide. Algunos son obvios, otros son sutiles. Algunos son bugs, otros son red flags de AI. Tu objetivo es encontrar al menos 6 de los 8.
Contexto del PR
Requisitos del negocio
La plataforma de conferencias necesita un sistema de registro de eventos con estas reglas:
- Eventos tienen título, descripción, fecha, capacidad máxima, y precio
- Registro: los usuarios pueden registrarse a eventos que no estén llenos
- Pricing: eventos con precio > $0 requieren pago. Eventos gratuitos no requieren pago
- Cancelación: un usuario puede cancelar su registro hasta 24 horas antes del evento. El reembolso es del 80% (se retiene 20% como fee)
- Capacidad: cuando un evento llega a capacidad máxima, no se permiten más registros
- Lista de espera: no implementada en esta fase — simplemente rechazar si está lleno
PR Description (escrita por tu compañero)
## PR: Event Registration System
Generated with Claude Code. Implements event creation, registration,
and cancellation for the conference platform.
- Event CRUD endpoints
- Registration with capacity check
- Cancellation with refund calculation
- Pydantic models for validation
Tested manually — works for basic flow.
El Código del PR
Lee el siguiente código como si fuera un PR real. No mires las soluciones hasta completar tu review.
models.py
from pydantic import BaseModel, Field, validator
from typing import Optional, List
from datetime import datetime
from decimal import Decimal
from enum import Enum
import uuid
class EventStatus(str, Enum):
DRAFT = "draft"
PUBLISHED = "published"
CANCELLED = "cancelled"
COMPLETED = "completed"
class RegistrationStatus(str, Enum):
CONFIRMED = "confirmed"
CANCELLED = "cancelled"
WAITLISTED = "waitlisted"
class EventCreate(BaseModel):
title: str = Field(..., min_length=1, max_length=200)
description: Optional[str] = Field(None, max_length=5000)
event_date: datetime
capacity: int = Field(..., ge=1)
price: float = Field(default=0, ge=0)
@validator("event_date")
def event_must_be_future(cls, v):
if v < datetime.utcnow():
raise ValueError("Event date must be in the future")
return v
class Config:
orm_mode = True
class EventResponse(BaseModel):
id: str
title: str
description: Optional[str]
event_date: datetime
capacity: int
price: float
status: EventStatus
registered_count: int
created_at: datetime
class RegistrationResponse(BaseModel):
id: str
event_id: str
user_id: str
status: RegistrationStatus
amount_paid: float
registered_at: datetime
routes.py
from fastapi import FastAPI, HTTPException, Depends, Query
from datetime import datetime, timedelta
from decimal import Decimal
from typing import List, Optional
import uuid
import os
app = FastAPI(title="Conference Event API")
DB_CONNECTION = os.getenv("DATABASE_URL", "postgresql://admin:admin123@localhost/events")
events_db = {}
registrations_db = {}
async def get_current_user():
return {"id": "user-123", "name": "Test User", "email": "test@example.com"}
@app.post("/events", response_model=dict, status_code=201)
async def create_event(event: "EventCreate"):
from models import EventCreate, EventResponse, EventStatus
event_id = str(uuid.uuid4())
now = datetime.utcnow()
event_record = {
"id": event_id,
"title": event.title,
"description": event.description,
"event_date": event.event_date,
"capacity": event.capacity,
"price": event.price,
"status": EventStatus.PUBLISHED,
"registered_count": 0,
"created_at": now,
}
events_db[event_id] = event_record
return event_record
@app.get("/events", response_model=List[dict])
async def list_events(
status: Optional[str] = None,
min_price: Optional[float] = None,
max_price: Optional[float] = None,
):
from models import EventStatus
events = list(events_db.values())
if status:
events = [e for e in events if e["status"] == status]
if min_price is not None:
events = [e for e in events if e["price"] >= min_price]
if max_price is not None:
events = [e for e in events if e["price"] <= max_price]
return events
@app.post("/events/{event_id}/register")
async def register_for_event(
event_id: str,
current_user: dict = Depends(get_current_user),
):
from models import RegistrationStatus
event = events_db.get(event_id)
if not event:
raise HTTPException(status_code=404, detail="Event not found")
if event["registered_count"] > event["capacity"]:
raise HTTPException(status_code=400, detail="Event is full")
for reg in registrations_db.values():
if reg["event_id"] == event_id and reg["user_id"] == current_user["id"]:
raise HTTPException(
status_code=400, detail="Already registered"
)
registration_id = str(uuid.uuid4())
amount = event["price"]
registration = {
"id": registration_id,
"event_id": event_id,
"user_id": current_user["id"],
"status": RegistrationStatus.CONFIRMED,
"amount_paid": amount,
"registered_at": datetime.utcnow(),
}
registrations_db[registration_id] = registration
event["registered_count"] += 1
return registration
@app.post("/events/{event_id}/cancel-registration")
async def cancel_registration(
event_id: str,
current_user: dict = Depends(get_current_user),
):
from models import RegistrationStatus
registration = None
for reg in registrations_db.values():
if reg["event_id"] == event_id and reg["user_id"] == current_user["id"]:
registration = reg
break
if not registration:
raise HTTPException(
status_code=404, detail="Registration not found"
)
if registration["status"] == RegistrationStatus.CANCELLED:
raise HTTPException(
status_code=400, detail="Registration already cancelled"
)
event = events_db.get(event_id)
hours_until_event = (
event["event_date"] - datetime.utcnow()
).total_seconds() / 3600
if hours_until_event < 24:
raise HTTPException(
status_code=400,
detail="Cannot cancel less than 24 hours before event",
)
refund_amount = registration["amount_paid"] * 0.80
registration["status"] = RegistrationStatus.CANCELLED
registration["refund_amount"] = refund_amount
event["registered_count"] -= 1
return {
"message": "Registration cancelled",
"refund_amount": refund_amount,
}
@app.get("/events/{event_id}/registrations")
async def list_registrations(event_id: str):
event = events_db.get(event_id)
if not event:
raise HTTPException(status_code=404, detail="Event not found")
regs = [r for r in registrations_db.values() if r["event_id"] == event_id]
return regs
Tu Tarea
Instrucciones
- Lee el código completo como lo harías en un code review real
- Aplica la pirámide de prioridades (seguridad → lógica → edge cases → AI-specific → estilo)
- Usa el checklist de 20 items
- Documenta cada finding con este formato:
FINDING #N
- Archivo: [models.py / routes.py]
- Línea(s): [referencia aproximada]
- Categoría: [Seguridad / Lógica / Edge Case / AI-Specific / Calidad]
- Severidad: [Critical / High / Medium / Low]
- Descripción: [qué está mal]
- Impacto: [qué puede pasar]
- Corrección: [cómo arreglarlo]
- Al final, incluye:
- Resumen: ¿cuántos findings por categoría?
- Decisión: ¿aprobar, solicitar cambios, o rechazar?
- Tiempo invertido
Intenta encontrar al menos 6 de los 8 problemas antes de ver la solución.
Tu Espacio de Trabajo
Usa este espacio para documentar tu review antes de ver la solución:
MI CODE REVIEW
==============
Tiempo inicio: __:__
FINDINGS:
FINDING #1
- Archivo:
- Categoría:
- Severidad:
- Descripción:
- Corrección:
FINDING #2
- Archivo:
- Categoría:
- Severidad:
- Descripción:
- Corrección:
(continuar...)
RESUMEN:
- Seguridad: ___ findings
- Lógica: ___ findings
- Edge Cases: ___ findings
- AI-Specific: ___ findings
- Calidad: ___ findings
DECISIÓN: [Aprobar / Solicitar cambios / Rechazar]
TIEMPO INVERTIDO: ___ minutos
Solución Detallada
Ver solución completa (no abrir hasta completar tu review)
Los 8 problemas del PR
FINDING #1: Database URL con credenciales hardcoded
- Archivo: routes.py
- Línea: DB_CONNECTION = os.getenv("DATABASE_URL", "postgresql://admin:admin123@localhost/events")
- Categoría: Seguridad
- Severidad: Critical
- Descripción: El fallback del database URL contiene credenciales
(usuario: admin, password: admin123). Si DATABASE_URL no está
configurada en producción, la app se conecta con credenciales
de desarrollo que están en el código fuente.
- Impacto: Cualquiera con acceso al repo conoce las credenciales
de la base de datos. Si el fallback se activa en producción,
es acceso directo a la DB.
- Corrección:
DB_CONNECTION = os.environ["DATABASE_URL"] # Falla si no existe
FINDING #2: Fake authentication (no hay auth real)
- Archivo: routes.py
- Línea: async def get_current_user(): return {"id": "user-123"...}
- Categoría: Seguridad
- Severidad: Critical
- Descripción: get_current_user() está hardcoded — siempre devuelve
el mismo usuario fake. No hay autenticación real. Todos los
endpoints que usan Depends(get_current_user) son públicos.
- Impacto: Cualquiera puede registrarse, cancelar registros, y
acceder a datos de otros usuarios. El endpoint de list_registrations
ni siquiera usa auth — expone datos de todos los registros.
- Corrección: Implementar auth real con JWT o OAuth2. Como mínimo,
agregar un TODO visible y no hacer merge sin auth.
FINDING #3: Capacity check usa > en vez de >=
- Archivo: routes.py
- Línea: if event["registered_count"] > event["capacity"]:
- Categoría: Lógica de negocio
- Severidad: High
- Descripción: La comparación es > (mayor estricto) cuando
debería ser >= (mayor o igual). Si capacity = 50 y
registered_count = 50, la condición es False y permite
un registro #51.
- Impacto: Cada evento permite 1 registro más de la capacidad
máxima. En una conferencia con sala limitada, hay 1 persona
sin asiento.
- Corrección:
if event["registered_count"] >= event["capacity"]:
raise HTTPException(status_code=400, detail="Event is full")
FINDING #4: Refund calcula con float, no con Decimal
- Archivo: routes.py
- Línea: refund_amount = registration["amount_paid"] * 0.80
- Categoría: Lógica de negocio
- Severidad: Medium-High
- Descripción: El cálculo de reembolso usa float (0.80). Esto
es consistente con que price es float en el modelo, pero AMBOS
deberían ser Decimal. Con float: 19.99 * 0.80 = 15.991999...
que puede causar errores de centavos.
- Impacto: Errores de redondeo en reembolsos. Puede ser fracciones
de centavo, pero en volumen se acumula. También inconsistencia:
el módulo importa Decimal pero no lo usa para dinero.
- Corrección: Cambiar price a Decimal en el modelo y usar
Decimal("0.80") para el cálculo del reembolso.
# En models.py
price: Decimal = Field(default=Decimal("0"), ge=0)
# En routes.py
refund_amount = Decimal(str(registration["amount_paid"])) * Decimal("0.80")
FINDING #5: Pydantic v1 API (@validator, Config, orm_mode)
- Archivo: models.py
- Líneas: @validator, class Config, orm_mode = True
- Categoría: AI-Specific (Red Flag 2: APIs de versión anterior)
- Severidad: Medium
- Descripción: El código usa Pydantic v1 API:
- @validator → debería ser @field_validator (v2)
- class Config: orm_mode = True → debería ser
model_config = ConfigDict(from_attributes=True) (v2)
Si el proyecto usa Pydantic v2, esto causa deprecation
warnings y puede fallar en versiones futuras.
- Impacto: Funciona con Pydantic v2 (hay compatibilidad), pero
genera warnings. En Pydantic v3 (cuando llegue) dejará de
funcionar.
- Corrección:
from pydantic import BaseModel, Field, field_validator, ConfigDict
class EventCreate(BaseModel):
model_config = ConfigDict(from_attributes=True)
# ...fields...
@field_validator("event_date")
@classmethod
def event_must_be_future(cls, v: datetime) -> datetime:
from datetime import timezone
if v < datetime.now(timezone.utc):
raise ValueError("Event date must be in the future")
return v
FINDING #6: RegistrationStatus tiene "waitlisted" pero los requisitos dicen "no implementada"
- Archivo: models.py
- Línea: WAITLISTED = "waitlisted" en RegistrationStatus
- Categoría: AI-Specific (Red Flag 4: problema ligeramente diferente)
- Severidad: Low-Medium
- Descripción: Los requisitos dicen explícitamente "Lista de espera:
no implementada en esta fase — simplemente rechazar si está lleno."
Pero AI generó un status WAITLISTED en el enum. No se usa en el
código, pero su presencia sugiere que AI implementó para un
sistema más complejo del pedido.
- Impacto: El status WAITLISTED no se usa, pero un developer futuro
podría asumir que hay lógica de waitlist y buscarla. Código muerto
que genera confusión.
- Corrección: Eliminar WAITLISTED del enum. Si se implementa en el
futuro, se agrega entonces.
FINDING #7: Cancelación no verifica que el evento no esté ya pasado
- Archivo: routes.py
- Línea: cancel_registration endpoint
- Categoría: Edge case
- Severidad: Medium
- Descripción: La cancelación verifica que falten más de 24 horas
para el evento, pero no verifica que el evento no haya pasado ya.
Si event_date es en el pasado, hours_until_event es negativo,
y la condición "< 24" es True → rechaza la cancelación con
"Cannot cancel less than 24 hours before event."
El mensaje es confuso: el evento YA PASÓ, no es que falten
menos de 24 horas. Debería tener un check separado para
eventos pasados.
- Impacto: Mensaje de error incorrecto. El usuario recibe
"Cannot cancel less than 24 hours before event" cuando el
evento fue ayer. Debería decir "Event has already occurred."
- Corrección:
if event["event_date"] < datetime.utcnow():
raise HTTPException(
status_code=400,
detail="Cannot cancel — event has already occurred",
)
hours_until_event = (
event["event_date"] - datetime.utcnow()
).total_seconds() / 3600
if hours_until_event < 24:
raise HTTPException(
status_code=400,
detail="Cannot cancel less than 24 hours before event",
)
FINDING #8: Endpoint list_registrations no tiene autenticación ni autorización
- Archivo: routes.py
- Línea: @app.get("/events/{event_id}/registrations")
- Categoría: Seguridad
- Severidad: High
- Descripción: El endpoint list_registrations no usa
Depends(get_current_user). Cualquiera puede ver la lista de
registros de cualquier evento, incluyendo user IDs y montos
pagados. Incluso si get_current_user fuera auth real, este
endpoint lo bypasea completamente.
- Impacto: Leak de datos de usuarios. Un atacante puede enumerar
registros y ver quién asiste a qué eventos y cuánto pagó.
- Corrección: Agregar auth y verificar que el usuario tiene
permisos (admin o creador del evento).
@app.get("/events/{event_id}/registrations")
async def list_registrations(
event_id: str,
current_user: dict = Depends(get_current_user),
):
# Verificar que current_user es admin o creador del evento
event = events_db.get(event_id)
if not event:
raise HTTPException(status_code=404, detail="Event not found")
regs = [r for r in registrations_db.values() if r["event_id"] == event_id]
return regs
Resumen de Findings
┌─────┬────────────────────────────┬──────────┬───────────┐
│ # │ Finding │ Categoría│ Severidad │
├─────┼────────────────────────────┼──────────┼───────────┤
│ 1 │ DB URL con credenciales │ Seguridad│ Critical │
│ 2 │ Fake auth hardcoded │ Seguridad│ Critical │
│ 3 │ Capacity > en vez de >= │ Lógica │ High │
│ 4 │ Float para dinero │ Lógica │ Med-High │
│ 5 │ Pydantic v1 API │ AI-Spec │ Medium │
│ 6 │ Waitlisted no pedido │ AI-Spec │ Low-Med │
│ 7 │ No verifica evento pasado │ Edge Case│ Medium │
│ 8 │ list_registrations sin auth│ Seguridad│ High │
└─────┴────────────────────────────┴──────────┴───────────┘
Por categoría:
- Seguridad: 3 findings (2 Critical, 1 High)
- Lógica de negocio: 2 findings (1 High, 1 Medium-High)
- Edge Cases: 1 finding (1 Medium)
- AI-Specific: 2 findings (1 Medium, 1 Low-Medium)
- Calidad/Estilo: 0 findings
Decisión
RECHAZAR — solicitar cambios obligatorios antes de merge.
Justificación:
- 2 issues Critical de seguridad (credenciales hardcoded, sin auth real) son deal-breakers absolutos
- 1 issue High de seguridad (endpoint sin auth) agrava el problema
- 1 issue High de lógica (capacity off-by-one) permite overselling
- El PR no debería hacer merge hasta que los 4 issues de severidad High+ estén resueltos
- Los issues Medium pueden resolverse en un follow-up PR
Tiempo esperado
- Si encontraste 6-8 findings: Excelente. Tu checklist y pirámide están funcionando.
- Si encontraste 4-5 findings: Bien. Revisa qué categorías te faltaron — probablemente edge cases o AI-specific.
- Si encontraste 1-3 findings: Necesitas más práctica. Vuelve a las cápsulas 02-05 y aplica el checklist item por item.
Tiempo esperado: 20-35 minutos para un review completo.
Evaluación de Tu Review
Después de ver la solución, evalúa tu review:
Distribución por pirámide
¿Encontraste los issues de cada nivel?
Nivel 1 (Seguridad):
□ Finding #1 (DB credentials) → ¿lo encontraste?
□ Finding #2 (fake auth) → ¿lo encontraste?
□ Finding #8 (endpoint sin auth) → ¿lo encontraste?
Nivel 2 (Lógica):
□ Finding #3 (capacity > vs >=) → ¿lo encontraste?
□ Finding #4 (float para dinero) → ¿lo encontraste?
Nivel 3 (Edge Cases):
□ Finding #7 (evento pasado) → ¿lo encontraste?
Nivel AI-Specific:
□ Finding #5 (Pydantic v1) → ¿lo encontraste?
□ Finding #6 (waitlisted) → ¿lo encontraste?
Análisis de lo que se escapó
Para cada finding que NO encontraste, responde:
-
¿Qué item del checklist lo habría detectado?
- Finding #1 → Item 1 (secrets hardcoded)
- Finding #2 → Item 3 (auth en endpoints)
- Finding #3 → Item 7 (condiciones correctas)
- Finding #4 → Item 7 (cálculos correctos)
- Finding #5 → Item 16 (APIs de librerías correctas)
- Finding #6 → Item 6 (resuelve el problema pedido)
- Finding #7 → Item 10 (maneja edge cases)
- Finding #8 → Item 3 (auth en endpoints)
-
¿Por qué se te escapó?
- ¿No aplicaste ese item del checklist?
- ¿Lo aplicaste pero no en suficiente profundidad?
- ¿No conocías el problema subyacente?
-
¿Qué harás diferente en el próximo review?
Bonus: Issues Adicionales (No en los 8 Principales)
Si encontraste alguno de estos, demuestra un ojo agudo:
BONUS A: datetime.utcnow() deprecated en Python 3.12+
- Se usa en múltiples lugares
- Debería ser datetime.now(timezone.utc)
BONUS B: Imports dentro de funciones
- "from models import..." dentro de cada endpoint
- Debería estar al inicio del archivo
- Esto puede ser un síntoma de circular imports que AI "resolvió"
moviendo imports
BONUS C: response_model=dict en vez de un modelo Pydantic
- create_event y list_events usan dict como response_model
- No filtra campos sensibles, no valida output
BONUS D: No hay paginación en list_events
- Devuelve TODOS los eventos sin límite
BONUS E: El status filter en list_events compara string con Enum
- status: Optional[str] compara con e["status"] que es un EventStatus
- "published" != EventStatus.PUBLISHED (dependiendo de la comparación)
BONUS F: No verifica que el evento esté PUBLISHED antes de registrar
- Se puede registrar en eventos DRAFT o CANCELLED
Conexión con Proyecto
Del ejercicio al proyecto integrador (Módulo 8)
Este ejercicio es la versión simplificada del proyecto integrador:
| Este ejercicio | Proyecto integrador (M8) |
|---|---|
| 1 PR, 2 archivos, ~130 líneas | Codebase completo, 8-12 archivos, ~500-800 líneas |
| 8 problemas plantados | 15-20 problemas plantados |
| Código aislado (in-memory) | Código con DB, auth, tests |
| 20-35 minutos | 90-120 minutos |
| Categorías distribuidas | Categorías + interacciones entre archivos |
La diferencia principal en el módulo 8: los problemas interactúan entre sí. Un issue en models.py puede causar un error en service.py que se manifiesta en routes.py. Aquí los problemas son aislados; allá forman una red.
Troubleshooting
Problema 1: "Encontré menos de 4 — ¿soy malo en code review?"
Causa: Los primeros reviews siempre encuentran menos. El checklist es nuevo y no está internalizado aún. Solución: No es habilidad — es práctica. El checklist funciona como guía mecánica: aplica cada item uno por uno, sin saltar ninguno. Con práctica, la detección se vuelve automática. Haz el ejercicio de nuevo en una semana — encontrarás más.
Problema 2: "Encontré issues que no están en la lista de 8"
Causa: El código tiene más de 8 problemas. Los 8 listados son los principales. Solución: Excelente. Si encontraste issues adicionales (los Bonus A-F), tienes un ojo agudo. Documéntalos y clasifícalos. Todo finding legítimo cuenta.
Problema 3: "Me tomó más de 40 minutos"
Causa: Todavía no has internalizado la pirámide y el checklist. Solución: Con la pirámide, deberías empezar por seguridad (encontrar findings 1, 2, 8 en los primeros 10 minutos). Luego lógica (findings 3, 4 en 5-7 minutos). Luego edge cases y AI-specific (findings 5, 6, 7 en 5-7 minutos). Total: ~25 minutos. Si tardas más, probablemente estás leyendo sin checklist.
Problema 4: "Marqué cosas como problemas que no lo son"
Causa: False positives son comunes al principio — es preferible a false negatives. Solución: En code review real, los false positives se resuelven en la discusión con el autor del PR. Es mejor marcar 10 cosas y que 2 sean false positives, que marcar 3 y perder un Critical. Con experiencia, tus false positives disminuyen.
Problema 5: "No sabía que Pydantic v1 vs v2 era un problema"
Causa: Conocimiento específico de la librería que no todos tienen. Solución: El item 16 del checklist (APIs de librerías correctas) te protege: si un patrón te resulta desconocido o ligeramente diferente a lo que recuerdas, verifica en la documentación. No necesitas memorizar cada cambio de API — necesitas desarrollar la intuición de "esto no se ve como el Pydantic que yo uso."
Ejercicios Adicionales
Ejercicio 1: Escribir los fixes (Medio)
Para cada uno de los 8 findings, escribe el código corregido. No solo la línea — el bloque completo con contexto.
Ver solución
Fix #1 — DB URL sin fallback:
DB_CONNECTION = os.environ["DATABASE_URL"]
Fix #2 — Auth real (placeholder correcto):
from fastapi.security import OAuth2PasswordBearer
oauth2_scheme = OAuth2PasswordBearer(tokenUrl="token")
async def get_current_user(token: str = Depends(oauth2_scheme)):
user = verify_token(token)
if not user:
raise HTTPException(status_code=401, detail="Invalid token")
return user
Fix #3 — Capacity check:
if event["registered_count"] >= event["capacity"]:
raise HTTPException(status_code=400, detail="Event is full")
Fix #4 — Decimal para dinero:
# models.py
price: Decimal = Field(default=Decimal("0"), ge=0)
# routes.py
refund_amount = Decimal(str(registration["amount_paid"])) * Decimal("0.80")
Fix #5 — Pydantic v2:
from pydantic import BaseModel, Field, field_validator, ConfigDict
class EventCreate(BaseModel):
model_config = ConfigDict(from_attributes=True)
title: str = Field(..., min_length=1, max_length=200)
description: Optional[str] = Field(None, max_length=5000)
event_date: datetime
capacity: int = Field(..., ge=1)
price: Decimal = Field(default=Decimal("0"), ge=0)
@field_validator("event_date")
@classmethod
def event_must_be_future(cls, v: datetime) -> datetime:
if v < datetime.now(timezone.utc):
raise ValueError("Event date must be in the future")
return v
Fix #6 — Eliminar WAITLISTED:
class RegistrationStatus(str, Enum):
CONFIRMED = "confirmed"
CANCELLED = "cancelled"
Fix #7 — Verificar evento pasado:
if event["event_date"] < datetime.utcnow():
raise HTTPException(
status_code=400,
detail="Cannot cancel — event has already occurred",
)
Fix #8 — Auth en list_registrations:
@app.get("/events/{event_id}/registrations")
async def list_registrations(
event_id: str,
current_user: dict = Depends(get_current_user),
):
event = events_db.get(event_id)
if not event:
raise HTTPException(status_code=404, detail="Event not found")
regs = [r for r in registrations_db.values() if r["event_id"] == event_id]
return regs
Ejercicio 2: Escribir el review comment (Medio)
Escribe el comentario de code review que dejarías en el PR. Incluye:
- Resumen general (2-3 oraciones)
- Los blocking issues (deben resolverse antes de merge)
- Los non-blocking issues (pueden resolverse en follow-up)
- Tono profesional y constructivo
Ver solución
## Code Review: Event Registration System
Buen progreso con la estructura general — los modelos Pydantic y
el flujo de registro son sólidos. Sin embargo, hay varios issues
que necesitan resolverse antes de merge, principalmente en seguridad
y lógica de negocio.
### 🔴 Blocking (resolver antes de merge):
1. **DB URL con credenciales hardcoded** (routes.py) — Eliminar el
fallback con credentials del getenv. Usar os.environ["DATABASE_URL"]
sin fallback.
2. **Sin autenticación real** (routes.py) — get_current_user() devuelve
un usuario hardcoded. Necesita implementación real con JWT/OAuth2
antes de merge a cualquier ambiente.
3. **list_registrations sin auth** (routes.py) — Este endpoint no tiene
Depends(get_current_user). Expone datos de registros a cualquiera.
4. **Capacity check: > debería ser >=** (routes.py) — Permite 1 registro
extra por encima de la capacidad. Cambiar a >=.
### 🟡 Non-blocking (resolver en follow-up):
5. **Float para dinero** — Cambiar price y refund a Decimal para evitar
errores de precisión.
6. **Pydantic v1 API** — Migrar @validator a @field_validator y class Config
a model_config.
7. **WAITLISTED enum no pedido** — Eliminar status que no se usa.
8. **Cancelación no verifica evento pasado** — Agregar check de
event_date < now antes del check de 24 horas.
Sugiero resolver los 4 blocking issues y crear un follow-up PR para
los non-blocking. ¡Buen trabajo con la estructura base!
Ejercicio 3: Diseñar un test para cada finding (Difícil)
Para cada uno de los 8 findings, escribe un test que habría detectado el problema ANTES del code review.
Ver solución para findings 3 y 7
Test para Finding #3 (capacity > vs >=):
def test_cannot_register_when_at_capacity():
event = create_test_event(capacity=2)
register_user(event["id"], user_id="user-1")
register_user(event["id"], user_id="user-2")
response = client.post(
f"/events/{event['id']}/register",
headers={"Authorization": "Bearer user-3-token"},
)
assert response.status_code == 400
assert "full" in response.json()["detail"].lower()
assert events_db[event["id"]]["registered_count"] == 2
Test para Finding #7 (evento pasado):
def test_cannot_cancel_past_event():
event = create_test_event(
event_date=datetime.utcnow() - timedelta(days=1),
)
register_user(event["id"], user_id="user-1")
response = client.post(
f"/events/{event['id']}/cancel-registration",
headers={"Authorization": "Bearer user-1-token"},
)
assert response.status_code == 400
assert "already occurred" in response.json()["detail"].lower()
Ejercicio 4: Review de un PR tuyo (Difícil)
Genera código con Claude Code para una funcionalidad de tu proyecto real. Luego haz el code review completo usando:
- La pirámide de prioridades
- El checklist de 20 items
- Los 8 red flags de AI
- El proceso de verificación de lógica de negocio
Documenta todo con el formato de findings.
Ver guía de evaluación
Tu review debería:
- ✅ Seguir la pirámide (seguridad primero)
- ✅ Aplicar al menos 10 items del checklist
- ✅ Buscar los 8 red flags de AI
- ✅ Verificar lógica de negocio con el proceso de 4 pasos
- ✅ Documentar findings con formato profesional
- ✅ Incluir decisión final con justificación
- ✅ Registrar tiempo invertido
Si no encontraste ningún issue, tu prompt fue demasiado simple (CRUD puro sin lógica de negocio) o tu review no fue suficientemente profundo.
Resumen
En esta cápsula completaste:
- Un code review profesional de un PR realista generado por Claude Code
- Aplicaste la pirámide de prioridades para distribuir tu atención
- Usaste el checklist de 20 items para buscar problemas sistemáticamente
- Identificaste red flags de AI (Pydantic v1, código no pedido)
- Verificaste lógica de negocio (capacity check, cálculo de reembolso)
- Documentaste findings con formato profesional (categoría, severidad, corrección)
- Tomaste una decisión de review justificada
Lo que te llevas del módulo 4:
Artefactos construidos:
✅ Pirámide de prioridades (Seguridad → Lógica → Edge Cases → Performance → Estilo)
✅ Checklist de 20 items en 5 categorías
✅ Catálogo de 8 red flags específicos de AI
✅ Proceso de 4 pasos para verificar lógica de negocio
✅ Experiencia práctica con un PR review completo
Próximo módulo: Patrones de Error Comunes — profundizar en los tipos específicos de errores que tu checklist debe encontrar.
Recursos Adicionales
- Google — How to Write Code Review Comments - Cómo escribir comentarios de review profesionales
- Conventional Comments - Formato estandarizado para comentarios de review
- Ship / Show / Ask — PR Strategies - Cuándo merge directo, cuándo review, cuándo discusión
- FastAPI — Testing Tutorial - Cómo escribir tests para FastAPI que validan findings
- Anthropic — Claude Code Best Practices - Prácticas recomendadas para validar output de Claude Code
Debugging & Code Review with Claude Code — Módulo 4, Cápsula 06 Claude Code Agentic Development Path — Guía #6 de 11