Review Code

review-code — one dimension per invocation

Audit a change along one review dimension and report findings. Load only that dimension's reference.

Input

$ARGUMENTS carries the dimension plus optional scope, e.g. dimension=code-bugs or code-bugs src/foo.ts. A manifest gate's body activates this skill with the dimension under /do's selected evaluator; the gate names the dimension and this skill owns the threshold. If no dimension is given, list the available dimensions and ask which one — one invocation, one dimension.

Dimensions and thresholds

Load references/<dimension>.md for the requested dimension and follow it. The threshold is the bar a change must clear to PASS on that dimension:

The split is structural: defect-finders report only divergences/defects/contract-mismatches/type-holes that name their trigger — every LOW there is real signal. Advisory dimensions surface taste-level improvements where LOW is usually could-be-better, not is-broken.

Some dimensions are conditional, and the caller decides whether they apply before requesting them: contracts where the change touches API surfaces or durable data contracts, type-safety on typed code, and defect-class where the change fixes a defect. defect-class is the one dimension that takes a defect as input rather than producing one as output — it asks whether the fix accounted for every site its mechanism reaches, so on a change that fixes nothing it has no input and nothing to judge. Requesting it anyway is not an error; it reports PASS and says no fix was found in scope.

Determining scope (shared across dimensions)

1. Caller specifies files/paths → review exactly those.
2. Otherwise → review the complete change. Resolve the comparison base from the PR target or the repository's remote default branch; ask only if neither can be established. Inspect committed changes with git diff <base>...HEAD, staged changes with git diff --cached, unstaged changes with git diff, and relevant new files from git ls-files --others --exclude-standard. Read each new file's contents; a path listing alone is not its diff.
3. Empty / non-reviewable diff → ask the caller to clarify scope.

Stay within scope — never audit the whole project unless explicitly asked. Skip generated files (*.generated.*, generated/, dist/, build/), lock files, vendored deps (vendor/, node_modules/, third_party/), and binaries.

Reading scope ≠ reporting scope. The diff bounds what you report (defaults: only issues introduced or exposed by the change). It does not bound what you read — trace mechanisms wherever they lead (existing callers, shared state, upstream writers) so boundary issues between changed and unchanged code stay visible. Read widely; report narrowly.

This skill is read-only: never modify repository files; write only to /tmp/ for analysis artifacts. The author implements fixes from your report.

Severity ladder (shared)

• Critical — blocks release: data loss, corruption, security breach, or complete failure for all users; no workaround.
• High — blocks merge: core/happy-path functionality broken for common inputs.
• Medium — edge cases, degraded behavior, or failures needing multiple preconditions.
• Low — rare scenarios with workarounds, or (for advisory dimensions) taste-level could-be-better.

Each dimension's reference refines these with domain-specific calibration — defer to it.

Report format (shared)

# Code Review — <dimension>

**Dimension**: <dimension>
**Threshold**: no LOW+ | no MEDIUM+
**Status**: PASS | FAIL
**Files analyzed**: [...]

## Findings
### Finding #1: [title]
- **Location**: `file:line`
- **Severity**: Critical | High | Medium | Low
- **Description**: [what's wrong]
- **Trigger**: [the condition under which the defect manifests — the concrete input, state, or sequence that produces the failure, or for a static mismatch such as a contract or signature divergence, the call or consumer path that reaches it — required on defect-finder dimensions]
- **Impact**: [what it breaks / why it matters — on advisory dimensions a Medium-or-higher finding must name the concrete cost that earns that grade: the future change it makes harder, the reader or operator task it defeats, or the condition under which behavior degrades]
- **Recommended fix**: [specific change]

## Summary
- Critical: N | High: N | Medium: N | Low: N
- Verdict: PASS if no finding meets/exceeds the dimension threshold, else FAIL.

An empty report (Status: PASS) is a valid, expected outcome. Do not invent findings to fill it.

Verifier contract (PASS / FAIL / BLOCKED)

When invoked as a manifest acceptance gate, end with one verdict:

• PASS — no finding at or above the dimension's threshold.
• FAIL — at least one finding meets/exceeds the threshold; the findings list is the fix hint.
• BLOCKED — the dimension can't be evaluated yet (e.g., the change can't be located, a needed external doc is unreachable). Say what blocks it and what would unblock it.

Gotchas

• One ref only. Load exactly the reference for the requested dimension. Loading several defeats progressive disclosure and blurs the orthogonality boundaries each dimension defines against its neighbors.
• Respect the role threshold. Reporting a LOW on an advisory dimension as if it blocks (or dropping a LOW on a defect-finder) miscalibrates the gate.
• Reportability is a stated trigger, not a confidence level (defect-finders). Name the condition under which the defect manifests: "this WILL fail when X", with X written down. Where the defect is static rather than sequential — a contract or signature mismatch that is wrong on every call — the trigger is the call or consumer path that reaches it. A finding that cannot name its trigger is not reportable however strongly you suspect it — "this might be a bug", "this looks fragile", "this could break under load" all fail the test. Someone reading only the finding must be able to tell whether it clears this bar, which is what holds the bar in one place across reviewers rather than moving it with whoever is reading. A stated trigger is necessary rather than sufficient — each dimension's own actionability filter still applies on top of it. An empty report beats false positives.
• A Medium is a stated cost, not a strength of feeling (advisory dimensions). Medium is the blocking grade there, so it carries the same auditability the defect-finders get from a stated trigger: name the cost in the finding — the change made harder, the task defeated, the condition under which behavior degrades — specifically enough that someone reading only the finding can tell the grade is earned. A finding whose case rests on "could be cleaner", "feels off", or "not ideal" has not named one, and sits at Low. This holds the bar in one place across reviewers and across models rather than letting it move with whoever is reading; each dimension's own calibration still applies on top.
• Stay inside the loaded dimension. Each reference lists what it owns and what belongs to a sibling dimension. A maintainability smell is not a bug; a type hole is not a design critique. Report only what the loaded dimension owns; let the other dimensions' invocations catch the rest.