Code Review

Code Review

Overview

Review code for quality, user impact, tests, and documentation. Balance technical excellence with practical simplicity.

Core principle: Clean code should serve users, not just developers.

When to Use

• After completing a feature or bug fix
• Before creating a pull request
• When reviewing changes before merge

When NOT to Use

• Trivial changes (typo fixes)
• Documentation-only changes
• Initial exploration/prototyping

Review Process

Phase 1: Identify Changes

Categorize changed files:

• Backend: lib/, app.py
• Frontend: frontend/src/
• Tests: tests/
• Docs: llm/, *.md

Note change type: new feature | bug fix | refactoring | enhancement

Phase 2: User Impact

Ask for each change:

1. Does this affect what users see or do?
2. Are error messages user-friendly (not technical jargon)?
3. Are loading states shown?
4. Can users recover from errors?
5. Is this the simplest UX possible?

Red flags:

• Silent failures (user doesn't know something failed)
• Lost work on errors
• Unclear feedback ("Error: 500" vs "Could not save")
• Unnecessary complexity exposed to users

Phase 3: Code Quality

KISS Check

Can each function be explained in one sentence? If not, it's too complex.

Backend Anti-patterns (blocking)

• Silent failures (empty except blocks)
• God functions (>30 lines, >3 params)
• SQL injection (f-strings in queries)
• Missing error context
• Walrus operators / complex one-liners

Frontend Anti-patterns (blocking)

• Empty catch blocks
• Inline fetch (not in service layer)
• Missing useEffect cleanup
• any types or as assertions
• Hardcoded colors (use theme: fg., canvas.)
• Prop drilling (>5 props)

Security

• Inputs validated at API boundary
• SQL parameterized (? placeholders)
• No secrets in code/logs

Phase 4: Test Coverage

Test quality:

• Naming: test_<method>_<scenario>_<expected>
• One behavior per test
• Error cases tested, not just happy path

Phase 5: Documentation

Update llm/state-*.md when:

• New API endpoint → state-backend.md
• New block → state-backend.md
• New component/page → state-frontend.md
• Architecture change → state-project.md

Code comments: explain WHY, not what. Lowercase, concise.

Output Format

### User Impact
[UX improvements or issues found]

### Anti-patterns
[location + violation + fix, or "none"]

### Code Quality Issues
[severity + location + fix, or "none"]

### Test Coverage
[required: present/missing | gaps if any]

### Documentation Updates
[files needing update, or "none"]

### Verdict
[BLOCK | REQUEST CHANGES | APPROVE]
Reason: [brief explanation]

Verdict Rules

Golden Rules

1. Anti-patterns are blocking - always reject
2. User experience matters - clean code that hurts UX is bad code
3. KISS wins - one sentence explanation or it's too complex
4. Tests are not optional - new code needs tests
5. Fail loudly - silent failures are never acceptable

Source: nicofretti/DataGenFlow — distributed by TomeVault.