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.
- 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.
- 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.
- 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?
- 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?
- Feedback — Generate a prioritized, categorized report with specific code examples and concrete improvements. Recognize strong patterns and good decisions explicitly.
Output Format
markdown# 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
- Injection — Are user inputs parameterized? Check SQL, NoSQL, OS command, LDAP
- Broken Auth — Sessions secure? Tokens rotated? Passwords hashed (bcrypt/argon2)?
- Sensitive Data Exposure — Secrets in env vars (not code)? PII encrypted at rest?
- XXE — XML parsing disabled or configured to reject external entities?
- Broken Access Control — Every endpoint checks authorization, not just authentication?
- Misconfiguration — Debug modes off? CORS restrictive? Security headers set?
- XSS — Outputs encoded? No
dangerouslySetInnerHTML/v-htmlwith user data? - Insecure Deserialization — Serialized objects from untrusted sources validated?
- Vulnerable Components — Dependencies up to date? Any known CVEs?
- 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.postsin afor user in usersloop) - GraphQL resolvers fetching related data per parent without DataLoader
- Fix: Eager loading, batch queries, or DataLoader pattern
Language-Agnostic Red Flags
- Functions longer than 40 lines or more than 3 levels of nesting
- Boolean params that change behavior (use separate functions)
- Catch blocks that swallow exceptions silently
- Hardcoded values that should be configuration
- Missing input validation on public API boundaries
Related Skills
security-review— chain for a deep-dive security audit when critical security findings surfacearchitecture-design— chain when code review reveals structural problems that need architectural rethinking
Examples
Example prompt: "Review this Express route handler for creating users."
Good output snippet:
# Code Review: POST /api/users Handler ## Summary The handler creates users but has a critical SQL injection vulnerability and missing input validation. Needs fixes before merge. Clean async/await usage is a positive. ## Findings ### Critical - **[CRT-1] SQL Injection** — routes/users.js:14 — Email interpolated directly into SQL string. Use parameterized queries: `db.query('INSERT INTO users (email) VALUES ($1)', [email])` ### Major - **[MAJ-1] No input validation** — routes/users.js:12 — Email from `req.body` used with no validation. Add zod/joi schema validation. - **[MAJ-2] Missing error handling** — routes/users.js:18 — DB errors propagate as 500 with stack trace. Wrap in try/catch, return generic error. ### Positive - **[POS-1] Clean async/await** — routes/users.js:11 — Easy to follow, no callback nesting. ## Questions - Is there a validation middleware already in the project that should be reused here?
