| name | code-review-and-quality |
| description | Two-stage code review: spec compliance first, then code quality. Use when reviewing code changes, conducting PR reviews, or when subagents complete implementation tasks. Ensures code matches spec AND meets quality standards.
|
| version | 1.0.0 |
| permissions | ["fs.read","shell.exec"] |
| triggers | [{"context":"reviewing code"},{"context":"PR review"},{"context":"subagent completed a task"},{"command":"/review"}] |
| platforms | ["claude-code","cursor","gemini-cli","copilot","codex","opencode","windsurf","kiro"] |
| dependencies | ["test-driven-development@^1.0.0"] |
| model_variants | {"claude":{"enforcement_style":"human-partner collaborative"},"gemini":{"enforcement_style":"structured-checklist"},"gpt":{"enforcement_style":"directive-imperative"}} |
| author | codehands-core |
| signed | true |
| tier | 1 |
Overview
Code review has two jobs: verify the code does what it should (spec compliance) and verify it does it well (quality). These are separate stages — don't mix them. Spec issues block quality review. Quality issues block merge.
When to Use
- When reviewing any code changes before merge
- When a subagent completes an implementation task
- During PR review
- During
/review phase
- NOT for reviewing specs or plans (use brainstorming/planning skills)
Process
Stage 1: Spec Compliance Review
Complete this stage BEFORE starting Stage 2. If spec issues exist, they must be fixed first.
- Read the spec/plan task. What was the implementation supposed to do?
- Read the code changes.
git diff or review the changed files.
- Check completeness:
- ✅ Every requirement from the spec implemented?
- ✅ All acceptance criteria satisfied?
- ❌ Nothing EXTRA added (no scope creep)?
- ❌ Nothing MISSING from requirements?
- Verdict:
- PASS → Proceed to Stage 2
- FAIL → List specific issues. Implementer fixes. Re-review.
Stage 2: Code Quality Review
- Correctness: Does the code actually work? Are edge cases handled?
- Tests: Do tests cover the new behavior? Are they meaningful (not just "passes")?
- Readability: Can someone understand this code without explanation?
- Security: Any of the
codehands:security-and-hardening red flags present?
- Performance: Any obvious performance issues (N+1 queries, unnecessary allocations)?
- Consistency: Does the code follow existing patterns? (
codehands:source-driven-development)
- Simplicity: Is there a simpler way to achieve the same result?
Output Format
## Code Review
### Spec Compliance: [PASS / FAIL]
- [List of spec issues if FAIL]
### Quality Review: [APPROVED / CHANGES REQUESTED]
**Strengths:**
- [What was done well]
**Issues:**
| Severity | File:Line | Issue | Suggestion |
|---|---|---|---|
| Important | src/auth.ts:45 | SQL concatenation | Use parameterized query |
| Suggestion | src/auth.ts:12 | Magic number | Extract as constant |
| Nit | src/auth.ts:3 | Unused import | Remove |
**Verdict:** [APPROVED / CHANGES REQUESTED (with priority)]
Severity Levels
| Severity | Meaning | Action |
|---|
| Blocker | Security vulnerability, data loss risk, or spec violation | Must fix before merge |
| Important | Bug, logic error, or significant quality issue | Must fix before merge |
| Suggestion | Better approach exists but current works | Fix if easy, otherwise note for future |
| Nit | Style, naming, formatting | Fix if easy, don't block on it |
Common Rationalizations
| Rationalization | Reality |
|---|
| "The code works, that's good enough" | Working code that's unreadable, insecure, or untested is technical debt. Review exists to catch what "works" misses. |
| "I don't want to be too picky" | Specific, actionable feedback is not picky — it's professional. Vague "looks good" reviews are worthless. |
| "Spec compliance and quality are the same thing" | No. Code can perfectly match the spec but be poorly written. Code can be beautifully written but miss requirements. Two stages. |
| "Self-review is sufficient" | Self-review catches 40% of issues. External review catches 80%. The 40% gap is where bugs live. |
Red Flags
- Reviewing quality before spec compliance (wrong order)
- "LGTM" without reading the code
- No review comments at all (either everything is perfect or nothing was reviewed)
- Blocking on nits while ignoring important issues
- Accepting "close enough" on spec compliance
Verification
See Also
codehands:security-and-hardening — Security checks during reviewcodehands:performance-optimization — Performance checks during reviewcodehands:code-simplification — Simplification during reviewcodehands:multi-agent-orchestration — Review pipeline for subagent output
