review-loop

What this skill does

# Review Loop

A source-agnostic, convergence-paced review-resolve loop for code/PR diffs: it drives a pluggable review source to convergence (verdict=approve), auto-applying mechanical fixes and gating the judgment calls.

## Caller Signature

```
/review-loop [source?] [scope?]

source : { review-ensemble | codex | code-review }   -- optional; review source behind the (diff) → { findings[], verdict } interface
                                                     --   absent → Phase 0 asks which source to use (no default)
scope  : PR number | (implicit)                      -- optional; PR number, or implicit current-PR / working-tree detection
```

The review source is pluggable: any source satisfying the `(diff) → { findings[], verdict }` interface can drive the loop. `review-ensemble`, `codex`, and `code-review` are the three sources documented in the Source Interface section; all are runtime-selected, not fixed at definition time. When `source` is omitted, Phase 0 asks which source to use (no preselected default). When `scope` is omitted, Phase 0 detects it (current-branch PR or working tree).

## Pipeline Overview

```
/review-loop [source?] [scope?]
  Phase 0  : source designation (arg → relay | absent → ask) + scope detect (PR diff | working tree)
  Phase 1  : review    — source(diff) → { findings[], verdict }
  Phase 2  : verify     — per finding: /inquire (vs codebase) + /contextualize (vs work-flow);
                          drop findings failing support-integrity / context-fit (cite basis)
  Phase 3  : classify  — Mechanical → Extension (auto)
                          Judgment   → cluster by shared disposition → Constitution scope-gate
  Phase 4  : apply      — /attend risk classification → apply approved edits
  Phase 5  : re-review  — source(diff') → verdict'
               verdict'=approve (or 0 new) → converge ; else round k+1: these findings → Phase 2 (re-review already done; no second source call)
  free-exit : user may end the loop at any time (declared once in Phase 0)
```

The loop is the skill's identity; the review source is a parameter behind it. Loop control (verify → classify → apply → re-review) is fixed; the source that produces `{ findings[], verdict }` is swappable.

## When to Use

- Driving code/PR review findings all the way to resolution and convergence (verdict=approve)
- A review pass surfaced findings and you want them verified, dispositioned, applied, and re-checked in one controlled loop
- Multiple review rounds are expected before the diff is clean

## When NOT to Use

- Reviewing a markdown artifact before fixation — that is `/comment-review` (this skill targets code/PR diffs)
- Wanting a one-shot cross-model review with no apply phase — that is `/review-ensemble` alone (it surfaces findings and a verdict without applying fixes; `/review-loop` is the loop that drives those findings to approve)
- Trivial single-line edits where a direct Edit is faster than a review loop

## Phase 0: Source Designation + Scope Detection

**Source designation.** If a `source` argument is given, use it directly — this is relay (Extension): the user already decided. If `source` is absent, **ask** — an init Constitution gate with no preselected default: present the available sources as a choice (`review-ensemble` for cross-model coverage when the `prothesis:frame` skill is available; `codex` for a single independent external model when the codex CLI is present; `code-review` for a Claude-native built-in review, always available), each with its coverage/cost trade-off, and let the user constitute the selection. Unless a `source` is named at invocation, the loop does not pick one on the user's behalf — source selection determines the cost and coverage of every round, so it waits for the answer.

**Scope detection** (mirrors review-ensemble Phase 1):

1. PR number given as `scope`: scope = `gh pr diff {N}`
2. No PR argument: `gh pr view --json number,title,headRefName,changedFiles 2>/dev/null` to detect a current-branch PR; if found, scope = its diff
3. No PR: scope = working tree (`git diff HEAD`)
4. No changes anywhere: ask the user what to review (stop here)

Capture the **resolved base SHA** (the merge-base or PR base commit the diff is taken against) and the changed-files list — this base SHA + file list is the **pointer** the codex prompt passes; codex re-derives the diff locally with its own git (read-only sandbox, no network), so the full diff content is not inlined. Also capture diff stats for context.

**Free-exit affordance (declared once).** Announce here, before the first review round: *"You can end this loop at any time by saying so; on exit I will present the convergence trace so far and stop."* This is a free-response pathway, not a gate option — it does not reappear as a peer option at later gates.

## Phase 1: Review

Call the designated source over the current diff and obtain `{ findings[], verdict }`. Each finding carries the form `[severity] file:line — description`; `severity ∈ critical | high | medium | low | suggestion`; `verdict ∈ approve | needs-attention`. The per-source mechanics (how `review-ensemble` versus `codex` produce this output) live in the Source Interface section below — Phase 1 only consumes the interface.

## Phase 2: Verify (against codebase + work-flow)

Before acting on any finding, verify it — a review model can hallucinate or flag a stale issue, and currency is not the same as fidelity. For each finding:

- Call `/inquire` to check the finding against the current codebase: does the asserted issue actually hold against the code as it stands? This is the support-integrity / currency check — an artifact being fresh does not establish that it tracks current behavior.
- Call `/contextualize` to check fit against the work-flow and surrounding context: is the flagged pattern intentional here, and is the proposed fix contextually appropriate for this codebase's conventions?

This phase is primarily relay — read-only codebase checks with cited basis. A finding that fails verification (the issue does not hold, or the fix is contextually wrong) is **dropped**, and the drop is surfaced with its cited basis rather than silently discarded or applied. Genuine Constitution uncertainties that `/inquire` raises (where the user's judgment is genuinely needed) fold forward into the Phase 3 disposition gate rather than firing as separate in-round chat gates — keeping the in-round gate count down to the Phase 3 cluster gates.

## Phase 3: Classify + Disposition Gate

Classify each surviving finding:

- **Mechanical** — typo, rename, mechanical symbol or format fix; deterministic, one correct edit, no design judgment. → **Extension**: auto-apply, no gate. Mechanical findings keep the loop running — only Constitution warrants interruption.
- **Judgment** — multiple valid resolutions, a design tradeoff, or a change with irreversible divergence. → **Constitution**: the user's judgment constitutes the resolution.

**Settled-policy check (before opening any disposition gate).** A finding whose disposition policy was already constituted in a prior round is **Extension by default**: a consistent application of that settled policy auto-resolves per the prior disposition (apply / dismiss / defer), and any non-trivial side-effect (for example, an edit that touches a line outside the original diff hunk) is surfaced as a **relay annotation on its Relay trace entry**, not as a gate. The Extension/Constitution boundary for a finding moves with what is already decided — a prior round's direction and the PR's stated purpose can close an axis that would otherwise be live. A gate reopens only when a genuinely competing disposition (defer / separate follow-up / dismiss) is still live — one not already foreclosed by the PR's purpose or an established precedent. Re-gating the consistent propagation of settled policy is the over-gating failure mode.

For Judgment findings whose axis remains live, **cluster by shared disposition**