core-verify-code-review

name	core-verify-code-review
description	Use when reviewing changes since a fixed point, or when a non-trivial in-flight decision needs adversarial scrutiny — keep Standards and Spec separate, measure relevant complexity, and use the bounded doubt cycle (claim → extract → fresh-context adversarial review → reconcile → stop).

Code Review

What a review looks for and how findings are handled — for human and agent reviewers alike. Adapted from Matt Pocock's code-review and Addy Osmani's doubt-driven-development, with the complexity pass informed by Saurabh Kumar's cyclomatic-complexity skill. The local entrypoint keeps the two-axis review model and adds complexity as a measured maintainability signal.

Areas under consideration

• Review structure — the two-axis model
• Baseline smell taxonomy
• Review scope — diff against a pinned fixed point
• In-flight adversarial review — the doubt cycle
• Complexity measurement and behavior-preserving refactoring
• Priority order — correctness > security > simplicity > style? The user's ranking
• Severity taxonomy — what blocks merge vs what's a suggestion
• Self-review checklist — what's checked before requesting review
• How /code-review passes fit in, at what effort

Skill

Review along two independent axes

A change can pass one axis and fail the other — code that follows every standard but implements the wrong thing, or does exactly what the issue asked while breaking conventions. Review Standards (does the code conform to the repo's documented standards?) and Spec (does the diff faithfully implement the originating issue/PRD/spec?) separately — as parallel sub-agents when available so they do not pollute each other's context — and report them under separate headings. Never merge or rerank findings across axes. End with findings-per-axis counts and the worst issue within each axis.

Pin the fixed point first

The review target is git diff <fixed-point>...HEAD (three-dot — against the merge-base), plus the commit list from git log <fixed-point>..HEAD --oneline. If the user did not name a fixed point, ask. Confirm the ref resolves and the diff is non-empty before doing any review work.

Find the spec source

In order: issue references in commit messages (#123, Closes #45) → a path the user passed → a PRD/spec file under docs/, specs/, or scratch dirs matching the branch → ask the user. If there is no spec, skip the Spec axis and say so in the report. The Spec axis reports: (a) requirements missing or partial, (b) behavior not asked for (scope creep), (c) requirements implemented but implemented wrong — quote the spec line for each finding.

The Standards axis: repo docs plus the smell baseline

Use whatever the repo documents (CODING_STANDARDS.md, CONTRIBUTING.md, CLAUDE.md) — plus this fixed baseline of Fowler smells (Refactoring, ch. 3), which applies even when the repo documents nothing. The repo overrides the baseline. Every baseline smell is a judgment call, never a hard violation. Skip anything tooling already enforces. Each smell reads what it is → how to fix:

• Mysterious Name — a name does not reveal what it does or holds → rename it; if no honest name comes, the design is unclear.
• Duplicated Code — the same logic shape appears in multiple hunks or files → extract the shared shape and call it from both.
• Feature Envy — a method reaches into another object's data more than its own → move it onto the data it envies.
• Data Clumps — the same few fields or parameters travel together → bundle them into one type.
• Primitive Obsession — a primitive stands in for a domain concept → give the concept a type.
• Repeated Switches — the same switch or conditional cascade on the same type recurs → replace it with polymorphism or one shared map.
• Shotgun Surgery — one logical change forces scattered edits → gather the change into one module.
• Divergent Change — one module is edited for unrelated reasons → split it so each part changes for one reason.
• Speculative Generality — hooks or abstractions serve no current requirement → inline them until a real need appears.
• Message Chains — long navigation such as a.b().c().d() leaks an object graph → hide the walk behind a method.
• Middle Man — a thing mostly delegates onward → cut it and call the target directly.
• Refused Bequest — an implementer ignores most inherited behavior → use composition.

Distinguish hard violations (documented-standard breaches can be) from judgment calls (baseline smells always are), citing the standard or naming the smell and quoting the hunk.

Measure complexity before judging it

Cyclomatic complexity is a count of independent decision paths: decision points + 1. Count the constructs recognized by the project's analyzer, commonly conditionals, case arms, loops, exception handlers, conditional expressions, and logical operators in conditions. Counting rules differ by tool, so report the tool and rule when one exists.

Project configuration wins. Read the repository's linter, analyzer, or quality gate first. Use the analyzer already present when possible; common choices include:

• Python: radon cc -s -a <path>
• JavaScript or TypeScript: the ESLint complexity rule
• Go: gocyclo
• Polyglot repositories: lizard <path>

If no analyzer is available, count manually per touched function and show the count. When the project has no threshold, use these as triage bands rather than universal merge laws:

• 1–5: usually leave it alone.
• 6–10: inspect it, especially if the function is already being changed.
• 11–15: refactor when the branches obscure the behavior or the change adds more paths.
• Above 15: split or document the reason to keep it before accepting more behavior.

A number alone is not a finding. Report complexity when it makes behavior harder to read, test, review, or change, or when it violates a project threshold. Do not demand unrelated cleanup in untouched code.

Refactor complexity without hiding it

When the diff changes a complex function, measure all touched functions, rank the hotspots, and review the worst one first. Prefer these tactics in order:

1. Guard clauses — return early to remove unnecessary nesting.
2. Extract functions — give each extracted behavior a name that says what it does.
3. Lookup tables or maps — replace repeated selection branches when the variation is data.
4. Named predicates — replace a dense boolean expression with a domain-level question.
5. Polymorphism or a strategy — use it for a recurring type-based variation, not a single switch that is still clear.
6. Flatten loops — extract the loop body or continue early instead of nesting conditions.

Preserve behavior and public interfaces. Run the relevant tests before and after when they exist; when they do not, say so and refactor conservatively. Move complexity into small, well-named units rather than clever expressions, compressed one-liners, or opaque helpers. If a function's name needs “and,” examine whether it has more than one responsibility.

Every complexity refactor ends with a compact before/after report:

## Complexity report
| Function | Before | After |
|----------|--------|-------|
| parseOrder | 14 | 4 |

Extracted: validateHeader, resolveDiscount
Behavior verified: <tests or other evidence>

Doubt-driven review: in-flight, not just post-hoc

The two-axis review above is a verdict on a finished diff; doubt-driven review applies the same adversarial energy while course-correction is still cheap. A confident answer is not a correct one.

Apply it to non-trivial decisions only: new or changed branching logic, a module or service boundary, properties the compiler cannot verify (thread safety, idempotence, ordering), correctness that depends on invisible context, or an irreversible blast radius. Do not use it for renames, formatting, or clear instructions.

The cycle is bounded to three rounds:

1. Claim — name the decision in two or three lines and say why it matters. If it cannot be stated that compactly, the decision is not clear yet.
2. Extract — isolate the smallest reviewable artifact and its contract. Strip the journey and conclusions from what the reviewer receives.
3. Doubt — use a fresh-context reviewer with an explicitly adversarial prompt: find what is wrong, assume the author is overconfident, and look for unstated assumptions, edge cases, hidden coupling, and contract violations. Pass the artifact and contract, not the claim.
4. Reconcile — classify each finding as a contract misread, valid and actionable, valid trade-off, or noise. Fix actionable findings and repeat.
5. Stop — stop when findings become trivial, after three rounds, or when the user says to ship. Three unresolved rounds mean the artifact should be decomposed or escalated.

Watch for doubt theater: repeated rounds with substantive findings and no actionable change mean the review is validating instead of doubting.