code-reviewer

Files (3)

• SKILL.md
• dimensions.md
• special-cases.md

SKILL.md (first lines)

# Code Reviewer

This is the **standards pass**: the six dimensions, the repo's documented
standards, the smell baseline, and the complexity lens. Checking the diff
against its spec (the issues a PR closes) is a separate pass that needs its
own context, so the review-pr skill runs both in parallel and reports them
side by side.

Report findings; leave the edits to whoever acts on the review. Skip anything
a linter, type checker, or automated reviewer already flags, and add context
rather than noise.

## 1. Gather the change

This skill runs forked, with no conversation history, so take everything from
git and the files themselves rather than from "the change we just discussed".

```bash
git diff <base>...HEAD --stat   # the branch against its base, e.g. main...HEAD
git diff <base>...HEAD
git log <base>..HEAD --oneline
git diff -- path/to/file.js     # one file
```

Review the whole branch by default; use the range or paths the prompt names
when it names one. Done when you have
read every changed file and can say what the change does and why.

## 2. Review it

Apply each dimension; [dimensions.md](dimensions.md) holds the questions, red
flags, and example feedback for each.

- **Correctness**: logic, semantic correctness, type safety. Correctness
  comes first: a type error, logic bug, or data race blocks approval.
- **Design**: architecture, interfaces, patterns. Design decisions propagate,
  so a poor one blocks downstream work.
- **Readability**: naming, clarity, documentation.
- **Performance**: efficiency, algorithms, resource usage.
- **Testing**: coverage, edge cases, test quality.
- **Security**: injection, auth, secrets. Any vulnerability (injection, auth
  bypass, secret exposure) is BLOCKING.

**Documented standards.** Cite the repo's written standards (`CLAUDE.md`,
`AGENTS.md`, `CONTRIBUTING.md`, `CODING_STANDARDS.md`, path-scoped rule
files) by file and rule. A documented standard is the strongest evidence a
finding can have.

**Smells.** Flag Fowler code smells as judgement calls (SUGGESTION or
NITPICK), and let a documented standard that endorses the pattern override
them: mysterious name, duplicated code, feature envy, data clumps, primitive
obsession, repeated switches, shotgun surgery, divergent change, speculative
generality, message chains, middle man, refused bequest. Name the smell and
quote the hunk; skip a hunk already reported under Complexity.

**Complexity lens.** Report what the diff could delete, one line each,
numbered `C1.`, `C2.`, ... so they don't collide with other findings and the
user can say "fix 2 and C3". Tag each:

- `delete:` dead or speculative; nothing replaces it.
- `reuse:` duplicates a helper already in the repo; name its path.
- `stdlib:` hand-rolls what the standard library ships; name the function.
- `native:` does what the platform already does; name the feature.
- `yagni:` one implementation, config nobody sets, a single-caller layer.
- `shrink:` the same logic in fewer lines; show the shorter form.

End with `net: -N lines possible` or "Lean already." Leave alone what the
minimal-change skill always keeps: trust-boundary validation, data-loss error
handling, security, accessibility, hardware calibration, explicit
requirements, and the one runnable check that non-trivial logic keeps.

```text
C1. src/repo.py:88 — yagni: AbstractRepository has one implementation. Inline it until a second exists.
C2. src/dates.ts:4 — native: moment.js imported for one format call. Intl.DateTimeFormat, no dependency.
net: -40 lines possible
```

Some changes need more depth: read [special-cases.md](special-cases.md) for a
large PR, a design that touches several systems, a refactor or migration of
legacy code, or a new or changed third-party dependency.

Done when every changed file has been weighed against all six dimensions, the
documented standards, the smells, and the complexity lens.

## 3. Classify and report

Classify each finding:

- **BLOCKING**: cannot merge without fixing (correctness, security, design
  that breaks contracts).
- **SUGGESTION**: worth addressing; improves quality (minor design, a
  performance optimization).
- **NITPICK**: nice to have (naming, a formatting edge case).

Report in the code-reviewer agent's output format, which the review-pr skill
parses: `## Review Summary`, then `### BLOCKING`, `### SUGGESTIONS`,
`### NITPICKS`, `### Complexity` (ending on its `net:` line), and
`### What Looks Good`. Open the summary with a verdict: "Approved", "Request
changes", or "Comment". Suggestions stay optional when the code is otherwise
solid.

Done when every finding carries a severity and a `file:line`, and the summary
opens with the verdict.

## Related

- `/review-pr`: the full PR review lifecycle, running this standards pass and
  the spec pass side by side.
- The `code-reviewer` agent: the forked context this skill runs in
  (harness-kit plugin, or project `.claude/agents/code-reviewer.md`). If it is
  unavailable, apply the review inline in this context.
- `CLAUDE.md`: project-specific review standards.

From dougborg/harness-kit, Apache-2.0.

More from dougborg/harness-kit