review

name

review

description

Unified code review — supersedes code-review. Classifies PRs by change type, size, and risk to select the right specialized reviewers (3-8 instead of all 15). Dispatches in sequential batches, validates findings with review-judge, and produces actionable output. Triggers on: 'review this code', 'review PR #N', 'review PR <url>', 'review current branch', 'review uncommitted changes', 'code review', or any natural request to review code.

Unified Code Review Orchestrator

You are the review lead. You detect the review source, dispatch specialized reviewers, validate their findings with a judge, and produce actionable output.

This skill supersedes the code-review skill from shop-pi-fy. If both are available, always use this one. This version adds: smart source detection, a judge validation step, model diversity across reviewers, and actionable output (PR comment drafts or fix plans).

Step 1: Detect the Review Source

Parse the user's input to determine the review source. Try each in order:

1a. PR number or URL

If the user provided a PR number (e.g., #1234, PR 1234) or URL:

Resolve the PR system first per ../_shared/pr-system.md — GitHub and Meteorite PRs have separate number namespaces and different CLIs. Every command in the rest of this skill branches on the resulting PR_SYS.

# PR_SYS=gh — for a PR number, detect the repo from the current directory; a URL carries org/repo/number
gh pr view <number_or_url> --json title,url,baseRefName,headRefName,body

# PR_SYS=gs — `--json` takes no field list and there is no `--jq`; pipe through jq
gs pr view <number> --json | jq

Extract the base and head branches, then get the diff:

gh pr diff <number_or_url>   # PR_SYS=gh
gs pr diff <number>          # PR_SYS=gs

Set SOURCE_TYPE=pr and save the PR metadata (title, URL, number, headRefName) for output formatting. For a Meteorite PR the browsable link is htmlUrl — never report the url field, which is the unopenable api.gitstream.shopify.io host. gs pr view --json field names differ from gh; run gs pr view <number> --json | jq keys once before scripting against them.

Checkout into a WTP worktree

For PR reviews, always checkout the PR branch into an isolated WTP worktree so reviewers can read, grep, and find the full codebase — not just the diff. Follow the WTP checkout flow in ../_shared/wtp-checkout.md:

WTP_BIN="$HOME/src/github.com/shopify-playground/wtp/bin/_wtp"
BRANCH="<headRefName from PR metadata>"
TARGET_DIR="$($WTP_BIN "$BRANCH")"
cd "$TARGET_DIR"
git fetch origin "$BRANCH"
git checkout "$BRANCH" 2>/dev/null || git checkout -b "$BRANCH" "origin/$BRANCH"
git reset --hard "origin/$BRANCH"

Set REVIEW_CWD=$TARGET_DIR. All subsequent tool calls (read, grep, find) and all reviewer subagent cwd fields should use this path. This ensures reviewers see the actual source code at the PR's HEAD, not just the diff.

Report the worktree claim:

Checked out [branch] in [TARGET_DIR] (WTP slot: [slot name])

If WTP is unavailable (not installed, no free slots): fall back to diff-only review. Warn the user:

⚠️ WTP unavailable — reviewing from diff only. Reviewers cannot read surrounding code.

Cleanup: After the review is complete and the user has finished acting on findings, free the WTP slot:

cd ~
"$WTP_BIN" free

Or if running from a different directory, use _wtp free <slot> with the slot name.

1b. Uncommitted changes

If no PR was specified, check for uncommitted changes:

# Staged + unstaged changes
git diff HEAD

If the diff is non-empty, set SOURCE_TYPE=local_uncommitted.

1c. Current branch

If no uncommitted changes, diff the current branch against its base:

# Determine the base branch
git merge-base HEAD main || git merge-base HEAD master

# Diff against base
git diff $(git merge-base HEAD main)..HEAD

If the diff is non-empty, set SOURCE_TYPE=local_branch.

1d. Nothing to review

If all of the above produce empty diffs, inform the user:

"No changes found to review. Checked: uncommitted changes, staged changes, and current branch against main."

Stop here.

1e. Detect merge conflicts

After getting the diff, check for unresolved conflict markers:

git diff --check HEAD 2>&1 | grep -i "conflict"

If conflicts exist, stop and inform the user with affected files.

Step 2: Gather and Sanitize the Diff

2a. Sanitize

Large or noisy diffs waste reviewer context. Apply these filters:

Strip binary files — replace with:

Binary file changed: path/to/image.png (+X KB)

Summarize autogenerated files — do NOT include full diffs for:

• Lockfiles (package-lock.json, yarn.lock, Gemfile.lock)
• Generated types (GraphQL codegen, protobuf, OpenAPI)
• Schema dumps (schema.rb, structure.sql)
• Snapshots, VCR cassettes
• Vendored dependencies

Replace each with:

Autogenerated file changed: package-lock.json (+450/-320 lines, not shown)

2b. Assess diff size

After sanitization, note:

• Raw diff: total files and lines (before filtering)
• Reviewable diff: files and lines of human-written code (after filtering)

Include both in context passed to reviewers.

2c. Inform the user

Before dispatching reviewers, briefly tell the user what you found:

Reviewing [SOURCE_TYPE]: [title/branch name] > [N files, +X/-Y lines reviewable] — dispatching [M] reviewers...

This keeps the user informed during the parallel phase, which can take 30-60 seconds.

Step 3: Classify and Select Reviewers

Read references/reviewer-matrix.md for the full classification rules and reviewer mapping.

3a. Auto-discover all review-* agents

List all available agents and filter for names starting with review-. This picks up:

• Built-in reviewers (scope, architecture, security, performance, correctness, nullsafety, testing, operations, shopify)
• Craft reviewers (design, simplify, consistency, naming, readability, intent)
• Any user-defined review-* agents (e.g., review-accessibility, review-i18n)

Exclude review-judge — the judge is dispatched separately in Step 5.

Custom review-* agents not in the matrix are added to the optional set and get a dynamic section in the output.

3b. Handle user-specified subset

If the user specified reviewers (e.g., "review this code — just security and performance"):

• Map short names to agent names: security → review-security
• Run ONLY those (do not force review-scope unless the user included it)
• Skip classification entirely — the user's explicit selection overrides all heuristics

3c. Classify the PR

When no user subset is specified, classify the diff to determine which reviewers to run. Using the changed files list and PR metadata from Step 2:

1. Detect language — from file extensions in the diff (see matrix)
2. Detect change type — evaluate rules in order, first match wins (see matrix)
3. Detect size — from reviewable line count (Step 2b)
4. Detect risk signals — check all, additive (see matrix)

Report the classification to the user:

Classification: [change_type] ([language], [size]) [risk signals if any] Core reviewers: [list] | Optional: [list]

5. Map to reviewers — look up the change type in the matrix to get core and optional sets
6. Apply risk escalation — add risk-signal reviewers to core if not already present
7. Apply size adjustments — per the matrix rules:
   • tiny: core only, skip optional, skip judge if 0 findings
   • small: core only, optional only if core produces 3+ findings
   • medium: core + optional
   • large: core + optional + always include intent

3d. Model rationale

Each reviewer's model is declared in its agent frontmatter — that is the source of truth. Do not override models here.

Design principle: Most individual reviewers use Sonnet because (1) each covers a narrow domain where Sonnet is sufficient, (2) the judge (Opus) validates all findings downstream so no reviewer output reaches the user unchecked, and (3) parallel Opus calls would be prohibitively slow and expensive. The two exceptions — review-design (reads surrounding code broadly) and review-judge (validates every claim) — use Opus because they require the deepest reasoning.

Step 4: Dispatch Reviewers

Kick off devx pair-review in the background (PR mode only)

Before composing agent tasks, if SOURCE_TYPE=pr and the user did not opt out, start devx pair-review as a background job so it runs in parallel with the specialist agents (it is slow — minutes). It provides a second independent review source (Claude Opus 4.8) that feeds the judge alongside the agents in Step 5.

Read references/pair-review.md for eligibility, invocation gotchas, and the exact bg_run command. Kick it off from $REVIEW_CWD with the full PR URL (bare PR numbers fail git detection in World/Gitstream), capture the job id, then proceed to dispatch the agents.

Skip this entirely for local_uncommitted / local_branch (no PR to fetch) or if the tool is unavailable — pair-review is an enhancement, not a hard dependency, and the review degrades gracefully to agents-only.

4a. Compose the task

Each reviewer receives two things:

1. Review standards — read references/reviewer-persona.md and prepend its FULL content
2. Code context — the sanitized diff and changed files list

<review-standards>
[full contents of references/reviewer-persona.md]
</review-standards>

<review-source>
Source: [PR #N / uncommitted changes / branch: feature-xyz]
[PR URL if applicable]
</review-source>

<pr-description>
[Full PR title and body if SOURCE_TYPE=pr. Omit this section entirely for local_uncommitted or local_branch.]
</pr-description>

<code-changes>
[the sanitized diff from step 2]
</code-changes>

Set reviewer working directory: If REVIEW_CWD was set in Step 1a (WTP worktree checkout), pass it as cwd on every reviewer subagent task. This gives reviewers access to the full codebase via read, grep, and find:

{
  "agent": "review-correctness",
  "task": "<composed task>",
  "cwd": "<REVIEW_CWD>"
}

For local reviews (local_uncommitted, local_branch), omit cwd — reviewers inherit the current working directory which already has the code.

4b. Dispatch in sequential batches

Dispatch reviewers in two batches to limit parallel subagent load. This prevents the instability seen when 15+ subagents run simultaneously.

Batch 1 — Core reviewers:

{
  "tasks": [
    { "agent": "review-scope", "task": "<composed task>" },
    { "agent": "review-correctness", "task": "<composed task>" },
    { "agent": "review-architecture", "task": "..." }
  ]
}

Run only the core reviewers identified in Step 3c (typically 3-5 agents). Wait for all to complete.

Evaluate early exit (tiny/small PRs only):

• If size is tiny and batch 1 produced 0 findings → skip batch 2 and judge. Report clean.
• If size is small and batch 1 produced <3 findings → skip batch 2. Proceed to judge with batch 1 findings only.

Batch 2 — Optional reviewers:

{
  "tasks": [
    { "agent": "review-design", "task": "<composed task>" },
    { "agent": "review-naming", "task": "..." },
    { "agent": "review-simplify", "task": "..." }
  ]
}

Run the optional reviewers identified in Step 3c (typically 2-4 agents). Wait for all to complete.

Combine results from both batches and proceed to Step 5 (Judge).

If the user specified a subset (Step 3b), run all specified reviewers in a single batch — no core/optional split needed since the user has already curated the list.

4c. Detect and recover from silent batch failures

After every parallel batch returns, inspect the response before treating it as a real review.

Detect. Treat the batch as a silent failure if any of these are true:

• Summary reads Parallel: 0/N succeeded or any M/N succeeded where M < N
• An agent block renders as ## <agent-name> (failed) with empty / (no output) body
• A reviewer task body contains Agent error: or a transport/protocol error string

The reviewer never ran — the failure is in the subagent infrastructure or the underlying model call (Gemini 400 on thinking_budget, Anthropic overload, MCP transport, etc.). Do not pass empty output to the judge.

Surface. For each failed agent, re-dispatch as a single subagent call (not parallel) so the underlying error surfaces in full:

{
  "agent": "review-scope",
  "task": "<same composed task>",
  "cwd": "<REVIEW_CWD if set>"
}

Capture the error verbatim, add it to the user-facing report (see Step 6 — Reviewers status line), and continue with whichever reviewers succeeded — never abandon the whole review on a silent failure.

Mitigate. If the per-agent error is model-specific (thinking_budget out of range, Overloaded, rate-limit), retry the original batch without that agent and note the omission. If the same agent also fails on single re-dispatch, mark it failed and move on — do not loop. If >50% of a batch fails with infrastructure errors, flag the review as INCOMPLETE per Step 5d and ask the user whether to retry.

4d. Failure modes — quick reference

• 0/N succeeded on a parallel batch → re-dispatch each singly, capture errors, retry batch without the bad ones
• thinking_budget out of range (Gemini 2.5 Flash) → drop the agent, note in report, file follow-up
• Overloaded / rate_limited (Anthropic) or MCP transport error → one retry, then drop and report
• Empty body but succeeded count looks fine → treat as single-agent failure; re-dispatch that one agent

Step 5: Judge

After ALL reviewers complete, feed their combined output to review-judge.

Collect pair-review findings (if kicked off in Step 4)

If a devx pair-review job was started in Step 4, collect it now — before dispatching the judge:

1. bg_wait on the job (short timeout, e.g. 120s). If it is still running, proceed agents-only and note the timeout in the status line.
2. Parse the JSON envelope per references/pair-review.md: split suggestions into findings (severity high/medium/minor) and positive observations (null severity — fold these into the report's positive highlights, not the judge). A consolidation: "failed" outcome is fine — the judge dedupes.
3. If the job exited non-zero or ok:false, capture the .err reason, mark devx pair-review ❌ in the Reviewers status line, and run the judge with agent findings only.

The pair-review findings are merged into the judge task in Step 5b, tagged by source.

5a. Compress reviewer output

Always compress reviewer output before passing to the judge. Raw reviewer output contains verbose explanations, full code snippets, and educational context that's valuable for the final report but wastes judge context. The judge needs findings, not essays.

For each reviewer's output, extract into this compact format:

## [reviewer-name]
Findings: [count] | Clean: [yes/no]

1. [SEVERITY] [title] @ `file:lines`
   Problem: [one sentence]
   Fix: [one sentence or `[code at file:lines]`]
2. ...

Positive: [one-line summary of good observations, or "none"]

Rules:

• Strip all code snippets — replace with [code at file:lines] references the judge can look up itself
• Strip educational explanations ("this matters because...") — the judge doesn't need pedagogy
• Preserve: severity, location, problem, fix direction — these are what the judge validates
• Keep positive observations as a single line — the judge aggregates these
• If a reviewer found nothing: Findings: 0 | Clean: yes (one line, no elaboration)

This typically compresses 14 reviewer outputs from ~60-80KB to ~5-10KB — a 6-8x reduction that keeps the judge well within effective context even for large PRs.

Preserve the uncompressed originals — you will need them in Step 6

Compression is for the judge only. Step 6 renders the full finding detail for the user, so the originals must survive.

When a subagent batch result is truncated, the harness writes the full text to a file and prints the path (Full: /Users/…/pi-output-guard/subagent-*.txt). Record every one of those paths immediately — that file is the only remaining copy of the uncompressed reviewer output. Same for the pair-review JSON (/tmp/pair-review-<N>.json).

Keep a running manifest as you go:

batch 1 (scope, correctness, architecture, testing, comments, readability) → /Users/…/subagent-msf18780ii4e.txt
batch 2 (design, performance, intent)                                      → /Users/…/subagent-msf1j6qnzeq8.txt
judge                                                                      → /Users/…/subagent-msf296foqhmd.txt
pair-review                                                                → /tmp/pair-review-966288.json

In Step 6, read these back with read_output_chunk to recover any detail the compression dropped. Do not reconstruct a finding from memory or from the judge's one-line summary of it — go back to the source.

5b. Compose the judge task

Agents-only (no pair-review, or it failed/timed out):

You are judging code review findings from [N] specialized reviewers.

<review-source>
Source: [PR #N / uncommitted / branch]
[URL if applicable]
</review-source>

<diff>
[the sanitized diff — so the judge can verify claims]
</diff>

<reviewer-findings>

## review-scope
[scope reviewer output]

## review-architecture
[architecture reviewer output]

## review-security
[security reviewer output]

[... all reviewer outputs ...]

</reviewer-findings>

Two-source (agents + pair-review): when pair-review produced findings, add the dual-source preamble and group all findings into CONVERGENT / PAIR-REVIEW ONLY / AGENT ONLY buckets per references/pair-review.md. Preamble:

You are judging code review findings for [PR #N].

Findings come from TWO independent sources: (A) [N] specialized pi review agents,
and (B) Shopify's `devx pair-review` tool ([run.model]). Verify EVERY claim
against the real code, deduplicate, reconcile severity disagreements, reject
false positives. Note cross-source convergence (agents AND pair-review) vs
single-source — convergence raises confidence but you must still verify.

Then replace <reviewer-findings> with a <findings> block organized by convergence bucket. For CONVERGENT items, record each side's severity (e.g. [security=HIGH; pair-review=MEDIUM]) so the judge reconciles the disagreement. Keep pair-review findings in the same compact format as the compressed agent findings (title + severity + conf + file:line + one-line body).

5c. Dispatch the judge

{
  "agent": "review-judge",
  "task": "<composed judge task>",
  "cwd": "<REVIEW_CWD if set, else omit>"
}

The judge validates, deduplicates, rates, and produces the consolidated findings. Pass cwd so the judge can read and grep the actual source files to verify reviewer claims.

5d. Degraded mode

If the judge fails (timeout, error, empty output):

1. Retry once — transient failures are common with Opus.
2. If retry fails, inform the user:

   "The judge (review-judge) failed after retry. Presenting unvalidated reviewer findings — these have NOT been checked for false positives or severity accuracy. Treat with more skepticism than usual."

3. Fall back to raw synthesis: deduplicate obvious overlaps yourself, present findings ordered by severity, and mark the review as UNVALIDATED in the verdict.

If individual reviewers fail:

• Note which reviewers failed and why (if known) in the review output.
• If >50% of reviewers fail, flag the review as INCOMPLETE and suggest the user retry.
• Always continue with whatever reviewers succeeded — a partial review is better than no review.

Step 6: Format and Present Output

The zero-context rule

The user has not seen a single line of subagent output. Not the reviewers', not pair-review's, not the judge's. Everything they know about this review is what you put in this message.

That makes one phrasing pattern forbidden. Never write a finding as a reference to a conversation the user wasn't in:

❌ Assumes context	✅ Self-contained
"The correctness reviewer flagged a HIGH on the save path"	"Expired content reaches the save payload. handleSubmit passes Array.from(selectedIds) with no expiration re-filter (AdContentSelection.tsx:467) → handleSave spreads it into adContentIds (:104). No server guard in sync_ad_contents. Filed HIGH by review-correctness."
"The judge rejected finding 2 as invalid"	"Rejected — diff() rewrite would introduce a boundary bug. review-readability proposed replacing isBefore(now.add(31,'day'),'day') with diff(now,'day') <= 30. diff floors a millisecond delta, so a day-31 date scores 30 for any now past midnight."
"3-source convergence on the empty state"	"Empty state hard-codes one caller's default (AdContentSelection.tsx:310) — independently flagged by review-architecture, review-testing, and pair-review 1674."

A finding the user can't evaluate without asking you a follow-up question is a finding you rendered wrong.

Build the findings inventory

Before any verdict, produce a complete inventory covering every finding any source produced — including the ones you and the judge threw out. Write it to a file (/tmp/review-<N>-findings.md) so it survives compaction and can be grepped later, then present it in chat.

Each entry carries:

Field	Notes
ID + title	Stable, referenceable — the user will reply "drop 4, keep 7"
Filed → adjudicated severity	HIGH → MEDIUM if the judge downgraded it. Never show only the final value — the movement is the signal
Source(s)	Which agent(s), and/or pair-review with its finding id + confidence. Mark convergence explicitly
Location	file:line, verified to exist at HEAD
Problem	What is actually wrong, in plain terms. Not "reviewer X says…"
Proposed fix	Concrete, with the code where it helps
Verdict + evidence	Valid / rejected / downgraded / merged — and what was checked to decide that

Group by adjudicated severity, with rejections in their own section.

Rejected findings are first-class output — never collapse them

The rejections are frequently the most valuable part of the review, and they are the part the user cannot reconstruct on their own. Give each one full treatment, ahead of the verdict:

• What the reviewer proposed, quoted or shown as code
• Why it's wrong, with the specific evidence that settles it
• Whether the underlying concern is still valid even though the proposed fix isn't (this happens often — split them)

Two rejection classes deserve to be called out loudly, because they are the pipeline earning its cost:

• Suggestions that would introduce a bug — a "simplification" that breaks a boundary condition, a "fix" that does the opposite of its stated intent. Lead with these.
• Suggestions that would delete something load-bearing — a comment documenting a non-obvious invariant, a deliberate duplication that preserves a11y semantics. Say plainly that the author should keep what they have.

Never write "the judge filtered 5 false positives" and move on. Show all five.

Show the reclassification

When the judge changed a severity, merged duplicates, or dropped findings under the calibration rules, render the arithmetic so the user can audit it:

17 distinct findings → 10 posted
  1 HIGH   → MEDIUM   save-payload gap (not a regression; intended and tested)
  3 MEDIUM → dropped  merged into #2 (same root cause, 3 locations)
  5 LOW    → dropped  "don't raise" per review-calibration (premature optimization ×1, test nits ×3, speculative API ×1)

If you dropped findings under the don't raise rules in review-calibration.md, name them and name the rule. A silent drop is indistinguishable from an oversight.

Reviewers status line

Lead the consolidated output with a one-line roster showing each dispatched reviewer's result, including any silent failures recovered in Step 4c and the devx pair-review source when it ran:

Reviewers: review-correctness ✅ · review-architecture ✅ · review-simplify ✅ · review-scope ❌ (Gemini thinking_budget bug — see /tmp/...) · devx pair-review ✅ (Opus 4.8, N findings)

Use ✅ for completed (with or without findings), ❌ for failed/dropped, with the captured error in parentheses. This makes coverage gaps visible without burying them in the specialist sections. When pair-review ran, note whether any findings were convergent with the agents (e.g. "3 convergent, 2 pair-review-only") — convergence is a signal worth surfacing.

Do not dump raw specialist reports

The roster line above replaces the old per-reviewer transcript sections. Nine verbatim reviewer outputs is 40KB of overlapping prose the user will not read, and it buries the findings that matter.

Every finding is already in the inventory, attributed to its source. A reviewer that found nothing needs no section at all — ✅ on the roster line says it. Reach back into the preserved raw output (Step 5a manifest) only to recover detail for a specific finding, never to paste a whole report.

Attribution and disagreement

Attribute each finding to its source inside the entry, so convergence and single-sourcing are visible per finding rather than per section:

• [review-architecture + review-testing + pair-review 1674] — 3-source convergence, higher confidence
• [pair-review 1671, conf 0.90] — single-source, judge verified more carefully
• [review-comments] — single agent

When two sources disagreed, surface the disagreement and how it was settled. Reviewers contradicting each other is signal, not noise to be smoothed over — it usually means the call is genuinely a judgment call, and it's the user's to make. Render it as: what each side argued → what the judge checked → how it resolved.

The same applies when pair-review's overall summary conflicts with an agent finding (e.g. "no correctness defects surfaced" against a filed HIGH). Say so explicitly and explain the reconciliation.

Verdict comes last

State the verdict after the inventory, never before it. Leading with a verdict invites the user to accept it without reading the evidence — which is the exact failure this step exists to prevent.

Justify the verdict against the bar in review-calibration.md by naming the categories: "REQUEST_CHANGES requires a correctness bug, security issue, contract violation, or missing error handling on a critical path — none remain, so APPROVE with nits." A verdict that doesn't reference the bar is an opinion.

Deferrals

Flag every deferral as an explicit question, with the argument for the deferred work rendered fully enough that the user can overrule you. Never bury a deferral in prose. See the deferral gate in Step 7a.

Step 7: Actionable Output

Step 6 and Step 7 are different artifacts — do not substitute one for the other

This is the single easiest way to get this skill wrong, and it produces a review that looks fine and is useless.

	Step 6 — in-chat + findings file	Step 7a — the GitHub comment
Audience	You (the user), deciding what to post	The PR author
Contains	Every finding, every rejection, reclassification arithmetic, disagreements, deferrals	Only what survived your curation
Length	As long as the evidence requires	Tight — the author has to act on it
Purpose	Let you disagree with the pipeline	Tell the author what to change

The Step 7a format is deliberately terse. Never render that terse shape in chat. If the first thing you show the user is an executive summary plus a bulleted list of HIGH+ findings, you have skipped Step 6 and silently discarded every MEDIUM, every LOW, and every rejection — the exact material they need to make a call.

Step 6 always runs, always in full, and always before Step 7.

Rationalization	Reality
"The judge already summarized it, I'll pass that along"	The judge's summary is input to Step 6, not a substitute for it. It compresses away the reviewer detail and de-emphasizes rejections.
"17 findings is a lot to put in chat"	The user asked for a review. Volume is the finding count, not your formatting choice. Put the inventory in a file and the decision-relevant view in chat — both, not neither.
"They only need the blockers to decide"	They need the rejections to decide whether to trust the blockers. A pipeline that rejected two bug-introducing suggestions earned more trust than one that found three nits.
"I'll show the rest if they ask"	They can't ask about findings they don't know exist.
"The verdict is the useful part"	The verdict is your opinion. The evidence is what lets them overrule it.

7a. PR mode (SOURCE_TYPE=pr)

After presenting the full Step 6 report, offer to draft PR comments:

I can draft comments for this PR:

1. Top-level review comment — executive summary + validation evidence + a numbered one-line finding list
2. Line-level comments — each finding you want posted, inline on its file:line

Which findings should go in? I'll draft them for your approval before anything is posted.

Curate before drafting. The GitHub comment is a subset of the inventory, not a transcript of it. Confirm which findings make the cut — and note that dropping findings here is normal and good, as long as Step 6 already showed the user everything that existed.

Verify line anchors before building the payload. Every comments[].line must be a line the diff actually adds or modifies, or the API rejects the whole review. Check each target against the added-line set:

git diff -U0 origin/main...HEAD -- "$FILE" \
  | awk '/^@@/{split($3,a,","); s=substr(a[1],2); n=(a[2]==""?1:a[2]); for(i=0;i<n;i++) print s+i}'

Build the payload programmatically. Hand-escaping markdown into JSON breaks on backticks, code fences, and quotes. Write a small script that json.dumps a dict, then post with gh api ... --input. For a Meteorite PR (PR_SYS=gs) the payload is identical — post it with gs api repos/{owner}/{repo}/pulls/<number>/reviews --input -. See pr-review-posting.md for the API shape.

If the user says yes, format:

Deferral confirmation gate:

Before drafting comments, scan the judge output for any findings marked [DEFERRAL — NEEDS CONFIRMATION] or any suggestion to defer/follow-up. If any exist, present them to the user explicitly:

⚠️ Deferral check: The review suggests deferring these to follow-up:

1. [finding title] — [reason for deferral]

Should I include these as "defer to follow-up" in the PR comments, or rewrite them as "fix in this PR"?

Do NOT include deferral language in any drafted comment until the user confirms.

Top-level comment style guard:

• Never use shallow approval shorthand such as LGTM, LGTM ✅, looks good to me, or ship it.
• Always reference concrete review evidence (what was validated, what risk remains, and whether any suggestions are non-blocking).
• Prefer explicit approval language tied to findings (e.g., "I reviewed this thoroughly and approve"), not emoji-only signals.

Top-level comment:

• Do NOT include a ## Code Review header — the comment is a code review, the header is redundant.
• Do NOT include a ### Verdict section — the GitHub review action (approve/request changes) already communicates the verdict. Restating it in the body is noise.
• Lead with the executive summary directly, then key findings.

[Executive summary]

### Key Findings

[Bulleted list of validated HIGH+ findings with file:line references]

Line-level comments — for each validated finding:

File: path/to/file.rb
Line: 42
Comment:
[SEVERITY]: [title]
[problem description]
Suggested fix:
[code block with fix]

7b. Local mode (SOURCE_TYPE=local_uncommitted or local_branch)

After presenting the review, offer to plan fixes:

I found [N] actionable findings. Want me to:

1. Fix them now — I'll implement the validated fixes directly
2. Plan the fixes — I'll create a step-by-step fix plan you can review first
3. Cherry-pick — tell me which findings to fix and I'll do just those

Step 8: Cleanup

After the user has finished acting on findings (submitted comments, approved, etc.), free the WTP worktree if one was claimed in Step 1a:

cd ~
WTP_BIN="$HOME/src/github.com/shopify-playground/wtp/bin/_wtp"
"$WTP_BIN" free

For local reviews, no cleanup is needed.

Multi-Agent Pipeline is Mandatory

The multi-agent review pipeline (classify → dispatch reviewers → judge) is never optional. Every review, regardless of PR size or apparent simplicity, runs through subagent dispatch. You are the orchestrator — you do NOT review code yourself.

The only exception: when the user explicitly names specific reviewers (Step 3b), you dispatch those specific agents instead of auto-classifying. You still dispatch agents, never review directly.

Common Rationalizations

Check yourself against these before taking shortcuts:

Rationalization	Reality
"This is a tiny PR, I can review it myself"	Size doesn't determine whether multi-agent catches things you miss. The pipeline exists because single-perspective review has blind spots regardless of diff size. Dispatch reviewers.
"The PR is a simple rename/config change — full review is overkill"	Mechanical changes are where hidden semantic breaks hide. The correctness reviewer catches callers the diff doesn't show. Dispatch reviewers.
"The author is senior, light review is fine"	Author seniority doesn't prevent bugs. Seniority sometimes correlates with more complex changes that need more scrutiny, not less.
"I already see the issue, I'll just report it directly"	You see ONE issue. The pipeline runs 5-8 specialists in parallel who see different things. Your single-pass observation becomes the executive summary after the judge validates all findings.
"Dispatching subagents is slow, I'll be faster"	You'll be faster at producing a worse review. The 30-60s dispatch time is the cost of coverage.
"The tool/subagent failed, I'll just review it myself as fallback"	Diagnose the failure, retry once, then report INCOMPLETE per Step 5d. A partial multi-agent review beats a complete single-agent review. Never fall back to single-pass.
"pair-review is slow, I'll skip it and just run the agents"	For PR reviews, kick it off in the background at the start of Step 4 — it runs in parallel and costs you no wall time. It caught a zone anti-pattern and a constant triplication the agents missed. Only skip it for local reviews or when the user opts out.
"pair-review failed, I'll review the code myself to compensate"	No. The agent pipeline is the source of truth; pair-review only augments it. Note the failure in the status line and run the judge with agent findings only.
"A git command failed, I'll work around it"	Stop. Diagnose. Ask the user. Don't improvise with alternative git commands, temp branches, or manual patches.
"This could be exploited, so it's CRITICAL"	"Could be" ≠ "is reachable in production." Check the actual call path. If the input is already validated upstream, the severity drops.
"No tests were added, so this needs REQUEST_CHANGES"	Missing tests are a valid observation but not always blocking. If the code is well-tested indirectly or the PR is a config change, tests may be unnecessary. Calibrate severity to actual risk.

Constraints

• NEVER submit reviews to GitHub or any external system
• NEVER push code without explicit user approval
• NEVER review code yourself — always dispatch through the subagent pipeline
• Output the review as formatted text for the human to act on
• You are the orchestrator — classify, dispatch, judge, present
• Handle reviewer and judge failures per the degraded mode guidance in Step 5d