code-review

[ ] Logic handles all cases correctly
[ ] Edge cases are handled (empty, null, max values)
[ ] Error conditions are handled appropriately
[ ] Concurrent access is safe
[ ] State mutations are atomic where needed

[ ] Input validation is present
[ ] No injection vulnerabilities
[ ] Secrets are not logged or exposed
[ ] File paths are validated
[ ] Permissions are checked

[ ] No unnecessary clones
[ ] Appropriate use of references vs ownership
[ ] Error types are informative
[ ] No unwrap() in library code
[ ] Unsafe code is documented and minimal

[ ] No unnecessary allocations in hot paths
[ ] Appropriate data structures used
[ ] No blocking in async code
[ ] Caching where beneficial

[ ] Code is readable and self-documenting
[ ] Functions are focused (single responsibility)
[ ] Dependencies are justified
[ ] Tests cover the changes

**Issue**: [Brief description]
**Location**: `file.rs:123`
**Severity**: Critical | Important | Suggestion
**Problem**: [What's wrong and why it matters]
**Suggestion**: [How to fix it]

```rust
// Before
let result = data.unwrap();

// After
let result = data.ok_or(Error::MissingData)?;

### For Questions
```markdown
**Question**: [What you're unsure about]
**Location**: `file.rs:45-50`
**Context**: [Why you're asking]

**Looks good**: [Specific thing that's well done]
**Note**: [Any minor observations]

// Bad: Silent failure
fn process(data: Option<Data>) {
    if let Some(d) = data {
        // process
    }
    // Silent no-op if None
}

// Good: Explicit error
fn process(data: Option<Data>) -> Result<(), Error> {
    let d = data.ok_or(Error::MissingData)?;
    // process
    Ok(())
}

// Bad: Manual cleanup
fn read_file(path: &Path) -> Result<String> {
    let file = File::open(path)?;
    // What if this panics? File not closed properly
    let content = read_all(&file)?;
    drop(file); // Manual cleanup
    Ok(content)
}

// Good: RAII handles cleanup
fn read_file(path: &Path) -> Result<String> {
    let content = std::fs::read_to_string(path)?;
    Ok(content)
}

// Bad: Race condition
static mut COUNTER: u64 = 0;
fn increment() {
    unsafe { COUNTER += 1; }
}

// Good: Atomic operations
use std::sync::atomic::{AtomicU64, Ordering};
static COUNTER: AtomicU64 = AtomicU64::new(0);
fn increment() {
    COUNTER.fetch_add(1, Ordering::Relaxed);
}

[ ] cargo fmt --check clean
[ ] cargo clippy --all-targets --all-features clean
[ ] All #[allow(...)] annotations have justification comments
[ ] Tests added/updated; includes edge cases and regressions
[ ] If perf-related: benchmark script + before/after results + build profile noted
[ ] If unsafe: invariants documented + tests proving them
[ ] Public-facing changes: docs/README/help text updated

# Format check
cargo fmt --check

# Clippy check (treat warnings as errors)
RUSTFLAGS="-D warnings" cargo clippy --all-targets --all-features

# Run tests
cargo test --all-features

# Run benchmarks (if perf-related)
cargo bench

[ ] Errors explain WHAT failed
[ ] Errors explain HOW to fix it
[ ] No cryptic error codes without explanation
[ ] File paths included in I/O errors
[ ] Suggestions for common mistakes

[ ] --help is comprehensive and accurate
[ ] Examples included for complex commands
[ ] Man page or README updated for new features
[ ] Breaking changes documented in CHANGELOG

[ ] UTF-8 errors handled explicitly (not silently ignored)
[ ] File not found errors are actionable
[ ] Permission errors suggest fix (e.g., "check permissions with ls -la")
[ ] Behavior documented for edge cases (empty files, binary input)