domains / software-engineering / code-review / code-review

code-review

Skill card

combined

The four skills define fundamentally different review engines. mattpocock/skills uses a two-axis (Standards vs Spec) parallel sub-agent model anchored to a git diff. phuryn/pm-skills uses a defect-focused 'agreement engine' that forces violating executions to prove bugs. dzhng/skills is a heuristic checklist for code hygiene (naming, comments, complexity). davepoon/buildwithclaude is a thin dispatcher for a phase-based workflow that spawns a specific agent. Because they disagree on the core methodology (diff-based vs defect-proving vs heuristic vs phase-based) and each contains unique steps (e.g., Fowler smells, violating execution construction, stale reference grepping, phase validation), crowning one would hide the others' distinct approaches.

Repositories

Other copies

Combined skill

---
name: code-review
description: "Review code changes using a multi-strategy approach: diff-based standards/spec review, defect-proofing via violating executions, or heuristic hygiene checks. Select the strategy based on user intent and context."
---

# Code Review

This skill supports three distinct review strategies. Determine which to use based on the user's request and the available context.

## Strategy Selection

1. **Diff-Based Standards & Spec Review** (Use when: user provides a base commit/branch and wants to check against repo standards or a specific issue/spec).
2. **Defect-Proofing Review** (Use when: user asks to "find bugs", "audit for correctness", or "check if a fix is safe" without a specific spec).
3. **Heuristic Hygiene Review** (Use when: user asks to "clean up code", "review naming/comments", or after a refactor to remove stale references).

## Strategy 1: Diff-Based Standards & Spec Review

1. **Pin the Fixed Point**: Identify the base (commit, branch, tag). Verify it resolves and the diff is non-empty using `git diff <base>...HEAD`.
2. **Identify Spec Source**: Look for issue references in commits, user-provided paths, or spec files in `docs/`/`specs/`. If none, ask the user.
3. **Identify Standards Sources**: Find `CODING_STANDARDS.md`, `CONTRIBUTING.md`, etc. Always include the **Smell Baseline** (Fowler's code smells: Mysterious Name, Duplicated Code, Feature Envy, etc.).
4. **Run Parallel Sub-Agents**:
   - **Standards Agent**: Check diff against documented standards and the Smell Baseline. Report hard violations vs. judgement calls.
   - **Spec Agent**: Check diff against the spec. Report missing requirements, scope creep, and incorrect implementations.
5. **Aggregate**: Present findings under `## Standards` and `## Spec` headings.

## Strategy 2: Defect-Proofing Review

1. **Map the Flow**: Identify inputs, execution flows, state owners, and external dependencies.
2. **Identify Agreements**: Find boundaries where participants (caller/callee, producer/consumer) must agree on state or values.
3. **Force Violating Executions**: For each agreement, construct a scenario that breaks it (e.g., race conditions, identity changes, boundary inputs). Prove reachability.
4. **Refute Candidates**: Attempt to disprove each suspected defect. Only report if the violation is feasible and the consequence is observable.
5. **Report**: List findings with the required behavior, trigger, contradiction, and consequence.

## Strategy 3: Heuristic Hygiene Review

Review the code against these principles:
1. **Names reflect reality**: Update names if the mechanism changed.
2. **No stale references**: Remove dead logic, abandoned flags, and comments about removed code.
3. **Simplify guards**: Check only what matters; avoid chained fallbacks.
4. **Comments document WHY**: Keep technical discoveries; remove narrative of changes.
5. **Separate install/runtime**: Don't mix setup and execution logic.
6. **No intermediary artifacts**: Remove workarounds for abandoned approaches.
7. **Tighten guarantees**: Remove redundant null checks if a value is now guaranteed.
8. **Start clean if tangled**: If a file is messy, restore and re-apply changes.
9. **Verify values in tests**: Assert on actual values, not just collection sizes.
10. **No thin wrappers**: Delete re-export-only modules.
11. **Extract around ownership**: Split files only for coherent responsibilities.
12. **Decouple tests**: Drive the system end-to-end where possible.

## Conflicts

- **mattpocock/skills** vs **phuryn/pm-skills**: The former focuses on conformance to standards/specs; the latter focuses on proving defects via execution. They are complementary but distinct.
- **dzhng/skills** vs **davepoon/buildwithclaude**: The former is a manual heuristic checklist; the latter is an automated phase-based dispatcher. Use dzhng for ad-hoc cleanup and davepoon for structured phase reviews.