## Code Quality

#### `cmd_similar` JSON output emits 3 fields; batch `dispatch_similar` emits 9 — silent parity drop in the canonical SearchResult shape
- **Difficulty:** easy
- **Location:** `src/cli/batch/handlers/info.rs:139-148` (batch path), vs `src/cli/display.rs:397-413` (CLI path via `display_similar_results_json`) which routes through `cqs::store::SearchResult::to_json()` at `src/store/helpers/types.rs:143-156`
- **Description:** The CLI `cmd_similar` json branch calls `display_similar_results_json` → `r.to_json()`, which emits the canonical 9-field shape (`file, line_start, line_end, name, signature, language, chunk_type, score, content, has_parent`). The comment at display.rs:401-402 explicitly says "Delegate to SearchResult::to_json() for canonical base keys. Previously missing `type` and `has_parent` (CQ-NEW-5)." The batch path's `dispatch_similar` re-rolls JSON inline as `{name, file, score}` — only 3 fields, missing line numbers, signature, language, chunk_type, content, has_parent. Net effect: agents get a different schema for `cqs similar Foo` depending on whether the daemon is up. CQ-V1.29-3 fixed the *resolution* divergence here; this is the JSON-shape sister bug, same source.
- **Suggested fix:** In `dispatch_similar` at `src/cli/batch/handlers/info.rs:139-148`, replace the manual `serde_json::json!` map with `filtered.iter().map(|r| r.to_json()).collect::<Vec<_>>()`. Same line count, same envelope (`{results, target, total}`), now schema-identical to the CLI path. Add a snapshot test that asserts CLI and batch produce identical key sets for the same query.

#### `Reranker::new` silently ignores the `[reranker]` config section — `resolve_reranker(None)` is the only call site
- **Difficulty:** easy
- **Location:** `src/reranker.rs:127-154` (`Reranker::new`), `src/reranker.rs:61-77` (`resolve_reranker`), `src/reranker.rs:442-446` (`model_paths` calls `resolve_reranker(None)`)
- **Description:** `resolve_reranker` is the entire precedence chain documented at `:59-60`: "CLI → `CQS_RERANKER_MODEL` → `[reranker] model_path` → `[reranker] preset` → hardcoded `ms-marco-minilm`." Its signature accepts `section: Option<&AuxModelSection>` to thread `Config::reranker` (defined at `src/config.rs:228`) into the resolver. But `model_paths` always passes `None`. `Reranker::new()` doesn't accept a config at all; it only reads `CQS_RERANKER_MAX_LENGTH` and `CQS_RERANKER_MODEL` (via env inside resolve_reranker). Net effect: a user who writes `[reranker] preset = "bge-reranker-base"` in `.cqs.toml` gets the default ms-marco-MiniLM with zero error or warning — silent config drop, exactly the class of bug CLAUDE.md flags as "Docs Lying Is P1." The `section` parameter on `resolve_reranker` is also dead code as long as the only caller passes `None`.
- **Suggested fix:** Add `Reranker::with_section(section: Option<&AuxModelSection>)` (or change `new` to accept it) and thread `&config.reranker` through from the two production call sites (`src/cli/batch/mod.rs:1274`, `src/cli/store.rs:276` — both already have `Config` in scope). Inside `model_paths`, store the section on `self` (or pass through) and call `resolve_reranker(self.section.as_ref())` instead of `None`. Same pattern SPLADE already uses.

#### `dispatch_diff` builds a `target_store` placeholder that's never read in its else branch
- **Difficulty:** easy
- **Location:** `src/cli/batch/handlers/misc.rs:354-364, 367-387`
- **Description:** Lines 354-364 set `target_store` via `if/else`. The `if target_label == "project"` arm correctly captures `&ctx.store()`. The `else` arm calls `ctx.get_ref(target_label)?` (caches the reference store in BatchContext), then sets `target_store = &ctx.store()` with a comment "placeholder -- replaced below." But "below" at lines 367-387 is a *second* `if target_label == "project"` that only consumes `target_store` in the project branch. The else branch at line 376-387 never reads `target_store` — it calls `resolve_reference_store` afresh at line 378, opening yet another Store handle and bypassing the `get_ref` cache that line 360 just populated. So: (1) wasted I/O — the `get_ref` cached store is loaded then discarded; (2) dead-variable initialization in else; (3) a duplicate match on `target_label` that's structurally inevitable. Easy to misread and easy to break the cache invariant on the next refactor.
- **Suggested fix:** Collapse to one match. Either: (a) use `get_ref` + `borrow_ref` for both project and ref targets, since `borrow_ref` returns `Option<RefMut<RefIndex>>` and you can take `&store` from inside the borrow's scope — `cqs::semantic_diff` is fully synchronous and the borrow lives long enough; or (b) drop the `get_ref` call entirely from the else branch and let `resolve_reference_store` own the lifetime, removing the placeholder. Either way: one `if target_label == "project"` block, no placeholder variable.

#### Hardcoded "200" in user-facing gather warning lies when `CQS_GATHER_MAX_NODES` is set
- **Difficulty:** easy
- **Location:** `src/cli/commands/search/gather.rs:200` (the println), `src/gather.rs:153-172` (`gather_max_nodes` with env override)
- **Description:** The text-mode warning is `println!("{}", "Warning: expansion capped at 200 nodes".yellow());` — a string literal. The actual cap is `gather_max_nodes()` which honors `CQS_GATHER_MAX_NODES` (logged at gather.rs:161 as "BFS node cap overridden via CQS_GATHER_MAX_NODES"). With `CQS_GATHER_MAX_NODES=500` the user sees "capped at 200" while results were actually capped at 500 — directly contradicts the env var the same module advertises in its tracing message. This is the exact "limit advertised ≠ limit applied" footgun that bites agents debugging "why am I getting only X results."
- **Suggested fix:** Either capture the cap on `GatherResult` (add `pub expansion_cap_used: usize` next to `expansion_capped`) and format the message with the real number, or look up `gather_max_nodes()` again at warn time. Latter is simpler and `gather_max_nodes` already memoizes via OnceLock. Prefer the former because it makes the cap visible in `--json` output too, where agents can react to it.

#### Embedding/Query cache `open_with_runtime` are ~80% copy-paste — 90+ duplicated lines in cache.rs
- **Difficulty:** medium
- **Location:** `src/cache.rs:103-220` (`EmbeddingCache::open_with_runtime`) and `src/cache.rs:1412-1522` (`QueryCache::open_with_runtime`)
- **Description:** Both methods do, in identical order: (1) `info_span!`, (2) `create_dir_all` parent + `#[cfg(unix)] set_permissions(parent, 0o700)` with identical "best effort" warn block (16 lines × 2), (3) Tokio runtime fallback (`if let Some(rt) = runtime { rt } else { Builder::new_current_thread()... }` — 9 lines × 2), (4) `SqliteConnectOptions` + `SqlitePoolOptions::new().max_connections(1).idle_timeout(30s).connect_with(opts)` — only `busy_timeout(5000 vs 2000)` differs, (5) `CREATE TABLE IF NOT EXISTS` (different schemas), (6) `0o600` chmod loop on `["", "-wal", "-shm"]` with identical comment shape (22 lines × 2). Total: ~90 lines duplicated. The two `Drop` impls (`:723-754`, `:1717-1745`) repeat the same panic-message extraction (4 lines × 2). Bug surface: any future hardening of the chmod block, or change to runtime construction, has to be applied twice. PB-V1.29-7 already noticed both blocks separately — the extracted helper is the structural fix.
- **Suggested fix:** Extract three private helpers in `cache.rs` (or `store/helpers/sql.rs` next to `busy_timeout_from_env`): `prepare_cache_dir_perms(parent: &Path)` (parent chmod), `apply_db_file_perms(path: &Path)` (the 0o600 loop), and `connect_cache_pool(path, busy_ms, runtime, schema_sql)` (the 4-step open). Both `EmbeddingCache::open_with_runtime` and `QueryCache::open_with_runtime` collapse to ~30 lines each. The two `Drop` impls share `extract_panic_msg(payload)` — same shape as the existing `cli/pipeline/mod.rs::panic_message` (see next finding).

#### `panic_message` helper duplicated 4 ways across 3 modules — `cli/pipeline/mod.rs::panic_message` and 3 inline copies in Drop impls
- **Difficulty:** easy
- **Location:** `src/cli/pipeline/mod.rs:223-232` (`fn panic_message`), `src/store/mod.rs:1322-1326` (Store::drop), `src/cache.rs:743-747` (EmbeddingCache::drop), `src/cache.rs:1735-1739` (QueryCache::drop)
- **Description:** Four copies of the same `payload.downcast_ref::<&str>().or(downcast_ref::<String>()).unwrap_or("unknown panic")` extraction. The pipeline version is a free function; the three Drop versions inline it. Functions return `String` vs `&str`, but the logic is identical. Anyone tightening the panic-extraction (e.g. adding `Box<dyn Error>` or a `format!` of the type id when both downcasts fail) has to update four sites. Low-risk debt but easy to delete.
- **Suggested fix:** Promote `panic_message` to `pub(crate) fn` in a single common module (`src/lib.rs` next to `temp_suffix`, or a new `src/panic_msg.rs`). Make all four sites use it. The Drop sites currently take `&Box<dyn Any + Send>`; harmonize on that signature so all callers fit. Drop the pipeline-private version.

#### Repeated `match std::env::var("CQS_*") { Ok(v) => v.parse()... }` pattern at 25+ sites — `limits::parse_env_*` helpers exist but are pub(crate)-private
- **Difficulty:** medium
- **Location:** `src/limits.rs:230-260` (`parse_env_f32` / `parse_env_usize` / `parse_env_u64`); duplicated open-coded equivalents in: `src/cli/watch.rs:65,74,100,498,510,766,942,1430` (8 sites), `src/llm/mod.rs:176,315,406,434` (4 sites), `src/cli/pipeline/types.rs:80,98,117,144` (4 sites), `src/hnsw/persist.rs:19,41,63` (3 sites), `src/embedder/models.rs:565,571`, `src/embedder/mod.rs:330`, `src/cache.rs:206,1509`, `src/gather.rs:156`, `src/cli/commands/graph/trace.rs:357`, `src/impact/bfs.rs:16`, `src/reranker.rs:129`
- **Description:** The library limits module (`src/limits.rs:224`) clearly notes "shared parsing helpers" with three carefully-tested fns covering `f32 / usize / u64` shapes. They reject zero, garbage, and non-finite values uniformly, with consistent test coverage. But the helpers are `pub(crate)` — visible only inside `cqs::limits` callers. Every other module that wants the same behavior re-rolls the pattern, frequently with slight variations: some accept `0`, some don't; some warn on bad input, most don't; some clamp, some don't. `EmbeddingCache::open_with_runtime` accepts `CQS_CACHE_MAX_SIZE=0` (silently sets cap to 0 → always-evict mode), while `QueryCache` rejects `0` (`.filter(|&n: &u64| n > 0)`) and falls back to default. Same env-shape, opposite behavior. Standardizing fixes this and shrinks the codebase ~150 lines. (See related finding on `CQS_CACHE_MAX_SIZE` zero-handling below.)
- **Suggested fix:** Move `parse_env_usize` / `_u64` / `_f32` to `src/lib.rs` (or new `src/env.rs`) as `pub fn`, add a `parse_env_duration_secs` variant for `Duration` cases (`local_timeout` etc.), and do a focused sweep replacing the open-coded sites. Drop the `limits.rs` private copies, re-export from there for convenience. One `pub fn` change + ~25 site edits.

#### `EmbeddingCache::open_with_runtime` accepts `CQS_CACHE_MAX_SIZE=0`; `QueryCache::open_with_runtime` rejects it — opposite behavior for sister env vars
- **Difficulty:** easy
- **Location:** `src/cache.rs:206-209` (Embedding, no zero filter), `src/cache.rs:1509-1513` (Query, `.filter(|&n: &u64| n > 0)`)
- **Description:** Side-by-side in the same file, two sibling caches handle their `CQS_*_MAX_SIZE` env knob differently. `EmbeddingCache` does `env::var(...).ok().and_then(|s| s.parse().ok()).unwrap_or(10 * 1024^3)` — `CQS_CACHE_MAX_SIZE=0` parses fine to `0`, sets `max_size_bytes = 0`, and `evict()` then aggressively evicts everything to fit under 0 bytes. `QueryCache` adds `.filter(|&n: &u64| n > 0)` so `CQS_QUERY_CACHE_MAX_SIZE=0` falls back to the 100 MB default. Whichever you intended — uniform fallback, or "0 disables the cache" — picking only one accidentally is the wrong outcome. This is the exact class of inconsistency the proposed shared `parse_env_u64` helper above closes.
- **Suggested fix:** Pick a semantic and document it. If "0 disables": uniformize both to accept 0, document, and have `evict()` short-circuit when `max_size_bytes == 0`. If "0 is invalid, use default": add `.filter(|&n: &u64| n > 0)` to the EmbeddingCache parse at `src/cache.rs:208`. Either way, drive the parse through one helper and write the test once.

#### `cli/commands/resolve.rs::find_reference` and the inline lookup inside `resolve_reference_db` re-roll the same "find ref by name + nice error" twice
- **Difficulty:** easy
- **Location:** `src/cli/commands/resolve.rs:26-39` (`find_reference`), `src/cli/commands/resolve.rs:46-57` (inline `references.iter().find(|r| r.name == name)` with byte-identical error message)
- **Description:** Both functions: (1) call `Config::load(root)`, (2) iterate `config.references`, (3) `.find(|r| r.name == name)`, (4) return `anyhow::anyhow!("Reference '{}' not found. Run 'cqs ref list' to see available references.", name)` on miss. The only difference is `find_reference` returns the full `ReferenceIndex` (loaded via `reference::load_references`), while `resolve_reference_db` only needs the `ReferenceConfig.path`. The error-message duplication is verbatim — change "Run 'cqs ref list'" to anything else and one call site silently lies. Three other call sites across `resolve_reference_store` / `resolve_reference_store_readonly` chain through `resolve_reference_db`, so the bug surfaces on any of them.
- **Suggested fix:** Extract `find_reference_config(config: &Config, name: &str) -> Result<&ReferenceConfig>` returning the typed config row (no IO, just the find + message). `find_reference` then becomes `let cfg = find_reference_config(&config, name)?; reference::load_references(slice::from_ref(cfg)).into_iter().next().ok_or(...)`; `resolve_reference_db` uses the same helper to read `cfg.path`. One source of truth for the error string.

#### `slot::libc_exdev` hardcodes errno 18 with comment claiming `libc` would be "just for this constant" — but `libc` is already a workspace dep
- **Difficulty:** easy
- **Location:** `src/slot/mod.rs:640-647`
- **Description:** Comment (lines 640-643): "We hardcode 18 (Linux) since `libc::EXDEV` would pull in a libc dep just for this constant. macOS also uses 18; Windows doesn't surface EXDEV the same way." `libc = "0.2"` is in `Cargo.toml` and already imported in `src/cli/watch.rs` (`extern "C" fn on_sigterm(_sig: libc::c_int)`). The hardcode is correct in practice (Linux + macOS both define EXDEV=18; FreeBSD too), but the *justification* is wrong, and a future maintainer reading the comment will assume libc isn't available. Worse, on the unlikely platform where EXDEV diverges (some BSD variants in theory), the hardcoded `18` silently mis-classifies the error and the EXDEV → copy+remove fallback in `move_file` skips. Self-documenting fix: use `libc::EXDEV` and delete the comment.
- **Suggested fix:** Replace the `fn libc_exdev() -> i32 { 18 }` shim with `libc::EXDEV` directly at the call site in `src/slot/mod.rs:631`. `#[cfg(unix)]` already gates the EXDEV branch implicitly via `raw_os_error()` returning a Linux/macOS errno; the `libc` dep is unconditionally available on those targets through the existing watch.rs usage. Remove the misleading comment.
