Ce Code Review

Code Review

Reviews code changes using dynamically selected reviewer personas. Spawns parallel sub-agents that return structured JSON, then merges and deduplicates findings into a single report.

When to Use

• Before creating a PR
• After completing a task during iterative implementation
• When feedback is needed on any code changes
• Can be invoked standalone
• Can run inside larger workflows; use mode:agent when the caller needs JSON instead of markdown tables

Argument Parsing

Parse $ARGUMENTS for optional tokens. Strip each recognized token before interpreting the remainder as a PR number, GitHub URL, or branch name.

Mode alias: mode:headless normalizes to mode:agent. mode:agent + mode:headless is not a conflict.

Conflicting arguments: Stop without dispatching reviewers when:

• Multiple incompatible scope selectors appear together (e.g. base: and a PR number/branch target — base: means "review the current checkout against this base")
• Multiple distinct mode: tokens other than the mode:agent/mode:headless alias pair

Deprecated mode:autofix is not a conflict — ignore the token and proceed with the normal flow (see below).

Emit a one-line failure reason. In mode:agent, return JSON: {"status":"failed","reason":"..."}.

Operating principles

Same pipeline for default and mode:agent:

• Apply locally; never push. Never push, open PRs, or file tickets in any mode — push is the outward step the user owns. In default (interactive) mode the review applies safe, verified fixes and commits them when the pre-review tree was clean (Stage 5c owns the full rule). In `mode:agent` it never mutates the tree — it reports and the caller applies.
• No blocking prompts. Never use AskUserQuestion, request_user_input, ask_user, or other blocking question tools. Infer intent, plan, and scope from explicit tokens, git state, PR metadata, and conversation. Note uncertainty in Coverage or the verdict — do not stop to ask.
• Explicit mutations only. Never run gh pr checkout, git checkout, git switch, or similar branch-switch commands. Passing a PR number, URL, or branch name selects review scope, not permission to mutate the working tree. To review local uncommitted work on a feature branch, check out that branch yourself (or stay on it) and pass base: or no target.
• Smart defaults. Untracked files: review tracked changes only and list excluded paths in Coverage. Plan: use plan: when passed; otherwise discover conservatively from PR body or branch keywords. Weak advisory P2/P3 from testing/maintainability alone: demote to testing_gaps / residual_risks per Stage 5.

Output format

mode:agent is report-only: it skips the Stage 5c apply (the caller applies) and serializes findings as JSON instead of markdown. It does not change reviewer selection, merge logic, or scope rules — the JSON is the deterministic contract for programmatic and cross-harness callers (Codex, Gemini, etc.). The default markdown is the human view; keep it ASCII-safe (pipe tables, -> not middot ·, no box-drawing) so it degrades gracefully across terminals.

Quick Review Short-Circuit

If $ARGUMENTS indicates the user wants a quick, fast, or light code review — and `mode:agent` is not active — do not dispatch the multi-agent flow.

Announce the chosen path before any other work (Quick review vs Multi-agent review). Skip this announcement when mode:agent is active.

Sequence:

1. Run the harness's built-in code review. Forward any review target after stripping tokens. Then stop — do not dispatch the multi-agent pipeline. 2. Exemption: If no built-in review exists, continue into the full multi-agent review. 3. `mode:agent` bypasses this short-circuit — always run the full multi-agent review and return JSON.

Deprecated: mode:autofix is no longer supported — there is no apply mode. If passed, ignore the token and proceed with the normal flow (default applies safe fixes via Stage 5c; mode:agent reports and the caller applies).

Severity Scale

All reviewers use P0-P3:

Action Routing

Severity answers urgency. autofix_class and owner are signal describing follow-up shape for callers — not apply permission or an apply gate. The apply decision is judgment (Stage 5c), not a function of autofix_class: default mode applies; in mode:agent this skill does not mutate the checkout — the caller applies. See references/action-class-rubric.md for persona guidance.

Routing rules:

• Synthesis owns the final route. Persona-provided routing metadata is input, not the last word.
• Choose the more conservative route on disagreement. A merged finding may move from gated_auto to manual, but never widen without stronger evidence.
• Reject `safe_auto` and `review-fixer` if present — drop the finding or remap to gated_auto / downstream-resolver during synthesis.
• `requires_verification: true` means any caller-applied fix needs targeted tests or follow-up validation.

Reviewers

14 reviewer personas in layered conditionals, plus CE agents. Quick roster with one-line triggers below; the persona catalog included at the bottom has the full per-persona selection criteria and spawn gates.

Always-on (every review): ce-correctness-reviewer, ce-testing-reviewer, ce-maintainability-reviewer, ce-project-standards-reviewer, plus CE agents ce-agent-native-reviewer and ce-learnings-researcher.

Cross-cutting conditional (per diff):

• ce-security-reviewer — auth, public endpoints, user input, permissions
• ce-performance-reviewer — DB queries, data transforms, caching, async
• ce-api-contract-reviewer — routes, serializers, type signatures, versioning
• ce-data-migration-reviewer — migration files / schema dumps / backfills (see spawn gate in Stage 3)
• ce-reliability-reviewer — error handling, retries, timeouts, background jobs
• ce-adversarial-reviewer — >=50 changed code lines, or auth / payments / data mutations / external APIs
• ce-previous-comments-reviewer — PR with existing review comments (PR-only, comment-gated)

Stack-specific conditional (per diff): ce-julik-frontend-races-reviewer (Stimulus/Turbo, DOM events, async UI) and ce-swift-ios-reviewer (Swift/SwiftUI/UIKit, entitlements, Core Data, .pbxproj).

CE conditional (migration-specific): ce-deployment-verification-agent — deployment checklist + rollback when the migration gate applies and the change is risky.

Review Scope

Every review spawns all 4 always-on personas plus the 2 CE always-on agents, then adds whichever cross-cutting and stack-specific conditionals fit the diff. The model naturally right-sizes: a small config change triggers 0 conditionals = 6 reviewers. A Rails auth feature might trigger security + reliability + adversarial = 9 reviewers.

Protected Artifacts

The following paths are compound-engineering pipeline artifacts and must never be flagged for deletion, removal, or gitignore by any reviewer:

• docs/brainstorms/* -- requirements documents created by ce-brainstorm
• docs/plans/*.md -- plan files created by ce-plan (decision artifacts; execution progress is derived from git, not stored in plan bodies)
• docs/solutions/*.md -- solution documents created during the pipeline

If a reviewer flags any file in these directories for cleanup or removal, discard that finding during synthesis.

How to Run

Stage 1: Determine scope

Compute the diff range, file list, and diff. Minimize permission prompts by combining into as few commands as possible.

If `base:` argument is provided (fast path):

The caller already knows the diff base. Skip all base-branch detection, remote resolution, and merge-base computation. Use the provided value directly:

BASE_ARG="{base_arg}"
BASE=$(git merge-base HEAD "$BASE_ARG" 2>/dev/null) || BASE="$BASE_ARG"

Then produce the same output as the other paths:

echo "BASE:$BASE" && echo "FILES:" && git diff --name-only $BASE && echo "DIFF:" && git diff -U10 $BASE && echo "UNTRACKED:" && git ls-files --others --exclude-standard

This path works with any ref — a SHA, origin/main, a branch name. Callers reviewing the current checkout should pass explicit base: when auto-detection is unnecessary. Do not combine `base:` with a PR number or branch target. If both are present, stop with an error: "Cannot use base: with a PR number or branch target — base: implies the current checkout is already the correct branch. Pass base: alone, or pass the target alone and let scope detection resolve the base."

If a PR number or GitHub URL is provided as an argument:

Do not check out the PR branch. Scope comes from GitHub read APIs plus optional local alignment when HEAD already matches the PR head branch.

Skip-condition pre-check. Before scope detection, run a PR-state probe:

gh pr view <number-or-url> --json state,title,body,files

Apply skip rules in order:

• state is CLOSED or MERGED -> stop with reason PR is closed/merged; not reviewing.
• Trivial-PR judgment: spawn a lightweight sub-agent (use model: haiku in Claude Code; gpt-5.4-nano or equivalent in Codex) with the PR title, body, and changed file paths. The agent's task: "Is this an automated or trivial PR that does not warrant a code review? Consider: dependency lock-file or manifest-only bumps, automated release commits, chore version increments with no substantive code changes. When in doubt, answer no — false negatives (skipped reviews that should have run) are more costly than false positives (unnecessary reviews)." If the judgment returns yes: stop with reason PR appears to be a trivial automated PR; not reviewing. Run without a PR argument to review the current branch, or pass base:<ref> if review is intended.

When any skip rule fires, stop without dispatching reviewers. Default mode: emit the reason as plain text. `mode:agent`: emit JSON only — {"status":"skipped","reason":"<same message>"} — so programmatic callers can parse the outcome. Standalone, `base:`, and branch-remote paths are unaffected. Draft PRs are reviewed normally.

If no skip rule fires, fetch PR metadata without checkout:

gh pr view <number-or-url> --json title,body,baseRefName,headRefName,headRefOid,isCrossRepository,url,files,reviews,comments --jq '{title, body, baseRefName, headRefName, headRefOid, isCrossRepository, url, files: [.files[].path], hasPriorComments: ((.reviews | map(select(.state != "APPROVED" or .body != "")) | length) > 0 or (.comments | length) > 0)}'

Set BASE: to pr:<number-or-url> (logical marker — not a git SHA). Set UNTRACKED: from git ls-files --others --exclude-standard on the current checkout (usually empty during PR-remote review).

PR scope mode. Classify as `local-aligned` only when all of these hold; otherwise use `pr-remote`. A matching branch name alone is not enough — a fork PR or a stale local branch can share a name with the PR head while pointing at unrelated code, and trusting the name would diff and inspect the wrong tree.

1. git rev-parse --abbrev-ref HEAD equals headRefName. 2. The PR is not cross-repository (isCrossRepository is false). 3. The PR head commit is contained in the local checkout: git merge-base --is-ancestor <headRefOid> HEAD exits 0. This confirms the working tree actually carries the PR head (allowing unpushed local fixes layered on top) rather than an unrelated same-named branch.

• `local-aligned` — all three checks pass. Local Read/Grep/git blame against workspace files are valid for PR changed paths.
• `pr-remote` — any check fails. The working tree is not the PR head; workspace file contents for changed paths may be stale or unrelated.

Diff by scope mode (do not mix remote and local diffs — contradictory hunks cause false positives):

• `local-aligned`: Resolve <resolved-base-ref> from baseRefName (fetch if needed). Compute BASE=$(git merge-base HEAD <resolved-base-ref>), then set FILES: from git diff --name-only $BASE and DIFF: from git diff -U10 $BASE (includes committed, staged, and unstaged changes on the PR branch). Do not call gh pr diff or append remote hunks — when unpushed fixes exist, the local tree is canonical. Note in Coverage: scope: local-aligned (PR; local tree diff).
• `pr-remote`: Set FILES: from the PR files array. Set DIFF: from gh pr diff <number-or-url> --color=never. If gh pr diff fails, stop with an actionable error — do not fall back to checkout.

When `pr-remote`, before Stage 4:

1. Best-effort fetch PR head without checkout: git fetch --no-tags origin <headRefName>:refs/review/pr-<number>-head (substitute PR number from metadata). 2. When fetch succeeds, set PR_HEAD_REF=refs/review/pr-<number>-head for reviewers and validators. When fetch fails, omit PR_HEAD_REF and note in Coverage — reviewers must rely on diff hunks only. 3. Best-effort fetch the PR base without checkout: git fetch --no-tags origin <baseRefName>. When it succeeds, resolve a concrete ref with git rev-parse FETCH_HEAD and set PR_BASE_REF to that SHA — a real git base ref reviewers and validators use for file-level git diffs (e.g. ce-data-migration-reviewer runs git diff <PR_BASE_REF> -- db/schema.rb/structure.sql). The pr:<number-or-url> logical marker in BASE: stays the scope marker; PR_BASE_REF is the diffable base. When the fetch fails, omit PR_BASE_REF and note in Coverage — schema-drift and other git-diff checks fall back to diff hunks only and must not assume main. 4. Include <pr-scope-mode>pr-remote</pr-scope-mode> and, when set, <pr-head-ref>...</pr-head-ref> and <pr-base-ref>...</pr-base-ref> in the Stage 4 review context bundle.

Reviewers and Stage 5b validators in `pr-remote` mode must not Read/Grep workspace paths for files in FILES:. Inspect via git show <PR_HEAD_REF>:<path> when PR_HEAD_REF is set, otherwise use only the provided diff hunks. `local-aligned` uses normal workspace inspection.

If a branch name is provided as an argument:

Substitute the provided branch name as <branch>. Do not check out <branch>.

If git rev-parse --abbrev-ref HEAD equals <branch>, use the standalone (current branch) path below — same tree, explicit branch name; do not use remote-only diff.

Otherwise diff the remote/local ref without checkout:

1. Try gh pr view <branch> --json baseRefName,url,headRefName — if a PR exists, prefer the PR number/URL path above (same remote diff rules). 2. Else resolve <branch> as origin/<branch> or <branch> after git fetch --no-tags origin <branch> when needed. 3. Resolve default base branch (same logic as standalone). Compute BASE=$(git merge-base <base-ref> <branch-ref>) and git diff -U10 $BASE <branch-ref>. 4. If <branch-ref> cannot be resolved locally, stop: "Cannot diff branch <branch> without checkout. Check out that branch, pass its open PR URL/number, or review the current branch with base:."

On success for remote branch diff, set branch-remote scope. The working tree is not <branch>. Include <pr-scope-mode>branch-remote</pr-scope-mode> and <branch-head-ref><branch-ref></branch-head-ref> in the Stage 4 review context bundle. Reviewers and Stage 5b validators must not Read/Grep workspace paths for files in FILES:. Inspect via git show <branch-ref>:<path> or diff hunks only.

Produce:

echo "BASE:$BASE" && echo "FILES:" && git diff --name-only $BASE <branch-ref> && echo "DIFF:" && git diff -U10 $BASE <branch-ref> && echo "UNTRACKED:" && git ls-files --others --exclude-standard

If no argument (standalone on current branch):

Apply the same base-detection logic as branch mode above, using the current branch (i.e., gh pr view --json baseRefName,url with no argument defaults to the current branch).

If no base can be resolved, stop. Do not fall back to git diff HEAD — a standalone review without the base would only show uncommitted changes and silently miss all committed work on the branch.

On success, produce the diff:

echo "BASE:$BASE" && echo "FILES:" && git diff --name-only $BASE && echo "DIFF:" && git diff -U10 $BASE && echo "UNTRACKED:" && git ls-files --others --exclude-standard

Using git diff $BASE (without ..HEAD) diffs the merge-base against the working tree, which includes committed, staged, and unstaged changes together.

Untracked file handling: Always inspect UNTRACKED:. Untracked paths are out of scope unless staged. When non-empty, list excluded files in Coverage and continue on tracked changes only — never stop or prompt.

Stage 2: Intent discovery

Understand what the change is trying to accomplish. The source of intent depends on which Stage 1 path was taken:

PR/URL mode: Use the PR title, body, and linked issues from gh pr view metadata. Supplement with commit messages from the PR if the body is sparse.

Branch mode: Run git log --oneline ${BASE}..<branch-ref> using the resolved merge-base and resolved branch ref from Stage 1. Use <branch-ref> (the resolved origin/<branch> or fetched ref), not the raw <branch> argument — a remote-only branch has no matching local ref, so the raw name would fail or read a stale same-named local branch.

Standalone (current branch): Run:

echo "BRANCH:" && git rev-parse --abbrev-ref HEAD && echo "COMMITS:" && git log --oneline ${BASE}..HEAD

Combined with conversation context (plan section summary, PR description), write a 2-3 line intent summary:

Intent: Simplify tax calculation by replacing the multi-tier rate lookup
with a flat-rate computation. Must not regress edge cases in tax-exempt handling.

Pass this to every reviewer in their spawn prompt. Intent shapes how hard each reviewer looks, not which reviewers are selected.

When intent is ambiguous: Infer from branch name, commits, PR title/body, diff, plan:, and conversation. Write the best-effort intent summary and note uncertainty in Coverage — never block on a clarifying question.

Stage 2b: Plan discovery (requirements verification)

Locate the plan document so Stage 6 can verify requirements completeness. Check these sources in priority order — stop at the first hit:

1. `plan:` argument. If the caller passed a plan path, use it directly. Read the file to confirm it exists. 2. PR body. If PR metadata was fetched in Stage 1, scan the body for paths matching docs/plans/*.md. If exactly one match is found and the file exists, use it as plan_source: explicit. If multiple plan paths appear, treat as ambiguous — demote to plan_source: inferred for the most recent match that exists on disk, or skip if none exist or none clearly relate to the PR title/intent. Always verify the selected file exists before using it — stale or copied plan links in PR descriptions are common. 3. Auto-discover. Extract 2-3 keywords from the branch name (e.g., feat/onboarding-skill -> onboarding, skill). Glob docs/plans/* and filter filenames containing those keywords. If exactly one match, use it. If multiple matches or the match looks ambiguous (e.g., generic keywords like review, fix, update that could hit many plans), skip auto-discovery — a wrong plan is worse than no plan. If zero matches, skip.

Confidence tagging: Record how the plan was found:

• plan: argument -> plan_source: explicit (high confidence)
• Single unambiguous PR body match -> plan_source: explicit (high confidence)
• Multiple/ambiguous PR body matches -> plan_source: inferred (lower confidence)
• Auto-discover with single unambiguous match -> plan_source: inferred (lower confidence)

If a plan is found, read its Requirements section — ## Requirements in current plans, ## Requirements Trace in legacy ones — and the R-IDs (R1, R2, etc.) listed there, plus Implementation Units (current numeric subsections such as ### U1., ### U2., or ### Unit 1: under ## Implementation Units; legacy bullet or checkbox unit entries under that section also count). Store the extracted requirements list and plan_source for Stage 6. Do not block the review if no plan is found — requirements verification is additive, not required.

Stage 3: Select reviewers

Read the diff and file list from Stage 1. The 4 always-on personas and 2 CE always-on agents are automatic. For each cross-cutting and stack-specific conditional persona in the persona catalog included below, decide whether the diff warrants it. This is agent judgment, not keyword matching.

File-type awareness for conditional selection: Instruction-prose files (Markdown skill definitions, JSON schemas, config files) are product code but do not benefit from runtime-focused reviewers. The adversarial reviewer's techniques (race conditions, cascade failures, abuse cases) target executable code behavior. For diffs that only change instruction-prose files, skip adversarial unless the prose describes auth, payment, or data-mutation behavior. Count only executable code lines toward line-count thresholds.

`previous-comments` is PR-only AND comment-gated. Only select this persona when both conditions hold:

1. Stage 1 gathered PR metadata (PR number or URL was provided as an argument, or gh pr view returned metadata for the current branch). 2. hasPriorComments from Stage 1 is true (the PR has at least one review submission or issue comment).

Skip it for standalone branch reviews with no associated PR, and skip it for PRs with no prior feedback yet -- there is nothing for the persona to verify, and a spawned subagent that returns empty findings still costs the full subagent startup overhead (persona spec, diff, schema, plus its own gh calls).

Stack-specific personas are additive when runtime behavior warrants them. A Hotwire UI change may warrant julik-frontend-races; a TypeScript API diff may warrant api-contract and reliability.

`data-migration` spawn gate. Select ce-data-migration-reviewer only when the diff includes at least one migration or schema artifact: db/migrate/*, db/schema.rb, db/structure.sql, Alembic/Flyway/Liquibase migration paths, or explicit backfill/data-transform scripts (rake tasks, one-off data migration classes). Do not spawn for model-only changes, query-only refactors, serializers/controllers that reference columns without a migration or schema dump in the diff, or migration tests alone.

For ce-deployment-verification-agent, use the same migration-artifact gate when the change is risky (destructive DDL, backfills, NOT NULL without default, column renames/drops).

Announce the team before spawning:

Review team:
- correctness (always)
- testing (always)
- maintainability (always)
- project-standards (always)
- ce-agent-native-reviewer (always)
- ce-learnings-researcher (always)
- security -- new endpoint in routes.rb accepts user-provided redirect URL
- julik-frontend-races -- Stimulus controller with async DOM updates
- data-migration -- adds migration 20260303_add_index_to_orders
- ce-deployment-verification-agent -- destructive migration with backfill

This is progress reporting, not a blocking confirmation.

Stage 3b: Discover project standards paths

Before spawning sub-agents, find the file paths (not contents) of all relevant standards files for the project-standards persona. Use the native file-search/glob tool to locate:

1. Use the native file-search tool (e.g., Glob in Claude Code) to find all **/CLAUDE.md and **/AGENTS.md in the repo. 2. Filter to those whose directory is an ancestor of at least one changed file. A standards file governs all files below it (e.g., plugins/compound-engineering/AGENTS.md applies to everything under plugins/compound-engineering/).

Pass the resulting path list to the project-standards persona inside a <standards-paths> block in its review context (see Stage 4). The persona reads the files itself, targeting only the sections relevant to the changed file types. This keeps the orchestrator's work cheap (path discovery only) and avoids bloating the subagent prompt with content the reviewer may not fully need.

Stage 4: Spawn sub-agents

Model tiering

Three reviewers inherit the session model with no override: ce-correctness-reviewer, ce-security-reviewer, and ce-adversarial-reviewer. These perform the highest-stakes analysis — logic bugs, security vulnerabilities, adversarial failure scenarios — and should run at whatever capability level the user has configured. If the user is on Opus, these get Opus.

All other persona sub-agents and CE agents use the platform's mid-tier model to reduce cost and latency. See the Spawning subsection below for the exact dispatch-time override.

The orchestrator (this skill) also inherits the session model; it handles intent discovery, reviewer selection, finding merge/dedup, and synthesis.

Run ID

Generate a unique run identifier before dispatching any agents. This ID scopes all agent artifact files and the post-review run artifact to the same directory.

RUN_ID=$(date +%Y%m%d-%H%M%S)-$(head -c4 /dev/urandom | od -An -tx1 | tr -d ' ')
mkdir -p "/tmp/compound-engineering/ce-code-review/$RUN_ID"

Pass {run_id} to every persona sub-agent so they can write their full analysis to /tmp/compound-engineering/ce-code-review/{run_id}/{reviewer_name}.json.

Large shared context — pass paths, not contents. The diff and file list go to every reviewer and validator. When inlining them into each subagent prompt would be wasteful (many files / a big diff), write them once into the run dir (e.g. full.diff, files.txt) and pass those paths in the diff / changed-files slots instead of inline content — the subagent and validator templates instruct the child to Read a staged path. Inline a small diff directly.

Spawning

Omit the mode parameter when dispatching sub-agents so the user's configured permission settings apply. Do not pass mode: "auto".

Model override at dispatch time. Pass the platform's mid-tier model on every dispatch except ce-correctness-reviewer, ce-security-reviewer, and ce-adversarial-reviewer, which inherit the session model (per the Model tiering subsection above). In Claude Code, add model: "sonnet" to the Agent tool call. In Codex, pass the equivalent mid-tier on spawn_agent (e.g., gpt-5.4-mini as of April 2026). In Pi, pass the equivalent on subagent via the pi-subagents extension. On platforms where the dispatch primitive has no model-override parameter or the available model names are unknown, omit the override — a working review on the parent model beats a broken dispatch on an unrecognized name. Check this on every Agent / spawn_agent / subagent call in the parallel dispatch; omitting it on Opus sessions silently 3-4x's the cost of a review.

Bounded parallel dispatch. Respect the current harness's active-subagent limit. Queue selected reviewers, dispatch only as many as the harness accepts, and fill freed slots as reviewers complete. Treat active-agent/thread/concurrency-limit spawn errors as backpressure, not reviewer failure: leave the reviewer queued and retry after a slot frees. Record a reviewer as failed only after a successful dispatch times out/fails, or when dispatch fails for a non-capacity reason.

Spawn each selected persona reviewer using the subagent template included below. Each persona sub-agent receives:

1. Their persona file content (identity, failure modes, calibration, suppress conditions) 2. Shared diff-scope rules from the diff-scope reference included below 3. The JSON output contract from the findings schema included below 4. PR metadata: title, body, and URL when reviewing a PR (empty string otherwise). Passed in a <pr-context> block so reviewers can verify code against stated intent 5. Review context: intent summary, file list, diff, scope mode (local-aligned | pr-remote | branch-remote), and remote head ref (PR_HEAD_REF or <branch-head-ref>) when set 6. Run ID and reviewer name for the artifact file path 7. For `project-standards` only: the standards file path list from Stage 3b, wrapped in a <standards-paths> block appended to the review context 8. For `data-migration` only: the resolved review base ref from Stage 1 (BASE: marker), wrapped in <review-base> inside the review context so schema drift checks never assume main

Persona sub-agents are read-only with respect to the project: they review and return structured JSON. They do not edit project files or propose refactors. The one permitted write is saving their full analysis to the run-artifact path specified in the output contract (under /tmp/compound-engineering/ce-code-review/<run-id>/).

Read-only here means non-mutating, not "no shell access." Reviewer sub-agents may use non-mutating inspection commands when needed to gather evidence or verify scope, including read-oriented git / gh usage such as git diff, git show, git blame, git log, and gh pr view. In `pr-remote` or `branch-remote` scope (see Stage 1), inspect changed files via git show <remote-head-ref>:<path> or diff hunks — do not Read/Grep workspace paths for files in scope. They must not edit project files, change branches, commit, push, create PRs, or otherwise mutate the checkout or repository state.

Each persona sub-agent writes full JSON (all schema fields) to /tmp/compound-engineering/ce-code-review/{run_id}/{reviewer_name}.json and returns compact JSON with merge-tier fields only:

{
  "reviewer": "security",
  "findings": [
    {
      "title": "User-supplied ID in account lookup without ownership check",
      "severity": "P0",
      "file": "orders_controller.rb",
      "line": 42,
      "confidence": 100,
      "autofix_class": "gated_auto",
      "owner": "downstream-resolver",
      "requires_verification": true,
      "pre_existing": false,
      "suggested_fix": "Add current_user.owns?(account) guard before lookup"
    }
  ],
  "residual_risks": [...],
  "testing_gaps": [...]
}

The artifact file must carry the detail-tier fields (why_it_matters, evidence); the compact return omits them, but writing the compact shape to the artifact (a common reviewer slip) silently strips the detail Coverage and the keyed detail lines depend on. However review context is delivered — inlined, or staged to disk for a large diff — each reviewer still receives the full subagent-template output contract; staging context never licenses a thinner one. suggested_fix is optional in both tiers -- included in compact returns when present so callers can apply fixes after review. If the file write fails, the compact return still provides everything the merge needs.

CE always-on agents (ce-agent-native-reviewer, ce-learnings-researcher) are dispatched as standard Agent calls through the same bounded parallel scheduler as the persona agents. Give them the same review context bundle the personas receive: entry mode, any PR metadata gathered in Stage 1, intent summary, review base branch name when known, BASE: marker, file list, diff, and UNTRACKED: scope notes. Do not invoke them with a generic "review this" prompt. Their output is unstructured and synthesized separately in Stage 6.

CE conditional agents (ce-deployment-verification-agent only) are dispatched as standard Agent calls through the same bounded parallel scheduler when the migration-artifact gate applies. Pass the same review context bundle plus the applicability reason (for example, which migration files triggered the agent). Their output is unstructured and must be preserved for Stage 6 synthesis just like the CE always-on agents. Schema drift is handled by the data-migration persona as structured findings — not here.

Stage 5: Merge findings

Convert multiple reviewer compact JSON returns into one deduplicated, confidence-gated finding set. The compact returns contain merge-tier fields (title, severity, file, line, confidence, autofix_class, owner, requires_verification, pre_existing) plus the optional suggested_fix. Detail-tier fields (why_it_matters, evidence) are on disk in the per-agent artifact files and are not loaded at this stage.

confidence is one of 5 discrete anchors (0, 25, 50, 75, 100) with behavioral definitions in the findings schema. Synthesis treats anchors as integers; do not coerce to floats.

1. Validate. Check each compact return for required top-level and per-finding fields, plus value constraints. Drop malformed returns or findings. Record the drop count.

• Top-level required: reviewer (string), findings (array), residual_risks (array), testing_gaps (array). Drop the entire return if any are missing or wrong type.
• Per-finding required: title, severity, file, line, confidence, autofix_class, owner, requires_verification, pre_existing
• Value constraints:
• severity: P0 | P1 | P2 | P3
• autofix_class: gated_auto | manual | advisory
• owner: downstream-resolver | human | release
• confidence: integer in {0, 25, 50, 75, 100}
• line: positive integer
• pre_existing, requires_verification: boolean
• Do not validate against the full schema here -- the full schema (including why_it_matters and evidence) applies to the artifact files on disk, not the compact returns.

2. Deduplicate. Compute fingerprint: normalize(file) + line_bucket(line, +/-3) + normalize(title). When fingerprints match, merge: keep highest severity, keep highest anchor, note which reviewers flagged it. Dedup runs over the full validated set (including anchor 50) so cross-reviewer promotion in step 3 can lift matching anchor-50 findings into the actionable tier. 3. Cross-reviewer agreement. When 2+ independent reviewers flag the same issue (same fingerprint), promote the merged finding by one anchor step: 50 -> 75, 75 -> 100, 100 -> 100. Note the agreement in the Reviewer column of the output (e.g., "security, correctness"). 4. Separate pre-existing. Pull out findings with pre_existing: true into a separate list. 5. Resolve disagreements. When reviewers flag the same code region but disagree on severity, autofix_class, or owner, annotate the Reviewer column with the disagreement (e.g., "security (P0), correctness (P1) -- kept P0"). 6. Normalize routing. For each merged finding, set the final autofix_class, owner, and requires_verification. If reviewers disagree, keep the more conservative route. Remap any legacy safe_auto or review-fixer to gated_auto / downstream-resolver. 6b. Mode-aware demotion of weak general-quality findings. Some persona output is real signal but does not warrant primary-findings attention. Reroute it to the existing soft buckets so the primary findings table stays focused on actionable issues.

A finding qualifies for demotion when all of these hold:

• Severity is P2 or P3 (P0 and P1 always stay in primary findings)
• autofix_class is advisory (concrete-fix findings stay in primary)
• All contributing reviewers are testing or maintainability — if any other persona also flagged this finding, cross-reviewer corroboration is present and the finding stays in primary findings regardless of its severity or advisory status (expand the weak-signal list later only with evidence)

When a finding qualifies:

• Move demoted findings out of the primary set. If the contributing reviewer is testing, append <file:line> -- <title> to testing_gaps. If maintainability, append to residual_risks. Use title-only lines (compact return omits why_it_matters). Record the demotion count for Coverage.

7. Confidence gate. After dedup, promotion, and demotion have shaped the primary set, suppress remaining findings below anchor 75. Exception: P0 findings at anchor 50+ survive the gate -- critical-but-uncertain issues must not be silently dropped. Record the suppressed count by anchor (so Coverage can report "N findings suppressed at anchor 50, M at anchor 25"). The gate runs late deliberately: anchor-50 findings need a chance to be promoted by step 3 (cross-reviewer corroboration) or rerouted by step 6b (mode-aware demotion to soft buckets) before any drop decision. 8. Partition the work. Build two sets:

• actionable queue: gated_auto or manual findings whose owner is downstream-resolver (hand off to caller)
• report-only queue: advisory findings plus anything owned by human or release

9. Sort and number. Order by severity (P0 first) -> anchor (descending) -> file path -> line number, then assign monotonically increasing # values across the full primary finding set in that sorted order. Do not restart numbering inside each severity table or autofix/routing bucket. If later sections repeat a finding (for example Actionable Findings), reuse the same stable # so users and downstream workflows can reference findings by # across the report and caller handoff. 10. Collect coverage data. Union residual_risks and testing_gaps across reviewers. 11. Preserve CE agent artifacts. Keep the learnings, agent-native, and deployment-verification outputs alongside the merged finding set. Do not drop unstructured agent output just because it does not match the persona JSON schema. Schema drift from data-migration is already in the merged finding set.

Stage 5b: Validation pass (optional quality gate)

Independent verification gate. Spawn one validator sub-agent per surviving finding using references/validator-template.md. Findings the validator rejects are dropped; confirmed findings flow through unchanged.

When this stage runs: After Stage 5 whenever at least one finding survives — skip only when zero survive. When more than 15 survive, do not skip the stage; validate per the budget cap in step 2. The default method is the per-finding validator wave (steps below); a surviving P2/P3 finding at anchor 100 may instead be validated by direct first-party verification (see below). Same rule for default and mode:agent.

Steps:

1. Select findings to validate. All survivors of Stage 5. 2. Apply dispatch budget cap. If the selected set exceeds 15 findings, validate the highest-severity 15 (P0 first, then P1, then P2, then P3, breaking ties by anchor descending), dropping only from the P2/P3 tail. Never drop a P0 or P1 from validation — if P0/P1 findings alone exceed 15, raise the cap to include all of them. Record the over-budget count (the dropped P2/P3 tail) for the Coverage section. 3. Spawn validators with bounded parallelism. One sub-agent per finding, dispatched independently using the validator template and the same bounded scheduler from Stage 4. Each validator receives:

• The finding's title, severity, file, line, suggested_fix, original reviewer name, and confidence anchor
• why_it_matters when available — loaded from the per-agent artifact file at /tmp/compound-engineering/ce-code-review/{run_id}/{reviewer_name}.json; omit when the file is absent or the artifact write failed. The validator proceeds without it, using the diff and cited code directly.
• The full diff
• The scope mode and remote head ref, mirroring the Stage 4 reviewer bundle: inject <pr-scope-mode>local-aligned | pr-remote | branch-remote</pr-scope-mode> and, when set, <pr-head-ref>...</pr-head-ref> or <branch-head-ref>...</branch-head-ref>. The validator template defaults to local-aligned workspace inspection when these are absent, so omitting them in pr-remote/branch-remote makes validators verify findings against the stale working tree — dropping valid findings or confirming false ones on the wrong tree.
• Inspection access scoped by mode: in local-aligned, Read/Grep/git blame the cited code, callers, guards, framework defaults, and history; in pr-remote/branch-remote, inspect via git show <remote-head-ref>:<path> or the provided diff hunks only — do not Read/Grep workspace paths for files in scope.

4. Collect verdicts. Each validator returns { "validated": true | false, "reason": "<one sentence>" }.

• validated: true -> finding survives unchanged into Stage 6
• validated: false -> finding is dropped; record the validator's reason in Coverage
• Validator infrastructure failure (timeout, dispatch error, malformed JSON — not a validated:false verdict): for P2/P3, drop the finding with reason "validator failed" (conservative bias). For P0/P1, do not drop on infra failure — keep the finding and mark its validation degraded (note in Coverage). A transient validator failure must never silently remove a critical/high finding; a genuine validated:false rejection above still drops at any severity.

5. Use mid-tier model for validators. Same model class (sonnet) the persona reviewers use. Validators are read-only — same constraints as persona reviewers. They may use non-mutating inspection commands (Read, Grep, Glob, git blame, gh). 6. Record metrics for Coverage. Total dispatched, validated true count, validated false count (with reasons), infra failures (and any P0/P1 kept-on-failure as degraded), and over-budget drops.

Orchestrator direct verification. When a finding hinges on a fact the orchestrator can check cheaply and authoritatively — a pinned dependency's source, a wiring/config fact in this repo, a build tag — verify it directly with single-purpose native tools (Read/Grep/Glob, one git command at a time), never chained or error-suppressed shell. Fold confirmed facts into synthesis. Whether it can replace the independent validator turns on a single distinction: the orchestrator is not an independent second opinion (it synthesized these findings), so direct verification catches a wrong fact but not the orchestrator's own bias. Independence adds nothing to a mechanically-checkable fact and everything to a judgment call:

• P0/P1, any anchor: the per-finding validator wave is required; direct verification only complements it, never replaces it.
• P2/P3 at anchor 100 (verifiable from code alone — compile/type error, definitive logic bug, quotable standards violation, no interpretation): direct verification may stand in for the wave; note the method in Coverage.
• P2/P3 at anchor 75 (judgment call — "will affect users," not airtight): the independent wave is required — this is exactly where a fresh second opinion filters false positives, and the orchestrator cannot supply that for its own findings.

Why per-finding bounded dispatch (not batched): Independence is the point. A single batched validator looking at all findings together pattern-matches across them and recreates the persona-bias problem. Per-finding dispatch preserves fresh context while the scheduler respects harness limits.

Stage 5c: Act on findings (default mode only)

Skip entirely in `mode:agent` — that mode is a machine handoff and the caller owns apply. In default (interactive) mode the review is the top-level agent, so it applies the fixes it is confident in before presenting the report.

Act policy (bias to act). Default to applying every finding that is a clear improvement and a reversible edit, regardless of severity. The work is a tracked, visible diff that can be reverted — so leaving a clean fix unapplied "to be safe" is the failure mode, not the safe choice. Decide by judgment, not a safety checklist:

• Apply clear improvements — the common case (test hardening, dead-code removal, a localized fix with a concrete suggested_fix).
• Push back — do not apply — when the reviewer is wrong; keep the finding and state the disagreement with reasoning.
• Skip with judgment taste calls and conflicting suggestions, but surface what was skipped and why. Never silently drop.

Severity, confidence, and cross-reviewer agreement tell you what to do first and what to flag loudly — they do not gate the decision. There is no deny-list: downside is controlled after the fact (revert + visible diff + the commit checkpoint), not by a precondition.

Scope invariant. Apply only when the working tree is what was reviewed — local-aligned or standalone. In pr-remote / branch-remote the working tree is not the reviewed head; do not apply — report instead.

Verify, then keep. After applying, run the affected tests and lint (targeted by default; broaden when fixes span files). If they fail, revert that fix and report it as a finding instead — an unverified fix is not finished. Never leave the tree red.

Commit when the pre-review tree was clean. Before applying, note whether the working tree already had uncommitted changes (git status --porcelain). The permanence gate is the push, not the commit — a local commit is private and reversible (git reset --soft HEAD~1).

• Clean before the review: after applying and verifying, commit the fixes as one isolated, review-labeled fix commit — fix(review): <summary>, or the repo's nearest convention if review isn't an allowed scope. Labeled and reversible, returning the tree to a known state.
• Dirty before the review: apply but do not commit — the fixes interleave with the user's in-flight work and ride along with the commit they were already going to make. The Applied section lists what changed.
• Never push, open a PR, or file tickets — that's the outward-facing step the user owns.

Surface green-but-unverifiable edits. When an applied fix touches auth/authz, a public or cross-service contract/schema, or concurrency/ordering, a passing test does not prove safety — flag it prominently in the Applied section so the diff reviewer's attention goes there.

Stage 6: Synthesize and present

Assemble the final report. Default: pipe-delimited markdown tables for findings (mandatory — see review output template). `mode:agent`: skip markdown and emit JSON (see ### JSON output format). Other sections (Actionable Findings, Learnings, Coverage, etc.) use bullets and --- before the verdict in markdown mode only.

Before writing the report, load `references/review-output-template.md` and mirror it — that file is the canonical skeleton (full per-section structure). The block below is the always-loaded fallback so the shape survives a long session even if the template was not reloaded.

Findings table shape (default mode — load-bearing, do not improvise). Every finding is a row in a pipe-delimited table grouped by severity, with a terse Issue cell; depth goes in a keyed detail line under the table. Copy this shape; do not invent a layout:

• #1 — full explanation here (why it matters + concrete fix direction), as a keyed detail line under the table.

Per-severity tables are 5 columns — Route is not shown here (it appears only in the Actionable Findings table and the JSON). Keep the Issue cell to one short clause (roughly 12 words or fewer, no second sentence, no because/so/which explanation) — it is the scannable index, not the explanation. The moment a cell wants a comma-plus-clause or a reason, move that depth into the keyed detail line (- **#N** — …) instead of packing it in — usually for P0/P1; P2/P3 are typically terse-only.

*Never produce these shapes (instant fail — applies to every tabular section, the Applied table included, not just the severity findings; if you catch one mid-draft, re-render before delivering):*

• Any row — a finding or an Applied entry — rendered as Field:-prefixed blocks (#: / Sev: / File: / Issue: / Fix: / Route: lines) — depth goes in the keyed detail line, never a field block
• Per-row separators made of horizontal rules or box-drawing characters (────, ———)
• A table replaced by a plain bulleted/numbered list (the keyed - **#N** — detail line under a table is a supplement, not a replacement — that is expected)
• Unicode separators or arrows in cells (middot ·); use ASCII ->
• Inconsistent treatment across severities or sections (e.g. P1 as blocks while P2/P3 are tables, or the Applied table as field-blocks while findings are tables) — every table uses the same pipe-delimited shape

1. Header. Scope, intent, mode, reviewer team with per-conditional justifications. 2. Applied (default mode only). When Stage 5c applied fixes, list them first — before the findings tables — in an Applied section (see review output template) as a pipe table | # | File | Fix | Reviewer | — never Field:-blocks or ──── separators, same rules as the findings tables — then a one-line validation outcome (e.g. "pin tests 4 -> 6; suite 94 pass, lint clean") and commit status (committed on a clean tree as fix(review): … or the repo's nearest convention, or left uncommitted for the user on a dirty one). Flag green-but-unverifiable edits (auth/contract/concurrency) prominently. Omit this section in mode:agent and when nothing was applied. Applied findings appear here, not in the severity tables. 3. Findings. Pipe-delimited tables grouped by severity (### P0 -- Critical, ### P1 -- High, ### P2 -- Moderate, ### P3 -- Low), using the shape above — the same shape for every severity. Omit empty severity levels. Finding numbers come from the stable assignment in Stage 5 -- never re-derive them per severity table. 4. Requirements Completeness. Include only when a plan was found in Stage 2b. For each requirement (R1, R2, etc.) and implementation unit in the plan, report whether corresponding work appears in the diff. Use a simple checklist: met / not addressed / partially addressed. Routing depends on plan_source:

• `explicit` (caller-provided or PR body): Flag unaddressed requirements or implementation units as P1 findings with autofix_class: manual, owner: downstream-resolver. These enter the actionable queue.
• `inferred` (auto-discovered): Flag unaddressed requirements or implementation units as P3 findings with autofix_class: advisory, owner: human. These stay in the report only — no autonomous follow-up. An inferred plan match is a hint, not a contract.

Omit this section entirely when no plan was found — do not mention the absence of a plan. 5. Actionable Findings. Include when the actionable queue is non-empty — findings the caller should address (gated_auto / manual with downstream-resolver), plus anything Stage 5c chose not to apply. In default mode, findings already applied appear in the Applied section, not here. 6. Pre-existing. Separate section, does not count toward verdict. 7. Learnings & Past Solutions. Surface ce-learnings-researcher results: if past solutions are relevant, flag them as "Known Pattern" with links to docs/solutions/ files. 8. Agent-Native Gaps. Surface ce-agent-native-reviewer results. Omit section if no gaps found. 9. Deployment Notes. If ce-deployment-verification-agent ran, surface the key Go/No-Go items: blocking pre-deploy checks, the most important verification queries, rollback caveats, and monitoring focus areas. Keep the checklist actionable rather than dropping it into Coverage. Schema drift appears in the findings tables as data-migration P1 rows — do not add a separate Schema Drift section. 10. Coverage. Applied count (when Stage 5c ran), suppressed count by anchor (e.g., "N findings suppressed at anchor 50, M at anchor 25"), mode-aware demotion count, validator drop count and reasons (when Stage 5b ran), any P0/P1 with degraded validation (kept on validator infra failure), validator over-budget drops (when the 15-cap fired), residual risks, testing gaps, failed/timed-out reviewers, and inferred-intent uncertainty when applicable. 11. Verdict. Ready to merge / Ready with fixes / Not ready. Fix order if applicable. When an explicit plan has unaddressed requirements or implementation units, the verdict must reflect it — a PR that's code-clean but missing planned requirements is "Not ready" unless the omission is intentional. When an inferred plan has unaddressed requirements or implementation units, note it in the verdict reasoning but do not block on it alone.

Do not include time estimates.

Format verification (default only — last gate before delivering). Before delivering, scan every table — the Applied table and each severity findings table — for the forbidden shapes: Field:-prefixed blocks (#: / File: / Fix: / Issue:), box-drawing or horizontal-rule separators (────), middot ·, or a list replacing a table. The Applied table is the most common offender — check it explicitly. If any table hit one of these, STOP and re-render it as the same pipe-delimited shape before delivering. (The keyed - **#N** — detail line under a table is expected — not a failure.) Skip only when mode:agent is active.

JSON output format (mode:agent only)

Emit one raw JSON object as the primary response — a single bare JSON value, no markdown code fence. A leading ``` `json `` fence makes the response start with backticks and breaks naive JSON.parse consumers, so never wrap it. Also write review.json under /tmp/compound-engineering/ce-code-review/<run-id>/` with the same payload.

mode:agent does not apply fixes — the caller does — so there is no applied_fixes field; the handoff is actionable_findings. Applied work surfaces only in the default-mode markdown Applied section (Stage 5c/6).

Minimum shape:

{
  "status": "complete",
  "verdict": "Ready to merge | Ready with fixes | Not ready",
  "scope": {
    "base": "<merge-base sha, pr:NNN marker, or base: ref>",
    "branch": "<current branch name>",
    "head_sha": "<git rev-parse HEAD>",
    "pr_url": "<url or null>",
    "files_changed": 0
  },
  "intent": "<2-3 line summary>",
  "intent_confidence": "explicit | inferred | uncertain",
  "reviewers": ["correctness", "security"],
  "findings": [],
  "actionable_findings": [],
  "pre_existing_findings": [],
  "requirements_completeness": null,
  "learnings": [],
  "agent_native_gaps": [],
  "deployment_notes": [],
  "residual_risks": [],
  "testing_gaps": [],
  "coverage": {},
  "artifact_path": "/tmp/compound-engineering/ce-code-review/<run-id>/",
  "run_id": "<run-id>"
}

Each object in findings uses the merged finding fields: #, title, severity, file, line, confidence, autofix_class, owner, requires_verification, pre_existing, suggested_fix, why_it_matters, evidence, reviewers.

actionable_findings lists the gated_auto / manual + downstream-resolver subset with the same fields plus stable #.

On failure before review completes, set "status": "failed" and "reason": "<one sentence>". When all reviewers fail, use "status": "degraded" with a reason. When a PR skip rule fires (closed/merged/trivial), use "status": "skipped" with the skip reason. Do not emit markdown tables when mode:agent is active.

Quality Gates

Before delivering the review, verify:

1. Every finding is actionable. Re-read each finding. If it says "consider", "might want to", or "could be improved" without a concrete fix, rewrite it with a specific action. Vague findings waste engineering time. 2. No false positives from skimming. For each finding, verify the surrounding code was actually read. Check that the "bug" isn't handled elsewhere in the same function, that the "unused import" isn't used in a type annotation, that the "missing null check" isn't guarded by the caller. 3. Severity is calibrated. A style nit is never P0. A SQL injection is never P3. Re-check every severity assignment. 4. Line numbers are accurate. Verify each cited line number against the file content. A finding pointing to the wrong line is worse than no finding. 5. Protected artifacts are respected. Discard any findings that recommend deleting or gitignoring files in docs/brainstorms/, docs/plans/, or docs/solutions/. 6. Findings don't duplicate linter output. Don't flag things the project's linter/formatter would catch (missing semicolons, wrong indentation). Focus on semantic issues.

Language-Aware Conditionals

Stack-specific reviewers fire only when the diff touches runtime behavior they specialize in (async UI races, iOS/Swift lifecycle) — never mechanically from file extensions alone; the trigger is meaningful changed behavior in that stack's runtime domain. Structural quality (complexity deletion, 1k-line regressions, type-boundary leaks) lives in the always-on ce-maintainability-reviewer; do not spawn extra reviewers for language conventions, philosophy, or "strict bar" passes.

After Review

After Stage 6, stop. Never push, open PRs, or file tickets from this skill. In default (interactive) mode, Stage 5c has already applied and (on a clean pre-review tree) committed the safe fixes; in mode:agent the review mutates nothing — the caller (for example ce-work) and the user apply fixes, file tickets, or accept residual risk using the report and artifact.

Emit actionable findings summary (default mode only)

After Stage 6 in default mode, emit a compact Actionable Findings summary for callers:

• List each actionable finding (gated_auto or manual with downstream-resolver) with stable #, severity, file:line, title, autofix_class, whether suggested_fix is present, and confidence.
• Include the run-artifact path when one was written: /tmp/compound-engineering/ce-code-review/<run-id>/
• When the actionable queue is empty, state Actionable findings: none. explicitly.

In mode:agent do not emit this markdown summary — the actionable findings are carried solely by the actionable_findings field of the JSON object. Emit nothing after the JSON object, so the response stays a single parseable JSON value.

Do not run post-review triage (no per-finding walk-through, bulk ticket filing, or routing questions). The report and summary are the complete handoff.

Mode-specific completion

Do not offer push/PR/create-branch next steps from this skill.

Run artifacts

Always write run artifacts under /tmp/compound-engineering/ce-code-review/<run-id>/:

• synthesized findings
• actionable findings list
• advisory outputs
• per-agent {reviewer_name}.json from Stage 4
• report.md — the rendered markdown report exactly as presented to the user (default mode only), so format and numbering stay auditable after the run

metadata.json minimum fields:

{
  "run_id": "<run-id>",
  "branch": "<git branch --show-current at dispatch time>",
  "head_sha": "<git rev-parse HEAD at dispatch time>",
  "verdict": "<Ready to merge | Ready with fixes | Not ready>",
  "completed_at": "<ISO 8601 UTC timestamp>"
}

Capture branch and head_sha at dispatch time (no in-skill fixes will land afterward).

Fallback

If the platform doesn't support parallel sub-agents, run reviewers sequentially. If the platform supports sub-agents but caps active concurrency, use the bounded queueing rules in Stage 4 rather than treating cap-related spawn failures as reviewer failures. Everything else (stages, output format, merge pipeline) stays the same.

---

Included References

The files below are inlined at load time. The review output template is not inlined — Stage 6 loads it on demand (references/review-output-template.md).

Persona Catalog

@./references/persona-catalog.md

Subagent Template

@./references/subagent-template.md

Diff Scope Rules

@./references/diff-scope.md

Action class rubric

@./references/action-class-rubric.md

Findings Schema

@./references/findings-schema.json

autofix_class rubric (personas)

autofix_class describes the intrinsic shape of follow-up work — it is signal, not an apply gate or permission. In mode:agent the caller interprets findings and owns apply; in default (interactive) mode the review applies safe fixes itself by judgment (SKILL.md Stage 5c). Either way the class informs what to do first and what to flag — it does not mechanically decide what gets applied.

Persona guidance

• Prefer gated_auto when you can write a defensible suggested_fix for a localized change.
• Use manual when the right fix depends on product intent, architecture, or cross-cutting refactors.
• Use advisory when nothing breaks if left unfixed but the observation has value.
• Do not emit safe_auto — callers decide what to apply; reviewers classify and propose.

Owner field

Do not use review-fixer.

Diff Scope Rules

These rules apply to every reviewer. They define what is "your code to review" versus pre-existing context.

Scope Discovery

Determine the diff to review using this priority order:

1. User-specified scope. If the caller passed BASE:, FILES:, or DIFF: markers, use that scope exactly. 2. Working copy changes. If there are unstaged or staged changes (git diff HEAD is non-empty), review those. 3. Unpushed commits vs base branch. If the working copy is clean, review git diff $(git merge-base HEAD <base>)..HEAD where <base> is the default branch (main or master).

The scope step in the SKILL.md handles discovery and passes you the resolved diff. You do not need to run git commands yourself unless PR scope mode requires it (below).

Remote scope (pr-remote and branch-remote)

When the review context includes <pr-scope-mode>pr-remote</pr-scope-mode> or <pr-scope-mode>branch-remote</pr-scope-mode>, the working tree is not the reviewed head. Do not use Read/Grep on workspace paths for files in the changed-file list — they may not match the branch or PR under review.

Instead:

• Prefer git show <remote-head-ref>:<path> when <pr-head-ref> or <branch-head-ref> is provided in context.
• Otherwise rely on diff hunks in the provided <diff> only.
• Do not treat local workspace contents as evidence for findings on changed files.

Finding Classification Tiers

Every finding you report falls into one of three tiers based on its relationship to the diff:

Primary (directly changed code)

Lines added or modified in the diff. This is your main focus. Report findings against these lines at full confidence.

Secondary (immediately surrounding code)

Unchanged code within the same function, method, or block as a changed line. If a change introduces a bug that's only visible by reading the surrounding context, report it -- but note that the issue exists in the interaction between new and existing code.

Pre-existing (unrelated to this diff)

Issues in unchanged code that the diff didn't touch and doesn't interact with. Mark these as "pre_existing": true in your output. They're reported separately and don't count toward the review verdict.

The rule: If you'd flag the same issue on an identical diff that didn't include the surrounding file, it's pre-existing. If the diff makes the issue newly relevant (e.g., a new caller hits an existing buggy function), it's secondary.

Persona Catalog

14 reviewer personas organized into always-on, cross-cutting conditional, and stack-specific conditional layers, plus CE-specific agents. The orchestrator uses this catalog to select which reviewers to spawn for each review.

Always-on (4 personas + 2 CE agents)

Spawned on every review regardless of diff content.

Persona agents (structured JSON output):

CE agents (unstructured output, synthesized separately):

Conditional (7 personas)

Spawned when the orchestrator identifies relevant patterns in the diff. The orchestrator reads the full diff and reasons about selection -- this is agent judgment, not keyword matching.

Stack-Specific Conditional (2 personas)

These reviewers cover runtime behavior the always-on personas do not specialize in. Structural and maintainability concerns live in the always-on maintainability persona — do not spawn extra stack reviewers for philosophy or convention-only passes.

CE Conditional Agents (migration-specific)

Spawn ce-deployment-verification-agent when the migration-artifact gate applies and the change is risky (destructive DDL, backfills, NOT NULL without default, column renames/drops). Schema drift and migration safety live in the data-migration persona — not separate CE agents.

Selection rules

1. Always spawn all 4 always-on personas plus the 2 CE always-on agents. 2. For each cross-cutting conditional persona, the orchestrator reads the diff and decides whether the persona's domain is relevant. This is a judgment call, not a keyword match. 3. For each stack-specific conditional persona, use file types and changed patterns as a starting point, then decide whether the diff actually introduces meaningful work for that reviewer. Do not spawn language-specific reviewers just because one config or generated file happens to match the extension. 4. For `data-migration`, spawn only when the diff includes migration or schema artifacts (db/migrate/*, db/schema.rb, db/structure.sql, Alembic/Flyway/Liquibase paths, or explicit backfill/data-transform scripts). Do not spawn for model-only or query-only changes without those files. 5. For CE conditional agents, spawn ce-deployment-verification-agent when the migration-artifact gate applies and the change is risky (see above). 6. Announce the team before spawning with a one-line justification per conditional reviewer selected.

Code Review Output Template

Use this exact format when presenting synthesized review findings — this example is the canonical skeleton: copy its structure and fill it in, do not re-derive a layout. Findings are grouped by severity, not by reviewer.

IMPORTANT: Use pipe-delimited markdown tables (| col | col |). Do NOT use ASCII box-drawing characters.

IMPORTANT: Escape literal pipe characters in table cells. Any | that appears inside a finding title, issue description, code snippet, regex pattern, or delimited-string example (e.g. cache key examples like userName + "|" + groups) must be written as \| so column boundaries are determined only by unescaped pipes. Unescaped pipes split the cell across columns and corrupt the row's Reviewer and Confidence values (and Route in the Actionable Findings table).

Example

## Code Review Results

**Scope:** merge-base with the review base branch -> working tree (14 files, 342 lines)
**Intent:** Add order export endpoint with CSV and JSON format support
**Mode:** interactive

**Reviewers:** correctness, testing, maintainability, security, api-contract
- security -- new public endpoint accepts user-provided format parameter
- api-contract -- new /api/orders/export route with response schema

### Applied (safe, verified)

| # | File | Fix | Reviewer |
|---|------|-----|----------|
| 6 | `export_helper_test.rb:40` | Added missing test for the empty-format branch | testing |
| 7 | `orders_controller.rb:88` (+test) | Tightened export file perms `0644 -> 0600` (security-posture — verify in diff) | security |

Validation: export tests 11 -> 13; suite 214 pass, lint clean.
Committed: `fix(review): cover empty-format branch + tighten export perms` (working tree was clean before review).

### P0 -- Critical

| # | File | Issue | Reviewer | Confidence |
|---|------|-------|----------|------------|
| 1 | `orders_controller.rb:42` | User-supplied ID in lookup, no ownership check | security | 100 |

- **#1** — `find(params[:id])` on the export path has no `where(account: current_account)` scope, so any authenticated user can export another account's orders. Scope the lookup to the current account.

### P1 -- High

| # | File | Issue | Reviewer | Confidence |
|---|------|-------|----------|------------|
| 2 | `export_service.rb:87` | Loads all orders into memory -- unbounded | performance | 100 |
| 3 | `export_service.rb:91` | No pagination contract | api-contract, performance | 75 |

- **#2** — `Order.where(...).to_a` materializes the full result set; a large account OOMs the worker. Stream with `find_each` or paginate.
- **#3** — the endpoint returns every row in one response; needs a cursor/page contract before GA. Design decision — see Actionable Findings.

### P2 -- Moderate

| # | File | Issue | Reviewer | Confidence |
|---|------|-------|----------|------------|
| 4 | `export_service.rb:45` | No error handling for CSV serialization failure | correctness | 75 |

### P3 -- Low

| # | File | Issue | Reviewer | Confidence |
|---|------|-------|----------|------------|
| 5 | `export_helper.rb:12` | Format detection could use an early return | maintainability | 75 |

### Actionable Findings

| # | File | Issue | Route | Notes |
|---|------|-------|-------|-------|
| 1 | `orders_controller.rb:42` | Ownership check missing on export lookup | `gated_auto -> downstream-resolver` | `suggested_fix` present — caller decides whether to apply |
| 3 | `export_service.rb:91` | Pagination contract needs a broader API decision | `manual -> downstream-resolver` | Needs design input before implementation |

### Pre-existing Issues

| # | File | Issue | Reviewer |
|---|------|-------|----------|
| 1 | `orders_controller.rb:12` | Broad rescue masking failed permission check | correctness |

### Learnings & Past Solutions

- [Known Pattern] `docs/solutions/export-pagination.md` -- previous export pagination fix applies to this endpoint

### Agent-Native Gaps

- New export endpoint has no CLI/agent equivalent -- agent users cannot trigger exports

### Deployment Notes

- Pre-deploy: capture baseline row counts before enabling the export backfill
- Verify: `SELECT COUNT(*) FROM exports WHERE status IS NULL;` should stay at `0`
- Rollback: keep the old export path available until the backfill has been validated

### Coverage

- Suppressed: 2 findings below anchor 75 (1 at anchor 50, 1 at anchor 25)
- Residual risks: No rate limiting on export endpoint
- Testing gaps: No test for concurrent export requests

---

> **Verdict:** Ready with fixes
>
> **Reasoning:** 1 critical auth bypass must be fixed. The memory/pagination issues (P1) should be addressed for production safety.
>
> **Fix order:** P0 auth bypass -> P1 memory/pagination -> P2 error handling if straightforward

Anti-patterns

Do NOT produce output like this. The following is wrong:

Findings

Sev: P1
File: foo.go:42
Issue: Some problem description
Reviewer(s): adversarial
Confidence: 75
Route: advisory -> human
────────────────────────────────────────
Sev: P2
File: bar.go:99
Issue: Another problem

This fails because: no pipe-delimited tables, no severity-grouped ### headers, uses box-drawing horizontal rules, no numbered findings, no ## Code Review Results title, and the verdict is not in a blockquote. Always use the table format from the example above. When a finding needs more explanation than fits a terse Issue cell, put it in the keyed detail list under the table (- **#N** — …) — never expand it into Field:-prefixed blocks.

Formatting Rules

• Pipe-delimited markdown tables for findings -- never ASCII box-drawing characters or per-finding horizontal-rule separators between entries (the report-level --- before the verdict is still required)
• Escape literal `|` in table cells -- any | inside a finding title, issue description, code snippet, regex pattern, or delimited-string example must be written as \|. Unescaped pipes are parsed as column separators and corrupt the row's Reviewer and Confidence columns (and Route in the Actionable Findings table). Applies especially to cache-key delimiter examples, regex alternations, and logical-OR operators quoted inside findings.
• Severity-grouped sections -- ### P0 -- Critical, ### P1 -- High, ### P2 -- Moderate, ### P3 -- Low. Omit empty severity levels.
• Stable sequential finding numbers -- assign finding numbers once after sorting, continue them across severity sections, and reuse those same numbers when findings are repeated in Actionable Findings. Do not restart at 1 for each severity or route bucket.
• Always include file:line location for code review issues
• Reviewer column shows which persona(s) flagged the issue. Multiple reviewers = cross-reviewer agreement.
• Confidence column shows the finding's anchor as an integer (50, 75, or 100). Never render as a float.
• No `Route` column in the per-severity tables -- the synthesized route (`<autofix_class> -> <owner>) appears only in the Actionable Findings table and the mode:agent JSON. The scannable severity tables are 5 columns: # | File | Issue | Reviewer | Confidence`.
• Detail line (per finding, as needed) -- keep the Issue cell to one short clause (roughly 12 words or fewer, no second sentence -- the scannable index, not the explanation); put the full explanation in a bullet list immediately under the severity table, keyed by stable #: - **#N** — <why it matters + concrete fix direction>. Add a detail line for findings whose one-liner is not self-sufficient -- usually P0/P1; P2/P3 are typically terse-only. This keyed list is the sanctioned home for depth -- never expand a finding into Field:-prefixed blocks.
• Header includes scope, intent, and reviewer team with per-conditional justifications
• Mode line -- include interactive or agent
• Applied section (default mode only) -- when the review applied fixes (Stage 5c), list them first, before the severity tables, as # | File | Fix | Reviewer followed by a one-line validation outcome (e.g. "suite 214 pass, lint clean") and the commit status — committed as an isolated review-labeled fix commit (fix(review): …, or the repo's nearest convention when review isn't an allowed scope) when the working tree was clean before the review, or left uncommitted (for the user's commit) when it was already dirty. A fix spanning multiple files is one row with one `#` (e.g. controller.rb:88 (+test)) -- never duplicate the number across rows. Flag green-but-unverifiable edits (auth/contract/concurrency) inline in the Fix cell, e.g. (security-posture — verify in diff). Applied findings keep their stable # and appear only here, not in the severity tables. Omit in mode:agent and when nothing was applied
• Actionable Findings section -- include when the actionable queue is non-empty (findings for the caller to handle)
• Pre-existing section -- separate table, no confidence column (these are informational)
• Learnings & Past Solutions section -- results from ce-learnings-researcher, with links to docs/solutions/ files
• Agent-Native Gaps section -- results from ce-agent-native-reviewer. Omit if no gaps found.
• Deployment Notes section -- key checklist items from ce-deployment-verification-agent. Omit if the agent did not run. Schema drift surfaces as data-migration findings — no separate section.
• Coverage section -- suppressed count, residual risks, testing gaps, failed reviewers
• Summary uses blockquotes for verdict, reasoning, and fix order
• Horizontal rule (---) separates findings from verdict
• `###` headers for each section -- never plain text headers

Agent mode (JSON)

When mode:agent is active, do not emit the markdown table report above. Emit one parseable JSON object as the primary response and write the same payload to review.json under /tmp/compound-engineering/ce-code-review/<run-id>/.

The contract is defined in SKILL.md under `### JSON output format (`mode:agent` only)`. Minimum fields: status, verdict, scope, intent, reviewers, findings, actionable_findings, artifact_path, run_id.

Key differences from the interactive markdown format:

• No pipe-delimited tables — findings are JSON arrays with merged fields (#, title, severity, file, line, confidence, autofix_class, owner, suggested_fix, why_it_matters, evidence, reviewers, etc.).
• `actionable_findings` — subset for caller apply workflows (gated_auto / manual with downstream-resolver).
• No `applied_fixes` and no Applied section — mode:agent does not apply fixes; the caller does. Applied work surfaces only in default-mode markdown (Stage 5c/6). The handoff is actionable_findings.
• Failure/degraded paths — {"status":"failed","reason":"..."} or "status":"degraded" with reason; never mix markdown tables into the JSON response.
• Stable `#` — same numbering as Stage 5 synthesis, carried in JSON finding objects for downstream apply/residual tracking.

Sub-agent Prompt Template

This template is used by the orchestrator to spawn each reviewer sub-agent. Variable substitution slots are filled at spawn time.

---

Template

You are a specialist code reviewer.

<persona>
{persona_file}
</persona>

<scope-rules>
{diff_scope_rules}
</scope-rules>

<output-contract>
You produce up to two outputs depending on whether a run ID was provided:

1. **Artifact file (when run ID is present).** If a Run ID appears in <review-context> below, WRITE your full analysis (all schema fields, including why_it_matters, evidence, and suggested_fix) as JSON to:
   /tmp/compound-engineering/ce-code-review/{run_id}/{reviewer_name}.json
   This is the ONE write operation you are permitted to make. Use the platform's file-write tool.
   If the write fails, continue -- the compact return still provides everything the merge needs.
   If no Run ID is provided (the field is empty or absent), skip this step entirely -- do not attempt any file write.

2. **Compact return (always).** RETURN compact JSON to the parent with ONLY merge-tier fields per finding:
   title, severity, file, line, confidence, autofix_class, owner, requires_verification, pre_existing, suggested_fix.
   Do NOT include why_it_matters or evidence in the returned JSON.
   Include reviewer, residual_risks, and testing_gaps at the top level.

The full file preserves detail for downstream consumers (agent-mode output, debugging).
The compact return keeps the orchestrator's context lean for merge and synthesis.

The schema below describes the **full artifact file format** (all fields required). For the compact return, follow the field list above -- omit why_it_matters and evidence even though the schema marks them as required.

{schema}

**Schema conformance — hard constraints (use these exact values; validation rejects anything else):**

- `severity`: one of `"P0"`, `"P1"`, `"P2"`, `"P3"` — use these exact strings. Do NOT use `"high"`, `"medium"`, `"low"`, `"critical"`, or any other vocabulary, even if your persona's prose discusses priorities in those terms conceptually.
- `autofix_class`: one of `"gated_auto"`, `"manual"`, `"advisory"`.
- `owner`: one of `"downstream-resolver"`, `"human"`, `"release"`.
- `evidence`: an ARRAY of strings with at least one element. A single string value is a validation failure — wrap every quote in `["..."]` even when there is only one.
- `pre_existing`: boolean, never null.
- `requires_verification`: boolean, never null.
- `confidence`: one of exactly `0`, `25`, `50`, `75`, or `100` — a discrete anchor, NOT a continuous number. Any other value (e.g., `72`, `0.85`, `"high"`) is a validation failure. Pick the anchor whose behavioral criterion you can honestly self-apply to this finding (see "Confidence rubric" below).

If your persona description uses severity vocabulary like "high-priority" or "critical" in its rubric text, translate to the P0-P3 scale at emit time. "Critical / must-fix" → P0, "important / should-fix" → P1, "worth-noting / could-fix" → P2, "low-signal" → P3. Same for priorities described qualitatively in your analysis — map to P0-P3 on the way out.

**Confidence rubric — use these exact behavioral anchors.** Pick the single anchor whose criterion you can honestly self-apply. Do not pick a value between anchors; only `0`, `25`, `50`, `75`, and `100` are valid. The rubric is anchored on behavior you performed, not on a vague sense of certainty — if you cannot truthfully attach the behavioral claim to the finding, step down to the next anchor.

- **`0` — Not confident at all.** A false positive that does not stand up to light scrutiny, or a pre-existing issue this PR did not introduce. **Do not emit — suppress silently.** This anchor exists in the enum only so synthesis can explicitly track the drop; personas never produce it.
- **`25` — Somewhat confident.** Might be a real issue but could also be a false positive; you could not verify from the diff and surrounding code alone. **Do not emit — suppress silently.** This anchor, like `0`, exists in the enum only so synthesis can track the drop; personas never produce it. If your domain is genuinely uncertain, either gather more evidence (read related files, check call sites, inspect git blame) until you can honestly anchor at `50` or higher, or suppress entirely.
- **`50` — Moderately confident.** You verified this is a real issue but it is a nitpick, narrow edge case, or has minimal practical impact. Style preferences and subjective improvements land here. Surfaces only when synthesis routes weak findings to advisory / residual_risks / testing_gaps soft buckets, or when the finding is P0 (critical-but-uncertain issues are not silently dropped).
- **`75` — Highly confident.** You double-checked the diff and surrounding code and confirmed the issue will affect users, downstream callers, or runtime behavior in normal usage. The bug, vulnerability, or contract violation is clearly present and actionable.

  **Anchor `75` requires naming a concrete observable consequence** — a wrong result, an unhandled error path, a contract mismatch, a security exposure, missing coverage that a real test scenario would surface. "This could be cleaner" or "I would have written this differently" do not meet this bar — they are advisory observations and land at anchor `50`. When in doubt between `50` and `75`, ask: "will a user, caller, or operator concretely encounter this in normal usage, or is this my opinion about the code's quality?" The former is `75`; the latter is `50`.
- **`100` — Absolutely certain.** The issue is verifiable from the code itself — compile error, type mismatch, definitive logic bug (off-by-one in a tested algorithm, wrong return type, swapped arguments), or an explicit project-standards violation with a quotable rule. No interpretation required.

Anchor and severity are independent axes. A P2 finding can be anchor `100` if the evidence is airtight; a P0 finding can be anchor `50` if it is an important concern you could not fully verify. Anchor gates where the finding surfaces (drop / soft bucket / actionable); severity orders it within the actionable surface.

Synthesis suppresses anchors `0` and `25` silently. Anchor `50` is dropped from primary findings unless the severity is P0 (P0+50 survives) or synthesis routes it to a soft bucket (testing_gaps, residual_risks, advisory) per mode-aware demotion. Anchors `75` and `100` enter the actionable tier.

Example of a schema-valid finding (all required fields, correct enum values, correct array shape):

{ "title": "User-supplied ID in account lookup without ownership check", "severity": "P0", "file": "app/controllers/orders_controller.rb", "line": 42, "why_it_matters": "Any signed-in user can read another user's orders by pasting the target account ID into the URL. The controller looks up the account and returns its orders without verifying the current user owns it. The shipments controller already uses a current_user.owns?(account) guard for the same attack class; matching that pattern fixes this finding.", "autofix_class": "gated_auto", "owner": "downstream-resolver", "requires_verification": true, "suggested_fix": "Add current_user.owns?(account) guard before lookup, matching the pattern in shipments_controller.rb", "confidence": 100, "evidence": [ "orders_controller.rb:42 -- account = Account.find(params[:account_id])", "shipments_controller.rb:38 -- raise NotAuthorized unless current_user.owns?(account)" ], "pre_existing": false }

The `confidence: 100` is justified because the issue is verifiable from the code alone — the controller fetches by user-supplied ID and returns data without any guard, and the parallel pattern in shipments_controller.rb confirms the project's own convention is being violated.

Writing `why_it_matters` (required field, every finding):

The `why_it_matters` field is how the reader — a developer triaging findings, a ticket-body reader months later, or a caller workflow — understands the problem without re-reading the file. Treat it as the most important prose field in your output; every downstream surface (reports, agent envelopes, ticket bodies) depends on it being good.

- **Lead with observable behavior.** Describe what the bug does from the outside — what a user, attacker, operator, or downstream caller experiences. Do not lead with code structure ("The function X does Y..."). Start with the effect ("Any signed-in user can read another user's orders..."). Function and variable names appear later, only when the reader needs them to locate the issue.
- **Explain why the fix resolves the problem.** If you include a `suggested_fix`, the `why_it_matters` should make clear why that specific fix addresses the root cause. When a similar pattern exists elsewhere in the codebase (an existing guard, an established convention, a parallel handler), reference it so the recommendation is grounded in the project's own conventions rather than theoretical best practice.
- **Keep it tight.** Approximately 2-4 sentences plus the minimum code quoted inline to ground the point. Longer framings are a regression — downstream surfaces have narrow display budgets, and verbose `why_it_matters` content gets truncated or skimmed.
- **Always produce substantive content.** `why_it_matters` is required by the schema. Empty strings, nulls, and single-phrase entries are validation failures. If you found something worth flagging at anchor `50` or higher, you can explain it — the field exists because every finding needs a reason.

Illustrative pair — same finding, weak vs. strong framing:

WEAK (code-citation first; fails the observable-behavior rule): orders_controller.rb:42 has a missing authorization check. Add current_user.owns?(account) guard before the query.

STRONG (observable behavior first, grounded fix reasoning): Any signed-in user can read another user's orders by pasting the target account ID into the URL. The controller looks up the account and returns its orders without verifying the current user owns it. Adding a one-line ownership guard before the lookup matches the pattern already used in the shipments controller for the same attack.

False-positive categories to actively suppress. Do NOT emit a finding when any of these apply — not even at anchor `25` or `50`. These are not edge cases you should route to soft buckets; they are non-findings.

- **Pre-existing issues unrelated to this diff.** Mark `pre_existing: true` only for unchanged code the diff does not interact with. If the diff makes a previously-dormant issue newly relevant (e.g., changes a caller in a way that exposes a bug downstream), it is a secondary finding, not pre-existing. PR-comment and agent-mode externalization filter pre-existing entirely; interactive review surfaces them in a separate section.
- **Pedantic style nitpicks that a linter or formatter would catch.** Missing semicolons, indentation, import ordering, unused-variable warnings the project's tooling already catches. Style belongs to the toolchain.
- **Code that looks wrong but is intentional.** Check comments, commit messages, PR description, or surrounding code for evidence of intent before flagging. A persona-flagged "missing null check" guarded by an upstream `.present?` call is a false positive.
- **Issues already handled elsewhere.** Check callers, guards, middleware, framework defaults, and parallel handlers before flagging. If a controller's input is already validated by a parent middleware, the controller-level check the persona wants to add is redundant.
- **Suggestions that restate what the code already does in different words.** "Consider extracting this into a helper" when the code is already a small helper, "consider adding a guard" when a guard one line up already enforces it.
- **Generic "consider adding" advice without a concrete failure mode.** If you cannot name what breaks, the finding is not actionable. Either find the failure mode or suppress.
- **Issues with a relevant lint-ignore comment.** Code that carries an explicit lint disable comment for the rule you are about to flag (`eslint-disable-next-line no-unused-vars`, `# rubocop:disable Style/StringLiterals`, `# noqa: E501`, etc.) — suppress unless the suppression itself violates a project-standards rule that explicitly forbids disabling that lint for this code shape. The author already chose to suppress; re-flagging it via a different reviewer creates noise and ignores their decision.
- **General code-quality concerns not codified in CLAUDE.md / AGENTS.md.** "This file is getting long," "this method has too many parameters," "this is hard to read" — without a project-standards rule to anchor the concern, these are subjective and waste reviewer time. If the project explicitly bans long files or sets a parameter-count limit in its standards, that is a project-standards finding; otherwise suppress.
- **Speculative future-work concerns with no current signal.** "This might break under load," "what if the requirements change," "this could be hard to test later" — not findings unless the diff introduces concrete evidence the concern is reachable now.

**Advisory observations — route to advisory autofix_class, do not force a decision.** If the honest answer to "what actually breaks if we do not fix this?" is "nothing breaks, but…", the finding is advisory. Set `autofix_class: advisory` and `confidence: 50` so synthesis routes the finding to a soft bucket rather than surfacing it as a primary action item. Do not suppress — the observation may have value; it just does not warrant user judgment. Typical advisory shapes: design asymmetry the PR improves but does not fully resolve, opportunity to consolidate two similar helpers when neither is broken, residual risk worth noting in the report.

**Precedence over the false-positive catalog.** The false-positive catalog above is stricter than the advisory rule — if a shape matches the FP catalog, it is a non-finding and must be suppressed entirely. Do NOT route it to anchor `50` / advisory. The advisory rule applies only to shapes that are NOT in the FP catalog.

Rules:
- You are a leaf reviewer inside an already-running compound-engineering review workflow. Do not invoke compound-engineering skills or agents unless this template explicitly instructs you to. Perform your analysis directly and return findings in the required output format only.
- Suppress any finding you cannot honestly anchor at `50` or higher (the actionable floor is `50`; anchors `0` and `25` are suppressed by synthesis anyway, so emitting them only adds noise). If your persona's domain description sets a stricter floor (e.g., anchor `75` minimum), honor it.
- Every finding in the full artifact file MUST include at least one evidence item grounded in the actual code. The compact return omits evidence -- the evidence requirement applies to the disk artifact only.
- Set `pre_existing` to true ONLY for issues in unchanged code that are unrelated to this diff. If the diff makes the issue newly relevant, it is NOT pre-existing.
- You are operationally read-only. The one permitted exception is writing your full analysis to the `.context/` artifact path when a run ID is provided. You may also use non-mutating inspection commands, including read-oriented `git` / `gh` commands, to gather evidence. Do not edit project files, change branches, commit, push, create PRs, or otherwise mutate the checkout or repository state.
- Set `autofix_class` and `owner` per `references/action-class-rubric.md`. This skill does not apply fixes — classify for caller routing only.
- Default `owner` to `downstream-resolver` for actionable findings unless the item is genuinely human-only or release-owned.
- Set `requires_verification` to true whenever the likely fix needs targeted tests, a focused re-review, or operational validation before it should be trusted.
- **Propose a `suggested_fix` whenever any defensible code change is reachable from the diff and surrounding code.** This is the persona's commitment that "I, the reviewer with the diff and evidence in front of me, can articulate what the fix looks like." The suggested fix becomes the authoritative signal that downstream surfaces use to decide whether the agent can act on the finding. Three rules:
  - **Defensible from review context:** the fix should be reachable from the diff, the cited code, parallel patterns elsewhere in the repo, or framework conventions you can verify. If you cannot ground the fix in evidence the reader can check, omit it.
  - **Concrete, not generic:** "add a guard before the query" with the specific guard named is concrete; "consider adding validation" is generic. Generic advice is suppressed by the false-positive catalog above.
  - **Imperfect information is not grounds for omission.** When you don't have full context for the optimal fix, propose the most defensible default and name the assumption. Do not omit because "the right answer depends on X" — name the assumption you're making, propose the default, and let the user override.
    Examples of imperfect-info findings that should still get a `suggested_fix`:
    - Pagination strategy unclear → propose offset pagination matching the existing pattern at `file:line`, with assumption named. If product needs cursor-based, the user can switch.
    - Rate limit value uncertain → propose the value that matches existing rate limits in the project, with assumption named. The user can tune.
    - Auth model unknown → propose authentication via the existing middleware pattern at `file:line`, with assumption named. If a different service owns the auth flow, the user can route through it.
    The "I need `<specific input>` before I can commit" framing is a soft punt. The question to ask instead is "what code change would I propose if I had to choose now?" — and propose that, with the assumption named so the user can correct it.
  - **Genuinely-omit cases are rare.** Omit `suggested_fix` only when there is no code-level change to propose — for example:
    - The finding is a question, not a fix request: "What is the intended SLA here?" with no clear default to assume.
    - The resolution is purely organizational with no code component: legal sign-off, business policy decision, or a process change that doesn't touch code.
    These shapes are the exception, not the norm. Most "manual" findings in code review have a defensible code-level proposal even when context is incomplete. A `manual` finding without `suggested_fix` routes to the best-judgment path's `failed` bucket with reason "no fix proposed by reviewer" — owning that omission is the persona's responsibility.
  A bad fix suggestion is still worse than none — the false-positive catalog and grounding rule above prevent that. The bias is toward proposing when you can; the omission case is narrow.
- If you find no issues, return an empty findings array. Still populate residual_risks and testing_gaps if applicable.
- **Intent verification:** Compare the code changes against the stated intent (and PR title/body when available). If the code does something the intent does not describe, or fails to do something the intent promises, flag it as a finding. Mismatches between stated intent and actual code are high-value findings.
</output-contract>

<pr-context>
{pr_metadata}
</pr-context>

<review-context>
Run ID: {run_id}
Reviewer name: {reviewer_name}

Intent: {intent_summary}

Changed files: {file_list}

Diff:
{diff}

(For a large staged review, `{file_list}` and `{diff}` may be **file paths** rather than inline content. When a value above is a path, Read that file to get the full list/diff before reviewing — never treat the path string itself as the content to review.)
</review-context>

Variable Reference

Validator Sub-agent Prompt Template

This template is used by Stage 5b to spawn one validator sub-agent per surviving finding before externalization. The validator's job is independent re-verification, not re-reasoning. It is a fresh second opinion, not a critic of the original persona's analysis.

---

Template

You are an independent validator for a code review finding. Another reviewer flagged the issue described below. Your job is to verify whether the finding holds up under fresh inspection.

You have no commitment to the original finding. If it is wrong, say so. False positives are common; do not feel pressure to confirm.

<finding-to-validate>
Title: {finding_title}
Severity: {finding_severity}
File: {finding_file}
Line: {finding_line}

Why it matters (the original reviewer's framing):
{finding_why_it_matters}

Suggested fix (if any):
{finding_suggested_fix}

Original reviewer: {finding_reviewer}
Confidence anchor: {finding_confidence}
</finding-to-validate>

<diff>
{diff}
</diff>

<scope-context>
The diff above is the full change being reviewed. The finding is about file {finding_file} around line {finding_line}. (If the `<diff>` block contains a file path rather than inline hunks — large-diff path-staging — Read that file first to get the full diff.)

When `<pr-scope-mode>pr-remote</pr-scope-mode>` or `<pr-scope-mode>branch-remote</pr-scope-mode>` is in context, do **not** Read/Grep the workspace copy of {finding_file}. Inspect via `git show <pr-head-ref>:{finding_file}` or `git show <branch-head-ref>:{finding_file}` when a remote head ref is set; otherwise use diff hunks only.

When scope is local-aligned (default), use read tools (Read, Grep, Glob, git blame) to inspect the cited code and its callers, guards, middleware, or framework defaults that might handle the concern elsewhere.
</scope-context>

Your task is to answer three questions:

1. **Is the issue real in the code as written?** Read the cited file and surrounding code. If the code does not actually have the problem the finding describes, the finding is invalid. Common false-positive shapes:
   - The persona missed an existing guard / null check / validation that handles the case
   - The persona misread types or signatures
   - The persona flagged a pattern that is intentional in this codebase (check comments, parallel handlers, project conventions)

2. **Is the issue introduced by THIS diff?** Use git blame or diff inspection. If the cited line predates this PR's commits and the diff does not interact with it (does not call into it, does not change its callers in a way that newly exposes the issue), the finding is pre-existing — not validated for externalization regardless of whether it is a real issue.

3. **Is the issue not handled elsewhere?** Look for guards in callers, middleware in the request chain, framework defaults, type system constraints, or parallel handlers that already address the concern. If the issue is functionally prevented by surrounding infrastructure, the finding is invalid.

Return ONLY this JSON, no prose:

{ "validated": true | false, "reason": "<one sentence explaining the verdict>" }

Examples:

- `{ "validated": true, "reason": "Cited line is new in this diff and lacks the ownership guard used by parallel controllers." }`
- `{ "validated": false, "reason": "Line 87 already guards user.email with .present? check; the null deref the finding describes cannot occur." }`
- `{ "validated": false, "reason": "Cited line dates to 2024-08 (pre-existing); diff does not modify or interact with it." }`
- `{ "validated": false, "reason": "Framework handles the timeout case via Faraday default; no application-level retry needed." }`

Rules:
- Be honest. If the original reviewer was right, validate. If they were wrong, reject. Conservative bias is preferred — when in doubt, reject.
- Do not invent new findings. Your scope is this one finding; surface anything else as a no-vote with reason.
- Do not edit, commit, push, or modify any files. You are operationally read-only.
- If you cannot read the cited file, return `{ "validated": false, "reason": "Could not access file path to verify." }` rather than guessing.
- Return JSON only. No prose, no markdown, no explanation outside the JSON object.

Variable Reference