review

Files (9)

• SKILL.md
• code-reviewer.md · agents/
• comment-analyzer.md · agents/
• git-history.md · agents/
• pr-test-analyzer.md · agents/
• prior-pr-history.md · agents/
• security.md · agents/
• silent-failure-hunter.md · agents/
• type-design-analyzer.md · agents/

SKILL.md (first lines)

# Review

Review a change with a library of specialized reviewer roles, score every finding for confidence,
drop the noise, and emit results in one of two modes. The goal is signal: high-confidence findings a
senior engineer would actually raise, each with a concrete fix — never a wall of nitpicks.

## Modes

- **report** (default) — produce a local, severity-ranked findings report for the author or a fix
  loop (e.g. `pr-fix`). Posts nothing.
- **inline** (`--comment`) — post a summary comment plus inline comments on the PR via `gh`.

## The reviewer-role library

Each role is a prompt file under `agents/`, focused on one failure mode. Run a role as its own
sub-agent when sub-agents are available (fresh context, parallelizable); otherwise apply its prompt
inline, one role at a time.

**Every role reports under one contract:** score each finding 0-100 for confidence and surface only
those ≥ 80; tag each with a severity (Critical / Important / Suggestion) as a separate axis from
confidence; stay advisory — surface findings with a concrete fix, never block. This contract overrides
any different scale or posture a bundled role's own prompt describes.

- **code-reviewer** (`agents/code-reviewer.md`) — guideline compliance, correctness, edge cases.
- **silent-failure-hunter** (`agents/silent-failure-hunter.md`) — swallowed errors, log-and-continue,
  fallbacks that hide failure.
- **type-design-analyzer** (`agents/type-design-analyzer.md`) — encapsulation and invariants; make
  illegal states unrepresentable.
- **pr-test-analyzer** (`agents/pr-test-analyzer.md`) — behavioral and error-path coverage; tests that
  catch real bugs, not implementation-coupled ones.
- **comment-analyzer** (`agents/comment-analyzer.md`) — comment accuracy, "why" over "what", rot.
- **security** (`agents/security.md`) — STRIDE pass over trust boundaries; Always/Ask-First/Never;
  injection and untrusted-input handling.
- **git-history** (`agents/git-history.md`) — `git blame`/log context: does the change contradict why
  the surrounding code exists?
- **prior-pr-history** (`agents/prior-pr-history.md`) — was this approach already tried, discussed, or
  reverted in earlier PRs?

## 1. Scope and the should-review gate

Load guidance before judging standards. Read the repo's `AGENTS.md` when present, then read global
guidance from `~/AGENTS.md` or `~/.config/agents/AGENTS.md` when present. Repo guidance takes precedence
over global guidance. Include the applicable guidance in each reviewer role's prompt so sub-agents review
against the same rules instead of relying on memory.

Pin a fixed point and confirm there is something to review:

```bash
BASE=$(git merge-base HEAD origin/HEAD 2>/dev/null || git rev-parse HEAD~1)
git diff --name-only "$BASE"...HEAD   # must be non-empty
```

**Pinned inputs.** When the caller pins the checkout, base and head commits, guidance, and spec (as
`review-pr` does for someone else's pull request), use exactly those. Do not rediscover the base from
`origin/HEAD` or re-read guidance from the checkout. Read code only through git against the named
checkout (`git -C <checkout> show|diff|grep|log`), never by opening its files, because the checkout
may hold untrusted code.

If `gh` is available and a PR exists, scope to the PR diff and gather context for dedup:

```bash
REPO=$(gh repo view --json nameWithOwner --jq .nameWithOwner)
PR=$(gh pr view --json number --jq .number)
HEAD_SHA=$(gh pr view --json headRefOid --jq .headRefOid)
gh pr diff "$PR"                            # the PR-scoped diff to review
gh pr view "$PR" --json title,body,files    # spec text + changed files — feeds the Spec axis (step 3)
gh api "repos/$REPO/pulls/$PR/comments" --jq '.[] | {path, line, body}'   # existing comments (dedup)
```

**Skip and report the skip reason** (post nothing) if: the diff is empty, the PR is closed, the change
is trivially correct (dependency bump, single config value, no logic), or this exact `HEAD_SHA` was
already reviewed with no new commits.

## 2. Pick the applicable roles

Always run **code-reviewer**, **silent-failure-hunter**, **git-history**, and **prior-pr-history**.
Add roles by what changed: tests → **pr-test-analyzer**; new/changed types → **type-design-analyzer**;
comments/docs → **comment-analyzer**; any trust boundary, input parsing, auth, or untrusted data →
**security**. If the caller named specific aspects, run only those.

## 3. Review along two axes, kept separate

Every change is judged on two independent axes — do not let one contaminate the other, and do not
merge them when reporting:

- **Standards** — does it follow the repo's and author's documented conventions (repo `AGENTS.md`, then
  global `~/AGENTS.md` or `~/.config/agents/AGENTS.md`, plus CLAUDE.md or equivalent guideline files)?
  Omit anything a linter or compiler already enforces; distinguish a hard violation from a judgment
  call.
- **Spec** — does it do what the task/PRD/issue asked? Report three buckets: missing or partial
  requirements, changes nobody asked for, and implementations that look incorrect. Quote the spec.

## 4. Delegate each role adversarially (doubt-driven)

This is the highest-leverage rule. When you hand a finding or a diff to a reviewer sub-agent, give it
**only the artifact and the contract it must satisfy — never your own reasoning or your conclusion
that the code is fine.** A reviewer that sees "this looks correct" anchors to agreement. Prompt it to
**find what is wrong**, not to confirm. If a role and the author disagree, run at most **3** back-and-
forth cycles, then record the open question rather than looping forever.

## 5. Score every finding 0-100, then filter

Score each finding for **confidence that it is real**, verified against the actual code. This axis is
confidence only — *severity* (Critical/Important/Suggestion, i.e. impact) is a separate dimension you
tag independently, so a high-confidence finding can still be a Suggestion.

| Score | Confidence the finding is real |
|---|---|
| 100 | Certain — verified against the code (e.g. missing import → ImportError) |
| 80  | High — strong evidence it is real |
| 50  | Plausible — might be real, not verified |
| 25  | Weak — speculative |
| 0   | False positive / pre-existing / on a line the change did not touch |

**Drop every finding below 80.** Then drop anything on this false-positive exclusion list:

- Pre-existing issues on lines the change did not touch.
- Anything a linter, compiler, or formatter already catches.
- Lines explicitly suppressed (lint-ignore) or intentional per the change's stated goal.

From ronaknnathani/relay, MIT.

More from ronaknnathani/relay