code-review

Files (4)

• SKILL.md
• correctness-taxonomy.md · references/
• performance-review.md · references/
• security-review.md · references/

SKILL.md (first lines)

# Code Review

## Purpose

Most review output is noise: a list of things that *look* wrong, unranked, unrefuted, and impossible
to act on. This skill produces the opposite — a small number of findings, each with a required
behaviour, a feasible trigger, a concrete contradiction, an observable consequence, and the strongest
counterargument already checked.

Its central bet: **the defects reviewers miss are rarely visible inside one file.** They are
disagreements between two participants that each look reasonable alone — a caller and a callee, a
producer and a consumer, a writer and a later reader, two branches that should establish the same
state. A checklist applied file-by-file cannot see those, because the two halves are never in view at
the same time. So the unit of review here is the **agreement**, not the file.

## Structure: one engine, three anchors

Code review is the skill. **Correctness is its core** — the dimension generic tooling covers worst,
and the one described in full below. **Performance and security are sub-cases**: the same engine, the
same refutation discipline, the same report contract, with a different anchor and one or two extra
rules each.

| Sub-case | Anchor | Where its rules live |
|---|---|---|
| **Correctness** *(core, default)* | Agreements between participants across a boundary | This file + `references/correctness-taxonomy.md` |
| **Performance** | Workload → resource demand → growth or contention → consequence | `references/performance-review.md` |
| **Security** | Source → trust boundary → sink, with an attacker controlling the source | `references/security-review.md` |

Read a sub-case's file only when that sub-case is selected. Each is short on purpose: it states what
*differs*, and the rest of this file still applies.

Sub-cases are independently *activated*, not mutually exclusive. One root cause can carry correctness
and security impact — report it once, with both impacts.

## Invocation

```
/pm-ai-shipping:code-review
/pm-ai-shipping:code-review dimensions=correctness scope=changes
/pm-ai-shipping:code-review dimensions=performance,security
/pm-ai-shipping:code-review dimensions=all
```

Claude Code ships its own bundled `/code-review`. Use the plugin-qualified form above when you mean
this one.

These are instruction arguments, not shell flags.

- **Default: `correctness`.** Bare "review this" or "find bugs" means correctness only.
- An explicit list selects exactly those sub-cases; `all` selects three. Never silently reinterpret
  an unknown or empty selection — ask.
- **Scope:** use what was asked. Otherwise review working changes if present, else the repository.
- **State the selected dimensions, the scope and the comparison baseline before investigating.**
- Reviewing changes means following dependencies *beyond* the changed lines, and distinguishing
  defects the change **introduced** from defects it merely **revealed**.
- Review and report. Apply fixes only when asked.

## Shared engine

Every sub-case uses one skeleton. Only the anchor and the refutation rules differ.

**Map a flow → identify an obligation → inspect every participant → construct a violating execution
→ trace the consequence → attempt refutation → report.**

Build one minimal map first: inputs, major execution flows, who owns which state, external
dependencies, observable effects. Each selected sub-case enriches it — do not build three maps, and
do not make a security-only run wait on correctness mapping.

## Correctness: the agreement engine

A *boundary* is semantic, not a file split. It separates a caller and a callee, two callbacks, two
executions of the same function, a producer and a consumer, or a value written now and read later.

For each consequential agreement, hold these in working notes — not in the report:

```
Participants:
Value, entity or effect exchanged:
Authority (who decides the real answer):
Identity and lifetime/version:
Required relationship:
Evidence for that relationship:
Relevant transitions or orderings:
Observable consumer or consequence:
```

**Establish the obligation without inventing intent.** Evidence comes from specifications,
documented contracts, language or protocol semantics, tests that encode an expectation, or a
necessary producer/consumer relationship. A consumer's implementation alone does not prove the
consumer is right. Where participants disagree, say why the disagreement produces a *wrong outcome* —
sometimes the contradiction is certain while which side should change is genuinely open. Missing
documentation is a limitation, not automatically a finding.

**Start where agreements are most likely to break:** values transformed or negotiated, identities
reassigned, work becoming asynchronous, state persisted and reloaded, several effects that must
agree. Then do a local pass over ordinary decisions, arithmetic, boundaries and error branches — the
anchor must not become a filter that discards plain bugs.

### Force a violating execution

A suspicion is not a finding until you construct the execution that breaks it. Where the
implementation permits:

- make a **requested** value differ from the **accepted or effective** one;
- keep two operations live at once and vary their completion order;
- change the relevant identity or generation between observation and use;
- compare distinct transitions that should end in equivalent state;
- inject failure between effects, and interruption before completion;
- exercise empty, exact-boundary and adjacent-boundary inputs.

Establish that each case is actually reachable. Do not assume it.

### Two lenses that need a forced probe, not a mention

Across a large evaluation of planted runtime defects in real codebases, two classes were almost never
*even reported* by strong agents — not missed at the fix, missed at the look. Naming them in a
checklist will not help; each needs an explicit probe:

1. **Authority reconciliation.** Follow a proposed value through validation, normalisation,

From phuryn/pm-skills, MIT.

More from phuryn/pm-skills