Review Patterns

Code Review

Patterns and methods for reviewing code changes against specifications and quality standards.

Review Method

Goal: Does the implementation match the spec? Nothing more, nothing less.

Step 1 - Extract every requirement from the task spec:

**Requirements Checklist:**
- [ ] Requirement 1: [exact text from spec]
- [ ] Requirement 2: [exact text from spec]

Step 2 - For EACH requirement:

1. Find the code that implements it
2. Read the actual code (don't trust the report)
3. Verify it satisfies the requirement
4. Mark: pass (met), fail (not met), or partial (partially met)

Step 3 - Check for scope creep:

• Features not in the spec
• Extra parameters/options not requested
• Abstractions beyond what was needed
• "Nice to have" additions

Scope creep is a failure. Building more than requested wastes time.

Pass 2: Code Quality

Only run this pass AFTER spec compliance passes.

Security:

• No hardcoded secrets
• Input validation at boundaries
• No injection risks (SQL, XSS, command)
• Proper auth checks

Performance:

• No N+1 query patterns
• Appropriate caching considerations
• Efficient algorithms for data size

Patterns:

• Follows existing codebase conventions
• No unnecessary complexity
• Clear naming and structure
• Appropriate error handling

Testing:

• Tests cover new functionality
• Edge cases considered
• Tests verify behavior (not mock behavior)

Pass 3: Reference Integrity (conditional)

Run this pass ONLY when changes involve renaming, moving, or deleting files, functions, exports, config keys, or documentation references. Skip entirely for changes that only add new code or modify internal logic.

Trigger conditions (any of these):

• A file was renamed, moved, or deleted
• A public function, class, or export was renamed
• A config key, environment variable, or CLI flag changed
• A documentation file, command, or skill was modified

Method:

1. For each renamed/moved/deleted item, search the codebase for the OLD name/path
2. For each modified export or API, search for consumers
3. For each changed config key, search code and docs that reference it
4. Check README files, CLAUDE.md, AGENTS.md, and documentation for stale references

This is a mechanical check - grep for old names and flag anything that still uses them.

Severity Levels

Scope Creep Red Flags

Security Checklist

Before marking COMPLIANT, verify:

• No hardcoded credentials, API keys, or secrets
• User input is validated and sanitized
• SQL queries use parameterized queries (no string concatenation)
• HTML output is escaped to prevent XSS
• File paths are validated (no path traversal)
• Authentication and authorization checks are present where needed
• Sensitive data is not logged or exposed in error messages

Communication Protocol

Message the lead when:

• Review is complete (pass or fail)
• A blocker is found that requires scope change
• After 3 review iterations, if still NON-COMPLIANT

Output Format

<reviewer-result>
status: [COMPLIANT | NON-COMPLIANT | PARTIAL]
requirements_total: [number]
requirements_met: [number]
scope_creep_found: [yes/no]
quality_issues: [number]
critical_issues: [number]
stale_references: [number or "n/a"]
</reviewer-result>

Pass 1: Spec Compliance

[IF NON-COMPLIANT:]

Missing Requirements:

1. [Requirement]: [What's missing, where it should be]

Scope Creep Found:

1. [Extra feature]: [Where it is, why it's not in spec]

[IF COMPLIANT:] All [N] requirements verified in code. No scope creep detected.

Pass 2: Code Quality

Pass 3: Reference Integrity (if applicable)

[IF NO RENAMES/MOVES: "Pass 3 skipped - no renames, moves, or deletions detected."]

Summary:

• Spec: [COMPLIANT/NON-COMPLIANT]
• Quality: [N] issues ([N] critical, [N] high, [N] medium, [N] low)
• References: [N stale / clean / skipped]
• Recommendation: [PASS / FIX REQUIRED / BLOCKED]