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.

Pre-Report Verification Checklist

Before flagging ANY issue, verify:

• I read the actual code - Not just the diff context, but the full function/impl block
• I searched for usages - Before claiming "unused", searched all references
• I checked surrounding code - The issue may be handled elsewhere (trait impls, error propagation)
• I verified syntax against current docs - Rust edition, crate versions, and API changes
• I distinguished "wrong" from "different style" - Both approaches may be valid
• I considered intentional design - Checked comments, 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 pub and used by other crates in the workspace
3. Check if it's used via derive macros, trait implementations, or conditional compilation (#[cfg])
4. Verify it's not a trait method required by the trait definition

Common false positives:

• Trait implementations where the method is defined by the trait
• #[cfg(test)] items only used in test builds
• Derive-generated code that uses struct fields
• Types used via From/Into conversions

"Missing Error Handling"

Before flagging, you MUST:

1. Check if the error is handled at a higher level (caller propagates with ?)
2. Check if the crate has a top-level error type that wraps this error
3. Verify the unwrap() isn't in test code or after a safety-ensuring check

Common false positives:

• unwrap() in tests and examples (expected pattern)
• expect("reason") after validation (e.g., regex::Regex::new on a literal)
• Error propagation via ? (the caller handles it)
• let _ = tx.send(...) — intentional when receiver may have dropped

"Unnecessary Clone"

Before flagging, you MUST:

1. Confirm the clone is actually avoidable (borrow checker may require it)
2. Check if the value needs to be moved into a closure/thread/task
3. Verify the type isn't Copy (clone on Copy types is a no-op)
4. Check if the clone is in a hot path (test/setup code cloning is fine)

Common false positives:

• Arc::clone(&arc) — this is the recommended explicit clone for Arc
• Clone before tokio::spawn — required for 'static bound
• Clone in test setup — clarity over performance

"Potential Race Condition"

Before flagging, you MUST:

1. Verify the data is actually shared across threads/tasks
2. Check if Mutex, RwLock, or atomic operations protect the access
3. Confirm the type doesn't already guarantee thread safety (e.g., Arc<Mutex<T>>)
4. Check if the "race" is actually benign (e.g., logging, metrics)