code-review

Files (1)

• SKILL.md

SKILL.md (first lines)

# Code review

Review code changes on two independent axes, then aggregate findings.
Both axes run as **parallel sub-agents** to prevent one axis's findings
from biasing the other.

## Process

1. **Pin the fixed point.** Identify the reference commit/branch/tag to
   diff against. Validate it exists and confirm the diff is non-empty.

2. **Identify the spec source.** Search commit messages for issue
   references, check user-provided paths, or look in `docs/`,
   `docs/agents/spec.md`. Ask the user if no spec source is found.

3. **Identify standards.** Gather repo documentation:
   `CODING_STANDARDS.md`, `CONTRIBUTING.md`, root `CLAUDE.md`, or
   equivalent. These override the baseline but don't replace it.

4. **Spawn two parallel sub-agents:**

   ### Axis 1: Standards
   Does the code conform to this repo's documented coding standards?
   Plus the smell baseline below. Report violations concisely (< 400
   words). Skip anything already enforced by tooling.

   ### Axis 2: Spec
   Does the code faithfully implement the originating spec or ticket?
   Flag missing requirements, scope creep, and implementation errors.
   Report concisely (< 400 words).

5. **Aggregate.** Present both reports under separate headings. Do not
   merge findings across axes.

## Smell baseline

Eleven heuristics from Fowler's *Refactoring* plus depth/locality
checks from `/codebase-design`:

| Smell | Signal |
|-------|--------|
| **Mysterious Name** | Reader can't guess what it does from the name |
| **Duplicated Code** | Same structure in multiple places |
| **Feature Envy** | Method uses another class's data more than its own |
| **Data Clumps** | Same group of fields/params appear together repeatedly |
| **Primitive Obsession** | Primitives where a domain type would clarify |
| **Repeated Switches** | Same switch/conditional scattered across the code |
| **Shotgun Surgery** | One change requires edits in many unrelated places |
| **Divergent Change** | One module changes for multiple unrelated reasons |
| **Speculative Generality** | Abstractions for hypothetical future needs |
| **Message Chains** | Long chains of calls (a.b().c().d()) |
| **Middle Man** | Class that only delegates without adding value |
| **Shallow Module** | Interface nearly as complex as implementation — low depth, callers gain little leverage |
| **Locality Violation** | Related changes scatter across many files instead of concentrating in one module |
| **Hypothetical Seam** | An abstraction boundary with only one adapter — variation doesn't actually exist |

Repo-specific standards override the baseline.

## What this skill must never do

- Never merge the two axes into a single report.
- Never run both axes in the same agent context.
- Never flag issues already caught by automated tooling.
- Never apply the smell baseline rigidly — documented intentional
  patterns override it.

From lynchypin/project-onboard, MIT.

More from lynchypin/project-onboard