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:

  1. Eventos tienen título, descripción, fecha, capacidad máxima, y precio
  2. Registro: los usuarios pueden registrarse a eventos que no estén llenos
  3. Pricing: eventos con precio > $0 requieren pago. Eventos gratuitos no requieren pago
  4. Cancelación: un usuario puede cancelar su registro hasta 24 horas antes del evento. El reembolso es del 80% (se retiene 20% como fee)
  5. Capacidad: cuando un evento llega a capacidad máxima, no se permiten más registros
  6. 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

  1. Lee el código completo como lo harías en un code review real
  2. Aplica la pirámide de prioridades (seguridad → lógica → edge cases → AI-specific → estilo)
  3. Usa el checklist de 20 items
  4. 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]
  1. 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:

  1. ¿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)
  2. ¿Por qué se te escapó?

    • ¿No aplicaste ese item del checklist?
    • ¿Lo aplicaste pero no en suficiente profundidad?
    • ¿No conocías el problema subyacente?
  3. ¿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 ejercicioProyecto integrador (M8)
1 PR, 2 archivos, ~130 líneasCodebase completo, 8-12 archivos, ~500-800 líneas
8 problemas plantados15-20 problemas plantados
Código aislado (in-memory)Código con DB, auth, tests
20-35 minutos90-120 minutos
Categorías distribuidasCategorí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:

  1. La pirámide de prioridades
  2. El checklist de 20 items
  3. Los 8 red flags de AI
  4. 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

  1. Google — How to Write Code Review Comments - Cómo escribir comentarios de review profesionales
  2. Conventional Comments - Formato estandarizado para comentarios de review
  3. Ship / Show / Ask — PR Strategies - Cuándo merge directo, cuándo review, cuándo discusión
  4. FastAPI — Testing Tutorial - Cómo escribir tests para FastAPI que validan findings
  5. 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