Code Review

intent

perform structured code reviews that catch bugs, security gaps, and maintainability issues before merge. use this skill when reviewing a pull request, merge request, diff, or codebase section. the skill runs a two-stage process (spec compliance first, then code quality), selects review mode based on change complexity, and surfaces findings ranked by severity with actionable guidance. stops early if the implementation doesn't match the stated intent rather than wasting effort on code-quality feedback for the wrong feature.

inputs

git repository context (required)

• working directory must be inside a git repository. the skill runs git rev-parse --git-dir to verify this upfront; if not in a repo, you must provide explicit file paths instead.
• git access to the target branch, base branch, and merge-base resolution. for branch reviews (not working-tree diffs), the hosting platform's authoritative base SHA (PR base_sha, MR base, or gh pr diff output) is preferred over locally computed merge-base.

code to review (required, sourced via scope resolution)

• explicit file/directory paths (user-specified), or auto-detected via fallback chain: unstaged/staged changes (git diff --name-only), all uncommitted files (git diff HEAD), untracked files (git ls-files --others).
• if no files resolve, the skill stops and asks what to review.

change metadata (required for mode selection, do NOT read the full diff before mode selection)

• PR/MR description, linked issue, or task spec (consumed in the "read the intent" step).
• git diff --stat output (lines changed, excluding test files).
• file count and module/directory span (from scope resolution).
• file paths matching security-sensitive patterns (auth, crypto, payments, permissions, migrations, API surface changes).

ci/testing infrastructure (optional but recommended)

• project's canonical test command (from CI config: GitHub Actions, GitLab CI, CircleCI, or similar). the skill runs this during the "run automated gates" step to catch test/lint failures before manual review.
• linter/formatter config (used in the "verify" step to flag formatting issues).

external review context (optional)

• prior PR comments or review discussion (fetched via gh pr view, gh api, or GitLab API if present; gated on a presence check to avoid empty work).

environment/auth (optional, for deep review mode)

• if mode selection triggers multi-agent deep review, specialist agents are dispatched per the deep-review.md reference. no additional auth beyond git is required for the standard review path.

procedure

step 1: pre-flight and scope resolution

1. run git rev-parse --git-dir to confirm we are in a git repository. if not found, ask the user for explicit file paths and skip to step 2b.
2. determine the review target (working-tree diff, branch, or explicit file list).
3. apply the scope fallback chain in order:
   • user-specified files/directories (explicit request).
   • unstaged + staged changes (git diff --name-only).
   • all uncommitted files (git diff --name-only HEAD).
   • untracked files (git ls-files --others --exclude-standard).
4. exclude lockfiles, minified/bundled output, and vendored/generated code from the scope.
5. if reviewing a branch (not a working-tree diff), resolve the merge-base using the hosting platform's authoritative base SHA (e.g., PR base_sha). fall back to git merge-base HEAD <default-branch> only if the platform SHA is unavailable. if the branch is stacked on another unmerged branch, fetch the base SHA from the hosting platform to avoid over-covering sibling-branch commits. after the review, intersect every finding's file path with the change's --name-only set and discard off-scope findings.
6. output of step 1: file list, scope exclusions applied, merge-base SHA (if branch review).

step 2: review mode selection

run this before reading the full diff. use metadata only (git diff --stat, file list) to count signals. reading the diff first creates analysis momentum that bypasses mode selection.

1. count signals from the file metadata:
   • lines changed: >300 (sum of insertions + deletions, excluding test files via git diff --stat -- ':!tests/' ':!*.test.*' ':!*.spec.*' ':!*_test.*'). report both totals: "450 lines changed (280 excluding tests)."
   • files touched: >8 (count from scope resolution, excluding test files).
   • modules/directories spanned: >3 (count unique top-level directories in non-test file list).
   • security-sensitive files present: any hit (auth, crypto, payments, permissions, migrations, public API changes).
2. check the exceptions list in deep-review.md , pure docs, mechanical refactors, and single-file <50-line changes override signal count.
3. allow manual override: deep forces multi-agent deep review, quick forces single-pass standard review.
4. decision logic:
   • 3+ signals (or override deep): inform the user ("this change triggers deep review mode"), dispatch parallel specialist agents per deep-review.md, pass the full diff to agents (do NOT read it first), and stop. do not proceed to step 3 or beyond.
   • 2 signals: suggest deep review ("this touches N files across M modules. deep review? (y/n)"). if user declines, proceed to step 3 (standard review).
   • 0-1 signals (or override quick): proceed to step 3 (standard review).
5. output of step 2: review mode (standard or deep), signal count summary.

step 3: context gathering (standard review path only)

skip this step if deep review was triggered in step 2.

1. scope drift check: compare git diff --stat against the PR's stated intent. classify the change as CLEAN / DRIFT DETECTED / REQUIREMENTS MISSING. if DRIFT is found, list the drifted files and ask the author: "should we ship as-is, split this, or remove the unrelated changes?"
2. read the intent: fetch the PR description, linked issue/task spec, and any commit messages. if the code implements something the intent doesn't describe, or fails to implement something the intent promises, flag this as a finding (correct code that solves the wrong problem is still wrong).
3. fetch existing discussions (presence-gated): before raising findings, check for prior PR comments or review discussion. if comments exist, fetch them via gh pr view <number>, gh api /repos/{owner}/{repo}/pulls/{number}/comments, or the GitLab API equivalent. reconcile prior resolved issues so you don't re-raise them. if no comments are present, skip this fetch.
4. run automated gates: execute the project's canonical test and lint commands (sourced from CI config). capture pass/fail status and error output. if tests fail, note which tests failed and why before proceeding to manual review.
5. output of step 3: intent summary, scope-drift classification, prior-discussion notes, test/lint status.

step 4: structural scan (standard review path only)

1. examine file organization, module boundaries, and API surface changes.
2. flag any breaking changes to public endpoints, exported interfaces, or contract changes.
3. for files marked as added (A) in the diff, use the diff content directly (do not attempt to read them from the working tree if reviewing a remote branch).
4. note any large file additions or removals.
5. output of step 4: structural findings (breaking changes, API surface changes, organization issues).

step 5: line-by-line review (standard review path only)

1. iterate through each file in the scope. review for correctness, edge cases, error handling, naming, and readability.
2. use question-based feedback ("what happens if input is empty here?") instead of declarative statements to encourage author thinking.
3. tie every finding to concrete code evidence: file path, line number, quoted code snippet. never fabricate references.
4. note patterns that may indicate AI-generated code (repetitive structure, generic naming, missing edge cases) and apply the AI-code lens from check-categories.md.
5. consult language-profiles.md for language-specific checks matching the file extensions in the diff (TypeScript/React, Python, PHP, Shell/CI, Configuration, Data Formats, LLM Trust Boundaries).
6. output of step 5: line-level findings grouped by category (correctness, maintainability, performance, naming, readability).

step 6: security review (standard review path only)

1. review for input validation, auth checks, secrets exposure, and injection vectors (SQL, XSS, CSRF, SSRF, command, path traversal, unsafe deserialization).
2. flag race conditions (TOCTOU, check-then-act patterns).
3. consult security-patterns.md for grep-able detection patterns across 11 vulnerability classes.
4. check for hidden-entity leaks on filter/redact/hide changes: enumerate every field in the response that surfaces the same entity (structured list AND raw documents, array AND *_count/*_ids/total, summary AND detail projections). require test assertions that the hidden entity is absent from each surfacing field.
5. output of step 6: security findings (auth, validation, secrets, injection, race conditions).

step 7: test coverage review (standard review path only)

1. verify new code paths have corresponding tests.
2. flag untested error paths, edge cases, and behavioral changes without test updates.
3. flag tests coupled to implementation details (mocking internals, testing private methods) and suggest testing behavior instead.
4. output of step 7: test-coverage findings (missing tests, implementation-coupled tests, untested error paths).

step 8: reliability review (standard review path only)

1. examine error handling completeness, timeout and retry logic, and resource cleanup on error paths.
2. flag incomplete graceful-degradation patterns.
3. consult reliability-patterns.md for detection patterns.
4. output of step 8: reliability findings (error handling gaps, resource leaks, timeout issues).

step 9: removal candidates (standard review path only)

1. identify dead code, unused imports, and feature-flagged code that can be cleaned up.
2. distinguish safe-to-delete (no references in the codebase) from defer-with-plan (needs migration or deprecation).
3. output of step 9: cleanup opportunities.

step 10: verification (standard review path only)

1. run formatter/lint/tests on touched files. note what was skipped and why.
2. if code changes affect README, ARCHITECTURE, CONTRIBUTING, or other docs, note staleness (informational, not blocking).
3. output of step 10: verification summary, skipped items and rationale.

step 11: severity classification and dedupe (standard review path only)

1. for each finding, assign a severity tier (Critical, Important, Medium, Minor) using severity-and-confidence.md.
2. assign a confidence score (0.0-1.0). suppress low-confidence findings per the 5-band rubric in severity-and-confidence.md.
3. suppress false positives using categories in false-positive-suppression.md (framework idioms, test-specific patterns, overridable conventions).
4. remove duplicate findings (same issue reported from multiple angles). keep the highest-severity version.
5. output of step 11: deduplicated findings with severity and confidence scores.

step 12: action routing (standard review path only)

1. for each finding, classify the fix into one of four tiers: safe_auto / gated_auto / manual / advisory using action-routing.md.
2. when in doubt, escalate to gated_auto rather than promoting to safe_auto.
3. resolve conflicting classifications per action-routing.md.
4. output of step 12: findings with action-routing tier.

step 13: compile and format output (standard review path only)

1. structure findings into severity buckets (Critical, Important, Medium, Minor).
2. limit output to 10 findings per severity; if more exist, note the count and show highest-impact findings.
3. number findings sequentially across all severities (CR-001, CR-002, ...) for referencability.
4. for each finding, include: file:line reference, quoted code, issue description, confidence score, impact/rationale, concrete suggestion or alternative.
5. prefix inline comments with labels: no prefix (required, blocks merge), Nit: (style, optional), Consider: (suggestion), FYI: (informational).
6. compile a "what's working well" section with specific positive observations.
7. list residual risks or unresolved assumptions.
8. issue a verdict: Ready to merge / Ready with fixes / Not ready with one-sentence rationale.
9. escape literal | as \| in table cells to prevent markdown parsing errors.
10. if the review is clean (no findings), explicitly summarize what was checked and why no issues were found.
11. output of step 13: formatted review report per the output format in the "output contract" section below.

step 2b: alternative path for non-git input

used when step 1 fails to find a git repository.

1. request explicit file paths from the user (e.g., "paste file paths, one per line, or attach a diff").
2. parse the provided file list or diff content.
3. proceed to step 2 (mode selection) using the provided file metadata.

decision points

not in a git repository (step 1 pre-flight fails)

• if git rev-parse --git-dir returns an error, ask the user for explicit file paths instead of failing. use the provided paths to determine scope.

zero files resolve from scope fallback chain (end of step 1)

• if all scope fallback steps yield empty, ask the user: "what files or directories should i review?" do not proceed without a review target.

mode selection signals (step 2)

• 3+ signals or deep override: trigger deep review mode. dispatch parallel specialist agents per deep-review.md and stop (do not proceed to step 3+).
• 2 signals: suggest to user. if user declines, proceed to standard review.
• 0-1 signals or quick override: proceed to standard review.
• exception override: pure docs, mechanical refactors, or single-file <50-line changes override signal count and trigger standard review instead of deep review.

prior ci failures (step 3, run automated gates)

• if tests or linting fail, note the failure in the review and proceed with manual review. do not block at this step; the findings may explain the failures.

behavioral change without baseline read (step 5, line-by-line)

• if a change appears to alter behavior, read the pre-change file at the base branch (git show <base>:<file>) before flagging as a regression. a re-spec may have intentionally redefined the contract; a dropped branch may have been a latent bug.
• if regression is confirmed, cite the introducing commit (SHA, author via git blame or git bisect) as evidence.

filter/redact/hide change (step 6, security review)

• enumerate every field in the response that surfaces the entity. check that the hidden data is absent from all surfacing fields, not just the primary list (hidden data may leak via *_count, *_ids, summary projections, etc.).

documented override or deliberate bypass (any step)

• if a CLAUDE.md, AGENTS.md, inline comment, or code comment documents a deliberate bypass (e.g., "we allow X because Y"), honor it. do not re-raise the concern or work around it. if the override lacks rationale, suggest documenting one rather than arguing the rule.

unverified absence claim (any step)

• before raising a finding that depends on "symbol/handler/path doesn't exist", check it directly: read the region or grep the exact symbol expecting zero hits. a subagent's confident negative or a broad pattern match is unreliable; proving absence requires whole-search-space coverage. positive findings ("here it is at file:line") are trustworthy; negatives must be re-derived.

large diff (step 4+)

• for diffs >500 lines, review by module rather than file-by-file. flag oversized PRs (ideal ~100-300 meaningful lines, excluding generated code) and suggest split strategies (stack, by-file-group, horizontal, vertical) per pr-sizing.md.

output contract

standard review report (text/markdown format)

• structured by severity (Critical, Important, Medium, Minor).
• findings numbered CR-001, CR-002, etc. sequentially across all severities.
• each finding includes: severity tag, finding ID, file:line reference, quoted code (if applicable), issue description, confidence score (0.0-1.0), impact/rationale, concrete suggestion or alternative, action-routing tier (safe_auto, gated_auto, manual, advisory).
• inline comment labels: no prefix (required change), Nit: (style), Consider: (suggestion), FYI: (informational).
• sections: Critical, Important, Medium, Minor, What's Working Well, Residual Risks, Verdict.
• verdict states one of: Ready to merge, Ready with fixes, Not ready with one-sentence rationale.
• if no findings: explicit summary of what was checked and why no issues were found (clean review).
• markdown table cells escape literal | as \| to prevent parsing errors.
• references to external docs (e.g., deep-review.md, security-patterns.md) indicate where to load additional context.

deep review report (if triggered in step 2)

• specialist agents produce parallel findings per [deep-review.md](