⛔ This skill MUST be read in full — not skimmed. Formal review gates depend on its workflow.
Skipping steps silently bypasses quality checks. Missing gates = undetected breakages.

Verifier gate: dispatches AI reviewers. Pipeline auto-advances when clean.

Cold-class seam (#512): reviewer/eval dispatches are cache-cold one-shot traffic — an operator who exports COLD_CLASS_PROVIDER=venice opts them into the venice leg (--provider venice --model deepseek-v4-flash; same model id — venice serves cold prompts with cache reads). Unset (default) = inert. Interactive/warm traffic never routes venice (docs/providers.md §8).

Canonical: agent-infra/skills/code-review/SKILL.md — git-tracked source of truth. Pi reads via ~/.pi/agent/skills; consumers hard-link into operations/skills.

Unified v3.3.0 — agent-neutral. Consolidates Claude (v1.8.0) and Pi (v2.0.0) versions. Test coverage Step 0, 5 merged reviewers (A Guidance & History, B Correctness & Security with the bug scan's two ordered passes, C Architecture & Data, D Surface & Config, E Infrastructure — conditional), convergence-gated fixer loop, --standard-tier flag, Supabase error logging, merge-dedup step. Uses agent-neutral sub-agent dispatch.

v3.3.0 (reviewer merge). The 11 dispatch slots are merged into 5 reviewers by concern — every former lens is covered by exactly one merged reviewer, and the bug scan's two passes stay two passes in order. RISK_CLASS (from proportional-gates §Change Classification) now caps the count of the A/B/C/D panel, at the counts proportional-gates §Review Cycles owns (Low 1, Low-Medium 2, Medium-High 3, High 4 — read its table; it is the single owner). The panel is filled in priority order — B (Correctness & Security), then the first surface-matched reviewer, then A (Guidance & History), then the second — and truncated to the class's count. File surfaces (UI_TOUCHED/ARCH_TOUCHED/DATA_TOUCHED/CONFIG_TOUCHED) therefore select which reviewers fill the panel and which sections of C/D apply — never how many run. INFRA_RISK scales depth inside the single reviewer E, which is appended whenever it is set — at every class, Low included (infra files are never trivial, Step 0.8) — and is never counted against the panel cap. The Low class runs one reviewer round (both of B's bug-scan passes, in order), no re-review loop. ISSUE: block shape and check_type values are unchanged. Supersedes the reviewer-count clause of agent-infra #80 (owner directive 2026-09-30, "let's merge reviewers"); surface-first detection is preserved.

Code Review

Provide a code review for the given pull request.

Arguments

code-review <PR_NUMBER> [--re-review] [--standard-tier]

• PR_NUMBER: the pull request number to review
• --re-review: follow-up review after fix commits; see Smart Re-Review
• --standard-tier: reduced 2-agent review (Guidance Compliance + Bug Scan — pass 1 only, the blind diff scan; the deep pass 2 is skipped)

Routing:

• --standard-tier present → skip to Standard-Tier Review
• --re-review present (without --standard-tier) → skip to Smart Re-Review
• Neither flag → follow steps 1–10 below

Auto-Continue Protocol

After review completes clean (zero blocking findings — see §"Finding disposition — what BLOCKS,
what is a FOLLOW-UP": a finding blocks only when it is attributable to this change AND names the wrong
outcome it can produce; everything else is a filed follow-up), record the verdict (Step 10), then
auto-merge if applicable and proceed immediately. Do NOT pause to ask "shall I merge?" or "review
complete — proceed?"

Self-Healing: When review finds issues, the fixer loop (Steps 6-6.5) handles all severity levels with confidence scoring, convergence detection, and escalation. Do NOT apply separate fixing logic — use the existing fixer loop. See Steps 6-6.5 for the full self-healing workflow.

Only pause when: P0 issue requires human architectural/security decision (data loss, security breach, irreversible) AND the fixer loop's escalation mechanism has been exhausted.

Process (Full Review)

Step 0.0 — Reviewer Test Scope: DERIVE, never accept a hand-picked list

On a full review, before the coverage check below. It applies to the full review. The
--standard-tier path starts at Step 1 and does not run it, and --re-review skips to Smart
Re-Review (Step A) — so on a re-review, derive the scope on the fix-commit delta yourself before
continuing; do not assume this step ran.

The controller must pass the reviewer the DIFF, never a hand-picked list of TEST files. (Affected
files as review context, per Step 4, are a different thing.) The reviewer derives its own scope and
names every file it did not run.

Run the legs CI runs — not just test_files. The selector subtracts the other legs out of
test_files, so a reviewer who runs only that set has a smaller denominator than CI, which is the
defect this step exists to prevent. These are the three diff-gated legs — they are not the whole
of CI, which also runs always-on Python jobs (test-track-b selecting the track_b markers that
every leg below marker-deselects, test-concurrency-falkor, packs-compile, and
test-d14-hosted-api — a whole-tree -m embedded_only selection that no other job executes outside
the carve-out file list); treat those as further files to run or to name as not-run — and enumerate
the workflow's unconditional jobs yourself rather than trusting this list to be complete:

JSON field	Run it when
test_files	always — if the value is the sentinel "ALL", expand it to the full suite
slow_selected	slow_run is true (CI's test-slow, gated on slow_run == 'true')
the carve-out leg	carve_out_run is true (CI's test-carve-out); the JSON carries the flag but not the file list — read it from the repo's CI surface config

If you deliberately leave a leg to CI, say so and name it as not-run. Do not silently drop one.

# The repo's CI diff selector. Guard the INPUT, not the selector's output: an empty or
# failed file fetch makes the selector answer with a plausible tier-1 smoke set (full=false,
# slow_run=false, carve_out_run=false) — a silently stripped suite that still reports green.
CHANGED="$(gh api "repos/{owner}/{repo}/pulls/<PR_NUMBER>/files?per_page=100" --paginate --jq '.[].filename')"
if [ -z "${CHANGED//[[:space:]]/}" ]; then
  echo "changed-file list EMPTY (or the fetch failed) — UNAVAILABLE, not 'no tests'"; exit 1
fi
printf '%s\n' "$CHANGED" | uv run python tools/ci_selection.py --changed-files - --event pull_request
# Then the callers/importers of every changed symbol (unbounded — do not cap this):
rg -ln '<changed_symbol>|<module_basename>' --glob '!*.md'

An empty changed-file list is UNAVAILABLE, never "no tests". Do not test the selector's output
for emptiness — it never is. This mirrors what CI and the rail already do: CI refuses an empty
$BASE...HEAD set before invoking the selector (#3442: refusing to certify this head on the
tier-1 smoke alone), and the rail guards it whitespace-aware (an unavailable answer, not a diff with no files).

The selector is repo-specific. tools/ci_selection.py is the tortoise tool; agent-infra,
eldato and premise-labs do not have it, and this snippet will not run there. The portable rule is:
use the repo's CI diff selector; if the repo has none, derive the scope from its test-file→source
mapping. Substitute your repo's selector for the snippet above.

Why this is a gate and not advice — the observed failure. On tortoise PR #7615 the head
8b7ac2d559 was CI-red on
tests/longmem_eval/test_ingest_stall_guard.py::test_ingest_haystack_v2_aborts_stalled_question
(AssertionError: SDK touched during a stalled ingest: _graph_write_retry_count, shard test (g),
run 37684958263, completed failure at 20:51:52Z) — while a fresh-context review on that same
sha, posted 21:21:26Z
(comment), returned
NO ISSUES FOUND. The guard that fired is a _BoomSDK.__getattr__ that raises on any attribute
access; it lives in a file the reviewer was not told to run. Do not cite a count of clean cycles
you cannot point at an artifact for — cite the sha, the failing test, and the permalink of the
clean verdict at that sha.

The mechanism is structural, not carelessness: scoping a reviewer by hand-picked files silently
converts its question from "is this change correct?" into "is this change correct within the
files I thought of?", and the second question has a far smaller denominator.

Corollaries.

• Intent, not an enforced gate: record the derived scope in the review comment. A clean verdict
  without it cannot be audited later — note that the merge gate does not read it (Step 10 is
  explicit that the artifact is an auditability boundary, not proof of review).
• A P3 fixed in round N can introduce the red found in round N+1. On #7615 a finally fold added
  to satisfy a prior round's P3 was half of that regression; removing only it leaves the test red.
  Re-derive and re-run the scope after every fix, not only at the end.

Step 0 — Test Coverage Check (MANDATORY, always runs)

Before reviewing code, check whether source changes are accompanied by test changes:

# List source files changed in the PR (exclude test files, type defs, config)
gh pr diff <PR_NUMBER> --name-only | grep -E '\.(ts|tsx)$' | grep -v '\.test\.' | grep -v '\.d\.ts'

For each source file, check if its test file was also changed:

• src/components/Foo.tsx → check src/components/Foo.test.tsx
• src/hooks/useBar.ts → check src/hooks/useBar.test.ts

If source changed but test did not: flag as a P1 issue with check_type: test-coverage-gap. Include in the review output:

ISSUE:
  check_type: test-coverage-gap
  severity: P1
  origin: diff
  file: <changed source file>
  outcome: <the wrong outcome this can produce, or none>
  location: <changed source file>
  description: Source file changed without corresponding test file update.
    <test file> was not modified. Tests may be stale.
  suggestion: Verify tests still pass and cover the changed behavior.
    Update tests if the source change affects behavior.

Exceptions (do NOT flag):

• The change is purely cosmetic (formatting, comments, import reordering)
• The changed file has no test file (new module, config file, type definition)
• The PR description explicitly states tests are not needed

Step 0.1 — Integration Test Coverage

For source files that touch database operations, check for integration tests:

# Files touching DB operations (supabase rpc, from, insert, update)
gh pr diff <PR_NUMBER> --name-only | xargs grep -El 'supabase\.(rpc|from|insert|update)' 2>/dev/null

For each DB-touching file, check if its integration test file exists:

• src/lib/deals.ts → check src/__tests__/integration/deals.integration.test.ts
• src/services/profiles.ts → check src/__tests__/integration/profiles.integration.test.ts

If DB surface changed but integration test missing: flag as P1 check_type: test-coverage-gap with specific layer:

ISSUE:
  check_type: test-coverage-gap
  severity: P1
  layer: integration
  location: <changed DB file>
  description: DB surface changed without integration test.
    Unit tests with mocked Supabase do not verify RLS, triggers, or schema.
  suggestion: Add integration test using supabase start + real DB.
    See test-integration skill for patterns.

Step 0.2 — E2E Test Coverage

For changes to critical-path pages or flows, check for e2e tests:

# Files in critical paths (auth, booking, payment, profile)
gh pr diff <PR_NUMBER> --name-only | grep -E 'src/(pages|app)/(auth|booking|payment|profile|deals)'

For each critical path change, check for e2e test files:

• Pages changed → check e2e/critical-paths/*.smoke.spec.ts or *.e2e.spec.ts

If critical path changed but e2e test missing: flag as P1 check_type: test-coverage-gap with layer:

ISSUE:
  check_type: test-coverage-gap
  severity: P1
  layer: e2e
  location: <changed page>
  description: Critical path changed without e2e coverage.
  suggestion: Add smoke test (@smoke tag) for happy path, or full e2e (@e2e tag) for multi-page flows.
    See test-e2e skill for patterns.

Step 0.3 — UX Verification Coverage

For PRs with UI changes (.tsx files in components/ or pages/), check for UX verification:

# UI files changed
gh pr diff <PR_NUMBER> --name-only | grep -E '\.tsx$' | grep -v '\.test\.'

If UI changed: invoke ux-verification skill to check component library compliance, common failure patterns, and accessibility.

Flag if UX verification returns violations without fixes:

ISSUE:
  check_type: ux-violation
  severity: P1
  location: <component file>
  description: UX verification found unresolved violations.
    <summary from ux-verification report>
  suggestion: Fix violations or document why they're acceptable.

Step 0.5 — Test Review (MANDATORY, always runs when tests changed)

For each changed test file, invoke test-review for an independent review-fix loop. This replaces the shallow checklist with a proper research+review+fix+re-review cycle.

# List test files changed in the PR
gh pr diff <PR_NUMBER> --name-only | grep -E '\.test\.(ts|tsx)$'

For each changed test file:

test-review <test-file> --surface-map "<extract from plan doc>" [--journey-map "<extract from plan doc>"]

test-review handles the full cycle:

• Phase 0 — Research Intake: Researches testing patterns for the surfaces under test
• Phase 1 — Review: 2 merged fresh sub-agents (A correctness + discriminating power; B coverage/surface/journey), plus C (e2e + reproducibility) when e2e surfaces are touched
• Phase 2-4 — Fix + Re-review Loop: Surgical fixes, fresh re-review, to test-review's own convergence rule (fingerprint-stall; 10-cycle safety cap — see test-review §Phase 4)
• Exit: Clean (Reviewer A's combined verdict is REVIEWER A VERDICT: CLEAN — both its ordered passes clean — Reviewer B's combined verdict is REVIEWER B VERDICT: CLEAN — every part it ran clean — and Reviewer C, when dispatched, returns NO ISSUES FOUND) or capped with documented remaining issues

If test-review returns capped: flag the remaining issues as P1 with check_type: test-quality-gap. Include them in the review output.

If no tests changed but source changed: defer to Step 0 (test-coverage-gap already flagged).

pgTAP-specific: For SQL test files (supabase/tests/), test-review includes pgTAP-specific checks (transaction isolation, RLS roles, concurrent access).

Step 0.6 — SQL Business Logic Function Check (MANDATORY, always runs)

When reviewing a PR that adds or modifies a Postgres function implementing business logic (transactions, reward claims, referral processing, RLS-guarded lookups): verify the plan or PR includes a SQL-level test — npm run test:db (pgTAP) or execute_sql verification via Supabase MCP. Mocked TypeScript tests cannot verify SQL logic.

Flag absence of SQL-level testing as a blocking P0 issue with check_type: sql-test-gap. Include in the review output:

ISSUE:
  check_type: sql-test-gap
  severity: P0
  location: <SQL function file>
  description: Postgres business logic function added/modified without SQL-level test.
    Mocked TypeScript tests cannot verify SQL logic (transaction isolation, RLS guards,
    RETURN QUERY side effects, auth.uid() behavior). A pgTAP test or execute_sql
    verification is required for any business logic function.
  suggestion: Add a SQL-level test — either a pgTAP test in supabase/tests/
    (run via npm run test:db) or an execute_sql verification block in the PR description
    demonstrating correct behavior for each code path.

Past incidents: referral transaction missing referred-user reward row, RETURN QUERY not setting FOUND, missing auth.uid() guard in get_referidos_links.

Exceptions (do NOT flag):

• The function is purely DDL/schema (CREATE TABLE, ALTER TABLE, INDEX operations)
• The function is a migration helper that runs once and is not callable at runtime
• The PR description explicitly states SQL tests will follow in a separate tracked PR

Trigger pattern: Identify business logic functions by scanning the PR diff for CREATE OR REPLACE FUNCTION blocks containing INSERT, UPDATE, DELETE, or SELECT ... auth.uid(), or function names matching claim_*, redeem_*, process_*, transfer_*, refer_*, reward_*.

Step 0.7 — Content Generation Check (MANDATORY, always runs)

When reviewing a PR that generates or sends content (messages, emails, notifications, templates, push notifications, SMS): verify the execution path reaches the real generation/send function — not a hardcoded fallback, early return, or placeholder string. A function that silently skips real generation and returns a hardcoded string is a bug, not a test gap.

Flag skipped-generation paths as a blocking P0 issue with check_type: content-generation-gap. Include in the review output:

ISSUE:
  check_type: content-generation-gap
  severity: P0
  location: <file path>:<line>
  description: Content generation/send function has a code path that silently
    skips real generation and returns a hardcoded fallback, early return, or
    placeholder string. The execution path never reaches the actual send API.
  suggestion: Trace the execution path from the call site through the
    generation/send function. Verify every branch reaches the real send API
    in production mode. Remove any debug-only early returns that bypass
    actual delivery.

Trigger pattern: Scan the PR diff for functions that send or generate content — look for calls to email APIs (Resend, SendGrid, SES), push notification APIs (OneSignal, Firebase), SMS APIs (Twilio), in-app notification generators, or template renderers. Verify each function's return path reaches the real API call.

Exceptions (do NOT flag):

• Debug/dev-only code paths gated by if (process.env.NODE_ENV === 'development') that clearly log instead of sending
• Test files where mock generation is expected
• Functions whose purpose is to return preview/placeholder content (e.g., email template preview endpoints)

Step 0.8 — Infrastructure File Detection (MANDATORY, always runs)

Skill files, ontology files, templates, and extension code are critical agent pipeline infrastructure. A broken skill breaks the entire agent. These files must NEVER be skipped as "trivial" or "non-code."

Detection:

INFRA_FILES=$(gh pr diff <PR_NUMBER> --name-only | grep -E 'skills/.*SKILL\.md|skills/.*workflow/.*\.md|templates/.*|extensions/.*\.ts|scripts/.*\.(cjs|mjs|js|sh|py)|\.mcp\.json')

Ontology note: the canonical ontology now lives in the tortoise repo (tortoise/docs/ONTOLOGY.md, v3.1) and org data lives in swarm's Supabase SOR — neither is in this repo's PR diff. Ontology changes are reviewed via tortoise-repo PRs; agent-infra skill PRs get ontology review when they touch skills/.*SKILL\.md (vocabulary drift in skill text).

If no infrastructure files detected: INFRA_RISK="" — skip infrastructure reviewer E.

If infrastructure files detected: Classify risk tier based on what changed. The tier
selects depth inside merged reviewer E — it does not select a reviewer count. E is
dispatched whenever INFRA_RISK is set; the tier decides which of E's three sections apply:

Risk	Examples	Depth inside reviewer E
Infrastructure-low	Typo fix in SKILL.md description, comment update, minor wording change	E1 — Skill Infrastructure section
Infrastructure-medium	New Bounded skill, workflow file change, ontology-touching skill change, .mcp.json change	E1 + E2 — Skill Infrastructure + Ontology & Templates sections
Infrastructure-high	New Workflow skill, ONTOLOGY.md § change (tortoise repo), extension code, _template.md change	E1 + E2 + E3 — all three sections (adds Extension Safety)

Classification heuristic:

• PR diff adds a new file matching skills/**/SKILL.md → read the skill to determine if Workflow (high) or Bounded/Modular (medium)
• PR diff modifies ontology-touching lines in skills/**/SKILL.md (entity/term drift) → high
• PR diff modifies extension code (extensions/) → high
• PR diff modifies templates/ → high
• PR diff modifies .mcp.json → medium
• Other infra file changes → read the diff to classify; default to medium if uncertain

Store INFRA_RISK=low|medium|high for use in Step 4 dispatch (it scales depth inside reviewer E; skills, ontology, templates and extensions are never trivial).

Step 0.9 — Cross-PR Overlap Detection (advisory notice, not a severity-gated finding)

Detect open PRs touching the same files as the current PR to surface merge-risk early.
This is an advisory notice — NOT a severity-gated finding. The P2-blocking merge gate
applies to CODE findings; overlap is a coordination signal and never blocks, gates, or
pauses the workflow (conflicts are resolved at merge time).

# Get changed files in this PR
gh pr diff <PR_NUMBER> --name-only > /tmp/pr_files_$$.txt

# Get all open PRs (excluding this one) with their file lists
gh pr list --state open --json number,title,files \
  --jq '.[] | select(.number != <PR_NUMBER>) | "\(.number)|\(.title)|" + (.files[].path)' \
  > /tmp/all_pr_files_$$.txt

# Extract unique files touched by other PRs
cut -d'|' -f3- /tmp/all_pr_files_$$.txt | sort -u > /tmp/other_pr_files_$$.txt

# Find overlap between this PR's files and other open PRs' files
comm -12 /tmp/pr_files_$$.txt /tmp/other_pr_files_$$.txt > /tmp/overlap_files_$$.txt

# For each overlapping file, find which PR(s) touch it
if [ -s /tmp/overlap_files_$$.txt ]; then
  while read -r file; do
    grep -F "|${file}" /tmp/all_pr_files_$$.txt | cut -d'|' -f1,2 | sort -u
  done < /tmp/overlap_files_$$.txt | sort -u > /tmp/overlap_prs_$$.txt
fi

If overlap found: flag as advisory P2 with check_type: cross-pr-overlap. Include in review output:

ISSUE:
  check_type: cross-pr-overlap
  severity: P2
  advisory: true
  description: File(s) also modified in N other open PR(s).
    Overlapping files: <comma-separated list>
    Open PRs touching same files: <PR #N (title), ...>
  suggestion: Review overlap for potential merge conflicts.
    Coordinate with PR authors if changes touch the same logic.

This step never blocks. The review proceeds regardless of overlap count.
Do not pause, gate, or escalate on cross-pr-overlap findings. (The advisory: true flag
on the ISSUE template keeps overlap findings outside the P0/P1/P2 severity-gated merge policy.)
The overlap signal feeds commit-workflow merge condition 5 (04-merge-deploy.md): when overlap
is detected, the merge step refreshes the branch against main and re-runs the affected regression
tests before merging — the safety net for cross-PR breakage without blocking on the signal itself.

Step 0.10 — Process-Weight Check (advisory by default; P1 only on the two A-category triggers)

Run this when the diff ADDS MACHINERY rather than changing behaviour: a new guard/checker
script, a new test file, a new assertion family, a new config surface, or a new CI job. It answers
ONE question — is this the least machinery that makes the behaviour genuinely trustworthy, or is
it accumulation?

This is not an anti-test step. "This adds too many tests" is NOT a valid finding: a test that
proves a behaviour is product work. The judgement is about machinery around behaviour.

The canonical owner of the admission question is AGENTS.md §Product Over Process (hard rule),
implemented as Gate 0 in skills/issue-workflow/SKILL.md. Those two decide the CATEGORY (A/B),
and therefore what may block a merge; this step decides only the MEASUREMENT — what the machinery
costs, what it duplicates, and whether a cheaper seam already owns the invariant. When they
disagree, the A/B rule wins. This step does not create a second severity taxonomy.

Measure first — raw size is the wrong number. This repo is prose-heavy, so a 1,300-line file
can be ~690 code lines and 629 comment/docstring lines. Count non-blank lines that are neither
comments nor docstrings, then take three measurements:

1. Code size — code lines added, and what family it joins (an existing guard's code lines).
2. CI cost — is the new test registered in the repo's own test-shard/surface registry, and
   what is its measured duration? The registry is repo-specific and per test kind, so name
   the one you read: tortoise — config/ci-surfaces.yml; agent-infra — scripts/run-bash-shards.sh
   for .sh shards (its list IS the shard surface; pinned by scripts/check-ci-lane-parity.sh),
   and .github/workflows/ci.yml for the rest — the test job's concrete .py target list, the
   verify job's npx tsx <.test.ts> registrations. A .py/.test.ts file is not in the
   shard list, so a no-match there says nothing about whether it is registered. If the repo has
   none (eldato, premise-labs), say so and drop this measurement — never read "no output" as
   "not registered".
   rg -n "<new_test_file>" <the repo's registry>
3. Duplication — does machinery ALREADY own this invariant? Search the repo's own
   guard/assertion trees and name the trees you searched: agent-infra — scripts/,
   extensions/, tests/; tortoise — tools/, .github/scripts/. A path that does not exist
   makes rg exit 2 and print nothing — that is an unavailable search, not "no duplicate".
   rg -l "<the invariant's key symbol>" <the repo's guard trees> | sort
   Then check whether the new file even names what it overlaps. Zero mentions is a finding.

The two P1 triggers — only these block (both are category A under AGENTS.md §Product Over
Process: each can produce a false PASS or a bypass):

1. A DUPLICATE OWNER exists. Another guard/checker already asserts the same invariant,
   especially when the new one does not name it. Cite the existing file and its clause id.
   If you cannot name both, it is not a duplicate.
2. An anti-accumulation clause already fires. RUN the existing guards this diff touches and
   report their exit codes verbatim. If one goes red because of the new machinery — not because
   of a missing re-cut — that clause IS this project's recorded decision not to accumulate, and
   the diff is re-litigating a decision. Report it as such.

The one advisory trigger — reported, never blocking. Added CI wall-clock is friction, which
AGENTS.md §Product Over Process classifies as category B ("Fail-closed / over-block —
something right is refused"), and "a B item … is still only a note — never routed, never
planned", so it can never hold a merge:

3. It lands on the binding constraint. The machinery adds wall-clock to the critical CI path
   (a shard's max, or a gate every merge waits on). Report the measured delta, and compare it
   to the repo's CI-duration guideline if it declares one — this repo does not gate on one: owner
   ruling #6792 demoted GUIDELINE_CI_MIN out of the verdict precisely because "a slow suite
   is a reason to improve the suite, not a verdict" (scripts/objective.py). Emit it as
   severity: P2, advisory: true — the shape Step 0.9 gives cross-pr-overlap. CI cost is a real
   cost, and the author decides.

OVERRIDES: the common practice of gating a merge on CI wall-clock — a CI-duration figure is
context, never a verdict (owner ruling #6792, 2026-09-30; AGENTS.md §Product Over Process: an
over-block of a correct change is category B).

Otherwise: advisory only. Report the ratio and the measurements; the author decides. Never
block on a lopsided ratio by itself.

What a GOOD finding cites (all three):

• The failure it prevents, pointing at it having happened (incident, red run, lost merge).
  If there is no such failure, say so — that is a removal candidate, not a bug.
• Whether deleting it tomorrow would harm the product or only the process.
• The concrete overlap / complexity / CI number.

What a BAD finding looks like — do not write these:

• "This PR adds 2,000 lines of tests." (size alone is not a finding)
• "We should not add more guards." (a stance, not an argument)
• "CI is slow, so skip the test." Removed verification that prevents a real observed failure is
  not simplification — it is removing a guard rail.

A lopsided ratio is a SMELL pointing at a missing seam, not a verdict. When you report one,
name the seam that would make the behaviour trustworthy by construction — a single authoritative
source, a generated mirror, or a shared helper beats a second checker re-deriving the same
invariant. That seam is usually the real fix; the second checker is the symptom.

Output when triggered:

ISSUE:
  check_type: process-weight
  severity: P1   # only for the two A-category triggers; otherwise P2, advisory: true
  location: <the file/clause the finding is about — the duplicate owner, or the new file>
  description: <the duplicate owner, or the firing clause, or the measured CI delta>
  measurement: code_lines=<n> (of total=<n>); ci_surface=<name> measured=<s>s; overlap=<file + clause id>
  suggestion: <the seam — or "fold into <existing file> and delete <new file>">

Worked example (the case this step was written from, 2026-10-04). A PR adding a
"required-set sync" guard (+3,484/-8) measured: 1,886 code lines added to an existing guard
family of 2,619 (+72%, i.e. near-doubling a heavy family); test_required_set_sync.py
measured 19.0s on the core surface; and it overlapped an existing owner —
tools/mergify_config_guard.py, whose _settings_contexts() already read .github/settings.yml
branch-protection, i.e. the exact surface the new guard's docstring claimed "nothing read". It named
that file zero times. Running the existing guard on the merged tree returned
STATIC RESULT: DIVERGED (exit 1) — clause (viii)/I11, whose stated purpose is "Any OTHER reader
[of the declaration home] is the bug I11 exists to catch" — and that command runs in
python-ci.yml, so the tree was RED. Note the shape of the correct conclusion: the guard's
purpose passed the test (a real recorded incident — a prior surface split silently dropped nine
checks), and the finding was about duplication and size, not about the guard being unnecessary.

Step 1 — Eligibility Check

Dispatch a sub-agent via Pi task to check if the PR (a) is closed, (b) does not need review (automated, trivial, obvious), or (c) already has a code review. If ineligible, stop. Draft PRs are eligible. Infrastructure files (skill files, ontology, templates, extensions, .mcp.json) are NEVER considered trivial — these are critical pipeline infrastructure that must always be reviewed. If infrastructure files were detected in Step 0.8, the PR is always eligible regardless of triviality.

Step 2 — CLAUDE.md File List

Dispatch a sub-agent to list relevant CLAUDE.md file paths: root CLAUDE.md and any in directories the PR modified. Return paths only, not contents.

Step 3 — PR Summary

Dispatch a sub-agent to read the PR diff via gh pr view <N> --json body,title,commits and gh pr diff <N>. Return a summary of the change.

Step 3.5 — Research Brief Resolution

Resolve a research brief if one exists for the linked issue:

ISSUE_NUMBER=$(gh pr view <PR_NUMBER> --json closingIssuesReferences --jq '.closingIssuesReferences[0].number')
if [ -n "$ISSUE_NUMBER" ]; then
  ISSUE_BODY=$(gh issue view "$ISSUE_NUMBER" --json body --jq '.body')
  RESEARCH_PATH=$(bash scripts/_research_path.sh --issue-body "$ISSUE_BODY" --epic-path "")
  if [ -n "$RESEARCH_PATH" ] && [ -f "$RESEARCH_PATH" ]; then
    RESEARCH_KIND=natural
    RESEARCH_CONTENT=$(cat "$RESEARCH_PATH")
  fi
fi

If RESEARCH_KIND=natural, inject before every reviewer sub-agent prompt:

## Verified Research Context (author-provided)

Treat this as authoritative; weigh findings against it. Downgrade reviewer recommendations that contradict it.

{RESEARCH_CONTENT}

---

Step 3.6 — Risk Class, Complexity Ratings + File Surface Detection

Resolve the change's risk class and extract complexity ratings from the linked issue, then detect file surfaces from the PR diff. The class caps the reviewer count; surfaces and ratings select which sections of a merged reviewer's brief apply (Step 4).

0. Resolve the risk class — BEFORE dispatch (do not skip):

Classify the change with proportional-gates §Change Classification (overall risk = the highest dimension), and read the reviewer count and cycle cap from proportional-gates §Review Cycles — the single owner of reviewer counts and cycle caps. Do not restate its cells here. This step must produce one of the four values §Review Cycles' own row labels name, and Step 4 consumes that exact value:

Classification result (overall = the highest dimension)	RISK_CLASS
Highest dimension = Low — docs/CSS/strings only, no runtime change, existing pattern, trivially revertible	low
Highest dimension = Medium, small change following an existing pattern	low-medium
Highest dimension = Medium, large change with some novelty	medium-high
Highest dimension = High — multi-file/shared types/DB/auth, new integration, new pattern, destructive	high

# Resolve RISK_CLASS per the table above and store it for Step 4. It MUST be set —
# Step 4's panel cap reads it, and an unset value would otherwise silently shrink the panel.
# Tier crosswalk (secondary route, proportional-gates §Review Cycles): complexity:micro ->
# low, complexity:standard -> low-medium, complexity:complex -> high. The crosswalk cannot
# produce `medium-high`; that value comes from the two Medium rows in the table above.
RISK_CLASS=<low|low-medium|medium-high|high>

The Low code-impact class is a property of the CHANGE, not of the linked issue (#1348): a docs/stylesheet-only PR linked to a complexity:standard issue is still Low.

1. Extract complexity ratings:

ISSUE_NUMBER=$(gh pr view <PR_NUMBER> --json closingIssuesReferences --jq '.closingIssuesReferences[0].number')
if [ -n "$ISSUE_NUMBER" ]; then
  ISSUE_BODY=$(gh issue view "$ISSUE_NUMBER" --json body --jq '.body')
  UX_RATING=$(echo "$ISSUE_BODY" | grep -oP '(?<=UX_RATING: )\w+' || echo "")
  ARCH_RATING=$(echo "$ISSUE_BODY" | grep -oP '(?<=ARCH_RATING: )\w+' || echo "")
  ONTOLOGY_RATING=$(echo "$ISSUE_BODY" | grep -oP '(?<=ONTOLOGY_RATING: )\w+' || echo "")
fi

Ratings are OPTIONAL — they scale review DEPTH inside a reviewer's section, not trigger and
not count. The PRIMARY trigger is the PR diff surface (step 2): a surface match ALONE
activates the matching section of a merged reviewer at default (medium) depth. If ratings
not found in issue body, check the plan doc or scoping comment.

2. Detect file surfaces from PR diff (surface = PRIMARY trigger, generalized beyond TS):

FILES_CHANGED=$(gh pr diff <PR_NUMBER> --name-only)

# UI surface (UX review) — TS/JS/React AND Python/web/markup
UI_TOUCHED=$(echo "$FILES_CHANGED" | grep -qE '\.(tsx|jsx|vue|svelte|html|htm|css|scss|less)$|(components/|pages/|screens/|templates/)' && echo "true" || echo "false")

# Architecture surface — services/lib/api AND framework dirs (Python: tortoise/, src/, app/)
ARCH_TOUCHED=$(echo "$FILES_CHANGED" | grep -qE '(services/|lib/|api/|src/|app/|tortoise/|core/)' && echo "true" || echo "false")

# Data/Ontology surface — migrations, schema, types, data-model, projection
DATA_TOUCHED=$(echo "$FILES_CHANGED" | grep -qE '(supabase/migrations/|migrations/|schema|types/|models/|projection/|ontology)' && echo "true" || echo "false")

# Config surface — config files, manifests, env templates
CONFIG_TOUCHED=$(echo "$FILES_CHANGED" | grep -qE '\.(yaml|yml|toml)$|(\.mcp\.json|fly\.toml|docker-compose|\.env\.example|config\.)' && echo "true" || echo "false")

3. Section matrix (applied in Step 4) — the surface selects a SECTION, never a reviewer:

The merged reviewer named in each row is dispatched once; the surface flag selects which
of its sections apply — and, through Step 4's priority fill, which surface reviewer occupies
a slot in the capped panel. A surface match must never increase the number of reviewers
(proportional-gates §Review Cycles owns the count; the risk class caps it).

Surface	Default (no rating)	medium rating	high rating	Reviewer
UI_TOUCHED	ux-consistency + ux-coverage sections	same	+ ux-realism section	D
ARCH_TOUCHED	integration + architectural-soundness sections	same	+ contract-completeness section	C
DATA_TOUCHED	schema-correctness section	same	+ ontology-alignment section	C
CONFIG_TOUCHED	config-validation section	—	—	D

Store all variables (RISK_CLASS, UX_RATING, ARCH_RATING, ONTOLOGY_RATING, the four
*_TOUCHED flags, and INFRA_RISK) for Step 4 dispatch.

Whether a matched surface reviewer actually runs is Step 4's panel cap: at the smaller
classes a second surface reviewer may not win a slot. That is the cap doing its job — the
match selected it, the class's count decided the panel — not a silent coverage loss.

---

Step 4 — Parallel Review (the risk class caps the panel; the surface selects which reviewers)

The risk class caps the count of the A/B/C/D panel. The file surface selects which
reviewers fill it — it never decides how many run. Read the count and the cycle cap from
proportional-gates §Review Cycles (the single owner of reviewer counts and cycle caps — do
not restate its cells here) and the class from §Change Classification (RISK_CLASS, resolved
in Step 3.6).

• Fill in priority order, then truncate. B (Correctness & Security) first, then the
  first surface-matched reviewer, then A (Guidance & History), then the second
  surface-matched reviewer — truncated to the class's count. So a surface match can displace
  A at the smaller classes; it can never extend the panel.
• Surface tiebreak: when both surfaces match, C (Architecture & Data) takes the earlier
  slot and D (Surface & Config) is the second — the reviewer-table order below.
• A class whose count exceeds the eligible roles is not padded. With no surface match the
  panel is just B + A, whatever the class — the count is a ceiling, not a quota.
• RISK_CLASS=low → one reviewer round (both of B's bug-scan passes, in order), no re-review
  loop. proportional-gates §Review Cycles gives the Low row the skip sentinel (—) — one
  reviewer pass, no re-review. Both of B's passes run; what Low skips is the re-review loop,
  not the deep caller/callee pass (that pass is skipped only by --standard-tier).
• INFRA_RISK is orthogonal to the panel cap. Reviewer E is appended whenever
  INFRA_RISK is set — at every class, Low included (Step 0.8: skills, ontology, templates
  and extensions are never trivial) — and is never counted against the class's count. The
  largest dispatched set is therefore the class's panel plus E. Surface flags never add E.
• The ratings scale depth inside each merged reviewer. The class does not — it selects
  the panel and its size.

Five merged reviewers, each covering the lenses that were previously separate agents
(11 → 5; for a normal, non-infrastructure PR only the primary set fires):

Reviewer	Covers	Fires when
A — Guidance & History	CLAUDE.md + code-comment compliance; git history/blame; previous PR comments	always (class above Low)
B — Correctness & Security	bug scan — two ordered passes (blind diff scan, then deep caller/callee); security review	always (at the Low class it is the only panel member)
C — Architecture & Data	ARCH_TOUCHED → integration + architectural soundness (+ contract completeness at ARCH_RATING=high); DATA_TOUCHED → schema correctness (+ ontology alignment at ONTOLOGY_RATING=high)	ARCH_TOUCHED or DATA_TOUCHED, and it wins a slot in the class's panel (priority fill above)
D — Surface & Config	UI_TOUCHED → UX consistency/coverage (+ realism at UX_RATING=high); CONFIG_TOUCHED → config validation	UI_TOUCHED or CONFIG_TOUCHED, and it wins a slot in the class's panel (priority fill above)
E — Infrastructure	skill infrastructure; ontology & templates; extension safety — INFRA_RISK low/medium/high scales depth inside this one reviewer	INFRA_RISK is set

Reviewer C's and D's sections are implemented by the catalog skills under
skills/reviewers/ — each section below names the catalog it dispatches to, and that catalog is
the specification (these sections are routing glosses, not a second copy of the checks).

Each selected reviewer runs in parallel via Pi task. Each receives the PR diff, CLAUDE.md
paths, affected files, and research context (if any), and returns ISSUE: blocks or
NO ISSUES FOUND. The ISSUE: block shape and every check_type value are unchanged except
that every block ALSO carries the three disposition fields established at Step 0.1 — origin,
file, outcome (#1537). Say so in the brief: "For every ISSUE you report, add origin:
(diff if it is about a file this PR modifies, process if it is about this PR's description or
this review's process, pre-existing if the code is not part of this change), file: the path you
are citing, and outcome: the wrong outcome this can produce for a user, caller or gate — or
none if you cannot name one." A block without them is dispositioned malformed-blocking
(AGENTS.md → Finding Verification: it must be verified against a primary source anyway).

Dispatch logic (bash; run it as written — the panel cap must be the class's count and
nothing else):

#!/usr/bin/env bash
# ── Reviewer panel selection ───────────────────────────────────────────────────
# The risk class caps the COUNT. The counts below are `proportional-gates` §Review
# Cycles' — that table owns them; this `case` is its implementation, not a second source.
# The surface flags select WHICH reviewers fill the panel — never how many.
# Reviewer E is orthogonal: appended on INFRA_RISK at every class (Step 0.8).

case "$RISK_CLASS" in
  low)         PANEL_MAX=1 ;;   # §Review Cycles row "Low"
  low-medium)  PANEL_MAX=2 ;;   # §Review Cycles row "Low-Medium"
  medium-high) PANEL_MAX=3 ;;   # §Review Cycles row "Medium-High"
  high)        PANEL_MAX=4 ;;   # §Review Cycles row "High"
  *)
    PANEL_MAX=4
    echo "[code-review] RISK_CLASS unset or unrecognised ('${RISK_CLASS:-}') — Step 3.6 must resolve it before dispatch; failing safe to the widest panel (high)"
    ;;
esac

# Surface-matched reviewers — C before D (the reviewer-table order is the tiebreak).
SURFACE=()
if [ "$ARCH_TOUCHED" = "true" ] || [ "$DATA_TOUCHED" = "true" ]; then
  SURFACE+=("Reviewer C (Architecture & Data)")
fi
if [ "$UI_TOUCHED" = "true" ] || [ "$CONFIG_TOUCHED" = "true" ]; then
  SURFACE+=("Reviewer D (Surface & Config)")
fi

# Priority order: B, first surface reviewer, A, second surface reviewer — then truncate.
CANDIDATES=("Reviewer B (Correctness & Security)")
if [ "${#SURFACE[@]}" -gt 0 ]; then CANDIDATES+=("${SURFACE[0]}"); fi
CANDIDATES+=("Reviewer A (Guidance & History)")
if [ "${#SURFACE[@]}" -gt 1 ]; then CANDIDATES+=("${SURFACE[1]}"); fi

PANEL=()
for ((i = 0; i < PANEL_MAX && i < ${#CANDIDATES[@]}; i++)); do
  PANEL+=("${CANDIDATES[$i]}")
done

AGENTS=""
for r in "${PANEL[@]}"; do AGENTS="${AGENTS:+$AGENTS, }$r"; done

# Ratings scale depth INSIDE a reviewer; they never add one.
if [ -z "$UX_RATING$ARCH_RATING$ONTOLOGY_RATING" ]; then
  echo "[code-review] No complexity ratings in issue — surface-matched sections still apply (default depth); ratings scale depth when present"
fi

# Reviewer E — Infrastructure: ONE reviewer, appended whenever INFRA_RISK is set, at EVERY
# class (Low included), because infra files are never trivial (Step 0.8). Orthogonal to the
# panel cap: it is appended after truncation and is never counted against PANEL_MAX.
if [ -n "$INFRA_RISK" ]; then
  AGENTS="${AGENTS:+$AGENTS, }Reviewer E (Infrastructure — depth: $INFRA_RISK)"
  echo "[code-review] Infrastructure files detected (risk: $INFRA_RISK) — dispatching reviewer E; the tier scales depth inside it"
fi

echo "[code-review] RISK_CLASS=${RISK_CLASS:-<unset>} panel_max=$PANEL_MAX — dispatching: $AGENTS"

⛔ A surface match is not a reviewer. UI_TOUCHED/ARCH_TOUCHED/DATA_TOUCHED/
CONFIG_TOUCHED decide what a reviewer looks at and which surface reviewer occupies a
panel slot; RISK_CLASS caps the panel's size and the ratings decide how deep each
reviewer goes. Inverting that — letting a file extension raise the panel past the class's
count — is the defect this section exists to prevent.

Shared dispatch preamble — inject into EVERY reviewer prompt below (#1141):

ISOLATED CHECKOUTS — never copy the repo. If your review needs a checkout other
than the one you are in (e.g. to run a check or probe the PR branch), get it with
    bash scripts/scratch-worktree.sh run --repo <repo> --ref <ref> [--paths <p1,p2> | --full] -- <cmd...>
`run` uses a git worktree (object store SHARED — no second .git) and REMOVES it on
EXIT/INT/TERM/HUP (SIGKILLing a TERM-ignoring child after a bounded grace), so a
crashed probe leaves nothing behind. `--paths` is a sparse checkout (a few KB) —
prefer it whenever you only read some paths. `--paths` is literal: absolute
paths, `.`/`..`, and globs are refused, and an absent path fails rather than
yielding an empty checkout.
BANNED for scratch checkouts: `git clone`, `cp -R`, `cp -r`, `cp -a`, `rsync` of
the repo, `git archive | tar -x` into a temp dir. One measured review loop left 13
copies of ~126 MB each in /private/tmp and drove ~2.4M files of I/O per cycle; the
debris helped take host load to 42.9. If you create a worktree by hand, the trap is
mandatory and MUST re-raise the status and name the repo (a trap that does not
`exit` swallows the signal and keeps running; one without `-C` silently fails once
the probe has changed cwd):
`REPO=<repo>; D="$REPO/.worktrees/scratch-$$"; git -C "$REPO" worktree add --detach "$D" <ref>; GD="$(sed -n 's/^gitdir: //p' "$D/.git" 2>/dev/null)"; trap 'rc=$?; rm -rf "${D:-/nonexistent}" 2>/dev/null || true; if [ ! -e "${D:-/nonexistent}" ]; then git -C "$REPO" worktree remove --force "${D:-/nonexistent}" 2>/dev/null || true; case "${GD:-}" in /*/.git/worktrees/*) rm -rf "$GD";; esac; else echo "leftover ${D:-?} - record KEPT so list shows it" >&2; fi; exit $rc' EXIT; trap 'exit 130' INT; trap 'exit 143' TERM`.
The trap removes the TREE first and only then deregisters: `git worktree remove --force`
drops the record even when it FAILS to delete the directory, so deregistering first would
leave a surviving checkout invisible to `list`, to a second `clean <path>` and to the
reaper (#1141).
Cleanup is TARGETED for a reason: a bare `git worktree prune` deregisters every
record whose directory is not currently stat-able — an unmounted volume, a
permission blip, a stale network mount — so it can silently destroy an unrelated
sibling worktree's checkout while its files sit on disk. Remove your own record
(the trap above reads its own `gitdir:` line); never prune the whole repo.
Before reporting done: `bash scripts/scratch-worktree.sh list --repo <repo> --root <the root you used>` must not show a
scratch worktree of YOURS. Clean only your own path (`scratch-worktree.sh clean
<path>`). A bare `clean --all` refuses to sweep (it cannot see a holder whose argv
does not name the path, so it would delete a sibling's in-flight probe);
`clean --all --force-all` is the deliberate sibling sweep — do not run it while
sibling sessions are running.
The check is ROOT-SCOPED (`$SCRATCH_WORKTREE_ROOT`, else `/tmp`), so a bare `list` is
a false PASS for a worktree created under another root.

Reviewer A — Guidance & History (always-on; absorbs the former Guidance Compliance and Git History + Previous PR Comments agents — both halves walk the same file history):

You cover two lenses over the PR changes: guidance compliance, and history/prior context.

Part A — Guidance compliance.
1. CLAUDE.md guidance — all relevant CLAUDE.md files. Note: CLAUDE.md is guidance for writing code, so not all instructions apply during review.
2. Code comments — IMPORTANT/MUST/keep-in-sync/JSDoc contracts in modified files. Flag violations of binding comments.

Part B — Git history/blame + previous PR comments.
Git history/blame. Read git blame and history of the code modified. Identify bugs visible only in historical context:
- Regressions after a previous fix
- Removed safety checks or guards
- Repeated bugfix attempts that indicate a deeper issue
- Patterns of breakage on these files/lines

Previous PR comments. Find prior PRs that touched the same files. For each prior PR, read its comments. Flag cases where the current PR repeats an issue flagged in a past review on the same files.
Steps:
1. For each affected file: gh api '/repos/{owner}/{repo}/commits?path={file}&per_page=20'
2. Extract PR numbers from commit messages (#NNN)
3. For each prior PR: gh pr view <N> --json reviews,comments
4. Return matches

For each issue found, return:
ISSUE:
  check_type: CLAUDE.md-adherence|comment-compliance|historical-context|pr-comment-history
  severity: P0|P1|P2
  location: <file path>:<line>
  description: <what's wrong; for the history half say "repeated issue from PR #N">
  suggestion: <what to fix>

Reviewer B — Correctness & Security (always-on; at RISK_CLASS=low this reviewer is the whole panel — one reviewer round: BOTH bug-scan passes below run, in order, and the round carries no re-review loop; absorbs the former Bug Scan and Security Review agents):

Part 1 — Bug scan (two passes, in order):

Run BOTH passes in order in this one reviewer. Pass 1 is deliberately blind — do NOT read extra context before it reports; that blindness is load-bearing (it prevents the reviewer talking itself out of a finding). Pass 2 then reads full files plus the call graph. Keep check_type: bug on both; prefix pass-2 findings with [deep].

Pass 1 — Shallow Diff (blind — the diff ONLY):

Shallow scan of the PR diff for obvious bugs. Focus on the changes themselves — avoid reading extra context.
Look for: null pointer dereferences, wrong variable, inverted condition, missing async/await, incorrect API usage.
Ignore likely false positives, pre-existing issues, linter/compiler-detectable issues.

For each issue found, return:
ISSUE:
  check_type: bug
  severity: P0|P1|P2
  location: <file path>:<line>
  description: <what's wrong>
  suggestion: <what to fix>

Pass 2 — Deep (Caller/Callee) — run only AFTER pass 1 has reported:

Deep bug scan — read full changed files plus their import graph. Trace callers and callees. Check for broken contracts, cascading side effects, missing error propagation.

1. Read each changed file in full (not just the diff).
2. Trace imports to identify callers and callees across the codebase:
   git grep -n -e "from '.*<module-path>'" -- '*.ts' '*.tsx'
   git grep -n -e "<symbol>(" -- '*.ts' '*.tsx'
3. Map the call graph around each change. Check for:
   - Broken interface contracts: function signature changes that callers don't handle (changed return type, added/removed parameter, different error shape)
   - Cascading side effects: mutations, events, or DB writes that downstream code assumes won't change
   - Missing error propagation across the call chain
   - Type narrowing that breaks downstream consumers
   - API shape changes (return type, thrown errors) that callers don't handle
   - Removed exports that other modules import
   - Default export ↔ named export changes
4. Ignore likely false positives, pre-existing issues, linter/compiler-detectable issues.

For each issue found, return:
ISSUE:
  check_type: bug
  severity: P0|P1|P2
  location: <file path>:<line>
  description: <what's wrong, prefix with [deep]>
  suggestion: <what to fix>

Part 2 — Security review (keeps its own output section; always-on — absorbed into B, not demoted):

You are the security reviewer for this PR. Apply the security-review skill discipline:
1. Read the security-review skill IN FULL first (resolve via $AGENT_INFRA_PATH/skills/security-review/SKILL.md or skills/security-review/SKILL.md relative to the agent-infra checkout).
2. RESEARCH before reporting: trace where the changed code's inputs come from, check for
   validation/sanitization elsewhere, check config/middleware, note framework protections.
3. Report ONLY HIGH-CONFIDENCE findings: a clear vulnerable pattern WITH attacker-controlled
   input. MEDIUM confidence → "Needs verification" note. LOW/theoretical → do not report.
4. Do NOT flag: test files, dead/commented code, documentation strings, server-controlled
   config values, code requiring prior auth (note the auth requirement instead), or
   pre-existing issues not touched by this PR.
5. Focus areas for this PR: the diff (git show main...HEAD or gh pr diff <N>).

For each issue return:
ISSUE:
check_type: security
severity: P0|P1|P2
location: :
description: <vulnerability, why exploitable, attacker-controlled input path>
suggestion:
If no high-confidence findings: NO ISSUES FOUND

At RISK_CLASS=low this is the whole review: one reviewer round, and BOTH bug-scan passes above run in it, in order — the skipped thing is the re-review loop (proportional-gates §Review Cycles — the Low row's skip sentinel), never pass 2. Return this reviewer's findings and stop.

Reviewer C — Architecture & Data (surface: ARCH_TOUCHED or DATA_TOUCHED — one dispatch; the flags select which sections apply, ratings scale depth inside them; absorbs the former Architecture and Data + Schema agents):

Sections applied per Step 3.6: ARCH_TOUCHED → architecture section; DATA_TOUCHED → data/schema section. When both flags are true, cover both. If neither is true, this reviewer is not dispatched.

Adaptation note — apply both sections to the code diff, not to a planning document:

You are reviewing a CODE PR DIFF, not an epic planning document. Apply your domain expertise to the code changes as if they were the implementation of a plan you validated.

PR DIFF:
<full diff>

---

Section C1 — Architecture (ARCH_TOUCHED; rating absent or medium → integration + architectural-soundness; ARCH_RATING = high → also contract-completeness):

Dispatch to the real reviewer definitions — do not work from the gloss below: read
skills/reviewers/integration/SKILL.md (INT1–INT7) and
skills/reviewers/architectural-soundness/SKILL.md (AS1–AS7, incl. AS3 — a common architectural concern left unaddressed: scalability, observability, security, deployment) in full, plus
skills/reviewers/contract-completeness/SKILL.md (CC1–CC9) at ARCH_RATING=high, and apply
their checks to the diff.

Review the change for integration correctness and architectural soundness:
- integration: does the change fit the surrounding system's contracts, wiring, and call sites?
- architectural-soundness: is the design consistent with the module's established boundaries and responsibilities?
- contract-completeness (ARCH_RATING=high only): are all interface contracts the change touches fully specified and honoured?

Section C2 — Data + Schema (DATA_TOUCHED; rating absent or medium → schema-correctness; ONTOLOGY_RATING = high → also ontology-alignment):

Dispatch to the real reviewer definitions — do not work from the gloss below: read
skills/reviewers/schema-correctness/SKILL.md (SC1–SC9) in full, plus
skills/reviewers/ontology-alignment/SKILL.md (OA1–OA6) at ONTOLOGY_RATING=high, and apply
their checks to the diff.

Review the change for data/schema correctness:
- schema-correctness: are migrations/schema/types correct, ordered, and safe? Do types match the stored shape?
- ontology-alignment (ONTOLOGY_RATING=high only): does the change align with the canonical ontology's vocabulary and entity classes?

For each issue found, return:
ISSUE:
check_type: integration|architectural-soundness|contract-completeness|schema-correctness|ontology-alignment
severity: P0|P1|P2
location: :
description: <what's wrong>
suggestion:

If no issues: NO ISSUES FOUND

Reviewer D — Surface & Config (surface: UI_TOUCHED or CONFIG_TOUCHED — one dispatch; the flags select which sections apply, ratings scale depth inside them; absorbs the former UX and Config agents):

Sections applied per Step 3.6: UI_TOUCHED → UX sections; CONFIG_TOUCHED → config section. When both flags are true, cover both. If neither is true, this reviewer is not dispatched.

Adaptation note — apply the UX sections to the code diff, not to a planning document:

You are reviewing a CODE PR DIFF, not an epic planning document. Apply your domain expertise to the code changes as if they were the implementation of a plan you validated.

PR DIFF:
<full diff>

---

Section D1 — UX (UI_TOUCHED; rating absent or medium → ux-consistency + ux-coverage; UX_RATING = high → also ux-realism):

Dispatch to the real reviewer definitions — do not work from the gloss below: read
skills/reviewers/ux-consistency/SKILL.md (UXN1–UXN6) and skills/reviewers/ux-coverage/SKILL.md
(UXC1–UXC6, incl. UXC5 — focus management, screen-reader announcements, reduced-motion) in
full, plus skills/reviewers/ux-realism/SKILL.md (UXR1–UXR7, incl. UXR6 — device constraint)
at UX_RATING=high, and apply their checks to the diff.

Review the UI change through the UX lenses:
- ux-consistency: does the change follow the established component/pattern conventions?
- ux-coverage: are the relevant states, breakpoints, empty/error paths and edge cases covered?
- ux-realism (UX_RATING=high only): does the change hold up against realistic content, data volumes and user flow?

Measuring line-level layout (whenever the change affects text wrapping, line boxes, widows/orphans or
horizontal overflow). Range.getClientRects() returns one rect per line fragment, so a word broken at its
hyphen (AI-forward → head AI- on one line, tail forward on the next) yields two rects. A read that
takes getClientRects()[0] keeps only the head and attributes the word to its head's line — silently, with a
precise-looking number that can invert the verdict: on daniel-ospina/tortoise#8657 a last line was reported
at 84px where it renders at 152px.

So read all rects, never [0] — and before any line-level number goes into a finding, confirm it against
a screenshot of the rendered block, the artifact a reader can check. A bad line-level number cannot be
told from a good one by re-reading it, which is why it needs a check that does not come from the same
reading. Report the method alongside the numbers.

Section D2 — Config (CONFIG_TOUCHED):

You are the config reviewer for this PR. Apply the config-validation skill discipline
(read skills/config-validation/SKILL.md if available):
1. Config changes are validated by the mapped check script for their file type
   (migrations → check-migration-*, skills → check-skill-lint, etc.).
2. Verify: env var names match what the code actually reads; no secrets in configs
   (placeholders only); default values are safe (fail-closed, not fail-open);
   config is internally consistent (e.g. .env.example vs .mcp.json vs defaults).
3. Report only concrete, actionable issues.

For each issue found, return:
ISSUE:
check_type: ux-consistency|ux-coverage|ux-realism|config-validity|config-consistency|secret-leak|insecure-default
severity: P0|P1|P2
location: :
description: <what's wrong>
suggestion:

If no issues: NO ISSUES FOUND

Reviewer E — Infrastructure (conditional: dispatched only when INFRA_RISK is set; ONE reviewer. INFRA_RISK low/medium/high scales depth inside E — it selects which of E1/E2/E3 apply, never a reviewer count. Absorbs the former Skill Infrastructure, Ontology & Templates, and Extension Safety agents):

INFRA_RISK	Sections that apply
low	E1
medium	E1 + E2
high	E1 + E2 + E3

Section E1 — Skill Infrastructure (INFRA_RISK = low and above):

You are reviewing a PR that changes skill infrastructure files. These files control agent behavior — errors here break the entire pipeline.

PR DIFF: <full diff>
INFRA_FILES: <list of infrastructure files from Step 0.8>

CHECK:

1. GATE WARNING (mandatory — every SKILL.md):
   - Does the SKILL.md have a gate warning block?
   - Acceptable patterns: "MUST be read in full", "mandatory before", "do not skip", "hard gate"
   - Missing gate warning in a skill that has quality gates → P0

2. CONTINUITY DIRECTIVE (mandatory — every SKILL.md):
   - Per skill type taxonomy (#5900):
     - Workflow skills → "as mandated by this skill" (or equivalent)
     - Bounded skills → "as mandated by the orchestrator skill" (or equivalent)
     - Modular skills → no continuity directive needed
   - Wrong or missing continuity directive → P1

3. FRONTMATTER VALIDITY (mandatory):
   - Required fields: `name`, `description`, `allowed-tools`, `version`
   - Missing required field → P0
   - Malformed YAML frontmatter → P0

4. BROKEN REFERENCES:
   - Does the skill reference other skills that don't exist?
   - Does it reference deprecated/archived paths?
   - Does it reference files that don't exist?
   - Broken reference → P1

5. SKILL TYPE CHECKS (domain-aware):
   - **Workflow skills:** Sequence correctness — do phases execute in the right order? Are handover contracts clear between phases? Are gate placements correct (before dangerous operations)?
   - **Bounded skills:** Is the orchestrator dependency clear? Are input/output contracts defined? Does it integrate correctly with the parent workflow's review gates?
   - **Modular skills:** Is it independently invocable? Are there no broken cross-skill assumptions? Is the interface self-contained and reusable?

For each issue found, return:
ISSUE:
  check_type: gate-warning|continuity-directive|frontmatter|broken-reference|sequence-correctness|handover-contract|gate-placement|orchestrator-dependency|io-contract|review-gate-integration|standalone-invocability|cross-skill-assumption
  severity: P0|P1|P2
  location: <file path>:<line>
  description: <what's wrong>
  suggestion: <what to fix>

If no issues: NO ISSUES FOUND

Section E2 — Ontology & Templates (INFRA_RISK = medium and above):

You are reviewing a PR that changes skill infrastructure, templates, or extension code. These define the canonical vocabulary and agent pipeline for the project. The canonical ontology lives in the tortoise repo (`tortoise/docs/ONTOLOGY.md`, v3.1 — fetch: `gh api repos/daniel-ospina/tortoise/contents/docs/ONTOLOGY.md --jq .content | base64 -d`, §5 = controlled vocabulary) and org data (teams/roles) lives in swarm's Supabase SOR. Neither is in this PR's diff — compare skill text against the fetched ontology.

PR DIFF: <full diff>
INFRA_FILES: <list of infrastructure files from Step 0.8>

CHECK:

1. VOCABULARY CONSISTENCY:
   - Does the change introduce terms that conflict with tortoise/docs/ONTOLOGY.md definitions?
   - Does the change use canonical entity names from the ontology's §5 controlled vocabulary?
   - New term without ontology registration → P1

2. ONTOLOGY DRIFT:
   - Does the change alter the meaning of an existing concept without updating downstream references?
   - Changed entity class but old name used elsewhere → P0
   - Semantic shift without changelog → P1

3. DOWNSTREAM IMPACT:
   - What other files reference the changed term/concept?
   - Are those references now stale?
   - Stale downstream reference → P1

4. TEMPLATE VALIDITY (for templates/ changes):
   - Are all required frontmatter fields still present?
   - Does the template match the filing workflow spec?
   - Missing required field → P0

5. ORG-DATA CONSISTENCY (team/role slugs in skill text):
   - Team/role slugs should match swarm's Supabase SOR (teams/roles tables) — verify via `node scripts/swarm-org.mjs list-teams` / `resolve-role <slug>`
   - Invalid slug → P1

For each issue found, return:
ISSUE:
  check_type: vocabulary-conflict|ontology-drift|downstream-impact|template-validity|subject-registry
  severity: P0|P1|P2
  location: <file path>:<line>
  description: <what's wrong>
  suggestion: <what to fix>

If no issues: NO ISSUES FOUND

Section E3 — Extension Safety (INFRA_RISK = high only):

You are reviewing a PR that changes Pi extensions or MCP server configuration. These run in the agent's process — failures are silent and break agent capabilities.

PR DIFF: <full diff>
INFRA_FILES: <list of infrastructure files from Step 0.8>

CHECK:

1. RUNTIME SAFETY:
   - Does the extension handle errors gracefully or fail silently?
   - Are there uncaught exceptions?
   - Silent failure path → P0

2. ERROR HANDLING:
   - Does every operation have a catch/error boundary?
   - Are errors logged or surfaced to the user?
   - Missing error handler → P1

3. NO SILENT FAILURES:
   - If the extension can't load, does the agent know?
   - If an MCP server is unreachable, is it reported?
   - Silent degradation → P0

4. CONFIGURATION VALIDITY:
   - .mcp.json: valid JSON? All required fields (name, command, args)?
   - Extensions: correct manifest structure? Required exports present?
   - Invalid config → P0

For each issue found, return:
ISSUE:
  check_type: runtime-safety|error-handling|silent-failure|config-validity
  severity: P0|P1|P2
  location: <file path>:<line>
  description: <what's wrong>
  suggestion: <what to fix>

If no issues: NO ISSUES FOUND

Step 4.5 — Merge-Dedup Bug Scan Results

Reviewer B's pass 1 (blind diff scan) and pass 2 (deep caller/callee) may find overlapping or complementary bugs. After all reviewers return, merge their results:

1. Dedup by location: If both passes flag the same location (file:line), keep only the more severe issue (prefer P0 > P1 > P2). If severity matches, prefer the deeper analysis (pass 2).

2. Complement check: For each pass-2 issue that targets a caller/callee outside the diff, cross-reference with pass-1 results. If pass 1 flagged the same root cause at the source location, merge into a single issue with both locations cited.

3. Output: Produce a deduplicated, merged list of check_type: bug issues for confidence scoring in Step 5. Document merge decisions:

   MERGE: pass 1 location X + pass 2 location Y → merged (same root cause, source at X, side effect at Y)
   SKIP: pass 1 location Z (superseded by pass 2 deeper finding at same location)
   KEEP: pass 2 location W (unique deep finding, no shallow overlap)

Return the merged bug issues alongside Reviewer A, C, D and E issues unchanged.

Step 5 — Confidence Scoring

For each issue found in Step 4, dispatch an isolated Pi task sub-agent to score confidence. Each sub-agent gets:

• The PR number, issue description + location, CLAUDE.md file paths, and this rubric:

Score on a scale from 0–100:
a. 0: Not confident. False positive or pre-existing issue.
b. 25: Somewhat confident. Might be real but couldn't verify.
c. 50: Moderately confident. Real issue but may be a nitpick.
d. 75: Highly confident. Very likely a real issue that will be hit in practice. Important.
e. 100: Absolutely certain. Definitely a real issue, frequent in practice.

Return ONLY the number.

Dispatch strategy: Parallel for ≤15 issues, serial with 100ms stagger for >15. Extract score with regex \b([0-9]{1,3})\b, clamp to 0-100, default to 0 on parse failure.

Step 6 — Filter & Fix

Filter out issues with score < 50. If none survive → go to Step 9 (Logging), then Step 10 (record the verdict), then stop.

Fixer configuration — read operations/ai-workflow-tools/config.json (defaults if missing):

• fixer_enabled: default true
• max_fix_cycles: REMOVED — no cycle cap (convergence-gated instead)
• stall_threshold: default 0.8

If fixer_enabled == false OR --re-review is active → skip fixer loop, go to Step 7.

Gate Loop — MANDATORY (Fixer + Re-Review)

Each fix cycle dispatches a FRESH fixer sub-agent, and each re-review dispatches
FRESH reviewer sub-agents. Neither has memory of prior cycles — this prevents
confirmation bias.

Simplified Fixer Loop:

Serialize surviving issues to JSON: [{"severity":"P1","origin":"diff","file":"...","outcome":"...","location":"...","description":"...","suggestion":"..."}]

The disposition fields travel with the finding from here on (origin / file / outcome, #1537):
the Gate Loop's filter reads them, and a missing origin/outcome means the loop treats that
finding as blocking — exactly the pre-#1537 behaviour, never a silent deferral.

⛔ A finding is a CLAIM, not a fact — the fixer VERIFIES before it applies (AGENTS.md →
"Finding Verification"). Dropping this step is how a false finding ships: a missed finding leaves the
code as it was, but a false correction changes it and marks the change as an improvement (#5014).
The fixer reports an unverifiable or falsified finding back instead of "fixing" it.

For each cycle:

1. Dispatch fixer sub-agent via Pi task (fresh pi -p session):

   Fix these code-review issues in the current branch. Do NOT create a worktree —
   your working directory IS the controller-provided worktree for the PR branch (#626);
   never run `git checkout` to change branches inside a hub main checkout.
   
   Issues to fix:
   <surviving issues JSON>
   
   Affected files: <list from PR diff>
   PR branch: <branch name>
   
   1. VERIFY FIRST — a finding is a CLAIM, not a fact: for EACH issue, open the primary source it CITES (the line range, the file, the `gh` output, the commit) and confirm the source actually says what the finding says it does. The finding's prose is not the source. If it does NOT hold: do NOT change the code — report it back as `FALSIFIED: <issue> — <what the source actually says>` and exclude it from FILES_WRITTEN. If it cites NOTHING that can be checked, report `UNVERIFIABLE: <issue>` and do not apply it. A `⚠️ CORRECTED` marker in the finding is not evidence — it records that text changed, not that the change was right. **A decline does not clear the gate**: the fresh scan decides, and you should decline rather than apply a finding you have just verified as false.
   2. RESEARCH FIRST: For each VERIFIED issue involving external APIs, library behavior, or unfamiliar patterns → run web_search to verify the correct approach before fixing. Skip only for purely internal issues. No query cap — mistakes cost more than queries.
   3. Confirm the PR branch is checked out in your worktree — if not, STOP and report; do NOT `git checkout <branch>` in a hub (#626)
   4. For each VERIFIED issue, make the minimal fix (using research findings)
   5. Commit with message: fix(code-review): automated fixer cycle N — PR #<N>
   6. Push
   
   Return FILES_WRITTEN: <comma-separated>, STATUS: done|failed, and any FALSIFIED:/UNVERIFIABLE: lines.

   Fold any FALSIFIED: / UNVERIFIABLE: lines the fixer returned back into the surviving
   issues — a declined finding is NOT a resolved one. It must reach the PR comment and the
   re-review, and it can never be counted toward a clean exit: the gate clears only when a fresh
   reviewer returns zero issues (see references/fixer-loop.md: the Gate-clearing rule and the L4
   no-change branch, which falls through to the fresh scan instead of exiting clean — exactly for
   this reason).

2. Re-review: Run --re-review on the new commits. This dispatches FRESH reviewer
   sub-agents via task — they see only the current code, not what was "just fixed."

3. Stuckness detection (3-layer algorithm):

   a. Fingerprint-stall: Hash each surviving issue's location + severity (SHA256) — the defect's stable identity, never its description/suggestion. Wording is the part a fresh, memoryless reviewer varies; hashing it made an identical defect re-described in new words hash differently, so recurrence read ~0 on a cycle where nothing was fixed — the most common real stall, and invisible to the primary detector. Recurrence is |current ∩ prev| / |prev| — "what fraction of last cycle's issues came back?". |prev| is the correct denominator and the only one: it is the stall signal, so a cycle that repeats all 10 prior issues must score 1.0. (A symmetric denominator — max(|current|, |prev|) — inverts this: repeating all 10 prior issues plus 5 new ones scores 10/15 = 0.67, below threshold, so the loop reads as ordinary churn on a cycle where nothing was resolved. Do not use it.) If recurrence ≥ stall_threshold → escalate to human with stuck issues and attempted fixes. Do NOT auto-exit.

   b. Honest-stuck: Track blocking_per_cycle — the count of findings whose disposition is
   blocking, not the total (issues_per_cycle is still recorded, but a loop that keeps filing
   non-blocking follow-ups is converging, not stuck; #1537). This is an INDEPENDENT signal, not a refinement of recurrence — it fires when the count is non-decreasing for 3 consecutive cycles, regardless of recurrence. Gating it behind recurrence ≥ stall_threshold was itself a regression: a loop that churns one-for-one (every issue fixed, a new one appearing in its place) has low recurrence but is not converging, and the gate left it undiagnosed until the cycle cap. Exit reason = honest-stuck. Escalate to human.

   ⚠️ Fire if recurrence ≥ stall_threshold OR blocking_per_cycle is non-decreasing for 3 consecutive cycles. Either is sufficient on its own. The two tests exist because they catch different pathologies — recurrence catches the same issues coming back, the count test catches the loop not shrinking, however caused. Recurrence is used only to pick the label (honest-stuck when the count is not shrinking, else fingerprint-stall); it is never a precondition for the count signal. The executable form lives in references/fixer-loop.md; that file and this section must agree.

   c. Zero-progress: Track files_changed_per_cycle. If files changed = 0 for 2 consecutive cycles, the fixer is making zero code progress. Exit reason = zero-progress. Escalate to human. (Previously mapped onto fingerprint-stall, which made three distinct conditions indistinguishable in the persisted record.)

Adversarial domain — declared threat surface (bound: 2 cycles). Engages when the scoping comment carries an ### Adversarial Threat Surface declaration (gate/enforcement code: argv/path/symlink resolution, working-tree discard, merge/verify gates — anything whose correctness is "an attacker cannot make it fail open").

• Acceptance = every in-scope threat class has a test that fails without the fix, plus green CI — not "the reviewer ran out of ideas". The scoping declaration is the reference: the reviewer verifies each listed class is covered and tries to reproduce an in-scope bypass.
• Clean-exit verdict: a fresh reviewer returning THREAT SURFACE COVERED (all declared classes covered, no in-scope bypass reproduced) satisfies the clean-exit conditions below. A literal NO ISSUES FOUND is not required — never manufacture one.
• Cap: at most 2 cycles. Enter the loop with ADVERSARIAL_BOUND=1 exported (the reference's
  pre-loop setup reads it; unset → the change's own class cap). It tightens the class's cap and
  never widens it, so High → 2 and a tighter class cap (e.g. low's —) stays as it is. Findings outside the declared surface are filed as issues and not chased; in-scope findings surviving cycle 2 are filed and recorded, not iterated.
• Disclose: when the merge rests on threat-list coverage rather than NO ISSUES FOUND, the PR body must carry [ADVERSARIAL-BOUND] cycles=<N> threats=<K> covered=<K> residuals=<#N,…|none> and the report must say so in plain words. Exit reason: adversarial-capped.

Finding disposition — what BLOCKS, what is a FOLLOW-UP (#1537)

A finding is not automatically in scope, and severity is not a category. Two rules, applied to
every surviving finding BEFORE it reaches the fixer. The executable form is
extensions/loop-enforcer/review-scale.ts::dispositionOf — this section is the rule it implements;
review-scale.test.ts pins both directions (a correctness finding still blocks; a prose-about-process
finding does not).

Rule A — the consequence decides, not the letter. A finding is non-blocking by default. It
becomes blocking only when its author states the wrong outcome it can produce. Ask: what goes
wrong for a user, a caller, or a gate if this ships? If the answer is "a document is imprecise",
"the wording overlaps", or "this could be tidier", it is a follow-up, not a blocker — record it,
file it, and do not start another round for it.

Rule B — a finding must be attributable to the change. Every finding declares where it lives:

origin	Meaning	Disposition
diff	about a file this PR modifies (checked against gh pr diff --name-only)	blocking iff Rule A is met and severity is not P3 (so P0/P1/P2 block, and an absent or unknown severity blocks too — fail-closed)
process	about the change's own description, commit message, or prose about this review process	follow-up — resolved by DELETION or a pointer to a checkable artifact, never by another wording round
pre-existing	code the change does not touch	follow-up — file an owned issue (origin-keyed deferral)

⛔ The scope check is mechanical, not a judgement call. A finding that claims origin: diff but
cites a file that is not in the PR's changed-file set is reclassified as pre-existing and
becomes a follow-up — a reviewer cannot claim the change for a file the change does not touch. A
finding that cites no file at all is likewise not attributable: per AGENTS.md → "a finding that
cites nothing is not applicable", it is recorded and reported back as unverifiable — never applied,
never blocking. An absent origin or an absent outcome is different: that is malformed, and it
is treated as BLOCKING until the reviewer states it — absence is never a pass (fail-closed by construction, review-scale.ts). The one field whose absence defers is file, and only because an
unverifiable finding cannot be applied at all.

Where the fields come from — the reviewer's contract, not the loop's guess. Every ISSUE: block
carries three fields alongside severity/location (the shape is in Step 0's coverage block and in
each reviewer's brief): origin (diff | process | pre-existing), file (the path the finding
cites — required for origin: diff), and outcome (the wrong outcome, or none). Without them the
disposition is malformed-blocking and the loop behaves exactly as it did before #1537 — it cannot
get shorter by omission. origin: pre-existing / process findings are reported in the review
comment as follow-ups, with the issue number once filed.

What this does NOT do. It is not a ban on prose, and not a ban on reviewer-requested work. Code
comments that misdescribe behaviour carry a measured ~1.5× odds of a bug-introducing commit, so a
comment inside a changed file that names a wrong outcome still blocks. A reviewer may still ask
for anything; what changes is that the ask is filed as an owned issue unless it names the outcome
the merge would ship. Every follow-up must name an issue number — never a TODO, never "noted"
(the deferral queue is a graveyard otherwise: ~46.7% of TODOs are low-quality).

Exit conditions — ALL must be true before proceeding to Step 8:

• Last --re-review returned zero BLOCKING findings — zero findings, or findings whose
  disposition (above) is follow_up and which are filed as owned issues and named in the
  review comment
• If cycle 1 found any issues → at least 1 re-review cycle completed
• Cycle log posted: each cycle's issues, fixes, dispositions, and re-review results documented
• Adversarial domain only: a fresh reviewer returned THREAT SURFACE COVERED (every declared threat class test-covered, no in-scope bypass reproduced) — this substitutes for the first box

RISK_CLASS=low does not enter this loop. proportional-gates §Review Cycles gives the
Low row the skip sentinel (—): one reviewer round (both of B's bug-scan passes, in
order), no re-review loop — Step 4 dispatches reviewer B (plus E if INFRA_RISK is set)
and nothing else. So for a Low change the exit conditions above are not the gate — B's
single round is the whole budget. If that round returns no blocking findings, record the verdict
clean (Step 10): clean attests that the review ran at this head and found nothing that
blocks — not that a re-review loop converged — so a single-round Low review satisfies it (see
Step 10's class note). Non-blocking findings from that round are filed as owned follow-ups and
named in the review comment. If the round returns a blocking finding, post it with the fix
(Step 8) and escalate to a human; do not run a re-review loop the — cell never budgeted.

The ceiling is the change's own — it is not a flat 10 (#1537). The loop's bound is the cap
proportional-gates §Review Cycles assigns to this change's RISK_CLASS — that table owns the
numbers, Step 3.6 already resolved the class before dispatch, and this file does not restate its
cells. extensions/loop-enforcer/review-scale.ts::cycleCapForClass is the executable
reader and tier-config-parity.test.ts pins it to that table. What happens at the ceiling is
decided by the disposition rule above, not by the count:

• No blocking finding remains → LAND. File the follow-ups, name their issue numbers in the PR
  comment, put [PROPORTIONAL] cycles=<N> cap=<B> blocking=0 follow-ups=<#issue,…|none> in the PR
  body, record the verdict (Step 10). The round does not start another round — that is the
  stopping rule, and it is what ended the 9- and 13-cycle loops.
• A blocking finding remains → ESCALATE, naming each blocker and the wrong outcome it produces
  (⚠️ Auto-fix reached this change's proportional cycle ceiling (N of B cycles) — blocking issues remain; escalate to a human). Exit reason proportional-capped at the tighter rows,
  cycle-cap at the High row's own 10. ⛔ A ceiling is never a landing
  permission: landing with an unresolved blocking finding is forbidden at every cycle number, and a
  ceiling exit is never reported as clean (AGENTS.md §Hard Cap).
• The adversarial domain's own bound is 2 (above) — it can only tighten the class's cap, never
  widen it, so High + adversarial = 2, not 20.

The loop is not entered at all when nothing blocks. The Gate Loop's first act is to disposition
the Step 6 findings; if none is blocking, the round that already ran (Step 4) is the whole review, the
follow-ups are filed, and the change lands with [PROPORTIONAL] cycles=0 cap=<B> blocking=0 … (cycles counts LOOP cycles:
a pre-loop landing spends none, because Step 4's round is the one that decided it). That
is the 9-cycle case fixed at its source: the first round decides, and a follow-up never buys a second.

A review round cannot be skipped, only repeated. decideRound() returns continue, never
land, at zero cycles spent — nothing lands on an unreviewed head. The #485 ≥1-dispatch floor, the
fresh-context re-review requirement and the ai-review-gate evidence record (Step 10) are untouched
by this rule: it bounds the repeat, not the review.

Convergence rule: If re-review issues are a strict subset of the previous cycle's issues (no new dimensions or files flagged), the fixer is in a refinement loop. Log convergence and escalate to human: present remaining issues with attempted fixes. Do NOT auto-exit — remaining issues must be acknowledged by a human before proceeding.

Stall guard: there is no separate fixed-cycle stall rule. "The same issue survives N cycles" is exactly what the 3-layer detection above measures — recurrence over a wording-invariant fingerprint, plus the count signal. Do not restate it here as a cycle count; a second, weaker statement of the same rule is how the loop ends up with two different answers to "are we stuck?".

Exit outcomes:

• STATUS=failed → fixer couldn't push. Post with ⚠️ Auto-fix failed (push error) — N issues remain.
• Stall detected → Escalate to human with stuck issues. Do NOT auto-exit.
• Convergence reached → Escalate to human with remaining issues (shrinking, no new). Do NOT auto-exit.
• Clean → no blocking findings after re-review. Proceed to Step 8 with "No issues found."
• Landed on the proportional ceiling (proportional-capped with blocking=0) → proceed to Step 8:
  post the review with the follow-ups and their issue numbers, note the [PROPORTIONAL] line in
  the PR body, then record the verdict (Step 10). Report it as a landing bounded by the disposition
  rule, never as a NO ISSUES FOUND clean — the follow-ups exist and are named.

Cycle-status YAML: Write operations/logs/cycle-status.yaml on loop exit:

exit_reason: <clean|fingerprint-stall|honest-stuck|zero-progress|cycle-cap|proportional-capped|tool-unavailable|pr-closed|git-error|push-failed|adversarial-capped|convergence>
cycles: <N>
issues_per_cycle: <json array>
blocking_per_cycle: <json array>       # the count that actually drives the loop (#1537)
follow_ups: <json array>               # {severity,file,outcome,reason} — filed, never chased
risk_class: <low|low-medium|medium-high|high>
cycle_cap: <int>                       # this change's cap, from proportional-gates §Review Cycles
files_changed_per_cycle: <json array>
detector_fired: <fingerprint-stall|honest-stuck|zero-progress|''>   # which layer fired, or empty
fingerprint_recurrence_last_cycle: <0.0-1.0|null>   # the predicate's actual input; null on a zero-progress exit, which fires before the scan
skill: code-review

cycle_cap and risk_class are recorded because a bare cycles: N cannot be audited. The flat
10 made every exit look in-budget; the class-keyed ceiling only means something if the record says
which ceiling applied. blocking_per_cycle is the same point one level down — issues_per_cycle
counts everything, and everything is what used to drive the loop.

detector_fired is never empty when exit_reason names a detector — zero-progress sets it too. An empty detector_fired alongside a detector exit reads as "no layer fired", which is the opposite of what happened.

convergence is the one agent-judged exit — "the remaining issues are a strict subset with no new dimensions" is a reading of the issue set, not something the classifier emits. Every other value in this enum is written by the loop itself. Do not add a value here that nothing produces: a never-emitted enum member reads as a live exit path and hides the fact that the loop exits somewhere else.

Why detector_fired and fingerprint_recurrence_last_cycle are recorded: without them, "why did this loop exit?" cannot be answered after the fact — the predicate's inputs are gone and only the verdict survives. A run that exited fingerprint-stall under the old asymmetric denominator could not be distinguished from a genuine recurring set.

This file enables cross-session staleness detection and loop enforcer integration.

Critical: remaining issues are NEVER dropped. They are always included in the PR comment body alongside the warning prefix. The fixer loop exits, but the issues persist in the review.

FORBIDDEN — these bypass the quality gate entirely:

• ❌ Run fixer → push → declare done without re-reviewing
  Fixing without re-reviewing = no review. Always run --re-review after fixes.

• ❌ Self-declare "the fix should address the issue" as completion
  Only a clean re-review (zero blocking findings — see the disposition rule) is a valid exit signal.

• ❌ Spend another round on a non-blocking finding
  A finding with no named wrong outcome, a process-origin finding, or a finding in a file this PR
  does not touch is a follow-up issue: filed, named, and never chased with another round (#1537).

• ❌ Re-review in the same conversation context
  Confirmation bias makes same-context re-review unreliable.
  Always use --re-review which dispatches fresh task sub-agents.

Step 6.5 — Orchestrator Escalation (unattended recovery)

When the fixer loop exits with remaining issues (stall/cap/fail), the orchestrator (you, the main session) takes over BEFORE posting the PR comment:

1. Read the remaining issues — understand what the fixer couldn't solve
2. Dispatch a deep-fix sub-agent via Pi task:

   The automated fixer couldn't resolve these code-review issues after N cycles. You are the escalation path with FULL context and research capability.
   
   Remaining issues:
   <surviving issues JSON>
   
   PR branch: <branch>
   Affected files: <list>
   
   1. For EACH issue: research first — use web_search to understand the correct approach before touching code
   2. Checkout the PR branch
   3. Fix each issue with the correct, researched approach
   4. Commit with message: fix(code-review): deep escalation fix — PR #<N>
   5. Push
   6. Return FILES_WRITTEN and list which issues were resolved vs which remain unresolved with a brief explanation why

3. If the deep-fix sub-agent resolves all issues → re-review, then post "No issues found"
4. If issues still remain → THEN post the PR comment with the warning + remaining issues. These are the genuinely hard problems that need human judgment. Add a ### Requires Human Attention section explaining what was tried and why each issue couldn't be auto-resolved.

Pause only when: a taxonomy-matching decision arises (human input taxonomy). Everything else proceeds unattended.

Step 7 — Eligibility Re-check

Dispatch a sub-agent to re-check eligibility (same as Step 1).

Step 8 — Post Comment

Use gh pr comment to post the review. ALL surviving issues must be included — even when the fixer loop exited with remaining issues.

Capture the comment's URL — gh pr comment PRINTS it on success, and Step 10 must NAME it as the review artifact (--evidence comment:<id>). Without the capture, Step 10 has no artifact to name and the record is not written:

REVIEW_URL="$(gh pr comment <PR_NUMBER> --body-file <review-comment-file>)"
echo "review comment: $REVIEW_URL"   # keep this URL — Step 10 names its id

If fixer loop stalled or fixer failed (issues remain):

⚠️ Auto-fix [stalled after N cycles | failed: <reason>] — M issues require human attention

### Code review

Found M issues:

1. <brief description> (<source>)
<link to file with full sha + line range>
...

🤖 Generated with [Pi](https://pi.dev)
<sub>- If this code review was useful, please react with 👍. Otherwise, react with 👎.</sub>

If clean (no issues, or all fixed):

### Code review

Found N issues:

1. <brief description> (<source>)

<link to file with full sha + line range>

2. ...

🤖 Generated with [Pi](https://pi.dev)

<sub>- If this code review was useful, please react with 👍. Otherwise, react with 👎.</sub>

Or if no issues:

### Code review

No issues found. Checked for bugs, guidance compliance, historical context, and prior PR comments.

🤖 Generated with [Pi](https://pi.dev)

Link format: https://github.com/<owner>/<repo>/blob/<full sha>/<file>#L<start>-L<end>. Require full SHA, provide 1 line of context before and after the issue.

Step 9 — Logging (Supabase)

Best-effort logging — never block the workflow. For each issue ≥50 confidence:

JSON=$(python3 -c "
import json
from datetime import datetime, timezone
entry = {
  'ts': datetime.now(timezone.utc).isoformat(),
  'skill': 'code-review',
  'version': '2.0.0',
  'pr_number': <PR_NUMBER>,
  'pr_author': '<AUTHOR>',
  'file_path': '<FILE>',
  'check_type': '<TYPE>',
  'severity': '<SEVERITY>',
  'problem': '<≤200 char description>',
  'snippet': '<≤150 char code>',
  'confidence': <SCORE>
}
print(json.dumps(entry, ensure_ascii=False))
")
curl -s -o /dev/null -X POST \
  'https://axaeagulqhanatyoxrdv.supabase.co/rest/v1/code_review_errors' \
  -H 'apikey: <KEY>' \
  -H 'Authorization: Bearer <KEY>' \
  -H 'Content-Type: application/json' \
  -H 'Prefer: return=minimal' \
  -d "[$JSON]" \
  2>/dev/null \
|| (mkdir -p operations/logs && echo "$JSON" >> operations/logs/code-review-errors.jsonl)
true

Check types: CLAUDE.md-adherence, comment-compliance, bug, historical-context, pr-comment-history, security, sql-test-gap, content-generation-gap, gate-warning, continuity-directive, frontmatter, broken-reference, sequence-correctness, handover-contract, gate-placement, orchestrator-dependency, io-contract, review-gate-integration, standalone-invocability, cross-skill-assumption, vocabulary-conflict, ontology-drift, downstream-impact, template-validity, subject-registry, runtime-safety, error-handling, silent-failure, config-validity, ux-consistency, ux-coverage, ux-realism, integration, architectural-soundness, contract-completeness, schema-correctness, ontology-alignment, e2e-coverage, e2e-reproducibility, config-consistency, secret-leak, insecure-default
Severity: high (90-100), medium (70-89), low (50-69)

Step 10 — Record Review Verdict (ai-review-gate evidence, #2058)

When the review completed clean (zero blocking findings — zero findings, or findings whose
disposition is follow_up and which are filed as owned issues and named in the review comment),
record the verdict so the ai-review-gate required
check can go green. Best-effort — NEVER fails the workflow:

Who is running the review — you, or a lander with no prompt to compose? If the answer is "I
need a review of PR N obtained and recorded by someone else" (a merge-duty lane, a rail operator, a
controller with no reviewer session), do NOT hand-compose a reviewer prompt and do NOT record by
hand: scripts/review-pr.sh <PR> (farmed to ~/.pi/agent/scripts/review-pr.sh on a cell) spawns a
genuinely fresh-context reviewer over the PR's CURRENT head, posts its findings as the PR comment
this step names as --evidence, and records ONLY a clean verdict — bound to the head it actually
reviewed — through the recorder below. Findings ⇒ exit 7, no record, findings printed; an
unresolvable repo/auth, a reviewer with no verdict, a head that moved mid-review, and a head that
already carries a clean record each refuse with a named cause. A session that runs THIS skill keeps
using the block below.

Which verdict, by class. record-review.sh accepts clean, clean-micro and clean-low:

• clean — the normal verdict, and the one a Low code-impact change records when its
  single reviewer round returns no issues. clean carries no tier guard in record-review.sh:
  it attests that the review ran at this head and found nothing, not that a re-review loop
  converged. The Low row's — cycle cell is a budget, not a verdict.
• clean-micro — micro tier, which skips this skill entirely (Step 10a).
• clean-low — a deliberately narrower class: the diff shape is content-only
  (Step 10b). It is not the verdict for a general Low-class change.

# Resolve record-review.sh explicitly — ~/.pi/agent/scripts is not on PATH.
RECORD_SH="$(command -v record-review.sh 2>/dev/null || true)"
if [ -z "$RECORD_SH" ] && [ -x "$HOME/.pi/agent/scripts/record-review.sh" ]; then
  RECORD_SH="$HOME/.pi/agent/scripts/record-review.sh"
fi
if [ -n "$RECORD_SH" ]; then
  # REST head resolution (GraphQL-exhaustion resilience, per #192).
  HEAD_SHA="$(gh api "repos/{owner}/{repo}/pulls/<PR_NUMBER>" --jq .head.sha 2>/dev/null || true)"
  # The review artifact this record NAMES (REQUIRED for `clean` — see the bullet
  # below): the comment Step 8 posted, whose URL Step 8 tells you to capture.
  # This block RESOLVES the id itself rather than assuming the capture happened —
  # an unset EV_ID silently short-circuited the record (the `&&` chain below did
  # nothing and printed nothing), which is a quiet no-op on the one path that
  # must keep working.
  EV_ID="${REVIEW_URL##*#issuecomment-}"
  [ "$EV_ID" = "${REVIEW_URL:-}" ] && EV_ID=""   # no fragment in $REVIEW_URL ⇒ nothing was captured
  if [ -z "$EV_ID" ]; then
    # Newest comment on the PR. `--paginate` matters: without it this returns the
    # 30th comment on any PR with more than 30, i.e. an artifact older than the
    # head, which the evidence check then refuses.
    EV_ID="$(gh api --paginate "repos/{owner}/{repo}/issues/<PR_NUMBER>/comments" --jq '.[-1].id' 2>/dev/null | tail -1 || true)"
  fi
  if [ -z "$HEAD_SHA" ] || [ -z "$EV_ID" ]; then
    echo "⚠️ record-review: no review artifact could be resolved (HEAD_SHA='${HEAD_SHA:-}', EV_ID='${EV_ID:-}') — NOT recording. Post the review comment, then name it: record-review.sh <PR> <head-sha> clean --evidence 'comment:<id>'" >&2
  else
    "$RECORD_SH" <PR_NUMBER> "$HEAD_SHA" clean --evidence "comment:$EV_ID"
  fi
fi

• --evidence <artifact> is REQUIRED for clean (tortoise#7391): naming the
  PR's current head is not evidence that anything was reviewed, so
  record-review.sh demands a named review artifact and VERIFIES it — it must
  exist, belong to THIS PR, POSTDATE the recorded head's commit, and not assert an
  UNRESOLVED outcome — NOT-CLEAN, or this skill's own stalled/failed/cap exit
  markers ("Auto-fix stalled", "Auto-fix failed", "require human attention",
  "N issues remain"); the clean template ("Found N issues", all fixed) passes.
  Anything unverifiable, including an unreachable API, REFUSES
  (exit 3) and prints the remedy. Accepted forms: comment:<id> (an issue
  comment — what Step 8 posts), review:<id> (a PR review),
  review-comment:<id> (an inline review comment), or the GitHub URL of one of
  those. This is an auditability and friction boundary, not proof of review:
  there is no reviewer-identity check, and the gate does not check that the
  artifact IS a review — a bot or CI comment satisfies it when it postdates the
  head, so naming a non-review is a deliberate act rather than an accident (the
  path this guard closes is the silent one: naming the head itself took no
  argument at all). clean-micro, clean-low and a genuine
  carry-forward are unaffected — a carry records "mint":"carried" and carries
  no evidence field, because its evidence is the prior signed marker it
  re-verified.

• record-review.sh auto-resolves <owner/repo>; pass it explicitly as the 4th
  arg when run outside the git repo.

• The evidence post triggers the edited event, which re-runs the
  ai-review-gate workflow and turns the check green (the marker is HMAC-signed;
  see the workflow header in .github/workflows/ai-review-gate.yml).

• If the PR head moves AFTER recording (fix commits, merge of main), the gate
  turns red again: re-review the new head and re-record — UNLESS a carry arm
  proves the reviewed artifact is unchanged (the same three-dot DIGEST, or a
  base-only move — the base advanced, YOUR OWN commits did not), AND the target
  head is not measurably red (#1575 clause E refuses the carry; re-run the
  failing checks and re-record instead), in which case nothing is owed: pass
  the recorded head and the recorded verdict to record-review.sh and it
  re-binds (mint:"carried", no --evidence). See
  commit-workflow/workflow/04-merge-deploy.md condition 6. NEVER re-record a
  moved head with a CHANGED artifact without a fresh review — the gate binds the
  full head sha.

• A saved record with a FAILED evidence post (transient API error) leaves the
  gate red: re-run record-review.sh to retry the post (the script warns and
  exits 0 on transient failures).

• If record-review.sh is unavailable (repo without the gate), skip silently.

Step 10a — Micro-tier recording: verdict clean-micro (#513)

Micro tier SKIPS this skill entirely (commit-workflow 03-code-review.md
Step 2) — the Step 10 auto-record above NEVER fires at micro, yet the merge
gate demands a review record at EVERY tier (04-merge-deploy.md condition 6).
Micro PRs therefore record the PROCESS verdict clean-micro via the micro
flow. Before recording, ALL of these must hold:

1. The linked issue is complexity:micro. The PR body's closing keyword
   resolves a same-repo issue carrying the complexity:micro label at record
   time. record-review.sh verifies this itself and REFUSES (exit 4, no
   write) any clean-micro record whose linked same-repo issue is NOT micro —
   a standard/complex issue is never recorded clean-micro: run THIS skill
   and record clean (Step 10). Where the linked ref's complexity label
   cannot be read (label-fetch failure, absent label, no closing ref, or
   only cross-repo refs) record-review.sh WARNS and proceeds — tier
   attestation UNVERIFIED at mint; do not overread this record as
   micro-certified.
2. Pre-flight passed per risk tier (01-preflight.md: typecheck/tests on
   code-bearing micro sets; Low-risk docs/CSS/static sets exempt).
3. The #485 ≥1-dispatch floor was met. Code-bearing micro sets satisfy it
   via VGATE's own [VGATE] verification dispatch (naming the diff);
   docs-only sets dispatch a lightweight reviewer naming the diff — a
   one-line "NO ISSUES FOUND" check counts (the floor is deliberately
   content-free).

clean-micro certifies the MICRO PROCESS — tier verified against the linked
issue's label, pre-flight per risk tier, the #485 dispatch floor — NOT
multi-agent code review and NOT code quality. Record at the current head only:

One PR closes one issue. A closing keyword in the PR body auto-closes
that issue on merge. record-review.sh therefore refuses (exit 4) a
clean-micro record whose body closes ANY same-repo issue that is not
complexity:micro. If the body references an issue the PR does NOT close,
write Related: #N prose instead of a closing keyword — or run THIS skill
and record clean for a standard/complex-linked change.

# ~/.pi/agent/scripts/record-review.sh is not on PATH — use the explicit path.
~/.pi/agent/scripts/record-review.sh <PR_NUMBER> <FULL_HEAD_SHA> clean-micro

Recorder = the micro flow (03-code-review.md Step 2). The merge ceremony must
NOT self-certify a fresh record: if the gate blocks, run the review
appropriate to the tier (this skill at standard/complex; the micro flow at
micro), then record.

Step 10b — Low-risk content-only recording: verdict clean-low (#1348)

The canonical tier table's Low code-impact class — the Code impact column's
Low value in proportional-gates §Change Classification (docs/CSS/strings,
mapped by that file's §Review Cycles to a single reviewer and no cycle loop) —
had no representation in the merge gate. A PR whose DIFF is
content-only but whose LINKED ISSUE is complexity:standard/complex could
record neither verdict honestly: clean attests a code-review convergence
that, per the Low class, did not happen; clean-micro is refused (exit 4) by
the #513 tier guard, because that guard reads the LINKED ISSUE's tier, not the
DIFF's shape. clean-low is that missing representation. It attests:

every changed path of the recorded revision is prose or a stylesheet — no
program code, no config file, no enforcement input. (The class is path+extension
based and does not consult a build graph: a repo may package or consume a
docs/** file as build data. That is accepted, declared, and the attestation
wording is scoped to what the guard can actually read. The Low tier's single
reviewer pass is your obligation — nothing in the script can observe it.)

Use it ONLY when the whole diff is content-only. record-review.sh verifies the
shape itself, from the diff AT THE RECORDED SHA, and REFUSES (exit 4, no record,
no marker) otherwise. The class is deliberately NARROWER than the tier table's
Low code-impact class: config and i18n strings are excluded (a config change is where a
runtime-behaviour change hides), and so are root-level instruction files
(AGENTS.md, MEMORY.md, VENDOR.md), skills/**, .github/**,
templates/**, scripts/** and extensions/**. Admitted: docs/** with a
content extension (.md, .markdown, .txt, .rst, .adoc, .css,
.scss) and the named root prose files (README.md, CHANGELOG.md,
CONTRIBUTING.md, SECURITY.md, CODE_OF_CONDUCT.md). .mdx and .html are
NOT admitted — both are build-consumed program content. Because the class is an
ALLOWLIST, a docs change that ALSO adds a non-prose file (an image, .json,
.csv, .svg) is refused as well: it has no honest Low verdict, so it takes
the normal route — the code-review skill, then clean.

# ~/.pi/agent/scripts/record-review.sh is not on PATH — use the explicit path.
~/.pi/agent/scripts/record-review.sh <PR_NUMBER> <FULL_HEAD_SHA> clean-low <owner/repo>

Every guard arm is FAIL-CLOSED — an unverifiable shape is never "certified
Low". Unlike clean-micro, whose fail-open arm is safe because its label only
cross-checks a flow that already ran, there is no second evidence behind
clean-low: the shape IS the attestation. Refused: repo undetectable or gh
missing; the PR-meta or diff read fails; an empty file list; a file list at
GitHub's 300-entry compare cap (it may be truncated); a distinct-path count
that disagrees with the PR's .changed_files; a malformed diff row, a rename
without its old path, a copy, or an unknown status; --force-stale (refused at
exit 2 as an argument-level contradiction, not exit 4 — the attestation must
describe the revision a consumer will actually read); and any
path outside the class — including a content-only path mixed into a diff that
also touches code. Re-record at the current head after any push.

The local merge gate accepts clean-low under the SAME head binding as clean
PLUS a content binding. The record carries the merge base of the three-dot
diff compare/<base>...<head> — the commit that identifies the certified
content — and the gate re-derives it, refusing with reason base_advanced when
it no longer matches and base_unverifiable when either side cannot be read
(absent/invalid in the record, or a gh/API failure). A benign advance of the
base branch, which changes its tip but not the merge base, does NOT block: the
certified diff is unchanged. It does NOT re-derive the content shape (the record
is the attestation — review-enforcer binds the head and the merge base, not the
shape). The shape itself is read in exactly TWO places, both over the ONE class
the producer declares: record-review.sh's own guard at mint time, and
check-pipeline-compliance.sh check (c), which SOURCES that class to refuse a
verdict=clean-low marker on a diff that is not Low-shaped (#1356).

The two base_* refusals are different events, and they are NOT cleared the same
way. base_advanced means the certified diff ITSELF changed — re-check whether
the current diff is still content-only and only then re-record clean-low; if it
is not, take the normal route (the code-review skill, then clean).
base_unverifiable is a read failure (a gh/API error, an absent or invalid
field in the record) and is cleared by re-recording once the base is readable —
and, as with base_advanced, only if the diff is still content-only; otherwise
take the normal route (the code-review skill, then clean).

Three boundaries worth knowing, all declared rather than silent:

• The #138 interactive fail-open also bypasses the content binding. When the
  HEAD itself cannot be fetched, an interactive session's merge gets fail-open
  (a task sub-agent's is fail-closed) BEFORE the merge-base branch is reached, so
  a clean-low record can merge there with its merge base never compared. That is
  the pre-existing #138 posture — the same path already merged clean/
  clean-micro with no head verification at all, so the content binding is
  strictly additive — and narrowing it is #138's decision to make, not this
  verdict's.
• The remote ai-review-gate required check must accept the verdict. It is a
  cross-repo contract (the marker regex lives in the consuming repo), so a repo
  whose workflow still matches verdict=clean(-micro)? will keep failing the
  check on a clean-low marker. record-review.sh says so on stderr at record
  time (widening tracked in daniel-ospina/tortoise#4755); the record and the
  local merge gate are unaffected.
• The remote check is still base-blind for every verdict, clean-low
  included — the signed marker binds the head only. Tracked with the widening
  requirement above.

Standard-Tier Review (--standard-tier)

Runs 2 reviewers. Used when full review is disproportionate.

Step 1-3: Same as full review (eligibility, CLAUDE.md paths, summary).

Step 4: Launch only A (Guidance & History) and B (Correctness & Security). B runs pass 1 only — the blind diff scan — and keeps its security section; skip B's deep pass 2. Add neither C, D nor E.

Step 5 onward: Same as full review (scoring, filter, fixer, re-check, comment, logging), then record the verdict (Step 10). Append (standard-tier review: guidance + history + bug scan + security) to comment header.

Standard-Tier + Re-review

If --re-review with --standard-tier: identify fix commits (delta diff identification from Smart Re-Review Step A), scope the reviewers to the delta diff. If >4 files changed → fall back to full standard-tier. Append (standard-tier re-review: fix-commits delta).

Smart Re-Review (--re-review)

Targeted review on fix commits only. Reviewer A's history half is static — prior history and prior PR comments won't have changed — but A still re-runs its guidance half on the delta.

Step A — Identify fix commits: Sub-agent finds commits pushed after the most recent "### Code review" bot comment. Returns fix commit SHAs + distinct file count + combined delta diff.

Step B — Scope check: If >4 files changed → fall back to full review.

Step C — CLAUDE.md paths: Same as Step 2.

Step D — Targeted review: Launch A and B (2 reviewers) on the delta diff only. B runs both bug-scan passes in order on the delta plus its security section. A near-duplicate "code comments subset" dispatch is no longer needed — the comments lens lives inside A.

Step E — Continue from Step 5: Scoring, filter (skip fixer loop — --re-review disables it), re-check, comment, logging, then record the verdict (Step 10). Append (re-review: fix-commits delta) to comment header.

False Positives to Ignore

• Pre-existing issues
• Something that looks like a bug but isn't
• Pedantic nitpicks a senior engineer wouldn't call out
• Issues a linter/typechecker/compiler would catch
• General code quality issues (test coverage, security, docs) unless explicitly required
• Issues silenced by lint ignore comments
• Changes likely intentional or directly related to the broader change
• Issues on lines the user did not modify

Notes

• Do NOT use Bash for reasoning or narration. Only for commands that produce new information.
• Prefer Read/Grep over Bash for file analysis.
• Do not check build signal or attempt to build/typecheck.
• Use gh for GitHub interaction.
• Make a todo list first.
• Cite and link each bug with full SHA + line range.

---

Continue following the workflow as mandated by this skill. Do not skip steps.