/code-review

name: code-review
description: When the user asks for a code review, shares code for feedback, or says "review this", "check my code", "what's wrong with this". Also activate when reviewing a pull request or diff.
related: [security-review, architecture-design]
reads: [startup-context]

Code Review

When to Use

• The user shares code (file, snippet, or diff) and asks for feedback
• They paste a pull request or want to know if code is production-ready
• Pre-merge quality gate or bug hunting
• Reviewing architectural decisions in a PR

Context Required

From `startup-context`: tech stack, product stage, team size. Also need from the user:

• The code or diff to review
• What the code is supposed to do (PR purpose or feature context)
• Any specific concerns (performance, security, correctness)
• Language and framework if not obvious from the code

Workflow

Follow a structured five-step methodology. Each step must be completed before moving to the next.

1. **Context** — Understand and summarize the PR's purpose before any analysis. Recap the intent back to the user in 1-2 sentences. If unclear, ask before proceeding. Never start reviewing without understanding what the code is trying to accomplish. 2. **Structure** — Evaluate architectural decisions and design patterns:

• Does the code belong in the right module/layer?
• Are abstractions appropriate (not too many, not too few)?
• Does this change align with the existing codebase patterns?
• For non-obvious design choices, acknowledge the author's reasoning before proposing alternatives.

3. **Details** — Assess code quality across multiple dimensions:

• **Correctness:** Logic errors, off-by-one bugs, null/undefined handling, race conditions, edge cases
• **Security:** OWASP Top 10 baseline — injection, broken auth, data exposure, XSS, access control, misconfig, insecure deserialization, vulnerable components
• **Performance:** N+1 queries, unnecessary re-renders, O(n^2) on large datasets, missing caching, memory leaks
• **Naming and clarity:** Do names communicate intent? Are functions focused on a single responsibility?

4. **Tests** — Validate test coverage with equal rigor as code review:

• Are behavioral assertions present (not just implementation testing)?
• Are edge cases and error paths covered?
• Are tests brittle or resilient to refactoring?
• What test cases are missing?

5. **Feedback** — Generate a prioritized, categorized report with specific code examples and concrete improvements. Recognize strong patterns and good decisions explicitly.

Output Format

# Code Review: [Feature/File Name]

## Summary
One-paragraph assessment: what the PR does, whether it is ready to merge, needs minor fixes, or needs rework.

## Findings

### Critical (must fix before merge)
- **[CRT-1] Title** — file:line — description, why it matters, suggested fix with code example

### Major (should fix before merge)
- **[MAJ-1] Title** — file:line — description, why it matters, suggested fix

### Minor (fix when convenient)
- **[MIN-1] Title** — file:line — description, suggestion

### Positive (things done well)
- **[POS-1] Title** — file:line — what was done well and why it matters

## Questions
Clarifying questions about non-obvious design choices before blocking on them.

## Suggested Tests
- Test case 1
- Test case 2

Frameworks & Best Practices

Severity Definitions

| Severity | Definition | Action | |----------|-----------|--------| | **Critical** | Security vulnerability, data loss risk, crash in production, broken core functionality | Block merge | | **Major** | Significant bug, performance regression, missing error handling on critical path, architectural violation | Should fix before merge | | **Minor** | Style issue, naming improvement, minor optimization, documentation gap | Fix when convenient | | **Positive** | Well-written code, good pattern usage, thoughtful error handling | Acknowledge and reinforce |

Review Principles

• **Always ground feedback in specifics.** Every finding must reference a file, line, and include a concrete improvement — not just "this could be better."
• **Recognize good work explicitly.** Call out strong patterns, clean abstractions, and thoughtful error handling. Reviews that only flag problems are demoralizing and incomplete.
• **Acknowledge author reasoning.** For non-obvious choices, assume the author had a reason. Ask before overriding. Phrase as "I see you chose X — was that because of Y? If so, consider Z as an alternative."
• **Do not block on style when automated tooling handles it.** Linting and formatting are the job of CI, not reviewers. Focus on logic, architecture, and correctness.
• **Treat test review with equal weight.** Tests are not an afterthought. Missing tests for critical paths is a major finding, not a minor one.

OWASP Top 10 Quick Checks

1. **Injection** — Are user inputs parameterized? Check SQL, NoSQL, OS command, LDAP 2. **Broken Auth** — Sessions secure? Tokens rotated? Passwords hashed (bcrypt/argon2)? 3. **Sensitive Data Exposure** — Secrets in env vars (not code)? PII encrypted at rest? 4. **XXE** — XML parsing disabled or configured to reject external entities? 5. **Broken Access Control** — Every endpoint checks authorization, not just authentication? 6. **Misconfiguration** — Debug modes off? CORS restrictive? Security headers set? 7. **XSS** — Outputs encoded? No `dangerouslySetInnerHTML` / `v-html` with user data? 8. **Insecure Deserialization** — Serialized objects from untrusted sources validated? 9. **Vulnerable Components** — Dependencies up to date? Any known CVEs? 10. **Insufficient Logging** — Auth failures, access violations, and errors logged?

N+1 Query Detection

• Looping over a collection and making a DB call per item
• ORM lazy loading inside a loop (e.g., `user.posts` in a `for user in users` loop)
• GraphQL resolvers fetching related data per parent without DataLoader
• **Fix:**