Skip to content

How the Review Model Works

amirbena edited this page Sep 18, 2026 · 6 revisions

How the Review Model Works

A concept-level walkthrough of the review pipeline both Skills run. Full detail: docs/ARCHITECTURE.md §2 "Core Pipeline."

The shared standard, split delivery

                     Shared Review Standard
             (shared/policies/ — scope, severity, evidence,
              review-context, review-evidence, ownership)
                              │
              ┌───────────────┴───────────────┐
              │                               │
      local-code-review                github-pr-review
              │                               │
   local Git delta                   GitHub PR delta
              │                               │
   one returned report                report, or GitHub
                                       publication (active)

Everything above the split is one copy consumed identically by both Skills — P0/P1/P2 semantics never diverge. See Repository-Documentation-Map for exactly which file owns which rule.

Three phases, every review

  1. Resolve inputs — turn the invocation into a normalized Review Target (never widened later), optional Review Context, Repository Context, and optional Existing Review Evidence.
  2. Reason — read-only state inspection, compute the exact delta, optionally prepare a repository-backed checkout (github-pr-review only), plan sequential or parallel execution, produce candidate findings, then reconcile them centrally.
  3. Decide and deliver — one aggregator applies the shared severity model to produce one P0/P1/P2 finding set and one decision, rendered as a local report or a GitHub review.

Concepts you'll see repeatedly

  • Review Target — the only thing whose defects a decision blocks: the local delta, or the PR delta. Never expanded by context.
  • Review Context — optional evidence about intended scope (requirements, a ticket, an ADR, a plan). Shapes what's inspected, is never itself a verdict. See Review-Context-and-Existing-Evidence.
  • Repository Context — surrounding code, AGENTS.md/CLAUDE.md instructions, architecture docs, tests. Refines how code is judged.
  • Existing Review Evidence — prior findings, resolved findings, settled decisions from an earlier review pass, reconciled against the current target — never blindly inherited.

Always-on reasoning passes

Every review — regardless of size — runs a fixed set of deterministic passes emitted as subordinate metadata, never as a merge gate on their own:

  • Change-risk / review-depth classification — a fixed signal catalog (auth, migration/schema, concurrency, public API, sensitive path, infra/config, diff size) maps to standard / elevated / deep.
  • Repository-expansion — bounded, ring-based follow-through on fired triggers (call site, interface/contract, migration/schema, config consumer), scaled by that same depth.
  • Large-PR partitioning — conditional; activates only above a fixed diff-size threshold, splitting a large change into coherent review units that are each fully reviewed and then aggregated/de-duplicated.
  • Review stopping criteria — states whether coverage was complete; an incomplete review renders as an explicit incomplete outcome, never as clean.

Canonical detail for all four: shared/policies/ (change-risk-signals.md, repository-expansion.md, large-pr-partitioning.md, review-stopping-criteria.md).

Base reasoning, then evidence-driven depth

Every dimension "Semantic change-implication reasoning" identifies gets base-level reasoning, unconditionally — no risk domain is opt-in. On top of that, one further question is asked per materially implicated dimension: does the evidence already gathered justify going deeper with domain-specific reasoning?

base semantic reasoning
    ↓
materially implicated concern
    ↓
evidence-driven specialist-depth decision
    ↓
0..N domain-specific deepening capabilities
    ↓
one unified review

That decision is evidence-driven — never a file-type, path, or framework router — and zero, one, or several capabilities may engage on the same review, composing into one review rather than separate verdicts. Full walkthrough, the implemented capabilities, and the benchmark that pins this contract: Specialist-Depth-Review.

From an observation to a candidate to a finding

"Produce candidate findings" above is not one step — a suspicious observation still has to earn its way to a severity-bearing finding before the mechanical decision derivation ever sees it. Canonical: docs/candidate-finding-validation/candidate-finding-validation-model.md (this page only summarizes it).

observation → candidate claim → validated finding → severity
  • Observation ≠ finding. Noticing an unusual shape — a branch difference, a missing check, a differently-handled field — is not itself evidence of a defect. It only becomes a candidate claim once the reviewer can state what specifically differs from what should hold, and why.
  • Semantic-role validation. When a candidate's own reasoning compares two usages of the same field, function, or path, it must first establish they serve the same responsibility. The same primitive handled differently in two places is evidence of a difference, not automatically evidence of a defect — a standalone technical-invariant violation with no such comparison skips this gate entirely.
  • Evidence/contract grounding. Every candidate names the contract, invariant, or expected behavior it violates, from a source hierarchy running from explicit requirements down to reviewer inference alone. A technically-grounded blocking finding never needs a Jira reference — a TOCTOU race, a broken atomicity guarantee, or a security-boundary bypass stands on its own technical merit.
  • Causal validation and regression-proof discipline. A candidate must show reviewed change → changed assumption → concrete failure condition → observable incorrect result, not just "these paths are inconsistent." A regression claim additionally needs evidence of the prior behavior, the change, the failure scenario, and the causal link between them — absent that, it is never presented as a proven regression.
  • Disconfirmation pass. Before a blocking candidate is accepted, the reviewer actively tries to invalidate it against the bounded review context already gathered. The candidate either survives, is dropped, is downgraded, or is reclassified — this is a genuine attempt to falsify the candidate, not a restatement of its own supporting evidence.
  • Finding validity and blocking-justification validity are separate questions. A candidate that clears every evidence gate but whose impact doesn't clear the P0/P1 bar is kept, not discarded and not silently reclassified as ambiguity or a coverage gap — it is reported at whatever classification the evidence actually earned, typically at P2. Requirement ambiguity, test-coverage gaps, and maintainability concerns are never silently promoted to correctness defects, and a correctness defect is never silently downgraded to one of them merely because its impact was modest.

Four short examples:

Case Outcome
A field is strictly validated in an API controller but read unvalidated in an internal batch-report job. Not a candidate — the two usages serve different responsibilities (input-boundary check vs. reporting on already-validated data), so the divergence isn't evidence of a defect.
A read-then-write balance debit has no lock or concurrency check between the read and the write. A valid, technically-grounded P1 with no Jira reference — a TOCTOU race is a technical-invariant violation that stands on its own.
The same TOCTOU shape shows up in a rarely-invoked internal debug endpoint, on a display-only counter with no downstream consequence. Still reported as a proven correctness defect — classification never consults impact — but downgraded to P2 because material impact doesn't clear the blocking bar.
A reviewer suspects a changed default "used to be" false, but no test, changelog, or prior code shows what the previous value actually was. Not presented as a proven regression. Reported instead on whatever the causal chain does establish, at the classification that evidence actually supports.

This validation happens before a finding is labeled and before severity is derived — it changes nothing about how findings are labeled or how P0/P1/P2 are derived (see "From findings to a decision" below), and none of its intermediate reasoning steps are exposed in a rendered finding.

Evidence location, symptom location, and fix/action location

Once a finding clears the validation above, one more question decides where it is reported: not every place the review looked, and not necessarily where the defect was first noticed, is where the finding belongs. Canonical: shared/templates/finding.md (this page only summarizes it; finding.md also owns the separate "evidence location" and "publication" fields this section doesn't restate).

evidence location ≠ symptom location ≠ finding (fix/action) location

Evidence can move. The finding must not drift with it. A review follows callers, callees, sibling implementations, tests, and precedent code to gather evidence — but visiting a location and anchoring a finding there are different acts.

  • Causal ownership. The claim decides the anchor, not proximity or where the diff is easiest to annotate: a claim about cause anchors at the site that introduces the incorrect state or value, even when the failure is only observed downstream; a claim about unsafe handling of an otherwise-valid upstream state anchors at the downstream site that fails to guard against it.
  • Contract ownership. There is no mechanical caller/callee preference. Ownership is established from repository evidence — a documented contract, existing enforcement elsewhere, established lifecycle ordering — never from naming or position alone, and the same reasoning extends to every contract-owning boundary (validation/guard, state-transition/mutation, lifecycle, authorization, encoding/decoding, synchronization/state-assumption). See Specialist-Depth-Review for the underlying bounded caller/callee model this reuses.
  • Locality preservation during context expansion. Reading a caller, a test, a utility, or a precedent implementation for evidence never by itself relocates the finding. A location becomes the anchor only when the causal/contract reasoning above affirmatively establishes that it owns the claim.
  • Semantic honesty over false precision. The narrowest location that is still honest about where the defect lives wins — never a nearby, merely-commentable line chosen because it's convenient to anchor there.
  • GitHub-inline availability never redefines ownership. Whether a resolved location happens to be commentable on a GitHub PR decides how it is published (inline vs. review body) — it never decides where the finding semantically belongs. See Feature-GitHub-Publication-and-Authorization for that publication mechanic.

Three short examples:

Case Anchor
Cause vs. symptom. A pricing function returns a negative discount for a malformed coupon; a caller three frames away renders that value straight into an invoice total. The claim "a malformed coupon can produce a negative discount" is about cause — the pricing function is the anchor, and the invoice-rendering call site is evidence, not the anchor.
The precedent trap. A review reads a sibling implementation that handles the same input correctly, to show how the analogous case should behave. The sibling implementation is evidence supporting the finding's claim — it is never itself the anchor, because it is not the thing that must change.
The nearest-line trap. A missing state-machine transition is a property of a function's overall control flow, with no single line that represents it. The function (or the described mechanism) is the anchor, not one of its lines picked merely to manufacture a precise-looking location.

When more than one location remains plausible, ranking and the fallback-to-unresolved behavior are exactly what finding.md's canonical section governs — this page doesn't duplicate that ranking.

Where a validated finding is anchored across several sites

The reasoning above selects one primary location for a single finding. Separately, when one shared defect (a validator, helper, query, config value, contract, or invariant) is what actually propagates to several call paths or sites, that is one finding at the shared cause, not one near-duplicate finding per symptom. Every known manifestation site is still represented, in an affected locations list on that finding — nothing is silently dropped. This consolidation only applies once the shared cause is positively established; sites that merely rhyme or share a theme, without a demonstrated common cause, are reported as separate findings rather than over-merged. Canonical: shared/policies/root-cause-consolidation.md.

From findings to a decision

Once findings are finalized, the decision is derived mechanically, never as separate judgment — see Severity-and-Decision-Model.

Where this doesn't cover you

This page is explanatory. For the exact pipeline diagram, stage-by-stage ownership, and the optional-capability list (parallel review, runtime validation, stateful delta re-review, stacked-PR review), read docs/ARCHITECTURE.md directly — this page never overrides it.


Wiki Page Map

Page What it covers
Home Wiki entry point: what the two Skills are and where to go next.
Getting-Started First steps to package and run either Skill.
Choosing-a-Review-Skill How to decide between local-code-review and github-pr-review.
How-the-Review-Model-Works Concept-level walkthrough of the review pipeline shared by both Skills.
Architecture-Overview Component-level map of how the repository's pieces fit together.
Specialist-Depth-Review When and how a review goes deeper than its base pass.
Local-Code-Review-Guide Usage guide for reviewing local, not-yet-PR'd Git changes.
GitHub-PR-Review-Guide Usage guide for reviewing an existing GitHub Pull Request.
Review-Context-and-Existing-Evidence How optional requirements/context and prior review evidence are supplied.
Feature-Pages Index of per-feature capability pages and their shared template.
Feature-Delta-Re-Review How a re-review scopes itself to only what changed since the last pass.
Feature-GitHub-Publication-and-Authorization What a PR review publishes to GitHub and under what authority.
Feature-Human-Style-Review-Output The senior-engineer-voice prose summary output option.
Feature-Requirement-Coverage How supplied requirements are checked and reported as covered or not.
Feature-Runtime-Validation-Evidence How a review can execute commands or reproductions as evidence.
Severity-and-Decision-Model The P0/P1/P2 severity levels and how the merge decision is derived.
Packaging-and-Portability How the Skills go from source repository to a standalone archive.
Development-Workflow Contributor guide for working on this repository itself.
Benchmark-Testing Orientation to this repository's own review-quality benchmark pipeline.
Repository-Documentation-Map Where each concern's canonical, authoritative source lives.
AI-Code-Review-Landscape Neutral snapshot comparing this project against other AI code-review products.
FAQ Short, practical answers to common questions.

Clone this wiki locally