ce-code-review

name	ce-code-review
description	Structured code review for bugs, regressions, tests, and standards. Use before PRs or when asked for review; interactive mode can fix locally, while mode:agent reports only for pipeline callers.
argument-hint	[mode:agent] [blank to review current branch, or provide PR link]

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.

Token	Example	Effect
mode:agent	mode:agent	Report-only: return JSON instead of markdown tables and skip the Stage 5c apply (the caller applies). Does not change reviewer selection, merge logic, or scope rules (see Output format)
mode:headless	mode:headless	Deprecated alias for mode:agent
mode:report-only	mode:report-only	Deprecated — ignored. Former no-artifacts mode; default behavior is review-only without checkout
base:<sha-or-ref>	base:abc1234 or base:origin/main	Diff base on the current checkout (explicit; skips auto base detection)
plan:<path>	plan:docs/plans/2026-03-25-001-feat-foo-plan.md	Plan file for requirements verification (explicit). Supports markdown and HTML unified plans.
depth:full	depth:full	Force the full reviewer roster — skip the Stage 3c small-diff lite path so every always-on persona runs regardless of diff size. Use when a deep/thorough review is explicitly requested (the one escalation signal Stage 3c cannot infer from the diff). Does not change conditional selection, merge, or scope.
depth:auto	depth:auto	Default — self-right-size via Stage 3c (lite roster for trivial, low-risk, code-only diffs; full roster otherwise).
grouping:auto	grouping:auto	Default — build thematic triage groups when findings span distinct concerns (Stage 5 step 9b)
grouping:off	grouping:off	Suppress triage groups: no Triage Groups section, empty triage_groups in JSON
grouping:always	grouping:always	Always build triage groups, even for small reviews

Grouping is presentation, not a mode. The grouping: tokens change how the finding set is organized for triage — never reviewer selection, merge logic, scope rules, or the Stage 5c apply decision.

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
• Multiple distinct grouping: tokens (e.g. grouping:off and grouping:always)

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.
• Report outcomes, not machinery. What you show the user is about the review: what's being examined (the PR/branch), which coverage is included and the one-line reason for each conditional lens, the independent cross-model pass and which model runs it, and the findings. Keep the skill's internals out of user-facing text — model-tier assignments, raw scope-mode codenames (local-aligned/pr-remote), staging the diff to disk, loading persona files, parallel-dispatch bookkeeping, and step-by-step narration of your own setup. Name what the user would recognize (a PR number, a reviewer's concern, a peer model), not the plumbing. This governs what you surface and suppress; it does not script the wording — use your own voice.

Output format

Invocation	Deliverable
Default	Markdown report (pipe-delimited finding tables) + Actionable Findings summary
mode:agent	One JSON object (see ### JSON output format below) + the same /tmp/.../ce-code-review/<run-id>/ artifacts

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:

Level	Meaning	Action
P0	Critical breakage, exploitable vulnerability, data loss/corruption	Must fix before merge
P1	High-impact defect likely hit in normal usage, breaking contract	Should fix
P2	Moderate issue with meaningful downside (edge case, perf regression, maintainability trap)	Fix if straightforward
P3	Low-impact, narrow scope, minor improvement	User's discretion

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.

autofix_class	Default owner	Meaning
gated_auto	downstream-resolver or human	Concrete suggested_fix proposed; caller applies after judgment
manual	downstream-resolver or human	Actionable work needing design input or handoff
advisory	human or release	Report-only — learnings, rollout notes, residual risk

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 local prompt assets. Quick roster with one-line triggers below; the persona catalog included at the bottom has the full per-persona selection criteria and spawn gates. Each selected reviewer is a generic subagent seeded with a local prompt file from references/personas/; do not dispatch standalone agents by type/name.

Always-on (full review): local prompt assets correctness-reviewer, testing-reviewer, maintainability-reviewer, project-standards-reviewer, plus CE local prompt assets agent-native-reviewer and learnings-researcher. (Stage 3c may reduce this set to a lite roster for trivial, low-risk diffs.)

Cross-cutting conditional (per diff):

• security-reviewer — auth, public endpoints, user input, permissions
• performance-reviewer — DB queries, data transforms, caching, async
• api-contract-reviewer — routes, serializers, type signatures, versioning
• data-migration-reviewer — migration files / schema dumps / backfills (see spawn gate in Stage 3)
• reliability-reviewer — error handling, retries, timeouts, background jobs
• adversarial-reviewer — >=50 changed code lines, or auth / payments / data mutations / external APIs. When selected, a cross-model adversarial pass (Stage 4) additionally runs the same brief through a different model family via a peer CLI — additive, non-blocking
• previous-comments-reviewer — PR with existing review comments (PR-only, comment-gated)

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

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

Review Scope

A full review spawns generic subagents for all 4 always-on personas plus the 2 CE always-on local prompt assets, then adds whichever cross-cutting and stack-specific conditionals fit the diff (Stage 3c can collapse this to a lite roster for trivial, low-risk diffs). 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/* -- legacy requirements documents created by older ce-brainstorm versions
• docs/plans/*.{md,html} -- unified plan artifacts created by ce-brainstorm or 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.

Plan Requirements Completeness

When a plan is provided via plan:<path> or discovered from PR/branch context, classify readiness before checking completeness:

• Unified artifact: metadata includes artifact_contract: ce-unified-plan/v1.
  • artifact_readiness: requirements-only can inform product intent, but it must not trigger implementation-unit completeness findings. Report that the artifact was not implementation-ready if the diff appears to implement it.
  • artifact_readiness: implementation-ready is eligible for full requirements and U-ID completeness checks.
  • Invalid progress-like readiness values (active, in_progress, completed, done) are contract errors.
• Legacy plan: use the existing completeness checks.

Extract requirements from these shapes, in order:

1. Unified Product Contract -> ### Requirements
2. Legacy top-level ## Requirements
3. Legacy ## Requirements Trace

For unified implementation-ready plans, also extract U-IDs from ## Implementation Units and compare against PR body/branch context when available. Do not require every Product Contract R-ID to map one-to-one to a single U-ID; verify that implemented U-IDs cite the relevant R/F/AE/KTD IDs and that no claimed U-ID is missing from the plan.

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 on the platform's cheapest capable model when a known override exists; otherwise omit the model override and inherit. Give it 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.