# P3 + P4 fix prompts — v1.30.0 audit

Inputs: `docs/audit-triage.md` + `docs/audit-findings.md`. P3 are minimal Edit-style fix prompts. P4 are paste-ready GitHub issue bodies.

---

## P3.1 — Hoist `panic_message` helper into one place

**Finding:** P3.1
**Files:** `src/cli/pipeline/mod.rs:223-232`, `src/store/mod.rs:1322-1326`, `src/cache.rs:743-747`, `src/cache.rs:1735-1739`
**Why:** Four copies of identical panic-payload extraction logic across 3 modules. Make it `pub(crate)` and use it everywhere.

### Current code
```rust
// src/cli/pipeline/mod.rs:223-232 — pub(crate)? actually `fn` (private)
fn panic_message(payload: &Box<dyn std::any::Any + Send>) -> String {
    if let Some(s) = payload.downcast_ref::<&str>() {
        (*s).to_string()
    } else if let Some(s) = payload.downcast_ref::<String>() {
        s.clone()
    } else {
        "unknown panic".to_string()
    }
}
```
Plus 3 inline copies inside `Drop` impls (`Store::drop`, `EmbeddingCache::drop`, `QueryCache::drop`) — each a 4-arm `match payload.downcast_ref::<&str>()` ladder.

### Replacement
1. Promote `panic_message` to `pub(crate) fn` in `src/lib.rs` (next to `temp_suffix`) keeping the same signature `&Box<dyn Any + Send> -> String`.
2. Delete the private function in `src/cli/pipeline/mod.rs`; replace the 3 inline copies in the Drop impls with `crate::panic_message(payload)`.

---

## P3.2 — Extract one `find_reference_config` helper for resolve.rs

**Finding:** P3.2
**Files:** `src/cli/commands/resolve.rs:26-39, 46-57`
**Why:** `find_reference` and `resolve_reference_db` re-roll the same `iter().find(|r| r.name == name)` + verbatim error message. Single source of truth.

### Current code
```rust
// resolve.rs:26-39  find_reference
let cfg = config.references.iter()
    .find(|r| r.name == name)
    .ok_or_else(|| anyhow::anyhow!(
        "Reference '{}' not found. Run 'cqs ref list' to see available references.", name
    ))?;
// ...load_references for full ReferenceIndex

// resolve.rs:46-57  resolve_reference_db (inline duplicate)
let cfg = config.references.iter()
    .find(|r| r.name == name)
    .ok_or_else(|| anyhow::anyhow!(
        "Reference '{}' not found. Run 'cqs ref list' to see available references.", name
    ))?;
// uses cfg.path
```

### Replacement
Add a private helper at the top of `resolve.rs`:
```rust
fn find_reference_config<'a>(
    config: &'a Config,
    name: &str,
) -> anyhow::Result<&'a ReferenceConfig> {
    config.references.iter()
        .find(|r| r.name == name)
        .ok_or_else(|| anyhow::anyhow!(
            "Reference '{}' not found. Run 'cqs ref list' to see available references.", name
        ))
}
```
Call it from both sites.

---

## P3.3 + P3.19 — `slot::libc_exdev` hardcode (combined)

**Finding:** P3.3 (cosmetic) + P3.19 (Windows wrong constant)
**Files:** `src/slot/mod.rs:628-647`
**Why:** The `libc_exdev() -> 18` shim is justified by an outdated comment (libc is already a workspace dep), AND it mis-identifies the cross-device error on Windows (`ERROR_NOT_SAME_DEVICE = 17`). Drop the magic number entirely and fall back to copy+remove on any rename failure.

### Current code
```rust
// src/slot/mod.rs:628-647
pub(crate) fn move_file(src: &Path, dst: &Path) -> std::io::Result<()> {
    match fs::rename(src, dst) {
        Ok(()) => Ok(()),
        Err(e) if e.raw_os_error() == Some(libc_exdev()) => {
            fs::copy(src, dst)?;
            fs::remove_file(src)?;
            Ok(())
        }
        Err(e) => Err(e),
    }
}

/// EXDEV `errno` value (cross-device link). 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 (rename across
/// filesystems just succeeds via the win32 API).
#[inline]
fn libc_exdev() -> i32 { 18 }
```

### Replacement
```rust
pub(crate) fn move_file(src: &Path, dst: &Path) -> std::io::Result<()> {
    match fs::rename(src, dst) {
        Ok(()) => Ok(()),
        // Fall back to copy+remove on ANY rename failure (cross-device,
        // ERROR_NOT_SAME_DEVICE on Windows, EXDEV on Unix). Cheaper than
        // tracking platform-specific errno constants — if the source is
        // gone after the copy, callers see the I/O error from copy() instead.
        Err(_) => {
            fs::copy(src, dst)?;
            fs::remove_file(src)?;
            Ok(())
        }
    }
}
```
Delete `fn libc_exdev()` entirely.

---

## P3.4 — Doc: `enumerate_files` honors .cqsignore too

**Finding:** P3.4
**Files:** `src/lib.rs:542-547`
**Why:** Public-API doc comment claims gitignore-only; body adds `.cqsignore` when `no_ignore=false`.

### Current code
```rust
/// Enumerate files to index in a project directory.
///
/// Respects .gitignore, skips hidden files and files larger than
/// `CQS_MAX_FILE_SIZE` bytes (default 1MB — generated code can exceed this).
/// Returns relative paths from the project root.
///
/// Shared file enumeration for consistent indexing.
pub fn enumerate_files(
```

### Replacement
```rust
/// Enumerate files to index in a project directory.
///
/// Respects `.gitignore` and `.cqsignore` (additive on top of `.gitignore`,
/// both disabled by `no_ignore=true`); skips hidden files and files larger
/// than `CQS_MAX_FILE_SIZE` bytes (default 1 MiB — generated code can
/// exceed this). Returns relative paths from the project root.
pub fn enumerate_files(
```

---

## P3.5 — Drop dead `generate_nl_with_call_context` from public API

**Finding:** P3.5
**Files:** `src/lib.rs:165` (`pub use nl::*`); `src/nl/mod.rs:43-59`
**Why:** Five-arg wrapper that hardcodes `summary=None, hyde=None`. Zero production callers, only test references; leaks via glob re-export.

### Current code
```rust
// src/nl/mod.rs:43-59
pub fn generate_nl_with_call_context(
    chunk: &Chunk, callers: &[String], callees: &[String],
    note: Option<&str>, template: NlTemplate,
) -> String {
    generate_nl_with_call_context_and_summary(
        chunk, callers, callees, note, /*summary=*/ None,
        /*hyde=*/ None, template,
    )
}
```

### Replacement
Delete the wrapper. Update tests in `src/nl/mod.rs` that call it to call `generate_nl_with_call_context_and_summary(.., None, None, ..)` directly. Optionally tighten the glob: replace `pub use nl::*` in `src/lib.rs:165` with an explicit `pub use nl::{NlTemplate, generate_nl_description, generate_nl_with_template, generate_nl_with_call_context_and_summary};`.

---

## P3.6 — Rename `GatherArgs::expand` to `--depth`

**Finding:** P3.6
**Files:** `src/cli/args.rs:GatherArgs::expand`
**Why:** Top-level `--expand-parent` (bool) and `cqs gather --expand <N>` (usize) collide; v1.30.0 only half-fixed the rename. Align `gather` with `onboard`/`impact`/`test-map` which already use `--depth`.

### Current code
```rust
// in GatherArgs (src/cli/args.rs)
#[arg(long, default_value = "2")]
pub expand: usize,
```

### Replacement
```rust
/// Call-graph BFS depth for gather expansion (matches onboard/impact/test-map).
#[arg(long, default_value = "2", visible_alias = "expand")]
pub depth: usize,
```
Sweep references to `args.expand` in `src/cli/commands/search/gather.rs` to `args.depth`.

---

## P3.7 — `cqs eval --save` requires `.json`

**Finding:** P3.7
**Files:** `src/cli/commands/eval/mod.rs` (`EvalCmdArgs::save`); call site that opens the file
**Why:** Accepts any path; eval reports are JSON-only. Asymmetric with `--baseline` which already requires the file exist.

### Current code
```rust
// EvalCmdArgs::save: Option<PathBuf>  — no validation
// In the runner: File::create(&save_path)?
```

### Replacement
At the runner's open site (or top of `cmd_eval`):
```rust
let save_path = args.save.as_deref().map(|p| {
    let ext = p.extension().and_then(|e| e.to_str());
    match ext {
        Some("json") => Ok(p.to_path_buf()),
        Some(other) => anyhow::bail!(
            "--save must end in .json (got .{other}); eval reports are JSON-only"
        ),
        None => {
            let with_ext = p.with_extension("json");
            tracing::info!(path = %with_ext.display(), "appending .json to --save path");
            Ok(with_ext)
        }
    }
}).transpose()?;
```

---

## P3.8 — Eval runner: `eprintln!` → `tracing::info!`

**Finding:** P3.8
**Files:** `src/cli/commands/eval/runner.rs:163-168`
**Why:** Every other progress signal uses `tracing::info!`; `eprintln!` defeats `RUST_LOG` filtering and JSON log redirect.

### Current code
```rust
// runner.rs:167 (approx)
eprintln!("[eval] {done}/{total} queries ({qps:.1} q/s)");
```

### Replacement
```rust
tracing::info!(done, total = total_queries, qps, "eval progress");
```

---

## P3.9 — Add a span to `nl::generate_nl_with_template`

**Finding:** P3.9
**Files:** `src/nl/mod.rs:209` (and transitively covers `:43, :65, :189`)
**Why:** All four NL generators flow into `generate_nl_with_template`; a single `debug_span!` at that root site covers them all.

### Current code
```rust
pub fn generate_nl_with_template(
    chunk: &Chunk, callers: &[String], callees: &[String],
    note: Option<&str>, summary: Option<&str>, hyde: Option<&str>,
    template: NlTemplate,
) -> String {
    // ...
}
```

### Replacement
Insert at line 1 of the body:
```rust
let _span = tracing::debug_span!(
    "generate_nl",
    template = ?template,
    chunk_kind = ?chunk.chunk_type,
    len = chunk.content.len(),
).entered();
```

---

## P3.10 — `embed_documents`/`embed_query` completion events

**Finding:** P3.10
**Files:** `src/embedder/mod.rs:683` (`embed_documents`), `:722` (`embed_query`)
**Why:** Entry spans only carry input fields; no completion event with output dim/count/time.

### Current code
```rust
// inside embed_documents, after the loop returns embeddings:
Ok(embeddings)

// inside embed_query, before returning:
Ok(embedding)
```

### Replacement
At the bottom of `embed_documents` (just before `Ok(embeddings)`):
```rust
tracing::info!(
    total = embeddings.len(),
    dim = self.embedding_dim(),
    input_count = texts.len(),
    "embed_documents complete"
);
```
At the bottom of `embed_query` (just before `Ok(embedding)`):
```rust
tracing::debug!(
    dim = self.embedding_dim(),
    "embed_query complete"
);
```

---

## P3.11 — Reranker `rerank_with_passages` length-mismatch warn + error

**Finding:** P3.11
**Files:** `src/reranker.rs:200-220`
**Why:** When passages.len() != results.len(), the function silently scores arbitrary pairs and corrupts ranks. Hard error + structured warn.

### Current code
```rust
pub fn rerank_with_passages(
    &self, query: &str, passages: &[&str], results: &mut Vec<SearchResult>,
) -> Result<(), RerankerError> {
    let _span = tracing::info_span!("rerank_with_passages",
        n = passages.len()).entered();
    if results.is_empty() { return Ok(()); }
    // ... compute_scores etc.
}
```

### Replacement
After the entry span / early-return:
```rust
if passages.len() != results.len() {
    tracing::warn!(
        passages = passages.len(),
        results = results.len(),
        "rerank_with_passages: length mismatch — caller bug, refusing to score",
    );
    return Err(RerankerError::InvalidArguments(format!(
        "passages.len()={} != results.len()={}",
        passages.len(), results.len()
    )));
}
```
Add the `InvalidArguments(String)` variant to `RerankerError` if not present.

---

## P3.12 — `train_data` git wrappers log non-zero exits

**Finding:** P3.12
**Files:** `src/train_data/git.rs:65-242` (`git_log` ~65, `git_diff_tree` ~131, `git_show` ~173)
**Why:** Each wrapper bundles exit + stderr into an `Err` and returns silently. Operators with shallow clones hit "50% calls fail" with no journal trail.

### Current code (pattern repeated 3x)
```rust
if !output.status.success() {
    return Err(TrainDataError::Git(format!(
        "git diff-tree failed: {}",
        String::from_utf8_lossy(&output.stderr).trim()
    )));
}
```

### Replacement
At each site, before the early-return:
```rust
if !output.status.success() {
    let stderr = String::from_utf8_lossy(&output.stderr);
    tracing::warn!(
        exit = output.status.code(),
        stderr = %stderr.trim(),
        "git_diff_tree failed",  // change message per fn: git_log / git_show
    );
    return Err(TrainDataError::Git(format!(
        "git diff-tree failed: {}", stderr.trim()
    )));
}
```
Apply consistently to `git_log` (~line 65), `git_diff_tree` (~131), `git_show` (~173). Keep the per-fn message identifier so operators can grep.

---

## P3.13 — Convert format-string `tracing::info!` to structured fields (9 sites)

**Finding:** P3.13
**Files:** `src/hnsw/build.rs:78,236`, `src/hnsw/persist.rs:210,638,771`, `src/reference.rs:220`, `src/cli/commands/train/export_model.rs:76`, `src/audit.rs:85,93`, `src/embedder/provider.rs:149`
**Why:** Format-string interpolation produces unparseable rendered messages once OB-V1.30-1 lands JSON formatting. Pure-mechanical change to structured fields.

### Current code → Replacement (one-pass sweep)
```rust
// src/hnsw/build.rs:78
- tracing::info!("Building HNSW index with {} vectors", nb_elem);
+ tracing::info!(count = nb_elem, "Building HNSW index");

// src/hnsw/build.rs:236
- tracing::info!("HNSW index built: {} vectors", id_map.len());
+ tracing::info!(count = id_map.len(), "HNSW index built");

// src/hnsw/persist.rs:210
- tracing::info!("Saving HNSW index to {}/{}", dir.display(), basename);
+ tracing::info!(dir = %dir.display(), basename, "Saving HNSW index");

// src/hnsw/persist.rs:638
- tracing::info!("Loading HNSW index from {}/{}", dir.display(), basename);
+ tracing::info!(dir = %dir.display(), basename, "Loading HNSW index");

// src/hnsw/persist.rs:771
- tracing::info!("HNSW index loaded: {} vectors", id_map.len());
+ tracing::info!(count = id_map.len(), "HNSW index loaded");

// src/reference.rs:220
- tracing::info!("Loaded {} reference indexes", refs.len());
+ tracing::info!(count = refs.len(), "Loaded reference indexes");

// src/cli/commands/train/export_model.rs:76
- tracing::info!("Model exported to {}", output.display());
+ tracing::info!(output = %output.display(), "Model exported");

// src/audit.rs:85
- tracing::debug!("Failed to parse audit-mode.json: {}", e);
+ tracing::debug!(error = %e, "Failed to parse audit-mode.json");

// src/audit.rs:93
- .map_err(|e| tracing::debug!("Failed to parse expires_at: {}", e))
+ .map_err(|e| tracing::debug!(error = %e, "Failed to parse expires_at"))

// src/embedder/provider.rs:149
- tracing::debug!("Failed to symlink {}: {}", lib, e);
+ tracing::debug!(lib = %lib, error = %e, "Failed to symlink");
```
Verify post-fix with `rg 'tracing::(info|warn|debug|error)!\("[^"]*\{' src/` — should return zero hits in these files.

---

## P3.14 — `build_cluster` warn when corpus has chunks but no UMAP coords

**Finding:** P3.14
**Files:** `src/serve/data.rs:901, 1020` (in `build_cluster`)
**Why:** Empty cluster view leaves operators staring at a blank pane with no journal hint that `cqs index --umap` is needed.

### Current code (sketch — at the end of `build_cluster`)
```rust
Ok(ClusterResponse { nodes, skipped, total_chunks })
```

### Replacement
Right before the return:
```rust
if nodes.is_empty() && skipped > 0 {
    tracing::warn!(
        skipped,
        total_chunks,
        "build_cluster: corpus has chunks but no UMAP coordinates — run `cqs index --umap`",
    );
}
Ok(ClusterResponse { nodes, skipped, total_chunks })
```

---

## P3.15 — Reject leading/trailing-dash slot names

**Finding:** P3.15
**Files:** `src/slot/mod.rs:159-178` (`validate_slot_name`); test block `:661+`
**Why:** `-foo` collides with clap's flag parser; trailing dashes get stripped by various copy-paste pipelines.

### Current code
```rust
pub fn validate_slot_name(name: &str) -> Result<(), SlotError> {
    if name.is_empty() || name.len() > 32 { /* ... */ }
    if !name.chars().all(|c| c.is_ascii_lowercase()
        || c.is_ascii_digit() || c == '_' || c == '-') { /* ... */ }
    Ok(())
}
```

### Replacement
After the existing checks, add:
```rust
if name.starts_with('-') || name.ends_with('-') {
    return Err(SlotError::InvalidName(format!(
        "slot name '{name}' cannot start or end with '-' \
         (clap parses leading dash as a flag)"
    )));
}
```
Add tests in `src/slot/mod.rs::tests`: `validate_rejects_leading_dash`, `validate_rejects_trailing_dash`.

---

## P3.16 — Provider tests for malformed cmdline / `LD_LIBRARY_PATH`

**Finding:** P3.16
**Files:** `src/embedder/provider.rs:67-123`; new `#[cfg(test)] mod tests` in same file
**Why:** No tests in `provider.rs` today; silent CPU fallback on weird inputs is the production failure mode.

### Replacement
Add at the bottom of `src/embedder/provider.rs`:
```rust
#[cfg(test)]
mod tests {
    use super::*;
    use std::env;
    use std::sync::Mutex;

    static ENV_LOCK: Mutex<()> = Mutex::new(());

    #[test]
    fn find_ld_library_dir_skips_empty_entries() {
        let _g = ENV_LOCK.lock().unwrap();
        let prev = env::var_os("LD_LIBRARY_PATH");
        // SAFETY: serialized via ENV_LOCK
        unsafe { env::set_var("LD_LIBRARY_PATH", ":/tmp:"); }
        let dir = find_ld_library_dir();
        // /tmp is the only non-empty entry that exists
        assert_eq!(dir.as_deref(), Some(std::path::Path::new("/tmp")));
        unsafe {
            match prev {
                Some(p) => env::set_var("LD_LIBRARY_PATH", p),
                None => env::remove_var("LD_LIBRARY_PATH"),
            }
        }
    }

    #[test]
    fn find_ld_library_dir_handles_unset() {
        let _g = ENV_LOCK.lock().unwrap();
        let prev = env::var_os("LD_LIBRARY_PATH");
        unsafe { env::remove_var("LD_LIBRARY_PATH"); }
        let dir = find_ld_library_dir();
        assert!(dir.is_none());
        unsafe { if let Some(p) = prev { env::set_var("LD_LIBRARY_PATH", p); } }
    }
}
```

---

## P3.17 — `blake3_hex_or_passthrough` boundary tests

**Finding:** P3.17
**Files:** `src/cache.rs:709-721` + `src/cache.rs::tests`
**Why:** Pin the uppercase / short-hex / passthrough surprises so a future "always-encode" tightening surfaces as an intentional break.

### Replacement
Add to `src/cache.rs::tests`:
```rust
#[test]
fn blake3_hex_or_passthrough_uppercase_64_chars_passthrough() {
    let upper = "ABCDEF0123456789".repeat(4); // 64 chars, all hex
    assert_eq!(blake3_hex_or_passthrough(upper.as_bytes()), upper);
}

#[test]
fn blake3_hex_or_passthrough_short_hex_string_gets_encoded() {
    let short = "abcd"; // 4 hex chars
    let out = blake3_hex_or_passthrough(short.as_bytes());
    assert_eq!(out, "61626364"); // hex of ASCII 'a','b','c','d'
}

#[test]
fn blake3_hex_or_passthrough_64_byte_non_hex_gets_encoded() {
    let bytes = vec![0xAB; 64];
    let out = blake3_hex_or_passthrough(&bytes);
    assert_eq!(out.len(), 128);
    assert!(out.chars().all(|c| c.is_ascii_hexdigit()));
}
```

---

## P3.18 — `SystemTime → i64` cache cast: guard against year-2554 wrap

**Finding:** P3.18
**Files:** `src/cache.rs:349-352, 551-555`
**Why:** `as_secs() as i64` wraps silently above i64::MAX. Defense-in-depth.

### Current code
```rust
// cache.rs:349-352 (write_batch)
let now = std::time::SystemTime::now()
    .duration_since(std::time::UNIX_EPOCH)
    .unwrap_or_default()
    .as_secs() as i64;

// cache.rs:551-555 (prune_older_than)
let cutoff = std::time::SystemTime::now()
    .duration_since(std::time::UNIX_EPOCH)
    .unwrap_or_default()
    .as_secs() as i64
    - (days as i64 * 86400);
```

### Replacement
Add a helper at module top:
```rust
fn now_unix_i64() -> Result<i64, CacheError> {
    let secs = std::time::SystemTime::now()
        .duration_since(std::time::UNIX_EPOCH)
        .map_err(|_| CacheError::Internal("clock before unix epoch".into()))?
        .as_secs();
    i64::try_from(secs)
        .map_err(|_| CacheError::Internal("clock above i64 cap".into()))
}
```
Replace both sites:
```rust
let now = now_unix_i64()?;
// ...
let cutoff = now_unix_i64()? - (days as i64 * 86400);
```

---

## P3.20 — Clamp `cqs cache prune --older-than` to sane ceiling

**Finding:** P3.20
**Files:** `src/cache.rs:548, 551-555`
**Why:** `u32::MAX * 86400` overflow / underflow → silent "prune everything" on typo.

### Current code
```rust
pub fn prune_older_than(&self, days: u32) -> Result<usize, CacheError> {
    let _span = tracing::info_span!("cache_prune", days).entered();
    let cutoff = /* now */ - (days as i64 * 86400);
    // ...
}
```

### Replacement
At the top of the function, clamp + reject:
```rust
pub fn prune_older_than(&self, days: u32) -> Result<usize, CacheError> {
    const MAX_PRUNE_DAYS: u32 = 36_500; // 100 years
    let days = days.min(MAX_PRUNE_DAYS);
    let _span = tracing::info_span!("cache_prune", days).entered();
    let now = now_unix_i64()?; // see P3.18
    let cutoff = now - (days as i64 * 86400);
    if cutoff < 0 {
        return Err(CacheError::Internal(
            format!("prune cutoff below epoch (days={days})")
        ));
    }
    // ...
}
```

---

## P3.21 — Centralize `i64.max(0) as u32` clamp (8+ sites)

**Finding:** P3.21
**Files:** `src/serve/data.rs:290, 299, 300, 587, 588, 777, 778, 993, 994` (verified 9 sites — note line 290 is a `n_callers` clamp, not just line_start/line_end; audit said 8)
**Why:** Repeated open-coded clamp pattern silently masks DB-corruption / migration bugs. Replace with a named helper that logs once on negative input.

### Current code (one example site)
```rust
line_start: line_start.max(0) as u32,
line_end: line_end.max(0) as u32,
```

### Replacement
Add a helper at top of `src/serve/data.rs`:
```rust
/// Clamp an i64 SQL line number to u32, warning once if the input was
/// negative (signals DB corruption or migration bug).
#[inline]
fn clamp_line_to_u32(v: i64) -> u32 {
    if v < 0 {
        tracing::warn!(value = v, "negative line number clamped to 0");
        0
    } else {
        v.min(u32::MAX as i64) as u32
    }
}
```
Sweep all 8/9 occurrences of `<x>.max(0) as u32` to `clamp_line_to_u32(<x>)`. Verify with `rg 'max\(0\) as u32' src/serve/data.rs` returning zero hits.

---

## P3.22 — Daemon socket-thread join: warn on detach-after-timeout

**Finding:** P3.22
**Files:** `src/cli/watch.rs:2374-2400`
**Why:** Doc-comment claims "joined cleanly" but deadline-fall-through silently detaches the thread. Add the warn so logs match reality.

### Current code (sketch — the polling loop)
```rust
let deadline = Instant::now() + Duration::from_secs(5);
loop {
    if handle.is_finished() {
        let _ = handle.join();
        tracing::info!("Daemon socket thread joined cleanly");
        break;
    }
    if Instant::now() > deadline { break; }
    std::thread::sleep(Duration::from_millis(50));
}
```

### Replacement
```rust
let deadline = Instant::now() + Duration::from_secs(5);
let mut joined = false;
loop {
    if handle.is_finished() {
        let _ = handle.join();
        tracing::info!("Daemon socket thread joined cleanly");
        joined = true;
        break;
    }
    if Instant::now() > deadline { break; }
    std::thread::sleep(Duration::from_millis(50));
}
if !joined {
    tracing::warn!(
        "Daemon socket thread did not exit within 5s; detaching"
    );
}
```

---

## P3.23 — `diff::EMBEDDING_BATCH_SIZE` env override

**Finding:** P3.23
**Files:** `src/diff.rs:158`
**Why:** Hardcoded 1000 doesn't scale with model dim; ~12 MB only at 1024-dim.

### Current code
```rust
const EMBEDDING_BATCH_SIZE: usize = 1000;
```

### Replacement
```rust
fn embedding_batch_size() -> usize {
    std::env::var("CQS_DIFF_EMBEDDING_BATCH_SIZE")
        .ok()
        .and_then(|v| v.parse::<usize>().ok())
        .filter(|&n| n > 0)
        .unwrap_or(1000)
}
```
Replace the const reference at the call site with `embedding_batch_size()`. Update the surrounding comment to mention `CQS_DIFF_EMBEDDING_BATCH_SIZE` and the dim sensitivity.

---

## P3.24 — Daemon `worker_threads` env override

**Finding:** P3.24
**Files:** `src/cli/watch.rs:115-119`
**Why:** Hardcoded `min(num_cpus, 4)` caps large-machine parallelism with no escape hatch.

### Current code
```rust
let worker_threads = std::thread::available_parallelism()
    .map(|n| n.get()).unwrap_or(1).min(4);
```

### Replacement
```rust
let worker_threads = std::env::var("CQS_DAEMON_WORKER_THREADS")
    .ok().and_then(|v| v.parse::<usize>().ok()).filter(|&n| n > 0)
    .unwrap_or_else(|| {
        std::thread::available_parallelism()
            .map(|n| n.get()).unwrap_or(1).min(4)
    });
```

---

## P3.25 — `train_data::MAX_SHOW_SIZE` env override

**Finding:** P3.25
**Files:** `src/train_data/git.rs:167`; ideally moved into `src/limits.rs`
**Why:** Hardcoded 50 MB silently drops large files from training-data extraction with no log signal.

### Current code
```rust
const MAX_SHOW_SIZE: usize = 50 * 1024 * 1024;
```

### Replacement
```rust
fn max_show_size() -> usize {
    std::env::var("CQS_TRAIN_GIT_SHOW_MAX_BYTES")
        .ok().and_then(|v| v.parse::<usize>().ok()).filter(|&n| n > 0)
        .unwrap_or(50 * 1024 * 1024)
}
```
Update the caller to call `max_show_size()` and add a `tracing::warn!(path = %path, size, max, "git_show output exceeds max — skipping")` at the early-return site so callers can distinguish "too large" from "binary".

---

## P3.26 — Lift `BatchCmd::is_pipeable` into the registry

**Finding:** P3.26
**Files:** `src/cli/batch/commands.rs:325-364` (`is_pipeable`); registry at `src/cli/registry.rs`
**Why:** #1114 collapsed Group-A/B exhaustive matches but the batch-side `is_pipeable` enum-match was missed; one row per command becomes one row + one batch arm.

### Current code
```rust
// src/cli/batch/commands.rs:325-364
impl BatchCmd {
    pub fn is_pipeable(&self) -> bool {
        match self {
            BatchCmd::Search { .. } | BatchCmd::Gather { .. }
            | BatchCmd::Scout { .. } | BatchCmd::Onboard { .. }
            | /* ... ~30 arms ... */ => true,
            _ => false,
        }
    }
}
```

### Replacement
Add an `is_pipeable: bool` flag to each row of `for_each_command!` in `src/cli/registry.rs`. Generate `BatchCmd::is_pipeable(&self)` from the table via a new `gen_is_pipeable_impl!()` macro. Delete the manual match in `commands.rs:325-364`.

(If the BatchCmd enum itself isn't yet driven from the registry, this prompt narrows to "drive `is_pipeable` from the table"; the wider refactor of folding `BatchCmd` into the registry is P2-level and listed as #2.)

---

## P3.27 — `LlmProvider` resolver via registry slice

**Finding:** P3.27
**Files:** `src/llm/mod.rs:200-205, 284-304, 362-398`
**Why:** Three hand-coded match arms (enum, env-var resolve, factory) per provider. Walk a `&[&dyn ProviderRegistry]` slice instead.

### Current code
```rust
// llm/mod.rs:284-304 (resolve)
match std::env::var("CQS_LLM_PROVIDER").as_deref() {
    Ok("anthropic") | Err(_) => LlmProvider::Anthropic,
    Ok("local") => LlmProvider::Local,
    Ok(other) => { tracing::warn!(provider = other, "unknown CQS_LLM_PROVIDER, defaulting to anthropic"); LlmProvider::Anthropic }
}

// llm/mod.rs:362-398 (create_client)
match provider {
    LlmProvider::Anthropic => /* build AnthropicClient */,
    LlmProvider::Local => /* build LocalProvider */,
}
```

### Replacement
Introduce a registry trait + slice:
```rust
pub(crate) trait ProviderRegistry: Sync {
    fn name(&self) -> &'static str;
    fn build(&self, cfg: &LlmConfig) -> Result<Box<dyn BatchProvider>, LlmError>;
}

static PROVIDERS: &[&dyn ProviderRegistry] = &[
    &AnthropicRegistry, &LocalRegistry,
];
```
`resolve()` walks `PROVIDERS.iter().find(|p| p.name() == requested)`; `create_client()` calls the matched `build()`. Add a provider = add one impl + one slice row.

---

## P3.28 — Tree-sitter registry coverage self-test

**Finding:** P3.28
**Files:** `src/language/queries/*.scm`; new test in `src/language/mod.rs`
**Why:** An empty `chunks.scm` `include_str!`s as `""` and silently emits zero chunks. One assertion catches it.

### Replacement
Add to `src/language/mod.rs::tests`:
```rust
#[test]
fn registry_languages_have_nonempty_chunk_query() {
    for lang in REGISTRY.all() {
        if !lang.has_grammar() { continue; }
        assert!(
            !lang.chunk_query.is_empty(),
            "{:?}: chunk_query is empty — silent zero-chunk language",
            lang.lang
        );
    }
}
```
(`has_grammar` may already exist or can be a small helper that returns true when the lang's tree-sitter feature is enabled.)

---

## P3.29 — `find_project_root` markers as a data table

**Finding:** P3.29
**Files:** `src/cli/config.rs:155-162`
**Why:** Hardcoded array works today; converting to a `static` table makes adding Maven / Gradle / .NET / Bazel a one-row change.

### Current code
```rust
let markers = [
    "Cargo.toml", "package.json", "pyproject.toml",
    "setup.py", "go.mod", ".git",
];
for current in path.ancestors() {
    for marker in &markers {
        if current.join(marker).exists() { return Some(current.to_path_buf()); }
    }
}
```

### Replacement
At module top:
```rust
/// (marker filename, label) — label is informational, not used in lookup.
static PROJECT_ROOT_MARKERS: &[(&str, &str)] = &[
    ("Cargo.toml", "rust"),
    ("package.json", "node"),
    ("pyproject.toml", "python"),
    ("setup.py", "python"),
    ("go.mod", "go"),
    (".git", "fallback"),
];
```
Iterate `PROJECT_ROOT_MARKERS.iter().any(|(m, _)| current.join(m).exists())`.

---

## P3.30 — `structural_matchers` shared library

**Finding:** P3.30
**Files:** `src/language/mod.rs:191, 345`; `src/structural.rs`
**Why:** Currently per-language `Option<&[(name, fn)]>`; sharing common matchers (SwallowedException, AsyncIO, Mutex, Unsafe) across Python/JS/Go/Rust requires copying fn bodies.

### Replacement
Define a small set of cross-language matcher functions in `src/structural.rs` keyed by `(Pattern, Language)`:
```rust
pub(crate) static SHARED_MATCHERS: &[(Pattern, Language, StructuralMatcherFn)] = &[
    (Pattern::SwallowedException, Language::Rust, matchers::rust::swallow_exc),
    (Pattern::SwallowedException, Language::Python, matchers::python::swallow_exc),
    // ...
];
```
Have `LanguageDef::structural_matchers` either be derived from this table at lookup time (filter by `lang`), or keep the field but have each `definition_*` row reference the shared fn pointer. Adding a pattern-language pair = one slice row, not a new fn body.

---

## P3.31 — Embedder preset `extras` map (deferred-friendly)

**Finding:** P3.31
**Files:** `src/embedder/models.rs:163-300`
**Why:** New cross-cutting preset attribute fans out to every row; flagging now since presets are still stable but pressure is rising.

### Replacement
Skip if presets are stable. If/when frequent attribute additions land, extend the `define_embedder_presets!` macro grammar with an `extras: { gpu_only = true, expects_bos = true }` block per row that desugars to a `HashMap<&'static str, ModelAttr>` field on `ModelConfig`. Required fields stay positional; sparse/optional ones go through `extras`.

---

## P3.32 — Cache paths use `dirs::cache_dir()` on Windows

**Finding:** P3.32
**Files:** `src/cache.rs:80-84` (`EmbeddingCache::default_path`), `src/cache.rs:1399-1403` (`QueryCache::default_path`), `src/cli/batch/commands.rs:373-376` (query_log)
**Why:** Hardcoded `~/.cache/cqs/...` becomes a hidden `.cache` folder under `C:\Users\X\` on Windows; native is `%LOCALAPPDATA%\cqs\`.

### Current code (pattern at all 3 sites)
```rust
dirs::home_dir()
    .map(|h| h.join(".cache").join("cqs").join("embeddings.db"))
```

### Replacement
```rust
dirs::cache_dir()
    .or_else(|| dirs::home_dir().map(|h| h.join(".cache")))
    .map(|c| c.join("cqs").join("embeddings.db"))
```
Apply the same shape to `QueryCache::default_path` (`query_cache.db`) and the `query_log.jsonl` path in `cli/batch/commands.rs:373-376`.

---

## P3.33 — `dispatch_drift/diff` JSON file fields use `normalize_path`

**Finding:** P3.33
**Files:** `src/suggest.rs:101`, `src/store/types.rs:220`
**Why:** Two more PB-V1.29-5 sites that emit Windows backslashes via `.display().to_string()`.

### Current code
```rust
// src/suggest.rs:101
let file = dead.chunk.file.display().to_string();

// src/store/types.rs:220
let file_display = file.display().to_string();
```

### Replacement
```rust
// src/suggest.rs:101
let file = crate::normalize_path(&dead.chunk.file);

// src/store/types.rs:220
let file_display = crate::normalize_path(file);
```
(If `normalize_path` accepts `&Path` not `&PathBuf`, adjust the borrow.)

---

## P3.34 — `find_ld_library_dir` Windows arm (or doc the gap)

**Finding:** P3.34
**Files:** `src/embedder/provider.rs:115-123`
**Why:** Currently `cfg(target_os="linux")`-gated; no Windows equivalent. Either add one or doc the delegation explicitly.

### Replacement (lower-cost: doc the gap)
Add at the top of `ensure_ort_provider_libs`:
```rust
/// On Linux this walks `LD_LIBRARY_PATH` (`:`-separated) and symlinks ORT
/// provider .so files into the runtime's search dir. On Windows and macOS
/// provider DLL/dylib resolution is delegated entirely to ORT's loader
/// (Windows: `PATH` search; macOS: `DYLD_*` paths). If a future regression
/// surfaces on those platforms, add an arm with `;`-split for `PATH` (Win)
/// or `DYLD_LIBRARY_PATH` (mac).
```
Keep behavior unchanged.

---

## P3.35 — Document `index.lock` advisory-vs-mandatory split

**Finding:** P3.35
**Files:** `src/cli/files.rs:120-213`
**Why:** Same code, two very different concurrency contracts (Linux advisory `flock` vs Windows mandatory `LockFileEx`). Doc + one-time warn so callers can reason about it.

### Replacement
1. Extend the `acquire_index_lock` doc-comment to spell out: Linux/macOS = advisory (non-cqs writers can corrupt); Windows = mandatory (third-party tools may see "sharing violation"); WSL `/mnt/c/` follows Linux semantics for the call but Windows for the underlying file.
2. On Windows only, add a one-shot `tracing::warn!` (gated on `OnceLock`) at first lock acquisition:
```rust
#[cfg(windows)]
{
    static WARNED: std::sync::OnceLock<()> = std::sync::OnceLock::new();
    WARNED.get_or_init(|| {
        tracing::warn!(
            "index.lock is mandatory on Windows — third-party tools opening \
             index.db may fail with sharing violations while cqs is running"
        );
    });
}
```

---

## P3.36 — `is_wsl_drvfs_path` matches UNC + uppercase drives

**Finding:** P3.36
**Files:** `src/config.rs:92-101`
**Why:** Misses `//wsl.localhost/`, `//wsl$/`, and uppercase drive letters when WSL `automount.options=case=force` is set.

### Current code
```rust
pub fn is_wsl_drvfs_path(path: &Path) -> bool {
    let s = path.to_string_lossy();
    s.starts_with("/mnt/")
        && s.chars().nth(5).is_some_and(|c| c.is_ascii_lowercase())
        && s.chars().nth(6) == Some('/')
}
```

### Replacement
```rust
pub fn is_wsl_drvfs_path(path: &Path) -> bool {
    let s = path.to_string_lossy();
    // Standard /mnt/<letter>/ — accept upper or lowercase
    if let Some(rest) = s.strip_prefix("/mnt/") {
        if let (Some(c), Some('/')) = (rest.chars().next(), rest.chars().nth(1)) {
            if c.is_ascii_alphabetic() { return true; }
        }
    }
    // UNC paths reaching back into WSL
    if s.starts_with("//wsl.localhost/") || s.starts_with("//wsl$/") {
        return true;
    }
    false
}
```

---

## P3.37 — `blame.rs` git_file via `normalize_slashes`

**Finding:** P3.37
**Files:** `src/cli/commands/io/blame.rs:113-115`
**Why:** Current `replace('\\', "/")` only handles backslashes; verbatim `\\?\` prefix slips through, breaking `git log --format=... -- <file>`.

### Current code
```rust
let git_file = rel_file.replace('\\', "/");
```

### Replacement
```rust
let git_file = crate::normalize_slashes(&rel_file);
```

---

## P3.38 — Daemon socket-path: warn on `XDG_RUNTIME_DIR` unset (Linux only)

**Finding:** P3.38
**Files:** `src/daemon_translate.rs:179-188`
**Why:** Silent fallback to `/tmp` (mode 1777) hides a meaningful trust drop on multi-user Linux. macOS `/var/folders/...` is fine.

### Current code
```rust
fn daemon_socket_path() -> PathBuf {
    let dir = std::env::var_os("XDG_RUNTIME_DIR")
        .map(PathBuf::from)
        .unwrap_or_else(std::env::temp_dir);
    dir.join(/* per-user socket name */)
}
```

### Replacement
```rust
fn daemon_socket_path() -> PathBuf {
    if let Some(d) = std::env::var_os("XDG_RUNTIME_DIR") {
        return PathBuf::from(d).join(/* socket name */);
    }
    #[cfg(target_os = "linux")]
    {
        static WARNED: std::sync::OnceLock<()> = std::sync::OnceLock::new();
        WARNED.get_or_init(|| {
            tracing::info!(
                "XDG_RUNTIME_DIR unset — daemon socket falls back to temp_dir; \
                 consider XDG_RUNTIME_DIR=/run/user/$(id -u)"
            );
        });
    }
    std::env::temp_dir().join(/* socket name */)
}
```

---

## P3.39 — `write_slot_model` / `write_active_slot` use `atomic_replace`

**Finding:** P3.39
**Files:** `src/slot/mod.rs:237-277` (`write_slot_model`), `src/slot/mod.rs:363-406` (`write_active_slot`)
**Why:** Bespoke temp+rename without parent-dir fsync. `crate::fs::atomic_replace` already does this for `notes.toml` / `audit-mode.json`.

### Current code (sketch — both functions)
```rust
let tmp = dir.join(format!(".slot.toml.{}.tmp", temp_suffix()));
let mut f = fs::File::create(&tmp)?;
f.write_all(contents.as_bytes())?;
f.sync_all()?;
fs::rename(&tmp, &final_path)?;
// no parent-dir fsync
```

### Replacement
```rust
crate::fs::atomic_replace(&final_path, contents.as_bytes())?;
```
Drop the bespoke temp+rename block from each function. ~20 LOC each.

---

## P3.40 — `update_umap_coords_batch`: DROP TEMP TABLE before CREATE

**Finding:** P3.40
**Files:** `src/store/chunks/crud.rs:392-450`
**Why:** TEMP TABLE is connection-scoped, not transaction-scoped; rollback can leave a stale `_update_umap` between calls on the same pooled connection.

### Current code
```rust
sqlx::query("CREATE TEMP TABLE IF NOT EXISTS _update_umap (...)").execute(&mut *tx).await?;
sqlx::query("DELETE FROM _update_umap").execute(&mut *tx).await?;
// ... INSERT INTO _update_umap, UPDATE chunks FROM _update_umap, DROP TABLE IF EXISTS _update_umap;
```

### Replacement
```rust
sqlx::query("DROP TABLE IF EXISTS _update_umap").execute(&mut *tx).await?;
sqlx::query("CREATE TEMP TABLE _update_umap (...)").execute(&mut *tx).await?;
// no DELETE needed; INSERT INTO _update_umap proceeds cleanly
```

---

## P3.41 — `reindex_files`: build embeddings without placeholders

**Finding:** P3.41
**Files:** `src/cli/watch.rs:2918-2924`
**Why:** Allocates N empty `Embedding::new(vec![])` placeholders then overwrites each — pure waste plus a future zero-norm-vector landmine.

### Current code
```rust
let mut embeddings: Vec<Embedding> = vec![Embedding::new(vec![]); chunk_count];
for (i, e) in cached { embeddings[i] = e; }
for (i, e) in new_embeddings { embeddings[i] = e; }
```

### Replacement
Mirror the bulk-pipeline pattern (`src/cli/pipeline/embedding.rs::create_embedded_batch`):
```rust
let mut by_index: HashMap<usize, Embedding> = HashMap::with_capacity(chunk_count);
for (i, e) in cached { by_index.insert(i, e); }
for (i, e) in new_embeddings { by_index.insert(i, e); }
let embeddings: Vec<Embedding> = (0..chunk_count)
    .map(|i| by_index.remove(&i)
        .unwrap_or_else(|| panic!("missing embedding at index {i}")))
    .collect();
```
(Or refactor to call `create_embedded_batch` directly if its signature fits.)

---

## P3.42 — `prepare_for_embedding`: skip store query when global cache satisfies all

**Finding:** P3.42
**Files:** `src/cli/pipeline/embedding.rs:64-82`
**Why:** Always issues `store.get_embeddings_by_hashes` even when global cache has every entry; one wasted bind-heavy SELECT per warm reindex.

### Current code (sketch)
```rust
let global_hits = global_cache.read_batch(&hashes, ...)?;
let store_hits = store.get_embeddings_by_hashes(&hashes)?; // unconditional
```

### Replacement
```rust
let global_hits = global_cache.read_batch(&hashes, ...)?;
let missed: Vec<&str> = hashes.iter()
    .filter(|h| !global_hits.contains_key(h.as_str()))
    .map(|h| h.as_str()).collect();
let store_hits = if missed.is_empty() {
    HashMap::new()
} else {
    store.get_embeddings_by_hashes(&missed)?
};
```

---

## P3.43 — Daemon socket: fold args validation + extraction into one pass

**Finding:** P3.43
**Files:** `src/cli/watch.rs:266-297`
**Why:** Every daemon query walks the args array twice (validation + extraction). Single pass collects both.

### Current code
```rust
let bad_arg_indices: Vec<usize> = arr.iter().enumerate()
    .filter_map(|(i, v)| (!v.is_string()).then_some(i)).collect();
if !bad_arg_indices.is_empty() { /* reject */ }
let args: Vec<String> = arr.iter()
    .filter_map(|v| v.as_str().map(String::from)).collect();
```

### Replacement
```rust
let mut args = Vec::with_capacity(arr.len());
let mut bad_arg_indices = Vec::new();
for (i, v) in arr.iter().enumerate() {
    match v.as_str() {
        Some(s) => args.push(s.to_string()),
        None => bad_arg_indices.push(i),
    }
}
if !bad_arg_indices.is_empty() { /* reject as before */ }
```

---

## P3.44 — `build_graph` edge dedup with hashed key

**Finding:** P3.44
**Files:** `src/serve/data.rs:367-373`
**Why:** `(file.clone(), caller.clone(), callee.clone())` allocates 3 Strings per row even on dedup miss. Hash to u64 instead.

### Current code
```rust
let key = (file.clone(), caller_name.clone(), callee_name.clone());
if seen.insert(key) {
    accum.push((file, caller_name, callee_name));
}
```

### Replacement
```rust
use std::hash::{Hash, Hasher};
use std::collections::hash_map::DefaultHasher;
let mut h = DefaultHasher::new();
file.hash(&mut h); caller_name.hash(&mut h); callee_name.hash(&mut h);
if seen.insert(h.finish()) {
    accum.push((file, caller_name, callee_name));
}
```
Change `seen` type from `HashSet<(String,String,String)>` to `HashSet<u64>`.

---

## P3.45 — `extract_imports`: borrow keys, allocate only on accept

**Finding:** P3.45
**Files:** `src/where_to_add.rs:258-276`
**Why:** `seen.insert(trimmed.to_string())` per candidate line allocates even on rejection.

### Current code
```rust
let mut seen: HashSet<String> = HashSet::new();
let mut imports: Vec<String> = Vec::new();
for chunk in chunks {
    for line in chunk.content.lines() {
        let trimmed = line.trim();
        for &prefix in prefixes {
            if trimmed.starts_with(prefix) && imports.len() < max
                && seen.insert(trimmed.to_string()) {
                imports.push(trimmed.to_string());
                break;
            }
        }
    }
}
```

### Replacement
```rust
let mut seen: HashSet<&str> = HashSet::new();
let mut imports: Vec<String> = Vec::new();
for chunk in chunks {
    for line in chunk.content.lines() {
        let trimmed = line.trim();
        for &prefix in prefixes {
            if trimmed.starts_with(prefix) && imports.len() < max
                && seen.insert(trimmed) {
                imports.push(trimmed.to_string());
                break;
            }
        }
    }
}
```

---

## P3.46 — Watch reindex: `existing.remove` instead of `.get().clone()`

**Finding:** P3.46
**Files:** `src/cli/watch.rs:2879-2887`
**Why:** Per cache hit clones a 4 KB `Embedding` (1024-dim). `.remove()` takes ownership.

### Current code
```rust
let existing = store.get_embeddings_by_hashes(&hashes)?;
for (i, chunk) in chunks.iter().enumerate() {
    if let Some(emb) = existing.get(&chunk.content_hash) {
        cached.push((i, emb.clone()));
    } else {
        to_embed.push((i, chunk));
    }
}
```

### Replacement
```rust
let mut existing = store.get_embeddings_by_hashes(&hashes)?;
for (i, chunk) in chunks.iter().enumerate() {
    if let Some(emb) = existing.remove(&chunk.content_hash) {
        cached.push((i, emb));
    } else {
        to_embed.push((i, chunk));
    }
}
```

---

## P3.47 — `LocalProvider` worker thread stack 512 KB

**Finding:** P3.47
**Files:** `src/llm/local.rs:163-256`
**Why:** Default 2 MB stack × concurrency=64 = 128 MB just for the fan-out. Worker body is shallow.

### Current code (sketch)
```rust
std::thread::scope(|s| {
    for _ in 0..workers {
        s.spawn(|| { /* recv → http → parse → mutex.insert */ });
    }
});
```

### Replacement
Switch to `Builder`-based threads with manual join, since `std::thread::scope::Scope::spawn` lacks a stack-size hook:
```rust
let mut handles = Vec::with_capacity(workers);
for i in 0..workers {
    let h = std::thread::Builder::new()
        .name(format!("cqs-llm-worker-{i}"))
        .stack_size(512 * 1024)
        .spawn(/* worker closure */)?;
    handles.push(h);
}
for h in handles { let _ = h.join(); }
```
Adjust the captured borrows to be `Arc`-cloned since we're outside `scope`. Drop the upper concurrency clamp from 64 to 16 in `local_concurrency()` while you're there.

---

## P3.48 — `LocalProvider::http`: cap idle pool

**Finding:** P3.48
**Files:** `src/llm/local.rs:97-100`
**Why:** Default reqwest pool = unbounded idle, 90 s timeout. Long indexing runs leak idle slots into vLLM/llama.cpp.

### Current code
```rust
Client::builder()
    .timeout(timeout)
    .redirect(Policy::none())
    .build()?
```

### Replacement
```rust
Client::builder()
    .timeout(timeout)
    .redirect(Policy::none())
    .pool_max_idle_per_host(self.concurrency)
    .pool_idle_timeout(Duration::from_secs(30))
    .build()?
```

---

## P3.49 — `cmd_similar` (CLI) integration test

**Finding:** P3.49
**Files:** `src/cli/commands/search/similar.rs:41`
**Why:** Library `find_similar` is tested; the CLI wrapper (target lookup + pattern filter + JSON build) is not.

### Replacement
Add to `src/cli/commands/search/similar.rs::tests` (or new `tests/cli_similar_test.rs` if `cmd_similar` is hard to drive in-process):
```rust
#[test]
fn cmd_similar_returns_other_seeded_chunks() {
    let fix = common::InProcessFixture::new();
    fix.seed_chunks(&[("foo", "fn foo() { bar(); }"),
                      ("bar", "fn bar() { 1+1; }"),
                      ("baz", "fn baz() { unrelated(); }")]);
    let mut sink = Vec::<u8>::new();
    let ctx = fix.command_context_with_json_sink(&mut sink);
    cmd_similar(&ctx, "foo", /*limit*/ 2).unwrap();
    let json: serde_json::Value = serde_json::from_slice(&sink).unwrap();
    let names: Vec<&str> = json["data"]["results"].as_array().unwrap().iter()
        .map(|r| r["name"].as_str().unwrap()).collect();
    assert!(names.contains(&"bar"));
    assert!(!names.contains(&"foo")); // self excluded
}
```

---

## P3.50 — `cmd_ci` happy-path test

**Finding:** P3.50
**Files:** `src/cli/commands/review/ci.rs:9` + `tests/cli_train_review_test.rs` (or new `tests/cli_ci_test.rs`)
**Why:** Existing tests cover error paths only; markdown formatting + exit-code mapping for a real diff are unpinned.

### Replacement
Add a test that feeds a real unified diff touching a hotspot function in an `InProcessFixture`-seeded corpus:
```rust
#[test]
fn cmd_ci_high_risk_diff_emits_high_risk_section_and_nonzero_exit() {
    let fix = common::InProcessFixture::new();
    fix.seed_chunks(&[("hotspot", "fn hotspot() { /* many callers */ }")]);
    fix.seed_callers("hotspot", 12); // synthetic call edges
    let diff = "--- a/src/lib.rs\n+++ b/src/lib.rs\n@@ -1,3 +1,3 @@\n \
                fn hotspot() {\n-    do_old();\n+    do_new();\n }\n";
    let mut stdout = Vec::<u8>::new();
    let exit = cmd_ci_with_input(&fix.context(), diff, &mut stdout).unwrap();
    let out = String::from_utf8(stdout).unwrap();
    assert!(out.contains("High-risk"));
    assert_ne!(exit, 0);
}
```

---

## P3.51 — `cmd_gather` (CLI) integration test

**Finding:** P3.51
**Files:** `src/cli/commands/search/gather.rs:77`
**Why:** Library `gather()` is tested; CLI-only steps (`--max-files` clamp, content injection, token-budget trim) are not.

### Replacement
Add `tests/cli_gather_test.rs`:
```rust
#[test]
fn cli_gather_clamps_max_files_and_injects_content() {
    let fix = common::InProcessFixture::new();
    fix.seed_chunks(&[/* 5 chunks across 5 files */]);
    let out = cqs(&["gather", "needle", "--json", "--max-files", "2"]).output();
    let json: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap();
    let groups = json["data"]["results"].as_array().unwrap();
    assert_eq!(groups.len(), 2);                              // clamp
    assert!(groups[0]["content"].as_str().unwrap().len() > 0); // content injected
}
```

---

## P3.52 — `dispatch_line` happy-path test for a valid command

**Finding:** P3.52
**Files:** `src/cli/batch/mod.rs:557` (`dispatch_line`); test block in same file
**Why:** Existing tests are all error/adversarial. A success-envelope shape regression would slip through.

### Replacement
Add to `src/cli/batch/mod.rs::tests`:
```rust
#[test]
fn test_dispatch_line_stats_emits_success_envelope() {
    let fix = common::InProcessFixture::new();
    let mut ctx = fix.batch_context();
    let mut sink = Vec::<u8>::new();
    ctx.dispatch_line("stats", &mut sink).unwrap();
    let line = std::str::from_utf8(&sink).unwrap().lines().next().unwrap();
    let v: serde_json::Value = serde_json::from_str(line).unwrap();
    assert!(v["error"].is_null());
    assert!(v["data"]["total_chunks"].is_number());
}
```

---

## P3.53 — `select_provider` / `detect_provider` direct tests

**Finding:** P3.53
**Files:** `src/embedder/provider.rs:171, 188, 258`; new `#[cfg(test)] mod tests` (overlaps P3.16; consolidate)
**Why:** #1120's provider split has zero tests for the cache-on-first-call invariant or the priority list.

### Replacement
Add to `src/embedder/provider.rs::tests` (alongside the P3.16 tests):
```rust
#[test]
fn select_provider_caches_first_call() {
    // OnceCell semantics — first call wins, second returns same value.
    let p1 = select_provider();
    let p2 = select_provider();
    assert_eq!(format!("{p1:?}"), format!("{p2:?}"));
}

#[cfg(not(any(feature = "cuda-index", feature = "ep-coreml", feature = "ep-rocm")))]
#[test]
fn detect_provider_returns_cpu_when_no_features() {
    // Pure-CPU build must yield the CPU branch.
    assert!(matches!(detect_provider(), ExecutionProvider::CPU));
}
```
(Add a `cuda-index`-gated test that asserts CUDA selection on a CUDA build if the runtime has a GPU; otherwise skip.)

---

# P4 — GitHub Issues (paste-ready)

## P4.1 — AuthToken alphabet invariant via type-state

**Disposition:** GitHub issue
**Files:** `src/serve/auth.rs:75-78` (`from_string`), `src/serve/auth.rs:218-220` (`HeaderValue::from_str(&cookie).expect(…)`)

### Issue body (paste-ready)

**Title:** auth: harden AuthToken alphabet invariant via type-state, not docstring

**Body:**

> `AuthToken::from_string` is currently `#[cfg(test)]` so production code cannot construct a token with arbitrary bytes. The contract that the alphabet is URL-safe base64 (and that `HeaderValue::from_str(&cookie)` therefore cannot fail) is enforced only by the docstring on `random()`. The `.expect(…)` at `auth.rs:218` is a real panic if the invariant ever fails.
>
> If a future feature lifts the cfg-gate (e.g. "stable token from env var" for scripted automation) and the env var contains CR/LF, `;`, or `,`, the worker panics on every redirect or smuggles a second cookie pair into the `Set-Cookie` header. Today the path is unreachable from outside tests, but the type does not enforce the invariant; one cfg-gate change is all it takes.
>
> **Why deferred:** a clean fix is a type-state refactor — wrap the inner `String` in a `pub struct AuthToken(String)` newtype constructed only via `random()` / a validated `from_str_validated`. Both constructors verify `[A-Za-z0-9_-]{32,}` and panic at construction, not at use. That changes the `pub` surface of the `auth` module and ripples into every test that builds tokens by hand.
>
> **Pointers:**
> - construction sites: `src/serve/auth.rs:75-78` (`from_string`), `src/serve/auth.rs:60-72` (`random`)
> - panic site that would convert into a real safety proof: `src/serve/auth.rs:218-220`
> - test usage to migrate: search `AuthToken::from_string` under `src/serve/`

---

## P4.2 — Path=/ cookie scope on 127.0.0.1 — multi-instance hijack

**Disposition:** GitHub issue
**Files:** `src/serve/auth.rs:211-214` (`Set-Cookie: cqs_token={token}; Path=/; HttpOnly; SameSite=Strict`)

### Issue body (paste-ready)

**Title:** serve: localhost cookie collision when running multiple `cqs serve` instances

**Body:**

> Browsers scope cookies by `(host, path)` but **not by port**. Two `cqs serve` instances on the same machine but different ports (project A on 8080, project B on 8081) both set `cqs_token` with `Path=/`. Authenticating to one overwrites the cookie set by the other; the previously-authenticated tab silently 401s on every navigation, and (worse) any link sends the wrong token to the wrong server.
>
> Threat model: an attacker who can convince a victim to run `cqs serve` against a malicious project on a port the attacker controls can drop any cookie they like into the victim's localhost cookie jar — combined with `SameSite=Strict`-bypass via top-level navigation, this is a real cookie-jar overwrite vector.
>
> **Steps:**
> 1. Run `cqs serve --port 8080` in project A; auth via the browser banner.
> 2. Run `cqs serve --port 8081` in project B; auth via the browser banner.
> 3. Reload project A's tab — every request now 401s because the project B token clobbered it.
>
> **Why deferred:** clean fixes all have trade-offs. `__Host-` cookie prefix requires `Secure`, which loopback HTTP doesn't satisfy. Port-suffixed cookie names (`cqs_token_8080`) increase knob count and break URL-bar bookmarks across launches when the port floats. Path-rewriting (`Path=/api/__cqs_<port>/`) is heavy. Pragmatic option: derive cookie name from a hash of `(bind_addr, launch_time)` so two instances don't collide and a new launch invalidates the old cookie — but that changes the CLI launch banner (the printed token URL must include the cookie-name salt) and requires a one-time UX audit.
>
> **Pointers:**
> - cookie set: `src/serve/auth.rs:211-214`
> - cookie read: `src/serve/auth.rs:158-172` (`check_request`)
> - launch-banner path: `src/serve/mod.rs:111-117`
> - existing acknowledgement of the issue: `src/serve/auth.rs:42-47`

---

## P4.3 — `Option<AuthToken>` permits silent no-auth router

**Disposition:** GitHub issue
**Files:** `src/serve/mod.rs:78-83` (`run_server` signature), `src/serve/mod.rs:154-178` (`build_router`)

### Issue body (paste-ready)

**Title:** serve: type-state `AuthMode` to prevent silently building a no-auth router

**Body:**

> `run_server` and `build_router` both take `auth: Option<AuthToken>`. Passing `None` silently disables auth — there is no compile-time gate. The `cmd_serve` entry path correctly defaults to `Some(random())` and only opts into `None` on `--no-auth`, but the type does not constrain future internal callers (an embedded smoke test, a feature-gated dev mode, an alternate CLI surface) from passing `None` and shipping a fully open server. The only runtime signal is an `eprintln!` gated on `quiet == false` (`mod.rs:120-123`).
>
> Today `run_server(store, addr, /* quiet */ true, /* auth */ None)` ships a wide-open server with zero output. The "default secure" property lives in convention, not in the type system. The equivalent invariant in `cqs ref add` is enforced by validate-by-construction; here it is not.
>
> **Why deferred:** the clean fix is a type-state refactor — replace `Option<AuthToken>` with `enum AuthMode { Required(AuthToken), Disabled { ack: NoAuthAcknowledgement } }` where `NoAuthAcknowledgement` is a `pub` zero-sized type only constructable inside `cmd_serve` after the `--no-auth` flag is parsed. Future internal callers cannot instantiate the disabled variant without intentionally importing the proof type. This ripples into every test that builds a router today via `build_router(.., None)` and the public-ish `run_server` signature.
>
> **Cheaper interim mitigation** (one-line, can land before the refactor): in the `None` arm of `build_router` add a `tracing::error!("serve: AUTH DISABLED — request layer is open")` so any caller (test or future) shows up loudly in journald. Not a substitute for the type-state fix; just blast-radius limiter.
>
> **Pointers:**
> - signature: `src/serve/mod.rs:78-83`
> - router construction: `src/serve/mod.rs:154-178`
> - the only caller that legitimately passes `None`: `src/cli/commands/serve.rs:61-65` (after `--no-auth` parse)
> - banner suppression: `src/serve/mod.rs:120-123`

---
