mhylle/claude-skills-collection/code-review

Code Review

<!-- contents: generated by tests/skill_structure.py --write; edit headings, not this block -->

Contents

• Design Philosophy
• When to Use This Skill
• Review Dimensions
• Review Workflow
• Integration with implement-phase
• Invoking the Skill
• Review Checklists
• Severity Levels
• Critical Rule: All Errors Are New Errors
• Best Practices
• Example Review Session

Reference files (open one when a step points to it):

• references/adr-compliance-checklist.md: ADR Compliance Checklist
• references/coding-standards-checklist.md: Coding Standards Compliance Checklist
• references/framework-standards-checklist.md: Framework Standards Compliance Checklist
• references/general-quality-checklist.md: General Code Quality Checklist
• references/service-delegation-checklist.md: Service Delegation & Single Responsibility Checklist

<!-- /contents -->

Systematic code review skill that validates implementation quality across six dimensions: service delegation, framework standards, ADR compliance, plan synchronization, coding standards, and general code quality.

Design Philosophy

This skill acts as a quality gate between implementation phases. It ensures code meets established standards before proceeding, catching issues early when they're cheapest to fix.

When to Use This Skill

Automatically invoked by implement-phase (Step 4):

• After the phase passes exit conditions and integration testing (Steps 2–3)
• Before marking a phase as complete
• Before proceeding to the next phase

Manually invoked when:

• Reviewing a PR or set of changes
• Validating code before committing
• Checking implementation against architectural decisions

Review Dimensions

1. Service Delegation & Single Responsibility

Verify that implementation follows proper separation of concerns:

2. Framework Standards Compliance

Verify code follows the project's established patterns:

3. ADR Compliance

Verify architectural decisions are followed and documented:

4. Plan Synchronization

Verify the implementation matches the phase as the plan specifies it. Per ADR-0001 the plan is a specification and progress is tracked outside it, so don't expect ticked checkboxes or status edits in the plan.

The scope comes from the plan file's phase section, or from the phase description and requirements the caller passes. With no plan or phase scope given (a plain manual review), report this dimension as PASS with "No plan given".

A missing work item is BLOCKING. Unplanned work, or a deviation with no evident reason, is a RECOMMENDATION: bring the code back in line with the plan or state the reason. A deviation with a reason, and an ADR reference the plan lacks, are NOTES: implement-phase records both in the plan at Step 6, after this review, so they need not be there yet.

5. General Code Quality

Standard code review concerns:

6. Coding Standards Compliance

Verify implementation follows project coding standards:

Source of truth: docs/standards/CODING_STANDARDS.md when it exists. When it doesn't, the project's own conventions are the standard: observe them in neighbouring code (Review Workflow, Step 2) and flag only departures from what that code consistently does. Framework-specific checks in the checklists (marked NestJS, TypeScript, …) apply only when that framework is detected in the project, or when the standards file adopts them.

Review Workflow

Step 1: Gather Context

1. READ the plan file to understand:
   - Current phase being reviewed
   - Expected deliverables (the objective and work items dimension 4 checks against)
   - Exit conditions that should be met
 
2. READ docs/decisions/INDEX.md to identify:
   - ADRs relevant to this change
   - Recently added ADRs
 
3. IDENTIFY changed files using git:
   - git diff --name-only [base]...HEAD
   - Or use provided file list

Step 2: Framework Pattern Analysis

1. DETECT the stack from manifest files (package.json, pyproject.toml, go.mod,
   *.csproj, pom.xml, ...). Framework-specific checklist items apply only to
   frameworks found here.
 
2. READ docs/standards/CODING_STANDARDS.md if it exists.
 
3. FIND the project's conventions for each kind of changed file
   (services, controllers, tests, error handling, logging, config):
   - Glob for siblings of the same kind (e.g. other *.service.ts in the module)
   - Read two or three of them; Grep for the idioms they share
   - In a large or unfamiliar codebase, if Agent is in your tool list, spawn an
     Explore subagent instead and ask for the conventions with file:line examples
 
4. COMPARE changed code against identified patterns

Step 3: Review Each Dimension

Each dimension produces the same block, however it runs:

DIMENSION: [name]
STATUS: PASS | NEEDS_CHANGES
FINDINGS:
- [BLOCKING|RECOMMENDATION|NOTE] path:line — issue — fix

If `Agent` is in your tool list and the change spans more than a few files, review the dimensions in parallel: send six Agent calls in one message, one per dimension, with subagent_type: "general-purpose". Subagents start without your context, so each prompt carries the changed files, the context from the table below, the path to its checklist in this skill's references/ directory, the Severity Levels table, an instruction not to edit files, and the block above as the required reply. Subagents run in the background; wait for all six completion notifications before writing the report.

Agent(
  description="Review: service delegation",
  subagent_type="general-purpose",
  prompt="Read-only code review; do not edit any file.
Dimension: service delegation & single responsibility.
Files: src/auth/auth.service.ts, src/auth/auth.guard.ts, src/auth/jwt.strategy.ts
Checklist: <absolute path to this skill>/references/service-delegation-checklist.md
Classify each finding BLOCKING, RECOMMENDATION or NOTE: <Severity Levels table>
Reply with only:
DIMENSION: service-delegation
STATUS: PASS | NEEDS_CHANGES
FINDINGS:
- [BLOCKING|RECOMMENDATION|NOTE] path:line — issue — fix"
)

Otherwise (no Agent in your tool list, or a change small enough to review in one pass), work through the six dimensions yourself, in order, as a checklist, and produce the same six blocks.

Step 4: Generate Review Report

Output a structured review report. A dimension's Status is NEEDS_CHANGES if it has any BLOCKING or RECOMMENDATION finding; NOTES alone leave it PASS.

# Code Review: [Phase/Feature Name]
 
**Date:** YYYY-MM-DD
**Files Reviewed:** [count]
**Verdict:** PASS | NEEDS_CHANGES
 
## Summary
 
[1-2 sentence overall assessment]
 
## Review Results
 
### Service Delegation & Single Responsibility
Status: PASS | NEEDS_CHANGES
 
[Findings or "All checks passed"]
 
### Framework Standards Compliance
Status: PASS | NEEDS_CHANGES
 
[Findings or "All checks passed"]
 
### ADR Compliance
Status: PASS | NEEDS_CHANGES
 
[Findings or "All checks passed"]
 
### Plan Synchronization
Status: PASS | NEEDS_CHANGES
 
[Findings or "All checks passed"]
 
### General Code Quality
Status: PASS | NEEDS_CHANGES
 
[Findings or "All checks passed"]
 
### Coding Standards Compliance
Status: PASS | NEEDS_CHANGES
 
[Findings or "All checks passed"]
 
## Required Changes
 
[Numbered list of every BLOCKING issue and RECOMMENDATION, each tagged with its class, file:line and the fix, or "None"]
 
## Notes
 
[Informational findings that don't affect the verdict, or "None"]
 
## Files Reviewed
 
| File | Status | Notes |
|------|--------|-------|
| `path/to/file.ts` | OK | |
| `path/to/other.ts` | ISSUE | [Brief note] |

Integration with implement-phase

When invoked by implement-phase (Step 4 of the phase pipeline), this skill:

1. Receives context: Phase number, changed files, plan path
2. Executes review: All six dimensions
3. Returns structured result:

STATUS: PASS | NEEDS_CHANGES
VERDICT: [Brief summary]
CHANGES_REQUIRED: [Count of blocking issues + recommendations; 0 on PASS]
BLOCKING_ISSUES:
- [path:line — issue — fix, or "None"]
RECOMMENDATIONS:
- [path:line — issue — fix, or "None"]
NOTES:
- [Informational items, or "None"]
REPORT: [Path to full review report, if written to disk]