{"id":"7a4607b7-64eb-4f1a-81dd-2ceb158b89e8","entityType":"agent","slug":"clawhub-athola-nm-pensive-rust-review","name":"rust-review","canonicalUrl":"https://www.xpersona.co/agent/clawhub-athola-nm-pensive-rust-review","canonicalPath":"/agent/clawhub-athola-nm-pensive-rust-review","generatedAt":"2026-10-10T11:54:01.011Z","source":"CLAWHUB","claimStatus":"UNCLAIMED","verificationTier":"NONE","summary":{"evidence":{"source":"editorial-content","verified":true,"confidence":"high","updatedAt":"2026-10-10T09:05:39.139Z","emptyReason":null},"description":"Audits Rust code for unsafe blocks, ownership issues, and Cargo dependency risks Skill: rust-review Owner: athola Summary: Audits Rust code for unsafe blocks, ownership issues, and Cargo dependency risks Tags: latest:1.9.19 Version history: v1.9.19 | 2026-08-26T13:19:16.072Z | user Release v1.9.19 v1.9.17 | 2026-07-30T05:39:27.389Z | user Release v1.9.17 v1.9.16 | 2026-07-14T19:56:10.799Z | user Release v1.9.16 v1.9.14 | 2026-06-30T18:04:28.776Z | user Release v1.9.14 v1.9.13 | 2026-06-27T16:22:2","descriptionLabel":"Technical summary","evidenceSummary":"Capability contract not published. No trust telemetry is available yet. 1.5K downloads reported by the source. Last updated 10/10/2026.","installCommand":"clawhub skill install s17emme0e2m3cpf7k2jvp3a84984b8z9:nm-pensive-rust-review","sourceUrl":"https://clawhub.ai/athola/nm-pensive-rust-review","homepage":"https://clawhub.ai/athola/skills/nm-pensive-rust-review","primaryLinks":[{"label":"View on ClawHub","url":"https://clawhub.ai/athola/nm-pensive-rust-review","kind":"source"},{"label":"Homepage","url":"https://clawhub.ai/athola/skills/nm-pensive-rust-review","kind":"homepage"}],"safetyScore":84,"overallRank":62,"popularityScore":64,"trustScore":null,"claimedByName":null,"isOwner":false,"seoDescription":"Audits Rust code for unsafe blocks, ownership issues, and Cargo dependency risks Skill: rust-review Owner: athola Summary: Audits Rust code for unsafe blocks, o"},"coverage":{"evidence":{"source":"public-profile","verified":false,"confidence":"medium","updatedAt":"2026-10-10T09:05:39.139Z","emptyReason":null},"protocols":[{"protocol":"OPENCLEW","label":"OpenClaw","status":"self-declared","notes":"Declared in the public agent profile."}],"capabilities":[],"verifiedCount":0,"selfDeclaredCount":1,"capabilityMatrix":{"rows":[{"key":"OPENCLEW","type":"protocol","support":"unknown","confidenceSource":"profile","notes":"Listed on profile"}],"flattenedTokens":"protocol:OPENCLEW|unknown|profile"}},"adoption":{"evidence":{"source":"CLAWHUB","verified":false,"confidence":"medium","updatedAt":"2026-10-10T09:05:39.139Z","emptyReason":null},"stars":null,"forks":null,"downloads":1538,"packageName":null,"latestVersion":"1.9.19","tractionLabel":"1.5K downloads"},"release":{"evidence":{"source":"CLAWHUB","verified":false,"confidence":"medium","updatedAt":"2026-10-10T09:05:39.139Z","emptyReason":null},"lastUpdatedAt":"2026-10-10T09:05:39.139Z","lastCrawledAt":"2026-10-10T09:05:39.139Z","lastIndexedAt":null,"nextCrawlAt":"2026-10-11T09:05:39.139Z","lastVerifiedAt":null,"highlights":[{"version":"1.9.19","createdAt":"2026-08-26T13:19:16.072Z","changelog":"Release v1.9.19","fileCount":19,"zipByteSize":33558},{"version":"1.9.17","createdAt":"2026-07-30T05:39:27.389Z","changelog":"Release v1.9.17","fileCount":19,"zipByteSize":33444},{"version":"1.9.16","createdAt":"2026-07-14T19:56:10.799Z","changelog":"Release v1.9.16","fileCount":19,"zipByteSize":33462},{"version":"1.9.14","createdAt":"2026-06-30T18:04:28.776Z","changelog":"Release v1.9.14","fileCount":19,"zipByteSize":33568},{"version":"1.9.13","createdAt":"2026-06-27T16:22:26.968Z","changelog":"Release v1.9.13","fileCount":19,"zipByteSize":33402},{"version":"1.9.12","createdAt":"2026-06-19T03:17:39.464Z","changelog":"Release v1.9.12","fileCount":19,"zipByteSize":33502},{"version":"1.0.3","createdAt":"2026-06-18T15:16:07.769Z","changelog":"Release v1.9.12","fileCount":19,"zipByteSize":33634},{"version":"1.0.2","createdAt":"2026-05-09T02:19:25.243Z","changelog":"Release v1.9.5","fileCount":15,"zipByteSize":16943}]},"execution":{"evidence":{"source":"CLAWHUB","verified":false,"confidence":"low","updatedAt":null,"emptyReason":"No published capability contract is available yet."},"installCommand":"clawhub skill install s17emme0e2m3cpf7k2jvp3a84984b8z9:nm-pensive-rust-review","setupComplexity":"low","setupSteps":["Setup complexity is classified as HIGH. You must provision dedicated cloud infrastructure or an isolated VM. Do not run this directly on your local workstation.","Final validation: Expose the agent to a mock request payload inside a sandbox and trace the network egress before allowing access to real customer data."],"contract":{"contractStatus":"missing","authModes":[],"requires":[],"forbidden":[],"supportsMcp":false,"supportsA2a":false,"supportsStreaming":false,"inputSchemaRef":null,"outputSchemaRef":null,"dataRegion":null,"contractUpdatedAt":null,"sourceUpdatedAt":null,"freshnessSeconds":null},"invocationGuide":{"preferredApi":{"snapshotUrl":"https://www.xpersona.co/api/v1/agents/clawhub-athola-nm-pensive-rust-review/snapshot","contractUrl":"https://www.xpersona.co/api/v1/agents/clawhub-athola-nm-pensive-rust-review/contract","trustUrl":"https://www.xpersona.co/api/v1/agents/clawhub-athola-nm-pensive-rust-review/trust"},"curlExamples":["curl -s \"https://www.xpersona.co/api/v1/agents/clawhub-athola-nm-pensive-rust-review/snapshot\"","curl -s \"https://www.xpersona.co/api/v1/agents/clawhub-athola-nm-pensive-rust-review/contract\"","curl -s \"https://www.xpersona.co/api/v1/agents/clawhub-athola-nm-pensive-rust-review/trust\""],"jsonRequestTemplate":{"query":"summarize this repo","constraints":{"maxLatencyMs":2000,"protocolPreference":["OPENCLEW"]}},"jsonResponseTemplate":{"ok":true,"result":{"summary":"...","confidence":0.9},"meta":{"source":"CLAWHUB","generatedAt":"2026-10-10T11:54:01.009Z"}},"retryPolicy":{"maxAttempts":3,"backoffMs":[500,1500,3500],"retryableConditions":["HTTP_429","HTTP_503","NETWORK_TIMEOUT"]}},"endpoints":{"dossierUrl":"https://www.xpersona.co/api/v1/agents/clawhub-athola-nm-pensive-rust-review/dossier","snapshotUrl":"https://www.xpersona.co/api/v1/agents/clawhub-athola-nm-pensive-rust-review/snapshot","contractUrl":"https://www.xpersona.co/api/v1/agents/clawhub-athola-nm-pensive-rust-review/contract","trustUrl":"https://www.xpersona.co/api/v1/agents/clawhub-athola-nm-pensive-rust-review/trust"}},"reliability":{"evidence":{"source":"runtime-metrics","verified":false,"confidence":"low","updatedAt":null,"emptyReason":"No trust, reliability, or runtime telemetry is available."},"trust":{"status":"unavailable","handshakeStatus":"UNKNOWN","verificationFreshnessHours":null,"reputationScore":null,"p95LatencyMs":null,"successRate30d":null,"fallbackRate":null,"attempts30d":null,"trustUpdatedAt":null,"trustConfidence":"unknown","sourceUpdatedAt":null,"freshnessSeconds":null},"decisionGuardrails":{"doNotUseIf":["Contract metadata is missing or unavailable for deterministic execution."],"safeUseWhen":[],"riskFlags":["missing_or_unavailable_contract","trust_data_unavailable","schema_references_missing"],"operationalConfidence":"low"},"executionMetrics":{"observedLatencyMsP50":null,"observedLatencyMsP95":null,"estimatedCostUsd":null,"uptime30d":null,"rateLimitRpm":null,"rateLimitBurst":null,"lastVerifiedAt":null,"verificationSource":null},"runtimeMetrics":{"successRate":null,"avgLatencyMs":null,"avgCostUsd":null,"hallucinationRate":null,"retryRate":null,"disputeRate":null,"p50Latency":null,"p95Latency":null,"lastUpdated":null}},"benchmarks":{"evidence":{"source":"no-benchmark-data","verified":false,"confidence":"low","updatedAt":null,"emptyReason":"No benchmark suites or observed failure patterns are available."},"suites":[],"failurePatterns":[]},"artifacts":{"evidence":{"source":"CLAWHUB","verified":false,"confidence":"high","updatedAt":"2026-10-10T09:05:39.139Z","emptyReason":null},"readme":"Skill: rust-review\n\nOwner: athola\n\nSummary: Audits Rust code for unsafe blocks, ownership issues, and Cargo dependency risks\n\nTags: latest:1.9.19\n\nVersion history:\n\nv1.9.19 | 2026-08-26T13:19:16.072Z | user\n\nRelease v1.9.19\n\nv1.9.17 | 2026-07-30T05:39:27.389Z | user\n\nRelease v1.9.17\n\nv1.9.16 | 2026-07-14T19:56:10.799Z | user\n\nRelease v1.9.16\n\nv1.9.14 | 2026-06-30T18:04:28.776Z | user\n\nRelease v1.9.14\n\nv1.9.13 | 2026-06-27T16:22:26.968Z | user\n\nRelease v1.9.13\n\nv1.9.12 | 2026-06-19T03:17:39.464Z | user\n\nRelease v1.9.12\n\nv1.0.3 | 2026-06-18T15:16:07.769Z | user\n\nRelease v1.9.12\n\nv1.0.2 | 2026-05-09T02:19:25.243Z | user\n\nRelease v1.9.5\n\nv1.0.1 | 2026-05-06T14:20:40.175Z | user\n\nRelease v1.9.4\n\nv1.0.0 | 2026-04-15T15:01:42.076Z | auto\n\n- Initial release of rust-review skill for Rust code audits.\n- Provides expert-level review focusing on unsafe blocks, ownership patterns, and dependency security.\n- Includes detailed workflow and checklist for safety, correctness, performance, and idiomatic Rust patterns.\n- Supports modular, progressive review (ownership, error handling, concurrency, unsafe, dependencies).\n- Output format specified for clear audit reporting and recommendations.\n- Special requirements: relies on \"night-market.pensive:shared\" and \"night-market.imbue:proof-of-work\" configuration.\n\nArchive index:\n\nArchive v1.9.19: 19 files, 33558 bytes\n\nFiles: modules/async-slop.md (7937b), modules/builtin-preference.md (3751b), modules/cargo-dependencies.md (1516b), modules/cfg-test-misuse.md (1438b), modules/collection-types.md (1539b), modules/concurrency-patterns.md (5998b), modules/duplicate-validators.md (1559b), modules/error-handling.md (1405b), modules/error-messages.md (1459b), modules/iterator-and-allocation-slop.md (9416b), modules/model-specific-tells.md (7032b), modules/ownership-analysis.md (1469b), modules/silent-returns.md (1678b), modules/sql-injection.md (1520b), modules/test-slop.md (8018b), modules/unsafe-audit.md (3876b), skill-card.md (2178b), SKILL.md (4549b), _meta.json (142b)\n\nFile v1.9.19:SKILL.md\n\n---\nname: rust-review\ndescription: Audits Rust code for unsafe blocks, ownership issues, and Cargo dependency risks\nversion: 1.9.8\ntriggers:\n - rust\n - ownership\n - concurrency\n - unsafe\n - traits\n - cargo\n - reviewing Rust code or before merging Rust changes\nmetadata: {\"openclaw\": {\"homepage\": \"https://github.com/athola/claude-night-market/tree/master/plugins/pensive\", \"emoji\": \"\\ud83d\\udd0d\", \"requires\": {\"config\": [\"night-market.pensive:shared\", \"night-market.imbue:proof-of-work\"]}}}\nsource: claude-night-market\nsource_plugin: pensive\n---\n\n> **Night Market Skill** — ported from [claude-night-market/pensive](https://github.com/athola/claude-night-market/tree/master/plugins/pensive). For the full experience with agents, hooks, and commands, install the Claude Code plugin.\n\n\n## Table of Contents\n\n- [Quick Start](#quick-start)\n- [When to Use](#when-to-use)\n- [Required TodoWrite Items](#required-todowrite-items)\n- [Progressive Loading](#progressive-loading)\n- [Core Workflow](#core-workflow)\n- [Rust Quality Checklist](#rust-quality-checklist)\n- [Safety](#safety)\n- [Correctness](#correctness)\n- [Performance](#performance)\n- [Idioms](#idioms)\n- [Output Format](#output-format)\n- [Summary](#summary)\n- [Ownership Analysis](#ownership-analysis)\n- [Error Handling](#error-handling)\n- [Concurrency](#concurrency)\n- [Unsafe Audit](#unsafe-audit)\n- [[U1] file:line](#[u1]-file:line)\n- [Dependencies](#dependencies)\n- [Recommendation](#recommendation)\n- [Exit Criteria](#exit-criteria)\n\n\n# Rust Review Workflow\n\nExpert-level Rust code audits with focus on safety, correctness, and idiomatic patterns.\n\n## Quick Start\n\n```bash\n/rust-review\n```\n**Verification:** Run the command with `--help` flag to verify availability.\n\n## When To Use\n\n- Reviewing Rust code changes\n- Auditing unsafe blocks\n- Analyzing concurrency patterns\n- Dependency security review\n- Performance optimization review\n\n## When NOT To Use\n\n- General code review without Rust - use unified-review\n- Performance profiling - use parseltongue:python-performance pattern\n\n## Required TodoWrite Items\n\n1. `rust-review:ownership-analysis`\n2. `rust-review:error-handling`\n3. `rust-review:concurrency`\n4. `rust-review:unsafe-audit`\n5. `rust-review:cargo-deps`\n6. `rust-review:evidence-log`\n\n## Progressive Loading\n\nLoad modules as needed based on review scope:\n\n**Quick Review** (ownership and errors):\n- See `modules/ownership-analysis.md` for borrowing and lifetime analysis\n- See `modules/error-handling.md` for Result/Option patterns\n\n**Concurrency Focus**:\n- See `modules/concurrency-patterns.md` for async and sync primitives\n\n**Safety Audit**:\n- See `modules/unsafe-audit.md` for unsafe block documentation\n\n**Dependency Review**:\n- See `modules/cargo-dependencies.md` for vulnerability scanning\n\n**Idiomatic Patterns**:\n- See `modules/builtin-preference.md` for conversion traits and builtin preference\n\n## Core Workflow\n\n1. **Ownership Analysis**: Check borrowing, lifetimes, clone patterns\n2. **Error Handling**: Verify Result/Option usage, propagation\n3. **Concurrency**: Review async patterns, sync primitives\n4. **Unsafe Audit**: Document invariants, FFI contracts\n5. **Dependencies**: Scan for vulnerabilities, updates\n6. **Evidence Log**: Record commands and findings\n\n## Rust Quality Checklist\n\n### Safety\n- [ ] All unsafe blocks documented with SAFETY comments\n- [ ] FFI boundaries properly wrapped\n- [ ] Memory safety invariants maintained\n- [ ] `mlock`/`munlock` calls: RLIMIT verified, page-aligned,\n ENOMEM handled\n\n### Correctness\n- [ ] Error handling complete\n- [ ] Concurrency patterns sound\n- [ ] Tests cover critical paths\n\n### Performance\n- [ ] No unnecessary allocations\n- [ ] Borrowing preferred over cloning\n- [ ] Async properly non-blocking\n\n### Idioms\n- [ ] Standard traits implemented\n- [ ] Conversion traits preferred over helper functions\n- [ ] Error types well-designed\n- [ ] Documentation complete\n\n## Output Format\n\n```markdown\n## Summary\nRust audit findings\n\n## Ownership Analysis\n[borrowing and lifetime issues]\n\n## Error Handling\n[error patterns and issues]\n\n## Concurrency\n[async and sync patterns]\n\n## Unsafe Audit\n### [U1] file:line\n- Invariants: [documented]\n- Risk: [assessment]\n- Recommendation: [action]\n\n## Dependencies\n[cargo audit results]\n\n## Recommendation\nApprove / Approve with actions / Block\n```\n**Verification:** Run the command with `--help` flag to verify availability.\n\n## Exit Criteria\n\n- All unsafe blocks audited\n- Concurrency patterns verified\n- Dependencies scanned\n- Evidence logged\n- Action items assigned\n\nFile v1.9.19:_meta.json\n\n{\n \"ownerId\": \"kn7d107jg9jv602h9ytsegydq184a42s\",\n \"slug\": \"nm-pensive-rust-review\",\n \"version\": \"1.9.19\",\n \"publishedAt\": 1787750356072\n}\n\nFile v1.9.19:modules/async-slop.md\n\n---\nmodule: async-slop\ncategory: detection\ndependencies: [Read, Grep]\nestimated_tokens: 500\n---\n\n# Async Slop\n\n**AI defaults to `async` and `tokio::spawn` even where\nsync code is faster, simpler, and correct.**\n\nThis module covers the high-frequency async patterns that\nlook idiomatic but are not. The clippy lints catch some;\nthe rest is structural.\n\n## Pattern 1: `async fn` that contains no `.await`\n\n```rust\n// SLOP\nasync fn compute_total(items: &[Item]) -> u64 {\n items.iter().map(|i| i.price).sum()\n}\n```\n\nIf the function body has no `.await`, it has no reason\nto be `async`. Async coloring is contagious: this\nfunction is callable only from async contexts, forcing\nevery caller to also be `async`. Strip `async` from the\nsignature unless the body actually awaits.\n\nDetection (preferred: clippy):\n\n```bash\ncargo clippy --all-targets -- -W clippy::async_yields_async\n```\n\nFile-level heuristic when clippy is unavailable:\n\n```bash\nfor f in $(rg -l \"async fn \" --type rust); do\n rg -q \"\\.await\" \"$f\" || echo \"no-await: $f\"\ndone\n```\n\n(Heuristic; manual review needed since `.await` may be in\na helper called by the async fn rather than inline.)\n\n## Pattern 2: blocking I/O inside an async runtime\n\n```rust\n// SLOP\nasync fn read_config() -> Result {\n Ok(std::fs::read_to_string(\"config.toml\")?)\n}\n\n// SLOP\nasync fn rate_limit_wait() {\n std::thread::sleep(Duration::from_secs(1)); // blocks the runtime\n}\n\n// SLOP\nasync fn query_db(conn: &Connection) -> Result> {\n conn.query(\"SELECT ...\")? // blocking driver\n}\n```\n\nBlocking calls inside `async` block the entire executor\nthread, defeating the runtime's concurrency model.\n\nFix:\n\n```rust\n// Use the async equivalent\nasync fn read_config() -> Result {\n Ok(tokio::fs::read_to_string(\"config.toml\").await?)\n}\n\n// Or wrap blocking work in spawn_blocking\nasync fn rate_limit_wait() {\n tokio::time::sleep(Duration::from_secs(1)).await;\n}\n\n// For unavoidable blocking work\nasync fn query_db(conn: Arc) -> Result> {\n let conn = conn.clone();\n tokio::task::spawn_blocking(move || conn.query(\"SELECT ...\"))\n .await?\n}\n```\n\nDetection:\n\n```bash\n# Find blocking ops inside async functions (heuristic)\nrg -B 5 \"(std::fs::|std::thread::sleep|std::net::TcpStream)\" --type rust |\n rg -B 5 \"async fn\"\n```\n\n## Pattern 3: `tokio::spawn` for synchronous-equivalent work\n\n```rust\n// SLOP\nasync fn handle_request(req: Request) -> Response {\n let result = tokio::spawn(async move {\n compute_response(&req)\n }).await.unwrap();\n result\n}\n```\n\nSpawning a task only to immediately await its single\ncompletion is equivalent to a direct call, plus the\noverhead of task creation, scheduling, and a join. Just\ncall the function:\n\n```rust\nasync fn handle_request(req: Request) -> Response {\n compute_response(&req)\n}\n```\n\n`tokio::spawn` is for *concurrent* work: when the\nspawned task should make progress while the caller does\nsomething else, or when the task should outlive the\ncaller. A spawn-then-immediately-await is a smell.\n\n## Pattern 4: `async-trait` on synchronous-equivalent traits\n\n```rust\n// SLOP\n#[async_trait]\ntrait Greeter {\n async fn greet(&self, name: &str) -> String;\n}\n```\n\nIf the implementation has no `.await` and just returns a\nsynchronous value, `async-trait` adds heap allocation\n(`Box`) for nothing. Make the trait sync:\n\n```rust\ntrait Greeter {\n fn greet(&self, name: &str) -> String;\n}\n```\n\nUse `async-trait` only when at least one implementation\ngenuinely awaits.\n\n## Pattern 5: explicit `Pin>` returns\n\n```rust\n// SLOP\nfn fetch_data(url: &str) -> Pin> + Send>> {\n Box::pin(async move {\n // body\n })\n}\n```\n\nModern Rust supports `impl Future` in return position:\n\n```rust\n// Idiomatic\nfn fetch_data(url: &str) -> impl Future> + Send {\n async move {\n // body\n }\n}\n```\n\n`Pin>` is needed only for trait method\nreturns or when storing futures in collections.\n\n## Pattern 6: `MutexGuard` held across `.await`\n\n```rust\n// SLOP — deadlock risk\nasync fn update_count(state: &Arc>) {\n let mut guard = state.lock().unwrap();\n guard.count += 1;\n save_to_disk(&guard).await; // holds guard across await\n}\n```\n\nHolding a sync `Mutex` guard across `.await` blocks the\nruntime if any other task tries to acquire the same lock.\nFor async paths, use:\n\n- `tokio::sync::Mutex` (async-aware, can hold guards\n across `.await`).\n- Or restructure to drop the guard before awaiting:\n\n```rust\nasync fn update_count(state: &Arc>) {\n let snapshot = {\n let mut guard = state.lock().unwrap();\n guard.count += 1;\n guard.clone()\n }; // guard dropped here\n save_to_disk(&snapshot).await;\n}\n```\n\nThis is the GPT-5.x signature failure (Sonar measured\n~470 concurrency issues per MLOC for GPT-5.2 High); see\n`model-specific-tells.md`.\n\n## Pattern 7: re-implementing `select!` / `join!` manually\n\nIf you find yourself manually polling multiple futures\nwith `Pin::new` and `Poll`, you almost certainly want\n`tokio::select!` or `tokio::join!`. Hand-rolled polling\nis a strong signal that the model copied something it\nshould not have.\n\nDetection:\n\n```bash\nrg \"Pin::new\" --type rust -B 2 -A 5 | rg -B 2 \"fn poll\"\n```\n\n## Pattern 8: `Send + Sync` bounds added \"in case\"\n\n```rust\n// SLOP\nfn add(a: T, b: T) -> T { ... }\n```\n\nTrait bounds should be added because the function\n*needs* them, not as defensive over-spec. `Send`/`Sync`\non a function that runs synchronously, `Clone` on a\nfunction that doesn't clone, `Debug` on a function that\ndoesn't print: all noise.\n\nThe right rule: add the bound when the compiler complains\nwithout it. Remove the bound when removing it does not\ncause a compile error.\n\n## Detection commands\n\n```bash\n# Catch most async slop with clippy\ncargo clippy --all-targets -- \\\n -W clippy::async_yields_async \\\n -W clippy::large_futures \\\n -D warnings\n\n# Manual scans for the structural patterns\n# Pattern 1: file-level \"async fn but no .await\" — see Pattern 1\n# section above for the loop form.\nrg \"tokio::spawn.*\\.await\" --type rust # Pattern 3\nrg \"#\\[async_trait\\]\" --type rust # Pattern 4\nrg \"Pin>), 8 (defensive Send+Sync).\n\nPattern 6 is the highest-blast-radius async finding;\nescalate to `severity: high` and to a senior reviewer.\n\n## Integration\n\nAsync slop lands in Pass 5 of the multi-pass cleanup\nworkflow (`Skill(scribe:slop-detector)` module\n`cleanup-workflow.md`). For GPT-family-generated\ncodebases, weight pattern 6 detection most heavily;\nfor Claude-family codebases, weight pattern 1 (the\n\"behavior-preserving refactor leaves async fn that no\nlonger awaits anything\") most heavily. See\n`model-specific-tells.md`.\n\nFile v1.9.19:modules/builtin-preference.md\n\n---\nname: builtin-preference\ndescription: Detection of helper functions that should be standard trait\n implementations and reimplemented Rust builtins\ncategory: rust-review\ntags: [from, into, tryfrom, fromstr, default, display, iterator, idioms]\n---\n\n# Builtin Preference\n\nDetects custom helper functions that duplicate Rust's standard\ntrait system and built-in combinators.\n\n## What This Detects\n\nFour categories of anti-patterns:\n\n1. **Conversion helpers**: `parse_foo()`, `foo_from_bar()`,\n `convert_*()`, `to_*(&self)` that should be `FromStr`,\n `From`, `TryFrom`, or `Into` implementations\n2. **Standard trait replacements**: `default_config()`,\n `format_error()`, `as_bytes(&self)`, `compare()` that\n should be `Default`, `Display`, `AsRef`, or `PartialEq`\n3. **Error conversion wrappers**: `io_to_my_error()`,\n `wrap_error()` that should be `impl From` or\n thiserror `#[from]`\n4. **Manual combinators**: `match opt { Some(x) => Some(f(x)),\n None => None }` that should be `.map()`, `.unwrap_or()`,\n `.flatten()`, etc.\n\n## Why It Matters\n\nRust's trait system is compositional by design:\n\n- `impl From for B` gives `impl Into for A` for free\n- `impl Display` gives `ToString` for free\n- `From` enables the `?` operator for error propagation\n- Trait impls participate in generic bounds and blanket impls\n- Standard combinators are optimized and well-tested\n\nHelper functions that bypass this system create API\ninconsistency, miss ergonomic benefits, and signal\nunfamiliarity with idiomatic Rust.\n\n## Safe Patterns\n\n```rust\n// Good: From trait enables .into() and ? operator\nimpl From for Settings {\n fn from(c: Config) -> Self {\n Settings { timeout: c.timeout }\n }\n}\n\n// Good: FromStr enables .parse()\nimpl FromStr for Config {\n type Err = ConfigError;\n fn from_str(s: &str) -> Result { ... }\n}\n\n// Good: Default via derive\n#[derive(Default)]\nstruct Config { timeout: u64 }\n\n// Good: Display for human-readable output\nimpl fmt::Display for MyError {\n fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {\n write!(f, \"Error: {}\", self.msg)\n }\n}\n\n// Good: Option combinators\nlet result = opt.map(|x| x.to_string());\nlet value = opt.unwrap_or(default);\n```\n\n## Patterns to Flag\n\n```rust\n// Flag: should be impl FromStr\nfn parse_config(s: &str) -> Config { ... }\n\n// Flag: should be impl From for Foo\nfn foo_from_bar(b: Bar) -> Foo { ... }\n\n// Flag: should be impl Default\nfn default_config() -> Config { ... }\n\n// Flag: should be impl From for MyError\nfn io_to_my_error(e: io::Error) -> MyError { ... }\n\n// Flag: should use .map()\nmatch opt {\n Some(x) => Some(x.to_string()),\n None => None,\n}\n```\n\n## Exclusions (Not Flagged)\n\n- Lossy conversions (`to_lossy_ascii`)\n- Builder methods (`with_timeout(self, ...)`)\n- Multi-parameter conversions (context-dependent)\n- Domain-specific operations (`serialize`, `encode`, `decode`)\n\n## Related Clippy Lints\n\n| Lint | Detects |\n|------|---------|\n| `clippy::from_over_into` | `impl Into` where `impl From` suffices |\n| `clippy::manual_map` | Match on Option rewriting `.map()` |\n| `clippy::manual_unwrap_or` | Match rewriting `.unwrap_or()` |\n| `clippy::derivable_impls` | Manual Default that derive handles |\n| `clippy::manual_flatten` | Nested iteration rewriting `.flatten()` |\n| `clippy::new_without_default` | `fn new()` without `impl Default` |\n\n## Output Section\n\n```markdown\n## Builtin Preference\n### Issues Found\n- [file:line] Conversion helper `parse_config`: use `impl FromStr`\n- [file:line] Manual combinator: use `.map()` (clippy::manual_map)\n\n### Recommendations\n- Implement standard traits to gain ecosystem composability\n- Enable relevant clippy lints for automated enforcement\n```\n\nFile v1.9.19:modules/cargo-dependencies.md\n\n---\nname: cargo-dependencies\ndescription: Dependency auditing, security scanning, and version management\ncategory: rust-review\ntags: [cargo, dependencies, security, audit]\n---\n\n# Cargo Dependencies\n\nAudit and management of Cargo dependencies and build configuration.\n\n## Audit Commands\n\nRun detailed dependency analysis:\n```bash\ncargo tree -d # Find duplicates\ncargo audit # Security vulnerabilities\ncargo outdated # Stale versions\ncargo deny check # Policy enforcement\n```\n\n## Dependency Evaluation\n\nCheck:\n- Feature flags usage\n- Optional dependencies\n- Build scripts safety\n- Binary size impact\n- Compilation time\n\n## Security Scanning\n\nReview for:\n- Known vulnerabilities\n- Abandoned crates\n- Unmaintained dependencies\n- Security advisories\n- Supply chain risks\n\n## Version Management\n\nVerify:\n- Semver compliance\n- Version pinning strategy\n- Dependency updates frequency\n- Breaking change handling\n\n## Common Issues\n\nFlag:\n- Abandoned crates\n- Excessively large dependencies\n- Security-vulnerable versions\n- Duplicate dependencies\n- Unnecessary dependencies\n\n## Alternatives Suggestion\n\nRecommend alternatives for:\n- Unmaintained crates\n- Heavy dependencies\n- Vulnerable versions\n- Better maintained options\n\n## Output Section\n\n```markdown\n## Dependencies\n### Security Issues\n- [crate@version] Vulnerability: [CVE/advisory]\n\n### Recommendations\n- Update [crate] from X to Y\n- Replace [abandoned-crate] with [alternative]\n- Remove unused dependency: [crate]\n```\n\nFile v1.9.19:modules/cfg-test-misuse.md\n\n---\nname: cfg-test-misuse\ndescription: Detection of #[cfg(test)] applied to individual functions or\n impls outside a mod tests block, which is a common structural mistake\ncategory: rust-review\ntags: [testing, cfg, attributes, structure]\n---\n\n# cfg(test) Misuse\n\nAnalysis of `#[cfg(test)]` placement on individual items outside a\n`mod tests { ... }` block.\n\n## What This Detects\n\n`#[cfg(test)]` on a standalone `fn`, `impl`, or `struct` that is not nested\ninside a `mod tests` block.\n\n## Why It Matters\n\nThe idiomatic pattern is a single `mod tests` block gated with `#[cfg(test)]`,\nwhich keeps test code in one place.\nApplying `#[cfg(test)]` to an individual `impl` block is particularly\nhazardous: it removes method implementations from the production binary\nwithout an obvious compiler warning.\n\n## Safe Patterns\n\n```rust\n// Good: single gated mod tests block\n#[cfg(test)]\nmod tests {\n use super::*;\n\n fn helper() { ... }\n\n #[test]\n fn test_something() { ... }\n}\n```\n\n## Patterns to Flag\n\n```rust\n// Bad: cfg(test) on standalone function\n#[cfg(test)]\nfn setup_fixture() { ... }\n\n// Bad: cfg(test) on impl block outside mod tests\n#[cfg(test)]\nimpl MyStruct {\n fn test_helper(&self) { ... }\n}\n```\n\n## Output Section\n\n```markdown\n## cfg(test) Misuse\n### Issues Found\n- [file:line] cfg(test) outside mod tests: [explanation]\n\n### Recommendations\n- Move all test-only items inside a single `#[cfg(test)] mod tests` block\n```\n\nFile v1.9.19:modules/collection-types.md\n\n---\nname: collection-types\ndescription: Detection of Vec used where HashSet or HashMap semantics apply,\n including contains loops, find loops, and dedup patterns\ncategory: rust-review\ntags: [collections, performance, vec, hashset, hashmap]\n---\n\n# Collection Types\n\nAnalysis of `Vec` usage where a different collection type would be more\ncorrect or more efficient.\n\n## What This Detects\n\n- `vec.contains(&x)` -- O(n) membership test; `HashSet` gives O(1)\n- `vec.dedup()` -- sorting + dedup pattern suggests a set\n- `vec.iter().find(...)` / `vec.iter().position(...)` -- linear key lookup\n suggests `HashMap`\n\n## Why It Matters\n\nUsing `Vec` for set or map operations produces O(n) behaviour where O(1) is\navailable.\nIt also signals unclear intent: a `HashSet` communicates uniqueness, a\n`HashMap` communicates keyed access.\n\n## Safe Patterns\n\n```rust\n// Good: set membership\nlet mut seen: HashSet = HashSet::new();\nif seen.contains(&id) { ... }\n\n// Good: keyed lookup\nlet mut index: HashMap = HashMap::new();\nif let Some(user) = index.get(&id) { ... }\n```\n\n## Patterns to Flag\n\n```rust\n// Flag: O(n) membership on unbounded Vec\nusers.contains(&new_user)\n\n// Flag: dedup implies uniqueness invariant\nids.sort();\nids.dedup();\n\n// Flag: linear key scan\nusers.iter().find(|u| u.id == target_id)\n```\n\n## Output Section\n\n```markdown\n## Collection Types\n### Issues Found\n- [file:line] Vec used as set/map: [explanation]\n\n### Recommendations\n- Replace with HashSet for membership checks\n- Replace with HashMap for keyed access\n```\n\nFile v1.9.19:modules/concurrency-patterns.md\n\n---\nname: concurrency-patterns\ndescription: Concurrency cost hierarchy, synchronization primitives, async patterns, and performance-aware concurrency review\ncategory: rust-review\ntags: [concurrency, async, sync, deadlock, atomics, contention, performance]\n---\n\n# Concurrency Patterns\n\nAnalysis of concurrent and async code patterns in Rust,\ngrounded in the concurrency cost hierarchy.\n\n## Concurrency Cost Hierarchy\n\n\"Acquiring a mutex isn't slow; contention is slow.\"\nBefore reviewing concurrency code, classify each\nsynchronization point by its cost tier.\n\n| Level | Name | Approx Cost | Description |\n|-------|------|-------------|-------------|\n| 0 | Thread-local | ~2 ns | No atomics at all; per-thread state |\n| 1 | Uncontended atomics | ~10 ns | Atomic ops, no cross-core sharing |\n| 2 | Contended atomics | ~40-400 ns | Cache-line transfer between cores |\n| 3 | Syscalls | ~1 us | Kernel transitions on lock paths |\n| 4 | Context switches | ~10 us | Blocking locks, scheduler involvement |\n| 5 | Catastrophe | ~ms+ | Spinning on oversubscribed systems |\n| 6 | Kernel page fault | ~100-400 ms | Paged-out buffer re-faulted on access |\n\nLevels 3-5 are performance bugs. Level 6 is invisible in\ntask scheduler traces (tokio-console shows tasks scheduling\nin microseconds while the actual latency occurs in the\nkernel page fault handler). Target Level 2 as the default.\nAchieve Level 1 through contention reduction. Level 0\nrequires architectural redesign (per-thread computation\nwith periodic merges).\n\n**Level 6: kernel paging latency**: Long-lived Tokio\nruntimes that co-reside with large heap users (e.g., ML\nmodel weights) are at risk: the kernel may page out\nlatency-sensitive buffers during idle periods. When audio\nor ring-ring buffers page back in on the next access, the\npage fault adds 100–400 ms of p99 latency with no Tokio\ntrace signal.\n\nDetection: production-only latency spikes, no reproduction\non dev box, `perf stat` shows elevated `page-faults` on\nthe audio/buffer threads.\n\nFix: `libc::mlock` on the buffer pages. See\n`modules/unsafe-audit.md` for the full production checklist\n(RLIMIT_MEMLOCK, page alignment, ENOMEM fallback).\n\n**Key insight**: Performance is dominated by atomic\ninstruction count, not total instruction count. An\nalgorithm with 9x more total instructions but the\nsame number of atomics performs identically.\n\n### What to Flag in Review\n\n- **Level 5**: Fair spin-locks on thread pools larger\n than core count. Always flag.\n- **Level 4**: `std::sync::Condvar` wake patterns that\n convoy. Flag when hot path.\n- **Level 3**: `sched_yield()` or `thread::yield_now()`\n in lock loops. Suggest backoff or parking.\n- **Level 2 (avoidable)**: Atomic RMW on shared counter\n when per-thread counters and merge would suffice.\n- **False sharing**: Independent atomics on the same\n cache line (64 bytes). Suggest `#[repr(align(64))]`\n or `crossbeam_utils::CachePadded`.\n\n## Synchronization Primitives\n\nReview primitives usage:\n\n- `Arc`, `Mutex`, `RwLock`\n- `Atomic*` types and ordering (`Relaxed` vs `SeqCst`)\n- `tokio::sync` (mpsc, broadcast, watch, Semaphore)\n- `Send`/`Sync` bounds\n- `parking_lot` vs `std::sync` trade-offs\n\n### Memory Ordering Review\n\nCheck ordering is neither too weak nor too strong:\n\n- `Relaxed`: Counters, statistics (no cross-variable\n ordering needed)\n- `Acquire`/`Release`: Publish/consume patterns,\n one-shot flags\n- `SeqCst`: Only when total order across multiple\n atomics is required (rare; flag overuse)\n\n## Async Patterns\n\nCheck async code:\n\n- No blocking in async functions\n- Proper `spawn_blocking` usage\n- Guards dropped before awaiting\n- Cancellation safety\n- Task spawning patterns\n\n## Best Practices\n\n```rust\n// Good: Drop guard before await\nasync fn update(data: Arc>) {\n let value = {\n let guard = data.lock().await;\n guard.value.clone()\n }; // Guard dropped\n process(value).await;\n}\n\n// Good: Cache-padded to prevent false sharing\nuse crossbeam_utils::CachePadded;\n\nstruct Counters {\n reads: CachePadded,\n writes: CachePadded,\n}\n```\n\n## Contention Reduction Patterns\n\nWhen review finds Level 2+ contention on hot paths:\n\n1. **Shard the lock**: `DashMap`, `ShardedLock`, or\n manual sharding by key hash\n2. **Per-thread accumulation**: Thread-local counters\n merged at read time (Level 2 to Level 0)\n3. **Read-copy-update (RCU)**: `arc-swap` for\n read-heavy, write-rare data\n4. **Lock-free structures**: `crossbeam` queues and\n deques when contention dominates\n\n## Deadlock Prevention\n\nIdentify potential deadlocks:\n\n- Lock ordering consistency\n- Nested locks\n- Await points while holding locks\n- Circular dependencies\n\n## Data Race Detection\n\nCheck for:\n\n- `static mut` misuse\n- Shared mutable state\n- Missing synchronization\n- Race conditions\n\n## Send/Sync Bounds\n\nVerify:\n\n- Proper trait bounds\n- Thread safety guarantees\n- Cross-thread data transfer\n- Closure captures\n\n## Common Issues\n\n- Blocking in async context\n- Guards held across await points\n- Inconsistent lock ordering\n- Missing bounds on generics\n- Unsafe Send/Sync implementations\n- `SeqCst` used everywhere (usually `Acquire`/`Release`\n suffices; `SeqCst` adds unnecessary fence cost)\n- Spinning without backoff on oversubscribed systems\n- False sharing between independent atomics\n\n## Output Section\n\n```markdown\n## Concurrency\n### Cost Classification\n- [file:line] Level N: [primitive] - [justification]\n\n### Issues Found\n- [file:line] Guard held across await: [details]\n- [file:line] Potential deadlock: [scenario]\n- [file:line] False sharing risk: [layout details]\n- [file:line] Unnecessary SeqCst: [suggest weaker ordering]\n\n### Recommendations\n- [concurrency improvements with cost tier impact]\n```\n\n## References\n\n- Jon Gjengset, \"The Cost of Concurrency Coordination\"\n (video: youtube.com/watch?v=tND-wBBZ8RY)\n- Travis Downs, \"A Concurrency Cost Hierarchy\"\n (travisdowns.github.io/blog/2020/07/06/concurrency-costs.html)\n- Mara Bos, \"Rust Atomics and Locks\" (O'Reilly)\n\nFile v1.9.19:modules/duplicate-validators.md\n\n---\nname: duplicate-validators\ndescription: Detection of multiple validate_*, check_*, or verify_* functions\n that share similar structure and could be consolidated\ncategory: rust-review\ntags: [design, duplication, validation, refactoring]\n---\n\n# Duplicate Validators\n\nAnalysis of `validate_*`, `check_*`, and `verify_*` functions for\nopportunities to consolidate repeated validation logic.\n\n## What This Detects\n\nThree or more functions sharing the same verb prefix (`validate_`, `check_`,\n`verify_`) within a single file, which often indicates copy-pasted validation\nlogic that could be unified.\n\n## Why It Matters\n\nDuplicated validation logic diverges over time: one copy gets a bug fix or a\nnew rule while the others do not.\nConsolidating into a generic validator ensures all callers benefit from each\nfix.\n\n## Safe Patterns\n\n```rust\n// Good: single generic validator with rule injection\nfn validate_field(value: &str, rules: &[ValidationRule])\n -> Result<(), ValidationError>\n{\n for rule in rules {\n rule.apply(value)?;\n }\n Ok(())\n}\n```\n\n## Patterns to Flag\n\n```rust\n// Flag when 3+ share the same prefix:\nfn validate_email(s: &str) -> bool { ... }\nfn validate_phone(s: &str) -> bool { ... }\nfn validate_username(s: &str) -> bool { ... }\nfn validate_password(s: &str) -> bool { ... }\n```\n\n## Output Section\n\n```markdown\n## Duplicate Validators\n### Issues Found\n- [file] 4 validate_* functions: [list]\n\n### Recommendations\n- Extract shared logic into a generic validator\n- Use a trait or rule-set parameter to unify related checks\n```\n\nFile v1.9.19:modules/error-handling.md\n\n---\nname: error-handling\ndescription: Result/Option patterns, custom error types, and error propagation analysis\ncategory: rust-review\ntags: [errors, result, option, propagation]\n---\n\n# Error Handling\n\nAnalysis of error handling patterns and correctness in Rust code.\n\n## Result and Option Usage\n\nEvaluate:\n- `Result` and `Option` usage patterns\n- Custom error types design\n- Context addition with `anyhow` or `thiserror`\n- `?` propagation correctness\n\n## Error Type Design\n\nCheck custom error types:\n- Implements `std::error::Error`\n- Provides meaningful context\n- Conversion traits (`From`, `Into`)\n- Error hierarchy structure\n\n## Error Propagation\n\nBest practices:\n```rust\n// Good: Proper error propagation\nfn process() -> Result<(), ProcessError> {\n let data = fetch().context(\"failed to fetch\")?;\n validate(&data)?;\n Ok(())\n}\n```\n\n## Common Issues to Flag\n\n- Panics in library code (`unwrap`, `expect`)\n- Logging side-effects in error paths\n- Mismatched error hierarchies\n- Missing retry/backoff logic\n- Silent error swallowing\n- Over-generic error types\n\n## Error Context\n\nVerify context is added:\n- Operation context\n- Input data context\n- Failure reasons\n- Recovery suggestions\n\n## Output Section\n\n```markdown\n## Error Handling\n### Issues Found\n- [file:line] Panic in library: [details]\n- [file:line] Missing context: [suggestion]\n\n### Recommendations\n- [error handling improvements]\n```\n\nFile v1.9.19:modules/error-messages.md\n\n---\nname: error-messages\ndescription: Detection of short error strings (under ~20 chars) in\n Err(), panic!(), and expect() that lack context or recovery hints\ncategory: rust-review\ntags: [error-handling, diagnostics, messages, quality]\n---\n\n# Error Messages\n\nAnalysis of error and panic messages for actionability.\nShort messages without context make production incidents harder to diagnose.\n\n## What This Detects\n\nString literals under roughly 20 characters used in:\n\n- `Err(\"short msg\")`\n- `panic!(\"short msg\")`\n- `.expect(\"short msg\")`\n- `Err(\"short msg\".to_string())`\n\n## Why It Matters\n\nA message like `\"not found\"` or `\"failed\"` gives an on-call engineer no\ninformation about what was not found, where the failure occurred, or how to\nrecover.\n\n## Safe Patterns\n\n```rust\n// Good: identifies operation and input\n.expect(\"failed to open config file at $CONFIG_PATH\")\n\n// Good: Err with context\nreturn Err(format!(\n \"user {} not found in tenant {}\",\n user_id, tenant_id\n));\n```\n\n## Patterns to Flag\n\n```rust\n// Bad: no context\n.expect(\"failed\")\n.expect(\"not found\")\n\n// Bad: Err with bare short string\nreturn Err(\"bad input\");\nreturn Err(\"denied\".to_string());\n```\n\n## Output Section\n\n```markdown\n## Error Messages\n### Issues Found\n- [file:line] Short error message: [explanation]\n\n### Recommendations\n- Add operation context: what were you trying to do?\n- Add input context: what value triggered the failure?\n- Add recovery hints where possible\n```\n\nFile v1.9.19:modules/iterator-and-allocation-slop.md\n\n---\nmodule: iterator-and-allocation-slop\ncategory: detection\ndependencies: [Read, Grep]\nestimated_tokens: 600\n---\n\n# Iterator and Allocation Slop\n\n**AI-generated Rust defaults to manual loops where\niterators read better, and to allocation where references\nsuffice. Both compile; both are slop.**\n\nThis module covers two of the highest-frequency AI Rust\nanti-patterns: imperative loops the iterator API expresses\nin one line, and unnecessary allocation that\nborrow-checker capitulation produces. The clippy lints\ncatch most of this; the rest is judgment.\n\n## Iterator slop\n\n### Pattern 1: index-based loops\n\n```rust\n// SLOP\nlet mut sum = 0;\nfor i in 0..vec.len() {\n sum += vec[i];\n}\n\n// Idiomatic\nlet sum: i32 = vec.iter().sum();\n```\n\nDetector: `clippy::needless_range_loop`.\n\n### Pattern 2: filter-then-push\n\n```rust\n// SLOP\nlet mut result = Vec::new();\nfor x in xs.iter() {\n if x.is_active() {\n result.push(x.id);\n }\n}\n\n// Idiomatic\nlet result: Vec<_> = xs.iter()\n .filter(|x| x.is_active())\n .map(|x| x.id)\n .collect();\n```\n\n### Pattern 3: map-filter-unwrap\n\n```rust\n// SLOP\nlet firsts: Vec<_> = xs.iter()\n .map(|x| x.first())\n .filter(|x| x.is_some())\n .map(|x| x.unwrap())\n .collect();\n\n// Idiomatic\nlet firsts: Vec<_> = xs.iter()\n .filter_map(|x| x.first())\n .collect();\n```\n\nDetector: `clippy::manual_filter_map`.\n\n### Pattern 4: collect-then-iterate\n\n```rust\n// SLOP\nlet intermediate: Vec<_> = xs.iter().map(transform).collect();\nfor item in intermediate {\n use_it(item);\n}\n\n// Idiomatic\nfor item in xs.iter().map(transform) {\n use_it(item);\n}\n```\n\nDetector: `clippy::needless_collect`.\n\n### Pattern 5: bool-match where if suffices\n\n```rust\n// SLOP\nmatch flag {\n true => do_a(),\n false => do_b(),\n}\n\n// Idiomatic\nif flag { do_a() } else { do_b() }\n```\n\nDetector: `clippy::match_bool`.\n\n## Allocation slop\n\n### Pattern A: `.clone()` to satisfy the borrow checker\n\nThe single most common AI-generated Rust anti-pattern. The\n`rust-unofficial/patterns` book lists it as the canonical\nanti-pattern: cloning to make a borrow-checker error go\naway rather than to express ownership.\n\n```rust\n// SLOP\nfn greet(name: String) {\n println!(\"Hello, {name}\");\n}\nfn main() {\n let n = String::from(\"world\");\n greet(n.clone()); // unnecessary clone\n greet(n.clone());\n}\n\n// Idiomatic\nfn greet(name: &str) {\n println!(\"Hello, {name}\");\n}\nfn main() {\n let n = String::from(\"world\");\n greet(&n);\n greet(&n);\n}\n```\n\nDetection heuristic: any `.clone()` on a `String`,\n`Vec<_>`, `HashMap<_,_>`, `Arc>`, or large\nstruct that is *not* paired with a comment explaining\nthe ownership rationale. If it disappeared, would the\nborrow checker complain? If yes, the right fix is\nusually to take a borrowed reference (`&str`, `&[T]`,\n`&T`).\n\nDetectors: `clippy::redundant_clone`,\n`clippy::clone_on_ref_ptr`.\n\n### Pattern B: owned parameters that should borrow\n\n| Slop signature | Idiomatic signature |\n|---|---|\n| `fn f(s: &String)` | `fn f(s: &str)` |\n| `fn f(v: &Vec)` | `fn f(v: &[T])` |\n| `fn f(s: String)` (read-only) | `fn f(s: &str)` |\n| `fn f(v: Vec)` (read-only) | `fn f(v: &[T])` |\n| `fn f(b: Box)` (no boxing reason) | `fn f(t: T)` |\n\nDetector: `clippy::ptr_arg` catches `&Vec` and `&String`.\n\n### Pattern C: `format!` then convert\n\n```rust\n// SLOP\nlet s = format!(\"{}\", x);\n\n// Idiomatic (when Display is implemented)\nlet s = x.to_string();\n\n// Inverse SLOP\nlet s = x.to_string();\nlet s = format!(\"{s}{rest}\");\n\n// Idiomatic (build the string once)\nlet s = format!(\"{x}{rest}\");\n```\n\nDetectors: `clippy::useless_format`, `clippy::str_to_string`.\n\n### Pattern D: redundant allocation\n\n```rust\n// SLOP\nlet owned = borrowed.to_owned();\nfn take_str(s: &str) { ... }\ntake_str(&owned);\n\n// Idiomatic — borrow directly\ntake_str(borrowed);\n\n// SLOP\nlet s = String::new();\nlet s = s + \"hello\" + \" \" + \"world\";\n\n// Idiomatic\nlet s = String::from(\"hello world\");\n```\n\n### Pattern E: `Box::new(...)` without indirection reason\n\n```rust\n// SLOP\nlet x = Box::new(42_u64);\nfn use_it(n: u64) { ... }\nuse_it(*x);\n\n// Idiomatic\nlet x = 42_u64;\nuse_it(x);\n```\n\nHeap allocation is justified for:\n- `dyn Trait` objects (`Box`, `Box`).\n- Recursive types (`Box`).\n- Large stack-frame avoidance (uncommon; measure first).\n- Pinning requirements (`Pin>`).\n\nAnywhere else, `Box::new` is unjustified allocation.\n\n### Pattern E2: `Box` or `&dyn Trait` in a hot inner loop\n\nThis is distinct from Pattern E. The box itself may be\njustified: the problem is calling a `dyn` method millions of\ntimes when the method body is tiny.\n\n```rust\n// Potentially slow: dyn dispatch in the inner loop\nlet decoders: Vec> = build_decoders(&schema, &batch);\n\nfor i in 0..n_rows {\n for d in &decoders {\n d.write_to_row(i, &mut row); // indirect call every iteration\n }\n}\n```\n\n**Why it matters**: each `dyn` call goes through a vtable\n(`call *0x18(%rax)`). The compiler cannot inline across that\nboundary, so it cannot fuse the inner loop, vectorize small\nstores, or eliminate the function-call prologue/epilogue overhead.\nWhen the method body is ~25 instructions (a null check, a bit\nflip, a 4-byte move), the prologue/epilogue and indirect\njump overhead can represent 40–50% of total runtime.\n\nNote: `&dyn Trait` has the same problem as `Box`.\nThe issue is dynamic dispatch, not heap allocation.\n\n**Fix 1: flip the loop order (batch-first)**: iterate\nall rows for each decoder, not all decoders for each row.\nThe dyn dispatch cost is paid once per decoder per batch\ninstead of once per cell:\n\n```rust\nlet mut rows: Vec = (0..n_rows)\n .map(|i| WriteRow::new(&segment, key_array.value(i)))\n .collect();\n\nfor d in &decoders {\n d.write_to_rows(0, &mut rows[..]); // dispatch once per decoder\n}\n```\n\n**Fix 2: enum dispatch**: replace `dyn Trait` with a\nclosed enum. The compiler can monomorphize and inline each\nvariant:\n\n```rust\nenum ColDecoder {\n F32(F32Decoder),\n Utf8(Utf8Decoder),\n Bool(BoolDecoder),\n}\n\nimpl ColDecoder {\n #[inline(always)]\n fn write_to_row(&self, index: usize, row: &mut WriteRow) {\n match self {\n ColDecoder::F32(d) => d.write_to_row(index, row),\n ColDecoder::Utf8(d) => d.write_to_row(index, row),\n ColDecoder::Bool(d) => d.write_to_row(index, row),\n }\n }\n}\n```\n\n**When to flag**: any `Vec>` or `Vec<&dyn Trait>`\niterated inside an inner loop where the method body is\ninlineable. The Java/JVM analogy is instructive: the JVM\nde-virtualizes and inlines at JIT time; rustc cannot cross\nthe `dyn` boundary. If reviewers come from JVM backgrounds,\nthis is the most important Rust performance lesson to surface.\n\nDetection:\n\n```bash\n# Find Vec> that appear inside nested loops\nrg \"Vec = vec![2, 3, 5, 7, 11];\n\n// Idiomatic for small, fixed-size\nlet primes: [u32; 5] = [2, 3, 5, 7, 11];\n\n// Or for stack-allocated growable\nlet primes: SmallVec<[u32; 5]> = smallvec![2, 3, 5, 7, 11];\n```\n\nThis one is judgment: arrays for compile-time-known\nsizes, `SmallVec`/`ArrayVec` for \"usually small but\nsometimes grows\", `Vec` for genuinely dynamic.\n\n## Detection commands\n\n```bash\n# Catch most iterator slop with clippy\ncargo clippy --all-targets -- \\\n -W clippy::needless_range_loop \\\n -W clippy::manual_filter_map \\\n -W clippy::needless_collect \\\n -W clippy::filter_map_next \\\n -W clippy::map_unwrap_or \\\n -W clippy::match_bool \\\n -W clippy::needless_match \\\n -D warnings\n\n# Catch most allocation slop with clippy\ncargo clippy --all-targets -- \\\n -W clippy::redundant_clone \\\n -W clippy::clone_on_ref_ptr \\\n -W clippy::ptr_arg \\\n -W clippy::useless_format \\\n -W clippy::str_to_string \\\n -W clippy::redundant_allocation \\\n -D warnings\n\n# Manual scan: every .clone() in the codebase\nrg \"\\.clone\\(\\)\" --type rust -n | head -50\n# For each: ask \"would the borrow checker complain if removed?\"\n```\n\n## False positives\n\nSome `.clone()` calls are correct and should stay:\n\n- **Async / spawn boundaries**: cloning an `Arc` to\n send into `tokio::spawn` is required, not slop.\n- **Genuine ownership transfer**: when two callers each\n need to mutate independently from the same source.\n- **Intentional defensive copy**: in security-sensitive\n paths where the original might be modified by an\n attacker. Mark with `// COPY: defense against ...`.\n\nWhen in doubt, comment the rationale next to the\n`.clone()`. A `.clone()` with a one-line \"why\" comment\nis documented intent; a `.clone()` with no comment is\nslop.\n\n## Output format\n\nFor each finding, use the\n`Skill(scribe:slop-detector)` module\n`structured-finding-output.md` format. Severity is\n`medium` for individual iterator/allocation slop;\n`high` if the same pattern appears 5+ times in the same\nfile (suggests systematic AI-generation rather than\nisolated mistake).\n\n## Integration\n\nIterator and allocation slop typically lands in Pass 5\n(code idiom sweep) of the multi-pass cleanup workflow\n(see `Skill(scribe:slop-detector)` module\n`cleanup-workflow.md`). Run after the linter floor\n(Pass 1) clears, since clippy will flag most of these\nautomatically.\n\nArchive v1.9.17: 19 files, 33444 bytes\n\nFiles: modules/async-slop.md (7937b), modules/builtin-preference.md (3751b), modules/cargo-dependencies.md (1516b), modules/cfg-test-misuse.md (1438b), modules/collection-types.md (1539b), modules/concurrency-patterns.md (5998b), modules/duplicate-validators.md (1559b), modules/error-handling.md (1405b), modules/error-messages.md (1459b), modules/iterator-and-allocation-slop.md (9416b), modules/model-specific-tells.md (7032b), modules/ownership-analysis.md (1469b), modules/silent-returns.md (1678b), modules/sql-injection.md (1520b), modules/test-slop.md (8018b), modules/unsafe-audit.md (3876b), skill-card.md (2032b), SKILL.md (4549b), _meta.json (142b)\n\nFile v1.9.17:SKILL.md\n\n---\nname: rust-review\ndescription: Audits Rust code for unsafe blocks, ownership issues, and Cargo dependency risks\nversion: 1.9.8\ntriggers:\n - rust\n - ownership\n - concurrency\n - unsafe\n - traits\n - cargo\n - reviewing Rust code or before merging Rust changes\nmetadata: {\"openclaw\": {\"homepage\": \"https://github.com/athola/claude-night-market/tree/master/plugins/pensive\", \"emoji\": \"\\ud83d\\udd0d\", \"requires\": {\"config\": [\"night-market.pensive:shared\", \"night-market.imbue:proof-of-work\"]}}}\nsource: claude-night-market\nsource_plugin: pensive\n---\n\n> **Night Market Skill** — ported from [claude-night-market/pensive](https://github.com/athola/claude-night-market/tree/master/plugins/pensive). For the full experience with agents, hooks, and commands, install the Claude Code plugin.\n\n\n## Table of Contents\n\n- [Quick Start](#quick-start)\n- [When to Use](#when-to-use)\n- [Required TodoWrite Items](#required-todowrite-items)\n- [Progressive Loading](#progressive-loading)\n- [Core Workflow](#core-workflow)\n- [Rust Quality Checklist](#rust-quality-checklist)\n- [Safety](#safety)\n- [Correctness](#correctness)\n- [Performance](#performance)\n- [Idioms](#idioms)\n- [Output Format](#output-format)\n- [Summary](#summary)\n- [Ownership Analysis](#ownership-analysis)\n- [Error Handling](#error-handling)\n- [Concurrency](#concurrency)\n- [Unsafe Audit](#unsafe-audit)\n- [[U1] file:line](#[u1]-file:line)\n- [Dependencies](#dependencies)\n- [Recommendation](#recommendation)\n- [Exit Criteria](#exit-criteria)\n\n\n# Rust Review Workflow\n\nExpert-level Rust code audits with focus on safety, correctness, and idiomatic patterns.\n\n## Quick Start\n\n```bash\n/rust-review\n```\n**Verification:** Run the command with `--help` flag to verify availability.\n\n## When To Use\n\n- Reviewing Rust code changes\n- Auditing unsafe blocks\n- Analyzing concurrency patterns\n- Dependency security review\n- Performance optimization review\n\n## When NOT To Use\n\n- General code review without Rust - use unified-review\n- Performance profiling - use parseltongue:python-performance pattern\n\n## Required TodoWrite Items\n\n1. `rust-review:ownership-analysis`\n2. `rust-review:error-handling`\n3. `rust-review:concurrency`\n4. `rust-review:unsafe-audit`\n5. `rust-review:cargo-deps`\n6. `rust-review:evidence-log`\n\n## Progressive Loading\n\nLoad modules as needed based on review scope:\n\n**Quick Review** (ownership and errors):\n- See `modules/ownership-analysis.md` for borrowing and lifetime analysis\n- See `modules/error-handling.md` for Result/Option patterns\n\n**Concurrency Focus**:\n- See `modules/concurrency-patterns.md` for async and sync primitives\n\n**Safety Audit**:\n- See `modules/unsafe-audit.md` for unsafe block documentation\n\n**Dependency Review**:\n- See `modules/cargo-dependencies.md` for vulnerability scanning\n\n**Idiomatic Patterns**:\n- See `modules/builtin-preference.md` for conversion traits and builtin preference\n\n## Core Workflow\n\n1. **Ownership Analysis**: Check borrowing, lifetimes, clone patterns\n2. **Error Handling**: Verify Result/Option usage, propagation\n3. **Concurrency**: Review async patterns, sync primitives\n4. **Unsafe Audit**: Document invariants, FFI contracts\n5. **Dependencies**: Scan for vulnerabilities, updates\n6. **Evidence Log**: Record commands and findings\n\n## Rust Quality Checklist\n\n### Safety\n- [ ] All unsafe blocks documented with SAFETY comments\n- [ ] FFI boundaries properly wrapped\n- [ ] Memory safety invariants maintained\n- [ ] `mlock`/`munlock` calls: RLIMIT verified, page-aligned,\n ENOMEM handled\n\n### Correctness\n- [ ] Error handling complete\n- [ ] Concurrency patterns sound\n- [ ] Tests cover critical paths\n\n### Performance\n- [ ] No unnecessary allocations\n- [ ] Borrowing preferred over cloning\n- [ ] Async properly non-blocking\n\n### Idioms\n- [ ] Standard traits implemented\n- [ ] Conversion traits preferred over helper functions\n- [ ] Error types well-designed\n- [ ] Documentation complete\n\n## Output Format\n\n```markdown\n## Summary\nRust audit findings\n\n## Ownership Analysis\n[borrowing and lifetime issues]\n\n## Error Handling\n[error patterns and issues]\n\n## Concurrency\n[async and sync patterns]\n\n## Unsafe Audit\n### [U1] file:line\n- Invariants: [documented]\n- Risk: [assessment]\n- Recommendation: [action]\n\n## Dependencies\n[cargo audit results]\n\n## Recommendation\nApprove / Approve with actions / Block\n```\n**Verification:** Run the command with `--help` flag to verify availability.\n\n## Exit Criteria\n\n- All unsafe blocks audited\n- Concurrency patterns verified\n- Dependencies scanned\n- Evidence logged\n- Action items assigned\n\nFile v1.9.17:_meta.json\n\n{\n \"ownerId\": \"kn7d107jg9jv602h9ytsegydq184a42s\",\n \"slug\": \"nm-pensive-rust-review\",\n \"version\": \"1.9.17\",\n \"publishedAt\": 1785389967389\n}\n\nFile v1.9.17:modules/async-slop.md\n\n---\nmodule: async-slop\ncategory: detection\ndependencies: [Read, Grep]\nestimated_tokens: 500\n---\n\n# Async Slop\n\n**AI defaults to `async` and `tokio::spawn` even where\nsync code is faster, simpler, and correct.**\n\nThis module covers the high-frequency async patterns that\nlook idiomatic but are not. The clippy lints catch some;\nthe rest is structural.\n\n## Pattern 1: `async fn` that contains no `.await`\n\n```rust\n// SLOP\nasync fn compute_total(items: &[Item]) -> u64 {\n items.iter().map(|i| i.price).sum()\n}\n```\n\nIf the function body has no `.await`, it has no reason\nto be `async`. Async coloring is contagious: this\nfunction is callable only from async contexts, forcing\nevery caller to also be `async`. Strip `async` from the\nsignature unless the body actually awaits.\n\nDetection (preferred: clippy):\n\n```bash\ncargo clippy --all-targets -- -W clippy::async_yields_async\n```\n\nFile-level heuristic when clippy is unavailable:\n\n```bash\nfor f in $(rg -l \"async fn \" --type rust); do\n rg -q \"\\.await\" \"$f\" || echo \"no-await: $f\"\ndone\n```\n\n(Heuristic; manual review needed since `.await` may be in\na helper called by the async fn rather than inline.)\n\n## Pattern 2: blocking I/O inside an async runtime\n\n```rust\n// SLOP\nasync fn read_config() -> Result {\n Ok(std::fs::read_to_string(\"config.toml\")?)\n}\n\n// SLOP\nasync fn rate_limit_wait() {\n std::thread::sleep(Duration::from_secs(1)); // blocks the runtime\n}\n\n// SLOP\nasync fn query_db(conn: &Connection) -> Result> {\n conn.query(\"SELECT ...\")? // blocking driver\n}\n```\n\nBlocking calls inside `async` block the entire executor\nthread, defeating the runtime's concurrency model.\n\nFix:\n\n```rust\n// Use the async equivalent\nasync fn read_config() -> Result {\n Ok(tokio::fs::read_to_string(\"config.toml\").await?)\n}\n\n// Or wrap blocking work in spawn_blocking\nasync fn rate_limit_wait() {\n tokio::time::sleep(Duration::from_secs(1)).await;\n}\n\n// For unavoidable blocking work\nasync fn query_db(conn: Arc) -> Result> {\n let conn = conn.clone();\n tokio::task::spawn_blocking(move || conn.query(\"SELECT ...\"))\n .await?\n}\n```\n\nDetection:\n\n```bash\n# Find blocking ops inside async functions (heuristic)\nrg -B 5 \"(std::fs::|std::thread::sleep|std::net::TcpStream)\" --type rust |\n rg -B 5 \"async fn\"\n```\n\n## Pattern 3: `tokio::spawn` for synchronous-equivalent work\n\n```rust\n// SLOP\nasync fn handle_request(req: Request) -> Response {\n let result = tokio::spawn(async move {\n compute_response(&req)\n }).await.unwrap();\n result\n}\n```\n\nSpawning a task only to immediately await its single\ncompletion is equivalent to a direct call, plus the\noverhead of task creation, scheduling, and a join. Just\ncall the function:\n\n```rust\nasync fn handle_request(req: Request) -> Response {\n compute_response(&req)\n}\n```\n\n`tokio::spawn` is for *concurrent* work: when the\nspawned task should make progress while the caller does\nsomething else, or when the task should outlive the\ncaller. A spawn-then-immediately-await is a smell.\n\n## Pattern 4: `async-trait` on synchronous-equivalent traits\n\n```rust\n// SLOP\n#[async_trait]\ntrait Greeter {\n async fn greet(&self, name: &str) -> String;\n}\n```\n\nIf the implementation has no `.await` and just returns a\nsynchronous value, `async-trait` adds heap allocation\n(`Box`) for nothing. Make the trait sync:\n\n```rust\ntrait Greeter {\n fn greet(&self, name: &str) -> String;\n}\n```\n\nUse `async-trait` only when at least one implementation\ngenuinely awaits.\n\n## Pattern 5: explicit `Pin>` returns\n\n```rust\n// SLOP\nfn fetch_data(url: &str) -> Pin> + Send>> {\n Box::pin(async move {\n // body\n })\n}\n```\n\nModern Rust supports `impl Future` in return position:\n\n```rust\n// Idiomatic\nfn fetch_data(url: &str) -> impl Future> + Send {\n async move {\n // body\n }\n}\n```\n\n`Pin>` is needed only for trait method\nreturns or when storing futures in collections.\n\n## Pattern 6: `MutexGuard` held across `.await`\n\n```rust\n// SLOP — deadlock risk\nasync fn update_count(state: &Arc>) {\n let mut guard = state.lock().unwrap();\n guard.count += 1;\n save_to_disk(&guard).await; // holds guard across await\n}\n```\n\nHolding a sync `Mutex` guard across `.await` blocks the\nruntime if any other task tries to acquire the same lock.\nFor async paths, use:\n\n- `tokio::sync::Mutex` (async-aware, can hold guards\n across `.await`).\n- Or restructure to drop the guard before awaiting:\n\n```rust\nasync fn update_count(state: &Arc>) {\n let snapshot = {\n let mut guard = state.lock().unwrap();\n guard.count += 1;\n guard.clone()\n }; // guard dropped here\n save_to_disk(&snapshot).await;\n}\n```\n\nThis is the GPT-5.x signature failure (Sonar measured\n~470 concurrency issues per MLOC for GPT-5.2 High); see\n`model-specific-tells.md`.\n\n## Pattern 7: re-implementing `select!` / `join!` manually\n\nIf you find yourself manually polling multiple futures\nwith `Pin::new` and `Poll`, you almost certainly want\n`tokio::select!` or `tokio::join!`. Hand-rolled polling\nis a strong signal that the model copied something it\nshould not have.\n\nDetection:\n\n```bash\nrg \"Pin::new\" --type rust -B 2 -A 5 | rg -B 2 \"fn poll\"\n```\n\n## Pattern 8: `Send + Sync` bounds added \"in case\"\n\n```rust\n// SLOP\nfn add(a: T, b: T) -> T { ... }\n```\n\nTrait bounds should be added because the function\n*needs* them, not as defensive over-spec. `Send`/`Sync`\non a function that runs synchronously, `Clone` on a\nfunction that doesn't clone, `Debug` on a function that\ndoesn't print: all noise.\n\nThe right rule: add the bound when the compiler complains\nwithout it. Remove the bound when removing it does not\ncause a compile error.\n\n## Detection commands\n\n```bash\n# Catch most async slop with clippy\ncargo clippy --all-targets -- \\\n -W clippy::async_yields_async \\\n -W clippy::large_futures \\\n -D warnings\n\n# Manual scans for the structural patterns\n# Pattern 1: file-level \"async fn but no .await\" — see Pattern 1\n# section above for the loop form.\nrg \"tokio::spawn.*\\.await\" --type rust # Pattern 3\nrg \"#\\[async_trait\\]\" --type rust # Pattern 4\nrg \"Pin>), 8 (defensive Send+Sync).\n\nPattern 6 is the highest-blast-radius async finding;\nescalate to `severity: high` and to a senior reviewer.\n\n## Integration\n\nAsync slop lands in Pass 5 of the multi-pass cleanup\nworkflow (`Skill(scribe:slop-detector)` module\n`cleanup-workflow.md`). For GPT-family-generated\ncodebases, weight pattern 6 detection most heavily;\nfor Claude-family codebases, weight pattern 1 (the\n\"behavior-preserving refactor leaves async fn that no\nlonger awaits anything\") most heavily. See\n`model-specific-tells.md`.\n\nFile v1.9.17:modules/builtin-preference.md\n\n---\nname: builtin-preference\ndescription: Detection of helper functions that should be standard trait\n implementations and reimplemented Rust builtins\ncategory: rust-review\ntags: [from, into, tryfrom, fromstr, default, display, iterator, idioms]\n---\n\n# Builtin Preference\n\nDetects custom helper functions that duplicate Rust's standard\ntrait system and built-in combinators.\n\n## What This Detects\n\nFour categories of anti-patterns:\n\n1. **Conversion helpers**: `parse_foo()`, `foo_from_bar()`,\n `convert_*()`, `to_*(&self)` that should be `FromStr`,\n `From`, `TryFrom`, or `Into` implementations\n2. **Standard trait replacements**: `default_config()`,\n `format_error()`, `as_bytes(&self)`, `compare()` that\n should be `Default`, `Display`, `AsRef`, or `PartialEq`\n3. **Error conversion wrappers**: `io_to_my_error()`,\n `wrap_error()` that should be `impl From` or\n thiserror `#[from]`\n4. **Manual combinators**: `match opt { Some(x) => Some(f(x)),\n None => None }` that should be `.map()`, `.unwrap_or()`,\n `.flatten()`, etc.\n\n## Why It Matters\n\nRust's trait system is compositional by design:\n\n- `impl From for B` gives `impl Into for A` for free\n- `impl Display` gives `ToString` for free\n- `From` enables the `?` operator for error propagation\n- Trait impls participate in generic bounds and blanket impls\n- Standard combinators are optimized and well-tested\n\nHelper functions that bypass this system create API\ninconsistency, miss ergonomic benefits, and signal\nunfamiliarity with idiomatic Rust.\n\n## Safe Patterns\n\n```rust\n// Good: From trait enables .into() and ? operator\nimpl From for Settings {\n fn from(c: Config) -> Self {\n Settings { timeout: c.timeout }\n }\n}\n\n// Good: FromStr enables .parse()\nimpl FromStr for Config {\n type Err = ConfigError;\n fn from_str(s: &str) -> Result { ... }\n}\n\n// Good: Default via derive\n#[derive(Default)]\nstruct Config { timeout: u64 }\n\n// Good: Display for human-readable output\nimpl fmt::Display for MyError {\n fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {\n write!(f, \"Error: {}\", self.msg)\n }\n}\n\n// Good: Option combinators\nlet result = opt.map(|x| x.to_string());\nlet value = opt.unwrap_or(default);\n```\n\n## Patterns to Flag\n\n```rust\n// Flag: should be impl FromStr\nfn parse_config(s: &str) -> Config { ... }\n\n// Flag: should be impl From for Foo\nfn foo_from_bar(b: Bar) -> Foo { ... }\n\n// Flag: should be impl Default\nfn default_config() -> Config { ... }\n\n// Flag: should be impl From for MyError\nfn io_to_my_error(e: io::Error) -> MyError { ... }\n\n// Flag: should use .map()\nmatch opt {\n Some(x) => Some(x.to_string()),\n None => None,\n}\n```\n\n## Exclusions (Not Flagged)\n\n- Lossy conversions (`to_lossy_ascii`)\n- Builder methods (`with_timeout(self, ...)`)\n- Multi-parameter conversions (context-dependent)\n- Domain-specific operations (`serialize`, `encode`, `decode`)\n\n## Related Clippy Lints\n\n| Lint | Detects |\n|------|---------|\n| `clippy::from_over_into` | `impl Into` where `impl From` suffices |\n| `clippy::manual_map` | Match on Option rewriting `.map()` |\n| `clippy::manual_unwrap_or` | Match rewriting `.unwrap_or()` |\n| `clippy::derivable_impls` | Manual Default that derive handles |\n| `clippy::manual_flatten` | Nested iteration rewriting `.flatten()` |\n| `clippy::new_without_default` | `fn new()` without `impl Default` |\n\n## Output Section\n\n```markdown\n## Builtin Preference\n### Issues Found\n- [file:line] Conversion helper `parse_config`: use `impl FromStr`\n- [file:line] Manual combinator: use `.map()` (clippy::manual_map)\n\n### Recommendations\n- Implement standard traits to gain ecosystem composability\n- Enable relevant clippy lints for automated enforcement\n```\n\nFile v1.9.17:modules/cargo-dependencies.md\n\n---\nname: cargo-dependencies\ndescription: Dependency auditing, security scanning, and version management\ncategory: rust-review\ntags: [cargo, dependencies, security, audit]\n---\n\n# Cargo Dependencies\n\nAudit and management of Cargo dependencies and build configuration.\n\n## Audit Commands\n\nRun detailed dependency analysis:\n```bash\ncargo tree -d # Find duplicates\ncargo audit # Security vulnerabilities\ncargo outdated # Stale versions\ncargo deny check # Policy enforcement\n```\n\n## Dependency Evaluation\n\nCheck:\n- Feature flags usage\n- Optional dependencies\n- Build scripts safety\n- Binary size impact\n- Compilation time\n\n## Security Scanning\n\nReview for:\n- Known vulnerabilities\n- Abandoned crates\n- Unmaintained dependencies\n- Security advisories\n- Supply chain risks\n\n## Version Management\n\nVerify:\n- Semver compliance\n- Version pinning strategy\n- Dependency updates frequency\n- Breaking change handling\n\n## Common Issues\n\nFlag:\n- Abandoned crates\n- Excessively large dependencies\n- Security-vulnerable versions\n- Duplicate dependencies\n- Unnecessary dependencies\n\n## Alternatives Suggestion\n\nRecommend alternatives for:\n- Unmaintained crates\n- Heavy dependencies\n- Vulnerable versions\n- Better maintained options\n\n## Output Section\n\n```markdown\n## Dependencies\n### Security Issues\n- [crate@version] Vulnerability: [CVE/advisory]\n\n### Recommendations\n- Update [crate] from X to Y\n- Replace [abandoned-crate] with [alternative]\n- Remove unused dependency: [crate]\n```\n\nFile v1.9.17:modules/cfg-test-misuse.md\n\n---\nname: cfg-test-misuse\ndescription: Detection of #[cfg(test)] applied to individual functions or\n impls outside a mod tests block, which is a common structural mistake\ncategory: rust-review\ntags: [testing, cfg, attributes, structure]\n---\n\n# cfg(test) Misuse\n\nAnalysis of `#[cfg(test)]` placement on individual items outside a\n`mod tests { ... }` block.\n\n## What This Detects\n\n`#[cfg(test)]` on a standalone `fn`, `impl`, or `struct` that is not nested\ninside a `mod tests` block.\n\n## Why It Matters\n\nThe idiomatic pattern is a single `mod tests` block gated with `#[cfg(test)]`,\nwhich keeps test code in one place.\nApplying `#[cfg(test)]` to an individual `impl` block is particularly\nhazardous: it removes method implementations from the production binary\nwithout an obvious compiler warning.\n\n## Safe Patterns\n\n```rust\n// Good: single gated mod tests block\n#[cfg(test)]\nmod tests {\n use super::*;\n\n fn helper() { ... }\n\n #[test]\n fn test_something() { ... }\n}\n```\n\n## Patterns to Flag\n\n```rust\n// Bad: cfg(test) on standalone function\n#[cfg(test)]\nfn setup_fixture() { ... }\n\n// Bad: cfg(test) on impl block outside mod tests\n#[cfg(test)]\nimpl MyStruct {\n fn test_helper(&self) { ... }\n}\n```\n\n## Output Section\n\n```markdown\n## cfg(test) Misuse\n### Issues Found\n- [file:line] cfg(test) outside mod tests: [explanation]\n\n### Recommendations\n- Move all test-only items inside a single `#[cfg(test)] mod tests` block\n```\n\nFile v1.9.17:modules/collection-types.md\n\n---\nname: collection-types\ndescription: Detection of Vec used where HashSet or HashMap semantics apply,\n including contains loops, find loops, and dedup patterns\ncategory: rust-review\ntags: [collections, performance, vec, hashset, hashmap]\n---\n\n# Collection Types\n\nAnalysis of `Vec` usage where a different collection type would be more\ncorrect or more efficient.\n\n## What This Detects\n\n- `vec.contains(&x)` -- O(n) membership test; `HashSet` gives O(1)\n- `vec.dedup()` -- sorting + dedup pattern suggests a set\n- `vec.iter().find(...)` / `vec.iter().position(...)` -- linear key lookup\n suggests `HashMap`\n\n## Why It Matters\n\nUsing `Vec` for set or map operations produces O(n) behaviour where O(1) is\navailable.\nIt also signals unclear intent: a `HashSet` communicates uniqueness, a\n`HashMap` communicates keyed access.\n\n## Safe Patterns\n\n```rust\n// Good: set membership\nlet mut seen: HashSet = HashSet::new();\nif seen.contains(&id) { ... }\n\n// Good: keyed lookup\nlet mut index: HashMap = HashMap::new();\nif let Some(user) = index.get(&id) { ... }\n```\n\n## Patterns to Flag\n\n```rust\n// Flag: O(n) membership on unbounded Vec\nusers.contains(&new_user)\n\n// Flag: dedup implies uniqueness invariant\nids.sort();\nids.dedup();\n\n// Flag: linear key scan\nusers.iter().find(|u| u.id == target_id)\n```\n\n## Output Section\n\n```markdown\n## Collection Types\n### Issues Found\n- [file:line] Vec used as set/map: [explanation]\n\n### Recommendations\n- Replace with HashSet for membership checks\n- Replace with HashMap for keyed access\n```\n\nFile v1.9.17:modules/concurrency-patterns.md\n\n---\nname: concurrency-patterns\ndescription: Concurrency cost hierarchy, synchronization primitives, async patterns, and performance-aware concurrency review\ncategory: rust-review\ntags: [concurrency, async, sync, deadlock, atomics, contention, performance]\n---\n\n# Concurrency Patterns\n\nAnalysis of concurrent and async code patterns in Rust,\ngrounded in the concurrency cost hierarchy.\n\n## Concurrency Cost Hierarchy\n\n\"Acquiring a mutex isn't slow; contention is slow.\"\nBefore reviewing concurrency code, classify each\nsynchronization point by its cost tier.\n\n| Level | Name | Approx Cost | Description |\n|-------|------|-------------|-------------|\n| 0 | Thread-local | ~2 ns | No atomics at all; per-thread state |\n| 1 | Uncontended atomics | ~10 ns | Atomic ops, no cross-core sharing |\n| 2 | Contended atomics | ~40-400 ns | Cache-line transfer between cores |\n| 3 | Syscalls | ~1 us | Kernel transitions on lock paths |\n| 4 | Context switches | ~10 us | Blocking locks, scheduler involvement |\n| 5 | Catastrophe | ~ms+ | Spinning on oversubscribed systems |\n| 6 | Kernel page fault | ~100-400 ms | Paged-out buffer re-faulted on access |\n\nLevels 3-5 are performance bugs. Level 6 is invisible in\ntask scheduler traces (tokio-console shows tasks scheduling\nin microseconds while the actual latency occurs in the\nkernel page fault handler). Target Level 2 as the default.\nAchieve Level 1 through contention reduction. Level 0\nrequires architectural redesign (per-thread computation\nwith periodic merges).\n\n**Level 6: kernel paging latency**: Long-lived Tokio\nruntimes that co-reside with large heap users (e.g., ML\nmodel weights) are at risk: the kernel may page out\nlatency-sensitive buffers during idle periods. When audio\nor ring-ring buffers page back in on the next access, the\npage fault adds 100–400 ms of p99 latency with no Tokio\ntrace signal.\n\nDetection: production-only latency spikes, no reproduction\non dev box, `perf stat` shows elevated `page-faults` on\nthe audio/buffer threads.\n\nFix: `libc::mlock` on the buffer pages. See\n`modules/unsafe-audit.md` for the full production checklist\n(RLIMIT_MEMLOCK, page alignment, ENOMEM fallback).\n\n**Key insight**: Performance is dominated by atomic\ninstruction count, not total instruction count. An\nalgorithm with 9x more total instructions but the\nsame number of atomics performs identically.\n\n### What to Flag in Review\n\n- **Level 5**: Fair spin-locks on thread pools larger\n than core count. Always flag.\n- **Level 4**: `std::sync::Condvar` wake patterns that\n convoy. Flag when hot path.\n- **Level 3**: `sched_yield()` or `thread::yield_now()`\n in lock loops. Suggest backoff or parking.\n- **Level 2 (avoidable)**: Atomic RMW on shared counter\n when per-thread counters and merge would suffice.\n- **False sharing**: Independent atomics on the same\n cache line (64 bytes). Suggest `#[repr(align(64))]`\n or `crossbeam_utils::CachePadded`.\n\n## Synchronization Primitives\n\nReview primitives usage:\n\n- `Arc`, `Mutex`, `RwLock`\n- `Atomic*` types and ordering (`Relaxed` vs `SeqCst`)\n- `tokio::sync` (mpsc, broadcast, watch, Semaphore)\n- `Send`/`Sync` bounds\n- `parking_lot` vs `std::sync` trade-offs\n\n### Memory Ordering Review\n\nCheck ordering is neither too weak nor too strong:\n\n- `Relaxed`: Counters, statistics (no cross-variable\n ordering needed)\n- `Acquire`/`Release`: Publish/consume patterns,\n one-shot flags\n- `SeqCst`: Only when total order across multiple\n atomics is required (rare; flag overuse)\n\n## Async Patterns\n\nCheck async code:\n\n- No blocking in async functions\n- Proper `spawn_blocking` usage\n- Guards dropped before awaiting\n- Cancellation safety\n- Task spawning patterns\n\n## Best Practices\n\n```rust\n// Good: Drop guard before await\nasync fn update(data: Arc>) {\n let value = {\n let guard = data.lock().await;\n guard.value.clone()\n }; // Guard dropped\n process(value).await;\n}\n\n// Good: Cache-padded to prevent false sharing\nuse crossbeam_utils::CachePadded;\n\nstruct Counters {\n reads: CachePadded,\n writes: CachePadded,\n}\n```\n\n## Contention Reduction Patterns\n\nWhen review finds Level 2+ contention on hot paths:\n\n1. **Shard the lock**: `DashMap`, `ShardedLock`, or\n manual sharding by key hash\n2. **Per-thread accumulation**: Thread-local counters\n merged at read time (Level 2 to Level 0)\n3. **Read-copy-update (RCU)**: `arc-swap` for\n read-heavy, write-rare data\n4. **Lock-free structures**: `crossbeam` queues and\n deques when contention dominates\n\n## Deadlock Prevention\n\nIdentify potential deadlocks:\n\n- Lock ordering consistency\n- Nested locks\n- Await points while holding locks\n- Circular dependencies\n\n## Data Race Detection\n\nCheck for:\n\n- `static mut` misuse\n- Shared mutable state\n- Missing synchronization\n- Race conditions\n\n## Send/Sync Bounds\n\nVerify:\n\n- Proper trait bounds\n- Thread safety guarantees\n- Cross-thread data transfer\n- Closure captures\n\n## Common Issues\n\n- Blocking in async context\n- Guards held across await points\n- Inconsistent lock ordering\n- Missing bounds on generics\n- Unsafe Send/Sync implementations\n- `SeqCst` used everywhere (usually `Acquire`/`Release`\n suffices; `SeqCst` adds unnecessary fence cost)\n- Spinning without backoff on oversubscribed systems\n- False sharing between independent atomics\n\n## Output Section\n\n```markdown\n## Concurrency\n### Cost Classification\n- [file:line] Level N: [primitive] - [justification]\n\n### Issues Found\n- [file:line] Guard held across await: [details]\n- [file:line] Potential deadlock: [scenario]\n- [file:line] False sharing risk: [layout details]\n- [file:line] Unnecessary SeqCst: [suggest weaker ordering]\n\n### Recommendations\n- [concurrency improvements with cost tier impact]\n```\n\n## References\n\n- Jon Gjengset, \"The Cost of Concurrency Coordination\"\n (video: youtube.com/watch?v=tND-wBBZ8RY)\n- Travis Downs, \"A Concurrency Cost Hierarchy\"\n (travisdowns.github.io/blog/2020/07/06/concurrency-costs.html)\n- Mara Bos, \"Rust Atomics and Locks\" (O'Reilly)\n\nFile v1.9.17:modules/duplicate-validators.md\n\n---\nname: duplicate-validators\ndescription: Detection of multiple validate_*, check_*, or verify_* functions\n that share similar structure and could be consolidated\ncategory: rust-review\ntags: [design, duplication, validation, refactoring]\n---\n\n# Duplicate Validators\n\nAnalysis of `validate_*`, `check_*`, and `verify_*` functions for\nopportunities to consolidate repeated validation logic.\n\n## What This Detects\n\nThree or more functions sharing the same verb prefix (`validate_`, `check_`,\n`verify_`) within a single file, which often indicates copy-pasted validation\nlogic that could be unified.\n\n## Why It Matters\n\nDuplicated validation logic diverges over time: one copy gets a bug fix or a\nnew rule while the others do not.\nConsolidating into a generic validator ensures all callers benefit from each\nfix.\n\n## Safe Patterns\n\n```rust\n// Good: single generic validator with rule injection\nfn validate_field(value: &str, rules: &[ValidationRule])\n -> Result<(), ValidationError>\n{\n for rule in rules {\n rule.apply(value)?;\n }\n Ok(())\n}\n```\n\n## Patterns to Flag\n\n```rust\n// Flag when 3+ share the same prefix:\nfn validate_email(s: &str) -> bool { ... }\nfn validate_phone(s: &str) -> bool { ... }\nfn validate_username(s: &str) -> bool { ... }\nfn validate_password(s: &str) -> bool { ... }\n```\n\n## Output Section\n\n```markdown\n## Duplicate Validators\n### Issues Found\n- [file] 4 validate_* functions: [list]\n\n### Recommendations\n- Extract shared logic into a generic validator\n- Use a trait or rule-set parameter to unify related checks\n```\n\nFile v1.9.17:modules/error-handling.md\n\n---\nname: error-handling\ndescription: Result/Option patterns, custom error types, and error propagation analysis\ncategory: rust-review\ntags: [errors, result, option, propagation]\n---\n\n# Error Handling\n\nAnalysis of error handling patterns and correctness in Rust code.\n\n## Result and Option Usage\n\nEvaluate:\n- `Result` and `Option` usage patterns\n- Custom error types design\n- Context addition with `anyhow` or `thiserror`\n- `?` propagation correctness\n\n## Error Type Design\n\nCheck custom error types:\n- Implements `std::error::Error`\n- Provides meaningful context\n- Conversion traits (`From`, `Into`)\n- Error hierarchy structure\n\n## Error Propagation\n\nBest practices:\n```rust\n// Good: Proper error propagation\nfn process() -> Result<(), ProcessError> {\n let data = fetch().context(\"failed to fetch\")?;\n validate(&data)?;\n Ok(())\n}\n```\n\n## Common Issues to Flag\n\n- Panics in library code (`unwrap`, `expect`)\n- Logging side-effects in error paths\n- Mismatched error hierarchies\n- Missing retry/backoff logic\n- Silent error swallowing\n- Over-generic error types\n\n## Error Context\n\nVerify context is added:\n- Operation context\n- Input data context\n- Failure reasons\n- Recovery suggestions\n\n## Output Section\n\n```markdown\n## Error Handling\n### Issues Found\n- [file:line] Panic in library: [details]\n- [file:line] Missing context: [suggestion]\n\n### Recommendations\n- [error handling improvements]\n```\n\nFile v1.9.17:modules/error-messages.md\n\n---\nname: error-messages\ndescription: Detection of short error strings (under ~20 chars) in\n Err(), panic!(), and expect() that lack context or recovery hints\ncategory: rust-review\ntags: [error-handling, diagnostics, messages, quality]\n---\n\n# Error Messages\n\nAnalysis of error and panic messages for actionability.\nShort messages without context make production incidents harder to diagnose.\n\n## What This Detects\n\nString literals under roughly 20 characters used in:\n\n- `Err(\"short msg\")`\n- `panic!(\"short msg\")`\n- `.expect(\"short msg\")`\n- `Err(\"short msg\".to_string())`\n\n## Why It Matters\n\nA message like `\"not found\"` or `\"failed\"` gives an on-call engineer no\ninformation about what was not found, where the failure occurred, or how to\nrecover.\n\n## Safe Patterns\n\n```rust\n// Good: identifies operation and input\n.expect(\"failed to open config file at $CONFIG_PATH\")\n\n// Good: Err with context\nreturn Err(format!(\n \"user {} not found in tenant {}\",\n user_id, tenant_id\n));\n```\n\n## Patterns to Flag\n\n```rust\n// Bad: no context\n.expect(\"failed\")\n.expect(\"not found\")\n\n// Bad: Err with bare short string\nreturn Err(\"bad input\");\nreturn Err(\"denied\".to_string());\n```\n\n## Output Section\n\n```markdown\n## Error Messages\n### Issues Found\n- [file:line] Short error message: [explanation]\n\n### Recommendations\n- Add operation context: what were you trying to do?\n- Add input context: what value triggered the failure?\n- Add recovery hints where possible\n```\n\nFile v1.9.17:modules/iterator-and-allocation-slop.md\n\n---\nmodule: iterator-and-allocation-slop\ncategory: detection\ndependencies: [Read, Grep]\nestimated_tokens: 600\n---\n\n# Iterator and Allocation Slop\n\n**AI-generated Rust defaults to manual loops where\niterators read better, and to allocation where references\nsuffice. Both compile; both are slop.**\n\nThis module covers two of the highest-frequency AI Rust\nanti-patterns: imperative loops the iterator API expresses\nin one line, and unnecessary allocation that\nborrow-checker capitulation produces. The clippy lints\ncatch most of this; the rest is judgment.\n\n## Iterator slop\n\n### Pattern 1: index-based loops\n\n```rust\n// SLOP\nlet mut sum = 0;\nfor i in 0..vec.len() {\n sum += vec[i];\n}\n\n// Idiomatic\nlet sum: i32 = vec.iter().sum();\n```\n\nDetector: `clippy::needless_range_loop`.\n\n### Pattern 2: filter-then-push\n\n```rust\n// SLOP\nlet mut result = Vec::new();\nfor x in xs.iter() {\n if x.is_active() {\n result.push(x.id);\n }\n}\n\n// Idiomatic\nlet result: Vec<_> = xs.iter()\n .filter(|x| x.is_active())\n .map(|x| x.id)\n .collect();\n```\n\n### Pattern 3: map-filter-unwrap\n\n```rust\n// SLOP\nlet firsts: Vec<_> = xs.iter()\n .map(|x| x.first())\n .filter(|x| x.is_some())\n .map(|x| x.unwrap())\n .collect();\n\n// Idiomatic\nlet firsts: Vec<_> = xs.iter()\n .filter_map(|x| x.first())\n .collect();\n```\n\nDetector: `clippy::manual_filter_map`.\n\n### Pattern 4: collect-then-iterate\n\n```rust\n// SLOP\nlet intermediate: Vec<_> = xs.iter().map(transform).collect();\nfor item in intermediate {\n use_it(item);\n}\n\n// Idiomatic\nfor item in xs.iter().map(transform) {\n use_it(item);\n}\n```\n\nDetector: `clippy::needless_collect`.\n\n### Pattern 5: bool-match where if suffices\n\n```rust\n// SLOP\nmatch flag {\n true => do_a(),\n false => do_b(),\n}\n\n// Idiomatic\nif flag { do_a() } else { do_b() }\n```\n\nDetector: `clippy::match_bool`.\n\n## Allocation slop\n\n### Pattern A: `.clone()` to satisfy the borrow checker\n\nThe single most common AI-generated Rust anti-pattern. The\n`rust-unofficial/patterns` book lists it as the canonical\nanti-pattern: cloning to make a borrow-checker error go\naway rather than to express ownership.\n\n```rust\n// SLOP\nfn greet(name: String) {\n println!(\"Hello, {name}\");\n}\nfn main() {\n let n = String::from(\"world\");\n greet(n.clone()); // unnecessary clone\n greet(n.clone());\n}\n\n// Idiomatic\nfn greet(name: &str) {\n println!(\"Hello, {name}\");\n}\nfn main() {\n let n = String::from(\"world\");\n greet(&n);\n greet(&n);\n}\n```\n\nDetection heuristic: any `.clone()` on a `String`,\n`Vec<_>`, `HashMap<_,_>`, `Arc>`, or large\nstruct that is *not* paired with a comment explaining\nthe ownership rationale. If it disappeared, would the\nborrow checker complain? If yes, the right fix is\nusually to take a borrowed reference (`&str`, `&[T]`,\n`&T`).\n\nDetectors: `clippy::redundant_clone`,\n`clippy::clone_on_ref_ptr`.\n\n### Pattern B: owned parameters that should borrow\n\n| Slop signature | Idiomatic signature |\n|---|---|\n| `fn f(s: &String)` | `fn f(s: &str)` |\n| `fn f(v: &Vec)` | `fn f(v: &[T])` |\n| `fn f(s: String)` (read-only) | `fn f(s: &str)` |\n| `fn f(v: Vec)` (read-only) | `fn f(v: &[T])` |\n| `fn f(b: Box)` (no boxing reason) | `fn f(t: T)` |\n\nDetector: `clippy::ptr_arg` catches `&Vec` and `&String`.\n\n### Pattern C: `format!` then convert\n\n```rust\n// SLOP\nlet s = format!(\"{}\", x);\n\n// Idiomatic (when Display is implemented)\nlet s = x.to_string();\n\n// Inverse SLOP\nlet s = x.to_string();\nlet s = format!(\"{s}{rest}\");\n\n// Idiomatic (build the string once)\nlet s = format!(\"{x}{rest}\");\n```\n\nDetectors: `clippy::useless_format`, `clippy::str_to_string`.\n\n### Pattern D: redundant allocation\n\n```rust\n// SLOP\nlet owned = borrowed.to_owned();\nfn take_str(s: &str) { ... }\ntake_str(&owned);\n\n// Idiomatic — borrow directly\ntake_str(borrowed);\n\n// SLOP\nlet s = String::new();\nlet s = s + \"hello\" + \" \" + \"world\";\n\n// Idiomatic\nlet s = String::from(\"hello world\");\n```\n\n### Pattern E: `Box::new(...)` without indirection reason\n\n```rust\n// SLOP\nlet x = Box::new(42_u64);\nfn use_it(n: u64) { ... }\nuse_it(*x);\n\n// Idiomatic\nlet x = 42_u64;\nuse_it(x);\n```\n\nHeap allocation is justified for:\n- `dyn Trait` objects (`Box`, `Box`).\n- Recursive types (`Box`).\n- Large stack-frame avoidance (uncommon; measure first).\n- Pinning requirements (`Pin>`).\n\nAnywhere else, `Box::new` is unjustified allocation.\n\n### Pattern E2: `Box` or `&dyn Trait` in a hot inner loop\n\nThis is distinct from Pattern E. The box itself may be\njustified: the problem is calling a `dyn` method millions of\ntimes when the method body is tiny.\n\n```rust\n// Potentially slow: dyn dispatch in the inner loop\nlet decoders: Vec> = build_decoders(&schema, &batch);\n\nfor i in 0..n_rows {\n for d in &decoders {\n d.write_to_row(i, &mut row); // indirect call every iteration\n }\n}\n```\n\n**Why it matters**: each `dyn` call goes through a vtable\n(`call *0x18(%rax)`). The compiler cannot inline across that\nboundary, so it cannot fuse the inner loop, vectorize small\nstores, or eliminate the function-call prologue/epilogue overhead.\nWhen the method body is ~25 instructions (a null check, a bit\nflip, a 4-byte move), the prologue/epilogue and indirect\njump overhead can represent 40–50% of total runtime.\n\nNote: `&dyn Trait` has the same problem as `Box`.\nThe issue is dynamic dispatch, not heap allocation.\n\n**Fix 1: flip the loop order (batch-first)**: iterate\nall rows for each decoder, not all decoders for each row.\nThe dyn dispatch cost is paid once per decoder per batch\ninstead of once per cell:\n\n```rust\nlet mut rows: Vec = (0..n_rows)\n .map(|i| WriteRow::new(&segment, key_array.value(i)))\n .collect();\n\nfor d in &decoders {\n d.write_to_rows(0, &mut rows[..]); // dispatch once per decoder\n}\n```\n\n**Fix 2: enum dispatch**: replace `dyn Trait` with a\nclosed enum. The compiler can monomorphize and inline each\nvariant:\n\n```rust\nenum ColDecoder {\n F32(F32Decoder),\n Utf8(Utf8Decoder),\n Bool(BoolDecoder),\n}\n\nimpl ColDecoder {\n #[inline(always)]\n fn write_to_row(&self, index: usize, row: &mut WriteRow) {\n match self {\n ColDecoder::F32(d) => d.write_to_row(index, row),\n ColDecoder::Utf8(d) => d.write_to_row(index, row),\n ColDecoder::Bool(d) => d.write_to_row(index, row),\n }\n }\n}\n```\n\n**When to flag**: any `Vec>` or `Vec<&dyn Trait>`\niterated inside an inner loop where the method body is\ninlineable. The Java/JVM analogy is instructive: the JVM\nde-virtualizes and inlines at JIT time; rustc cannot cross\nthe `dyn` boundary. If reviewers come from JVM backgrounds,\nthis is the most important Rust performance lesson to surface.\n\nDetection:\n\n```bash\n# Find Vec> that appear inside nested loops\nrg \"Vec = vec![2, 3, 5, 7, 11];\n\n// Idiomatic for small, fixed-size\nlet primes: [u32; 5] = [2, 3, 5, 7, 11];\n\n// Or for stack-allocated growable\nlet primes: SmallVec<[u32; 5]> = smallvec![2, 3, 5, 7, 11];\n```\n\nThis one is judgment: arrays for compile-time-known\nsizes, `SmallVec`/`ArrayVec` for \"usually small but\nsometimes grows\", `Vec` for genuinely dynamic.\n\n## Detection commands\n\n```bash\n# Catch most iterator slop with clippy\ncargo clippy --all-targets -- \\\n -W clippy::needless_range_loop \\\n -W clippy::manual_filter_map \\\n -W clippy::needless_collect \\\n -W clippy::filter_map_next \\\n -W clippy::map_unwrap_or \\\n -W clippy::match_bool \\\n -W clippy::needless_match \\\n -D warnings\n\n# Catch most allocation slop with clippy\ncargo clippy --all-targets -- \\\n -W clippy::redundant_clone \\\n -W clippy::clone_on_ref_ptr \\\n -W clippy::ptr_arg \\\n -W clippy::useless_format \\\n -W clippy::str_to_string \\\n -W clippy::redundant_allocation \\\n -D warnings\n\n# Manual scan: every .clone() in the codebase\nrg \"\\.clone\\(\\)\" --type rust -n | head -50\n# For each: ask \"would the borrow checker complain if removed?\"\n```\n\n## False positives\n\nSome `.clone()` calls are correct and should stay:\n\n- **Async / spawn boundaries**: cloning an `Arc` to\n send into `tokio::spawn` is required, not slop.\n- **Genuine ownership transfer**: when two callers each\n need to mutate independently from the same source.\n- **Intentional defensive copy**: in security-sensitive\n paths where the original might be modified by an\n attacker. Mark with `// COPY: defense against ...`.\n\nWhen in doubt, comment the rationale next to the\n`.clone()`. A `.clone()` with a one-line \"why\" comment\nis documented intent; a `.clone()` with no comment is\nslop.\n\n## Output format\n\nFor each finding, use the\n`Skill(scribe:slop-detector)` module\n`structured-finding-output.md` format. Severity is\n`medium` for individual iterator/allocation slop;\n`high` if the same pattern appears 5+ times in the same\nfile (suggests systematic AI-generation rather than\nisolated mistake).\n\n## Integration\n\nIterator and allocation slop typically lands in Pass 5\n(code idiom sweep) of the multi-pass cleanup workflow\n(see `Skill(scribe:slop-detector)` module\n`cleanup-workflow.md`). Run after the linter floor\n(Pass 1) clears, since clippy will flag most of these\nautomatically.\n\nArchive v1.9.16: 19 files, 33462 bytes\n\nFiles: modules/async-slop.md (7937b), modules/builtin-preference.md (3751b), modules/cargo-dependencies.md (1516b), modules/cfg-test-misuse.md (1438b), modules/collection-types.md (1539b), modules/concurrency-patterns.md (5998b), modules/duplicate-validators.md (1559b), modules/error-handling.md (1405b), modules/error-messages.md (1459b), modules/iterator-and-allocation-slop.md (9416b), modules/model-specific-tells.md (7032b), modules/ownership-analysis.md (1469b), modules/silent-returns.md (1678b), modules/sql-injection.md (1520b), modules/test-slop.md (8018b), modules/unsafe-audit.md (3876b), skill-card.md (2081b), SKILL.md (4549b), _meta.json (142b)\n\nFile v1.9.16:SKILL.md\n\n---\nname: rust-review\ndescription: Audits Rust code for unsafe blocks, ownership issues, and Cargo dependency risks\nversion: 1.9.8\ntriggers:\n - rust\n - ownership\n - concurrency\n - unsafe\n - traits\n - cargo\n - reviewing Rust code or before merging Rust changes\nmetadata: {\"openclaw\": {\"homepage\": \"https://github.com/athola/claude-night-market/tree/master/plugins/pensive\", \"emoji\": \"\\ud83d\\udd0d\", \"requires\": {\"config\": [\"night-market.pensive:shared\", \"night-market.imbue:proof-of-work\"]}}}\nsource: claude-night-market\nsource_plugin: pensive\n---\n\n> **Night Market Skill** — ported from [claude-night-market/pensive](https://github.com/athola/claude-night-market/tree/master/plugins/pensive). For the full experience with agents, hooks, and commands, install the Claude Code plugin.\n\n\n## Table of Contents\n\n- [Quick Start](#quick-start)\n- [When to Use](#when-to-use)\n- [Required TodoWrite Items](#required-todowrite-items)\n- [Progressive Loading](#progressive-loading)\n- [Core Workflow](#core-workflow)\n- [Rust Quality Checklist](#rust-quality-checklist)\n- [Safety](#safety)\n- [Correctness](#correctness)\n- [Performance](#performance)\n- [Idioms](#idioms)\n- [Output Format](#output-format)\n- [Summary](#summary)\n- [Ownership Analysis](#ownership-analysis)\n- [Error Handling](#error-handling)\n- [Concurrency](#concurrency)\n- [Unsafe Audit](#unsafe-audit)\n- [[U1] file:line](#[u1]-file:line)\n- [Dependencies](#dependencies)\n- [Recommendation](#recommendation)\n- [Exit Criteria](#exit-criteria)\n\n\n# Rust Review Workflow\n\nExpert-level Rust code audits with focus on safety, correctness, and idiomatic patterns.\n\n## Quick Start\n\n```bash\n/rust-review\n```\n**Verification:** Run the command with `--help` flag to verify availability.\n\n## When To Use\n\n- Reviewing Rust code changes\n- Auditing unsafe blocks\n- Analyzing concurrency patterns\n- Dependency security review\n- Performance optimization review\n\n## When NOT To Use\n\n- General code review without Rust - use unified-review\n- Performance profiling - use parseltongue:python-performance pattern\n\n## Required TodoWrite Items\n\n1. `rust-review:ownership-analysis`\n2. `rust-review:error-handling`\n3. `rust-review:concurrency`\n4. `rust-review:unsafe-audit`\n5. `rust-review:cargo-deps`\n6. `rust-review:evidence-log`\n\n## Progressive Loading\n\nLoad modules as needed based on review scope:\n\n**Quick Review** (ownership and errors):\n- See `modules/ownership-analysis.md` for borrowing and lifetime analysis\n- See `modules/error-handling.md` for Result/Option patterns\n\n**Concurrency Focus**:\n- See `modules/concurrency-patterns.md` for async and sync primitives\n\n**Safety Audit**:\n- See `modules/unsafe-audit.md` for unsafe block documentation\n\n**Dependency Review**:\n- See `modules/cargo-dependencies.md` for vulnerability scanning\n\n**Idiomatic Patterns**:\n- See `modules/builtin-preference.md` for conversion traits and builtin preference\n\n## Core Workflow\n\n1. **Ownership Analysis**: Check borrowing, lifetimes, clone patterns\n2. **Error Handling**: Verify Result/Option usage, propagation\n3. **Concurrency**: Review async patterns, sync primitives\n4. **Unsafe Audit**: Document invariants, FFI contracts\n5. **Dependencies**: Scan for vulnerabilities, updates\n6. **Evidence Log**: Record commands and findings\n\n## Rust Quality Checklist\n\n### Safety\n- [ ] All unsafe blocks documented with SAFETY comments\n- [ ] FFI boundaries properly wrapped\n- [ ] Memory safety invariants maintained\n- [ ] `mlock`/`munlock` calls: RLIMIT verified, page-aligned,\n ENOMEM handled\n\n### Correctness\n- [ ] Error handling complete\n- [ ] Concurrency patterns sound\n- [ ] Tests cover critical paths\n\n### Performance\n- [ ] No unnecessary allocations\n- [ ] Borrowing preferred over cloning\n- [ ] Async properly non-blocking\n\n### Idioms\n- [ ] Standard traits implemented\n- [ ] Conversion traits preferred over helper functions\n- [ ] Error types well-designed\n- [ ] Documentation complete\n\n## Output Format\n\n```markdown\n## Summary\nRust audit findings\n\n## Ownership Analysis\n[borrowing and lifetime issues]\n\n## Error Handling\n[error patterns and issues]\n\n## Concurrency\n[async and sync patterns]\n\n## Unsafe Audit\n### [U1] file:line\n- Invariants: [documented]\n- Risk: [assessment]\n- Recommendation: [action]\n\n## Dependencies\n[cargo audit results]\n\n## Recommendation\nApprove / Approve with actions / Block\n```\n**Verification:** Run the command with `--help` flag to verify availability.\n\n## Exit Criteria\n\n- All unsafe blocks audited\n- Concurrency patterns verified\n- Dependencies scanned\n- Evidence logged\n- Action items assigned\n\nFile v1.9.16:_meta.json\n\n{\n \"ownerId\": \"kn7d107jg9jv602h9ytsegydq184a42s\",\n \"slug\": \"nm-pensive-rust-review\",\n \"version\": \"1.9.16\",\n \"publishedAt\": 1784058970799\n}\n\nFile v1.9.16:modules/async-slop.md\n\n---\nmodule: async-slop\ncategory: detection\ndependencies: [Read, Grep]\nestimated_tokens: 500\n---\n\n# Async Slop\n\n**AI defaults to `async` and `tokio::spawn` even where\nsync code is faster, simpler, and correct.**\n\nThis module covers the high-frequency async patterns that\nlook idiomatic but are not. The clippy lints catch some;\nthe rest is structural.\n\n## Pattern 1: `async fn` that contains no `.await`\n\n```rust\n// SLOP\nasync fn compute_total(items: &[Item]) -> u64 {\n items.iter().map(|i| i.price).sum()\n}\n```\n\nIf the function body has no `.await`, it has no reason\nto be `async`. Async coloring is contagious: this\nfunction is callable only from async contexts, forcing\nevery caller to also be `async`. Strip `async` from the\nsignature unless the body actually awaits.\n\nDetection (preferred: clippy):\n\n```bash\ncargo clippy --all-targets -- -W clippy::async_yields_async\n```\n\nFile-level heuristic when clippy is unavailable:\n\n```bash\nfor f in $(rg -l \"async fn \" --type rust); do\n rg -q \"\\.await\" \"$f\" || echo \"no-await: $f\"\ndone\n```\n\n(Heuristic; manual review needed since `.await` may be in\na helper called by the async fn rather than inline.)\n\n## Pattern 2: blocking I/O inside an async runtime\n\n```rust\n// SLOP\nasync fn read_config() -> Result {\n Ok(std::fs::read_to_string(\"config.toml\")?)\n}\n\n// SLOP\nasync fn rate_limit_wait() {\n std::thread::sleep(Duration::from_secs(1)); // blocks the runtime\n}\n\n// SLOP\nasync fn query_db(conn: &Connection) -> Result> {\n conn.query(\"SELECT ...\")? // blocking driver\n}\n```\n\nBlocking calls inside `async` block the entire executor\nthread, defeating the runtime's concurrency model.\n\nFix:\n\n```rust\n// Use the async equivalent\nasync fn read_config() -> Result {\n Ok(tokio::fs::read_to_string(\"config.toml\").await?)\n}\n\n// Or wrap blocking work in spawn_blocking\nasync fn rate_limit_wait() {\n tokio::time::sleep(Duration::from_secs(1)).await;\n}\n\n// For unavoidable blocking work\nasync fn query_db(conn: Arc) -> Result> {\n let conn = conn.clone();\n tokio::task::spawn_blocking(move || conn.query(\"SELECT ...\"))\n .await?\n}\n```\n\nDetection:\n\n```bash\n# Find blocking ops inside async functions (heuristic)\nrg -B 5 \"(std::fs::|std::thread::sleep|std::net::TcpStream)\" --type rust |\n rg -B 5 \"async fn\"\n```\n\n## Pattern 3: `tokio::spawn` for synchronous-equivalent work\n\n```rust\n// SLOP\nasync fn handle_request(req: Request) -> Response {\n let result = tokio::spawn(async move {\n compute_response(&req)\n }).await.unwrap();\n result\n}\n```\n\nSpawning a task only to immediately await its single\ncompletion is equivalent to a direct call, plus the\noverhead of task creation, scheduling, and a join. Just\ncall the function:\n\n```rust\nasync fn handle_request(req: Request) -> Response {\n compute_response(&req)\n}\n```\n\n`tokio::spawn` is for *concurrent* work: when the\nspawned task should make progress while the caller does\nsomething else, or when the task should outlive the\ncaller. A spawn-then-immediately-await is a smell.\n\n## Pattern 4: `async-trait` on synchronous-equivalent traits\n\n```rust\n// SLOP\n#[async_trait]\ntrait Greeter {\n async fn greet(&self, name: &str) -> String;\n}\n```\n\nIf the implementation has no `.await` and just returns a\nsynchronous value, `async-trait` adds heap allocation\n(`Box`) for nothing. Make the trait sync:\n\n```rust\ntrait Greeter {\n fn greet(&self, name: &str) -> String;\n}\n```\n\nUse `async-trait` only when at least one implementation\ngenuinely awaits.\n\n## Pattern 5: explicit `Pin>` returns\n\n```rust\n// SLOP\nfn fetch_data(url: &str) -> Pin> + Send>> {\n Box::pin(async move {\n // body\n })\n}\n```\n\nModern Rust supports `impl Future` in return position:\n\n```rust\n// Idiomatic\nfn fetch_data(url: &str) -> impl Future> + Send {\n async move {\n // body\n }\n}\n```\n\n`Pin>` is needed only for trait method\nreturns or when storing futures in collections.\n\n## Pattern 6: `MutexGuard` held across `.await`\n\n```rust\n// SLOP — deadlock risk\nasync fn update_count(state: &Arc>) {\n let mut guard = state.lock().unwrap();\n guard.count += 1;\n save_to_disk(&guard).await; // holds guard across await\n}\n```\n\nHolding a sync `Mutex` guard across `.await` blocks the\nruntime if any other task tries to acquire the same lock.\nFor async paths, use:\n\n- `tokio::sync::Mutex` (async-aware, can hold guards\n across `.await`).\n- Or restructure to drop the guard before awaiting:\n\n```rust\nasync fn update_count(state: &Arc>) {\n let snapshot = {\n let mut guard = state.lock().unwrap();\n guard.count += 1;\n guard.clone()\n }; // guard dropped here\n save_to_disk(&snapshot).await;\n}\n```\n\nThis is the GPT-5.x signature failure (Sonar measured\n~470 concurrency issues per MLOC for GPT-5.2 High); see\n`model-specific-tells.md`.\n\n## Pattern 7: re-implementing `select!` / `join!` manually\n\nIf you find yourself manually polling multiple futures\nwith `Pin::new` and `Poll`, you almost certainly want\n`tokio::select!` or `tokio::join!`. Hand-rolled polling\nis a strong signal that the model copied something it\nshould not have.\n\nDetection:\n\n```bash\nrg \"Pin::new\" --type rust -B 2 -A 5 | rg -B 2 \"fn poll\"\n```\n\n## Pattern 8: `Send + Sync` bounds added \"in case\"\n\n```rust\n// SLOP\nfn add(a: T, b: T) -> T { ... }\n```\n\nTrait bounds should be added because the function\n*needs* them, not as defensive over-spec. `Send`/`Sync`\non a function that runs synchronously, `Clone` on a\nfunction that doesn't clone, `Debug` on a function that\ndoesn't print: all noise.\n\nThe right rule: add the bound when the compiler complains\nwithout it. Remove the bound when removing it does not\ncause a compile error.\n\n## Detection commands\n\n```bash\n# Catch most async slop with clippy\ncargo clippy --all-targets -- \\\n -W clippy::async_yields_async \\\n -W clippy::large_futures \\\n -D warnings\n\n# Manual scans for the structural patterns\n# Pattern 1: file-level \"async fn but no .await\" — see Pattern 1\n# section above for the loop form.\nrg \"tokio::spawn.*\\.await\" --type rust # Pattern 3\nrg \"#\\[async_trait\\]\" --type rust # Pattern 4\nrg \"Pin>), 8 (defensive Send+Sync).\n\nPattern 6 is the highest-blast-radius async finding;\nescalate to `severity: high` and to a senior reviewer.\n\n## Integration\n\nAsync slop lands in Pass 5 of the multi-pass cleanup\nworkflow (`Skill(scribe:slop-detector)` module\n`cleanup-workflow.md`). For GPT-family-generated\ncodebases, weight pattern 6 detection most heavily;\nfor Claude-family codebases, weight pattern 1 (the\n\"behavior-preserving refactor leaves async fn that no\nlonger awaits anything\") most heavily. See\n`model-specific-tells.md`.\n\nFile v1.9.16:modules/builtin-preference.md\n\n---\nname: builtin-preference\ndescription: Detection of helper functions that should be standard trait\n implementations and reimplemented Rust builtins\ncategory: rust-review\ntags: [from, into, tryfrom, fromstr, default, display, iterator, idioms]\n---\n\n# Builtin Preference\n\nDetects custom helper functions that duplicate Rust's standard\ntrait system and built-in combinators.\n\n## What This Detects\n\nFour categories of anti-patterns:\n\n1. **Conversion helpers**: `parse_foo()`, `foo_from_bar()`,\n `convert_*()`, `to_*(&self)` that should be `FromStr`,\n `From`, `TryFrom`, or `Into` implementations\n2. **Standard trait replacements**: `default_config()`,\n `format_error()`, `as_bytes(&self)`, `compare()` that\n should be `Default`, `Display`, `AsRef`, or `PartialEq`\n3. **Error conversion wrappers**: `io_to_my_error()`,\n `wrap_error()` that should be `impl From` or\n thiserror `#[from]`\n4. **Manual combinators**: `match opt { Some(x) => Some(f(x)),\n None => None }` that should be `.map()`, `.unwrap_or()`,\n `.flatten()`, etc.\n\n## Why It Matters\n\nRust's trait system is compositional by design:\n\n- `impl From for B` gives `impl Into for A` for free\n- `impl Display` gives `ToString` for free\n- `From` enables the `?` operator for error propagation\n- Trait impls participate in generic bounds and blanket impls\n- Standard combinators are optimized and well-tested\n\nHelper functions that bypass this system create API\ninconsistency, miss ergonomic benefits, and signal\nunfamiliarity with idiomatic Rust.\n\n## Safe Patterns\n\n```rust\n// Good: From trait enables .into() and ? operator\nimpl From for Settings {\n fn from(c: Config) -> Self {\n Settings { timeout: c.timeout }\n }\n}\n\n// Good: FromStr enables .parse()\nimpl FromStr for Config {\n type Err = ConfigError;\n fn from_str(s: &str) -> Result { ... }\n}\n\n// Good: Default via derive\n#[derive(Default)]\nstruct Config { timeout: u64 }\n\n// Good: Display for human-readable output\nimpl fmt::Display for MyError {\n fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {\n write!(f, \"Error: {}\", self.msg)\n }\n}\n\n// Good: Option combinators\nlet result = opt.map(|x| x.to_string());\nlet value = opt.unwrap_or(default);\n```\n\n## Patterns to Flag\n\n```rust\n// Flag: should be impl FromStr\nfn parse_config(s: &str) -> Config { ... }\n\n// Flag: should be impl From for Foo\nfn foo_from_bar(b: Bar) -> Foo { ... }\n\n// Flag: should be impl Default\nfn default_config() -> Config { ... }\n\n// Flag: should be impl From for MyError\nfn io_to_my_error(e: io::Error) -> MyError { ... }\n\n// Flag: should use .map()\nmatch opt {\n Some(x) => Some(x.to_string()),\n None => None,\n}\n```\n\n## Exclusions (Not Flagged)\n\n- Lossy conversions (`to_lossy_ascii`)\n- Builder methods (`with_timeout(self, ...)`)\n- Multi-parameter conversions (context-dependent)\n- Domain-specific operations (`serialize`, `encode`, `decode`)\n\n## Related Clippy Lints\n\n| Lint | Detects |\n|------|---------|\n| `clippy::from_over_into` | `impl Into` where `impl From` suffices |\n| `clippy::manual_map` | Match on Option rewriting `.map()` |\n| `clippy::manual_unwrap_or` | Match rewriting `.unwrap_or()` |\n| `clippy::derivable_impls` | Manual Default that derive handles |\n| `clippy::manual_flatten` | Nested iteration rewriting `.flatten()` |\n| `clippy::new_without_default` | `fn new()` without `impl Default` |\n\n## Output Section\n\n```markdown\n## Builtin Preference\n### Issues Found\n- [file:line] Conversion helper `parse_config`: use `impl FromStr`\n- [file:line] Manual combinator: use `.map()` (clippy::manual_map)\n\n### Recommendations\n- Implement standard traits to gain ecosystem composability\n- Enable relevant clippy lints for automated enforcement\n```\n\nFile v1.9.16:modules/cargo-dependencies.md\n\n---\nname: cargo-dependencies\ndescription: Dependency auditing, security scanning, and version management\ncategory: rust-review\ntags: [cargo, dependencies, security, audit]\n---\n\n# Cargo Dependencies\n\nAudit and management of Cargo dependencies and build configuration.\n\n## Audit Commands\n\nRun detailed dependency analysis:\n```bash\ncargo tree -d # Find duplicates\ncargo audit # Security vulnerabilities\ncargo outdated # Stale versions\ncargo deny check # Policy enforcement\n```\n\n## Dependency Evaluation\n\nCheck:\n- Feature flags usage\n- Optional dependencies\n- Build scripts safety\n- Binary size impact\n- Compilation time\n\n## Security Scanning\n\nReview for:\n- Known vulnerabilities\n- Abandoned crates\n- Unmaintained dependencies\n- Security advisories\n- Supply chain risks\n\n## Version Management\n\nVerify:\n- Semver compliance\n- Version pinning strategy\n- Dependency updates frequency\n- Breaking change handling\n\n## Common Issues\n\nFlag:\n- Abandoned crates\n- Excessively large dependencies\n- Security-vulnerable versions\n- Duplicate dependencies\n- Unnecessary dependencies\n\n## Alternatives Suggestion\n\nRecommend alternatives for:\n- Unmaintained crates\n- Heavy dependencies\n- Vulnerable versions\n- Better maintained options\n\n## Output Section\n\n```markdown\n## Dependencies\n### Security Issues\n- [crate@version] Vulnerability: [CVE/advisory]\n\n### Recommendations\n- Update [crate] from X to Y\n- Replace [abandoned-crate] with [alternative]\n- Remove unused dependency: [crate]\n```\n\nFile v1.9.16:modules/cfg-test-misuse.md\n\n---\nname: cfg-test-misuse\ndescription: Detection of #[cfg(test)] applied to individual functions or\n impls outside a mod tests block, which is a common structural mistake\ncategory: rust-review\ntags: [testing, cfg, attributes, structure]\n---\n\n# cfg(test) Misuse\n\nAnalysis of `#[cfg(test)]` placement on individual items outside a\n`mod tests { ... }` block.\n\n## What This Detects\n\n`#[cfg(test)]` on a standalone `fn`, `impl`, or `struct` that is not nested\ninside a `mod tests` block.\n\n## Why It Matters\n\nThe idiomatic pattern is a single `mod tests` block gated with `#[cfg(test)]`,\nwhich keeps test code in one place.\nApplying `#[cfg(test)]` to an individual `impl` block is particularly\nhazardous: it removes method implementations from the production binary\nwithout an obvious compiler warning.\n\n## Safe Patterns\n\n```rust\n// Good: single gated mod tests block\n#[cfg(test)]\nmod tests {\n use super::*;\n\n fn helper() { ... }\n\n #[test]\n fn test_something() { ... }\n}\n```\n\n## Patterns to Flag\n\n```rust\n// Bad: cfg(test) on standalone function\n#[cfg(test)]\nfn setup_fixture() { ... }\n\n// Bad: cfg(test) on impl block outside mod tests\n#[cfg(test)]\nimpl MyStruct {\n fn test_helper(&self) { ... }\n}\n```\n\n## Output Section\n\n```markdown\n## cfg(test) Misuse\n### Issues Found\n- [file:line] cfg(test) outside mod tests: [explanation]\n\n### Recommendations\n- Move all test-only items inside a single `#[cfg(test)] mod tests` block\n```\n\nFile v1.9.16:modules/collection-types.md\n\n---\nname: collection-types\ndescription: Detection of Vec used where HashSet or HashMap semantics apply,\n including contains loops, find loops, and dedup patterns\ncategory: rust-review\ntags: [collections, performance, vec, hashset, hashmap]\n---\n\n# Collection Types\n\nAnalysis of `Vec` usage where a different collection type would be more\ncorrect or more efficient.\n\n## What This Detects\n\n- `vec.contains(&x)` -- O(n) membership test; `HashSet` gives O(1)\n- `vec.dedup()` -- sorting + dedup pattern suggests a set\n- `vec.iter().find(...)` / `vec.iter().position(...)` -- linear key lookup\n suggests `HashMap`\n\n## Why It Matters\n\nUsing `Vec` for set or map operations produces O(n) behaviour where O(1) is\navailable.\nIt also signals unclear intent: a `HashSet` communicates uniqueness, a\n`HashMap` communicates keyed access.\n\n## Safe Patterns\n\n```rust\n// Good: set membership\nlet mut seen: HashSet = HashSet::new();\nif seen.contains(&id) { ... }\n\n// Good: keyed lookup\nlet mut index: HashMap = HashMap::new();\nif let Some(user) = index.get(&id) { ... }\n```\n\n## Patterns to Flag\n\n```rust\n// Flag: O(n) membership on unbounded Vec\nusers.contains(&new_user)\n\n// Flag: dedup implies uniqueness invariant\nids.sort();\nids.dedup();\n\n// Flag: linear key scan\nusers.iter().find(|u| u.id == target_id)\n```\n\n## Output Section\n\n```markdown\n## Collection Types\n### Issues Found\n- [file:line] Vec used as set/map: [explanation]\n\n### Recommendations\n- Replace with HashSet for membership checks\n- Replace with HashMap for keyed access\n```\n\nFile v1.9.16:modules/concurrency-patterns.md\n\n---\nname: concurrency-patterns\ndescription: Concurrency cost hierarchy, synchronization primitives, async patterns, and performance-aware concurrency review\ncategory: rust-review\ntags: [concurrency, async, sync, deadlock, atomics, contention, performance]\n---\n\n# Concurrency Patterns\n\nAnalysis of concurrent and async code patterns in Rust,\ngrounded in the concurrency cost hierarchy.\n\n## Concurrency Cost Hierarchy\n\n\"Acquiring a mutex isn't slow; contention is slow.\"\nBefore reviewing concurrency code, classify each\nsynchronization point by its cost tier.\n\n| Level | Name | Approx Cost | Description |\n|-------|------|-------------|-------------|\n| 0 | Thread-local | ~2 ns | No atomics at all; per-thread state |\n| 1 | Uncontended atomics | ~10 ns | Atomic ops, no cross-core sharing |\n| 2 | Contended atomics | ~40-400 ns | Cache-line transfer between cores |\n| 3 | Syscalls | ~1 us | Kernel transitions on lock paths |\n| 4 | Context switches | ~10 us | Blocking locks, scheduler involvement |\n| 5 | Catastrophe | ~ms+ | Spinning on oversubscribed systems |\n| 6 | Kernel page fault | ~100-400 ms | Paged-out buffer re-faulted on access |\n\nLevels 3-5 are performance bugs. Level 6 is invisible in\ntask scheduler traces (tokio-console shows tasks scheduling\nin microseconds while the actual latency occurs in the\nkernel page fault handler). Target Level 2 as the default.\nAchieve Level 1 through contention reduction. Level 0\nrequires architectural redesign (per-thread computation\nwith periodic merges).\n\n**Level 6: kernel paging latency**: Long-lived Tokio\nruntimes that co-reside with large heap users (e.g., ML\nmodel weights) are at risk: the kernel may page out\nlatency-sensitive buffers during idle periods. When audio\nor ring-ring buffers page back in on the next access, the\npage fault adds 100–400 ms of p99 latency with no Tokio\ntrace signal.\n\nDetection: production-only latency spikes, no reproduction\non dev box, `perf stat` shows elevated `page-faults` on\nthe audio/buffer threads.\n\nFix: `libc::mlock` on the buffer pages. See\n`modules/unsafe-audit.md` for the full production checklist\n(RLIMIT_MEMLOCK, page alignment, ENOMEM fallback).\n\n**Key insight**: Performance is dominated by atomic\ninstruction count, not total instruction count. An\nalgorithm with 9x more total instructions but the\nsame number of atomics performs identically.\n\n### What to Flag in Review\n\n- **Level 5**: Fair spin-locks on thread pools larger\n than core count. Always flag.\n- **Level 4**: `std::sync::Condvar` wake patterns that\n convoy. Flag when hot path.\n- **Level 3**: `sched_yield()` or `thread::yield_now()`\n in lock loops. Suggest backoff or parking.\n- **Level 2 (avoidable)**: Atomic RMW on shared counter\n when per-thread counters and merge would suffice.\n- **False sharing**: Independent atomics on the same\n cache line (64 bytes). Suggest `#[repr(align(64))]`\n or `crossbeam_utils::CachePadded`.\n\n## Synchronization Primitives\n\nReview primitives usage:\n\n- `Arc`, `Mutex`, `RwLock`\n- `Atomic*` types and ordering (`Relaxed` vs `SeqCst`)\n- `tokio::sync` (mpsc, broadcast, watch, Semaphore)\n- `Send`/`Sync` bounds\n- `parking_lot` vs `std::sync` trade-offs\n\n### Memory Ordering Review\n\nCheck ordering is neither too weak nor too strong:\n\n- `Relaxed`: Counters, statistics (no cross-variable\n ordering needed)\n- `Acquire`/`Release`: Publish/consume patterns,\n one-shot flags\n- `SeqCst`: Only when total order across multiple\n atomics is required (rare; flag overuse)\n\n## Async Patterns\n\nCheck async code:\n\n- No blocking in async functions\n- Proper `spawn_blocking` usage\n- Guards dropped before awaiting\n- Cancellation safety\n- Task spawning patterns\n\n## Best Practices\n\n```rust\n// Good: Drop guard before await\nasync fn update(data: Arc>) {\n let value = {\n let guard = data.lock().await;\n guard.value.clone()\n }; // Guard dropped\n process(value).await;\n}\n\n// Good: Cache-padded to prevent false sharing\nuse crossbeam_utils::CachePadded;\n\nstruct Counters {\n reads: CachePadded,\n writes: CachePadded,\n}\n```\n\n## Contention Reduction Patterns\n\nWhen review finds Level 2+ contention on hot paths:\n\n1. **Shard the lock**: `DashMap`, `ShardedLock`, or\n manual sharding by key hash\n2. **Per-thread accumulation**: Thread-local counters\n merged at read time (Level 2 to Level 0)\n3. **Read-copy-update (RCU)**: `arc-swap` for\n read-heavy, write-rare data\n4. **Lock-free structures**: `crossbeam` queues and\n deques when contention dominates\n\n## Deadlock Prevention\n\nIdentify potential deadlocks:\n\n- Lock ordering consistency\n- Nested locks\n- Await points while holding locks\n- Circular dependencies\n\n## Data Race Detection\n\nCheck for:\n\n- `static mut` misuse\n- Shared mutable state\n- Missing synchronization\n- Race conditions\n\n## Send/Sync Bounds\n\nVerify:\n\n- Proper trait bounds\n- Thread safety guarantees\n- Cross-thread data transfer\n- Closure captures\n\n## Common Issues\n\n- Blocking in async context\n- Guards held across await points\n- Inconsistent lock ordering\n- Missing bounds on generics\n- Unsafe Send/Sync implementations\n- `SeqCst` used everywhere (usually `Acquire`/`Release`\n suffices; `SeqCst` adds unnecessary fence cost)\n- Spinning without backoff on oversubscribed systems\n- False sharing between independent atomics\n\n## Output Section\n\n```markdown\n## Concurrency\n### Cost Classification\n- [file:line] Level N: [primitive] - [justification]\n\n### Issues Found\n- [file:line] Guard held across await: [details]\n- [file:line] Potential deadlock: [scenario]\n- [file:line] False sharing risk: [layout details]\n- [file:line] Unnecessary SeqCst: [suggest weaker ordering]\n\n### Recommendations\n- [concurrency improvements with cost tier impact]\n```\n\n## References\n\n- Jon Gjengset, \"The Cost of Concurrency Coordination\"\n (video: youtube.com/watch?v=tND-wBBZ8RY)\n- Travis Downs, \"A Concurrency Cost Hierarchy\"\n (travisdowns.github.io/blog/2020/07/06/concurrency-costs.html)\n- Mara Bos, \"Rust Atomics and Locks\" (O'Reilly)\n\nFile v1.9.16:modules/duplicate-validators.md\n\n---\nname: duplicate-validators\ndescription: Detection of multiple validate_*, check_*, or verify_* functions\n that share similar structure and could be consolidated\ncategory: rust-review\ntags: [design, duplication, validation, refactoring]\n---\n\n# Duplicate Validators\n\nAnalysis of `validate_*`, `check_*`, and `verify_*` functions for\nopportunities to consolidate repeated validation logic.\n\n## What This Detects\n\nThree or more functions sharing the same verb prefix (`validate_`, `check_`,\n`verify_`) within a single file, which often indicates copy-pasted validation\nlogic that could be unified.\n\n## Why It Matters\n\nDuplicated validation logic diverges over time: one copy gets a bug fix or a\nnew rule while the others do not.\nConsolidating into a generic validator ensures all callers benefit from each\nfix.\n\n## Safe Patterns\n\n```rust\n// Good: single generic validator with rule injection\nfn validate_field(value: &str, rules: &[ValidationRule])\n -> Result<(), ValidationError>\n{\n for rule in rules {\n rule.apply(value)?;\n }\n Ok(())\n}\n```\n\n## Patterns to Flag\n\n```rust\n// Flag when 3+ share the same prefix:\nfn validate_email(s: &str) -> bool { ... }\nfn validate_phone(s: &str) -> bool { ... }\nfn validate_username(s: &str) -> bool { ... }\nfn validate_password(s: &str) -> bool { ... }\n```\n\n## Output Section\n\n```markdown\n## Duplicate Validators\n### Issues Found\n- [file] 4 validate_* functions: [list]\n\n### Recommendations\n- Extract shared logic into a generic validator\n- Use a trait or rule-set parameter to unify related checks\n```\n\nFile v1.9.16:modules/error-handling.md\n\n---\nname: error-handling\ndescription: Result/Option patterns, custom error types, and error propagation analysis\ncategory: rust-review\ntags: [errors, result, option, propagation]\n---\n\n# Error Handling\n\nAnalysis of error handling patterns and correctness in Rust code.\n\n## Result and Option Usage\n\nEvaluate:\n- `Result` and `Option` usage patterns\n- Custom error types design\n- Context addition with `anyhow` or `thiserror`\n- `?` propagation correctness\n\n## Error Type Design\n\nCheck custom error types:\n- Implements `std::error::Error`\n- Provides meaningful context\n- Conversion traits (`From`, `Into`)\n- Error hierarchy structure\n\n## Error Propagation\n\nBest practices:\n```rust\n// Good: Proper error propagation\nfn process() -> Result<(), ProcessError> {\n let data = fetch().context(\"failed to fetch\")?;\n validate(&data)?;\n Ok(())\n}\n```\n\n## Common Issues to Flag\n\n- Panics in library code (`unwrap`, `expect`)\n- Logging side-effects in error paths\n- Mismatched error hierarchies\n- Missing retry/backoff logic\n- Silent error swallowing\n- Over-generic error types\n\n## Error Context\n\nVerify context is added:\n- Operation context\n- Input data context\n- Failure reasons\n- Recovery suggestions\n\n## Output Section\n\n```markdown\n## Error Handling\n### Issues Found\n- [file:line] Panic in library: [details]\n- [file:line] Missing context: [suggestion]\n\n### Recommendations\n- [error handling improvements]\n```\n\nFile v1.9.16:modules/error-messages.md\n\n---\nname: error-messages\ndescription: Detection of short error strings (under ~20 chars) in\n Err(), panic!(), and expect() that lack context or recovery hints\ncategory: rust-review\ntags: [error-handling, diagnostics, messages, quality]\n---\n\n# Error Messages\n\nAnalysis of error and panic messages for actionability.\nShort messages without context make production incidents harder to diagnose.\n\n## What This Detects\n\nString literals under roughly 20 characters used in:\n\n- `Err(\"short msg\")`\n- `panic!(\"short msg\")`\n- `.expect(\"short msg\")`\n- `Err(\"short msg\".to_string())`\n\n## Why It Matters\n\nA message like `\"not found\"` or `\"failed\"` gives an on-call engineer no\ninformation about what was not found, where the failure occurred, or how to\nrecover.\n\n## Safe Patterns\n\n```rust\n// Good: identifies operation and input\n.expect(\"failed to open config file at $CONFIG_PATH\")\n\n// Good: Err with context\nreturn Err(format!(\n \"user {} not found in tenant {}\",\n user_id, tenant_id\n));\n```\n\n## Patterns to Flag\n\n```rust\n// Bad: no context\n.expect(\"failed\")\n.expect(\"not found\")\n\n// Bad: Err with bare short string\nreturn Err(\"bad input\");\nreturn Err(\"denied\".to_string());\n```\n\n## Output Section\n\n```markdown\n## Error Messages\n### Issues Found\n- [file:line] Short error message: [explanation]\n\n### Recommendations\n- Add operation context: what were you trying to do?\n- Add input context: what value triggered the failure?\n- Add recovery hints where possible\n```\n\nFile v1.9.16:modules/iterator-and-allocation-slop.md\n\n---\nmodule: iterator-and-allocation-slop\ncategory: detection\ndependencies: [Read, Grep]\nestimated_tokens: 600\n---\n\n# Iterator and Allocation Slop\n\n**AI-generated Rust defaults to manual loops where\niterators read better, and to allocation where references\nsuffice. Both compile; both are slop.**\n\nThis module covers two of the highest-frequency AI Rust\nanti-patterns: imperative loops the iterator API expresses\nin one line, and unnecessary allocation that\nborrow-checker capitulation produces. The clippy lints\ncatch most of this; the rest is judgment.\n\n## Iterator slop\n\n### Pattern 1: index-based loops\n\n```rust\n// SLOP\nlet mut sum = 0;\nfor i in 0..vec.len() {\n sum += vec[i];\n}\n\n// Idiomatic\nlet sum: i32 = vec.iter().sum();\n```\n\nDetector: `clippy::needless_range_loop`.\n\n### Pattern 2: filter-then-push\n\n```rust\n// SLOP\nlet mut result = Vec::new();\nfor x in xs.iter() {\n if x.is_active() {\n result.push(x.id);\n }\n}\n\n// Idiomatic\nlet result: Vec<_> = xs.iter()\n .filter(|x| x.is_active())\n .map(|x| x.id)\n .collect();\n```\n\n### Pattern 3: map-filter-unwrap\n\n```rust\n// SLOP\nlet firsts: Vec<_> = xs.iter()\n .map(|x| x.first())\n .filter(|x| x.is_some())\n .map(|x| x.unwrap())\n .collect();\n\n// Idiomatic\nlet firsts: Vec<_> = xs.iter()\n .filter_map(|x| x.first())\n .collect();\n```\n\nDetector: `clippy::manual_filter_map`.\n\n### Pattern 4: collect-then-iterate\n\n```rust\n// SLOP\nlet intermediate: Vec<_> = xs.iter().map(transform).collect();\nfor item in intermediate {\n use_it(item);\n}\n\n// Idiomatic\nfor item in xs.iter().map(transform) {\n use_it(item);\n}\n```\n\nDetector: `clippy::needless_collect`.\n\n### Pattern 5: bool-match where if suffices\n\n```rust\n// SLOP\nmatch flag {\n true => do_a(),\n false => do_b(),\n}\n\n// Idiomatic\nif flag { do_a() } else { do_b() }\n```\n\nDetector: `clippy::match_bool`.\n\n## Allocation slop\n\n### Pattern A: `.clone()` to satisfy the borrow checker\n\nThe single most common AI-generated Rust anti-pattern. The\n`rust-unofficial/patterns` book lists it as the canonical\nanti-pattern: cloning to make a borrow-checker error go\naway rather than to express ownership.\n\n```rust\n// SLOP\nfn greet(name: String) {\n println!(\"Hello, {name}\");\n}\nfn main() {\n let n = String::from(\"world\");\n greet(n.clone()); // unnecessary clone\n greet(n.clone());\n}\n\n// Idiomatic\nfn greet(name: &str) {\n println!(\"Hello, {name}\");\n}\nfn main() {\n let n = String::from(\"world\");\n greet(&n);\n greet(&n);\n}\n```\n\nDetection heuristic: any `.clone()` on a `String`,\n`Vec<_>`, `HashMap<_,_>`, `Arc>`, or large\nstruct that is *not* paired with a comment explaining\nthe ownership rationale. If it disappeared, would the\nborrow checker complain? If yes, the right fix is\nusually to take a borrowed reference (`&str`, `&[T]`,\n`&T`).\n\nDetectors: `clippy::redundant_clone`,\n`clippy::clone_on_ref_ptr`.\n\n### Pattern B: owned parameters that should borrow\n\n| Slop signature | Idiomatic signature |\n|---|---|\n| `fn f(s: &String)` | `fn f(s: &str)` |\n| `fn f(v: &Vec)` | `fn f(v: &[T])` |\n| `fn f(s: String)` (read-only) | `fn f(s: &str)` |\n| `fn f(v: Vec)` (read-only) | `fn f(v: &[T])` |\n| `fn f(b: Box)` (no boxing reason) | `fn f(t: T)` |\n\nDetector: `clippy::ptr_arg` catches `&Vec` and `&String`.\n\n### Pattern C: `format!` then convert\n\n```rust\n// SLOP\nlet s = format!(\"{}\", x);\n\n// Idiomatic (when Display is implemented)\nlet s = x.to_string();\n\n// Inverse SLOP\nlet s = x.to_string();\nlet s = format!(\"{s}{rest}\");\n\n// Idiomatic (build the string once)\nlet s = format!(\"{x}{rest}\");\n```\n\nDetectors: `clippy::useless_format`, `clippy::str_to_string`.\n\n### Pattern D: redundant allocation\n\n```rust\n// SLOP\nlet owned = borrowed.to_owned();\nfn take_str(s: &str) { ... }\ntake_str(&owned);\n\n// Idiomatic — borrow directly\ntake_str(borrowed);\n\n// SLOP\nlet s = String::new();\nlet s = s + \"hello\" + \" \" + \"world\";\n\n// Idiomatic\nlet s = String::from(\"hello world\");\n```\n\n### Pattern E: `Box::new(...)` without indirection reason\n\n```rust\n// SLOP\nlet x = Box::new(42_u64);\nfn use_it(n: u64) { ... }\nuse_it(*x);\n\n// Idiomatic\nlet x = 42_u64;\nuse_it(x);\n```\n\nHeap allocation is justified for:\n- `dyn Trait` objects (`Box`, `Box`).\n- Recursive types (`Box`).\n- Large stack-frame avoidance (uncommon; measure first).\n- Pinning requirements (`Pin>`).\n\nAnywhere else, `Box::new` is unjustified allocation.\n\n### Pattern E2: `Box` or `&dyn Trait` in a hot inner loop\n\nThis is distinct from Pattern E. The box itself may be\njustified: the problem is calling a `dyn` method millions of\ntimes when the method body is tiny.\n\n```rust\n// Potentially slow: dyn dispatch in the inner loop\nlet decoders: Vec> = build_decoders(&schema, &batch);\n\nfor i in 0..n_rows {\n for d in &decoders {\n d.write_to_row(i, &mut row); // indirect call every iteration\n }\n}\n```\n\n**Why it matters**: each `dyn` call goes through a vtable\n(`call *0x18(%rax)`). The compiler cannot inline across that\nboundary, so it cannot fuse the inner loop, vectorize small\nstores, or eliminate the function-call prologue/epilogue overhead.\nWhen the method body is ~25 instructions (a null check, a bit\nflip, a 4-byte move), the prologue/epilogue and indirect\njump overhead can represent 40–50% of total runtime.\n\nNote: `&dyn Trait` has the same problem as `Box`.\nThe issue is dynamic dispatch, not heap allocation.\n\n**Fix 1: flip the loop order (batch-first)**: iterate\nall rows for each decoder, not all decoders for each row.\nThe dyn dispatch cost is paid once per decoder per batch\ninstead of once per cell:\n\n```rust\nlet mut rows: Vec = (0..n_rows)\n .map(|i| WriteRow::new(&segment, key_array.value(i)))\n .collect();\n\nfor d in &decoders {\n d.write_to_rows(0, &mut rows[..]); // dispatch once per decoder\n}\n```\n\n**Fix 2: enum dispatch**: replace `dyn Trait` with a\nclosed enum. The compiler can monomorphize and inline each\nvariant:\n\n```rust\nenum ColDecoder {\n F32(F32Decoder),\n Utf8(Utf8Decoder),\n Bool(BoolDecoder),\n}\n\nimpl ColDecoder {\n #[inline(always)]\n fn write_to_row(&self, index: usize, row: &mut WriteRow) {\n match self {\n ColDecoder::F32(d) => d.write_to_row(index, row),\n ColDecoder::Utf8(d) => d.write_to_row(index, row),\n ColDecoder::Bool(d) => d.write_to_row(index, row),\n }\n }\n}\n```\n\n**When to flag**: any `Vec>` or `Vec<&dyn Trait>`\niterated inside an inner loop where the method body is\ninlineable. The Java/JVM analogy is instructive: the JVM\nde-virtualizes and inlines at JIT time; rustc cannot cross\nthe `dyn` boundary. If reviewers come from JVM backgrounds,\nthis is the most important Rust performance lesson to surface.\n\nDetection:\n\n```bash\n# Find Vec> that appear inside nested loops\nrg \"Vec = vec![2, 3, 5, 7, 11];\n\n// Idiomatic for small, fixed-size\nlet primes: [u32; 5] = [2, 3, 5, 7, 11];\n\n// Or for stack-allocated growable\nlet primes: SmallVec<[u32; 5]> = smallvec![2, 3, 5, 7, 11];\n```\n\nThis one is judgment: arrays for compile-time-known\nsizes, `SmallVec`/`ArrayVec` for \"usually small but\nsometimes grows\", `Vec` for genuinely dynamic.\n\n## Detection commands\n\n```bash\n# Catch most iterator slop with clippy\ncargo clippy --all-targets -- \\\n -W clippy::needless_range_loop \\\n -W clippy::manual_filter_map \\\n -W clippy::needless_collect \\\n -W clippy::filter_map_next \\\n -W clippy::map_unwrap_or \\\n -W clippy::match_bool \\\n -W clippy::needless_match \\\n -D warnings\n\n# Catch most allocation slop with clippy\ncargo clippy --all-targets -- \\\n -W clippy::redundant_clone \\\n -W clippy::clone_on_ref_ptr \\\n -W clippy::ptr_arg \\\n -W clippy::useless_format \\\n -W clippy::str_to_string \\\n -W clippy::redundant_allocation \\\n -D warnings\n\n# Manual scan: every .clone() in the codebase\nrg \"\\.clone\\(\\)\" --type rust -n | head -50\n# For each: ask \"would the borrow checker complain if removed?\"\n```\n\n## False positives\n\nSome `.clone()` calls are correct and should stay:\n\n- **Async / spawn boundaries**: cloning an `Arc` to\n send into `tokio::spawn` is required, not slop.\n- **Genuine ownership transfer**: when two callers each\n need to mutate independently from the same source.\n- **Intentional defensive copy**: in security-sensitive\n paths where the original might be modified by an\n attacker. Mark with `// COPY: defense against ...`.\n\nWhen in doubt, comment the rationale next to the\n`.clone()`. A `.clone()` with a one-line \"why\" comment\nis documented intent; a `.clone()` with no comment is\nslop.\n\n## Output format\n\nFor each finding, use the\n`Skill(scribe:slop-detector)` module\n`structured-finding-output.md` format. Severity is\n`medium` for individual iterator/allocation slop;\n`high` if the same pattern appears 5+ times in the same\nfile (suggests systematic AI-generation rather than\nisolated mistake).\n\n## Integration\n\nIterator and allocation slop typically lands in Pass 5\n(code idiom sweep) of the multi-pass cleanup workflow\n(see `Skill(scribe:slop-detector)` module\n`cleanup-workflow.md`). Run after the linter floor\n(Pass 1) clears, since clippy will flag most of these\nautomatically.\n\nArchive v1.9.14: 19 files, 33568 bytes\n\nFiles: modules/async-slop.md (7937b), modules/builtin-preference.md (3751b), modules/cargo-dependencies.md (1516b), modules/cfg-test-misuse.md (1438b), modules/collection-types.md (1539b), modules/concurrency-patterns.md (5998b), modules/duplicate-validators.md (1559b), modules/error-handling.md (1405b), modules/error-messages.md (1459b), modules/iterator-and-allocation-slop.md (9416b), modules/model-specific-tells.md (7032b), modules/ownership-analysis.md (1469b), modules/silent-returns.md (1678b), modules/sql-injection.md (1520b), modules/test-slop.md (8018b), modules/unsafe-audit.md (3876b), skill-card.md (2339b), SKILL.md (4549b), _meta.json (142b)\n\nFile v1.9.14:SKILL.md\n\n---\nname: rust-review\ndescription: Audits Rust code for unsafe blocks, ownership issues, and Cargo dependency risks\nversion: 1.9.8\ntriggers:\n - rust\n - ownership\n - concurrency\n - unsafe\n - traits\n - cargo\n - reviewing Rust code or before merging Rust changes\nmetadata: {\"openclaw\": {\"homepage\": \"https://github.com/athola/claude-night-market/tree/master/plugins/pensive\", \"emoji\": \"\\ud83d\\udd0d\", \"requires\": {\"config\": [\"night-market.pensive:shared\", \"night-market.imbue:proof-of-work\"]}}}\nsource: claude-night-market\nsource_plugin: pensive\n---\n\n> **Night Market Skill** — ported from [claude-night-market/pensive](https://github.com/athola/claude-night-market/tree/master/plugins/pensive). For the full experience with agents, hooks, and commands, install the Claude Code plugin.\n\n\n## Table of Contents\n\n- [Quick Start](#quick-start)\n- [When to Use](#when-to-use)\n- [Required TodoWrite Items](#required-todowrite-items)\n- [Progressive Loading](#progressive-loading)\n- [Core Workflow](#core-workflow)\n- [Rust Quality Checklist](#rust-quality-checklist)\n- [Safety](#safety)\n- [Correctness](#correctness)\n- [Performance](#performance)\n- [Idioms](#idioms)\n- [Output Format](#output-format)\n- [Summary](#summary)\n- [Ownership Analysis](#ownership-analysis)\n- [Error Handling](#error-handling)\n- [Concurrency](#concurrency)\n- [Unsafe Audit](#unsafe-audit)\n- [[U1] file:line](#[u1]-file:line)\n- [Dependencies](#dependencies)\n- [Recommendation](#recommendation)\n- [Exit Criteria](#exit-criteria)\n\n\n# Rust Review Workflow\n\nExpert-level Rust code audits with focus on safety, correctness, and idiomatic patterns.\n\n## Quick Start\n\n```bash\n/rust-review\n```\n**Verification:** Run the command with `--help` flag to verify availability.\n\n## When To Use\n\n- Reviewing Rust code changes\n- Auditing unsafe blocks\n- Analyzing concurrency patterns\n- Dependency security review\n- Performance optimization review\n\n## When NOT To Use\n\n- General code review without Rust - use unified-review\n- Performance profiling - use parseltongue:python-performance pattern\n\n## Required TodoWrite Items\n\n1. `rust-review:ownership-analysis`\n2. `rust-review:error-handling`\n3. `rust-review:concurrency`\n4. `rust-review:unsafe-audit`\n5. `rust-review:cargo-deps`\n6. `rust-review:evidence-log`\n\n## Progressive Loading\n\nLoad modules as needed based on review scope:\n\n**Quick Review** (ownership and errors):\n- See `modules/ownership-analysis.md` for borrowing and lifetime analysis\n- See `modules/error-handling.md` for Result/Option patterns\n\n**Concurrency Focus**:\n- See `modules/concurrency-patterns.md` for async and sync primitives\n\n**Safety Audit**:\n- See `modules/unsafe-audit.md` for unsafe block documentation\n\n**Dependency Review**:\n- See `modules/cargo-dependencies.md` for vulnerability scanning\n\n**Idiomatic Patterns**:\n- See `modules/builtin-preference.md` for conversion traits and builtin preference\n\n## Core Workflow\n\n1. **Ownership Analysis**: Check borrowing, lifetimes, clone patterns\n2. **Error Handling**: Verify Result/Option usage, propagation\n3. **Concurrency**: Review async patterns, sync primitives\n4. **Unsafe Audit**: Document invariants, FFI contracts\n5. **Dependencies**: Scan for vulnerabilities, updates\n6. **Evidence Log**: Record commands and findings\n\n## Rust Quality Checklist\n\n### Safety\n- [ ] All unsafe blocks documented with SAFETY comments\n- [ ] FFI boundaries properly wrapped\n- [ ] Memory safety invariants maintained\n- [ ] `mlock`/`munlock` calls: RLIMIT verified, page-aligned,\n ENOMEM handled\n\n### Correctness\n- [ ] Error handling complete\n- [ ] Concurrency patterns sound\n- [ ] Tests cover critical paths\n\n### Performance\n- [ ] No unnecessary allocations\n- [ ] Borrowing preferred over cloning\n- [ ] Async properly non-blocking\n\n### Idioms\n- [ ] Standard traits implemented\n- [ ] Conversion traits preferred over helper functions\n- [ ] Error types well-designed\n- [ ] Documentation complete\n\n## Output Format\n\n```markdown\n## Summary\nRust audit findings\n\n## Ownership Analysis\n[borrowing and lifetime issues]\n\n## Error Handling\n[error patterns and issues]\n\n## Concurrency\n[async and sync patterns]\n\n## Unsafe Audit\n### [U1] file:line\n- Invariants: [documented]\n- Risk: [assessment]\n- Recommendation: [action]\n\n## Dependencies\n[cargo audit results]\n\n## Recommendation\nApprove / Approve with actions / Block\n```\n**Verification:** Run the command with `--help` flag to verify availability.\n\n## Exit Criteria\n\n- All unsafe blocks audited\n- Concurrency patterns verified\n- Dependencies scanned\n- Evidence logged\n- Action items assigned\n\nFile v1.9.14:_meta.json\n\n{\n \"ownerId\": \"kn7d107jg9jv602h9ytsegydq184a42s\",\n \"slug\": \"nm-pensive-rust-review\",\n \"version\": \"1.9.14\",\n \"publishedAt\": 1782842668776\n}\n\nFile v1.9.14:modules/async-slop.md\n\n---\nmodule: async-slop\ncategory: detection\ndependencies: [Read, Grep]\nestimated_tokens: 500\n---\n\n# Async Slop\n\n**AI defaults to `async` and `tokio::spawn` even where\nsync code is faster, simpler, and correct.**\n\nThis module covers the high-frequency async patterns that\nlook idiomatic but are not. The clippy lints catch some;\nthe rest is structural.\n\n## Pattern 1: `async fn` that contains no `.await`\n\n```rust\n// SLOP\nasync fn compute_total(items: &[Item]) -> u64 {\n items.iter().map(|i| i.price).sum()\n}\n```\n\nIf the function body has no `.await`, it has no reason\nto be `async`. Async coloring is contagious: this\nfunction is callable only from async contexts, forcing\nevery caller to also be `async`. Strip `async` from the\nsignature unless the body actually awaits.\n\nDetection (preferred: clippy):\n\n```bash\ncargo clippy --all-targets -- -W clippy::async_yields_async\n```\n\nFile-level heuristic when clippy is unavailable:\n\n```bash\nfor f in $(rg -l \"async fn \" --type rust); do\n rg -q \"\\.await\" \"$f\" || echo \"no-await: $f\"\ndone\n```\n\n(Heuristic; manual review needed since `.await` may be in\na helper called by the async fn rather than inline.)\n\n## Pattern 2: blocking I/O inside an async runtime\n\n```rust\n// SLOP\nasync fn read_config() -> Result {\n Ok(std::fs::read_to_string(\"config.toml\")?)\n}\n\n// SLOP\nasync fn rate_limit_wait() {\n std::thread::sleep(Duration::from_secs(1)); // blocks the runtime\n}\n\n// SLOP\nasync fn query_db(conn: &Connection) -> Result> {\n conn.query(\"SELECT ...\")? // blocking driver\n}\n```\n\nBlocking calls inside `async` block the entire executor\nthread, defeating the runtime's concurrency model.\n\nFix:\n\n```rust\n// Use the async equivalent\nasync fn read_config() -> Result {\n Ok(tokio::fs::read_to_string(\"config.toml\").await?)\n}\n\n// Or wrap blocking work in spawn_blocking\nasync fn rate_limit_wait() {\n tokio::time::sleep(Duration::from_secs(1)).await;\n}\n\n// For unavoidable blocking work\nasync fn query_db(conn: Arc) -> Result> {\n let conn = conn.clone();\n tokio::task::spawn_blocking(move || conn.query(\"SELECT ...\"))\n .await?\n}\n```\n\nDetection:\n\n```bash\n# Find blocking ops inside async functions (heuristic)\nrg -B 5 \"(std::fs::|std::thread::sleep|std::net::TcpStream)\" --type rust |\n rg -B 5 \"async fn\"\n```\n\n## Pattern 3: `tokio::spawn` for synchronous-equivalent work\n\n```rust\n// SLOP\nasync fn handle_request(req: Request) -> Response {\n let result = tokio::spawn(async move {\n compute_response(&req)\n }).await.unwrap();\n result\n}\n```\n\nSpawning a task only to immediately await its single\ncompletion is equivalent to a direct call, plus the\noverhead of task creation, scheduling, and a join. Just\ncall the function:\n\n```rust\nasync fn handle_request(req: Request) -> Response {\n compute_response(&req)\n}\n```\n\n`tokio::spawn` is for *concurrent* work: when the\nspawned task should make progress while the caller does\nsomething else, or when the task should outlive the\ncaller. A spawn-then-immediately-await is a smell.\n\n## Pattern 4: `async-trait` on synchronous-equivalent traits\n\n```rust\n// SLOP\n#[async_trait]\ntrait Greeter {\n async fn greet(&self, name: &str) -> String;\n}\n```\n\nIf the implementation has no `.await` and just returns a\nsynchronous value, `async-trait` adds heap allocation\n(`Box`) for nothing. Make the trait sync:\n\n```rust\ntrait Greeter {\n fn greet(&self, name: &str) -> String;\n}\n```\n\nUse `async-trait` only when at least one implementation\ngenuinely awaits.\n\n## Pattern 5: explicit `Pin>` returns\n\n```rust\n// SLOP\nfn fetch_data(url: &str) -> Pin> + Send>> {\n Box::pin(async move {\n // body\n })\n}\n```\n\nModern Rust supports `impl Future` in return position:\n\n```rust\n// Idiomatic\nfn fetch_data(url: &str) -> impl Future> + Send {\n async move {\n // body\n }\n}\n```\n\n`Pin>` is needed only for trait method\nreturns or when storing futures in collections.\n\n## Pattern 6: `MutexGuard` held across `.await`\n\n```rust\n// SLOP — deadlock risk\nasync fn update_count(state: &Arc>) {\n let mut guard = state.lock().unwrap();\n guard.count += 1;\n save_to_disk(&guard).await; // holds guard across await\n}\n```\n\nHolding a sync `Mutex` guard across `.await` blocks the\nruntime if any other task tries to acquire the same lock.\nFor async paths, use:\n\n- `tokio::sync::Mutex` (async-aware, can hold guards\n across `.await`).\n- Or restructure to drop the guard before awaiting:\n\n```rust\nasync fn update_count(state: &Arc>) {\n let snapshot = {\n let mut guard = state.lock().unwrap();\n guard.count += 1;\n guard.clone()\n }; // guard dropped here\n save_to_disk(&snapshot).await;\n}\n```\n\nThis is the GPT-5.x signature failure (Sonar measured\n~470 concurrency issues per MLOC for GPT-5.2 High); see\n`model-specific-tells.md`.\n\n## Pattern 7: re-implementing `select!` / `join!` manually\n\nIf you find yourself manually polling multiple futures\nwith `Pin::new` and `Poll`, you almost certainly want\n`tokio::select!` or `tokio::join!`. Hand-rolled polling\nis a strong signal that the model copied something it\nshould not have.\n\nDetection:\n\n```bash\nrg \"Pin::new\" --type rust -B 2 -A 5 | rg -B 2 \"fn poll\"\n```\n\n## Pattern 8: `Send + Sync` bounds added \"in case\"\n\n```rust\n// SLOP\nfn add(a: T, b: T) -> T { ... }\n```\n\nTrait bounds should be added because the function\n*needs* them, not as defensive over-spec. `Send`/`Sync`\non a function that runs synchronously, `Clone` on a\nfunction that doesn't clone, `Debug` on a function that\ndoesn't print: all noise.\n\nThe right rule: add the bound when the compiler complains\nwithout it. Remove the bound when removing it does not\ncause a compile error.\n\n## Detection commands\n\n```bash\n# Catch most async slop with clippy\ncargo clippy --all-targets -- \\\n -W clippy::async_yields_async \\\n -W clippy::large_futures \\\n -D warnings\n\n# Manual scans for the structural patterns\n# Pattern 1: file-level \"async fn but no .await\" — see Pattern 1\n# section above for the loop form.\nrg \"tokio::spawn.*\\.await\" --type rust # Pattern 3\nrg \"#\\[async_trait\\]\" --type rust # Pattern 4\nrg \"Pin>), 8 (defensive Send+Sync).\n\nPattern 6 is the highest-blast-radius async finding;\nescalate to `severity: high` and to a senior reviewer.\n\n## Integration\n\nAsync slop lands in Pass 5 of the multi-pass cleanup\nworkflow (`Skill(scribe:slop-detector)` module\n`cleanup-workflow.md`). For GPT-family-generated\ncodebases, weight pattern 6 detection most heavily;\nfor Claude-family codebases, weight pattern 1 (the\n\"behavior-preserving refactor leaves async fn that no\nlonger awaits anything\") most heavily. See\n`model-specific-tells.md`.\n\nFile v1.9.14:modules/builtin-preference.md\n\n---\nname: builtin-preference\ndescription: Detection of helper functions that should be standard trait\n implementations and reimplemented Rust builtins\ncategory: rust-review\ntags: [from, into, tryfrom, fromstr, default, display, iterator, idioms]\n---\n\n# Builtin Preference\n\nDetects custom helper functions that duplicate Rust's standard\ntrait system and built-in combinators.\n\n## What This Detects\n\nFour categories of anti-patterns:\n\n1. **Conversion helpers**: `parse_foo()`, `foo_from_bar()`,\n `convert_*()`, `to_*(&self)` that should be `FromStr`,\n `From`, `TryFrom`, or `Into` implementations\n2. **Standard trait replacements**: `default_config()`,\n `format_error()`, `as_bytes(&self)`, `compare()` that\n should be `Default`, `Display`, `AsRef`, or `PartialEq`\n3. **Error conversion wrappers**: `io_to_my_error()`,\n `wrap_error()` that should be `impl From` or\n thiserror `#[from]`\n4. **Manual combinators**: `match opt { Some(x) => Some(f(x)),\n None => None }` that should be `.map()`, `.unwrap_or()`,\n `.flatten()`, etc.\n\n## Why It Matters\n\nRust's trait system is compositional by design:\n\n- `impl From for B` gives `impl Into for A` for free\n- `impl Display` gives `ToString` for free\n- `From` enables the `?` operator for error propagation\n- Trait impls participate in generic bounds and blanket impls\n- Standard combinators are optimized and well-tested\n\nHelper functions that bypass this system create API\ninconsistency, miss ergonomic benefits, and signal\nunfamiliarity with idiomatic Rust.\n\n## Safe Patterns\n\n```rust\n// Good: From trait enables .into() and ? operator\nimpl From for Settings {\n fn from(c: Config) -> Self {\n Settings { timeout: c.timeout }\n }\n}\n\n// Good: FromStr enables .parse()\nimpl FromStr for Config {\n type Err = ConfigError;\n fn from_str(s: &str) -> Result { ... }\n}\n\n// Good: Default via derive\n#[derive(Default)]\nstruct Config { timeout: u64 }\n\n// Good: Display for human-readable output\nimpl fmt::Display for MyError {\n fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {\n write!(f, \"Error: {}\", self.msg)\n }\n}\n\n// Good: Option combinators\nlet result = opt.map(|x| x.to_string());\nlet value = opt.unwrap_or(default);\n```\n\n## Patterns to Flag\n\n```rust\n// Flag: should be impl FromStr\nfn parse_config(s: &str) -> Config { ... }\n\n// Flag: should be impl From for Foo\nfn foo_from_bar(b: Bar) -> Foo { ... }\n\n// Flag: should be impl Default\nfn default_config() -> Config { ... }\n\n// Flag: should be impl From for MyError\nfn io_to_my_error(e: io::Error) -> MyError { ... }\n\n// Flag: should use .map()\nmatch opt {\n Some(x) => Some(x.to_string()),\n None => None,\n}\n```\n\n## Exclusions (Not Flagged)\n\n- Lossy conversions (`to_lossy_ascii`)\n- Builder methods (`with_timeout(self, ...)`)\n- Multi-parameter conversions (context-dependent)\n- Domain-specific operations (`serialize`, `encode`, `decode`)\n\n## Related Clippy Lints\n\n| Lint | Detects |\n|------|---------|\n| `clippy::from_over_into` | `impl Into` where `impl From` suffices |\n| `clippy::manual_map` | Match on Option rewriting `.map()` |\n| `clippy::manual_unwrap_or` | Match rewriting `.unwrap_or()` |\n| `clippy::derivable_impls` | Manual Default that derive handles |\n| `clippy::manual_flatten` | Nested iteration rewriting `.flatten()` |\n| `clippy::new_without_default` | `fn new()` without `impl Default` |\n\n## Output Section\n\n```markdown\n## Builtin Preference\n### Issues Found\n- [file:line] Conversion helper `parse_config`: use `impl FromStr`\n- [file:line] Manual combinator: use `.map()` (clippy::manual_map)\n\n### Recommendations\n- Implement standard traits to gain ecosystem composability\n- Enable relevant clippy lints for automated enforcement\n```\n\nFile v1.9.14:modules/cargo-dependencies.md\n\n---\nname: cargo-dependencies\ndescription: Dependency auditing, security scanning, and version management\ncategory: rust-review\ntags: [cargo, dependencies, security, audit]\n---\n\n# Cargo Dependencies\n\nAudit and management of Cargo dependencies and build configuration.\n\n## Audit Commands\n\nRun detailed dependency analysis:\n```bash\ncargo tree -d # Find duplicates\ncargo audit # Security vulnerabilities\ncargo outdated # Stale versions\ncargo deny check # Policy enforcement\n```\n\n## Dependency Evaluation\n\nCheck:\n- Feature flags usage\n- Optional dependencies\n- Build scripts safety\n- Binary size impact\n- Compilation time\n\n## Security Scanning\n\nReview for:\n- Known vulnerabilities\n- Abandoned crates\n- Unmaintained dependencies\n- Security advisories\n- Supply chain risks\n\n## Version Management\n\nVerify:\n- Semver compliance\n- Version pinning strategy\n- Dependency updates frequency\n- Breaking change handling\n\n## Common Issues\n\nFlag:\n- Abandoned crates\n- Excessively large dependencies\n- Security-vulnerable versions\n- Duplicate dependencies\n- Unnecessary dependencies\n\n## Alternatives Suggestion\n\nRecommend alternatives for:\n- Unmaintained crates\n- Heavy dependencies\n- Vulnerable versions\n- Better maintained options\n\n## Output Section\n\n```markdown\n## Dependencies\n### Security Issues\n- [crate@version] Vulnerability: [CVE/advisory]\n\n### Recommendations\n- Update [crate] from X to Y\n- Replace [abandoned-crate] with [alternative]\n- Remove unused dependency: [crate]\n```\n\nFile v1.9.14:modules/cfg-test-misuse.md\n\n---\nname: cfg-test-misuse\ndescription: Detection of #[cfg\n\nArchive v1.9.13: 19 files, 33402 bytes\n\nFiles: modules/async-slop.md (7937b), modules/builtin-preference.md (3751b), modules/cargo-dependencies.md (1516b), modules/cfg-test-misuse.md (1438b), modules/collection-types.md (1539b), modules/concurrency-patterns.md (5998b), modules/duplicate-validators.md (1559b), modules/error-handling.md (1405b), modules/error-messages.md (1459b), modules/iterator-and-allocation-slop.md (9416b), modules/model-specific-tells.md (7032b), modules/ownership-analysis.md (1469b), modules/silent-returns.md (1678b), modules/sql-injection.md (1520b), modules/test-slop.md (8018b), modules/unsafe-audit.md (3876b), skill-card.md (1960b), SKILL.md (4549b), _meta.json (142b)\n\nArchive v1.9.12: 19 files, 33502 bytes\n\nFiles: modules/async-slop.md (7937b), modules/builtin-preference.md (3751b), modules/cargo-dependencies.md (1516b), modules/cfg-test-misuse.md (1438b), modules/collection-types.md (1539b), modules/concurrency-patterns.md (5998b), modules/duplicate-validators.md (1559b), modules/error-handling.md (1405b), modules/error-messages.md (1459b), modules/iterator-and-allocation-slop.md (9416b), modules/model-specific-tells.md (7032b), modules/ownership-analysis.md (1469b), modules/silent-returns.md (1678b), modules/sql-injection.md (1520b), modules/test-slop.md (8018b), modules/unsafe-audit.md (3876b), skill-card.md (2211b), SKILL.md (4549b), _meta.json (142b)\n\nArchive v1.0.3: 19 files, 33634 bytes\n\nFiles: modules/async-slop.md (7937b), modules/builtin-preference.md (3751b), modules/cargo-dependencies.md (1516b), modules/cfg-test-misuse.md (1438b), modules/collection-types.md (1539b), modules/concurrency-patterns.md (5998b), modules/duplicate-validators.md (1559b), modules/error-handling.md (1405b), modules/error-messages.md (1459b), modules/iterator-and-allocation-slop.md (9416b), modules/model-specific-tells.md (7032b), modules/ownership-analysis.md (1469b), modules/silent-returns.md (1678b), modules/sql-injection.md (1520b), modules/test-slop.md (8018b), modules/unsafe-audit.md (3876b), skill-card.md (2519b), SKILL.md (4549b), _meta.json (141b)\n\nArchive v1.0.2: 15 files, 16943 bytes\n\nFiles: modules/builtin-preference.md (3751b), modules/cargo-dependencies.md (1516b), modules/cfg-test-misuse.md (1438b), modules/collection-types.md (1539b), modules/concurrency-patterns.md (5101b), modules/duplicate-validators.md (1559b), modules/error-handling.md (1405b), modules/error-messages.md (1459b), modules/ownership-analysis.md (1469b), modules/silent-returns.md (1678b), modules/sql-injection.md (1520b), modules/unsafe-audit.md (1701b), skill-card.md (2109b), SKILL.md (4690b), _meta.json (141b)\n\nArchive v1.0.1: 14 files, 15800 bytes\n\nFiles: modules/builtin-preference.md (3751b), modules/cargo-dependencies.md (1516b), modules/cfg-test-misuse.md (1438b), modules/collection-types.md (1539b), modules/concurrency-patterns.md (5101b), modules/duplicate-validators.md (1559b), modules/error-handling.md (1405b), modules/error-messages.md (1459b), modules/ownership-analysis.md (1469b), modules/silent-returns.md (1678b), modules/sql-injection.md (1520b), modules/unsafe-audit.md (1701b), SKILL.md (4690b), _meta.json (141b)\n\nArchive v1.0.0: 14 files, 15801 bytes\n\nFiles: modules/builtin-preference.md (3751b), modules/cargo-dependencies.md (1516b), modules/cfg-test-misuse.md (1438b), modules/collection-types.md (1539b), modules/concurrency-patterns.md (5101b), modules/duplicate-validators.md (1559b), modules/error-handling.md (1405b), modules/error-messages.md (1459b), modules/ownership-analysis.md (1469b), modules/silent-returns.md (1678b), modules/sql-injection.md (1520b), modules/unsafe-audit.md (1701b), SKILL.md (4690b), _meta.json (141b)","readmeExcerpt":"Skill: rust-review Owner: athola Summary: Audits Rust code for unsafe blocks, ownership issues, and Cargo dependency risks Tags: latest:1.9.19 Version history: v1.9.19 | 2026-08-26T13:19:16.072Z | user Release v1.9.19 v1.9.17 | 2026-07-30T05:39:27.389Z | user Release v1.9.17 v1.9.16 | 2026-07-14T19:56:10.799Z | user Release v1.9.16 v1.9.14 | 2026-06-30T18:04:28.776Z | user Release v1.9.14 v1.9.13 | 2026-06-27T16:22:2","codeSnippets":[],"executableExamples":[{"language":"bash","snippet":"/rust-review"},{"language":"markdown","snippet":"## Summary\nRust audit findings\n\n## Ownership Analysis\n[borrowing and lifetime issues]\n\n## Error Handling\n[error patterns and issues]\n\n## Concurrency\n[async and sync patterns]\n\n## Unsafe Audit\n### [U1] file:line\n- Invariants: [documented]\n- Risk: [assessment]\n- Recommendation: [action]\n\n## Dependencies\n[cargo audit results]\n\n## Recommendation\nApprove / Approve with actions / Block"},{"language":"rust","snippet":"// SLOP\nasync fn compute_total(items: &[Item]) -> u64 {\n items.iter().map(|i| i.price).sum()\n}"},{"language":"bash","snippet":"cargo clippy --all-targets -- -W clippy::async_yields_async"},{"language":"bash","snippet":"for f in $(rg -l \"async fn \" --type rust); do\n rg -q \"\\.await\" \"$f\" || echo \"no-await: $f\"\ndone"},{"language":"rust","snippet":"// SLOP\nasync fn read_config() -> Result {\n Ok(std::fs::read_to_string(\"config.toml\")?)\n}\n\n// SLOP\nasync fn rate_limit_wait() {\n std::thread::sleep(Duration::from_secs(1)); // blocks the runtime\n}\n\n// SLOP\nasync fn query_db(conn: &Connection) -> Result> {\n conn.query(\"SELECT ...\")? // blocking driver\n}"}],"parameters":null,"dependencies":[],"permissions":[],"extractedFiles":[{"path":"SKILL.md","content":"---\nname: rust-review\ndescription: Audits Rust code for unsafe blocks, ownership issues, and Cargo dependency risks\nversion: 1.9.8\ntriggers:\n - rust\n - ownership\n - concurrency\n - unsafe\n - traits\n - cargo\n - reviewing Rust code or before merging Rust changes\nmetadata: {\"openclaw\": {\"homepage\": \"https://github.com/athola/claude-night-market/tree/master/plugins/pensive\", \"emoji\": \"\\ud83d\\udd0d\", \"requires\": {\"config\": [\"night-market.pensive:shared\", \"night-market.imbue:proof-of-work\"]}}}\nsource: claude-night-market\nsource_plugin: pensive\n---\n\n> **Night Market Skill** — ported from [claude-night-market/pensive](https://github.com/athola/claude-night-market/tree/master/plugins/pensive). For the full experience with agents, hooks, and commands, install the Claude Code plugin.\n\n\n## Table of Contents\n\n- [Quick Start](#quick-start)\n- [When to Use](#when-to-use)\n- [Required TodoWrite Items](#required-todowrite-items)\n- [Progressive Loading](#progressive-loading)\n- [Core Workflow](#core-workflow)\n- [Rust Quality Checklist](#rust-quality-checklist)\n- [Safety](#safety)\n- [Correctness](#correctness)\n- [Performance](#performance)\n- [Idioms](#idioms)\n- [Output Format](#output-format)\n- [Summary](#summary)\n- [Ownership Analysis](#ownership-analysis)\n- [Error Handling](#error-handling)\n- [Concurrency](#concurrency)\n- [Unsafe Audit](#unsafe-audit)\n- [[U1] file:line](#[u1]-file:line)\n- [Dependencies](#dependencies)\n- [Recommendation](#recommendation)\n- [Exit Criteria](#exit-criteria)\n\n\n# Rust Review Workflow\n\nExpert-level Rust code audits with focus on safety, correctness, and idiomatic patterns.\n\n## Quick Start\n\n```bash\n/rust-review\n```\n**Verification:** Run the command with `--help` flag to verify availability.\n\n## When To Use\n\n- Reviewing Rust code changes\n- Auditing unsafe blocks\n- Analyzing concurrency patterns\n- Dependency security review\n- Performance optimization review\n\n## When NOT To Use\n\n- General code review without Rust - use unified-review\n- Performance profiling - use parseltongue:python-performance pattern\n\n## Required TodoWrite Items\n\n1. `rust-review:ownership-analysis`\n2. `rust-review:error-handling`\n3. `rust-review:concurrency`\n4. `rust-review:unsafe-audit`\n5. `rust-review:cargo-deps`\n6. `rust-review:evidence-log`\n\n## Progressive Loading\n\nLoad modules as needed based on review scope:\n\n**Quick Review** (ownership and errors):\n- See `modules/ownership-analysis.md` for borrowing and lifetime analysis\n- See `modules/error-handling.md` for Result/Option patterns\n\n**Concurrency Focus**:\n- See `modules/concurrency-patterns.md` for async and sync primitives\n\n**Safety Audit**:\n- See `modules/unsafe-audit.md` for unsafe block documentation\n\n**Dependency Review**:\n- See `modules/cargo-dependencies.md` for vulnerability scanning\n\n**Idiomatic Patterns**:\n- See `modules/builtin-preference.md` for conversion traits and builtin preference\n\n## Core Workflow\n\n1. **Ownership Analysis**: Check borrowing, lifetimes, clone patterns\n2. **Error Handling**: Verif"},{"path":"_meta.json","content":"{\n \"ownerId\": \"kn7d107jg9jv602h9ytsegydq184a42s\",\n \"slug\": \"nm-pensive-rust-review\",\n \"version\": \"1.9.19\",\n \"publishedAt\": 1787750356072\n}"},{"path":"modules/async-slop.md","content":"---\nmodule: async-slop\ncategory: detection\ndependencies: [Read, Grep]\nestimated_tokens: 500\n---\n\n# Async Slop\n\n**AI defaults to `async` and `tokio::spawn` even where\nsync code is faster, simpler, and correct.**\n\nThis module covers the high-frequency async patterns that\nlook idiomatic but are not. The clippy lints catch some;\nthe rest is structural.\n\n## Pattern 1: `async fn` that contains no `.await`\n\n```rust\n// SLOP\nasync fn compute_total(items: &[Item]) -> u64 {\n items.iter().map(|i| i.price).sum()\n}\n```\n\nIf the function body has no `.await`, it has no reason\nto be `async`. Async coloring is contagious: this\nfunction is callable only from async contexts, forcing\nevery caller to also be `async`. Strip `async` from the\nsignature unless the body actually awaits.\n\nDetection (preferred: clippy):\n\n```bash\ncargo clippy --all-targets -- -W clippy::async_yields_async\n```\n\nFile-level heuristic when clippy is unavailable:\n\n```bash\nfor f in $(rg -l \"async fn \" --type rust); do\n rg -q \"\\.await\" \"$f\" || echo \"no-await: $f\"\ndone\n```\n\n(Heuristic; manual review needed since `.await` may be in\na helper called by the async fn rather than inline.)\n\n## Pattern 2: blocking I/O inside an async runtime\n\n```rust\n// SLOP\nasync fn read_config() -> Result {\n Ok(std::fs::read_to_string(\"config.toml\")?)\n}\n\n// SLOP\nasync fn rate_limit_wait() {\n std::thread::sleep(Duration::from_secs(1)); // blocks the runtime\n}\n\n// SLOP\nasync fn query_db(conn: &Connection) -> Result> {\n conn.query(\"SELECT ...\")? // blocking driver\n}\n```\n\nBlocking calls inside `async` block the entire executor\nthread, defeating the runtime's concurrency model.\n\nFix:\n\n```rust\n// Use the async equivalent\nasync fn read_config() -> Result {\n Ok(tokio::fs::read_to_string(\"config.toml\").await?)\n}\n\n// Or wrap blocking work in spawn_blocking\nasync fn rate_limit_wait() {\n tokio::time::sleep(Duration::from_secs(1)).await;\n}\n\n// For unavoidable blocking work\nasync fn query_db(conn: Arc) -> Result> {\n let conn = conn.clone();\n tokio::task::spawn_blocking(move || conn.query(\"SELECT ...\"))\n .await?\n}\n```\n\nDetection:\n\n```bash\n# Find blocking ops inside async functions (heuristic)\nrg -B 5 \"(std::fs::|std::thread::sleep|std::net::TcpStream)\" --type rust |\n rg -B 5 \"async fn\"\n```\n\n## Pattern 3: `tokio::spawn` for synchronous-equivalent work\n\n```rust\n// SLOP\nasync fn handle_request(req: Request) -> Response {\n let result = tokio::spawn(async move {\n compute_response(&req)\n }).await.unwrap();\n result\n}\n```\n\nSpawning a task only to immediately await its single\ncompletion is equivalent to a direct call, plus the\noverhead of task creation, scheduling, and a join. Just\ncall the function:\n\n```rust\nasync fn handle_request(req: Request) -> Response {\n compute_response(&req)\n}\n```\n\n`tokio::spawn` is for *concurrent* work: when the\nspawned task should make progress while the caller does\nsomething else, or when the task should outlive the\nca"},{"path":"modules/builtin-preference.md","content":"---\nname: builtin-preference\ndescription: Detection of helper functions that should be standard trait\n implementations and reimplemented Rust builtins\ncategory: rust-review\ntags: [from, into, tryfrom, fromstr, default, display, iterator, idioms]\n---\n\n# Builtin Preference\n\nDetects custom helper functions that duplicate Rust's standard\ntrait system and built-in combinators.\n\n## What This Detects\n\nFour categories of anti-patterns:\n\n1. **Conversion helpers**: `parse_foo()`, `foo_from_bar()`,\n `convert_*()`, `to_*(&self)` that should be `FromStr`,\n `From`, `TryFrom`, or `Into` implementations\n2. **Standard trait replacements**: `default_config()`,\n `format_error()`, `as_bytes(&self)`, `compare()` that\n should be `Default`, `Display`, `AsRef`, or `PartialEq`\n3. **Error conversion wrappers**: `io_to_my_error()`,\n `wrap_error()` that should be `impl From` or\n thiserror `#[from]`\n4. **Manual combinators**: `match opt { Some(x) => Some(f(x)),\n None => None }` that should be `.map()`, `.unwrap_or()`,\n `.flatten()`, etc.\n\n## Why It Matters\n\nRust's trait system is compositional by design:\n\n- `impl From for B` gives `impl Into for A` for free\n- `impl Display` gives `ToString` for free\n- `From` enables the `?` operator for error propagation\n- Trait impls participate in generic bounds and blanket impls\n- Standard combinators are optimized and well-tested\n\nHelper functions that bypass this system create API\ninconsistency, miss ergonomic benefits, and signal\nunfamiliarity with idiomatic Rust.\n\n## Safe Patterns\n\n```rust\n// Good: From trait enables .into() and ? operator\nimpl From for Settings {\n fn from(c: Config) -> Self {\n Settings { timeout: c.timeout }\n }\n}\n\n// Good: FromStr enables .parse()\nimpl FromStr for Config {\n type Err = ConfigError;\n fn from_str(s: &str) -> Result { ... }\n}\n\n// Good: Default via derive\n#[derive(Default)]\nstruct Config { timeout: u64 }\n\n// Good: Display for human-readable output\nimpl fmt::Display for MyError {\n fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {\n write!(f, \"Error: {}\", self.msg)\n }\n}\n\n// Good: Option combinators\nlet result = opt.map(|x| x.to_string());\nlet value = opt.unwrap_or(default);\n```\n\n## Patterns to Flag\n\n```rust\n// Flag: should be impl FromStr\nfn parse_config(s: &str) -> Config { ... }\n\n// Flag: should be impl From for Foo\nfn foo_from_bar(b: Bar) -> Foo { ... }\n\n// Flag: should be impl Default\nfn default_config() -> Config { ... }\n\n// Flag: should be impl From for MyError\nfn io_to_my_error(e: io::Error) -> MyError { ... }\n\n// Flag: should use .map()\nmatch opt {\n Some(x) => Some(x.to_string()),\n None => None,\n}\n```\n\n## Exclusions (Not Flagged)\n\n- Lossy conversions (`to_lossy_ascii`)\n- Builder methods (`with_timeout(self, ...)`)\n- Multi-parameter conversions (context-dependent)\n- Domain-specific operations (`serialize`, `encode`, `decode`)\n\n## Related Clippy Lints\n\n| Lint | Detects |\n|------|---------|\n"},{"path":"modules/cargo-dependencies.md","content":"---\nname: cargo-dependencies\ndescription: Dependency auditing, security scanning, and version management\ncategory: rust-review\ntags: [cargo, dependencies, security, audit]\n---\n\n# Cargo Dependencies\n\nAudit and management of Cargo dependencies and build configuration.\n\n## Audit Commands\n\nRun detailed dependency analysis:\n```bash\ncargo tree -d # Find duplicates\ncargo audit # Security vulnerabilities\ncargo outdated # Stale versions\ncargo deny check # Policy enforcement\n```\n\n## Dependency Evaluation\n\nCheck:\n- Feature flags usage\n- Optional dependencies\n- Build scripts safety\n- Binary size impact\n- Compilation time\n\n## Security Scanning\n\nReview for:\n- Known vulnerabilities\n- Abandoned crates\n- Unmaintained dependencies\n- Security advisories\n- Supply chain risks\n\n## Version Management\n\nVerify:\n- Semver compliance\n- Version pinning strategy\n- Dependency updates frequency\n- Breaking change handling\n\n## Common Issues\n\nFlag:\n- Abandoned crates\n- Excessively large dependencies\n- Security-vulnerable versions\n- Duplicate dependencies\n- Unnecessary dependencies\n\n## Alternatives Suggestion\n\nRecommend alternatives for:\n- Unmaintained crates\n- Heavy dependencies\n- Vulnerable versions\n- Better maintained options\n\n## Output Section\n\n```markdown\n## Dependencies\n### Security Issues\n- [crate@version] Vulnerability: [CVE/advisory]\n\n### Recommendations\n- Update [crate] from X to Y\n- Replace [abandoned-crate] with [alternative]\n- Remove unused dependency: [crate]\n```"}],"languages":[],"docsSourceLabel":"CLAWHUB","editorialOverview":"Audits Rust code for unsafe blocks, ownership issues, and Cargo dependency risks Skill: rust-review Owner: athola Summary: Audits Rust code for unsafe blocks, ownership issues, and Cargo dependency risks Tags: latest:1.9.19 Version history: v1.9.19 | 2026-08-26T13:19:16.072Z | user Release v1.9.19 v1.9.17 | 2026-07-30T05:39:27.389Z | user Release v1.9.17 v1.9.16 | 2026-07-14T19:56:10.799Z | user Release v1.9.16 v1.9.14 | 2026-06-30T18:04:28.776Z | user Release v1.9.14 v1.9.13 | 2026-06-27T16:22:2","editorialQuality":{"score":100,"threshold":65,"status":"ready","wordCount":1309,"uniquenessScore":52,"reasons":[]}},"media":{"evidence":{"source":"no-media","verified":false,"confidence":"low","updatedAt":"2026-10-10T09:05:39.139Z","emptyReason":"No screenshots, media assets, or demo links are available."},"primaryImageUrl":null,"mediaAssetCount":0,"assets":[],"demoUrl":null},"ownerResources":{"evidence":{"source":"unclaimed","verified":false,"confidence":"low","updatedAt":"2026-10-10T09:05:39.139Z","emptyReason":"This page has not been claimed by the agent owner."},"hasCustomPage":false,"customPageUpdatedAt":null,"customLinks":[],"structuredLinks":{"docsUrl":null,"demoUrl":null,"supportUrl":null,"pricingUrl":null,"statusUrl":null},"customPage":null},"relatedAgents":{"evidence":{"source":"protocol-neighbors","verified":false,"confidence":"medium","updatedAt":"2026-10-10T11:54:01.011Z","emptyReason":null},"items":[{"id":"8ebccd8e-3863-4187-8355-c3f14e1f9edf","entityType":"agent","canonicalPath":"/agent/iofficeai-aionui","slug":"iofficeai-aionui","name":"AionUi","description":"Free, local, open-source 24/7 Cowork app and OpenClaw for Gemini CLI, Claude Code, Codex, OpenCode, Qwen Code, Goose CLI, Auggie, and more | 🌟 Star if you like it!","url":"https://github.com/iOfficeAI/AionUi","homepage":"https://www.aionui.com","source":"GITHUB_REPOS","protocols":["MCP","OPENCLAW"],"capabilities":[],"safetyScore":100,"overallRank":70,"updatedAt":"2026-10-09T19:11:12.944Z","createdAt":"2026-02-25T03:38:16.584Z","downloads":null},{"id":"b917f68a-ebff-438e-84f8-3f4b2494c0bc","entityType":"agent","canonicalPath":"/agent/activepieces-activepieces","slug":"activepieces-activepieces","name":"activepieces","description":"AI Agents & MCPs & AI Workflow Automation • (~400 MCP servers for AI agents) • AI Automation / AI Agent with MCPs • AI Workflows & AI Agents • MCPs for AI Agents","url":"https://github.com/activepieces/activepieces","homepage":"https://www.activepieces.com","source":"GITHUB_REPOS","protocols":["OPENCLAW"],"capabilities":[],"safetyScore":100,"overallRank":70,"updatedAt":"2026-04-15T02:22:12.426Z","createdAt":"2026-02-25T03:38:12.412Z","downloads":null},{"id":"5cb26759-3a39-483f-94cf-276a98c13bb8","entityType":"agent","canonicalPath":"/agent/cherryhq-cherry-studio","slug":"cherryhq-cherry-studio","name":"cherry-studio","description":"AI productivity studio with smart chat, autonomous agents, and 300+ assistants. Unified access to frontier LLMs","url":"https://github.com/CherryHQ/cherry-studio","homepage":"https://cherry-ai.com","source":"GITHUB_REPOS","protocols":["MCP","OPENCLAW"],"capabilities":[],"safetyScore":100,"overallRank":70,"updatedAt":"2026-04-11T14:38:40.986Z","createdAt":"2026-02-25T03:38:19.379Z","downloads":null},{"id":"6f6582d0-5d76-4f0f-b81d-86520247950b","entityType":"agent","canonicalPath":"/agent/copilotkit-copilotkit","slug":"copilotkit-copilotkit","name":"CopilotKit","description":"The Frontend for Agents & Generative UI. React + Angular","url":"https://github.com/CopilotKit/CopilotKit","homepage":"https://docs.copilotkit.ai","source":"GITHUB_REPOS","protocols":["OPENCLAW"],"capabilities":[],"safetyScore":100,"overallRank":70,"updatedAt":"2026-03-25T09:50:57.846Z","createdAt":"2026-02-25T03:39:14.617Z","downloads":null}],"links":{"hub":"/agent","source":"/agent/source/clawhub","protocols":[{"label":"OpenClaw","href":"/agent/protocol/openclew"}]}}}