/review-verification-protocol

name: review-verification-protocol
description: Mandatory verification steps for all code reviews to reduce false positives. Load this skill before reporting ANY code review findings.
user-invocable: false

Review Verification Protocol

This protocol MUST be followed before reporting any code review finding. Skipping these steps leads to false positives that waste developer time and erode trust in reviews.

Anti-confabulation (gate 0 — applies to ALL review/verify skills)

Before issuing **any** verdict — confirm, reject, sever, fix, or adjudicate — you MUST echo the exact artifact you are judging, quoted from a source you read in **this** turn:

• For a code finding: the **file:line** plus the cited code, read freshly now (not recalled from earlier in the session).
• For a diff review: the actual **diff hunk** under review.
• For a structured report (e.g. `verify-llm-artifacts` adjudicating `findings[]`): the finding's id + file + line + description, printed from the **parsed source file**, not from memory.

> The artifact is the only source of truth. **Never** infer what you are reviewing from the branch name, the working directory, surrounding files, or recollection. If your mental model differs from the freshly read source, **the source wins.** A verdict issued without a same-turn echo of its target is invalid — emit the echo first, or do not emit the verdict.

This gate exists because an LLM under contextual priming will confidently adjudicate things that are not in the file. It runs **before** the per-finding hard gates below. Skills that consume this protocol implement it concretely: [verify-llm-artifacts](../verify-llm-artifacts/SKILL.md) (Load + ECHO + ID-lock gate), [review-llm-artifacts](../review-llm-artifacts/SKILL.md) (echo finding before writing JSON), [llm-artifacts-detection](../llm-artifacts-detection/SKILL.md) (anchor `FILE:LINE` from an opened buffer).

Hard gates (sequence)

Apply **once per finding** before it may appear in the review. If a gate fails, **omit** the finding, **downgrade** to Informational (per [Severity Calibration](#severity-calibration)), or **rephrase** as a question—do not ship soft accusations.

| Step | What you do | Pass condition (objective) | |------|----------------|----------------------------| | **1. Anchor** | Read the full enclosing symbol or module, not only the diff hunk. | You can state **file path** and **line range** (or symbol name + file) you are judging. | | **2. Evidence** | For this finding’s type, run the checks in [Verification by Issue Type](#verification-by-issue-type). | Each required check has an **artifact**: pasted tool output, **file:line** citation, or explicit **"none"** / **"N matches"** after a repo search—not a claim you "looked." | | **3. Severity** | Assign severity using [Severity Calibration](#severity-calibration). | Label matches the table; requests for net-new code that did not exist in scope are **Informational** only. | | **4. Format** | Draft the finding for the report. | Matches `[FILE:LINE] ISSUE_TITLE`; Informational items do not add to the actionable count. |

Style-only or preference items must fail gate 2 or map to **Do NOT Flag At All**—they do not get a severity.

Pre-Report Verification Checklist

Before flagging ANY issue, verify (these items are **what gate 2 must produce evidence for**):

• [ ] **I read the actual code** - Not just the diff context, but the full function/class
• [ ] **I searched for usages** - Before claiming "unused", searched all references
• [ ] **I checked surrounding code** - The issue may be handled elsewhere (guards, earlier checks)
• [ ] **I verified syntax against current docs** - Framework syntax evolves (Tailwind v4, TS 5.x, React 19)
• [ ] **I distinguished "wrong" from "different style"** - Both approaches may be valid
• [ ] **I considered intentional design** - Checked comments, project conventions (e.g. AGENTS.md or CLAUDE.md), architectural context

Verification by Issue Type

"Unused Variable/Function"

**Before flagging**, you MUST: 1. Search for ALL references in the codebase (grep/find) 2. Check if it's exported and used by external consumers 3. Check if it's used via reflection, decorators, or dynamic dispatch 4. Verify it's not a callback passed to a framework

**Common false positives:**

• State setters in React (may trigger re-renders even if value appears unused)
• Variables used in templates/JSX
• Exports used by consuming packages

"Missing Validation/Error Handling"

**Before flagging**, you MUST: 1. Check if validation exists at a higher level (caller, middleware, route handler) 2. Check if the framework provides validation (Pydantic, Zod, TypeScript) 3. Verify the "missing" check isn't present in a different form

**Common false positives:**

• Framework already validates (FastAPI + Pydantic, React Hook Form)
• Parent component validates before passing props
• Error boundary catches at higher level

"Type Assertion/Unsafe Cast"

**Before flagging**, you MUST: 1. Confirm it's actually an assertion, not an annotation 2. Check if the type is narrowed by runtime checks before the point 3. Verify if framework guarantees the type (loader data, form data)

**Valid patterns often flagged incorrectly:**

// Type annotation, NOT assertion
const data: UserData = await loader()

// Type narrowing makes this safe
if (isUser(data)) {
  data.name  // TypeScript knows this is User
}

"Potential Memory Leak/Race Condition"

**Before flagging**, you MUST: 1. Verify cleanup function is actually missing (not just in a different location) 2. Check if AbortController signal is checked after awaits 3. Confirm the component can actually unmount during the async operation

**Common false positives:**

• Cleanup exists in useEffect return
• Signal is checked (code reviewer missed it)
• Operation completes before unmount is possible

"Performance Issue"

**Before flagging**, you MUST: 1. Confirm the code runs freque