Module 4: Code Review of AI Output

Exercise: Code Review of an AI-Generated PR

Exercise: Code Review of an AI-Generated PR

Capsule overview

This is the module's integrative exercise. You've learned the priority pyramid (capsule 02), built a 20-item checklist (capsule 03), mastered the 8 AI red flags (capsule 04), and practiced business logic verification (capsule 05). Now you apply it all in a real code review.

The scenario: a teammate used Claude Code to generate an event management system for a conference platform. The PR has ~130 lines of FastAPI code with models, endpoints, and business logic. Your job is to do a professional code review using the tools from this module.

The code has an intentional mix of good code and 8 problems distributed across the pyramid's categories. Some are obvious, others are subtle. Some are bugs, others are AI red flags. Your goal is to find at least 6 of the 8.


PR Context

Business requirements

The conference platform needs an event registration system with these rules:

  1. Events have a title, description, date, maximum capacity, and price
  2. Registration: users can register for events that aren't full
  3. Pricing: events with a price > $0 require payment. Free events don't require payment
  4. Cancellation: a user can cancel their registration up to 24 hours before the event. The refund is 80% (20% is retained as a fee)
  5. Capacity: when an event reaches maximum capacity, no more registrations are allowed
  6. Waitlist: not implemented in this phase — simply reject if it's full

PR Description (written by your teammate)

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

The PR Code

Read the following code as if it were a real PR. Don't look at the solutions until you've completed your 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

Your Task

Instructions

  1. Read the complete code as you would in a real code review
  2. Apply the priority pyramid (security → logic → edge cases → AI-specific → style)
  3. Use the 20-item checklist
  4. Document each finding with this format:
FINDING #N
- File: [models.py / routes.py]
- Line(s): [approximate reference]
- Category: [Security / Logic / Edge Case / AI-Specific / Quality]
- Severity: [Critical / High / Medium / Low]
- Description: [what's wrong]
- Impact: [what can happen]
- Fix: [how to fix it]
  1. At the end, include:
    • Summary: how many findings per category?
    • Decision: approve, request changes, or reject?
    • Time invested

Try to find at least 6 of the 8 problems before looking at the solution.


Your Workspace

Use this space to document your review before looking at the solution:

MY CODE REVIEW
==============

Start time: __:__

FINDINGS:

FINDING #1
- File: 
- Category: 
- Severity: 
- Description: 
- Fix: 

FINDING #2
- File: 
- Category: 
- Severity: 
- Description: 
- Fix: 

(continue...)

SUMMARY:
- Security: ___ findings
- Logic: ___ findings
- Edge Cases: ___ findings
- AI-Specific: ___ findings
- Quality: ___ findings

DECISION: [Approve / Request changes / Reject]
TIME INVESTED: ___ minutes

Detailed Solution

See the complete solution (don't open until you've completed your review)

The 8 problems in the PR


FINDING #1: Database URL with hardcoded credentials

- File: routes.py
- Line: DB_CONNECTION = os.getenv("DATABASE_URL", "postgresql://admin:admin123@localhost/events")
- Category: Security
- Severity: Critical
- Description: The database URL fallback contains credentials 
  (user: admin, password: admin123). If DATABASE_URL isn't 
  configured in production, the app connects with development 
  credentials that are in the source code.
- Impact: Anyone with access to the repo knows the database 
  credentials. If the fallback triggers in production, 
  it's direct access to the DB.
- Fix:
DB_CONNECTION = os.environ["DATABASE_URL"]  # Fails if it doesn't exist

FINDING #2: Fake authentication (there's no real auth)

- File: routes.py
- Line: async def get_current_user(): return {"id": "user-123"...}
- Category: Security
- Severity: Critical
- Description: get_current_user() is hardcoded — it always returns 
  the same fake user. There's no real authentication. All the 
  endpoints that use Depends(get_current_user) are public.
- Impact: Anyone can register, cancel registrations, and 
  access other users' data. The list_registrations endpoint 
  doesn't even use auth — it exposes all the registration data.
- Fix: Implement real auth with JWT or OAuth2. At minimum,
  add a visible TODO and don't merge without auth.

FINDING #3: Capacity check uses > instead of >=

- File: routes.py
- Line: if event["registered_count"] > event["capacity"]:
- Category: Business logic
- Severity: High
- Description: The comparison is > (strictly greater) when 
  it should be >= (greater than or equal). If capacity = 50 and 
  registered_count = 50, the condition is False and it allows 
  a registration #51.
- Impact: Every event allows 1 registration more than the maximum 
  capacity. In a conference with a limited room, there's 1 person 
  without a seat.
- Fix:
if event["registered_count"] >= event["capacity"]:
    raise HTTPException(status_code=400, detail="Event is full")

FINDING #4: Refund calculates with float, not Decimal

- File: routes.py
- Line: refund_amount = registration["amount_paid"] * 0.80
- Category: Business logic
- Severity: Medium-High
- Description: The refund calculation uses float (0.80). This 
  is consistent with price being float in the model, but BOTH 
  should be Decimal. With float: 19.99 * 0.80 = 15.991999... 
  which can cause cent errors.
- Impact: Rounding errors in refunds. It may be fractions 
  of a cent, but at volume it accumulates. Also inconsistency: 
  the module imports Decimal but doesn't use it for money.
- Fix: Change price to Decimal in the model and use 
  Decimal("0.80") for the refund calculation.
# In models.py
price: Decimal = Field(default=Decimal("0"), ge=0)

# In routes.py
refund_amount = Decimal(str(registration["amount_paid"])) * Decimal("0.80")

FINDING #5: Pydantic v1 API (@validator, Config, orm_mode)

- File: models.py
- Lines: @validator, class Config, orm_mode = True
- Category: AI-Specific (Red Flag 2: APIs from an earlier version)
- Severity: Medium
- Description: The code uses the Pydantic v1 API:
  - @validator → should be @field_validator (v2)
  - class Config: orm_mode = True → should be 
    model_config = ConfigDict(from_attributes=True) (v2)
  If the project uses Pydantic v2, this causes deprecation 
  warnings and can fail in future versions.
- Impact: Works with Pydantic v2 (there's compatibility), but 
  generates warnings. In Pydantic v3 (when it arrives) it will stop 
  working.
- Fix:
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 has "waitlisted" but the requirements say "not implemented"

- File: models.py
- Line: WAITLISTED = "waitlisted" in RegistrationStatus
- Category: AI-Specific (Red Flag 4: slightly different problem)
- Severity: Low-Medium
- Description: The requirements explicitly say "Waitlist: 
  not implemented in this phase — simply reject if it's full."
  But AI generated a WAITLISTED status in the enum. It isn't used in the 
  code, but its presence suggests AI implemented for a 
  more complex system than the one requested.
- Impact: The WAITLISTED status isn't used, but a future developer 
  might assume there's waitlist logic and go looking for it. Dead code 
  that generates confusion.
- Fix: Remove WAITLISTED from the enum. If it's implemented in the 
  future, it's added then.

FINDING #7: Cancellation doesn't verify that the event hasn't already passed

- File: routes.py
- Line: cancel_registration endpoint
- Category: Edge case
- Severity: Medium
- Description: The cancellation checks that there are more than 24 hours 
  until the event, but it doesn't check that the event hasn't already passed. 
  If event_date is in the past, hours_until_event is negative, 
  and the "< 24" condition is True → it rejects the cancellation with 
  "Cannot cancel less than 24 hours before event."
  
  The message is confusing: the event ALREADY PASSED, it's not that there are 
  less than 24 hours left. It should have a separate check for 
  past events.
- Impact: Incorrect error message. The user gets 
  "Cannot cancel less than 24 hours before event" when the 
  event was yesterday. It should say "Event has already occurred."
- Fix:
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: The list_registrations endpoint has no authentication or authorization

- File: routes.py
- Line: @app.get("/events/{event_id}/registrations")
- Category: Security
- Severity: High
- Description: The list_registrations endpoint doesn't use 
  Depends(get_current_user). Anyone can view the list of 
  registrations for any event, including user IDs and amounts 
  paid. Even if get_current_user were real auth, this 
  endpoint bypasses it completely.
- Impact: A leak of user data. An attacker can enumerate 
  registrations and see who attends which events and how much they paid.
- Fix: Add auth and verify that the user has 
  permissions (admin or event creator).
@app.get("/events/{event_id}/registrations")
async def list_registrations(
    event_id: str,
    current_user: dict = Depends(get_current_user),
):
    # Verify that current_user is an admin or the event creator
    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

Findings Summary

┌─────┬────────────────────────────┬──────────┬───────────┐
│  #  │ Finding                    │ Category │ Severity  │
├─────┼────────────────────────────┼──────────┼───────────┤
│  1  │ DB URL with credentials    │ Security │ Critical  │
│  2  │ Fake auth hardcoded        │ Security │ Critical  │
│  3  │ Capacity > instead of >=   │ Logic    │ High      │
│  4  │ Float for money            │ Logic    │ Med-High  │
│  5  │ Pydantic v1 API            │ AI-Spec  │ Medium    │
│  6  │ Unrequested waitlisted     │ AI-Spec  │ Low-Med   │
│  7  │ Doesn't check past event   │ Edge Case│ Medium    │
│  8  │ list_registrations no auth │ Security │ High      │
└─────┴────────────────────────────┴──────────┴───────────┘

By category:
- Security: 3 findings (2 Critical, 1 High)
- Business logic: 2 findings (1 High, 1 Medium-High)
- Edge Cases: 1 finding (1 Medium)
- AI-Specific: 2 findings (1 Medium, 1 Low-Medium)
- Quality/Style: 0 findings

Decision

REJECT — request mandatory changes before merge.

Justification:

  • 2 Critical security issues (hardcoded credentials, no real auth) are absolute deal-breakers
  • 1 High security issue (endpoint without auth) aggravates the problem
  • 1 High logic issue (capacity off-by-one) allows overselling
  • The PR shouldn't merge until the 4 High+ severity issues are resolved
  • The Medium issues can be resolved in a follow-up PR

Expected time

  • If you found 6-8 findings: Excellent. Your checklist and pyramid are working.
  • If you found 4-5 findings: Good. Review which categories you missed — probably edge cases or AI-specific.
  • If you found 1-3 findings: You need more practice. Go back to capsules 02-05 and apply the checklist item by item.

Expected time: 20-35 minutes for a complete review.


Evaluating Your Review

After seeing the solution, evaluate your review:

Distribution by pyramid

Did you find the issues at each level?

Level 1 (Security):
□ Finding #1 (DB credentials) → did you find it?
□ Finding #2 (fake auth) → did you find it?
□ Finding #8 (endpoint without auth) → did you find it?

Level 2 (Logic):
□ Finding #3 (capacity > vs >=) → did you find it?
□ Finding #4 (float for money) → did you find it?

Level 3 (Edge Cases):
□ Finding #7 (past event) → did you find it?

Level AI-Specific:
□ Finding #5 (Pydantic v1) → did you find it?
□ Finding #6 (waitlisted) → did you find it?

Analysis of what slipped past

For each finding you did NOT find, answer:

  1. Which checklist item would have detected it?

    • Finding #1 → Item 1 (hardcoded secrets)
    • Finding #2 → Item 3 (auth on endpoints)
    • Finding #3 → Item 7 (correct conditions)
    • Finding #4 → Item 7 (correct calculations)
    • Finding #5 → Item 16 (correct library APIs)
    • Finding #6 → Item 6 (solves the problem asked for)
    • Finding #7 → Item 10 (handles edge cases)
    • Finding #8 → Item 3 (auth on endpoints)
  2. Why did it slip past you?

    • Did you not apply that checklist item?
    • Did you apply it but not in enough depth?
    • Did you not know the underlying problem?
  3. What will you do differently in the next review?


Bonus: Additional Issues (Not in the Main 8)

If you found any of these, you demonstrate a sharp eye:

BONUS A: datetime.utcnow() deprecated in Python 3.12+
- Used in multiple places
- Should be datetime.now(timezone.utc)

BONUS B: Imports inside functions
- "from models import..." inside each endpoint
- Should be at the top of the file
- This can be a symptom of circular imports that AI "solved" 
  by moving the imports

BONUS C: response_model=dict instead of a Pydantic model
- create_event and list_events use dict as the response_model
- It doesn't filter sensitive fields, doesn't validate the output

BONUS D: There's no pagination in list_events
- Returns ALL events with no limit

BONUS E: The status filter in list_events compares a string with an Enum
- status: Optional[str] compares with e["status"] which is an EventStatus
- "published" != EventStatus.PUBLISHED (depending on the comparison)

BONUS F: It doesn't verify that the event is PUBLISHED before registering
- You can register for DRAFT or CANCELLED events

Connection to the Project

From the exercise to the capstone project (Module 8)

This exercise is the simplified version of the capstone project:

This exerciseCapstone project (M8)
1 PR, 2 files, ~130 linesComplete codebase, 8-12 files, ~500-800 lines
8 planted problems15-20 planted problems
Isolated code (in-memory)Code with DB, auth, tests
20-35 minutes90-120 minutes
Distributed categoriesCategories + interactions between files

The main difference in module 8: the problems interact with each other. An issue in models.py can cause an error in service.py that shows up in routes.py. Here the problems are isolated; there they form a network.


Troubleshooting

Problem 1: "I found fewer than 4 — am I bad at code review?"

Cause: The first reviews always find fewer. The checklist is new and not yet internalized. Solution: It's not skill — it's practice. The checklist works as a mechanical guide: apply each item one by one, without skipping any. With practice, detection becomes automatic. Do the exercise again in a week — you'll find more.

Problem 2: "I found issues that aren't in the list of 8"

Cause: The code has more than 8 problems. The 8 listed are the main ones. Solution: Excellent. If you found additional issues (the Bonus A-F), you have a sharp eye. Document and classify them. Every legitimate finding counts.

Problem 3: "It took me more than 40 minutes"

Cause: You haven't internalized the pyramid and the checklist yet. Solution: With the pyramid, you should start with security (finding findings 1, 2, 8 in the first 10 minutes). Then logic (findings 3, 4 in 5-7 minutes). Then edge cases and AI-specific (findings 5, 6, 7 in 5-7 minutes). Total: ~25 minutes. If it takes longer, you're probably reading without a checklist.

Problem 4: "I flagged things as problems that aren't"

Cause: False positives are common at first — it's preferable to false negatives. Solution: In a real code review, false positives are resolved in the discussion with the PR author. It's better to flag 10 things and have 2 be false positives than to flag 3 and miss a Critical. With experience, your false positives decrease.

Problem 5: "I didn't know Pydantic v1 vs v2 was a problem"

Cause: Library-specific knowledge that not everyone has. Solution: Item 16 of the checklist (correct library APIs) protects you: if a pattern is unfamiliar or slightly different from what you remember, verify it in the documentation. You don't need to memorize every API change — you need to develop the intuition of "this doesn't look like the Pydantic I use."


Additional Exercises

Exercise 1: Write the fixes (Medium)

For each of the 8 findings, write the corrected code. Not just the line — the complete block with context.

See solution

Fix #1 — DB URL with no fallback:

DB_CONNECTION = os.environ["DATABASE_URL"]

Fix #2 — Real auth (correct placeholder):

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 for money:

# 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 — Remove WAITLISTED:

class RegistrationStatus(str, Enum):
    CONFIRMED = "confirmed"
    CANCELLED = "cancelled"

Fix #7 — Verify past event:

if event["event_date"] < datetime.utcnow():
    raise HTTPException(
        status_code=400,
        detail="Cannot cancel — event has already occurred",
    )

Fix #8 — Auth on 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

Exercise 2: Write the review comment (Medium)

Write the code review comment you'd leave on the PR. Include:

  • A general summary (2-3 sentences)
  • The blocking issues (must be resolved before merge)
  • The non-blocking issues (can be resolved in a follow-up)
  • A professional and constructive tone
See solution
## Code Review: Event Registration System

Good progress on the overall structure — the Pydantic models and 
the registration flow are solid. However, there are several issues 
that need to be resolved before merge, mainly in security 
and business logic.

### 🔴 Blocking (resolve before merge):

1. **DB URL with hardcoded credentials** (routes.py) — Remove the 
   fallback with credentials from the getenv. Use os.environ["DATABASE_URL"] 
   with no fallback.

2. **No real authentication** (routes.py) — get_current_user() returns 
   a hardcoded user. It needs a real implementation with JWT/OAuth2 
   before merging to any environment.

3. **list_registrations without auth** (routes.py) — This endpoint doesn't have 
   Depends(get_current_user). It exposes registration data to anyone.

4. **Capacity check: > should be >=** (routes.py) — Allows 1 extra 
   registration over capacity. Change to >=.

### 🟡 Non-blocking (resolve in a follow-up):

5. **Float for money** — Change price and refund to Decimal to avoid 
   precision errors.

6. **Pydantic v1 API** — Migrate @validator to @field_validator and class Config 
   to model_config.

7. **Unrequested WAITLISTED enum** — Remove the status that isn't used.

8. **Cancellation doesn't verify a past event** — Add a check for 
   event_date < now before the 24-hour check.

I suggest resolving the 4 blocking issues and creating a follow-up PR for 
the non-blocking ones. Nice work on the base structure!

Exercise 3: Design a test for each finding (Hard)

For each of the 8 findings, write a test that would have detected the problem BEFORE the code review.

See solution for findings 3 and 7

Test for 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 for Finding #7 (past event):

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

Exercise 4: Review one of your own PRs (Hard)

Generate code with Claude Code for a feature of your real project. Then do a complete code review using:

  1. The priority pyramid
  2. The 20-item checklist
  3. The 8 AI red flags
  4. The business logic verification process

Document everything with the findings format.

See evaluation guide

Your review should:

  • ✅ Follow the pyramid (security first)
  • ✅ Apply at least 10 checklist items
  • ✅ Look for the 8 AI red flags
  • ✅ Verify business logic with the 4-step process
  • ✅ Document findings with a professional format
  • ✅ Include a final decision with justification
  • ✅ Record the time invested

If you didn't find any issue, your prompt was too simple (pure CRUD with no business logic) or your review wasn't deep enough.


Summary

In this capsule you completed:

  • A professional code review of a realistic PR generated by Claude Code
  • You applied the priority pyramid to distribute your attention
  • You used the 20-item checklist to look for problems systematically
  • You identified AI red flags (Pydantic v1, unrequested code)
  • You verified business logic (capacity check, refund calculation)
  • You documented findings with a professional format (category, severity, fix)
  • You made a justified review decision

What you take from module 4:

Artifacts built:
✅ Priority pyramid (Security → Logic → Edge Cases → Performance → Style)
✅ 20-item checklist in 5 categories
✅ Catalog of 8 red flags specific to AI
✅ 4-step process for verifying business logic
✅ Practical experience with a complete PR review

Next module: Common Error Patterns — going deeper into the specific types of errors your checklist must find.


Additional resources

  1. Google — How to Write Code Review Comments - How to write professional review comments
  2. Conventional Comments - A standardized format for review comments
  3. Ship / Show / Ask — PR Strategies - When to merge directly, when to review, when to discuss
  4. FastAPI — Testing Tutorial - How to write FastAPI tests that validate findings
  5. Anthropic — Claude Code Best Practices - Recommended practices for validating Claude Code output

Debugging & Code Review with Claude Code — Module 4, Capsule 06 Claude Code Agentic Development Path — Guide #6 of 11