Caller contract: when the invoking task already defines scope, base SHA, or an output contract (subagent protocols, orchestrated reviews), skip Scope Resolution, Review Mode Selection, and Output Format — apply only the review discipline (two-stage check, severity, evidence rules, anti-patterns) within that contract.
Two-Stage Review
Stage 1 -- Spec compliance (do this FIRST): verify the changes implement what was intended — check the PR description, issue, or task spec for missing requirements, unnecessary additions, interpretation gaps. If the implementation is wrong, stop here -- reviewing quality on the wrong feature wastes effort.
Stage 2 -- Code quality: only after Stage 1 passes, review for correctness, maintainability, security, and performance.
Scope Resolution
Pre-flight: verify git rev-parse --git-dir exists before anything else. If not in a git repo, ask for explicit file paths — ask via AskUserQuestion (Claude Code; load with ToolSearch select:AskUserQuestion if not loaded) or request_user_input (Codex); fall back to numbered options in chat. Later asks reuse this channel.
When no specific files are given, resolve scope via this fallback chain:
- User-specified files/directories (explicit request)
- Session-modified files (
git diff --name-only, unstaged + staged) - All uncommitted files (
git diff --name-only HEAD) - Untracked files (
git ls-files --others --exclude-standard) -- often the most review-worthy - Zero files → stop. Ask what to review (ask channel above).
Exclude: lockfiles, minified/bundled output, vendored/generated code.
Base-branch resolution for branch reviews
When the review target is a branch (not a working-tree diff), the comparison range is the merge-base, not the working-tree delta — resolve it before reading any diff. Fallback chain (PR base → default-branch inference → origin/* → git merge-base → unshallow retry), stacked-branch detail, and the "never fall back to git diff HEAD" rule in scope-resolution.md. Stacked branches: prefer the platform's base_sha (gh pr diff) — a local merge-base over-covers.
Off-scope filter (always, after any branch review): intersect finding paths with the change's --name-only set; discard non-intersecting findings.
Review Mode Selection
Run this BEFORE reading the full diff. Use metadata only (git diff --stat, file list from scope resolution) — reading the diff first creates analysis momentum that bypasses mode selection.
Exceptions first — these change types stay single-pass regardless of signal count: pure documentation/markdown changes; mechanical refactors (renames, moves) with no logic changes; single-file changes under 50 lines.
Verification-mechanism carve-out: even when a change stays single-pass by the exceptions above, if it is a verification mechanism (CI/CD gate, merge-block check, coverage/lint gate, build/deploy step, or test infra/mock that could mask a real failure), apply the "can this silently false-pass?" lens during the single-pass review — the mechanism can go green while the thing it guards is red. In deep review this same lens runs as a size-independent red-team trigger (see deep-review.md).
| Signal | Threshold |
|---|---|
| Lines changed (excluding test files) | >300 |
| Files touched (excluding test files) | >8 |
| Top-level directories spanned (non-test) | >3 |
| Security-sensitive paths (auth, crypto, payments, permissions) | any |
| Database migrations | any |
| API surface changes (public endpoints, exported interfaces) | any |
Test file exclusion: filter test paths out of the size signals with git diff --stat -- ':!tests/' ':!*.test.*' ':!*.spec.*' ':!*_test.*' and report both totals: "450 lines changed (280 excluding tests)."
3+ signals → deep review. Inform the user, then dispatch parallel specialist agents per deep-review.md. Pass the diff to agents -- do NOT read it first. Stop here -- skip the Review Process section.
2 signals → suggest (ask channel above): "This touches N files across M modules. Deep review?"
0-1 signals → standard review. Proceed to Review Process below.
Override: deep forces multi-agent, quick forces single-pass.
Review Process
Standard reviews only -- deep review is handled by the dispatched specialists.
- Context — before reading code:
- Scope drift: compare
git diff --statagainst the PR's stated intent. Classify CLEAN / DRIFT DETECTED / REQUIREMENTS MISSING; on drift, ask the author: ship as-is, split, or remove? - Intent: read the PR description, linked issue, or task spec. Deviation or under-delivery is a finding — the wrong problem solved correctly is still wrong.
- Prior discussions: reconcile existing review comments so resolved issues aren't re-raised. Gate on a presence check; commands in scope-resolution.md.
- Automated gates: run the project's test/lint suite (canonical commands in CI config).
- Scope drift: compare
- Structural scan -- architecture, file organization, API surface; flag breaking changes. Added (
A) files on a remote branch: use the diff content, not the working tree. - Line-by-line -- correctness, edge cases, error handling, naming, readability. Category checklists (correctness, maintainability, performance, adversarial, AI-code lens) in check-categories.md; load the profile matching the diff's file extensions from language-profiles.md (TS/React, Python, PHP, Shell/CI, Config, Data, Security, LLM Trust). Prefer questions ("What happens if
inputis empty?") over declarations. - Security -- input validation, auth checks, secrets exposure, injection vectors (SQL, XSS, CSRF, SSRF, command, path traversal, unsafe deserialization), race conditions (TOCTOU). Grep-able patterns for 11 vulnerability classes in security-patterns.md.
- Test coverage -- untested new paths, error paths, and behavioral changes without test updates. Flag implementation-coupled tests (mocked internals, private methods) -- test behavior, not wiring.
- Reliability -- error handling completeness, timeout/retry, resource cleanup on error paths, graceful degradation. Patterns in reliability-patterns.md.
- Removal candidates -- dead code, unused imports, cleanup-ready feature flags; safe-to-delete (no references) vs defer-with-plan.
- Verify -- run formatter/lint/tests on touched files; state what was skipped and why. Note doc staleness (README/ARCHITECTURE/CONTRIBUTING) as informational.
- Summary -- findings grouped by severity with verdict: Ready to merge / Ready with fixes / Not ready.
Large diffs: >500 lines → review by module, not file-by-file. Flag oversized PRs (ideal ~100-300 meaningful lines) and suggest a split — thresholds and the four split strategies in pr-sizing.md.
Severity and Confidence
Four severity tiers (Critical / Important / Medium / Minor) order the report; a confidence score (0.0-1.0) per finding decides what lands in it:
Confidence bands: ≥0.70 report · 0.60-0.69 report-if-actionable · <0.60 suppress (exception: Critical security findings report at ≥0.50).
Full 5-band rubric, evidence-before-severity ordering, false-positive suppression categories, and the LLM prompt-injection exception in severity-and-confidence.md.
Evidence lives in the CR-XXX entry itself — [file:line] plus `quoted code`, not only in surrounding prose. Never fabricate references.
Action Routing
Classify every finding's fix into one of four tiers:
safe_auto-- deterministic, local, behavior-preserving fix: apply directlygated_auto-- concrete fix crossing a behavior/contract/permission/API boundary: present it, wait for sign-offmanual-- author judgment, rewrite, or redesign needed: flag with fix intent, don't applyadvisory-- report-only risk signal: record under Residual Risks
Full decision rules and conflict resolution in action-routing.md. When in doubt, escalate to gated_auto — never promote toward safe_auto on disagreement.
Comment Labels
Prefix inline comments so authors know what requires action: (no prefix) = required change (Critical/Important), blocks merge; Nit: = style preference, optional; Consider: = suggestion worth evaluating, not blocking; FYI: = informational, no action expected.
Anti-Patterns in Reviews
- Nitpicking style when linters exist -- defer to automated tools
- "While you're at it..." scope creep -- open a separate issue
- Blocking on personal preference -- approve with a Minor comment
- Skipping Stage 1 -- never review code quality before verifying spec compliance; rubber-stamping without reading is not a review
- Recommending fix patterns without checking currency -- verify the pattern is current for the project's framework version; prefer newer built-in alternatives
- Fighting documented overrides -- a rationale-backed bypass (
CLAUDE.md,AGENTS.md, inline comment) is owner-blessed: honor it, don't re-raise; if the rationale is missing, suggest documenting one. Plan-mandated defects are not self-justifying — report them labeled "plan-mandated" for the human to adjudicate - Resting a finding on an unverified absence -- read the region or grep the exact symbol expecting zero lines; a subagent's confident negative or a broad-pattern hit is not proof
- Calling a change a regression without a baseline read -- read the pre-change file (
git show <base>:<file>), not just the hunk; cite the introducing commit when confirmed - Widening/narrowing a key or guard without checking the mirror bug -- name one concrete opposite-defect case along the now-ignored axis before accepting the fix
- Checking only one projection on a hide/filter/redact change -- enumerate every field surfacing the same entity (list,
*_count/*_ids, raw documents, detail view); require a test per field - Pre-classifying own findings as weak -- no "INFO only" / "no action required" wording; anchor severity in concrete constants from the code, not hypotheticals
Extended rationale for the last six traps — and the broader trap catalog — in review-traps-catalog.md.
When to Stop and Ask
- Fixing the issues would require an API redesign beyond the PR's scope
- Intent behind a change is ambiguous -- ask rather than assume
- Missing validation tooling (no linter, no tests) -- flag the gap, don't guess
Output Format
## Review: [brief title]
### Critical
- **CR-001.** [file:line] `quoted code` -- [issue]. Score: [0.0-1.0]. [Impact if not fixed]. Fix: [concrete suggestion].
### Important / ### Medium
- (same shape; Important adds Consider: [alternative approach])
### Minor
- **CR-004.** [file:line] -- [observation].
### What's Working Well
- [specific positive observation with why it's good]
### Residual Risks
- [unresolved assumptions, areas not covered, open questions]
### Verdict
Ready to merge / Ready with fixes / Not ready -- [one-sentence rationale]
Number findings CR-001, CR-002... sequentially across severities for stable IDs. Cap 10 per severity; note any overflow and show the highest-impact ones.
Markdown safety: in table cells, escape literal | as \| — code excerpts with pipes (a | b, string | null) split rows silently. Bullet output is pipe-safe.
Multi-agent consolidation: apply the merge algorithm in deep-review.md (same-line dedupe, severity conflicts, NEEDS DECISION, cross-lens confidence boosts).
Clean review (no findings): a valid outcome, not insufficient effort — say so explicitly and summarize what was checked.
References
References load at their point of use above. Additionally: security-test-coverage.md — security-audit deliverable checklist; false-positive-suppression.md — framework-idiom and test-specific FP categories; external-review-subprocess.md — external-CLI reviewer protocol (heartbeat tolerance, run-until-clean, frozen-diff binding).
Integration
ia-receiving-code-review-- inbound side. Tier map:safe_auto≈ AUTO-FIX,gated_auto≈ ESCALATE-for-approval,manual≈ ESCALATE,advisory≈ FYIia-kieran-revieweragent -- persona-driven Python/TypeScript deep quality review/ia-review-- full ceremony (worktrees, ultra-thinking); deep review here is lighter: parallel specialists, no worktrees/resolve-pr-parallelcommand -- batch-resolve PR comments with parallel agentsia-security-sentinelagent -- deep security audit; threat-model mode for new trust boundaries