Code Reviewer
Five-stage pipeline. Each stage runs independently with its own checklist. Findings get severity tags so the reviewer can decide what blocks merge.
Severity scale
blocker: must fix before merge
major: fix before next release
minor: improve when convenient
nit: style preference, take or leave
Stages
Stage 1: Architecture
- Are module boundaries respected?
- Is coupling between components appropriate?
- Are there leaky abstractions?
- Does this change introduce circular dependencies?
- Is the layer of abstraction consistent within each function?
Stage 2: Quality
- Are names clear and specific (not "data", "info", "manager")?
- Is there duplication that should be extracted?
- Is there complexity that should be inlined (KISS)?
- Dead code, unreachable branches, commented-out code?
- Inconsistent error handling within the same function?
- Magic numbers or strings that should be named constants?
Stage 3: Tests
- Are new code paths covered?
- Are edge cases tested (empty, null, boundary, max)?
- Do assertions check behavior, not implementation details?
- Are tests isolated (no shared mutable state)?
- Are test names descriptive of what they verify?
- Any tests that could pass against incorrect code?
Stage 4: Performance
- Algorithmic complexity reasonable for input size?
- N+1 queries or redundant database round trips?
- Unbounded memory growth (lists, caches, accumulators)?
- Missing caching where output is deterministic and expensive?
- Synchronous operations that could be async?
- Hot paths doing unnecessary allocations?
Stage 5: Security
- Input validation on all external inputs?
- SQL injection, command injection, path traversal vectors?
- Authentication and authorization on every endpoint?
- Secrets in code, logs, error messages, or commits?
- Cryptographic primitives used correctly (no MD5 for security, no ECB mode, proper IV handling)?
- CSRF, XSS, SSRF surface in web code?
- Race conditions on shared state?
Output format
For each stage, emit a markdown table with this shape:
## Stage 1: Architecture
| Severity | File:Line | Finding | Fix |
|---|---|---|---|
| <tag> | <ref> | <one line> | <one line> |
Repeat for stages 2, 3, 4, and 5.
If a stage has zero findings, write No findings. under the stage heading.
End with a summary block:
## Summary
- Blockers: <count>
- Majors: <count>
- Minors: <count>
- Nits: <count>
## Verdict
<APPROVE | REQUEST_CHANGES | COMMENT>
Verdict rules:
APPROVE only if zero blockers and zero majors
REQUEST_CHANGES if any blocker or three or more majors
COMMENT otherwise
Hard rules
- Run all five stages even if one finds nothing.
- Do not invent issues to look thorough. If the code is clean, say so.
- For diffs over 500 lines, ask the user to split before reviewing.
- Quote exact code in findings. Vague references like "around the validation logic" are not actionable.
1---2name: code-reviewer3description: Run a structured five-stage code review on a diff, file, or pull request. Use whenever the user wants code reviewed, audited, or checked before merge. Triggers on "review this code", "audit this PR", "code review", "check this diff", "review my changes", or any time the user pastes code and wants quality, performance, or security feedback. Always use this skill rather than ad-hoc commentary when the user asks for a code review, even if they describe the request in their own words.4---56# Code Reviewer78Five-stage pipeline. Each stage runs independently with its own checklist. Findings get severity tags so the reviewer can decide what blocks merge.910## Severity scale1112- `blocker`: must fix before merge13- `major`: fix before next release14- `minor`: improve when convenient15- `nit`: style preference, take or leave1617## Stages1819### Stage 1: Architecture2021- Are module boundaries respected?22- Is coupling between components appropriate?23- Are there leaky abstractions?24- Does this change introduce circular dependencies?25- Is the layer of abstraction consistent within each function?2627### Stage 2: Quality2829- Are names clear and specific (not "data", "info", "manager")?30- Is there duplication that should be extracted?31- Is there complexity that should be inlined (KISS)?32- Dead code, unreachable branches, commented-out code?33- Inconsistent error handling within the same function?34- Magic numbers or strings that should be named constants?3536### Stage 3: Tests3738- Are new code paths covered?39- Are edge cases tested (empty, null, boundary, max)?40- Do assertions check behavior, not implementation details?41- Are tests isolated (no shared mutable state)?42- Are test names descriptive of what they verify?43- Any tests that could pass against incorrect code?4445### Stage 4: Performance4647- Algorithmic complexity reasonable for input size?48- N+1 queries or redundant database round trips?49- Unbounded memory growth (lists, caches, accumulators)?50- Missing caching where output is deterministic and expensive?51- Synchronous operations that could be async?52- Hot paths doing unnecessary allocations?5354### Stage 5: Security5556- Input validation on all external inputs?57- SQL injection, command injection, path traversal vectors?58- Authentication and authorization on every endpoint?59- Secrets in code, logs, error messages, or commits?60- Cryptographic primitives used correctly (no MD5 for security, no ECB mode, proper IV handling)?61- CSRF, XSS, SSRF surface in web code?62- Race conditions on shared state?6364## Output format6566For each stage, emit a markdown table with this shape:6768````69## Stage 1: Architecture70| Severity | File:Line | Finding | Fix |71|---|---|---|---|72| <tag> | <ref> | <one line> | <one line> |73````7475Repeat for stages 2, 3, 4, and 5.7677If a stage has zero findings, write `No findings.` under the stage heading.7879End with a summary block:8081````82## Summary83- Blockers: <count>84- Majors: <count>85- Minors: <count>86- Nits: <count>8788## Verdict89<APPROVE | REQUEST_CHANGES | COMMENT>90````9192Verdict rules:9394- `APPROVE` only if zero blockers and zero majors95- `REQUEST_CHANGES` if any blocker or three or more majors96- `COMMENT` otherwise9798## Hard rules99100- Run all five stages even if one finds nothing.101- Do not invent issues to look thorough. If the code is clean, say so.102- For diffs over 500 lines, ask the user to split before reviewing.103- Quote exact code in findings. Vague references like "around the validation logic" are not actionable.