pr-review · git:20260804.f88de7a · 2026-08-04 · sha256 4927efe2811971a5

pr-review git:20260804.f88de7aA

Immutable. This exact content is served forever at /api/v1/blob/4927efe2811971a5.

---
name: pr-review
description: Independently review a pull request that resolves an issue. Reconstructs the issue contract from repository evidence, dispatches the specialized FSL reviewer agents in parallel, audits the diff for green-faking, reproduces verification claims, and delivers a severity-ranked verdict without merging.
---

# Review a pull request

Orchestrate an independent review of a pull request, typically one authored by an agent to
resolve an issue. This skill is the coordination layer; the per-dimension analysis belongs to
the specialized read-only reviewer agents under `.claude/agents/`. Diagnose and report — do not
push fixes onto the branch, and never merge (leave the PR open for human review).

Inputs: a PR number or branch, and the linked issue. If no issue is linked, ask for the intended
contract before reviewing.

Lenses that apply to every phase:

- Trust order per `AGENTS.md`: contracts and tests, then the Rust implementation, then frozen
  Python behavior, then prose. The PR description and any implementer conversation are claims,
  not evidence.
- Suspect green. A confidently green false negative is more dangerous than a crash, so "the
  checks pass" is never sufficient on its own.
- Findings outside the diff still carry a reporting duty: an accepted construct with absent,
  placeholder, or hollow semantics is a soundness defect wherever it is found.

## Phase 0 — Reconstruct the contract

1. Read the PR (`gh pr view`, `gh pr diff`), the linked issue, and any `docs/DESIGN-*.md` the
   change touches or should have touched.
2. Restate, from the issue and repository evidence only: the requested outcome, the affected
   authority surface, the invariants at risk, and the verification the change needs.
3. Note which CI lanes actually ran. `merge readiness` is a bounded fail-fast lane
   (`docs/DESIGN-ci.md`), not product verification — record what remains unverified.

## Phase 1 — Contract alignment

Map the issue's acceptance criteria to the diff in both directions:

- Required but missing: criteria with no corresponding change or test.
- Present but not required: scope creep, speculative fallbacks, compatibility work no contract
  calls for.

Check authority placement: new behavior lives under `rust/`, not the frozen `src/fslc/`; design
decisions are backed by an accepted design note; the change is the smallest contract-preserving
one.

## Phase 2 — Dispatch the specialized reviewers

Select every applicable reviewer from the table and launch the selected ones in parallel. Each
returns compact findings; do not re-run their file-level sweeps in the main thread.

The reviewer agents have tight turn budgets and routinely exhaust them mid-exploration. In each
dispatch prompt: point the agent at a checkout of the PR's tree (create one with
`git worktree add <scratchpad>/prNNN <head-ref> --detach`; remove it after the review), state
the base commit for diffing, enumerate the audit items, and instruct the agent to reserve budget
for the final report — evidence gathered but never reported is wasted. If an agent still stops
without a report, resume it with a "produce your final report now" message instead of
re-launching it.

| Diff touches | Reviewer agent |
| --- | --- |
| Grammar, lowering, CLI commands, public Kernel/JSON contracts, docs, corpus specs | `fsl-coupled-change-reviewer` |
| `fsl-core`, `fsl-runtime`, `fsl-verifier`, `fsl-solver*`, refinement or dialect semantics | `fsl-soundness-reviewer` |
| `.fsl` files under `specs/` or `examples/` | `fsl-vacuity-reviewer` |

A language-feature change dispatches the coupled-change reviewer even when the diff looks
complete — the coupled-change list (LSP index, `LANGUAGE.ja.md` alignment, dialect registry,
changelog) is exactly what implementers forget.

## Phase 3 — Green-faking audit

On the diff itself, in the main thread, check for changes that make checks pass without
honoring the contract:

- A `.fsl` spec, property, or invariant weakened relative to the base branch.
- Hand-edited generated snapshots, or snapshots regenerated without an intentional contract
  change to justify the diff.
- Growth in an allowlist, exclusion, or known-divergence list (`tests/dialect_registry.py`
  entries, `KNOWN_DIVERGENT_*` fixtures, skip markers) presented as a fix.
- Errors suppressed, downgraded, or swallowed instead of resolved.
- Test assertions loosened, deleted, or rewritten to follow the new behavior without evidence
  the new behavior is the contract.
- New positive-path tests with no negative control — a test suite that cannot reject a known
  contract-violating variant proves nothing about drift detection.
- Weak assertions: each new test must verify through the strongest available oracle (re-parse,
  type-check, `build_model`, exact message/location/exit code), not substring or line-presence
  matching. Issue #691 survived characterization tests precisely because they matched substrings
  without re-parsing the rendered output. Ask of every new test: would it have caught the bug it
  claims to prevent?

## Phase 4 — Reproduce verification claims

Re-run the narrowest relevant tests and `fslc` commands yourself in the PR's worktree state and
compare observed output with the PR body's claims. Do not accept a pass that was only planned,
only claimed, or only observed in an earlier tree state. Delegate verbose or ambiguous failure
output to `fsl-test-diagnostician` and keep exact commands, exit codes, and failing test names.

## Phase 4b — Adversarial verification (soundness-critical PRs)

When the central change touches verifier, lowering, or dialect semantics, reproduced green is
still the implementer's chosen evidence. Add evidence the implementer did not choose:

1. **Novel inputs.** Author 2–3 new `.fsl` fixtures that exercise edge shapes the PR's own tests
   avoid (adjacent type constructors, generated names, boundary arities, nesting the fix does
   not demonstrate). Run them through the strongest oracle on the PR tree — for two-path
   surfaces, execute both paths and compare verdicts, not text. Prompt the authoring agent to
   refute the PR's claim, not to confirm it. Keep these fixtures in the review packet; promote
   any that find a divergence into the PR's regression suite as a requested change.
2. **Live mutation of the negative control.** For the PR's central fix, revert or corrupt the
   fixed arm in the scratch checkout and observe the claimed guard test actually fail. A
   negative control verified only by reading is uncalibrated; one compile cycle buys executed
   proof that the gate can detect the regression it pins. Restore the checkout afterwards.

Skip this phase for doc, fixture-only, or tooling changes; say so explicitly in the verdict.

## Phase 5 — Synthesize the verdict

Build a claim ledger before writing the verdict: enumerate every verification claim made by the
PR body and every claim the verdict itself is about to make, and classify each as verified by
execution (command + observed output), verified by reading (file:line evidence), or unverified.
The verdict must name its unverified residue explicitly — a claim the review neither executed
nor read is an assumption, and certifying it is the reviewer's own green-faking. Rank the
remainder by severity:

1. Soundness defect (false-negative risk, hollow accepted semantics, broken
   symbolic/concrete/BFS agreement).
2. Contract violation or drift (public envelope, exit codes, accepted design notes).
3. Coupled-change gap (missing docs, LSP index, registry, changelog, skill reference).
4. Test-evidence gap (missing negative control or boundary case, unreproduced claim).
5. Style and convention.

Separately, record local-optimum escalation triggers without acting on them: the fix works by
adding a compatibility layer or special-case branch; one semantic change fans out across many
files; the same shape of fix recurs across recent PRs; an allowlist or exclusion list grows
monotonically. Two or more recorded triggers on the same surface warrant a standalone
local-optima audit outside any single PR review — note it, do not run it here.

## Phase 6 — Deliver and persist

1. Post the review with `gh pr review` (comment or request-changes; approve only when every
   severity-1/2 finding is resolved or refuted). State what was reproduced, what was not, and
   what remains unverified. Never merge.
2. Every out-of-scope soundness finding must land as an issue URL or an explicit unresolved
   follow-up in the task packet — never only in the review text or chat.
3. Promote durable lessons (a recurring finding class, a missed coupled-change edge) to the
   smallest authoritative surface: a test, a design note, or an agent rule.