Rust Review

Table of Contents

• Quick Start
• When to Use
• Required TodoWrite Items
• Progressive Loading
• Core Workflow
• Rust Quality Checklist
• Safety
• Correctness
• Performance
• Idioms
• Output Format
• Summary
• Ownership Analysis
• Error Handling
• Concurrency
• Unsafe Audit
• [[U1] file:line](#[u1]-file:line)
• Dependencies
• Recommendation
• Exit Criteria

Rust Review Workflow

Expert-level Rust code audits with focus on safety, correctness, and idiomatic patterns.

Quick Start

/rust-review

Verification: Run the command with --help flag to verify availability.

When To Use

• Reviewing Rust code changes
• Auditing unsafe blocks
• Analyzing concurrency patterns
• Dependency security review
• Performance optimization review

When NOT To Use

• General code review without Rust - use unified-review
• Performance profiling - use parseltongue:python-performance pattern

Required TodoWrite Items

1. rust-review:ownership-analysis 2. rust-review:error-handling 3. rust-review:concurrency 4. rust-review:unsafe-audit 5. rust-review:cargo-deps 6. rust-review:native-modeling 7. rust-review:idiomatic-elision 8. rust-review:coercion-params 9. rust-review:conversion-traits 10. rust-review:numeric-cast-safety 11. rust-review:mutable-static-audit 12. rust-review:match-wildcard 13. rust-review:transmute-audit 14. rust-review:float-equality 15. rust-review:mem-forget-audit 16. rust-review:repr-packed-audit 17. rust-review:evidence-log 18. rust-review:findings-verified

Progressive Loading

Load modules as needed based on review scope:

Quick Review (ownership and errors):

• See modules/ownership-analysis.md for borrowing and lifetime analysis
• See modules/error-handling.md for Result/Option patterns

Concurrency Focus:

• See modules/concurrency-patterns.md for async and sync primitives

Safety Audit:

• See modules/unsafe-audit.md for unsafe block documentation
• See modules/mutable-static-audit.md for static mut globals and

their thread-safe replacements

• See modules/numeric-cast-safety.md for truncating and

precision-losing as casts

• See modules/match-wildcard.md for catch-all arms that defeat enum

exhaustiveness

• See modules/transmute-audit.md for mem::transmute/transmute_copy

calls that reinterpret bytes with no layout check

• See modules/repr-packed-audit.md for #[repr(packed)] layouts whose

field borrows become unaligned references

Correctness Audit:

• See modules/float-equality.md for ==/!= against float literals
• See modules/mem-forget-audit.md for mem::forget leaks and no-op

drop(&x) reference drops

Dependency Review:

• See modules/cargo-dependencies.md for vulnerability scanning

Idiomatic Patterns:

• See modules/builtin-preference.md for conversion traits and builtin preference
• See modules/native-type-modeling.md for enums-over-primitives,

newtype, type-state, and derived ordering

• See modules/idiomatic-elision.md for lifetime elision,

expression-oriented returns, and explicit -> () unit returns

• See modules/coercion-params.md for &String/&Vec<T>/&PathBuf

parameters that defeat deref coercion (prefer &str/&[T]/&Path)

• See modules/conversion-traits.md for impl Into that should be

impl From, and discarded try_into().unwrap() conversion errors

Core Workflow

1. Ownership Analysis: Check borrowing, lifetimes, clone patterns 2. Error Handling: Verify Result/Option usage, propagation 3. Concurrency: Review async patterns, sync primitives 4. Unsafe Audit: Document invariants, FFI contracts 5. Dependencies: Scan for vulnerabilities, updates 6. Evidence Log: Record commands and findings

Rust Quality Checklist

Safety

• [ ] All unsafe blocks documented with SAFETY comments
• [ ] FFI boundaries properly wrapped
• [ ] Memory safety invariants maintained
• [ ] No static mut globals; shared state uses OnceLock/LazyLock,

atomics, or a Mutex/RwLock

• [ ] No mem::transmute/transmute_copy; bytes converted with

from_le_bytes/from_bits/bytemuck or pointers with .cast()

• [ ] #[repr(packed)] fields copied out before borrowing (no unaligned

references)

• [ ] No mem::forget leaks (use ManuallyDrop/scope) and no no-op

drop(&x) reference drops

• [ ] mlock/munlock calls: RLIMIT verified, page-aligned,

ENOMEM handled

Correctness

• [ ] Error handling complete
• [ ] Concurrency patterns sound
• [ ] Lossy as casts (length truncation, as u8/i8, as f32)

replaced with TryFrom/From

• [ ] Enum matches exhaustive; no _ => unreachable!()/panic!/{}

catch-alls

• [ ] Floats compared with a tolerance, not exact ==/!= against a

float literal

• [ ] Tests cover critical paths

Performance

• [ ] No unnecessary allocations
• [ ] Borrowing preferred over cloning
• [ ] Async properly non-blocking

Idioms

• [ ] Standard traits implemented
• [ ] Conversion traits preferred over helper functions
• [ ] Stringly-typed values and boolean flags modeled as enums
• [ ] Domain invariants encoded with newtypes (private field +

validating constructor) or type-state where warranted

• [ ] Comparison/ordering traits derived, not hand-written
• [ ] Lifetimes elided where elision rules apply; '_ in paths
• [ ] Trailing return dropped in favor of the tail expression
• [ ] Explicit -> () unit returns dropped (default is elided)
• [ ] Parameters take &str/&[T]/&Path, not &String/&Vec<T>/

&PathBuf (deref coercion accepts both, so the slice is more general)

• [ ] Conversions implement From/TryFrom, not Into/TryInto; a

fallible conversion's error is propagated, not unwrap()ped

• [ ] Error types well-designed
• [ ] Documentation complete

Output Format

## Summary
Rust audit findings

## Ownership Analysis
[borrowing and lifetime issues]

## Error Handling
[error patterns and issues]

## Concurrency
[async and sync patterns]

## Unsafe Audit
### [U1] file:line
- Invariants: [documented]
- Anchor: `verbatim source text at file:line`
- Risk: [assessment]
- Recommendation: [action]

## Native Type Modeling
[stringly-typed comparisons, boolean blindness, newtype/type-state notes]

## Idiomatic Elision
[needless lifetimes, trailing returns, explicit `-> ()` unit returns]

## Coercion Params
[`&String`/`&Vec<T>`/`&PathBuf` params that should be borrowed slices]

## Conversion Traits
[`impl Into` over `impl From`; discarded `try_into().unwrap()` errors]

## Numeric Cast Safety
[length-truncating, byte-narrowing, and f32 precision-losing `as` casts]

## Mutable Static Audit
[`static mut` globals and their thread-safe replacements]

## Match Wildcard
[catch-all `_ =>` arms that defeat enum exhaustiveness]

## Transmute Audit
[`mem::transmute`/`transmute_copy` calls and their typed replacements]

## Float Equality
[exact `==`/`!=` comparisons against float literals]

## Mem Forget Audit
[`mem::forget` leaks and no-op `drop(&x)` reference drops]

## Repr Packed Audit
[`#[repr(packed)]` layouts whose field borrows become unaligned]

## Dependencies
[cargo audit results]

## Recommendation
Approve / Approve with actions / Block

Verification: Run the command with --help flag to verify availability.

Verify Findings Are Grounded (rust-review:findings-verified)

Every finding must cite a real location and a verbatim anchor. Write findings to .review/findings.json and confirm each citation resolves:

python plugins/imbue/scripts/citation_verifier.py \
  --findings .review/findings.json --repo-root .

Drop or label UNVERIFIED any finding the verifier fails (exit 1); only verified findings enter the report. See Skill(imbue:review-core) Step 5 and Skill(imbue:structured-output) for the schema.

Exit Criteria

• All unsafe blocks audited
• Concurrency patterns verified
• Dependencies scanned
• Evidence logged
• Action items assigned
• Every reported finding carries a Location + verbatim Anchor confirmed by citation_verifier.py (exit 0), or unverified findings were dropped or labeled UNVERIFIED

Async Slop

AI defaults to `async` and `tokio::spawn` even where sync code is faster, simpler, and correct.

This module covers the high-frequency async patterns that look idiomatic but are not. The clippy lints catch some; the rest is structural.

Pattern 1: async fn that contains no .await

// SLOP
async fn compute_total(items: &[Item]) -> u64 {
    items.iter().map(|i| i.price).sum()
}

If the function body has no .await, it has no reason to be async. Async coloring is contagious: this function is callable only from async contexts, forcing every caller to also be async. Strip async from the signature unless the body actually awaits.

Detection (preferred: clippy):

cargo clippy --all-targets -- -W clippy::async_yields_async

File-level heuristic when clippy is unavailable:

for f in $(rg -l "async fn " --type rust); do
  rg -q "\.await" "$f" || echo "no-await: $f"
done

(Heuristic; manual review needed since .await may be in a helper called by the async fn rather than inline.)

Pattern 2: blocking I/O inside an async runtime

// SLOP
async fn read_config() -> Result<String> {
    Ok(std::fs::read_to_string("config.toml")?)
}

// SLOP
async fn rate_limit_wait() {
    std::thread::sleep(Duration::from_secs(1));  // blocks the runtime
}

// SLOP
async fn query_db(conn: &Connection) -> Result<Vec<Row>> {
    conn.query("SELECT ...")?  // blocking driver
}

Blocking calls inside async block the entire executor thread, defeating the runtime's concurrency model.

Fix:

// Use the async equivalent
async fn read_config() -> Result<String> {
    Ok(tokio::fs::read_to_string("config.toml").await?)
}

// Or wrap blocking work in spawn_blocking
async fn rate_limit_wait() {
    tokio::time::sleep(Duration::from_secs(1)).await;
}

// For unavoidable blocking work
async fn query_db(conn: Arc<Connection>) -> Result<Vec<Row>> {
    let conn = conn.clone();
    tokio::task::spawn_blocking(move || conn.query("SELECT ..."))
        .await?
}

Detection:

# Find blocking ops inside async functions (heuristic)
rg -B 5 "(std::fs::|std::thread::sleep|std::net::TcpStream)" --type rust |
  rg -B 5 "async fn"

Pattern 3: tokio::spawn for synchronous-equivalent work

// SLOP
async fn handle_request(req: Request) -> Response {
    let result = tokio::spawn(async move {
        compute_response(&req)
    }).await.unwrap();
    result
}

Spawning a task only to immediately await its single completion is equivalent to a direct call, plus the overhead of task creation, scheduling, and a join. Just call the function:

async fn handle_request(req: Request) -> Response {
    compute_response(&req)
}

tokio::spawn is for concurrent work: when the spawned task should make progress while the caller does something else, or when the task should outlive the caller. A spawn-then-immediately-await is a smell.

Pattern 4: async-trait on synchronous-equivalent traits

// SLOP
#[async_trait]
trait Greeter {
    async fn greet(&self, name: &str) -> String;
}

If the implementation has no .await and just returns a synchronous value, async-trait adds heap allocation (Box<dyn Future>) for nothing. Make the trait sync:

trait Greeter {
    fn greet(&self, name: &str) -> String;
}

Use async-trait only when at least one implementation genuinely awaits.

Pattern 5: explicit Pin<Box<dyn Future>> returns

// SLOP
fn fetch_data(url: &str) -> Pin<Box<dyn Future<Output = Result<Data>> + Send>> {
    Box::pin(async move {
        // body
    })
}

Modern Rust supports impl Future in return position:

// Idiomatic
fn fetch_data(url: &str) -> impl Future<Output = Result<Data>> + Send {
    async move {
        // body
    }
}

Pin<Box<dyn Future>> is needed only for trait method returns or when storing futures in collections.

Pattern 6: MutexGuard held across .await

// SLOP — deadlock risk
async fn update_count(state: &Arc<Mutex<State>>) {
    let mut guard = state.lock().unwrap();
    guard.count += 1;
    save_to_disk(&guard).await;  // holds guard across await
}

Holding a sync Mutex guard across .await blocks the runtime if any other task tries to acquire the same lock. For async paths, use:

• tokio::sync::Mutex (async-aware, can hold guards

across .await).

• Or restructure to drop the guard before awaiting:

async fn update_count(state: &Arc<Mutex<State>>) {
    let snapshot = {
        let mut guard = state.lock().unwrap();
        guard.count += 1;
        guard.clone()
    };  // guard dropped here
    save_to_disk(&snapshot).await;
}

This is the GPT-5.x signature failure (Sonar measured ~470 concurrency issues per MLOC for GPT-5.2 High); see model-specific-tells.md.

Pattern 7: re-implementing select! / join! manually

If you find yourself manually polling multiple futures with Pin::new and Poll, you almost certainly want tokio::select! or tokio::join!. Hand-rolled polling is a strong signal that the model copied something it should not have.

Detection:

rg "Pin::new" --type rust -B 2 -A 5 | rg -B 2 "fn poll"

Pattern 8: Send + Sync bounds added "in case"

// SLOP
fn add<T: Send + Sync + Clone + Debug>(a: T, b: T) -> T { ... }

Trait bounds should be added because the function needs them, not as defensive over-spec. Send/Sync on a function that runs synchronously, Clone on a function that doesn't clone, Debug on a function that doesn't print: all noise.

The right rule: add the bound when the compiler complains without it. Remove the bound when removing it does not cause a compile error.

Detection commands

# Catch most async slop with clippy
cargo clippy --all-targets -- \
  -W clippy::async_yields_async \
  -W clippy::large_futures \
  -D warnings

# Manual scans for the structural patterns
# Pattern 1: file-level "async fn but no .await" — see Pattern 1
#            section above for the loop form.
rg "tokio::spawn.*\.await" --type rust                # Pattern 3
rg "#\[async_trait\]" --type rust                     # Pattern 4
rg "Pin<Box<dyn Future" --type rust                   # Pattern 5
rg -B 5 "\.await" --type rust | rg -B 5 "\.lock\(\)"  # Pattern 6
rg "Send \+ Sync" --type rust                         # Pattern 8

False positives

Some async patterns are correct and should stay:

• async fn with no .await is fine in a trait

implementation when other implementations need .await.

• tokio::spawn is fine when the task should outlive

the caller, or when the caller does work in parallel.

• Send + Sync bounds are required when the type will

be sent across threads (axum handlers, tokio tasks).

When in doubt, comment the rationale: // async because trait requires it; this impl is sync or // spawn so metrics flush in parallel with shutdown.

Output format

Per Skill(scribe:slop-detector) module structured-finding-output.md. Severity:

• High: pattern 6 (MutexGuard across await; deadlock

risk).

• Medium: patterns 2 (blocking inside async), 3

(spawn-then-await), 4 (async-trait on sync method).

• Low: patterns 1 (vacuous async), 5 (Pin<Box<dyn

Future>>), 8 (defensive Send+Sync).

Pattern 6 is the highest-blast-radius async finding; escalate to severity: high and to a senior reviewer.

Integration

Async slop lands in Pass 5 of the multi-pass cleanup workflow (Skill(scribe:slop-detector) module cleanup-workflow.md). For GPT-family-generated codebases, weight pattern 6 detection most heavily; for Claude-family codebases, weight pattern 1 (the "behavior-preserving refactor leaves async fn that no longer awaits anything") most heavily. See model-specific-tells.md.

Builtin Preference

Detects custom helper functions that duplicate Rust's standard trait system and built-in combinators.

What This Detects

Four categories of anti-patterns:

1. Conversion helpers: parse_foo(), foo_from_bar(), convert_*(), to_*(&self) that should be FromStr, From, TryFrom, or Into implementations 2. Standard trait replacements: default_config(), format_error(), as_bytes(&self), compare() that should be Default, Display, AsRef, or PartialEq 3. Error conversion wrappers: io_to_my_error(), wrap_error() that should be impl From<Error> or thiserror #[from] 4. Manual combinators: match opt { Some(x) => Some(f(x)), None => None } that should be .map(), .unwrap_or(), .flatten(), etc.

Why It Matters

Rust's trait system is compositional by design:

• impl From<A> for B gives impl Into<B> for A for free
• impl Display gives ToString for free
• From enables the ? operator for error propagation
• Trait impls participate in generic bounds and blanket impls
• Standard combinators are optimized and well-tested

Helper functions that bypass this system create API inconsistency, miss ergonomic benefits, and signal unfamiliarity with idiomatic Rust.

Safe Patterns

// Good: From trait enables .into() and ? operator
impl From<Config> for Settings {
    fn from(c: Config) -> Self {
        Settings { timeout: c.timeout }
    }
}

// Good: FromStr enables .parse()
impl FromStr for Config {
    type Err = ConfigError;
    fn from_str(s: &str) -> Result<Self, Self::Err> { ... }
}

// Good: Default via derive
#[derive(Default)]
struct Config { timeout: u64 }

// Good: Display for human-readable output
impl fmt::Display for MyError {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        write!(f, "Error: {}", self.msg)
    }
}

// Good: Option combinators
let result = opt.map(|x| x.to_string());
let value = opt.unwrap_or(default);

Patterns to Flag

// Flag: should be impl FromStr
fn parse_config(s: &str) -> Config { ... }

// Flag: should be impl From<Bar> for Foo
fn foo_from_bar(b: Bar) -> Foo { ... }

// Flag: should be impl Default
fn default_config() -> Config { ... }

// Flag: should be impl From<io::Error> for MyError
fn io_to_my_error(e: io::Error) -> MyError { ... }

// Flag: should use .map()
match opt {
    Some(x) => Some(x.to_string()),
    None => None,
}

Exclusions (Not Flagged)

• Lossy conversions (to_lossy_ascii)
• Builder methods (with_timeout(self, ...))
• Multi-parameter conversions (context-dependent)
• Domain-specific operations (serialize, encode, decode)

Related Clippy Lints

Output Section

## Builtin Preference
### Issues Found
- [file:line] Conversion helper `parse_config`: use `impl FromStr`
- [file:line] Manual combinator: use `.map()` (clippy::manual_map)

### Recommendations
- Implement standard traits to gain ecosystem composability
- Enable relevant clippy lints for automated enforcement

Cargo Dependencies

Audit and management of Cargo dependencies and build configuration.

Audit Commands

Run detailed dependency analysis:

cargo tree -d              # Find duplicates
cargo audit                # Security vulnerabilities
cargo outdated             # Stale versions
cargo deny check           # Policy enforcement

Dependency Evaluation

Check:

• Feature flags usage
• Optional dependencies
• Build scripts safety
• Binary size impact
• Compilation time

Security Scanning

Review for:

• Known vulnerabilities
• Abandoned crates
• Unmaintained dependencies
• Security advisories
• Supply chain risks

Version Management

Verify:

• Semver compliance
• Version pinning strategy
• Dependency updates frequency
• Breaking change handling

Common Issues

Flag:

• Abandoned crates
• Excessively large dependencies
• Security-vulnerable versions
• Duplicate dependencies
• Unnecessary dependencies

Alternatives Suggestion

Recommend alternatives for:

• Unmaintained crates
• Heavy dependencies
• Vulnerable versions
• Better maintained options

Output Section

## Dependencies
### Security Issues
- [crate@version] Vulnerability: [CVE/advisory]

### Recommendations
- Update [crate] from X to Y
- Replace [abandoned-crate] with [alternative]
- Remove unused dependency: [crate]

cfg(test) Misuse

Analysis of #[cfg(test)] placement on individual items outside a mod tests { ... } block.

What This Detects

#[cfg(test)] on a standalone fn, impl, or struct that is not nested inside a mod tests block.

Why It Matters

The idiomatic pattern is a single mod tests block gated with #[cfg(test)], which keeps test code in one place. Applying #[cfg(test)] to an individual impl block is particularly hazardous: it removes method implementations from the production binary without an obvious compiler warning.

Safe Patterns

// Good: single gated mod tests block
#[cfg(test)]
mod tests {
    use super::*;

    fn helper() { ... }

    #[test]
    fn test_something() { ... }
}

Patterns to Flag

// Bad: cfg(test) on standalone function
#[cfg(test)]
fn setup_fixture() { ... }

// Bad: cfg(test) on impl block outside mod tests
#[cfg(test)]
impl MyStruct {
    fn test_helper(&self) { ... }
}

Output Section

## cfg(test) Misuse
### Issues Found
- [file:line] cfg(test) outside mod tests: [explanation]

### Recommendations
- Move all test-only items inside a single `#[cfg(test)] mod tests` block

Coercion Params

Flags parameters typed as an owned-type reference (&String, &Vec<T>, &PathBuf) where the borrowed form (&str, &[T], &Path) is strictly more general. A borrowed-slice parameter accepts both an owned value's borrow and an already-borrowed one through deref coercion, so the owned-type reference needlessly narrows the callers the function admits.

What This Detects

The analyzer (analyze_coercion_params) flags three parameter shapes:

1. `&String`: recommend &str. String: Deref<Target = str>. 2. `&Vec<T>`: recommend &[T]. Vec<T>: Deref<Target = [T]>. 3. `&PathBuf`: recommend &Path. PathBuf: Deref<Target = Path>.

Detection keys on the typed-binding form : &Type (predominantly function parameters). The leading colon excludes return types (-> &String), and requiring the & adjacent to the type name excludes the load-bearing &mut String / &mut Vec<T> cases. An optional lifetime (&'a String) is tolerated. This mirrors clippy::ptr_arg.

Why Coercion Makes the Borrowed Form More General

The Rust Reference type-coercions chapter (src/type-coercions.md) lists deref coercion among the allowed coercions:

&T or &mut T to &U if T implements Deref<Target = U>.

and lists function-call arguments as a coercion site:

Arguments for function calls: the value being coerced is the actual

parameter, and it is coerced to the type of the formal parameter.

Because the argument position is a coercion site, a fn g(s: &str) accepts a &String (the compiler inserts the deref) and a &str. A fn g(s: &String) accepts only a &String. The borrowed-slice parameter is therefore a superset of the callers the owned-type reference admits.

// Flag: only callers holding a String can call this
fn count_words(text: &String) -> usize { text.split(' ').count() }
// More general: &String and &str both coerce in
fn count_words(text: &str) -> usize { text.split(' ').count() }

// Flag
fn sum(xs: &Vec<i64>) -> i64 { xs.iter().sum() }
// More general
fn sum(xs: &[i64]) -> i64 { xs.iter().sum() }

Exclusions (Not Flagged)

The deref-coercion argument only holds for a shared, immutable borrow whose body does not need the owned type. The detector and reviewer must leave these alone:

• `&mut String` / `&mut Vec<T>`: a slice cannot grow. If the body

calls push, push_str, clear, truncate, reserve, try_reserve, extend_from_slice, pop, or other length-changing methods, the owned-type reference is required (rust-clippy #8463, #9067, #9542). The &mut form is not matched.

• `&Box<T>`: clippy::ptr_arg deliberately does not flag &Box<T>;

neither does this detector.

• Owned-only method use: a &Vec<T> whose value is .clone()d to

obtain an owned Vec, or that uses Vec/String-specific API (capacity, as_mut_vec), genuinely needs the owned type. This is beyond a single-line check; the reviewer confirms before changing.

• Trait-fixed signatures: a parameter type imposed by a trait

definition or impl Trait for T method is a binding contract and cannot be narrowed (rust-clippy #8410). Confirm the signature is free before recommending.

• Generic / bound positions: deref coercion does not satisfy a

generic type parameter; a value flowing into T of fn f<T: Bound>(x: T) is not coerced. Do not narrow there.

• Macro-hidden use, FFI, and `extern` ABI: a Vec-only method may

live inside a macro the line scan cannot see; extern "C" signatures fix the concrete type. clippy::ptr_arg skips non-Rust ABI.

• Comments: a signature shown in a // comment is not code.

The known false-positive class is a &Vec<T>/&String parameter that the body needs as the owned type for one of the reasons above; the reviewer confirms the borrow is read-only and self-contained before applying the change.

Related Clippy Lints

Output Section

## Coercion Params
### Issues Found
- [file:line] `&String` param: take `&str`; deref coercion accepts both,
  so `&str` is strictly more general (clippy::ptr_arg)
- [file:line] `&Vec<T>` param: take `&[T]` (clippy::ptr_arg)
- [file:line] `&PathBuf` param: take `&Path` (clippy::ptr_arg)

Exit Criteria

• [ ] : &String, : &Vec<T>, and : &PathBuf typed parameters are

flagged with the borrowed-slice recommendation and clippy::ptr_arg

• [ ] &mut String / &mut Vec<T> parameters are not flagged (the

owned-type reference is load-bearing for growth)

• [ ] &Box<T>, by-value owned params, and already-borrowed &str /

&[T] parameters are not flagged

• [ ] Signatures in comments are not flagged
• [ ] Each finding names the borrowed alternative and cites the

deref-coercion rationale

Collection Types

Analysis of Vec usage where a different collection type would be more correct or more efficient.

What This Detects

• vec.contains(&x): O(n) membership test; HashSet gives O(1)
• vec.dedup(): sorting and dedup pattern suggests a set
• vec.iter().find(...) / vec.iter().position(...): linear key lookup

suggests HashMap

Why It Matters

Using Vec for set or map operations produces O(n) behaviour where O(1) is available. It also signals unclear intent: a HashSet communicates uniqueness, a HashMap communicates keyed access.

Safe Patterns

// Good: set membership
let mut seen: HashSet<u64> = HashSet::new();
if seen.contains(&id) { ... }

// Good: keyed lookup
let mut index: HashMap<u64, User> = HashMap::new();
if let Some(user) = index.get(&id) { ... }

Patterns to Flag

// Flag: O(n) membership on unbounded Vec
users.contains(&new_user)

// Flag: dedup implies uniqueness invariant
ids.sort();
ids.dedup();

// Flag: linear key scan
users.iter().find(|u| u.id == target_id)

Output Section

## Collection Types
### Issues Found
- [file:line] Vec used as set/map: [explanation]

### Recommendations
- Replace with HashSet for membership checks
- Replace with HashMap for keyed access

Concurrency Patterns

Analysis of concurrent and async code patterns in Rust, grounded in the concurrency cost hierarchy.

Concurrency Cost Hierarchy

"Acquiring a mutex isn't slow; contention is slow." Before reviewing concurrency code, classify each synchronization point by its cost tier.

Levels 3-5 are performance bugs. Level 6 is invisible in task scheduler traces (tokio-console shows tasks scheduling in microseconds while the actual latency occurs in the kernel page fault handler). Target Level 2 as the default. Achieve Level 1 through contention reduction. Level 0 requires architectural redesign (per-thread computation with periodic merges).

Level 6: kernel paging latency: Long-lived Tokio runtimes that co-reside with large heap users (e.g., ML model weights) are at risk: the kernel may page out latency-sensitive buffers during idle periods. When audio or ring-ring buffers page back in on the next access, the page fault adds 100–400 ms of p99 latency with no Tokio trace signal.

Detection: production-only latency spikes, no reproduction on dev box, perf stat shows elevated page-faults on the audio/buffer threads.

Fix: libc::mlock on the buffer pages. See modules/unsafe-audit.md for the full production checklist (RLIMIT_MEMLOCK, page alignment, ENOMEM fallback).

Key insight: Performance is dominated by atomic instruction count, not total instruction count. An algorithm with 9x more total instructions but the same number of atomics performs identically.

What to Flag in Review

• Level 5: Fair spin-locks on thread pools larger

than core count. Always flag.

• Level 4: std::sync::Condvar wake patterns that

convoy. Flag when hot path.

• Level 3: sched_yield() or thread::yield_now()

in lock loops. Suggest backoff or parking.

• Level 2 (avoidable): Atomic RMW on shared counter

when per-thread counters and merge would suffice.

• False sharing: Independent atomics on the same

cache line (64 bytes). Suggest #[repr(align(64))] or crossbeam_utils::CachePadded.

Synchronization Primitives

Review primitives usage:

• Arc, Mutex, RwLock
• Atomic* types and ordering (Relaxed vs SeqCst)
• tokio::sync (mpsc, broadcast, watch, Semaphore)
• Send/Sync bounds
• parking_lot vs std::sync trade-offs

Memory Ordering Review

Check ordering is neither too weak nor too strong:

• Relaxed: Counters, statistics (no cross-variable

ordering needed)

• Acquire/Release: Publish/consume patterns,

one-shot flags

• SeqCst: Only when total order across multiple

atomics is required (rare; flag overuse)

Async Patterns

Check async code:

• No blocking in async functions
• Proper spawn_blocking usage
• Guards dropped before awaiting
• Cancellation safety
• Task spawning patterns

Best Practices

// Good: Drop guard before await
async fn update(data: Arc<Mutex<Data>>) {
    let value = {
        let guard = data.lock().await;
        guard.value.clone()
    }; // Guard dropped
    process(value).await;
}

// Good: Cache-padded to prevent false sharing
use crossbeam_utils::CachePadded;

struct Counters {
    reads: CachePadded<AtomicU64>,
    writes: CachePadded<AtomicU64>,
}

Contention Reduction Patterns

When review finds Level 2+ contention on hot paths:

1. Shard the lock: DashMap, ShardedLock, or manual sharding by key hash 2. Per-thread accumulation: Thread-local counters merged at read time (Level 2 to Level 0) 3. Read-copy-update (RCU): arc-swap for read-heavy, write-rare data 4. Lock-free structures: crossbeam queues and deques when contention dominates

Deadlock Prevention

Identify potential deadlocks:

• Lock ordering consistency
• Nested locks
• Await points while holding locks
• Circular dependencies

Data Race Detection

Check for:

• static mut misuse
• Shared mutable state
• Missing synchronization
• Race conditions

Send/Sync Bounds

Verify:

• Proper trait bounds
• Thread safety guarantees
• Cross-thread data transfer
• Closure captures

Common Issues

• Blocking in async context
• Guards held across await points
• Inconsistent lock ordering
• Missing bounds on generics
• Unsafe Send/Sync implementations
• SeqCst used everywhere (usually Acquire/Release

suffices; SeqCst adds unnecessary fence cost)

• Spinning without backoff on oversubscribed systems
• False sharing between independent atomics

Output Section

## Concurrency
### Cost Classification
- [file:line] Level N: [primitive] - [justification]

### Issues Found
- [file:line] Guard held across await: [details]
- [file:line] Potential deadlock: [scenario]
- [file:line] False sharing risk: [layout details]
- [file:line] Unnecessary SeqCst: [suggest weaker ordering]

### Recommendations
- [concurrency improvements with cost tier impact]

References

• Jon Gjengset, "The Cost of Concurrency Coordination"

(video: youtube.com/watch?v=tND-wBBZ8RY)

• Travis Downs, "A Concurrency Cost Hierarchy"

(travisdowns.github.io/blog/2020/07/06/concurrency-costs.html)

• Mara Bos, "Rust Atomics and Locks" (O'Reilly)

Conversion Traits

Flags two conversion smells: implementing Into where From is the preferred direction, and discarding the error of a fallible conversion with .unwrap(). Both follow from the standard conversion-trait contract: implement the most general trait, and surface (do not panic away) the error TryFrom exists to report.

What This Detects

The analyzer (analyze_conversion_traits) flags two shapes:

1. `impl Into<X> for Y`: recommend impl From<Y> for X. Detection anchors on a line-leading impl ... Into<...> for <Type>, so a generic bound T: Into<U> (which is correct) is never matched. clippy::from_over_into. 2. `.try_into().unwrap()` / `T::try_from(..).unwrap()` (and the .expect(..) variants): the fallible conversion's error is discarded. Recommend propagating it with ? or handling it.

Why From Over Into

The Rust API Guidelines (C-CONV-TRAITS) state the rule directly:

The following conversion traits should never be implemented: Into,

TryInto. These traits have a blanket impl based on From and

TryFrom. Implement those instead.

The std convert module docs give the mechanism:

As a library author, you should always prefer implementing From<T>

or TryFrom<T> rather than Into<U> or TryInto<U>, as From and

TryFrom provide ... equivalent Into or TryInto implementations

for free, thanks to a blanket implementation in the standard library.

Implementing From also composes with the ? operator: an error type that is From<E> is usable in ? through the standard error conversion, which an Into impl does not wire into.

// Flag: forfeits the From direction and ?-composition
impl Into<Settings> for Config {
    fn into(self) -> Settings { Settings { timeout: self.timeout } }
}
// Preferred: From gives Into for free
impl From<Config> for Settings {
    fn from(c: Config) -> Self { Settings { timeout: c.timeout } }
}

Why Not Discard the Conversion Error

TryFrom exists to make a fallible conversion's failure a value, not a panic. .try_into().unwrap() throws that value away:

// Flag: an over-large value panics instead of being handled
let port: u16 = raw.try_into().unwrap();
// Surface the error
let port: u16 = raw.try_into()?;

.unwrap() is acceptable only where the value is statically known to fit (tests, a checked invariant, a const); the reviewer confirms.

Exclusions (Not Flagged)

• Generic bounds: where T: Into<U> and fn f<T: Into<U>>(..) are

correct and idiomatic. Only line-leading impl Into blocks match.

• Foreign target type (orphan rule): `impl Into<ForeignType> for

Local cannot be rewritten as impl From<Local> for ForeignType when ForeignType is defined in another crate; the orphan rule forbids it, so Into` is the only legal direction (rust-clippy #9638, #6607). The detector cannot tell whether the target is foreign from one line, so the recommendation carries this caveat and the reviewer confirms.

• `impl From<..>`: already the preferred direction; never flagged.
• Propagated conversions: try_into()? keeps the error and is not

flagged.

• Comments: a line in a // comment is not code.

The known false-positive class is the foreign-target impl Into; the recommendation states the orphan-rule exception inline so the reviewer can dismiss it without re-deriving the rule.

Related Clippy Lints

Output Section

## Conversion Traits
### Issues Found
- [file:line] `impl Into<X> for Y`: implement `impl From<Y> for X`
  instead (From gives Into for free); exception if X is foreign
  (clippy::from_over_into)
- [file:line] `try_into().unwrap()`: discards the conversion error;
  propagate it with `?` (clippy::unwrap_used)

Exit Criteria

• [ ] Line-leading impl Into<X> for Y is flagged with a concrete

impl From<Y> for X recommendation and clippy::from_over_into

• [ ] Generic bounds T: Into<U> and impl From<..> are not flagged
• [ ] .try_into().unwrap() and T::try_from(..).unwrap() are flagged

as a discarded conversion error; try_into()? is not

• [ ] The foreign-target orphan-rule exception is documented in the

recommendation

• [ ] Lines in comments are not flagged

Duplicate Validators

Analysis of validate_*, check_*, and verify_* functions for opportunities to consolidate repeated validation logic.

What This Detects

Three or more functions sharing the same verb prefix (validate_, check_, verify_) within a single file, which often indicates copy-pasted validation logic that could be unified.

Why It Matters

Duplicated validation logic diverges over time: one copy gets a bug fix or a new rule while the others do not. Consolidating into a generic validator ensures all callers benefit from each fix.

Safe Patterns

// Good: single generic validator with rule injection
fn validate_field(value: &str, rules: &[ValidationRule])
    -> Result<(), ValidationError>
{
    for rule in rules {
        rule.apply(value)?;
    }
    Ok(())
}

Patterns to Flag

// Flag when 3+ share the same prefix:
fn validate_email(s: &str) -> bool { ... }
fn validate_phone(s: &str) -> bool { ... }
fn validate_username(s: &str) -> bool { ... }
fn validate_password(s: &str) -> bool { ... }

Output Section

## Duplicate Validators
### Issues Found
- [file] 4 validate_* functions: [list]

### Recommendations
- Extract shared logic into a generic validator
- Use a trait or rule-set parameter to unify related checks

Error Handling

Analysis of error handling patterns and correctness in Rust code.

Result and Option Usage

Evaluate:

• Result and Option usage patterns
• Custom error types design
• Context addition with anyhow or thiserror
• ? propagation correctness

Error Type Design

Check custom error types:

• Implements std::error::Error
• Provides meaningful context
• Conversion traits (From, Into)
• Error hierarchy structure

Error Propagation

Best practices:

// Good: Proper error propagation
fn process() -> Result<(), ProcessError> {
    let data = fetch().context("failed to fetch")?;
    validate(&data)?;
    Ok(())
}

Common Issues to Flag

• Panics in library code (unwrap, expect)
• Logging side-effects in error paths
• Mismatched error hierarchies
• Missing retry/backoff logic
• Silent error swallowing
• Over-generic error types

Error Context

Verify context is added:

• Operation context
• Input data context
• Failure reasons
• Recovery suggestions

Output Section

## Error Handling
### Issues Found
- [file:line] Panic in library: [details]
- [file:line] Missing context: [suggestion]

### Recommendations
- [error handling improvements]

Error Messages

Analysis of error and panic messages for actionability. Short messages without context make production incidents harder to diagnose.

What This Detects

String literals under roughly 20 characters used in:

• Err("short msg")
• panic!("short msg")
• .expect("short msg")
• Err("short msg".to_string())

Why It Matters

A message like "not found" or "failed" gives an on-call engineer no information about what was not found, where the failure occurred, or how to recover.

Safe Patterns

// Good: identifies operation and input
.expect("failed to open config file at $CONFIG_PATH")

// Good: Err with context
return Err(format!(
    "user {} not found in tenant {}",
    user_id, tenant_id
));

Patterns to Flag

// Bad: no context
.expect("failed")
.expect("not found")

// Bad: Err with bare short string
return Err("bad input");
return Err("denied".to_string());

Output Section

## Error Messages
### Issues Found
- [file:line] Short error message: [explanation]

### Recommendations
- Add operation context: what were you trying to do?
- Add input context: what value triggered the failure?
- Add recovery hints where possible

Float Equality

f32 and f64 are IEEE-754 types whose arithmetic rounds. Two values that are mathematically equal often differ in their last bits, so an exact == or != test silently does the wrong thing. This dimension flags equality comparisons against a floating-point literal so the author switches to a tolerance.

What This Detects

The analyzer (analyze_float_equality) flags == and != where one operand is a floating-point literal. A float literal is recognized by a decimal point (1.5, 0.0) or a float type suffix (2f32, 1.0f64). The literal may sit on either side of the operator:

• ratio == 1.5
• 3.14 == angle
• total != 0.0
• x == 2.0f32

An integer literal (count == 5) has neither a decimal point nor a float suffix and is left alone, as is every ordering comparison (x >= 1.5), which is well defined on floats.

Why Exact Comparison Misleads

The Rust Reference defines f32/f64 as IEEE-754 single and double precision (src/types/numeric.md). The classic demonstration is 0.1 + 0.2 != 0.3: each literal is rounded to the nearest representable value, the sum carries its own rounding, and the result is one bit away from the literal 0.3. The comparison compiles, runs, and quietly takes the wrong branch. Exact-zero checks (== 0.0) are sometimes intentional, but even those are usually better written against a small epsilon.

The Fix

// Flag: exact equality on a rounded value
if ratio == 1.5 { adjust(); }

// Checked: compare the magnitude of the difference to a tolerance
if (ratio - 1.5).abs() < f64::EPSILON { adjust(); }

// A domain epsilon is clearer when units have scale
const TOL: f64 = 1e-6;
if (measured - expected).abs() < TOL { accept(); }

Compare (a - b).abs() to f64::EPSILON (or f32::EPSILON) for values near 1.0, or to a domain-specific tolerance when the quantities are large or small. Where you truly need exact bits (round-trip serialization, a sentinel), compare the bit pattern with a.to_bits() == b.to_bits() and say why in a comment.

Exclusions (Not Flagged)

• Integer comparisons: count == 5 has no float literal.
• Ordering comparisons: <, <=, >, >= are well defined on

floats and never match.

• Ranges: 0.0..1.0 uses .., not an equality operator.
• Comments: a comparison shown in a full-line // comment is not

code. The exclusion anchors to the line start (^\s*//), so a trailing inline comment on a code line is still scanned.

A known limitation is a float literal inside a string on the same line; this conservative, line-based pass does not strip string contents.

Related Clippy Lints

Output Section

## Float Equality
### Issues Found
- [file:line] Exact float comparison `ratio == 1.5`; rounding makes
  `==` unreliable. Compare `(ratio - 1.5).abs() < f64::EPSILON`
  (clippy::float_cmp)

Exit Criteria

• [ ] ==/!= against a decimal float literal (1.5, 0.0) is flagged
• [ ] A type-suffixed literal (2f32, 1.0f64) is flagged
• [ ] A literal on either side of the operator is flagged
• [ ] Integer comparisons, ordering comparisons, and ranges are not

flagged

• [ ] Each finding names the epsilon/tolerance alternative and

clippy::float_cmp

Idiomatic Elision

Flags annotations the compiler already infers. Writing what elision supplies is noise: it hides the signature's real shape and drifts out of sync. Two ports are detected, both grounded in the Rust Reference.

What This Detects

The analyzer (analyze_idiomatic_elision) flags:

1. Needless lifetimes: a function with a single named lifetime and no type parameters where exactly one input reference uses the lifetime and the output reuses it. Lifetime elision already assigns that input lifetime to the output, so the annotation is redundant (clippy::needless_lifetimes). 2. Needless return: a return <expr>; in the function tail position. A block's final expression (no semicolon) is its value, so the explicit return is redundant (clippy::needless_return). 3. Unused unit return: an explicit -> () return type. The unit return is the default and is elided, so -> () writes what the compiler already infers (clippy::unused_unit).

Lifetime Elision

The Rust Reference (src/lifetime-elision.md) gives three function rules. Two produce the common redundant annotations:

• Rule 2 (single input): if exactly one lifetime appears in the

parameters, it is assigned to all elided output lifetimes.

• Rule 3 (`&self`): with a &self/&mut self receiver, self's

lifetime is assigned to all elided output lifetimes.

// Flag: one input lifetime, reused on the output (rule 2)
fn substr<'a>(s: &'a str, until: usize) -> &'a str { &s[..until] }
// Elided:
fn substr(s: &str, until: usize) -> &str { &s[..until] }

// Flag: receiver lifetime, reused on the output (rule 3)
fn name<'a>(&'a self) -> &'a str { self.name }
// Elided:
fn name(&self) -> &str { self.name }

Where a lifetime is still required in a path, the Reference prefers the anonymous placeholder '_ over inventing a named one:

fn parser(&self) -> Parser<'_> { /* ... */ }   // not Parser<'a>
impl fmt::Debug for Wrapper<'_> { /* ... */ }

Needless Return

A block's tail expression (the final operand with no trailing semicolon) is the block's value, per src/expressions/block-expr.md. Adding a semicolon turns it into a statement of unit type, so the trailing return ...; is redundant:

// Flag
fn add(a: i32, b: i32) -> i32 {
    return a + b;
}
// Idiomatic
fn add(a: i32, b: i32) -> i32 {
    a + b
}

Unit Return

A function with no -> Type returns (), the unit type. The Rust Reference makes the unit return the default, so an explicit -> () states what is already inferred and reads as noise (clippy::unused_unit):

// Flag
fn log(msg: &str) -> () {
    println!("{msg}");
}
// Idiomatic
fn log(msg: &str) {
    println!("{msg}");
}

This is the return-type analogue of elision: just as the compiler infers an output lifetime, it infers the unit return. Type-inference elision extends the same idea to bindings: a redundant turbofish or annotation (let v: Vec<u8> = Vec::<u8>::new();) repeats a type the compiler already fixes from the other side; prefer one or the other.

Exclusions (Not Flagged)

• Load-bearing lifetimes: two input references tied to the same

lifetime (fn longest<'a>(x: &'a str, y: &'a str) -> &'a str) cannot be elided; elision would give the inputs distinct lifetimes. The annotation carries meaning, so it is left alone.

• Type parameters present: a signature like fn f<'a, T>(...) is

skipped; trait bounds can require the explicit lifetime, so this is the conservative case.

• Early/guard returns: a return followed by more code is control

flow, not a tail expression, and is never flagged.

• Bare `return;`: only return <expr>; is flagged; an early bare

return; is left to other lints.

Related Clippy Lints

Output Section

## Idiomatic Elision
### Issues Found
- [file:line] Needless lifetime: elide it; one input lifetime is
  assigned to the output (clippy::needless_lifetimes)
- [file:line] Needless return: drop `return`; the tail expression is
  the value (clippy::needless_return)
- [file:line] Unused unit return: drop `-> ()`; the unit return is the
  elided default (clippy::unused_unit)

Exit Criteria

• [ ] Single-input-lifetime signatures whose output reuses the lifetime

are flagged with an elision recommendation

• [ ] Two-input shared-lifetime and type-parameter signatures are not

flagged (load-bearing / conservative skip)

• [ ] Trailing return <expr>; is flagged; early guard returns and

bare return; are not

• [ ] Explicit -> () unit return types are flagged

(clippy::unused_unit); functions with a real return type are not

• [ ] The '_ anonymous-lifetime preference for paths is documented

Iterator and Allocation Slop

AI-generated Rust defaults to manual loops where iterators read better, and to allocation where references suffice. Both compile; both are slop.

This module covers two of the highest-frequency AI Rust anti-patterns: imperative loops the iterator API expresses in one line, and unnecessary allocation that borrow-checker capitulation produces. The clippy lints catch most of this; the rest is judgment.

Iterator slop

Pattern 1: index-based loops

// SLOP
let mut sum = 0;
for i in 0..vec.len() {
    sum += vec[i];
}

// Idiomatic
let sum: i32 = vec.iter().sum();

Detector: clippy::needless_range_loop.

Pattern 2: filter-then-push

// SLOP
let mut result = Vec::new();
for x in xs.iter() {
    if x.is_active() {
        result.push(x.id);
    }
}

// Idiomatic
let result: Vec<_> = xs.iter()
    .filter(|x| x.is_active())
    .map(|x| x.id)
    .collect();

Pattern 3: map-filter-unwrap

// SLOP
let firsts: Vec<_> = xs.iter()
    .map(|x| x.first())
    .filter(|x| x.is_some())
    .map(|x| x.unwrap())
    .collect();

// Idiomatic
let firsts: Vec<_> = xs.iter()
    .filter_map(|x| x.first())
    .collect();

Detector: clippy::manual_filter_map.

Pattern 4: collect-then-iterate

// SLOP
let intermediate: Vec<_> = xs.iter().map(transform).collect();
for item in intermediate {
    use_it(item);
}

// Idiomatic
for item in xs.iter().map(transform) {
    use_it(item);
}

Detector: clippy::needless_collect.

Pattern 5: bool-match where if suffices

// SLOP
match flag {
    true => do_a(),
    false => do_b(),
}

// Idiomatic
if flag { do_a() } else { do_b() }

Detector: clippy::match_bool.

Allocation slop

Pattern A: .clone() to satisfy the borrow checker

The single most common AI-generated Rust anti-pattern. The rust-unofficial/patterns book lists it as the canonical anti-pattern: cloning to make a borrow-checker error go away rather than to express ownership.

// SLOP
fn greet(name: String) {
    println!("Hello, {name}");
}
fn main() {
    let n = String::from("world");
    greet(n.clone());      // unnecessary clone
    greet(n.clone());
}

// Idiomatic
fn greet(name: &str) {
    println!("Hello, {name}");
}
fn main() {
    let n = String::from("world");
    greet(&n);
    greet(&n);
}

Detection heuristic: any .clone() on a String, Vec<_>, HashMap<_,_>, Arc<Mutex<_>>, or large struct that is not paired with a comment explaining the ownership rationale. If it disappeared, would the borrow checker complain? If yes, the right fix is usually to take a borrowed reference (&str, &[T], &T).

Detectors: clippy::redundant_clone, clippy::clone_on_ref_ptr.

Pattern B: owned parameters that should borrow

Detector: clippy::ptr_arg catches &Vec<T> and &String.

Pattern C: format! then convert

// SLOP
let s = format!("{}", x);

// Idiomatic (when Display is implemented)
let s = x.to_string();

// Inverse SLOP
let s = x.to_string();
let s = format!("{s}{rest}");

// Idiomatic (build the string once)
let s = format!("{x}{rest}");

Detectors: clippy::useless_format, clippy::str_to_string.

Pattern D: redundant allocation

// SLOP
let owned = borrowed.to_owned();
fn take_str(s: &str) { ... }
take_str(&owned);

// Idiomatic — borrow directly
take_str(borrowed);

// SLOP
let s = String::new();
let s = s + "hello" + " " + "world";

// Idiomatic
let s = String::from("hello world");

Pattern E: Box::new(...) without indirection reason

// SLOP
let x = Box::new(42_u64);
fn use_it(n: u64) { ... }
use_it(*x);

// Idiomatic
let x = 42_u64;
use_it(x);

Heap allocation is justified for:

• dyn Trait objects (Box<dyn Error>, Box<dyn Future>).
• Recursive types (Box<Node>).
• Large stack-frame avoidance (uncommon; measure first).
• Pinning requirements (Pin<Box<T>>).

Anywhere else, Box::new is unjustified allocation.

Pattern E2: Box<dyn Trait> or &dyn Trait in a hot inner loop

This is distinct from Pattern E. The box itself may be justified: the problem is calling a dyn method millions of times when the method body is tiny.

// Potentially slow: dyn dispatch in the inner loop
let decoders: Vec<Box<dyn ColumnDecoder>> = build_decoders(&schema, &batch);

for i in 0..n_rows {
    for d in &decoders {
        d.write_to_row(i, &mut row);  // indirect call every iteration
    }
}

Why it matters: each dyn call goes through a vtable (call *0x18(%rax)). The compiler cannot inline across that boundary, so it cannot fuse the inner loop, vectorize small stores, or eliminate the function-call prologue/epilogue overhead. When the method body is ~25 instructions (a null check, a bit flip, a 4-byte move), the prologue/epilogue and indirect jump overhead can represent 40–50% of total runtime.

Note: &dyn Trait has the same problem as Box<dyn Trait>. The issue is dynamic dispatch, not heap allocation.

Fix 1: flip the loop order (batch-first): iterate all rows for each decoder, not all decoders for each row. The dyn dispatch cost is paid once per decoder per batch instead of once per cell:

let mut rows: Vec<WriteRow> = (0..n_rows)
    .map(|i| WriteRow::new(&segment, key_array.value(i)))
    .collect();

for d in &decoders {
    d.write_to_rows(0, &mut rows[..]);  // dispatch once per decoder
}

Fix 2: enum dispatch: replace dyn Trait with a closed enum. The compiler can monomorphize and inline each variant:

enum ColDecoder {
    F32(F32Decoder),
    Utf8(Utf8Decoder),
    Bool(BoolDecoder),
}

impl ColDecoder {
    #[inline(always)]
    fn write_to_row(&self, index: usize, row: &mut WriteRow) {
        match self {
            ColDecoder::F32(d) => d.write_to_row(index, row),
            ColDecoder::Utf8(d) => d.write_to_row(index, row),
            ColDecoder::Bool(d) => d.write_to_row(index, row),
        }
    }
}

When to flag: any Vec<Box<dyn Trait>> or Vec<&dyn Trait> iterated inside an inner loop where the method body is inlineable. The Java/JVM analogy is instructive: the JVM de-virtualizes and inlines at JIT time; rustc cannot cross the dyn boundary. If reviewers come from JVM backgrounds, this is the most important Rust performance lesson to surface.

Detection:

# Find Vec<Box<dyn>> that appear inside nested loops
rg "Vec<Box<dyn" --type rust -n

# Find dyn method calls inside for loops (heuristic)
rg -A 5 "for .* in" --type rust | rg "\.write_to|\.decode|\.encode|\.process"

Pattern F: Vec for fixed small set

// SLOP
let primes: Vec<u32> = vec![2, 3, 5, 7, 11];

// Idiomatic for small, fixed-size
let primes: [u32; 5] = [2, 3, 5, 7, 11];

// Or for stack-allocated growable
let primes: SmallVec<[u32; 5]> = smallvec![2, 3, 5, 7, 11];

This one is judgment: arrays for compile-time-known sizes, SmallVec/ArrayVec for "usually small but sometimes grows", Vec for genuinely dynamic.

Detection commands

# Catch most iterator slop with clippy
cargo clippy --all-targets -- \
  -W clippy::needless_range_loop \
  -W clippy::manual_filter_map \
  -W clippy::needless_collect \
  -W clippy::filter_map_next \
  -W clippy::map_unwrap_or \
  -W clippy::match_bool \
  -W clippy::needless_match \
  -D warnings

# Catch most allocation slop with clippy
cargo clippy --all-targets -- \
  -W clippy::redundant_clone \
  -W clippy::clone_on_ref_ptr \
  -W clippy::ptr_arg \
  -W clippy::useless_format \
  -W clippy::str_to_string \
  -W clippy::redundant_allocation \
  -D warnings

# Manual scan: every .clone() in the codebase
rg "\.clone\(\)" --type rust -n | head -50
# For each: ask "would the borrow checker complain if removed?"

False positives

Some .clone() calls are correct and should stay:

• Async / spawn boundaries: cloning an Arc<T> to

send into tokio::spawn is required, not slop.

• Genuine ownership transfer: when two callers each

need to mutate independently from the same source.

• Intentional defensive copy: in security-sensitive

paths where the original might be modified by an attacker. Mark with // COPY: defense against ....

When in doubt, comment the rationale next to the .clone(). A .clone() with a one-line "why" comment is documented intent; a .clone() with no comment is slop.

Output format

For each finding, use the Skill(scribe:slop-detector) module structured-finding-output.md format. Severity is medium for individual iterator/allocation slop; high if the same pattern appears 5+ times in the same file (suggests systematic AI-generation rather than isolated mistake).

Integration

Iterator and allocation slop typically lands in Pass 5 (code idiom sweep) of the multi-pass cleanup workflow (see Skill(scribe:slop-detector) module cleanup-workflow.md). Run after the linter floor (Pass 1) clears, since clippy will flag most of these automatically.

Match Wildcard

A match over an enum is one of Rust's best safety nets: add a variant, and every non-exhaustive match becomes a compile error pointing you at the code to update. A wildcard _ => arm cuts that net. This dimension flags the catch-all arms that trade a compile error for a runtime panic or a silent drop.

What This Detects

The analyzer (analyze_match_exhaustiveness) flags three wildcard arm shapes:

1. `_ => unreachable!(...)`: claims the wildcard can never be hit. Add a variant and the claim is false: it panics at runtime instead of failing to compile. 2. `_ => panic!(...)` / `todo!()` / `unimplemented!()`: the same exhaustiveness defeat, hidden behind a deliberate crash. 3. `_ => {}`: an empty arm that silently swallows every unmatched case; a new variant is dropped with no trace.

A _ => arm that returns a real value (_ => 0, _ => Color::Black) is not flagged: that is a legitimate default, common and correct over open sets like integers, char, and strings.

Why Exhaustiveness Matters

The Rust Reference match chapter (src/expressions/match-expr.md) requires match arms to be exhaustive, and a wildcard _ pattern satisfies that by matching everything left over. That is exactly the problem for a closed set like an enum: the compiler can prove you covered every variant, but a _ arm tells it not to bother. The protection you paid for, an error when the type grows, is gone.

The Fix

// Flag: a new Shape variant becomes a runtime panic
match shape {
    Shape::Circle(r) => area_circle(r),
    Shape::Square(s) => s * s,
    _ => unreachable!(),
}

// Exhaustive: adding Shape::Triangle is now a compile error
match shape {
    Shape::Circle(r) => area_circle(r),
    Shape::Square(s) => s * s,
    Shape::Triangle(b, h) => 0.5 * b * h,
}

When you truly want a default for several variants, name them or use an or-pattern (Shape::Circle(_) | Shape::Square(_) => ...) so the set stays explicit. If a no-op is intentional, list the variants and add a comment, rather than letting _ => {} absorb future ones.

Exclusions (Not Flagged)

• Default values: _ => <expr> returning a real value is a normal

open-set default (integers, char, &str).

• Named catch-alls: other => handle(other) binds the value and is

not a bare wildcard.

• Comments: a wildcard arm shown in a // or /// comment is not

code.

Related Clippy Lints

Enabling #[deny(clippy::wildcard_enum_match_arm)] on enum-heavy modules is the durable enforcement once the existing arms are fixed.

Output Section

## Match Wildcard
### Issues Found
- [file:line] `_ => unreachable!()`: a new enum variant becomes a
  runtime panic; list the variants explicitly
  (clippy::wildcard_enum_match_arm)
- [file:line] `_ => {}`: empty wildcard silently drops unmatched cases
  (clippy::wildcard_enum_match_arm)

Exit Criteria

• [ ] _ => unreachable!() arms are flagged as wildcard_unreachable
• [ ] _ => panic!/todo!/unimplemented! arms are flagged as

wildcard_panic

• [ ] _ => {} empty arms are flagged as wildcard_empty_arm
• [ ] _ => arms returning a real value and named catch-alls are not

flagged

• [ ] The open-set exclusion (integers/char/strings) is documented

Mem Forget Audit

Rust runs destructors deterministically at end of scope. Two calls quietly defeat that guarantee: mem::forget(x) skips the destructor and leaks whatever the value owns, and drop(&x) drops a reference, which is a no-op that leaves the owned value alive. This dimension flags both so the reviewer can confirm the cleanup is intentional.

What This Detects

The analyzer (analyze_mem_forget) flags two shapes:

1. `mem::forget`: a forget(x) call, whether written mem::forget, std::mem::forget, or an imported bare forget. The value is consumed but its destructor never runs. 2. `drop(&x)`: a drop(...) call whose argument starts with &. It drops the borrow, not the value, so the destructor still has not run and the resource lives on.

A method call (cache.forget(key), guard.drop()) is left alone: the regex requires a non-. character before the name. An owning drop(value) is the correct idiom and is not flagged.

Why This Leaks or No-ops

The Rust Reference destructors chapter (src/destructors.md) describes the automatic, scope-based destructor that mem::forget opts out of. Forgetting a File leaks the descriptor, forgetting a MutexGuard poisons nothing but never unlocks, and forgetting a Box leaks the allocation. forget is safe (leaking is not unsound), which is exactly why it slips through review.

drop(&x) is the mirror-image mistake: drop consumes its argument, and a shared reference is Copy, so dropping the reference compiles and does nothing. The author believes the value is gone; it is not.

The Fix

// Flag: forget skips the destructor and leaks
mem::forget(guard);

// Defer cleanup explicitly with ManuallyDrop
let mut slot = ManuallyDrop::new(guard);
// ... later, when you have decided ownership ...
unsafe { ManuallyDrop::drop(&mut slot) };

// Or hand the resource across an FFI boundary as a raw pointer,
// documenting who frees it.
let raw = Box::into_raw(boxed);   // not mem::forget(boxed)

// Flag: dropping a reference is a no-op
drop(&resource);
// Drop the owned value (or just let it fall out of scope)
drop(resource);

Use ManuallyDrop when cleanup must be deferred, Box::into_raw / into_raw_fd when handing ownership across a boundary, and plain scope exit otherwise. For drop, pass the owned value, not a borrow.

Exclusions (Not Flagged)

• Method calls: cache.forget(key), guard.drop() are user methods,

not the std functions.

• Owning drop: drop(value) (no leading &) is the correct way to

end a value early.

• Comments: a forget shown in a full-line // comment is not code.

The exclusion anchors to the line start (^\s*//), so a trailing inline comment on a code line is still scanned.

Related Clippy Lints

Output Section

## Mem Forget Audit
### Issues Found
- [file:line] `mem::forget` skips the destructor and leaks the resource;
  use `ManuallyDrop` or let the value drop at scope end
  (clippy::mem_forget)
- [file:line] `drop(&x)` drops a reference (a no-op); drop the owned
  value `drop(x)` (clippy::drop_ref)

Exit Criteria

• [ ] mem::forget(...) and an imported bare forget(...) are flagged as

mem_forget

• [ ] drop(&x) is flagged as drop_ref
• [ ] Method calls (cache.forget(...)) and owning drop(value) are not

flagged

• [ ] A forget inside a // comment is not flagged
• [ ] Each finding names the ManuallyDrop/scope alternative and the

clippy lint

Model-Specific Tells

Calibrate the audit weighting to the model that generated the code. GPT fabricates; Claude omits; reasoning-mode amplifies both.

This module is meta-guidance: it tells the rust-review audit which of its other modules to weight more heavily based on which model produced the code under review. If you cannot determine the model, run the full audit. The 2025-Q1 2026 cross-evaluation work (Sonar leaderboard, Anthropic and DEV.to benchmarks) makes the calibration defensible.

GPT-5.x family (Codex, direct API)

Default failure mode: fabrication. Confidently invents function names, library methods, config keys, API endpoints, and lint names that look plausible but do not exist.

Audit weighting:

• High: verify every use statement resolves. Verify

every Cargo.toml entry exists on crates.io. Verify every method call against the relevant crate version. Verify every cfg(...) flag is actually defined. Verify every #[clippy::...] lint name appears in the upstream lint index.

• High: concurrency mistakes. Sonar measured ~470

concurrency issues per million lines (MLOC) for GPT-5.2 High, nearly 2x the next-closest model. Audit Send/Sync bounds, MutexGuard lifetimes across .await, channel patterns, Arc cycles, and atomics. See concurrency-patterns.md.

• Medium: over-thorough doc-comment headers (parameter

tables and usage examples on trivial functions). Trim; see error-messages.md for the doc-comment-bloat rule.

• Medium: Python-style defensive patterns (try-everything,

log-everything) translated mechanically into Rust Result chains. Collapse with ? or fold into typed errors. See error-handling.md.

Detection commands:

# Verify every use statement
rg "^use\s+" --type rust | sort -u > used.txt
# Cross-reference with declared deps
rg '^[a-z_]+\s*=' Cargo.toml | awk -F'=' '{print $1}' | tr -d ' "' | sort -u > deps.txt
# Diff (any dep used but not declared = potential phantom)

# Verify every cfg flag has a setter
rg "cfg\(([a-z_]+)\)" -o --no-filename --type rust | sort -u

# Verify every clippy::* lint name (cross-reference against upstream index)
rg "clippy::[a-z_]+" -o --no-filename --type rust | sort -u

Claude 4.x family (Sonnet, Opus, Claude Code)

Default failure mode: omission. Silently skips edge cases, drops match arms, leaves None paths unhandled, defaults to unreachable!() without justification.

Audit weighting:

• High: non-exhaustive match arms. Sonar measured

resource-management leaks at ~195/MLOC for Sonnet 4.5, nearly 4x the GPT-5.1 baseline. Audit explicit Drop order, File/socket close paths, scope-bound locks, any custom Drop impl.

• High: missing error paths. Look for Option::None

handled as unreachable!() without a // SAFETY: or invariant comment justifying the unreachability.

• High: edge cases not in the test suite. The

diagnostic question: for every input domain, what happens? If the test suite mirrors the implementation (asserts the same paths the implementation takes), the omitted cases are not covered. Use mutation testing (cargo mutants) to expose this.

• Medium: "behavior-preserving refactors" that leave

dead branches in place because the model copied surrounding idioms verbatim. Read commit messages skeptically.

• Medium: safety-caveat-heavy doc comments on

innocuous functions. Strip the caveats; keep the # Safety block only when actual unsafety exists.

Detection commands:

# Look for unreachable!() and panic!("not reachable") without SAFETY/invariant
rg -B 2 'unreachable!\(\)' --type rust | rg -v 'SAFETY|INVARIANT|invariant'

# Find non-exhaustive match arms (likely false positives, surface only)
rg "match\s+\w+\s*\{" --type rust -A 20 | grep -B 2 "_ =>" | head -40

# Custom Drop impls (high blast-radius for resource leaks)
rg "impl Drop for" --type rust

# Mutation testing exposes omission
cargo mutants --workspace 2>/dev/null | head -50

Gemini 3.x

Default failure mode: control-flow errors. Sonar measured ~200 control-flow mistakes per MLOC, ~4x Opus 4.5 Thinking. Off-by-one loops, missed early returns, incorrect match arms, monolithic functions.

Audit weighting:

• High: branch correctness in any function over ~50

lines. Read every branch top-to-bottom; check for the early-return that should have been there.

• High: off-by-one in any loop with manual indexing

(for i in 0..n vs. for x in slice). See collection-types.md for the clippy::needless_range_loop detector.

• Medium: monolithic functions (>100 lines) that

could be refactored into named helpers.

Any "Thinking" / "Reasoning" / "High-effort" mode

Verbosity scales with reasoning depth. The Sonar leaderboard plot puts bubble size = verbosity, x-axis = pass rate; both grow together. Reasoning-mode output needs aggressive trimming by default. The reasoning surfaces in:

• Higher cyclomatic complexity per function.
• Longer parameter lists.
• More cleverness (lifetime tricks, impl Trait

combinators where a concrete type would do).

• More layers of abstraction.

All compound the maintenance bill. Read reasoning-mode output as a first draft, not as production code.

Detection: run cargo clippy --all-targets -- -W clippy::cognitive_complexity -W clippy::too_many_lines and treat the resulting warnings as load-bearing, not cosmetic.

When you cannot tell which model

Default to running the full audit. It is never wrong, just sometimes redundant.

Heuristic for inferring the model from artifacts:

These are heuristics rather than proofs. Use them to weight the audit, not to attribute authorship.

Currency note

The model-specific multipliers above are tied to specific model versions and dates (Q4 2025 / Q1 2026). Model behavior changes faster than this module can. The pattern (model families have predictable failure profiles, reasoning amplifies verbosity) will persist; the specific numbers will not.

Re-validate against:

• Sonar's live LLM leaderboard.
• Latest CodeRabbit / Apiiro / Veracode reports.
• Your own internal defect data.

See Skill(scribe:slop-detector) module empirical-baseline.md for the full citation list and the language-agnostic version of this calibration.

Mutable Static Audit

static mut is shared mutable global state with no synchronization. It needs unsafe to touch, and every touch is a promise the author upholds the aliasing and thread-safety rules by hand. This dimension flags every static mut declaration and points at the safe replacement.

What This Detects

The analyzer (analyze_mutable_statics) flags any static mut declaration, including with leading visibility qualifiers:

static mut COUNTER: u64 = 0;          // flagged
pub static mut REGISTRY: *mut u8 = core::ptr::null_mut();  // flagged

A plain static (immutable) and a const are not flagged; only the mut form is shared mutable state.

Why static mut Is Dangerous

The Rust Reference static items chapter (src/items/static-items.md) is explicit:

• "an unsafe block is required when either reading or writing a

mutable static variable."

• Mutable statics exist because "one of Rust's goals is to make

concurrency bugs hard to run into, and this is obviously a very large source of race conditions or other bugs."

Since Rust 2024, taking a reference to a static mut is the deny-by-default static_mut_refs lint: the pattern is being designed out of the language because creating a &/&mut to one is almost always undefined behavior in the presence of any concurrency or re-entrancy.

The Fix

Pick the synchronized primitive that matches the access pattern:

use std::sync::atomic::{AtomicU64, Ordering};
static COUNTER: AtomicU64 = AtomicU64::new(0);   // counters / flags
COUNTER.fetch_add(1, Ordering::Relaxed);

use std::sync::OnceLock;
static CONFIG: OnceLock<Config> = OnceLock::new();  // set-once globals
CONFIG.get_or_init(Config::load);

use std::sync::Mutex;
static STATE: Mutex<State> = Mutex::new(State::new());  // shared data
STATE.lock().unwrap().update();

• `OnceLock` / `LazyLock`: a global initialized once, read freely

after.

• *`Atomic`**: lock-free counters, flags, and ids.
• `Mutex` / `RwLock`: arbitrary shared mutable data with

synchronized access.

Each is safe code, needs no unsafe, and the compiler enforces the threading contract instead of the author.

Exclusions (Not Flagged)

• Immutable `static`: read-only global data is Sync and safe.
• `const`: a compile-time constant, not a single-address global.
• Comments: static mut mentioned in a // or /// comment is

not a declaration.

Related Lints

Output Section

## Mutable Static Audit
### Issues Found
- [file:line] `static mut` shared mutable global: replace with
  `OnceLock`/`LazyLock`, an `Atomic*`, or a `Mutex`/`RwLock`
  (deny-by-default static_mut_refs)

Exit Criteria

• [ ] Every static mut declaration (with or without pub) is flagged
• [ ] Immutable static and const declarations are not flagged
• [ ] static mut inside a comment is not flagged
• [ ] Each finding names a thread-safe alternative and the

static_mut_refs lint

Native Type Modeling

Detects code that compares or branches on bare primitives where a native Rust type would let the compiler check every case. The companion builtin-preference.md module covers conversion traits and combinators; this module covers modeling state with the type system: enums for comparison, the newtype pattern, type-state, and derived ordering traits.

What This Detects

The analyzer (analyze_native_type_modeling) flags two crisp, low-false-positive ports:

1. Stringly-typed comparison: status == "active", mode != "fast". A value compared against a string literal is a missing enum. Model the states as an enum and compare with the matches! macro or derived ==. 2. Boolean blindness: a function signature carrying two or more : bool parameters, like fn paint(immediate: bool, antialias: bool). Call sites such as paint(true, false) are unreadable and silently transposable. Replace each flag with a two-variant enum. 3. Integer state constants: a named constant with a state-ish stem (STATUS, STATE, MODE, KIND, PHASE, STEP) and a plain decimal value, like const STATUS_ACTIVE: u8 = 0;. A group of these is a C-style enum waiting to happen. Detection requires a decimal literal so bitmask shifts (1 << 2) and hex are left for bitflags, not mis-flagged as enums.

Newtype and type-state are covered as guidance below rather than auto-flagged: reliable static detection produces too many false positives at I/O and storage boundaries.

Why It Matters

Enums turn runtime string and integer checks into compile-time exhaustiveness checks. Adding a variant produces a compile error at every match site instead of a silent fallthrough. This is the "make invalid states unrepresentable" principle: it predates Rust, coined by Yaron Minsky (Jane Street, OCaml) and popularized by Scott Wlaschin (F# for Fun and Profit / Domain Modeling Made Functional), and reached the Rust community through Alexis King's "Parse, Don't Validate." A product of boolean flags is representationally complete but semantically loose: it admits combinations that type-check but have no domain meaning, and each is a latent bug.

1. Enums for Comparison

// Flag: stringly typed — the compiler cannot check the cases
fn is_billable(status: &str) -> bool {
    status == "active" || status == "trial"
}

// Native: enum + matches!, exhaustive and self-documenting
enum Status { Active, Trial, Suspended, Cancelled }

fn is_billable(status: Status) -> bool {
    matches!(status, Status::Active | Status::Trial)
}

matches! (stable since Rust 1.42, no import) returns bool for a pattern check and composes inside .filter() and while conditions. Use it instead of a match whose arms only return true/false (clippy::match_like_matches_macro). Use a full match or if let when you must bind data out of the variant.

For a public library enum that may gain variants later, mark it #[non_exhaustive] so downstream match arms must include a _ wildcard and adding a variant is not a breaking change. Do not apply #[non_exhaustive] to crate-internal enums: it forces wildcard arms that defeat the exhaustiveness check you want.

2. Boolean Blindness

// Flag: two bool params — paint(true, false) is unreadable
fn paint(immediate: bool, antialias: bool) { /* ... */ }

// Native: enums make call sites self-documenting
enum PaintMode { Immediate, Deferred }
enum AntiAlias { On, Off }
fn paint(mode: PaintMode, aa: AntiAlias) { /* ... */ }
// paint(PaintMode::Immediate, AntiAlias::Off)

clippy::fn_params_excessive_bools warns past a threshold of bool parameters. A single, genuinely binary, self-evident parameter (set_visible(bool)) is idiomatic and is not flagged.

3. Integer State Constants

// Flag: C-style integer constants standing in for an enum
const STATUS_ACTIVE: u8 = 0;
const STATUS_SUSPENDED: u8 = 1;
const STATUS_CLOSED: u8 = 2;

// Native: an enum the compiler checks; add discriminants only when
// the on-the-wire integer values matter
#[repr(u8)]
enum Status { Active = 0, Suspended = 1, Closed = 2 }

Detection requires a state-stemmed name and a plain decimal value, so a composable bitmask is left alone:

// Not flagged: a mask is a `bitflags` candidate, not an enum
const STATE_MASK: u32 = 1 << 2;

Use the bitflags crate (or an explicit #[repr] enum with discrete discriminants) when the values combine; reach for a plain enum when they are mutually exclusive states.

4. Newtype Pattern (Guidance)

Wrap a primitive in a single-field tuple struct to get a distinct, zero-cost type that is not interchangeable with the underlying type (unlike a type alias):

struct Miles(f64);
struct Kilometers(f64);
// add_distance(miles, km) no longer compiles by accident

A newtype enforces an invariant only when its field is private and the only constructor validates:

mod email {
    pub struct Email(String);          // field private to the module
    impl Email {
        pub fn new(s: &str) -> Result<Self, Invalid> { /* validate */ }
    }
}

A name alone is not type safety (Alexis King, "Names are not type safety"): Email(raw) from inside the module still bypasses the check, so keep the field private and route callers through new.

5. Type-State (Guidance)

Encode lifecycle state in the type so invalid operations do not compile. A Connection<Authenticated> exposes request(); a Connection<Disconnected> does not. Strom & Yemini introduced typestate in 1986; Cliffle's "The Typestate Pattern in Rust" is the canonical Rust reference.

Reserve type-state for safety-critical or protocol APIs. Multiple practitioners (corrode.dev, Cliffle, greyblake/nutype) independently warn it raises cognitive load, yields obscure compiler errors, grows binary size through monomorphization, and is awkward with collections of mixed-state items. When transitions are runtime-determined, use a plain enum state machine instead.

6. Derived Ordering And Comparison

Derive PartialEq/Eq/PartialOrd/Ord/Hash/Default rather than hand-writing them, and replace three-way if/else if ladders with a match on Ordering:

// Flag: comparison chain (clippy::comparison_chain)
if x > y { a() } else if x < y { b() } else { c() }

// Native: exhaustive, evaluates the comparison once
match x.cmp(&y) {
    Ordering::Greater => a(),
    Ordering::Less => b(),
    Ordering::Equal => c(),
}

Hand-write an impl only when semantics differ from the field-wise derive (a non-zero Default, an Ord that ignores a cache field). Keep Hash and Eq consistent: deriving one and hand-writing the other can violate the Hash/Eq contract.

Exclusions (Not Flagged)

These guardrails come from cross-source practitioner consensus; a finding that hits one of them is a false positive.

• Storage and protocol boundaries: you still parse untyped

external data (JSON strings, DB integers, CLI args) into the enum once at the edge. The principle governs the internal domain model, not the wire or serialization format. A permissive type at a serde/DB boundary is correct, not a defect.

• Empty-string checks (s == ""): an emptiness test, not a

stringly-typed enum candidate (prefer .is_empty() separately).

• Single self-evident bool: one binary local flag does not need

an enum.

• Genuinely evolving business rules: encode only what is *truly

impossible, not what is merely currently disallowed*. Hard-coding every transient rule into types creates migration cost ("'Make invalid states unrepresentable' considered harmful").

• Open/extensible variant sets: when downstream crates must add

cases, a trait object beats an enum.

Related Clippy Lints

Output Section

## Native Type Modeling
### Issues Found
- [file:line] Stringly-typed comparison: model as enum, compare with
  `matches!` (clippy::match_like_matches_macro)
- [file:line] Boolean blindness (2 bool params): take two-variant
  enums (clippy::fn_params_excessive_bools)

### Recommendations
- Model internal state as enums; parse to them once at the boundary
- Keep newtype fields private with validating constructors
- Reserve type-state for safety-critical/protocol APIs

Exit Criteria

• [ ] Stringly-typed comparisons (x == "literal") are flagged with

an enum + matches! recommendation

• [ ] Function signatures with two or more : bool params are flagged

as boolean blindness; single-bool signatures are not

• [ ] Empty-string comparisons and matches! lines are not flagged
• [ ] Newtype, type-state, and derived-ordering guidance is present

with explicit storage-boundary and over-application exclusions

Numeric Cast Safety

The as operator never fails and never warns. It silently truncates, wraps, changes sign, and drops precision. This dimension flags the casts where the shape alone proves a loss is possible, so the author can reach for a checked conversion instead.

What This Detects

The analyzer (analyze_numeric_cast_safety) flags three shapes:

1. Length truncation: .len(), .count(), or .capacity() (each returns usize, 64-bit on common targets) cast to a narrower fixed-width integer, e.g. buf.len() as u32. An over-large length wraps to a small wrong value (clippy::cast_possible_truncation). 2. Narrowing to a byte: any as u8 / as i8. Nothing is narrower than a byte, so the cast cannot be a lossless widening; it truncates the upper bits, and u8/i8 swaps reinterpret sign (clippy::cast_possible_truncation). 3. Precision loss to `f32`: any as f32. From f64 it drops mantissa bits; from i64/u64 it loses precision past 2^24 (clippy::cast_precision_loss).

Why as Hides Bugs

The Rust Reference type-cast rules (src/expressions/operator-expr.md) spell out the silent behavior:

• "Casting from a larger integer to a smaller integer (e.g. u32 ->

u8) will truncate."

• Float-to-int "rounds towards zero", NaN becomes 0, and

out-of-range values "saturate" to the type's min/max instead of erroring.

• Integer-to-float "produces the closest possible float", which past

2^24 (f32) or 2^53 (f64) is not the original value.

None of this is a compile error and none of it warns by default, so a truncating cast reads as deliberate even when it is a bug.

The Fix

// Flag: a usize length truncated to u32
let n = data.len() as u32;
// Checked: an over-large length is an error, not a wrapped value
let n = u32::try_from(data.len())?;

// Flag: narrowing to a byte
let b = value as u8;
let b = u8::try_from(value)?;

// Lossless widening uses From/Into, never `as`
let wide: u64 = small.into();      // not `small as u64`

Use TryFrom/try_into when the conversion can fail (narrowing) and From/into when it cannot (widening). Keep as only where truncation is the documented intent (e.g. hashing, deliberate & 0xFF).

Exclusions (Not Flagged)

• Widening / index casts: as usize, as u64, as i64, as u128

are the common safe-widening targets and are not flagged.

• Pointer casts: as *const T / as *mut T are not numeric

conversions and are skipped.

• Inferred target: as _ defers the type to the compiler and is

left alone.

• Comments: a cast shown in a // comment is not code.

The known false-positive class is a genuinely lossless as u8/as i8 on a value already known to fit; the reviewer confirms intent and the recommendation is still the safer try_from.

Related Clippy Lints

Output Section

## Numeric Cast Safety
### Issues Found
- [file:line] Length truncation: `.len() as u32` truncates a usize;
  use `u32::try_from(...)` (clippy::cast_possible_truncation)
- [file:line] Narrowing to byte: `as u8` cannot widen; use
  `u8::try_from(...)` (clippy::cast_possible_truncation)
- [file:line] Precision loss: `as f32` can lose precision; prefer
  `f32::from(...)` where lossless (clippy::cast_precision_loss)

Exit Criteria

• [ ] .len()/.count()/.capacity() cast to a narrower integer is

flagged as a length truncation

• [ ] as u8 / as i8 casts are flagged as narrowing-to-byte
• [ ] as f32 casts are flagged as precision loss
• [ ] as usize, pointer casts (as *const/as *mut), and as _ are

not flagged

• [ ] Each finding names the TryFrom/From alternative and the

clippy lint

Ownership Analysis

Deep analysis of Rust's ownership, borrowing, and lifetime patterns.

Borrowing Patterns

Inspect diffs for:

• Unnecessary clones
• Temporary allocations
• Misuse of Rc<RefCell<_>>
• Excessive Arc wrapping

Lifetime Annotations

Check:

• Lifetime annotations correctness
• Reference scopes
• Ownership transfer semantics
• Lifetime elision applicability

Clone Avoidance Patterns

Prefer borrowing over cloning:

// Prefer borrowing
fn process(data: &[u8]) -> Result<()>

// Use Cow for flexibility
fn normalize(s: Cow<str>) -> Cow<str>

// Iterators over collections
fn sum(items: impl Iterator<Item = i32>) -> i32

Reference Scope Checking

Verify:

• Borrow checker satisfaction
• No dangling references
• Proper lifetime bounds
• Interior mutability patterns

Common Issues

• Unnecessary clones: Use references when possible
• Lifetime complexity: Simplify with helper methods
• Rc/Arc overuse: validate shared ownership is needed
• Temporary allocations: Use stack when possible

Output Section

## Ownership Analysis
### Borrowing Issues
- [file:line] Unnecessary clone: [explanation]
- [file:line] Lifetime annotation: [suggestion]

### Recommendations
- [specific improvements]

Repr Packed Audit

#[repr(packed)] removes the padding that keeps struct fields naturally aligned. Borrowing a field of a packed struct then produces an unaligned reference, which is undefined behavior. This dimension flags every packed representation so the reviewer can confirm field access copies out rather than borrows in place.

What This Detects

The analyzer (analyze_repr_packed) flags any repr attribute whose parentheses contain the packed token:

• #[repr(packed)]
• #[repr(C, packed)]
• #[repr(packed(2))]

A #[repr(C)] or #[repr(transparent)] attribute keeps natural alignment and is left alone. A repr shown in a comment is not an attribute and is skipped.

Why Packed Fields Are Dangerous

The Rust Reference type-layout chapter (src/type-layout.md) defines the packed representation: it lowers the struct alignment to the given value (default 1) and drops inter-field padding. A field can then sit at an address that does not satisfy its own alignment. Creating a reference to such a field (&s.field, or implicitly when calling a &self method on it, or via println!("{}", s.field)) is instant undefined behavior, and the compiler enforces this through the unaligned_references lint, which is a hard error rather than a warning.

The packed layout itself is legitimate (wire formats, hardware registers, FFI structs). The hazard is in how fields are read after the fact.

The Fix

#[repr(C, packed)]
struct Header {
    tag: u8,
    len: u32,   // not 4-byte aligned inside the packed struct
}

// Flag: borrowing a packed field is UB
let n = &header.len;            // unaligned reference

// Checked: copy the field into a local first (the field is Copy)
let len = header.len;           // reads by value, no reference taken
let n = &len;

// When you must take the address, use the raw-pointer macros, which do
// not create a reference, then read unaligned.
let p = std::ptr::addr_of!(header.len);
let len = unsafe { p.read_unaligned() };

Copy Copy fields out by value before borrowing, use ptr::addr_of! / read_unaligned when you need the address, and keep #[repr(packed)] only where an external format dictates the layout. If the only goal is a stable field order, #[repr(C)] without packed keeps alignment intact.

Exclusions (Not Flagged)

• `#[repr(C)]`: defines field order but keeps natural alignment.
• `#[repr(transparent)]`: a single-field wrapper with the field's own

layout.

• Comments: a repr shown in a // comment is not an attribute.

Related Lints

Output Section

## Repr Packed Audit
### Issues Found
- [file:line] `#[repr(packed)]` under-aligns fields; borrowing one
  (`&s.field`) is undefined behavior. Copy the field into a local first
  or use `ptr::addr_of!` + `read_unaligned` (unaligned_references)

Exit Criteria

• [ ] #[repr(packed)], #[repr(C, packed)], and #[repr(packed(2))]

are flagged

• [ ] #[repr(C)] and #[repr(transparent)] are not flagged
• [ ] A repr inside a // comment is not flagged
• [ ] Each finding explains the unaligned-reference hazard and names the

copy-out / addr_of! alternative

Silent Returns

Analysis of control-flow branches that discard Result or Option values without propagating or logging the reason for failure.

What This Detects

Patterns where a branch exits the function or loop without surfacing why a value was absent or erroneous:

• let x = expr else { return; }: let-else with bare return
• let x = expr else { continue; }: let-else with bare continue
• Match arms using => return or => continue on error/None variants

Why It Matters

Silent discards hide failure information from callers and make bugs hard to diagnose in production. The ? operator, return Err(...), or at minimum a log::warn! call should replace bare early exits.

Safe Patterns

// Good: propagate the error
let value = expr?;

// Good: log before discarding
let Some(value) = optional else {
    log::warn!("expected value not present, skipping");
    continue;
};

// Good: return meaningful error
let Some(value) = optional else {
    return Err(MyError::MissingValue);
};

Patterns to Flag

// Bad: silent discard
let Some(value) = optional else { return; };

// Bad: match arm drops error silently
match result {
    Ok(v) => v,
    Err(_) => return,
}

Output Section

## Silent Returns
### Issues Found
- [file:line] Silent discard: [explanation]

### Recommendations
- Replace bare `return` / `continue` with `return Err(...)` or log the cause

SQL Injection

Detection of format! calls that build SQL strings with {} interpolation, bypassing parameterized query protection.

What This Detects

format! strings containing SQL keywords (SELECT, INSERT, UPDATE, DELETE, DROP, WHERE) combined with {} placeholders, which interpolate values at the Rust string level rather than through the database driver's parameter binding.

Why It Matters

String interpolation into SQL is the classic SQL injection vector. Database drivers (sqlx, diesel, rusqlite) all provide parameterized query APIs that eliminate the risk at zero cost.

Safe Patterns

// Good: sqlx parameterized query
sqlx::query("SELECT * FROM users WHERE id = $1")
    .bind(user_id)
    .fetch_one(&pool)
    .await?;

Patterns to Flag

// Bad: format! with SQL keyword and {} interpolation
let query = format!("SELECT * FROM users WHERE name = '{}'", name);

// Bad: value injected before keyword
let q = format!("DELETE FROM {} WHERE id = 1", table_name);

Output Section

## SQL Injection
### Issues Found
- [file:line] format! SQL interpolation: [explanation]

### Recommendations
- Replace format! SQL strings with parameterized queries
- Use sqlx::query!(...).bind(...) or equivalent

Test Slop

AI-generated tests exist. They disproportionately mirror the implementation rather than validate behavior.

The CodeRabbit / Larridin (2026) data is unambiguous: AI-generated test suites pattern-match correctly but catch nothing. Mutation testing exposes this faster than code review can. This module covers the specific patterns to delete or replace.

Pattern 1: it_works-class names

// SLOP — the cargo-new default, almost always still there
#[test]
fn it_works() {
    assert_eq!(2 + 2, 4);
}

Rule: either delete it, or rename to assert one specific behavior. it_works tells the next reader nothing about what is being verified or what changes would break it.

Detection:

rg -n '#\[test\]\s*\nfn (it_works|test_default|test_something)' --type rust

Pattern 2: tautological tests

// SLOP
#[test]
fn test_creation() {
    let foo = Foo::new();
    assert!(foo.is_some());  // Foo::new() returns Foo, not Option<Foo>
}

// SLOP
#[test]
fn test_construction() {
    let s = Server::new();
    assert!(true);  // literal noise
}

These tests assert nothing the type system does not already prove. Delete on sight.

Detection:

# Tests that contain only assert!(true) or trivial unwrap
rg -A 5 "#\[test\]" --type rust | rg -B 5 "assert!\s*\(\s*true\s*\)"

Pattern 3: re-implementing the function under test

// SLOP — test computes expected the same way the impl does
#[test]
fn test_double() {
    let input = vec![1, 2, 3];
    let expected: Vec<i32> = input.iter().map(|x| x * 2).collect();
    let actual = double_all(&input);
    assert_eq!(actual, expected);
}

If the test computes the expected value with the same logic as the implementation, the test only verifies that the implementation calls itself. The right pattern is either:

• Hand-coded fixtures: assert_eq!(double_all(&[1,2,3]), vec![2,4,6]).
• Property tests: assert invariants (e.g. `for all x:

double_all(x).len() == x.len() && all elements doubled`).

• Golden-file tests: serialize input, write output, diff

against checked-in expected.

Detection: this one is judgment-bound. Heuristic:

# Tests where the expected value is computed via iter/map/filter
rg -A 10 "#\[test\]" --type rust | rg "let expected.*=.*\.iter\(\)"

Surface for human review with confidence: medium.

Pattern 4: mock-everything tests

// SLOP
#[tokio::test]
async fn test_handler() {
    let mock_db = MockDb::new();
    let mock_cache = MockCache::new();
    let mock_logger = MockLogger::new();

    let handler = Handler::new(mock_db.clone(), mock_cache.clone(), mock_logger.clone());
    handler.process().await;

    assert!(mock_db.was_called());
    assert!(mock_cache.was_called());
    assert!(mock_logger.was_called());
}

A test that mocks every collaborator and asserts that the mock was called proves only that the orchestration calls the orchestrator. The behavior under test (what process() actually computes and persists) is not verified.

Replace with:

• Integration test: real DB (sqlite or test container),

real cache (in-memory), real logger (capturing).

• Behavior test: assert against the externally

observable result, not against the call sequence.

Detection: count Mock* usages per test file:

rg "Mock[A-Z]\w+::" --type rust -c | sort -t: -k2 -nr | head -10
# Files with >5 Mock types per test are suspect

Pattern 5: snapshot tests on non-deterministic data

// SLOP
#[test]
fn test_serialization_snapshot() {
    let user = User::new();  // generates UUID
    let serialized = serde_json::to_string(&user).unwrap();
    insta::assert_snapshot!(serialized);  // UUID changes every run
}

Snapshots on UUIDs, timestamps, hash maps with non-deterministic iteration, or any randomness produce flaky tests that get suppressed (#[ignore]) instead of fixed.

Fix: use BTreeMap instead of HashMap for stable iteration; inject a deterministic clock; use a fixed UUID for testing; or assert on shape rather than literal value.

Detection:

# Look for snapshot assertions near time/uuid usage
rg -B 3 "assert_snapshot" --type rust | rg -B 3 "(Uuid|Instant|Now|HashMap)"

Pattern 6: one giant test_everything()

// SLOP
#[test]
fn test_full_flow() {
    // 200 lines asserting 30 unrelated things
}

A failing assertion stops the test, so subsequent assertions never run; you only learn one failure per suite execution. Split into focused tests.

Detection:

# Long test bodies (heuristic: tests with >50 lines of body)
awk '/^#\[test\]/ {start=NR; lines=0}
     /^fn / && start>0 {fn_start=NR}
     fn_start>0 {lines++}
     /^}$/ && fn_start>0 {if (lines > 50) print FILENAME":"start" length="lines; start=0; fn_start=0}' \
  $(find . -name "*.rs" -path "*/tests/*")

Pattern 7: hidden side-effects

// SLOP
#[test]
fn test_persists() {
    let path = PathBuf::from("/tmp/test_output.json");  // hard-coded path
    write_to(&path, data);
    // ... no cleanup, races with parallel test runs
}

// SLOP
#[test]
fn test_api_call() {
    let resp = reqwest::blocking::get("https://api.example.com/data").unwrap();
    // real network call; flaky, slow, leaks
}

Use tempfile::TempDir for filesystem isolation, wiremock or httpmock for HTTP isolation. Tests that touch shared mutable state (filesystem, network, database, env vars) without isolation cannot be parallel and cannot be reliable.

Detection:

# Hard-coded /tmp paths
rg "/tmp/" --type rust tests/

# Real network calls in tests
rg "(reqwest|hyper)::.*\.get\(\"http" --type rust tests/

Pattern 8: #[ignore] without a comment

// SLOP
#[test]
#[ignore]
fn test_flaky_thing() { ... }

A test marked ignored without explanation is technical debt with no removal date. Either:

1. Fix the underlying flakiness. 2. Add a comment naming the bug or environment requirement (#[ignore = "requires DB; #1234"]). 3. Delete the test.

Detection:

# Find #[ignore] without trailing reason
rg "#\[ignore\]\s*$" --type rust

Positive patterns to use instead

When pure functions are under-covered, prefer:

• Property-based tests (proptest, quickcheck):

assert invariants over random inputs.

• Golden-file tests: serialize input, run the process,

diff against the checked-in expected output.

• Mutation testing (cargo mutants): proves the

remaining tests catch real behavior changes.

Mutation testing as the cheapest signal

The fastest way to expose AI-generated test slop is to run mutation testing:

cargo install cargo-mutants
cargo mutants --workspace

Mutants that survive (the test suite still passes after the mutation) indicate uncovered behavior. AI-generated test suites typically have 60%+ surviving mutants because they assert what the implementation does rather than what the contract requires. A healthy human-written test suite typically catches 70-90% of mutants.

For CI, run cargo mutants on a sampled subset (the most recently changed files) to keep runtime under 10 minutes.

Output format

Per the Skill(scribe:slop-detector) module structured-finding-output.md format. Severity:

• High: any test in patterns 4 (mock-everything),

5 (non-deterministic snapshot), 7 (hidden side-effects).

• Medium: any test in patterns 1 (it_works),

2 (tautology), 6 (giant test).

• Low: any test in pattern 3 (re-implementation;

judgment-bound).

Always surface pattern-3 findings as confidence: medium since the line between "tests the contract" and "re-implements the function" is genuinely subjective.

Integration

Test slop landed in Pass 8 of the multi-pass cleanup workflow (see Skill(scribe:slop-detector) module cleanup-workflow.md). Run after architecture pass (Pass 7) since refactoring can invalidate tests.

Transmute Audit

mem::transmute reinterprets the bytes of one type as another with no layout check. When the source and target layouts disagree it produces an invalid value, which is undefined behavior. This dimension flags every transmute and transmute_copy so the reviewer can confirm a typed, checked conversion is not the better tool.

What This Detects

The analyzer (analyze_transmute_safety) flags two shapes:

1. `transmute`: a transmute(...) or transmute::<A, B>(...) call, whether written mem::transmute, std::mem::transmute, or an imported bare transmute. The call reinterprets bytes with only a compile-time size-equality check. 2. `transmute_copy`: a transmute_copy(&src) call. It does not even require the sizes to match and reads through a reference, so it can read past the end of the source.

A method call (pipeline.transmuter(x)) and any longer identifier are left alone: the regex requires a call or turbofish directly after the name and a non-. character before it.

Why transmute Hides Bugs

The Rust Reference lists "producing an invalid value" under behavior considered undefined (src/behavior-considered-undefined.md), and a transmute across mismatched layouts does exactly that. The std::mem::transmute documentation calls it "incredibly unsafe" and spells out the traps: differing alignment, niche-optimized layouts (Option<&T>), uninitialized padding, and lifetimes invented from nothing. None of this is checked beyond size equality, so a transmute reads as deliberate even when the layouts have silently drifted apart.

The Fix

// Flag: reinterpret integer bits as a float
let f: f32 = unsafe { mem::transmute(bits) };
// Checked: a named, total operation with no unsafe
let f = f32::from_bits(bits);

// Flag: bytes to a struct
let header: Header = unsafe { mem::transmute(buf) };
// Checked: validated, alignment-safe plain-old-data conversion
let header: Header = bytemuck::pod_read_unaligned(&buf);

// Numbers <-> bytes use the explicit endian methods
let n = u32::from_le_bytes(buf);   // not transmute(buf)
let buf = n.to_le_bytes();         // not transmute(n)

Reach for from_bits/to_bits and from_le_bytes/to_le_bytes for numbers, bytemuck/zerocopy for plain-old-data structs, as or .cast() for pointers, and From/TryFrom for ordinary conversions. Keep transmute only where no safe operation exists, and pair it with a // SAFETY: comment proving the layouts match.

Exclusions (Not Flagged)

• Method calls: value.transmute(...) is a user method, not the std

function.

• Longer identifiers: transmuter(...), transmute_foo(...) are not

the transmute call (the name is not followed by a call or turbofish).

• Comments: a transmute shown in a // comment is not code.

Related Clippy Lints

Output Section

## Transmute Audit
### Issues Found
- [file:line] `mem::transmute` reinterprets bytes with no layout check;
  prefer `f32::from_bits` / `from_le_bytes` / `bytemuck`
  (clippy::transmute_int_to_float)
- [file:line] `transmute_copy` skips the size check and can over-read;
  use a typed conversion or audited `ptr::read` (clippy::transmute_copy)

Exit Criteria

• [ ] mem::transmute(...) and transmute::<A, B>(...) calls are flagged

as transmute

• [ ] transmute_copy(...) is flagged separately as the more dangerous

sibling

• [ ] Method calls (x.transmute(...)) and longer identifiers

(transmuter(...)) are not flagged

• [ ] A transmute inside a // comment is not flagged
• [ ] Each finding names a typed alternative (from_bits,

from_le_bytes, bytemuck, TryFrom) and the clippy lint

Unsafe & FFI Audit

detailed audit of unsafe code and FFI boundaries.

Unsafe Block Invariants

For each unsafe block, document:

• Pointer validity requirements
• Aliasing rules adherence
• Memory ordering guarantees
• Uninitialized memory handling
• FFI contracts

SAFETY Comments

Every unsafe block must have:

• Clear safety comment
• Invariant documentation
• Caller requirements
• Pre/post conditions

FFI Boundaries

Audit extern "C" interfaces:

• Representation alignment (#[repr(C)])
• Ownership transfer semantics
• Resource cleanup guarantees
• Error code translation
• Null pointer handling

Safe Abstraction Wrappers

Recommend wrapping unsafe in safe APIs:

// Wrap unsafe in safe API
pub fn safe_operation(ptr: NonNull<Data>) -> Result<(), Error> {
    // SAFETY: ptr is non-null and properly aligned
    // Caller guarantees exclusive access
    unsafe {
        (*ptr.as_ptr()).process()
    }
}

Memory Pinning (mlock)

When reviewing real-time or latency-sensitive code that calls libc::mlock or libc::munlock, audit all four production gotchas before approving:

RLIMIT_MEMLOCK: Linux default is 64KB. Any mlock on a buffer larger than the process rlimit silently returns ENOMEM. Flag code that does not check the rlimit or document the deployment's securityContext/ulimit setting.

Page alignment: mlock operates at page granularity (4KB on x86). Locking a byte slice that is not page-aligned pins adjacent memory belonging to other allocations. Require posix_memalign or std::alloc::Layout::from_size_align with page_size() alignment on the underlying buffer.

ENOMEM fallback: mlock can fail with ENOMEM at runtime (e.g., when a co-located process allocates unexpectedly). Treat as a recoverable error: fall back to un-pinned buffers and emit a metric. Code that unwrap()s or ignores the return value is a production bug.

Lifetime coupling: the buffer must outlive the lock. munlock must be called before the slice is freed. Require a SAFETY comment on every pin_buffer/unpin_buffer call site that documents the lifetime guarantee.

// SAFETY: `buf` is page-aligned (allocated via posix_memalign),
// lives for the duration of the audio pipeline, and RLIMIT_MEMLOCK
// has been raised to 16MB in the container securityContext.
// munlock is called in AudioBuffer::drop.
unsafe { pin_buffer(&ring_buf)? };

Cross-platform note: macOS uses mlock (same interface), Windows uses VirtualLock. If the crate must be cross-platform, audit for the platform guard or check whether a wrapper crate is used instead of raw libc calls.

Production context: the canonical symptom of a missing mlock is inconsistent latency spikes in production that disappear on dev machines. The kernel pages out long-lived audio or ring-ring buffers when a co-resident large allocation (e.g., ML model weights) forces a page eviction. The page-fault on next buffer access adds 100–380ms of p99 latency that tokio-console will not surface, because the task scheduling itself is fast; the delay is in the kernel.

Common Unsafe Patterns

Check for:

• Raw pointer dereferences
• Mutable static access
• Type transmutations
• Inline assembly
• Trait object manipulation

Undefined Behavior Checks

Verify absence of:

• Use after free
• Double free
• Null pointer dereferences
• Data races in unsafe code
• Invalid enum discriminants

Output Section

## Unsafe Audit
### [U1] file:line
- Invariants: [documented]
- Risk: [high/medium/low]
- SAFETY comment: [present/missing]
- Recommendation: [action]

### Summary
- Total unsafe blocks: X
- Properly documented: Y
- Action required: Z