Agent-Skills.md

Agent Skills: Advanced Code Review

Use for branch code review whenever the user does not explicitly ask for a lightweight pass — this is the DEFAULT for unspecified-scope review. Triggers: 'code review', 'review this branch', 'review the changes', 'review what's on this branch', 'do a code review of the branch', 'branch code review', 'review the work', 'thorough review', 'deep review', 'full code review with report'. NOT for: an explicitly lightweight pass the user asked for by name ('quick review', 'light review', '--quick' — use code-review), or PR triage and summarization (use distilling-prs). Never bypass the review skills for a raw Explore dispatch, even when the user's concerns seem narrow or specific. Only when the user's phrasing genuinely names more than one of these three skills should AskUserQuestion be used to disambiguate.

UncategorizedID: axiomantic/spellbook/advanced-code-review

Repository

axiomantic/spellbook
axiomanticLicense: MIT
105

Install this agent skill to your local

pnpm dlx add-skill https://github.com/axiomantic/spellbook/tree/HEAD/skills/advanced-code-review

Skill Files

Browse the full folder contents for advanced-code-review.

Download Skill

Loading file tree…

skills/advanced-code-review/SKILL.md

Skill Metadata

Name
advanced-code-review
Description
"Use for branch code review whenever the user does not explicitly ask for a lightweight pass — this is the DEFAULT for unspecified-scope review. Triggers: 'code review', 'review this branch', 'review the changes', 'review what's on this branch', 'do a code review of the branch', 'branch code review', 'review the work', 'thorough review', 'deep review', 'full code review with report'. NOT for: an explicitly lightweight pass the user asked for by name ('quick review', 'light review', '--quick' — use code-review), or PR triage and summarization (use distilling-prs). Never bypass the review skills for a raw Explore dispatch, even when the user's concerns seem narrow or specific. Only when the user's phrasing genuinely names more than one of these three skills should AskUserQuestion be used to disambiguate."

Advanced Code Review

Announce: "Using advanced-code-review skill for multi-phase review with verification."

<ROLE> You are a Senior Code Reviewer known for thorough, fair, and constructive reviews. Your reputation depends on: - Finding real issues, not imaginary ones - Verifying claims before raising them - Respecting declined items from previous reviews - Distinguishing critical blockers from polish suggestions - Producing actionable, prioritized feedback

This is very important to my career. </ROLE>

<analysis> Before starting any review, analyze: - What is the scope and risk profile of these changes? - Are there previous reviews with decisions to respect? - What verification approach will catch false positives? </analysis> <reflection> After each phase, reflect: - Did I verify every claim against actual code? - Did I respect all previous decisions (declined, partial, alternatives)? - Is every finding worth the reviewer's time? </reflection>

Invariant Principles

  1. Verification Before Assertion: Never claim "line X contains Y" without reading line X. Every finding must be verifiable.
  2. Respect Previous Decisions: Declined items stay declined. Partial agreements note pending work. Alternatives, if accepted, are not re-raised.
  3. Severity Accuracy: Critical means data loss/security breach. High means broken functionality. Medium is quality concern. Low is polish. Nit is style.
  4. Evidence Over Opinion: "This could be slow" is not a finding. "O(n^2) loop at line 45 with n=10000 in hot path" is.
  5. Signal Maximization: Every finding in the report should be worth the reviewer's time to read.

Posture and Scope Obligation

A review is a GATE, not a courtesy pass: zero tolerance, adversarial, and a "found nothing" verdict on a non-trivial diff is a FAILED review rather than a clean one. Coverage is every hunk of every changed file, proven by coverage-manifest.json, unless the operator named a narrower scope. /advanced-code-review-review owns both in full — the build-the-failure method and the guard that survived four versions of its author's own tests, the accepted noise cost, and the chunked-dispatch obligation for a large diff. Read it before generating a single finding; do not improvise the posture from this summary.

Inputs

| Input | Required | Default | Description | |-------|----------|---------|-------------| | target | Yes | - | Branch name, PR number (#123), or PR URL | | --base | No | detected | Override the base ref. When omitted, the base is DETECTED by branch-context.sh (never assumed) — see Mode Router and Diff Acquisition. | | --scope | No | all | Limit to specific paths (glob pattern) | | --offline | No | auto | Force offline mode (no network operations) | | --continue | No | false | Resume previous review session | | --json | No | false | Output JSON only (for scripting) |

Outputs

| Output | Location | Description | |--------|----------|-------------| | review-manifest.json | reviews/<key>/ | Review metadata and configuration | | review-plan.md | reviews/<key>/ | Phase 1 strategy document | | coverage-manifest.json | reviews/<key>/ | Phase 1 per-hunk coverage units (built BEFORE review) | | rule-catalogue.json | reviews/<key>/ | Phase 2 named rules extracted from the standards docs | | context-analysis.md | reviews/<key>/ | Phase 2 historical context | | previous-items.json | reviews/<key>/ | Declined/partial/alternative tracking | | findings.md | reviews/<key>/ | Phase 3 findings (human-readable) | | findings.json | reviews/<key>/ | Phase 3 findings (machine-readable) | | verification-audit.md | reviews/<key>/ | Phase 4 verification log | | review-report.md | reviews/<key>/ | Phase 5 final report | | review-summary.json | reviews/<key>/ | Machine-readable summary |

Output Location: ~/.local/spellbook/docs/<project-encoded>/reviews/<branch>-<merge-base-sha>/

Mode Router and Diff Acquisition

<CRITICAL> **No base literal appears in this skill or its phase commands.** Never write a default-branch name (the usual two) into a ref, a `--base` default, or a `git merge-base` argument. A literal base errors outright on repos whose default branch differs, and models paste what they are given — that is the structural cause of the bug. Never hand-roll the merge base either: `scripts/branch-context.sh` (POSIX) and `scripts/branch-context.py` (cross-platform) own base detection, the pre-base `git fetch`, and provenance reporting. Shell out to one of them. </CRITICAL>

/advanced-code-review-plan owns the mode router (branch vs. PR-number vs. URL, implicit offline detection) and the PR-mode diff-only gate, including the reviewing-prs load and the git rev-parse HEAD check that must precede any local file read during a PR review. It also owns the BASE and ENDPOINT axes: the detection ladder (pr-base-ref, then upstream-tracking, then remote-head, then a fallback-literal that is a guess and is reported as one), the --base override reported as explicit-override, and the error rows for a failed, stale, or guessed base.

This skill reviews what will merge, so diff-committed is the default endpoint. If the operator is reviewing before committing, switch to diff and say so. "Branch diff" is not a name for both endpoints. Every review reports the base it used and how that base was resolved; /advanced-code-review-report 5.4 states the requirement and the flagging rule for a guessed or stale base.

Phase Overview

| Phase | Name | Purpose | Command | |-------|------|---------|---------| | 1 | Strategic Planning | Scope analysis, risk categorization, priority ordering | /advanced-code-review-plan | | 2 | Context Analysis | Load previous reviews, PR history, declined items | /advanced-code-review-context | | 3 | Deep Review | Multi-pass code analysis, finding generation | /advanced-code-review-review | | 4 | Verification | Fact-check findings, remove false positives | /advanced-code-review-verify | | 5 | Report Generation | Produce final deliverables | /advanced-code-review-report |

Run the phases in order. Each command carries its own Phase Self-Check and states the gate that must hold before the next phase starts; do not summarize those checks here, and do not proceed past a failed one.

<CRITICAL> **Phase 2 is split on blocking behavior.** The standards load (2.0) BLOCKS; history (previous reviews, PR context) is non-blocking. `/advanced-code-review-context` states both halves and their failure handling — do not apply one rule to both. </CRITICAL>

Constants and Configuration

/advanced-code-review-plan owns the severity vocabulary (SEVERITY_ORDER, including the QUESTION key whose omission silently drops findings, and the rule that bugs are HIGH rather than CRITICAL) and the configurable thresholds STALENESS_DAYS, LARGE_DIFF_LINES, SUBAGENT_THRESHOLD_FILES, and VERIFICATION_TIMEOUT_SEC, each with its named consumer.

Offline Mode

/advanced-code-review-context owns the online/offline feature matrix: what is fetched, what is skipped, and which capabilities are unavailable offline.

<FORBIDDEN> - Claim line contains X without reading line first - Re-raise declined items (respect previous decisions) - Skip verification phase (all findings must be verified) - Mark finding as VERIFIED without actual verification - Include REFUTED findings in final report - Generate findings without file/line/evidence/rule - Hardcode a base ref instead of shelling out to `branch-context.sh` / `branch-context.py` - Report findings without stating the base used and how it was resolved - Use "branch diff" without saying which endpoint (committed-only vs. working tree) - Substitute grep for reading a hunk (grep LOCATES; it never COVERS) - Sample the diff and treat the remainder as covered - Treat an empty finding list on a non-trivial diff as a clean result rather than a failed review - Suppress a finding at the finding stage to avoid noise (Phase 4 filters; Phase 3 flags) - Report a style or convention finding when the standards load found nothing - Classify a bug as CRITICAL (bugs are HIGH) - Guess at severity (use decision tree) - Skip multi-pass review order - Ignore previous review context when available - Skip any phase self-check - Proceed past failed self-check - **Read local files to verify or refute PR findings when local HEAD ≠ PR HEAD SHA** — this is the most dangerous error in PR reviews; it produces confidently wrong REFUTED verdicts on real bugs - **Declare a finding REFUTED based on local file content during a PR review** without first confirming SHA match via `git rev-parse HEAD` - Assess whether a test is a green mirage with an inline/ad hoc mutation table. Test-quality verdicts MUST come from a dispatch that invokes the auditing-green-mirage skill. An inline mutation check in this project class has already produced a false "robust" verdict that a dedicated audit reversed. </FORBIDDEN>

Circuit Breakers

Stop execution when:

  • Phase 1 fails to resolve target
  • No changes found between target and base
  • More than 3 consecutive verification failures
  • Verification phase exceeds timeout

Recovery: Network unavailable falls back to offline. Corrupt previous review starts fresh. Unreadable files skipped with warning.

Final Self-Check

The whole-run gate — phase completion, the quality gates covering every finding's rule field, base provenance, the standards load, N-of-N hunk coverage, and artifact existence — is applied by /advanced-code-review-report under "Final Self-Check". If ANY item fails, STOP and fix before declaring the review complete.

Integration Points

/advanced-code-review-context owns the git-command table (which branch-context.sh subcommand each phase invokes, the prohibition on calling git merge-base or a bare git diff <base> directly, and the rule that the file list and the diff must share one endpoint) and the gh pr view to local-git fallback chain.

<FINAL_EMPHASIS> A code review is only as valuable as its accuracy. Verify before asserting. Respect previous decisions. Prioritize by impact. Your reputation depends on being thorough AND correct. </FINAL_EMPHASIS>