← 返回
未分类 中文

Review Verification Protocol

Mandatory verification steps for all code reviews to reduce false positives. Load this skill before reporting ANY code review findings.
在报告任何代码审查结果前,必须先加载此技能,执行所有代码审查的强制验证步骤,以降低误报。
anderskev
未分类 clawhub v1.0.20 4 版本 100000 Key: 无需
★ 0
Stars
📥 699
下载
💾 1
安装
4
版本
#latest

概述

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 — runs before every other gate)

Before issuing any verdict — flag, reject, or downgrade a finding — 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.

> 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 flag code that is not in the file. It runs before the hard gates below.

Hard gates (sequenced)

Complete in order for each finding (or once per batch if every finding shares the same file/symbol). Do not advance while the prior gate fails.

  1. Read gate — Open and read the full containing symbol (function, class, component, hook), not only the diff hunk or snippet.

Pass: You can state the file path and symbol name you read without re-opening the file.

  1. Reference gate (required before “unused”, “dead code”, or “never called”) — Run a workspace search for the identifier (or equivalent: find references in the IDE).

Pass: One concrete artifact: e.g. “rg/search: N matches” or “only the definition in path” — not a guess.

  1. Mitigation gate — Look for handling elsewhere: callers, middleware, route/loaders, error boundaries, framework validation, earlier guards, or comments/ADR context.

Pass: Either cite where the concern is already addressed, or one explicit sentence: “No mitigating pattern found after checking [scope].”

  1. Claim gate — Each reported issue must include [FILE:LINE] and a specific line or behavior that demonstrates the problem; severity must match Severity Calibration below.

Pass: A reviewer could navigate to that line and see the same issue; “might” or “could” without an anchor fails this gate.

The checklist below restates the same expectations in checkbox form.

Pre-Report Verification Checklist

Before flagging ANY issue, verify:

  • [ ] I read the actual code - Not just the diff context, but the full function/class (see Read gate)
  • [ ] I searched for usages - Before claiming "unused", searched all references (see Reference gate)
  • [ ] I checked surrounding code - The issue may be handled elsewhere (guards, earlier checks) (see Mitigation gate)
  • [ ] 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 frequently enough to matter (render vs click handler)
  2. Verify the optimization would have measurable impact
  3. Check if the framework already optimizes this (React compiler, memoization)

Do NOT flag:

  • Functions created in click handlers (runs once per click)
  • Array methods on small arrays (< 100 items)
  • Object creation in event handlers

Severity Calibration

Critical (Block Merge)

ONLY use for:

  • Security vulnerabilities (injection, auth bypass, data exposure)
  • Data corruption bugs
  • Crash-causing bugs in happy path
  • Breaking changes to public APIs

Major (Should Fix)

Use for:

  • Logic bugs that affect functionality
  • Missing error handling that causes poor UX
  • Performance issues with measurable impact
  • Accessibility violations

Minor (Consider Fixing)

Use for:

  • Code clarity improvements
  • Documentation gaps
  • Inconsistent style (within reason)
  • Non-critical test coverage gaps

Informational (No Action Required)

Use for:

  • Improvements that require adding new dependencies or modules
  • Suggestions for net-new code that didn't exist in the codebase before (new modules, test suites, abstractions)
  • Architectural ideas for future consideration
  • Test infrastructure suggestions (new mock libraries, behaviour extraction)
  • Optimizations without measurable impact in the current context

These are NOT review blockers. They should be noted for the author's awareness but must not appear in the actionable issue count. The Verdict should ignore informational items entirely.

Do NOT Flag At All

  • Style preferences where both approaches are valid
  • Optimizations with no measurable benefit
  • Test code not meeting production standards (intentionally simpler)
  • Library/framework internal code (shadcn components, generated code)
  • Hypothetical issues that require unlikely conditions

Valid Patterns (Do NOT Flag)

TypeScript

PatternWhy It's Valid
-------------------------
`map.get(key) \\[]`Map.get() returns `T \undefined`, fallback is correct
Class exports without separate type exportClasses work as both value and type
as const on literal arraysCreates readonly tuple types
Type annotation on variable declarationNot a type assertion
satisfies instead of asType checking without assertion

React

PatternWhy It's Valid
-------------------------
Array index as key (static list)Valid when: items don't reorder, list is static, no item identity needed
Inline arrow in onClickValid for non-performance-critical handlers (runs once per click)
State that appears unusedMay be set via refs, external callbacks, or triggers re-renders
Empty dependency array with refsRefs are stable, don't need to be dependencies
Non-null assertion after checkTypeScript narrowing may not track through all patterns

Testing

PatternWhy It's Valid
-------------------------
toHaveTextContent without regexHandles nested text correctly
Mock at module levelDefined once, not duplicated
Index-based test dataTests don't need stable identity
Simplified error messagesTest clarity over production polish

General

PatternWhy It's Valid
-------------------------
+? lazy quantifier in regexPrevents over-matching, correct for many patterns
Direct string concatenationSimpler than template literals for simple cases
Multiple returns in functionCan improve readability
Comments explaining "why"Better than no comments

Context-Sensitive Rules

React Keys

Flag array index as key ONLY IF ALL of these are true:

  • [ ] Items CAN be reordered (sortable list, drag-drop)
  • [ ] Items CAN be inserted/removed from middle
  • [ ] Items HAVE stable identifiers available (id, uuid)
  • [ ] The list is NOT completely replaced atomically

useEffect Dependencies

Flag missing dependency ONLY IF:

  • [ ] The value actually changes during component lifetime
  • [ ] Stale closure would cause incorrect behavior
  • [ ] The value is NOT a ref (refs are stable)
  • [ ] The value is NOT a stable callback (useCallback with empty deps)

Error Handling

Flag missing try/catch ONLY IF:

  • [ ] No error boundary catches this at a higher level
  • [ ] The framework doesn't handle errors (loader errorElement)
  • [ ] The error would cause a crash, not just a failed operation
  • [ ] User needs specific feedback for this error type

Before Submitting Review

Final verification (after Hard gates for each finding):

  1. Re-read each finding and ask: "Did I verify this is actually an issue?"
  2. For each finding, can you point to the specific line that proves the issue exists? (must satisfy Claim gate)
  3. Would a domain expert agree this is a problem, or is it a style preference?
  4. Does fixing this provide real value, or is it busywork?
  5. Format every finding as: [FILE:LINE] ISSUE_TITLE
  6. For each finding, ask: "Does this fix existing code, or does it request entirely new code that didn't exist before?" If the latter, downgrade to Informational.
  7. If this is a re-review: ONLY verify previous fixes. Do not introduce new findings.

If uncertain about any finding, either:

  • Remove it from the review
  • Mark it as a question rather than an issue
  • Verify by reading more code context

版本历史

共 4 个版本

  • v1.0.20 当前
    2026-06-04 13:05
  • v1.0.18
    2026-06-01 20:46
  • v1.0.13
    2026-05-03 04:41 安全 安全
  • v1.1.0
    2026-03-30 21:59

安全检测

腾讯云安全 (Keen)

队列中

腾讯云安全 (Sanbu)

队列中

🔗 相关推荐

Rust Testing Code Review

anderskev
审查 Rust 测试代码,包括单元测试模式、集成测试结构、异步测试、模拟方式和属性测试,覆盖 Rust 2024 版。
★ 0 📥 763

Vitest Testing

anderskev
Vitest 测试框架模式与最佳实践。适用于编写单元测试、集成测试、配置 vitest.config、使用 vi.mock/vi.fn 进行模拟等...
★ 0 📥 895

Python Code Review

anderskev
审查 Python 代码的类型安全、异步模式、错误处理及常见错误。适用于审查 .py 文件、检查类型提示、async/await 用法。
★ 0 📥 683