# v1.30.0 Audit — P1 Fix Prompts

21 fix prompts for the P1 (easy + high-impact) findings. Verified against current source 2026-04-26.

Notes on drift:
- P1.16 — `--name-boost` CLI parser is already `parse_unit_f32` (clap-bounded). The audit text was stale; only the **defense-in-depth** clamp at the consumer remains.
- All other P1 line citations verified to current source.

---

## P1.1 + P1.2 — PRIVACY/SECURITY lie about query_log opt-in and query_cache TTL

**Finding:** P1.1 + P1.2 in audit-triage.md / Documentation in audit-findings.md (DOC-V1.30-1, DOC-V1.30-2)
**Files:** `PRIVACY.md:21-22`, `SECURITY.md:101`, `src/cli/batch/commands.rs:371-399`
**Why:** PRIVACY claims query_log is opt-in but `log_query` is unconditional. PRIVACY claims 7-day TTL on query_cache; only size cap exists. Both are P1 "docs lying" — fix the docs (defensible direction; gating the log requires touching every dispatch arm and changing user-visible behaviour).

### Current docs (PRIVACY.md:21-22)

```markdown
- `~/.cache/cqs/query_cache.db` — recent query embeddings with a 7-day TTL. Speeds up repeated searches.
- `~/.cache/cqs/query_log.jsonl` — opt-in query log, written only when `CQS_TELEMETRY=1` or the file already exists. Stays local.
```

### Replacement (PRIVACY.md:21-22)

```markdown
- `~/.cache/cqs/query_cache.db` — recent query embeddings, evicted oldest-first when the DB exceeds `CQS_QUERY_CACHE_MAX_SIZE` (100 MiB default). Prune older entries with `cqs cache prune <DAYS>`. Speeds up repeated searches.
- `~/.cache/cqs/query_log.jsonl` — local query log written by every `cqs chat` / `cqs batch` invocation (search/gather/onboard/scout/where/task). Append-only JSONL. Delete the file to disable; `cqs cache clear` does not remove it. Stays local.
```

### Current docs (SECURITY.md:101)

```markdown
| `~/.cache/cqs/query_log.jsonl` | Opt-in local query log | `CQS_TELEMETRY=1` or file exists |
```

### Replacement (SECURITY.md:101)

```markdown
| `~/.cache/cqs/query_log.jsonl` | Local query log (append-only) | `cqs chat` / `cqs batch` (search, gather, onboard, scout, where, task) |
```

### Notes

- The "fix the code" alternative (gate `log_query` on `CQS_TELEMETRY=1` plus existing-file check) is the *more defensible* contract but a behaviour change. Pick docs-fix here unless reviewer explicitly opts into a behaviour change. If the reviewer chooses code-fix, gate the body of `log_query` (`src/cli/batch/commands.rs:371`) on `std::env::var("CQS_TELEMETRY").as_deref() == Ok("1") || log_path.exists()` returning early otherwise.

---

## P1.3 — CHANGELOG names CQS_LLM_ENDPOINT — actual var is CQS_LLM_API_BASE

**Finding:** P1.3 in audit-triage.md / Documentation DOC-V1.30-3
**Files:** `CHANGELOG.md:19`
**Why:** CHANGELOG v1.30.0 entry references `CQS_LLM_ENDPOINT`, which does not exist in the codebase. Actual env var is `CQS_LLM_API_BASE` (plus `CQS_LLM_PROVIDER=local`). User copy-pasting fails at runtime.

### Current

```markdown
- **Local LLM provider (OpenAI-compatible)** — `cqs index --llm-summaries` accepts a local OpenAI-compatible endpoint via `CQS_LLM_ENDPOINT`, in addition to the existing Anthropic Batches API path (#1101 — closes audit finding EX-V1.29-3). `LlmProvider` trait, `LocalProvider` implementation, end-to-end summary generation via local vLLM / Ollama / etc.
```

### Replacement

```markdown
- **Local LLM provider (OpenAI-compatible)** — `cqs index --llm-summaries` accepts a local OpenAI-compatible endpoint via `CQS_LLM_PROVIDER=local` + `CQS_LLM_API_BASE=<url>`, in addition to the existing Anthropic Batches API path (#1101 — closes audit finding EX-V1.29-3). `LlmProvider` trait, `LocalProvider` implementation, end-to-end summary generation via local vLLM / Ollama / etc.
```

---

## P1.4 — CONTRIBUTING tells contributors to edit dispatch.rs (registry.rs now)

**Finding:** P1.4 in audit-triage.md / Documentation DOC-V1.30-4
**Files:** `CONTRIBUTING.md:339-355` (checklist), `CONTRIBUTING.md:153-158` (Architecture Overview)
**Why:** v1.30.0 #1097/#1114 collapsed five exhaustive matches in `dispatch.rs`/`definitions.rs` into one `for_each_command!` table in `src/cli/registry.rs`. The contributor checklist still says "match arm in `src/cli/dispatch.rs`" with no mention of `registry.rs`.

### Current (CONTRIBUTING.md:339-355)

```markdown
## Adding a New CLI Command

Checklist for every new command:

1. **Implementation** — `src/cli/commands/<category>/<name>.rs` with the core logic (pick category: search/, graph/, review/, index/, io/, infra/, train/)
2. **Category mod.rs** — add `mod <name>;` + `pub(crate) use <name>::*;` in `src/cli/commands/<category>/mod.rs`
3. **CLI definition** — `Commands` enum variant in `src/cli/definitions.rs` with clap args
4. **Dispatch** — match arm in `src/cli/dispatch.rs`
5. **`--json` support** — serde serialization for programmatic output
6. **Tracing** — `tracing::info_span!` at entry, `tracing::warn!` on error fallback
7. **Error handling** — `Result` propagation, no bare `.unwrap_or_default()` in production
8. **Tests** — happy path + empty input + error path + edge cases
9. **CLAUDE.md** — add to the command reference section
10. **Skills** — add to `.claude/skills/cqs/SKILL.md` and `.claude/skills/cqs-bootstrap/SKILL.md`
11. **CHANGELOG** — entry in the next release section
```

### Replacement (CONTRIBUTING.md:339-355)

```markdown
## Adding a New CLI Command

Checklist for every new command:

1. **Implementation** — `src/cli/commands/<category>/<name>.rs` with the core logic (pick category: search/, graph/, review/, index/, io/, infra/, train/)
2. **Category mod.rs** — add `mod <name>;` + `pub(crate) use <name>::*;` in `src/cli/commands/<category>/mod.rs`
3. **CLI definition** — `Commands` enum variant in `src/cli/definitions.rs` with clap args
4. **Registry row** — add a `(bind, wild, name, batch_support, body)` row to `group_a` or `group_b` in `src/cli/registry.rs`. The `for_each_command!` macro generates dispatch + variant_name + batch_support from this single row; a missing row is a compile error.
5. **`--json` support** — serde serialization for programmatic output
6. **Tracing** — `tracing::info_span!` at entry, `tracing::warn!` on error fallback
7. **Error handling** — `Result` propagation, no bare `.unwrap_or_default()` in production
8. **Tests** — happy path + empty input + error path + edge cases
9. **CLAUDE.md** — add to the command reference section
10. **Skills** — add to `.claude/skills/cqs/SKILL.md` and `.claude/skills/cqs-bootstrap/SKILL.md`
11. **CHANGELOG** — entry in the next release section
```

### Current (Architecture Overview, CONTRIBUTING.md:153-158)

```markdown
  cli/          - Command-line interface (clap)
    mod.rs      - Top-level CLI module, re-exports
    definitions.rs - Clap argument definitions and command enum
    dispatch.rs - Command dispatch (match on command, call handlers)
    commands/   - Command implementations (organized by category)
```

### Replacement (Architecture Overview)

```markdown
  cli/          - Command-line interface (clap)
    mod.rs      - Top-level CLI module, re-exports
    definitions.rs - Clap argument definitions and command enum
    registry.rs - `for_each_command!` table; single source of truth for dispatch + variant_name + batch_support
    dispatch.rs - Command dispatch helpers (entry points; per-command arms generated from `registry.rs`)
    commands/   - Command implementations (organized by category)
```

---

## P1.5 — ProjectRegistry doc lies about path on macOS/Windows

**Finding:** P1.5 in audit-triage.md / Platform Behavior section
**Files:** `src/project.rs:1-3, 176-179`
**Why:** Module-level doc claims `~/.config/cqs/projects.toml` but `dirs::config_dir()` returns macOS-specific (`~/Library/Application Support/`) and Windows-specific (`%APPDATA%\`) paths. Path is constructed correctly; only the doc is wrong.

### Current code (src/project.rs:1-4)

```rust
//! Cross-project search via global project registry.
//!
//! Maintains a registry of indexed projects at `~/.config/cqs/projects.toml`.
//! Enables searching across all registered projects from anywhere.
```

### Replacement

```rust
//! Cross-project search via global project registry.
//!
//! Maintains a registry of indexed projects in the platform config directory
//! (via `dirs::config_dir()`):
//! - Linux: `~/.config/cqs/projects.toml`
//! - macOS: `~/Library/Application Support/cqs/projects.toml`
//! - Windows: `%APPDATA%\cqs\projects.toml`
//!
//! Enables searching across all registered projects from anywhere.
```

### Current code (src/project.rs:175-179)

```rust
/// Get the registry file path
fn registry_path() -> Result<PathBuf, ProjectError> {
    let config_dir = dirs::config_dir().ok_or(ProjectError::ConfigDirNotFound)?;
    Ok(config_dir.join("cqs").join("projects.toml"))
}
```

### Replacement

```rust
/// Get the registry file path.
///
/// Resolves via `dirs::config_dir()`:
/// - Linux: `~/.config/cqs/projects.toml`
/// - macOS: `~/Library/Application Support/cqs/projects.toml`
/// - Windows: `%APPDATA%\cqs\projects.toml`
fn registry_path() -> Result<PathBuf, ProjectError> {
    let config_dir = dirs::config_dir().ok_or(ProjectError::ConfigDirNotFound)?;
    Ok(config_dir.join("cqs").join("projects.toml"))
}
```

---

## P1.6 — gather warning hardcodes "200" — lies when CQS_GATHER_MAX_NODES set

**Finding:** P1.6 in audit-triage.md / Code Quality
**Files:** `src/cli/commands/search/gather.rs:200`, `src/gather.rs:153-172` (`gather_max_nodes`)
**Why:** Text-mode warning says "capped at 200 nodes" but actual cap obeys `CQS_GATHER_MAX_NODES`. With `CQS_GATHER_MAX_NODES=500`, the user sees "capped at 200" while results were capped at 500.

### Current code (src/cli/commands/search/gather.rs:199-201)

```rust
        if result.expansion_capped {
            println!("{}", "Warning: expansion capped at 200 nodes".yellow());
        }
```

### Replacement

```rust
        if result.expansion_capped {
            let cap = cqs::gather::gather_max_nodes();
            println!(
                "{}",
                format!("Warning: expansion capped at {cap} nodes").yellow()
            );
        }
```

### Notes

- `gather_max_nodes` is already memoized via `OnceLock` (per `src/gather.rs:153-172`), so this is cheap.
- Verify that `gather::gather_max_nodes` is `pub`; if not, expose it (`pub fn`) and re-export from `lib.rs` if needed.
- Stretch goal flagged in audit: surface `expansion_cap_used: usize` on `GatherResult` so `--json` consumers also see the real cap. Out of scope for the easy fix here.

---

## P1.7 — Reranker silently ignores [reranker] config section

**Finding:** P1.7 in audit-triage.md / Code Quality
**Files:** `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)`), production callers `src/cli/store.rs:276`, `src/cli/batch/mod.rs:1274`
**Why:** `resolve_reranker` accepts `Option<&AuxModelSection>` to thread `Config::reranker` (`src/config.rs:228`) through, but `Reranker::model_paths` always passes `None`. A user `.cqs.toml` `[reranker] preset = "bge-reranker-base"` is silently dropped — user gets default ms-marco-MiniLM with no error or warn.

### Current code (src/reranker.rs:106-154)

```rust
pub struct Reranker {
    session: Mutex<Option<Session>>,
    tokenizer: Mutex<Option<Arc<tokenizers::Tokenizer>>>,
    model_paths: OnceCell<(PathBuf, PathBuf)>,
    provider: ExecutionProvider,
    max_length: usize,
    expects_token_type_ids: Mutex<Option<bool>>,
}

impl Reranker {
    /// Create a new reranker with lazy model loading
    pub fn new() -> Result<Self, RerankerError> {
        let provider = select_provider();
        let max_length = match std::env::var("CQS_RERANKER_MAX_LENGTH") {
            // ... unchanged ...
        };
        Ok(Self {
            session: Mutex::new(None),
            tokenizer: Mutex::new(None),
            model_paths: OnceCell::new(),
            provider,
            max_length,
            expects_token_type_ids: Mutex::new(None),
        })
    }
```

### Current code (src/reranker.rs:442-446)

```rust
    fn model_paths(&self) -> Result<&(PathBuf, PathBuf), RerankerError> {
        self.model_paths.get_or_try_init(|| {
            let _span = tracing::info_span!("reranker_model_resolve").entered();

            let resolved = resolve_reranker(None)?;
```

### Replacement (struct + constructors, src/reranker.rs:106-154)

```rust
pub struct Reranker {
    session: Mutex<Option<Session>>,
    tokenizer: Mutex<Option<Arc<tokenizers::Tokenizer>>>,
    model_paths: OnceCell<(PathBuf, PathBuf)>,
    provider: ExecutionProvider,
    max_length: usize,
    expects_token_type_ids: Mutex<Option<bool>>,
    /// Cached config-file `[reranker]` section so `resolve_reranker` honours
    /// `preset` / `model_path` / `tokenizer_path` set in `.cqs.toml`.
    section: Option<AuxModelSection>,
}

impl Reranker {
    /// Create a new reranker with lazy model loading (config-less; CLI/env only).
    pub fn new() -> Result<Self, RerankerError> {
        Self::with_section(None)
    }

    /// Create a reranker, threading a `[reranker]` config section through to
    /// `resolve_reranker` so `.cqs.toml` preset / model_path are honoured.
    pub fn with_section(section: Option<AuxModelSection>) -> Result<Self, RerankerError> {
        let provider = select_provider();
        let max_length = match std::env::var("CQS_RERANKER_MAX_LENGTH") {
            // ... unchanged body ...
        };
        Ok(Self {
            session: Mutex::new(None),
            tokenizer: Mutex::new(None),
            model_paths: OnceCell::new(),
            provider,
            max_length,
            expects_token_type_ids: Mutex::new(None),
            section,
        })
    }
```

### Replacement (model_paths, src/reranker.rs:442-446)

```rust
    fn model_paths(&self) -> Result<&(PathBuf, PathBuf), RerankerError> {
        self.model_paths.get_or_try_init(|| {
            let _span = tracing::info_span!("reranker_model_resolve").entered();

            let resolved = resolve_reranker(self.section.as_ref())?;
```

### Replacement (callers)

`src/cli/store.rs:276`:
```rust
        let r = cqs::Reranker::with_section(config.reranker.clone())
            .map_err(|e| anyhow::anyhow!("Reranker init failed: {e}"))?;
```

`src/cli/batch/mod.rs:1274`:
```rust
        let r = cqs::Reranker::with_section(config.reranker.clone())
            .map_err(|e| anyhow::anyhow!("Reranker init failed: {e}"))?;
```

### Notes

- `AuxModelSection` is owned (`Clone`-able) per `src/config.rs:228`; cloning is cheap (small struct of `Option<String>`s).
- Need `pub use` of `AuxModelSection` if not already exported from `cqs::` lib; check `src/lib.rs`.
- Re-export verify: `cqs::Reranker::with_section` must compile from outside the crate root (`cli` is in-crate, fine).
- Test sites in `src/reranker.rs:712, 718, 772, 823` continue using `Reranker::new()` (no section needed).
- Add a regression test `with_section_preset_overrides_default` that constructs a `Reranker::with_section(Some(AuxModelSection { preset: Some("bge-reranker-base".into()), .. }))`, calls `model_paths()`, and asserts the resolved repo matches the bge preset (not ms-marco).

---

## P1.8 — Embedder fingerprint falls back to repo:timestamp — cache thrash

**Finding:** P1.8 in audit-triage.md / Error Handling + Data Safety
**Files:** `src/embedder/mod.rs:435-466`
**Why:** Three error arms fall back to `format!("{}:{}", self.model_config.repo, ts)` where `ts = SystemTime::now()`. Every restart with a transient hash failure (AV scanner, EBUSY, I/O hiccup) writes cache rows under a NEW timestamp, so subsequent reads miss the cache forever. Cross-slot copy by `content_hash` also breaks because the model fingerprint isn't stable.

### Current code (src/embedder/mod.rs:432-466)

```rust
                                            hash
                                        }
                                        Err(e) => {
                                            tracing::warn!(error = %e, "Failed to stream-hash model, using repo+timestamp fallback");
                                            let ts = std::time::SystemTime::now()
                                                .duration_since(std::time::UNIX_EPOCH)
                                                .unwrap_or_default()
                                                .as_secs();
                                            format!("{}:{}", self.model_config.repo, ts)
                                        }
                                    }
                                }
                                Err(e) => {
                                    tracing::warn!(error = %e, "Failed to open model for fingerprint, using repo+timestamp fallback");
                                    let ts = std::time::SystemTime::now()
                                        .duration_since(std::time::UNIX_EPOCH)
                                        .unwrap_or_default()
                                        .as_secs();
                                    format!("{}:{}", self.model_config.repo, ts)
                                }
                            }
                        }
                    }
                }
                Err(e) => {
                    tracing::warn!(error = %e, "Failed to get model paths for fingerprint, using repo+timestamp fallback");
                    let ts = std::time::SystemTime::now()
                        .duration_since(std::time::UNIX_EPOCH)
                        .unwrap_or_default()
                        .as_secs();
                    format!("{}:{}", self.model_config.repo, ts)
                }
            }
        })
    }
```

### Replacement

Replace each `repo:timestamp` fallback with a stable `repo:fallback:size=<bytes>` (or `:size=unknown`) shape. The size+repo proxy is deterministic across restarts within the same model file.

```rust
                                            hash
                                        }
                                        Err(e) => {
                                            tracing::warn!(
                                                error = %e,
                                                "Failed to stream-hash model, using repo+size fallback (cache may miss until next successful hash)"
                                            );
                                            let size = std::fs::metadata(model_path)
                                                .ok()
                                                .map(|m| m.len())
                                                .unwrap_or(0);
                                            format!("{}:fallback:size={}", self.model_config.repo, size)
                                        }
                                    }
                                }
                                Err(e) => {
                                    tracing::warn!(
                                        error = %e,
                                        "Failed to open model for fingerprint, using repo+size fallback"
                                    );
                                    let size = std::fs::metadata(model_path)
                                        .ok()
                                        .map(|m| m.len())
                                        .unwrap_or(0);
                                    format!("{}:fallback:size={}", self.model_config.repo, size)
                                }
                            }
                        }
                    }
                }
                Err(e) => {
                    tracing::warn!(
                        error = %e,
                        "Failed to get model paths for fingerprint, using repo-only fallback"
                    );
                    format!("{}:fallback:no-path", self.model_config.repo)
                }
            }
        })
    }
```

### Notes

- The "model path resolution failed" arm (line 457) cannot read file size (path unknown), so use `:fallback:no-path` as a stable sentinel — still not time-varying.
- Audit suggests promoting the failure to a hard error. That's a behaviour change with downstream surface (every `Embedder::*` call now returns `Result<&str, EmbedderError>`). Out of scope here; the size-fallback closes the cache-thrash bug without breaking existing call signatures.
- `cqs doctor` should warn when `model_fingerprint` starts with `:fallback:` — separate small follow-up; cite as `audit doctor warn fallback fingerprint`.

---

## P1.9 — LocalProvider Mutex::into_inner().unwrap_or_default() loses all batch results on poison

**Finding:** P1.9 in audit-triage.md / Error Handling
**Files:** `src/llm/local.rs:271, 272, 278, 279, 305, 308`
**Why:** On `Mutex::into_inner` returning `Err` (poison), `unwrap_or_default()` substitutes an empty `HashMap` and `submit_via_chat_completions` reports `succeeded` ≠ map size with no error signal. Downstream `fetch_batch_results` returns `{}` and the user persists nothing.

### Current code (src/llm/local.rs:271-309)

```rust
        let ok = *succeeded.lock().unwrap();
        let err = *failed.lock().unwrap();
        let elapsed_ms = start.elapsed().as_millis() as u64;

        // Fatal-batch check: if every item that talked to the server saw
        // 401/403 on its first request, the credentials are wrong — abort
        // with a specific error instead of silently returning an empty stash.
        let auth_fail = *auth_failures.lock().unwrap();
        let auth_attempt = *auth_attempts.lock().unwrap();
        if auth_attempt > 0 && auth_fail == auth_attempt {
            tracing::error!(
                url = %self.api_base,
                "local batch aborted: all {} requests rejected with 401/403",
                auth_attempt
            );
            return Err(LlmError::Api {
                status: 401,
                message: format!(
                    "Authentication rejected at {}; check CQS_LLM_API_KEY",
                    self.api_base
                ),
            });
        }

        tracing::info!(
            batch_id = %batch_id,
            submitted = items.len(),
            succeeded = ok,
            failed = err,
            elapsed_ms,
            "local batch complete"
        );

        // Move results into the stash under the batch id.
        let results_map = results.into_inner().unwrap_or_default();
        self.stash
            .lock()
            .unwrap()
            .insert(batch_id.clone(), results_map);

        Ok(batch_id)
    }
```

### Replacement

```rust
        // Recover counters even on poison — counts are advisory and dropping
        // the count to 0 would mask real progress in the "complete" log.
        let ok = *succeeded.lock().unwrap_or_else(|p| p.into_inner());
        let err = *failed.lock().unwrap_or_else(|p| p.into_inner());
        let elapsed_ms = start.elapsed().as_millis() as u64;

        // Fatal-batch check: if every item that talked to the server saw
        // 401/403 on its first request, the credentials are wrong — abort
        // with a specific error instead of silently returning an empty stash.
        let auth_fail = *auth_failures.lock().unwrap_or_else(|p| p.into_inner());
        let auth_attempt = *auth_attempts.lock().unwrap_or_else(|p| p.into_inner());
        if auth_attempt > 0 && auth_fail == auth_attempt {
            tracing::error!(
                url = %self.api_base,
                "local batch aborted: all {} requests rejected with 401/403",
                auth_attempt
            );
            return Err(LlmError::Api {
                status: 401,
                message: format!(
                    "Authentication rejected at {}; check CQS_LLM_API_KEY",
                    self.api_base
                ),
            });
        }

        tracing::info!(
            batch_id = %batch_id,
            submitted = items.len(),
            succeeded = ok,
            failed = err,
            elapsed_ms,
            "local batch complete"
        );

        // Move results into the stash under the batch id. On poison we recover
        // the partially-populated map rather than silently substituting an
        // empty one — losing partial results is worse than the panic risk.
        let results_map = match results.into_inner() {
            Ok(m) => m,
            Err(poisoned) => {
                tracing::error!(
                    succeeded = ok,
                    "results mutex poisoned during local batch — recovering inner state"
                );
                poisoned.into_inner()
            }
        };

        // Invariant: if results_map.len() != ok, accounting drifted. Surface
        // it loudly rather than shipping a short stash silently.
        if results_map.len() != ok {
            tracing::error!(
                map_len = results_map.len(),
                succeeded = ok,
                "local batch accounting drift: results map size != succeeded count"
            );
            return Err(LlmError::Internal(format!(
                "local batch accounting drift: ok={ok} map_len={}",
                results_map.len()
            )));
        }

        self.stash
            .lock()
            .unwrap_or_else(|p| p.into_inner())
            .insert(batch_id.clone(), results_map);

        Ok(batch_id)
    }
```

### Notes

- `LlmError::Internal(String)` — verify this variant exists in `src/llm/mod.rs::LlmError`. If not, add it: `#[error("Internal error: {0}")] Internal(String)`.
- The `if let Ok(mut map) = results_ref.lock()` at line 196 (worker) intentionally tolerates poison (drops the result) — leave it. The poison cascade fix is the *finalisation* path.
- The same poison pattern exists at line 393-396 for `auth_attempts/auth_failures` increment inside the worker; that's tracked by P2.35 (RB-V1.30-7) as a separate sweep.

---

## P1.10 — LocalProvider unbounded HTTP body read — OOM on hostile/buggy server

**Finding:** P1.10 in audit-triage.md / Robustness RB-V1.30-1
**Files:** `src/llm/local.rs:97-100` (Client builder), `src/llm/local.rs:474-487` (`parse_choices_content`), `src/llm/local.rs:490-500` (`body_preview`)
**Why:** `reqwest::blocking::Client` builder sets timeout + redirect only — no body cap. `resp.json::<Value>()` and `resp.text()` buffer the entire response. A panicked / hostile / misconfigured OpenAI-compat server can return a multi-GB body and OOM the daemon. Up to `local_concurrency()` (≤64) workers compound the risk.

### Current code (src/llm/local.rs:88-115)

```rust
    pub fn new(llm_config: LlmConfig) -> Result<Self, LlmError> {
        let _span = tracing::info_span!("local_provider_new").entered();

        let concurrency = local_concurrency();
        let timeout = local_timeout();
        let api_key = std::env::var("CQS_LLM_API_KEY")
            .ok()
            .filter(|s| !s.is_empty());

        let http = Client::builder()
            .timeout(timeout)
            .redirect(reqwest::redirect::Policy::none())
            .build()?;

        tracing::info!(
            api_base = %llm_config.api_base,
            model = %llm_config.model,
            concurrency,
            timeout_secs = timeout.as_secs(),
            auth = api_key.is_some(),
            "LocalProvider ready"
        );

        Ok(Self {
            http,
            api_base: llm_config.api_base,
            model: llm_config.model,
```

### Current code (src/llm/local.rs:474-500)

```rust
fn parse_choices_content(resp: reqwest::blocking::Response) -> Result<Option<String>, LlmError> {
    let body: serde_json::Value = resp.json()?;
    // ... unchanged ...
}

fn body_preview(resp: reqwest::blocking::Response) -> String {
    let body = resp.text().unwrap_or_default();
    let cut = body
        .char_indices()
        .nth(256)
        .map(|(i, _)| i)
        .unwrap_or(body.len());
    body[..cut].to_string()
}
```

### Replacement (Client builder, lines 97-100)

No change here — keep timeout + redirect. Apply the cap at the read sites instead.

### Replacement (parse_choices_content, lines 474-488)

```rust
/// Hard cap on response body size. Summary outputs are typically a few hundred
/// bytes; 4 MiB is ~1000× headroom. Larger bodies are a sign of a misbehaving
/// or hostile endpoint and we'd rather error than OOM the daemon.
///
/// Override via `CQS_LOCAL_LLM_MAX_BODY_BYTES`.
fn local_max_body_bytes() -> usize {
    static MAX: std::sync::OnceLock<usize> = std::sync::OnceLock::new();
    *MAX.get_or_init(|| {
        std::env::var("CQS_LOCAL_LLM_MAX_BODY_BYTES")
            .ok()
            .and_then(|v| v.parse().ok())
            .filter(|&n: &usize| n > 0)
            .unwrap_or(4 * 1024 * 1024)
    })
}

fn parse_choices_content(resp: reqwest::blocking::Response) -> Result<Option<String>, LlmError> {
    use std::io::Read;
    let cap = local_max_body_bytes();
    let mut buf = Vec::with_capacity(8 * 1024);
    resp.take(cap as u64 + 1).read_to_end(&mut buf).map_err(|e| {
        LlmError::Http(format!("response body read failed: {e}"))
    })?;
    if buf.len() > cap {
        return Err(LlmError::Http(format!(
            "response body exceeds cap ({} > {})",
            buf.len(),
            cap
        )));
    }
    let body: serde_json::Value = serde_json::from_slice(&buf).map_err(|e| {
        LlmError::Http(format!("response body not valid JSON: {e}"))
    })?;
    let content = body
        .get("choices")
        .and_then(|v| v.as_array())
        .and_then(|arr| arr.first())
        .and_then(|first| first.get("message"))
        .and_then(|m| m.get("content"))
        .and_then(|v| v.as_str())
        .map(|s| s.trim().to_string());
    match content {
        Some(s) if !s.is_empty() => Ok(Some(s)),
        _ => Ok(None),
    }
}
```

### Replacement (body_preview, lines 490-500)

```rust
/// Read up to 2 KiB from an HTTP error response body for log context.
/// Returns the empty string if the body can't be read or is non-UTF-8.
/// Hard-capped to bound log spam and prevent OOM on hostile error bodies.
fn body_preview(resp: reqwest::blocking::Response) -> String {
    use std::io::Read;
    const PREVIEW_CAP: u64 = 2 * 1024;
    let mut buf = Vec::with_capacity(PREVIEW_CAP as usize);
    if resp.take(PREVIEW_CAP).read_to_end(&mut buf).is_err() {
        return String::new();
    }
    let body = String::from_utf8_lossy(&buf);
    let cut = body
        .char_indices()
        .nth(256)
        .map(|(i, _)| i)
        .unwrap_or(body.len());
    body[..cut].to_string()
}
```

### Notes

- `reqwest::blocking::Response` implements `Read`, and the builder's `.take(n)` method on a Read returns a length-capped adapter. The wrapping `take(cap as u64 + 1)` is so we can detect "exceeded cap" by reading 1 byte beyond and bailing.
- Confirm `LlmError::Http(String)` variant signature; if it requires a typed inner (e.g. `LlmError::Http(reqwest::Error)`), introduce a sibling `LlmError::BodyTooLarge { got: usize, cap: usize }` instead.
- Add tests per the audit's TC-ADV-1.30-1 (P2.26): `test_oversized_response_body_capped_at_max` and `test_4xx_with_large_body_does_not_buffer_entire_body` in `src/llm/local.rs::tests`.

---

## P1.11 — Auth token leaked into TraceLayer span URI logging

**Finding:** P1.11 in audit-triage.md / Security
**Files:** `src/serve/mod.rs:195` (`TraceLayer::new_for_http()`), interacts with `src/serve/auth.rs:194-232`
**Why:** `TraceLayer::new_for_http()` records `http.uri` (full path + query string) on every span. The first browser navigation `GET /?token=<43 chars>` lands the token in the span at DEBUG. Anyone with `RUST_LOG=tower_http=debug` or running under journald sees the token persisted.

### Current code (src/serve/mod.rs:189-196)

```rust
        // OB-V1.29-5: TraceLayer emits a span per request plus
        // on-response events with latency + status. Handlers already
        // log entry via `tracing::info!`; this layer closes the loop
        // by logging completion, giving per-endpoint latency in the
        // journal without hand-wrapping every handler body.
        .layer(TraceLayer::new_for_http())
}
```

### Replacement

```rust
        // OB-V1.29-5: TraceLayer emits a span per request plus
        // on-response events with latency + status. Handlers already
        // log entry via `tracing::info!`; this layer closes the loop
        // by logging completion, giving per-endpoint latency in the
        // journal without hand-wrapping every handler body.
        //
        // SEC: customise MakeSpan to record path only, NOT the full URI —
        // the `?token=…` query param lands in span fields otherwise and
        // bleeds the per-launch token into journald / RUST_LOG=debug.
        .layer(
            TraceLayer::new_for_http().make_span_with(|req: &axum::extract::Request| {
                tracing::info_span!(
                    "http_request",
                    method = %req.method(),
                    path = %req.uri().path(),
                )
            }),
        )
}
```

### Notes

- The closure drops `query` and `version` fields; if downstream tooling depends on `http.version`, add `version = ?req.version()` (avoid `%` Display because some HTTP version reps may differ across deps).
- `Request` import: `use axum::extract::Request;` — already imported at top of file (line 25).
- Add a regression test in `src/serve/tests.rs::auth_tests` that asserts `?token=…` does not appear in any captured tracing event.

---

## P1.12 — enforce_host_allowlist passes through missing Host header — DNS-rebinding bypass

**Finding:** P1.12 in audit-triage.md / Security + Platform
**Files:** `src/serve/mod.rs:234-251`
**Why:** Doc-comment justifies missing-Host pass-through as "test ergonomic". HTTP/1.0 doesn't require Host; `nc 127.0.0.1 8080` with `GET /api/chunk/<id> HTTP/1.0\r\n\r\n` reaches the handler with zero allowlist check. Test ergonomics privileged over runtime safety.

### Current code (src/serve/mod.rs:230-251)

```rust
/// A missing `Host:` header passes through — HTTP/1.1 requires one and
/// hyper always provides one on real traffic, but unit tests built via
/// `Request::builder()` without a `.uri()` that includes a host don't
/// get one synthesized, and we'd rather not break that ergonomic.
async fn enforce_host_allowlist(
    State(allowed): State<AllowedHosts>,
    req: Request,
    next: Next,
) -> Result<Response, (StatusCode, &'static str)> {
    match req.headers().get(header::HOST) {
        None => Ok(next.run(req).await),
        Some(value) => {
            let host = value.to_str().unwrap_or("");
            if allowed.contains(host) {
                Ok(next.run(req).await)
            } else {
                tracing::warn!(host = %host, "serve: rejected request with disallowed Host header");
                Err((StatusCode::BAD_REQUEST, "disallowed Host header"))
            }
        }
    }
}
```

### Replacement

```rust
/// Reject requests with no `Host:` header. HTTP/1.1 requires one; HTTP/1.0
/// does not, but a no-Host request bypasses DNS-rebinding protection so
/// we treat it as malformed. Tests must build requests with a Host header
/// (see `src/serve/tests.rs` fixtures).
async fn enforce_host_allowlist(
    State(allowed): State<AllowedHosts>,
    req: Request,
    next: Next,
) -> Result<Response, (StatusCode, &'static str)> {
    match req.headers().get(header::HOST) {
        None => {
            tracing::warn!("serve: rejected request with missing Host header");
            Err((StatusCode::BAD_REQUEST, "missing Host header"))
        }
        Some(value) => {
            let host = value.to_str().unwrap_or("");
            if allowed.contains(host) {
                Ok(next.run(req).await)
            } else {
                tracing::warn!(host = %host, "serve: rejected request with disallowed Host header");
                Err((StatusCode::BAD_REQUEST, "disallowed Host header"))
            }
        }
    }
}
```

### Notes

- Existing tests in `src/serve/tests.rs` that use `Request::builder().uri("/foo")` must add `.header(header::HOST, "127.0.0.1:8080")` (or `localhost`) to the builder. Sweep needed:
  - `grep -n 'Request::builder()' src/serve/tests.rs` and add Host headers to fixtures.
- Add a positive test `test_missing_host_header_rejected` that builds a request without Host and asserts 400.

---

## P1.13 — Auth token printed to stdout — captured by journald for 30-day retention

**Finding:** P1.13 in audit-triage.md / Security
**Files:** `src/serve/mod.rs:111-117`
**Why:** Banner `println!("cqs serve listening on http://{actual}/?token={token}")` writes to stdout. Under systemd `StandardOutput=journal` (default) or container log drivers, the token persists into a 30-day-retention store the user doesn't think about. Token doesn't rotate during a long-lived launch.

### Current code (src/serve/mod.rs:105-128)

```rust
        if !quiet {
            // #1096: when auth is enabled, emit the paste-ready URL
            // (token + bind addr) so a fresh launch is one click away
            // from being usable. The token appears here once and is
            // never logged via tracing — auditors can grep for serve
            // banners separately from the structured log stream.
            match auth.as_ref() {
                Some(token) => {
                    println!(
                        "cqs serve listening on http://{actual}/?token={}",
                        token.as_str()
                    );
                }
                None => {
                    println!("cqs serve listening on http://{actual}");
                    eprintln!(
                        "WARN: --no-auth in use — anyone with network access to {actual} \
                         can read this index"
                    );
                }
            }
            println!("press Ctrl-C to stop");
        }
        tracing::info!(addr = %actual, auth_enabled = auth.is_some(), "cqs serve started");
```

### Replacement

```rust
        if !quiet {
            // #1096: when auth is enabled, emit the paste-ready URL
            // (token + bind addr) so a fresh launch is one click away
            // from being usable. The token appears here once and is
            // never logged via tracing — auditors can grep for serve
            // banners separately from the structured log stream.
            //
            // SEC: route the token-bearing banner to STDERR when stdout
            // is not a TTY. systemd `StandardOutput=journal` and container
            // log drivers persist stdout into a 30-day retention store —
            // stderr is similarly captured but is the conventional place
            // for "informational interactive output" and operators can
            // redirect it (`2>/dev/null`) without losing structured logs.
            // For a stronger guarantee, set `--no-banner` (TODO).
            let stdout_is_tty = std::io::IsTerminal::is_terminal(&std::io::stdout());
            match auth.as_ref() {
                Some(token) => {
                    let line = format!(
                        "cqs serve listening on http://{actual}/?token={}",
                        token.as_str()
                    );
                    if stdout_is_tty {
                        println!("{line}");
                    } else {
                        eprintln!("{line}");
                        eprintln!(
                            "(stdout is not a TTY; token-bearing banner routed to stderr to avoid \
                             persisting into journald/container logs)"
                        );
                    }
                }
                None => {
                    println!("cqs serve listening on http://{actual}");
                    eprintln!(
                        "WARN: --no-auth in use — anyone with network access to {actual} \
                         can read this index"
                    );
                }
            }
            println!("press Ctrl-C to stop");
        }
        tracing::info!(addr = %actual, auth_enabled = auth.is_some(), "cqs serve started");
```

### Notes

- `std::io::IsTerminal` stable since 1.70. MSRV is 1.95, fine.
- Stretch (out of scope here): Jupyter-style `--no-banner` flag that writes the URL to a `0o600` file in `$XDG_RUNTIME_DIR` instead. Tracking-issue material.
- Update CHANGELOG: "`cqs serve` no longer prints the auth-bearing URL to stdout when stdout is non-interactive (systemd / container) — banner routes to stderr instead. Set `--no-auth` if banner suppression is required."

---

## P1.14 — cqs serve has no RequestBodyLimitLayer — authenticated client can OOM via large POST

**Finding:** P1.14 in audit-triage.md / Security
**Files:** `src/serve/mod.rs:154-196` (`build_router`)
**Why:** All routes are `GET`, but axum buffers the request body before dispatching. No `RequestBodyLimitLayer` means an authenticated client can `POST /api/stats` with `Content-Length: 99999999999` and OOM the daemon before the 405.

### Current code (src/serve/mod.rs:158-196)

```rust
    let mut app = Router::new()
        .route("/health", get(handlers::health))
        .route("/api/stats", get(handlers::stats))
        // ... routes ...
        .with_state(state);

    if let Some(token) = auth {
        app = app.layer(from_fn_with_state(token, auth::enforce_auth));
    }

    app
        .layer(from_fn_with_state(allowed_hosts, enforce_host_allowlist))
        .layer(CompressionLayer::new())
        .layer(TraceLayer::new_for_http())
}
```

### Replacement

```rust
    let mut app = Router::new()
        .route("/health", get(handlers::health))
        .route("/api/stats", get(handlers::stats))
        // ... routes unchanged ...
        .with_state(state);

    if let Some(token) = auth {
        app = app.layer(from_fn_with_state(token, auth::enforce_auth));
    }

    app
        .layer(from_fn_with_state(allowed_hosts, enforce_host_allowlist))
        // SEC: cap request bodies. Every route is GET; legitimate clients
        // never send a body. 64 KiB is plenty for query strings and cookies
        // (which travel in headers, not body); axum still rejects bodies
        // larger than this with 413 Payload Too Large before allocating.
        .layer(tower_http::limit::RequestBodyLimitLayer::new(64 * 1024))
        .layer(CompressionLayer::new())
        // ... TraceLayer + MakeSpan customisation per P1.11 ...
}
```

### Notes

- `tower_http::limit::RequestBodyLimitLayer` is already in the dep tree (`tower_http` is imported at the top of `serve/mod.rs`); no Cargo.toml change. Verify with `cargo tree -p tower-http -e features 2>&1 | grep limit` — if not enabled, add `"limit"` to the `tower-http` feature list in `Cargo.toml`.
- Layer order matters: this sits *outside* auth/host-allowlist (so the limit applies even to rejected requests, preventing OOM-then-401 attacks) but *inside* compression so the 413 response is still gzipped.

---

## P1.15 — UMAP coords not invalidated on chunk content change — cluster view serves stale

**Finding:** P1.15 in audit-triage.md / Data Safety
**Files:** `src/store/chunks/async_helpers.rs:339-362` (UPSERT)
**Why:** v22 added `umap_x`/`umap_y` columns; `cqs index --umap` writes them. The chunk UPSERT refreshes embedding/embedding_base/etc. on content_hash mismatch but does NOT touch UMAP coords. Cluster view then displays the chunk at a position computed from the old embedding.

### Current code (src/store/chunks/async_helpers.rs:339-362)

```rust
        qb.push(
            " ON CONFLICT(id) DO UPDATE SET \
             origin=excluded.origin, \
             source_type=excluded.source_type, \
             language=excluded.language, \
             chunk_type=excluded.chunk_type, \
             name=excluded.name, \
             signature=excluded.signature, \
             content=excluded.content, \
             content_hash=excluded.content_hash, \
             doc=excluded.doc, \
             line_start=excluded.line_start, \
             line_end=excluded.line_end, \
             embedding=excluded.embedding, \
             embedding_base=excluded.embedding_base, \
             source_mtime=excluded.source_mtime, \
             updated_at=excluded.updated_at, \
             parent_id=excluded.parent_id, \
             window_idx=excluded.window_idx, \
             parent_type_name=excluded.parent_type_name, \
             parser_version=excluded.parser_version \
             WHERE chunks.content_hash != excluded.content_hash \
                OR chunks.parser_version != excluded.parser_version",
        );
```

### Replacement

Add `umap_x = NULL, umap_y = NULL` so the cluster view's `IS NOT NULL` filter correctly reports "needs reprojection". The CASE clause restricts the NULL-out to the content-changed branch — pure parser_version bumps don't invalidate UMAP because the embedding hasn't changed.

```rust
        qb.push(
            " ON CONFLICT(id) DO UPDATE SET \
             origin=excluded.origin, \
             source_type=excluded.source_type, \
             language=excluded.language, \
             chunk_type=excluded.chunk_type, \
             name=excluded.name, \
             signature=excluded.signature, \
             content=excluded.content, \
             content_hash=excluded.content_hash, \
             doc=excluded.doc, \
             line_start=excluded.line_start, \
             line_end=excluded.line_end, \
             embedding=excluded.embedding, \
             embedding_base=excluded.embedding_base, \
             source_mtime=excluded.source_mtime, \
             updated_at=excluded.updated_at, \
             parent_id=excluded.parent_id, \
             window_idx=excluded.window_idx, \
             parent_type_name=excluded.parent_type_name, \
             parser_version=excluded.parser_version, \
             umap_x=CASE WHEN chunks.content_hash != excluded.content_hash \
                         THEN NULL ELSE chunks.umap_x END, \
             umap_y=CASE WHEN chunks.content_hash != excluded.content_hash \
                         THEN NULL ELSE chunks.umap_y END \
             WHERE chunks.content_hash != excluded.content_hash \
                OR chunks.parser_version != excluded.parser_version",
        );
```

### Notes

- Add a regression test in `src/store/chunks/` (or wherever upsert tests live) that:
  1. Inserts a chunk with `umap_x=Some(1.0), umap_y=Some(2.0)` (write via `update_umap_coords_batch`).
  2. UPSERTs the same chunk with new content (different `content_hash`).
  3. Asserts `umap_x IS NULL AND umap_y IS NULL`.
- Stretch (out of scope): metadata `umap_generation` counter to surface "needs reprojection" warnings in `cqs serve`. Tracking-issue material.

---

## P1.16 — --name-boost CLI: defensive clamp at consumer (CLI parser already fixed)

**Finding:** P1.16 in audit-triage.md / Algorithm
**Files:** `src/cli/args.rs:62` (already uses `parse_unit_f32`), `src/search/scoring/candidate.rs:283-289`
**Why:** **LINE DRIFT:** the audit text claims CLI uses `parse_finite_f32`. Verified at `src/cli/args.rs:62`: `value_parser = parse_unit_f32` (clap-bounded `[0.0, 1.0]` per `src/cli/definitions.rs:137-143`). CLI side is already fixed. The remaining defense-in-depth concern: the consumer in `apply_scoring_pipeline` does not clamp, so if `name_boost` ever flows from a path that bypasses `parse_unit_f32` (programmatic library usage, future config-file path that skips `clamp_config_f32`, deserialization), the multiplication still sign-flips.

### Current code (src/search/scoring/candidate.rs:277-289)

```rust
pub(crate) fn apply_scoring_pipeline(
    embedding_score: f32,
    name: Option<&str>,
    file_part: &str,
    ctx: &ScoringContext<'_>,
) -> Option<f32> {
    let base_score = if let Some(matcher) = ctx.name_matcher {
        let n = name.unwrap_or("");
        let name_score = matcher.score(n);
        (1.0 - ctx.filter.name_boost) * embedding_score + ctx.filter.name_boost * name_score
    } else {
        embedding_score
    };
```

### Replacement

```rust
pub(crate) fn apply_scoring_pipeline(
    embedding_score: f32,
    name: Option<&str>,
    file_part: &str,
    ctx: &ScoringContext<'_>,
) -> Option<f32> {
    // Defense-in-depth: clamp name_boost into [0.0, 1.0] regardless of where
    // it originated. CLI uses parse_unit_f32 (clap-bounded) and config uses
    // clamp_config_f32, but a future programmatic / deserialised path could
    // bypass both, in which case `(1.0 - 5.0) * embedding` would sign-flip
    // search results silently. Cheap insurance.
    let name_boost = ctx.filter.name_boost.clamp(0.0, 1.0);
    let base_score = if let Some(matcher) = ctx.name_matcher {
        let n = name.unwrap_or("");
        let name_score = matcher.score(n);
        (1.0 - name_boost) * embedding_score + name_boost * name_score
    } else {
        embedding_score
    };
```

### Notes

- LINE DRIFT noted: searched for `parse_finite_f32` on `name_boost`, found `parse_unit_f32` at `src/cli/args.rs:62`. Audit text was stale; CLI parser fix already shipped (per audit `AC-V1.29-5`). Only the consumer-side defensive clamp remains.
- Add a test `test_apply_scoring_pipeline_clamps_out_of_range_name_boost` in `src/search/scoring/candidate.rs::tests` that constructs a `ScoringContext` with `name_boost = 5.0` and asserts the embedding signal is not negated.

---

## P1.17 — drain_pending_rebuild dedup drops fresh embeddings during rebuild window

**Finding:** P1.17 in audit-triage.md / Algorithm
**Files:** `src/cli/watch.rs:1077-1105`
**Why:** Non-blocking HNSW rebuild (#1113) snapshots `(id, embedding)` from a read-only Store, while the watch loop captures fresh `(id, embedding)` pairs into `pending.delta`. On swap, dedup is by id — if a chunk was re-embedded mid-rebuild (file edit), its fresh embedding lands in delta but `known` already contains the id with the OLD embedding from the snapshot. The filter drops the fresh embedding; HNSW carries stale vector until next threshold rebuild.

### Current code (src/cli/watch.rs:1077-1105)

```rust
    match outcome {
        Ok(Some(mut new_index)) => {
            // Replay captured delta — but skip ids the rebuild thread already
            // saw via its store snapshot, so we don't double-insert. (hnsw_rs
            // has no dedup; duplicate ids would create twin vectors that bloat
            // the graph until the next threshold cleans them up.)
            let known: std::collections::HashSet<&str> =
                new_index.ids().iter().map(String::as_str).collect();
            let to_replay: Vec<(String, Embedding)> = pending
                .delta
                .into_iter()
                .filter(|(id, _)| !known.contains(id.as_str()))
                .collect();
            drop(known);
            if !to_replay.is_empty() {
                let items: Vec<(String, &[f32])> = to_replay
                    .iter()
                    .map(|(id, emb)| (id.clone(), emb.as_slice()))
                    .collect();
                match new_index.insert_batch(&items) {
                    Ok(n) => {
                        tracing::info!(replayed = n, "Replayed delta into rebuilt HNSW before swap")
                    }
                    Err(e) => tracing::warn!(
                        error = %e,
                        replayed_attempt = items.len(),
                        "Failed to replay delta into rebuilt HNSW; new chunks will surface on next rebuild"
                    ),
                }
            }
```

### Replacement

The dedup must compare by content fidelity, not just by id. Options:
1. **Cheapest, correct:** Replay every delta entry unconditionally; on duplicate id, *replace* the snapshot vector. `hnsw_rs` has no replace, so use a "remove then insert" path.
2. **Cheapest, accept stale:** Always insert delta; tolerate duplicate twin vectors until next rebuild (the audit explicitly flags this as a graph-size regression).
3. **Correct, more invasive:** Capture `content_hash` alongside the snapshot, replay only when the delta entry's hash differs.

Pick option 3 for correctness. `pending.delta` becomes `Vec<(String, Embedding, ContentHash)>`, the rebuild thread returns `Vec<(String, ContentHash)>` alongside the index, and the dedup filter checks the hash.

This is **>30 lines of change** spanning `pending` struct definition, the rebuild thread, and the drain — see Notes for the full API change.

### Notes

- **STRUCTURAL CHANGE — exceeds 30-line replacement budget.** API change required:
  - `PendingRebuild::delta: Vec<(String, Embedding)>` → `Vec<(String, Embedding, [u8; 32])>` (or `Vec<(String, Embedding, blake3::Hash)>`).
  - `spawn_hnsw_rebuild` closure (`src/cli/watch.rs:980-1030`) returns `RebuildOutcome` — extend with `snapshot_hashes: Vec<(String, [u8; 32])>` so the drain can dedup by `(id, hash)`.
  - Capture-site: wherever `pending.delta.push(...)` is called (search `pending.delta.push` and the watch reindex path). Each push must include the chunk's `content_hash`.
  - Drain-site replacement (lines 1077-1105):
    ```rust
    let known: std::collections::HashMap<&str, &[u8; 32]> =
        snapshot_hashes.iter().map(|(id, h)| (id.as_str(), h)).collect();
    let to_replay: Vec<(String, Embedding)> = pending
        .delta
        .into_iter()
        .filter(|(id, _, hash)| {
            // Replay if id is unknown OR if the snapshot's vector was built
            // from an older content_hash — fresh embedding wins.
            known.get(id.as_str()).is_none_or(|sh| sh != &hash)
        })
        .map(|(id, emb, _hash)| (id, emb))
        .collect();
    ```
- Add a regression test `test_rebuild_window_re_embedding_replays_fresh_vector` in `src/cli/watch.rs::tests` that:
  1. Spawns a rebuild with snapshot `[("a", emb_v1, hash_v1)]`.
  2. Mid-rebuild, pushes `("a", emb_v2, hash_v2)` to `pending.delta`.
  3. After drain, asserts the swapped HNSW contains `emb_v2`, not `emb_v1`.
- This finding overlaps with P2.29 (TC-ADV: Non-blocking HNSW rebuild — no panic/dim-drift/store-fail tests). Coordinate the fix with the rebuild test suite.

---

## P1.18 — token_pack break-on-first-oversized — drops smaller items that would fit

**Finding:** P1.18 in audit-triage.md / Algorithm
**Files:** `src/cli/commands/mod.rs:398-417`
**Why:** Greedy knapsack: `if used + tokens > budget && kept_any { break; }`. Once one item fails to fit, every lower-scored item is dropped — even items that fit comfortably. Repro: budget=300, items `[A=250, B=100, C=40]`. After A packs (used=250), B fails (350>300) → break → C silently dropped though `used+40=290 ≤ 300`.

### Current code (src/cli/commands/mod.rs:394-422)

```rust
    // Greedy pack in score order, tracking which indices to keep
    let mut used: usize = 0;
    let mut kept_any = false;
    let mut keep: Vec<bool> = vec![false; items.len()];
    for idx in order {
        let tokens = token_counts[idx] + json_overhead_per_item;
        if used + tokens > budget && kept_any {
            break;
        }
        if !kept_any && tokens > budget {
            // Always include at least one result, but cap at 10x budget to avoid
            // pathological cases (e.g., 50K-token item with 300-token budget).
            // When budget == 0, skip the 10x guard (0 * 10 == 0, which would reject
            // every item) and include the first item unconditionally.
            if budget > 0 && tokens > budget * 10 {
                tracing::debug!(tokens, budget, "First item exceeds 10x budget, skipping");
                continue;
            }
            tracing::debug!(
                tokens,
                budget,
                "First item exceeds token budget, including anyway"
            );
        }
        used += tokens;
        keep[idx] = true;
        kept_any = true;
    }
```

### Replacement

```rust
    // Greedy pack in score order, tracking which indices to keep.
    //
    // Note: when an oversized item appears mid-stream we `continue` rather
    // than `break` so subsequent (smaller, lower-scored) items can still
    // fit into the remaining budget. Score-ordered packing already prefers
    // higher-relevance items; the greedy fall-through is the right rounding
    // when one mid-list item won't fit.
    let mut used: usize = 0;
    let mut kept_any = false;
    let mut keep: Vec<bool> = vec![false; items.len()];
    for idx in order {
        let tokens = token_counts[idx] + json_overhead_per_item;
        if used + tokens > budget && kept_any {
            // Skip this oversized item but keep probing — smaller items
            // later in score order may still fit.
            continue;
        }
        if !kept_any && tokens > budget {
            // Always include at least one result, but cap at 10x budget to avoid
            // pathological cases (e.g., 50K-token item with 300-token budget).
            // When budget == 0, skip the 10x guard (0 * 10 == 0, which would reject
            // every item) and include the first item unconditionally.
            if budget > 0 && tokens > budget * 10 {
                tracing::debug!(tokens, budget, "First item exceeds 10x budget, skipping");
                continue;
            }
            tracing::debug!(
                tokens,
                budget,
                "First item exceeds token budget, including anyway"
            );
        }
        used += tokens;
        keep[idx] = true;
        kept_any = true;
    }
```

### Notes

- Single-token change: `break` → `continue`.
- Add a regression test in `src/cli/commands/mod.rs::tests`:
  ```rust
  #[test]
  fn token_pack_continues_past_oversized_midstream_item() {
      // items in score order: oversized, fits, fits
      let items = vec![/* score=1.0 250-token, score=0.9 100-token, score=0.8 40-token */];
      let token_counts = vec![250, 100, 40];
      let (kept, used) = token_pack(&items, &token_counts, /* budget */ 300, /* json_overhead */ 0, /* score_fn */ ...);
      assert_eq!(kept.len(), 2);  // A + C, B skipped
      assert_eq!(used, 290);
  }
  ```

---

## P1.19 — cqs serve shutdown handles only Ctrl-C — SIGTERM (systemctl) skips graceful drain

**Finding:** P1.19 in audit-triage.md / Platform Behavior
**Files:** `src/serve/mod.rs:253-260`
**Why:** `shutdown_signal()` awaits only `tokio::signal::ctrl_c()`. Under systemd or any supervisor that issues `SIGTERM` (default for `systemctl stop`), axum never sees the signal and gets `SIGKILL`'d. macOS `launchd` also sends SIGTERM by default.

### Current code (src/serve/mod.rs:253-260)

```rust
/// Listen for Ctrl-C to trigger axum's graceful shutdown.
async fn shutdown_signal() {
    if let Err(e) = tokio::signal::ctrl_c().await {
        tracing::warn!(error = %e, "failed to install ctrl-c handler; server will only stop on listener failure");
        std::future::pending::<()>().await;
    }
    tracing::info!("ctrl-c received, beginning graceful shutdown");
}
```

### Replacement

```rust
/// Listen for Ctrl-C, SIGTERM (Unix), or Ctrl-Break/Ctrl-Close (Windows) to
/// trigger axum's graceful shutdown. Without SIGTERM handling, `systemctl stop`
/// and `launchd` shutdowns escalate to SIGKILL with no graceful drain.
async fn shutdown_signal() {
    let ctrl_c = async {
        if let Err(e) = tokio::signal::ctrl_c().await {
            tracing::warn!(error = %e, "failed to install ctrl-c handler");
            std::future::pending::<()>().await;
        }
    };

    #[cfg(unix)]
    let terminate = async {
        match tokio::signal::unix::signal(tokio::signal::unix::SignalKind::terminate()) {
            Ok(mut s) => {
                s.recv().await;
            }
            Err(e) => {
                tracing::warn!(error = %e, "failed to install SIGTERM handler");
                std::future::pending::<()>().await;
            }
        }
    };

    #[cfg(not(unix))]
    let terminate = std::future::pending::<()>();

    tokio::select! {
        _ = ctrl_c => tracing::info!("ctrl-c received, beginning graceful shutdown"),
        _ = terminate => tracing::info!("SIGTERM received, beginning graceful shutdown"),
    }
}
```

### Notes

- `tokio::signal::unix::signal` is available with the `signal` feature on tokio. Verify Cargo.toml has `tokio = { ..., features = [..., "signal"] }` — likely already enabled for ctrl_c.
- Stretch: also handle `SignalKind::interrupt()` and Windows `ctrl_break()`/`ctrl_close()`. Skipping for the easy P1 fix; SIGTERM is the high-impact case.
- Companion finding in `src/cli/watch.rs:132-148` already installs a SIGTERM handler via `libc::signal` for the daemon — this fix brings serve to parity.

---

## P1.20 — Default subscriber drops every info_span — 150 spans invisible at default level

**Finding:** P1.20 in audit-triage.md / Observability OB-V1.30-1
**Files:** `src/main.rs:14-32`
**Why:** Default `EnvFilter` is `"warn,ort=error"`, but every span across `scout`, `gather`, `serve`, `cache`, `slot`, parser, store, embedder is `info_span!` or `debug_span!`. Subscriber drops them all. The heavy investment in span instrumentation is invisible until the user discovers `--verbose`/`RUST_LOG=info`. Daemon under `cqs watch --serve` inherits empty `RUST_LOG` and is doubly hit.

### Current code (src/main.rs:14-32)

```rust
fn main() -> Result<()> {
    // Parse CLI first to check verbose flag
    let cli = cli::Cli::parse();

    // Log to stderr to keep stdout clean for structured output
    // --verbose flag sets debug level, otherwise use RUST_LOG or default to warn
    let filter = if cli.verbose {
        EnvFilter::new("debug")
    } else {
        EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new("warn,ort=error"))
    };

    tracing_subscriber::fmt()
        .with_env_filter(filter)
        .with_writer(std::io::stderr)
        .init();

    cli::run_with(cli)
}
```

### Replacement

```rust
fn main() -> Result<()> {
    // Parse CLI first to check verbose flag
    let cli = cli::Cli::parse();

    // Log to stderr to keep stdout clean for structured output.
    // --verbose flag sets debug level for cqs (everything else stays at info),
    // otherwise honour RUST_LOG, defaulting to "cqs=info,warn" so the
    // ~150 span instrumentation sites in the codebase actually render
    // without third-party noise. (OB-V1.30-1.)
    let filter = if cli.verbose {
        EnvFilter::new("cqs=debug,info")
    } else {
        EnvFilter::try_from_default_env()
            .unwrap_or_else(|_| EnvFilter::new("cqs=info,warn,ort=error"))
    };

    // FmtSpan::CLOSE emits a synthetic event on span close with elapsed time —
    // turns every `info_span!("foo", ...).entered()` into a "foo" + latency
    // line in the journal automatically. Without it, only events emitted
    // *inside* a span produce log lines; entry/exit pairs disappear.
    tracing_subscriber::fmt()
        .with_env_filter(filter)
        .with_span_events(tracing_subscriber::fmt::format::FmtSpan::CLOSE)
        .with_writer(std::io::stderr)
        .init();

    cli::run_with(cli)
}
```

### Notes

- Verify the codebase's actual crate name in Cargo.toml — `cqs` is what `lib.rs` declares per `MEMORY.md`.
- Side effect: every `cqs <command>` now prints span boundaries to stderr at default level. May surprise existing users who pipe stdout but watch stderr. Mention in CHANGELOG: "default log level raised to `cqs=info,warn` so span instrumentation renders by default; set `RUST_LOG=warn` to silence."
- Companion finding P1.21 (auth failures log nothing) and P2.25 (per-request span disconnected from spawn_blocking) become valuable only after this lands.
- Stretch (out of scope): `--log-format=json` flag wired to `.json()` builder for daemon journals. Tracking-issue.

---

## P1.21 — Auth failures log nothing — no journal trail for 401s

**Finding:** P1.21 in audit-triage.md / Observability OB-V1.30-2
**Files:** `src/serve/auth.rs:194-232` (specifically the `AuthOutcome::Unauthorized` arm at lines 224-230)
**Why:** Auth middleware returns 401 silently. Brute-force scans, expired bookmarks, misconfigured clients leave no journal trail. Asymmetric with `enforce_host_allowlist` (`src/serve/mod.rs:246`) which DOES emit `tracing::warn!` on rejection.

### Current code (src/serve/auth.rs:224-230)

```rust
        AuthOutcome::Unauthorized => {
            // Body intentionally minimal: no debug data, no token-
            // length leak. Tracing happens once per launch (banner)
            // and never per-request — auditors can grep for the
            // count of 401s in access logs without seeing tokens.
            (StatusCode::UNAUTHORIZED, "Unauthorized").into_response()
        }
```

### Replacement

```rust
        AuthOutcome::Unauthorized => {
            // Body intentionally minimal: no debug data, no token-
            // length leak. Tracing emits method + path (NOT query
            // string — that may carry `?token=` candidates) so
            // operators get a journal trail for 401s without
            // logging token material.
            tracing::warn!(
                method = %req.method(),
                path = %req.uri().path(),
                "serve: rejected unauthenticated request",
            );
            (StatusCode::UNAUTHORIZED, "Unauthorized").into_response()
        }
```

### Notes

- Critical: log `path` only, NOT `uri()` — the URI carries `?token=…` for the OkViaQueryParam case but the same shape can appear on Unauthorized (bad token still has the param). `req.uri().path()` strips the query string.
- Companion to P1.11 (TraceLayer also leaks query string). Both must be fixed together; otherwise the OB-V1.30-1 default-level fix (P1.20) makes the leak more visible, not less.
- Add a regression test `test_unauthorized_logs_method_and_path_no_query` in `src/serve/auth.rs::tests` that wraps the test subscriber and asserts `?token=...` is absent from captured events.

---
