| name | code-review |
| description | Use when reviewing code changes for quality, correctness, and production readiness before merge |
Code Review
Orchestrates chunk-reviewer agents against diffs. Handles input parsing, context gathering, chunking, and result synthesis.
Premises (apply to orchestrator AND chunk-reviewer)
These two premises are non-negotiable. They are forwarded to every chunk-reviewer dispatch and they govern every decision in this skill.
-
Post-change state โ The working directory reflects the post-change state of the target ref. PR mode achieves this by checking out the PR head into a dedicated linked worktree (see Step 0). Non-PR modes (branch comparison, auto-detect) achieve this by verifying HEAD-match + clean-tree on the current working directory (also Step 0). Either way: read code freely from the working directory โ the diff is the delta, the working directory is the result. Do not pretend the file system is read-only or stuck at base.
-
No diff-only review โ A diff is a delta. The unit of review is the system the diff produces. Always trace dependencies, callers, callees, interfaces, configurations, and runtime context across files. If you cannot explain how the changed code behaves end-to-end against the surrounding system, you have not reviewed it.
These premises must be reflected in the chunk-reviewer dispatch prompt โ see Step 5.
Input Modes
/code-review pr 123
/code-review pr https://github.com/org/repo/pull/123
/code-review main feature/auth
/code-review
Do vs Delegate Decision Matrix
| Action | YOU Do | DELEGATE |
|---|
| Requirements 3-question gate | Yes | - |
| Diff range determination & git | Yes | - |
| Evidence Verification (build/test/lint) | Yes | - |
| Chunking decision | Yes | - |
| Findings synthesis (rank/class verified findings) | Yes | - |
| Individual candidate judgment inline (Phase 2) | Yes | - |
| Escalation verification (candidates below confidence threshold) | - | verifier subagent (one per escalated candidate) |
| Individual chunk review | NEVER | chunk-reviewer |
| Code modification | NEVER | (forbidden entirely) |
RULE: Orchestration, synthesis, and decisions = Do directly. Per-candidate inline judgment = Do directly in Phase 2 (reasoning โ confidence โ verdict + enrichment). Escalated candidates (confidence below threshold) = DELEGATE to verifier subagents (one per candidate). Multi-angle finding = DELEGATE to chunk-reviewer. Code modification = FORBIDDEN.
Role Separation
digraph role_separation {
rankdir=LR;
"Orchestrator" [shape=box];
"chunk-reviewer" [shape=box];
"verifier" [shape=box];
"Codebase" [shape=cylinder];
"Orchestrator" -> "chunk-reviewer" [label="dispatch with {DIFF_COMMAND}"];
"chunk-reviewer" -> "Orchestrator" [label="candidate findings"];
"Orchestrator" -> "Codebase" [label="Read/Grep inline (Phase 2)"];
"Orchestrator" -> "verifier" [label="dispatch per escalated candidate"];
"verifier" -> "Codebase" [label="Read/Grep to verify"];
"verifier" -> "Orchestrator" [label="verdict (supersedes inline)"];
}
Your role as orchestrator:
- Dispatch chunk-reviewer agents with diff command strings (each fans out the angle finders)
- Judge each deduped candidate inline in Phase 2 (reasoning โ confidence โ verdict + enrichment); enrich kept findings directly
- Escalate only candidates below the confidence threshold to verifier subagents; collect their final verdicts; supersede inline tentative verdicts with verifier verdicts in Phase 3
- Synthesize the kept findings into a ranked findings report (text only)
- Make chunking decisions and rank the verified findings (no merge verdict โ this review reports, it does not gate)
NOT your role:
- Modifying any source files
- Running the raw
git diff command (chunk-reviewers execute the diff)
Context Budget
Allowed in orchestrator context:
git diff {range} --stat output
git diff {range} --name-only output
git log {range} --oneline output
- CLAUDE.md file content
- Step 3 evidence summary (structured table โ build/test/lint status + test coverage mapping, truncated to summary on success / last 30 lines on failure)
- chunk-reviewer results (candidate findings)
- Phase 2 inline judgment output (reasoning, verdicts, enriched findings for non-escalated candidates)
- Escalated verifier subagent verdicts + enriched findings (Phase 2, candidates below confidence threshold)
- Code reading via Read/Grep for Phase 2 inline candidate judgment
Forbidden in orchestrator context:
- Running the raw
git diff command (chunk-reviewers execute the diff, not you)
- Modifying any source files
Context management:
- Phase 2 inline judgment reads candidate code directly in your context (non-escalated candidates); only escalated candidates' code reading moves into verifier subagent contexts
RULE: Inline judgment for non-escalated candidates runs in your context. Only escalated candidates are delegated to verifier subagents. You CANNOT run the raw diff command or modify files.
Step 0: Input Parsing
Environment setup runs first โ resolve the range and, in PR mode, check out the post-change code into the worktree before any code-reading step (intent acquisition, the derived-context sub-step, chunk-review). Every downstream step reads the working directory, so the working directory must already hold the post-change state.
Determine range and setup for subsequent steps:
| Input | Setup | Range |
|---|
pr <number or URL> | Fetch and check out PR ref into the worktree (see below) | origin/<baseRefName>...pr-<number> |
<base> <target> | Verify HEAD is <target> via git rev-parse --abbrev-ref HEAD; verify clean tree via git status --porcelain -uno. Abort if mismatch or dirty. | <base>...<target> |
| (none) | Detect default branch (origin/main or origin/master). Verify HEAD is the target branch via git rev-parse --abbrev-ref HEAD; verify clean tree via git status --porcelain -uno. Abort if mismatch or dirty. | <default>...HEAD |
PR Mode: Worktree Checkout (per Premise 1)
This skill assumes the orchestrator is already running inside a worktree dedicated to this review (the caller is responsible for creating the worktree). Therefore: fetch the PR ref AND check it out. The working directory must reflect the post-change state of the PR so that all subsequent code reading (Phase 2 verification, chunk-reviewer Step 2) sees the actual code under review.
PR ID ์ถ์ถ ๊ท์น: ์ฌ์ฉ์๊ฐ URL(https://github.com/<org>/<repo>/pull/<N>) ํ์์ผ๋ก ํธ์ถํ๋ฉด, ์๋ bash๋ก ์ง์
ํ๊ธฐ ์ ์ trailing path segment์์ numeric <N>์ ์ถ์ถํด <number> ์๋ฆฌ์ substituteํ๋ผ. URL์ ๊ทธ๋๋ก substituteํ๋ฉด git fetch origin pull/<URL>/head๊ฐ invalid refspec์ผ๋ก ์คํจํ๊ณ git checkout -B pr-<URL>์ด invalid ๋ธ๋์น๋ช
์ผ๋ก ์คํจํ๋ค.
set -euo pipefail
if [ -n "$(git status --porcelain -uno)" ]; then
echo "Error: working directory has uncommitted changes โ refusing to checkout over the user's work" >&2
exit 1
fi
if [ "$(git rev-parse --git-dir 2>/dev/null)" = "$(git rev-parse --git-common-dir 2>/dev/null)" ]; then
echo "Error: refusing to run in primary repo โ create a dedicated linked worktree first (Premise 1). Hint: 'git worktree add ../review-pr-<N> -b review/pr-<N>'" >&2
exit 1
fi
BASE_REF=$(gh pr view <number> --json baseRefName --jq '.baseRefName')
if [ -z "$BASE_REF" ]; then
echo "Error: gh pr view returned empty baseRefName โ aborting before any fetch" >&2
exit 1
fi
git fetch origin "${BASE_REF}"
git fetch origin pull/<number>/head
git checkout -B pr-<number> FETCH_HEAD
If the working directory is dirty (uncommitted changes) or the caller is not in a worktree, abort and report โ do not silently checkout over the user's work. The worktree premise is the safety net; without it, the safety net is gone.
All range formats use three-dot syntax (A...B), which is equivalent to git diff $(git merge-base A B)..B. This shows only changes introduced by the target since the common ancestor โ not changes on the base branch. This prevents false positives when origin/main has moved ahead after branching.
All subsequent steps use {range} from this table. All diff commands use git diff {range} -- <files> for path-filtered output. After checkout, code reading via Read/Grep/Glob reflects the post-change state, which is the intended behavior โ diff shows the delta, the working directory shows the result.
Early Exit
After the range is resolved and (PR mode) the checkout is done, before proceeding to Step 1:
- Run
git diff {range} --stat (using the range determined above)
- If empty diff: report "No changes detected (between and )" and exit
- If binary-only diff: report "Only binary file changes detected" and exit
Early Exit runs before intent acquisition on purpose โ an empty or binary-only diff needs no interview.
Step 1: Intent and Context Acquisition
Intent acquisition is non-negotiable. Either intent is confirmed (from artifacts, interview, or both), or the user explicitly defers to a code-quality-only review. There is no third option โ proceeding without intent and without explicit deferral is forbidden. Reviewing without intent produces wrong severities, missed scope creep, and false positives born from misunderstanding the author's goal.
Acquisition order
- PR/branch artifacts โ PR title, description, labels, commit history, code review comments and threads
- Linked references (recursive) โ every link found in the artifacts above, followed transitively until the trail ends
- Codebase signals โ CLAUDE.md, README, ADRs, related history in changed paths
- User interview โ only for what the artifacts cannot reveal
Acquire all reachable references
PR descriptions, commits, and comments routinely link to richer context (issue trackers, design docs, chat threads). Follow every link recursively โ a linked ticket may itself link to a doc which links to a discussion thread; keep following until the trail ends.
Do not name specific tools. Use whatever fetch capability the environment provides for each link type. If a link cannot be fetched directly (no credential, no MCP for that platform, network unreachable), do not skip it โ mark it for the user interview step.
Sources to consult per input mode:
| Input mode | Sources |
|---|
| PR | gh pr view --json title,body,labels,comments,reviews, gh pr view --comments, linked issues, gh issue view <n>, every external link found in the chain, commit messages on the PR branch |
| Branch comparison | Commit messages, branch name conventions, any linked tickets discovered in commits, related issues |
| Auto-detect | Recent commit messages on HEAD, any linked tickets found there |
User interview โ only for what artifacts cannot reveal
After exhausting fetchable sources, ask the user about:
- Intent โ what problem is this PR solving and why was this approach chosen
- Alternatives โ what was considered and rejected, and why
- Constraints โ deadlines, dependencies, compatibility commitments, hidden requirements
- Concerns โ known risks, untested paths, areas the author is uncertain about
DO NOT interview the user about codebase facts (file locations, patterns, architecture, who calls what). Use Read/Grep/Glob and the explore agent for those โ they are reachable from the working directory.
Intent Block Gate (hard exit condition)
Before exiting Step 1, the state must be one of:
| State | Action |
|---|
| Intent confirmed โ author's goal, approach, and constraints are understood from artifacts and/or interview | Proceed to Step 2 |
| User explicit deferral โ user says "skip", "๊ทธ๋ฅ ๋ฆฌ๋ทฐํด์ค", "์์ด", "code quality only", or unambiguous equivalent | Set {REQUIREMENTS} = "N/A โ code-quality-only review (user deferred)" and proceed |
Non-interactive dispatch (completion-gate) โ the dispatch prompt itself carries a {gate}-codereview-{sid}.json artifact path alongside a 5-slot intent payload (what_was_implemented/description/requirements/project_context/non_goals) | Treat as Intent confirmed (non-interactive, no user interview) and proceed to Step 2. Acquisition steps 1-3 (PR/branch artifacts, linked references, codebase signals) still run โ they backfill any slot whose value is the (none provided) marker. Only step 4 (user interview) is replaced by the payload. |
| Neither โ artifacts thin and user not yet asked, OR user gave vague answers without explicit deferral | BLOCK. Do not proceed. Continue interview until one of the two states above is reached. |
There is no "I tried hard enough, just review" path. The block IS the safety mechanism.
A fresh code-reviewer agent has no ambient session to check for an active artifact path โ the non-interactive discriminator above is prompt-borne: whether the dispatch prompt includes the path, not whether a session-scoped artifact happens to exist. This is the same {gate}-codereview-{sid}.json signal Step 5 later reads as runId (Find Phase Sink, below); Step 1 is where it first enters the pipeline. When the signal is absent, the main-session interactive gate above (Neither โ BLOCK) is unchanged.
Vague answer refinement
When the user gives a vague answer that is not an explicit deferral, refine ONCE with a specific follow-up:
๊ฐ follow-up ๋ฉ์์ง๋ deferral ์ต์
์ ํจ๊ป ์๋ดํ์ฌ ์ฌ์ฉ์๊ฐ "skip / ๊ทธ๋ฅ ๋ฆฌ๋ทฐํด์ค / ์์ด" ์ดํ๋ฅผ ๋ชฐ๋ผ๋ escape ๊ฐ๋ฅํ๋๋ก ํ๋ค.
| User says | Follow-up |
|---|
| "๋์ถฉ ์์ด" / "๋ญ ์ข ์๊ธด ํ๋ฐ" | "์ด๋์ ์ฐพ์ ์ ์๋์? ๋งํฌ๋ ๋ฌธ์ ์์น๋ฅผ ์๋ ค์ฃผ์ธ์. (๋ตํ๊ธฐ ์ด๋ ค์ฐ๋ฉด 'skip'์ผ๋ก ์ฝ๋ ํ์ง๋ง ๋ฆฌ๋ทฐ ๊ฐ๋ฅ)" |
| "๊ทธ๋ฅ ์ฑ๋ฅ ๊ฐ์ ์ด์ผ" | "์ด๋ค ์งํ๋ฅผ ๊ฐ์ ํ๋ ค ํ๋์? (latency, throughput, memory ๋ฑ โ ๋ตํ๊ธฐ ์ด๋ ค์ฐ๋ฉด 'skip'์ผ๋ก ์ฝ๋ ํ์ง๋ง ๋ฆฌ๋ทฐ ๊ฐ๋ฅ)" |
| "์ฌ๋ฌ ๊ฐ์ง ๊ณ ์ณค์ด" | "๊ฐ์ฅ ์ค์ํ 1-2๊ฐ๋ง ์๋ ค์ฃผ์ธ์. ๋๋จธ์ง๋ ์ฝ๋์์ ์๋ณํ๊ฒ ์ต๋๋ค. (๋ตํ๊ธฐ ์ด๋ ค์ฐ๋ฉด 'skip'์ผ๋ก ์ฝ๋ ํ์ง๋ง ๋ฆฌ๋ทฐ ๊ฐ๋ฅ)" |
If refinement still yields a vague answer, surface the block explicitly to the user:
"์๋๋ฅผ ๋ช
ํํ ์ก๊ธฐ ์ด๋ ต์ต๋๋ค. ๋ ์ค ํ๋๋ฅผ ์ ํํด์ฃผ์ธ์: (1) [๊ตฌ์ฒด์ ์ง๋ฌธ]์ ๋ตํ์ฌ ์๋ ํ์ , ๋๋ (2) 'skip / ์ฝ๋ ํ์ง๋ง ๋ฆฌ๋ทฐ' ๋ช
์์ deferral. ๋ ์ค ํ๋๋ฅผ ๋ช
์ํ๊ธฐ ์ ๊น์ง ๋ฆฌ๋ทฐ๋ ์์ํ์ง ์์ต๋๋ค."
This is not adversarial โ it is refusing to silently produce a worse review.
Question discipline
| Situation | Method |
|---|
| 2-4 structured choices (review scope, focus areas) | AskUserQuestion tool |
| Free-form / subjective (intent, alternatives, constraints, concerns) | Plain text question |
One question per message. Never bundle. Wait for the answer before the next question.
Question quality โ every question must include either a specific anchor (a summary the user can correct) or a default action in parentheses (so progress is possible without an answer):
| BAD | GOOD |
|---|
| "์๊ตฌ์ฌํญ์ด ์๋์?" | "PR ๋ณธ๋ฌธ๊ณผ ์ฐ๊ฒฐ๋ ์ด์์์ [์์ฝ]์ ์ถ์ถํ์ต๋๋ค. ๋ณด์ํ ๋ถ๋ถ์ด ์๋์?" |
| "์ด๋ค ๋ถ๋ถ์ ๋ณผ๊น์?" | "23๊ฐ ํ์ผ์ด ๋ณ๊ฒฝ๋์ต๋๋ค. ์ง์คํ ์์ญ์ด ์๋์? (์์ผ๋ฉด ์ ์ฒด ๋ฆฌ๋ทฐ)" |
Project Context
Include project context when interpolating the chunk-reviewer prompt template in Step 5. Describe what kind of software this is, who uses it, how it runs, and what depends on it โ based on CLAUDE.md, README.md, and the artifacts gathered above.
If available context is insufficient to characterize the project, ask the user once: "What kind of software is this? (e.g., personal CLI tool, internal team service, public-facing API, shared library, etc.)"
Step 1 Exit Condition
Proceed to Step 2 only when the Intent Block Gate state is Intent confirmed or User explicit deferral. Any other state โ continue at Step 1.
Bounded derived context (derived expected-items)
By this point {REQUIREMENTS} has settled โ via interview, the deferral sentinel, or the completion-gate payload. This sub-step adds one more thing to it: bounded derived context, a codebase-grounded prediction of expected-items, kept distinct from intent acquisition above. "Intent acquisition is non-negotiable" (the Step 1 charter) means received, stated author intent is authoritative; this sub-step instead generates a hypothesis from the codebase's own "Codebase signals" (acquisition step 3) โ it does not receive stated intent, it infers from what the codebase already does.
Mirror the same reasoning shape the regression and cleanup finder angles use: name a thing the codebase already establishes, then check whether the change re-establishes it. Here: name a same-role analog already in the codebase, then check whether the change wires the new addition into it the same way. Derive an expected-item only through this named-necessity gate:
| State | Condition | Action |
|---|
| Grounded + necessity-named | A citable codebase analog exists with a concrete file:line, AND a concrete runtime consequence of the item's absence can be named | Keep โ emit the derived item |
| Uncertain | Only one of the two holds, or either is fuzzy | Drop |
| Neither | No citable analog, no nameable consequence | never invent |
For each kept item, emit one bullet carrying four fields โ self-labeling for provenance, so the downstream requirement-gap finding needs no new field to explain where it came from:
- Analog (
file:line) โ the existing code whose role the missing item should mirror
- Why same role โ why the analog and the missing item play the same structural role
- Expected item absent here โ what the analog implies should exist in the changed code, and doesn't
- Runtime consequence of its absence โ what breaks, silently or loudly, if it stays missing
Phrase the bullet itself like "Codebase analog at file:line implies <wiring>; absent here" โ never "a requirement you stated is absent." The label must stay honest: a same-role analog implies wiring that is absent here, not a stated requirement that is absent.
Fold kept items into {REQUIREMENTS} as a Derived Expected Items sub-block:
{REQUIREMENTS} already holds real content (caller/goal-lane requirements, or the completion-gate dispatch payload's requirements field) โ append the sub-block after it; preserve what's there.
{REQUIREMENTS} holds the deferral sentinel N/A โ code-quality-only review (user deferred) (set at the Intent Block Gate above) AND at least one item was derived โ replace the sentinel with the sub-block, so coverage never sees a self-contradictory "N/A" plus derived items.
- Zero items derived โ leave
{REQUIREMENTS} exactly as it was โ no empty sub-block, no sentinel change.
This sub-step is unconditional on intent-source: it runs the same way regardless of whether intent came from a live interview, a caller-supplied artifact, the completion-gate dispatch payload, or explicit code-quality-only deferral. It never gates on live-interview-only or on requirements already being present โ it derives wherever the codebase grounds an item, and stays silent otherwise.
When the deferral is an explicit human code-quality-only deferral (a person typed "skip" / "๊ทธ๋ฅ ๋ฆฌ๋ทฐํด์ค" / "code quality only" at the Intent Block Gate) rather than the completion-gate's non-interactive payload, derived items are still surfaced โ always-run holds even here โ but their Runtime consequence of its absence text carries a short note such as "surfaced despite quality-only deferral," so the one mode where a person actively deferred scope stays framed honestly.
Step 2: Context Gathering
Collect in parallel (using {range} from Step 0):
git diff {range} --stat (change scale)
git diff {range} --name-only (file list)
git log {range} --oneline (commit history)
- CLAUDE.md files: repo root + each changed directory's CLAUDE.md (if exists)
Step 3: Evidence Verification
Run build, test, and lint checks BEFORE dispatching any chunk-reviewer agents. This is a fail-fast gate โ a failing check aborts the review immediately.
Command Discovery
Do NOT assume commands. Discover per-project commands in this order:
- Memory file first:
~/.omt/{project}/project-commands.md โ derive {project} with: basename -s .git $(git remote get-url origin 2>/dev/null), fallback: basename $(git rev-parse --show-toplevel 2>/dev/null || pwd)
- Project documentation:
CLAUDE.md, AGENTS.md, README.md, CONTRIBUTING.md
- Build files:
package.json scripts, build.gradle / build.gradle.kts tasks, Makefile targets, pyproject.toml, Cargo.toml
- If still unclear: Ask user for build/test/lint commands
Discovery is per-command and independent โ if build is found but lint is not, run build and test; skip only the undiscovered command. If no commands are discovered at all, skip this step with the unavailable message (see Output Format below).
Default timeout per command: 120 seconds.
Execution Order
Run in sequence โ stop immediately on first failure:
- Build / compile
- Project's declared test command (its verification contract's scopeable form โ not necessarily the entire suite)
- Linter / static analysis
Any failure โ do NOT dispatch chunk-reviewer agents. See Fail-Fast Gate below for the exact exit sequence (it branches on whether this run carries the completion-gate dispatch signal from Step 1).
Output Format: {EVIDENCE_RESULTS}
Produce a two-part structured table after all checks complete.
Part 1 โ Automated Checks
| Check | Status | Details |
|---|
| Build | PASS / FAIL | Success: one-line summary. Failure: last 30 lines of output |
| Tests | PASS (N passed) / FAIL (N/M failed) | Success: pass count. Failure: last 30 lines of output |
| Lint | PASS / FAIL | Success: one-line summary or "No errors". Failure: last 30 lines of output |
Part 2 โ Test Coverage Mapping
Identify production source files from the Step 2 file list: source code files (exclude non-code files such as config, documentation, build scripts, migrations, Markdown, YAML, JSON, images, etc.) that do NOT match test glob patterns (*Test*, *Spec*, *_test*, test_*, *.test.*, *.spec.*, *_spec*).
For each production source file, find its corresponding test file:
| Production Source File | Test File | Coverage Status |
|---|
path/to/File.kt | path/to/FileTest.kt | In diff / Exists, not in diff / No test found |
Coverage Status values:
- In diff: test file exists AND is in the diff (changed alongside production code)
- Exists, not in diff: test file exists in the repo but was NOT changed
- No test found: no test file matching the production source file name found
If more than 30 production source files are in the diff, group by directory instead of listing per file.
Unavailable message (no commands discovered): "Evidence verification unavailable โ no build/test/lint commands discovered"
Fail-Fast Gate
If any check fails:
- Populate {EVIDENCE_RESULTS} Part 1 with the failure details (last 30 lines of failing command output)
- Omit Part 2 (Test Coverage Mapping) โ it is not needed on failure
- Do NOT proceed to Step 4 or dispatch chunk-reviewer agents
- Completion-gate dispatch signal present (the dispatch prompt carried a
{gate}-codereview-{sid}.json artifact path โ the same non-interactive discriminator Step 1 uses): before reporting, write that artifact directly โ {"status": "INCONCLUSIVE", "reviewer": "<reviewer id>", "at": "<ISO timestamp>", "findings": []}. This is the exact code-review artifact schema skills/{gate}/references/completion-gate.md defines. A build/test/lint fail-fast is neither a finished review (status: "COMPLETE") nor a confirmed defect (findings stays empty โ the failing command and its last 30 lines of output belong in this reviewer's own failure report surfaced to the dispatching gate, not folded into a per-finding ref) โ it is the review itself failing to complete, which is exactly what INCONCLUSIVE means. Writing status: "INCONCLUSIVE" is sufficient by itself: it structurally blocks request-complete (the never-false-complete gate in {gate}-state.ts) without promoting to status: "COMPLETE" or introducing any new status value. If the artifact write itself fails, do not retry or invent a status โ leave the artifact absent, which request-complete's existing absent-artifact refusal already blocks on.
- Completion-gate dispatch signal absent (interactive main-session review โ no completion-gate artifact path in play): no artifact write; this path is unchanged from before.
- Report {EVIDENCE_RESULTS} and exit immediately
Step 4: Chunking Decision
Determine scale from --stat summary line (N files changed, X insertions(+), Y deletions(-)):
| Condition | Strategy |
|---|
| Total changed lines (insertions + deletions) < 1500 AND changed files < 30 | Single review |
| Total changed lines >= 1500 OR changed files >= 30 | Group into chunks by directory/module affinity |
Chunking heuristic: group files sharing a directory prefix or import relationships.
Per-chunk size guide:
- Target ~1500 lines per chunk (soft guide โ files are the atomic unit)
- If adding the next file exceeds ~1500 lines, start a new chunk
- If a single file alone exceeds ~1500 lines, it becomes its own chunk
- If a directory group is oversized, split by subdirectory; if still oversized (flat structure), batch alphabetically (~10-15 files per chunk)
Per-Chunk Diff Command Construction
For each chunk, construct the diff command string using git's native path filtering:
git diff {range} -- <file1> <file2> ... <fileN>
The orchestrator constructs this command string but does NOT execute it. The command is passed to the chunk-reviewer via {DIFF_COMMAND}, and each reviewer CLI executes it independently.
Step 5: Agent Dispatch
-
Read dispatch template from ${CLAUDE_SKILL_DIR}/../orchestrate-review/scripts/chunk-reviewer-prompt.md
-
Interpolate placeholders with context from Steps 0-4:
- {WHAT_WAS_IMPLEMENTED} โ Step 1 description (interactive) / JSON field
what_was_implemented (structured-output completion-gate dispatch)
- {DESCRIPTION} โ Step 1 or commit messages (interactive) / JSON field
description (completion-gate dispatch)
- {REQUIREMENTS} โ Step 1 requirements or "N/A - code quality review only" (interactive) / JSON field
requirements (completion-gate dispatch)
- {PROJECT_CONTEXT} โ Step 1 project context (interactive) / JSON field
project_context (completion-gate dispatch); if it resolves to the literal "(none provided)" backfill marker, backfill from codebase signals gathered in Step 1 acquisition steps 1-3 (CLAUDE.md/README/ADR)
- {NON_GOAL} โ Step 1 declared non-goals (interactive) / JSON field
non_goals (completion-gate dispatch); backfilled to the literal "(none provided)" marker when blank
- {FILE_LIST} โ Step 2 file list
- {DIFF_COMMAND} โ diff command string:
git diff {range} (single chunk) or git diff {range} -- <chunk-files> (multi-chunk). Orchestrator constructs this string but does NOT execute it.
- {COMMIT_HISTORY} โ Step 2 commit history
- {EVIDENCE_RESULTS} โ Step 3 evidence summary (Source: Step 3. Fallback: "Evidence verification unavailable โ no build/test/lint commands discovered")
The five intent placeholders above ({WHAT_WAS_IMPLEMENTED}/{DESCRIPTION}/{REQUIREMENTS}/{PROJECT_CONTEXT}/{NON_GOAL}) source differently depending on mode โ the same {gate}-codereview-{sid}.json dispatch signal Step 1's Intent Block Gate uses to discriminate non-interactive dispatch. In structured-output mode (completion-gate dispatch), the Step 1 payload is a JSON object with named fields what_was_implemented/description/requirements/project_context/non_goals โ JSON.parse it and read each named field 1:1 into its placeholder above. This is a named-field read, not a blob split โ never dump the whole payload into one placeholder. If the payload fails to parse as JSON, follow the same INCONCLUSIVE artifact bridge Step 3 uses on build failure and stop before dispatching chunk-reviewer agents โ do not guess field values from malformed input.
-
Dispatch chunk-reviewer agent(s) via Task tool (subagent_type: "chunk-reviewer") with interpolated prompt
Dispatch rules:
| Scale | Action |
|---|
| Single chunk | 1 agent call |
| Multiple chunks | Parallel dispatch -- all chunks in ONE response. Each chunk gets its own interpolated template with chunk-specific {DIFF_COMMAND} and {FILE_LIST} |
Result Scope Validation
Each chunk-reviewer scans its whole assigned diff through every angle and reports coverage per angle (the Angle Coverage block), not per file. A file that produced no candidates is clean, not omitted โ never treat an unmentioned file as a coverage gap.
Re-dispatch a chunk-reviewer for its chunk only when its response signals an infrastructure failure: a "Partial review"/"Limited review" degradation notice, an Angle Coverage entry marked Unavailable, or a reported diff-command failure.
Cap: maximum 1 re-dispatch per original chunk; if the re-dispatch also fails, accept partial coverage.
After all re-dispatches complete, merge all chunk results (original + re-dispatched) before proceeding to the Find Phase Sink.
Find Phase Sink
Establish runId before dispatching chunk-reviewers (Step 5):
- Dispatched from a completion gate (a
{gate}-codereview-{sid}.json artifact path is active for the current session): runId = {sid}
- Otherwise:
runId = crypto.randomUUID() (Tier-0 builtin โ no dependency required)
After all chunk-reviewers return and before Phase 2 begins, write the durable sink:
Parse the conductor's returned text for:
found โ sum of per-angle counts from the Angle Coverage block (each angle's K candidates value)
deduped โ the N in ### Candidate Findings ({N}/from M angles) header
findTokenUsage โ raw JSON object from the ### Find Token Usage block (omit when unavailable โ do not block the sink write)
Compute dispatched:
- v1:
dispatched = deduped โ no inline cap exists; the field records verify load for the find-inclusion gate and diverges only if a later inline cap or pre-filter is added
Invoke the sink helper:
bun "${CLAUDE_SKILL_DIR}/scripts/durable-sink.ts" \
"<runId>" <found> <deduped> <dispatched> \
'<findTokenUsageJson>'
Omit the last argument when findTokenUsage is unavailable. A D=0 review (zero candidates) still invokes the sink โ candidates.json is written with zeros.
Find-inclusion rule:
Read [$CLAUDE_CONFIG_DIR|~/.claude]/settings.json and ./.claude/settings.json (project overrides user):
- Resolve
omt.codeReview.findInclusionThreshold into <findInclusionThreshold>; if undefined, use 50000 (output tokens)
After writing the sink, if findTokenUsage is available, compare findTokenUsage.usage.output_tokens directly against findInclusionThreshold as an absolute output-token count. (v1 note: verify/synthesis tokens are not measured in v1, so a true share-of-total-review-tokens ratio is not computable; this threshold is an absolute cost signal instead.) If findTokenUsage.usage.output_tokens >= findInclusionThreshold, append a note to the Phase 3 report: "Find output tokens {findTokenUsage.usage.output_tokens} >= threshold {findInclusionThreshold} tokens โ find-phase redesign is in scope as a follow-up (Story 2)." This rule is measurement-only in v1 โ it does not block the review or change any verdict.
Step 6: Verification + Synthesis
After all chunk-reviewers return, produce the final findings in two phases: per-candidate inline judgment with selective escalation (Phase 2), and findings synthesis (Phase 3). The terminal deliverable is the Phase 3 findings text โ no walkthrough, no diagrams, no HTML.
Phase 2: Candidate Verification (MANDATORY)
Finders surface candidates; they do not judge them. You judge each deduped candidate inline โ reasoning through the evidence, reading the relevant code in your context, and issuing a confidence score and verdict. Inline judgment eliminates the multi-candidate batch-output parse hazard, and the orchestrator's context (already holding chunk-reviewer results and diff stat) is a clean enough frame for accurate triage: no anchoring from the authoring context, because this is already a separate review session.
Config resolution:
Read [$CLAUDE_CONFIG_DIR|~/.claude]/settings.json and ./.claude/settings.json (project overrides user):
- Resolve
omt.codeReview.escalationConfidenceThreshold into <threshold>; if undefined, use 0.35
- Resolve
omt.codeReview.escalationKCap into <k>; if undefined, use 3
- Resolve
omt.codeReview.findInclusionThreshold into <findInclusionThreshold>; if undefined, use 50000 (output tokens; also resolved in the Find Phase Sink above)
Inline judgment steps:
-
Dedup near-duplicates first (same defect, same location, same reason โ keep one, merging the found by angles and keeping the most concrete failure scenario). Deduplication reduces the judgment workload before it starts.
-
MANDATORY READ: references/verifier-prompt.md โ read it before beginning judgment. The verdict ladder (CONFIRMED / PLAUSIBLE / REFUTED), verification method, and 9-field output contract all live there. Escalated candidates reuse this file as their dispatch prompt.
-
For each remaining candidate, in order:
REASONING โ read the code at the issue location (Read/Grep on the candidate file), trace the call chain from the entry point, and check the execution context (threading, dispatch model, runtime configuration). Apply the verdict ladder from references/verifier-prompt.md. Reason explicitly before issuing a score.
CONFIDENCE โ assign a numeric value in 0.0โ1.0 reflecting certainty that the finding is real (1.0 = no doubt, 0.0 = clearly not a bug). This value is internal only: it drives the escalation comparison and candidate ranking but is never serialized into any artifact.
VERDICT โ exactly one of CONFIRMED / PLAUSIBLE / REFUTED (ladder in references/verifier-prompt.md).
For CONFIRMED or PLAUSIBLE (kept findings), emit the full 9-field enrichment inline:
VERDICT: <CONFIRMED | PLAUSIBLE>
TITLE: <short finding title>
LOCATION: <file>:<line> โ <section / function name>
CURRENT CODE:
<5-15 lines centered on the issue>
WHAT'S WRONG: <the problem, grounded in the quoted line>
FAILURE SCENARIO: <concrete inputs/state -> wrong output, crash, or lost effect; for a cleanup finding, the concrete cost>
FIX: <concrete diff, or design direction if structural>
BLAST RADIUS: <grep/reference evidence โ what else references this, or "This location only">
FOUND BY: <angle(s)>
For REFUTED, emit a one-line note:
VERDICT: REFUTED โ <one line quoting the line/guard/invariant that proves it is not a bug>
-
Escalation โ after all inline judgments complete, collect candidates where confidence < omt.codeReview.escalationConfidenceThreshold:
- If the count exceeds
omt.codeReview.escalationKCap, take the <k> lowest-confidence candidates for escalation; the overflow candidates keep their inline verdict and are surfaced in Phase 3 โ never silently dropped.
- For each escalated candidate, interpolate
references/verifier-prompt.md with the candidate's fields and dispatch a general-purpose subagent via the Task tool (subagent_type: "general-purpose"). All escalated candidates in ONE response โ parallel, foreground.
- The escalated verifier's verdict is FINAL and supersedes the inline tentative verdict in Phase 3.
Cap & batching: judge at most 25 candidates per batch inline. If more survive dedup, batch by file proximity, correctness candidates first, and state how many were deferred โ never silently drop.
The verdict ladder, verification method, and 9-field output contract all live in references/verifier-prompt.md โ that is the single source; the inline judgment mirrors this contract directly.
Phase 3: Findings Synthesis (report-only)
This is a report. You surface verified findings, ranked by what matters most. You do NOT decide whether to merge and you do NOT decide whether to fix โ that is the reader's call. Removing the merge verdict is deliberate: a reviewer that labels findings and shows the failure path is more trusted than one that issues a pass/fail an author then argues with.
- Merge verified findings that describe the same defect (same root cause, across chunks) โ combine their evidence and note the corroborating angles. (Near-duplicates within a chunk were already deduped before verification.)
- Class each finding: correctness (the change behaves wrong), cleanup (behaves correctly but is low quality), or requirement-gap (an AC or stated requirement is absent โ the behavior is missing, not wrong), from the angle that found it.
- Rank most-significant first: correctness before cleanup; within a class, CONFIRMED before PLAUSIBLE.
- Cap: keep the most significant findings. If a review produced an unwieldy number, keep the top ~15 and state how many were dropped โ never silently truncate.
- Pre-existing: a candidate on an unchanged context line is tagged
[Pre-existing] and listed under Out of Scope โ unless the change aggravates it (increases blast radius or frequency), in which case it stays in the main list.
Edge Cases
| Situation | Handling |
|---|
| Finding references a deleted file | Read the file at base branch (git show {base}:{file}). Note "(deleted file)" in Context. |
| Finding spans multiple files | Primary file gets the code snippet. Other files listed in Blast Radius with brief context. |
| Fix cannot be expressed as simple diff | State design direction + "Concrete diff not possible โ structural change required". |
| Zero findings after verification | Report a clean review: "No findings survived verification." |
| 50+ candidates requiring verification | Dispatch verifiers in batches per Phase 2 (โค25), correctness candidates first. |
Terminal Output
This is a report. It does not gate. There is no Assessment / "Ready to merge" section, and there is no HTML โ the deliverable is the Phase 3 findings as terminal text.
Emit the ranked findings directly: each finding carries its verdict (CONFIRMED / PLAUSIBLE), class (correctness / cleanup / requirement-gap), file:line, and enriched evidence (current code, what's wrong, failure scenario, fix, blast radius โ the 9-field shape from references/verifier-prompt.md, produced inline for non-escalated findings or by the escalated verifier for superseded ones). Pre-existing findings go under Out of Scope. This findings text is also the handoff contract consumed by review-report when it dispatches a code-reviewer agent that runs this skill โ do not invent a different format.
Reference Files (on-demand)
These files live in references/ alongside this skill. Each is loaded only when the workflow reaches the step that needs it โ do not preload all of them.
| Reference file | What it contains | When to read |
|---|
references/verifier-prompt.md | The per-candidate verifier contract: verdict ladder (CONFIRMED / PLAUSIBLE / REFUTED), verification method, read-only constraint | Phase 2 โ before dispatching verifier subagents |