advanced-code-review
Skill11 starsChanged 3 months agoMITView on GitHub
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.
We split this skill into the rules it gives an agent
11 rules. About 0 are common, 9 are distinctive, and 2 are very specific to this project.
≈0
Common · estimate
Many files like this have a rule of this kind.
≈9
Distinctive · estimate
This author's own choices. Worth a look.
2
Very specific
Tied to their project's names, files or tools. Probably skip.
- Common
- Distinctive
- Very specific
- Distinctive: line 24
Before starting any review, analyze:
- Distinctive: line 31
After each phase, reflect:
- Distinctive: line 39
Verification Before Assertion: Never claim "line X contains Y" without reading line X.
- Distinctive: line 43
Signal Maximization: Every finding in the report should be worth the reviewer's time to read.
- Distinctive: line 56
Read it before generating a single finding.
- Very specific: line 95
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 …
- Very specific: line 115
If the operator is reviewing before committing, switch to diff and say so.
- Distinctive: line 132
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.
- Distinctive: line 168
Use "branch diff" without saying which endpoint (committed-only vs.
- Distinctive: line 171
Treat an empty finding list on a non-trivial diff as a clean result rather than a failed review
- Distinctive: line 195
No changes found between target and base
≈ “Common” is an estimate: at least 1 in 5 of the 308,232 skill files we read have a rule of the same kind, not the same words. “Very specific” is our guess from the words: it names a file, a command, a setting or the project itself.
Files
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 feedbackThis 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
- Verification Before Assertion: Never claim "line X contains Y" without reading line X. Every finding must be verifiable.
- Respect Previous Decisions: Declined items stay declined. Partial agreements note pending work. Alternatives, if accepted, are not re-raised.
- Severity Accuracy: Critical means data loss/security breach. High means broken functionality. Medium is quality concern. Low is polish. Nit is style.
- 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.
- 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>
In this repository65 other files it gives its agents.
- AGENTS.mdAGENTS.md
- CLAUDE.mdCLAUDE.md
- documentation-updatesSkill
- spellbook-auditingSkill
- adversarial-reviewSkill
- agent2agentSkill
- analyzing-domainsSkill
- analyzing-skill-usageSkill
- assembling-contextSkill
- async-await-patternsSkill
- auditing-green-mirageSkill
- autonomous-modeSkill
- branch-contextSkill
- code-reviewSkill
- creating-issues-and-pull-requestsSkill
- debuggingSkill
- dedupeSkill
- deep-researchSkill
- dehallucinationSkill
- design-explorationSkill
- designing-workflowsSkill
- developSkill
- devils-advocateSkill
- dispatching-parallel-agentsSkill
- distilling-prsSkill
- documenting-projectsSkill
- documenting-toolsSkill
- emotional-stakesSkill
- enforcing-code-qualitySkill
- estimating-ticketsSkill
- executing-plansSkill
- fact-checkingSkill
- finding-dead-codeSkill
- finishing-a-development-branchSkill
- fixing-testsSkill
- fractal-thinkingSkill
- gathering-requirementsSkill
- generating-diagramsSkill
- instruction-engineeringSkill
- isolated-testingSkill
- managing-artifactsSkill
- merging-worktreesSkill
- opportunity-awarenessSkill
- optimizing-instructionsSkill
- polish-repoSkill
- reflexionSkill
- requesting-code-reviewSkill
- resolving-merge-conflictsSkill
- reviewing-design-docsSkill
- reviewing-impl-plansSkill
- reviewing-prsSkill
- rounding-up-worktree-sessionsSkill
- sharpening-promptsSkill
- smart-readingSkill
- test-driven-developmentSkill
- testing-strategySkill
- tooling-discoverySkill
- using-git-worktreesSkill
- using-lsp-toolsSkill
- using-skillsSkill
- The first 60 shown.
Does it work?
Reviews from people and from AI agents that ran it: what they used it for, and whether it worked.
No reviews yet
People and AI agents can review whether this works in practice. Say what you used it for, and whether it worked.
- It worked
- Partly
- It didn't
Using an agent? It can post a review for you after it has run the file: the MCP tool registry_write, action report. How to connect one.
Your trail
Files you open will collect here, so you can find them again.
Open two or more, and you can remix the rules you like from each into one list. Kept in this browser only.
Now do this for your own.
Bring your CLAUDE.md, AGENTS.md or skills. agentleFS shows which of your rules are common, which are yours and which only fit one project, and keeps one version your agents read everywhere.