## Data Safety

#### DS-V1.36-1: `collect_migration_files` misses `hnsw.ids` / `hnsw.checksum` / `index_base.*` sidecars
- **Difficulty:** easy
- **Location:** src/slot/mod.rs:912-927 (`collect_migration_files`)
- **Description:** The hardcoded `candidates` list moved by `migrate_legacy_index_to_default_slot` includes only `index.hnsw.data` and `index.hnsw.graph` (and the same two for `index_base`). It is missing `index.hnsw.ids`, `index.hnsw.checksum`, `index_base.hnsw.ids`, `index_base.hnsw.checksum`, `index.hnsw.lock` for `index_base`, and SPLADE's checksum-bearing companion files. Post-DS-V1.33-3 (PR #1325), `verify_hnsw_checksums` is strict: a checksum file referencing files that do not exist is a hard error. A user with a v1.32-era pre-slot legacy `.cqs/` containing all five `index.hnsw.*` files would, after `cqs slot ...` triggers the legacy migration, end up with `slots/default/index.hnsw.{data,graph}` only — `index.hnsw.ids` and `index.hnsw.checksum` are left behind in `.cqs/`. The next `cqs search` then either fails verification (no `hnsw.ids` to load) or rebuilds from scratch silently (because the `Self::load_with_dim` "any required file missing" path returns `NotFound`), losing all enrichment work captured in the original index. Also worth noting: `HNSW_ALL_EXTENSIONS` in `src/hnsw/persist.rs:101-108` is the canonical list — `collect_migration_files` should iterate that constant + `index_base` + SPLADE rather than hard-coding a partial list.
- **Suggested fix:** Replace the hardcoded `candidates` array with a programmatic enumeration: for each basename in `["index", "index_base"]` and each ext in `crate::hnsw::persist::HNSW_ALL_EXTENSIONS`, build `<basename>.<ext>`. Then add splade companion files. Add a `cargo test` regression that drops a fake legacy `.cqs/` containing all five `index.hnsw.*` files plus `splade.index.bin`, runs the migration, and asserts every file is present in `slots/default/`.

#### DS-V1.36-2: CAGRA `save` overwrites the previous good blob in place — no `.bak` or temp-rename
- **Difficulty:** medium
- **Location:** src/cagra.rs:888-975 (`CagraIndex::save`)
- **Description:** `gpu.index.serialize(&gpu.resources, path_str, true)` writes the cuVS blob directly to the final path. There is no temp-file staging or `.bak` rollback. A crash, OOM, or SIGKILL during `cuvsCagraSerialize` truncates or corrupts the only on-disk copy of the previous good `.cagra` index — and because the meta sidecar was already removed at line 924 (`let _ = std::fs::remove_file(&meta_path);`), the old metadata that would let load() validate the prior good blob is also gone. Net effect: a single ill-timed crash during `cqs index` finalization leaves the user with a partial `.cagra` and no `.meta`, which load() will reject as `NotFound` — forcing a full GPU rebuild (~minutes for a 50k-chunk corpus, much worse on the inference RTX 4000). The HNSW save path uses backup-temp-rename rollback (DS-V1.30.1-D7); CAGRA does not. The save is best-effort by design (load can rebuild) but the destructive overwrite is avoidable.
- **Suggested fix:** Stage cuVS output to `<path>.{:016x}.tmp` (using `crate::temp_suffix()` for the same-process collision protection used elsewhere), checksum it, then `crate::fs::atomic_replace(&tmp, path)`. Mirror the `.bak` rotation pattern used in `src/hnsw/persist.rs:467-484` so a save failure can restore the prior blob. Remove the meta_path *after* the new blob is in place, not before.

#### DS-V1.36-3: legacy `hnsw_dirty` key is never cleared after per-kind keys are written
- **Difficulty:** easy
- **Location:** src/store/metadata.rs:257-278 (`is_hnsw_dirty`) and 437-451 (`set_hnsw_dirty`)
- **Description:** The doc comment on `is_hnsw_dirty` claims that "the next `set_hnsw_dirty` call splits them apart" — meaning the legacy single `hnsw_dirty` key should be removed once a per-kind key is written. `set_hnsw_dirty` does not implement that: it only writes `hnsw_dirty_enriched` or `hnsw_dirty_base`, never deleting the legacy key. So on a v1.20-era DB with `hnsw_dirty=1` still in metadata, after `set_hnsw_dirty(Enriched, false)`, querying `is_hnsw_dirty(Base)` still falls through to the legacy key and returns `true` — Base is permanently reported dirty even though the user never wrote anything to it. The inverse case (legacy=0, per-kind enriched=1) makes `is_hnsw_dirty(Base)` return false even if Base was secretly dirty, which is the actually-dangerous direction: Base could go un-rebuilt indefinitely.
- **Suggested fix:** In `set_hnsw_dirty`, after writing the per-kind key, run `DELETE FROM metadata WHERE key = 'hnsw_dirty'` inside the same transaction. The legacy fallback in `is_hnsw_dirty` then becomes dead code that can be removed in a subsequent cleanup, but for safety keep it for one release while real DBs migrate.

#### DS-V1.36-4: `cqs index --force` deletes WAL/SHM without checkpointing the old store first
- **Difficulty:** easy
- **Location:** src/cli/commands/index/build.rs:362-386 (`--force` rebuild path)
- **Description:** The summary-recovery branch runs `Store::open` to read summaries, drops the store (line 362), then renames `index.db` → `index.db.bak` and unconditionally `remove_file`s `index.db-wal` and `index.db-shm`. The drop path on `Store` runs only `wal_checkpoint(PASSIVE)` with a 1s timeout (V1.36.2 / src/store/mod.rs:1430-1480); PASSIVE bails on any reader/writer contention without copying. So if the user happens to have a `cqs watch` daemon attached to the same DB (legitimate workflow — watch is supposed to coexist with index), the PASSIVE checkpoint can return without flushing pages. Then deleting `-wal` discards any uncommitted-from-mainfile pages permanently. The summaries we just read are unaffected (they came from a fresh `SELECT`), but any other writes the daemon was making (e.g. notes, telemetry counters) get silently truncated. The previous `Store::close()` API (which runs `TRUNCATE`) is not invoked here.
- **Suggested fix:** Replace `drop(old_store)` (line 362) with `old_store.close().context("...")?;` so the `wal_checkpoint(TRUNCATE)` runs synchronously. Then the `-wal`/`-shm` removal at lines 376-385 is safe because all pages have been merged into `index.db` before the rename. This is the structured-shutdown path the V1.36.2 fix preserved precisely for this kind of operator-initiated quiesce.

#### DS-V1.36-5: SPLADE save uses two-pass rewrite without `.bak` rollback for the final file
- **Difficulty:** medium
- **Location:** src/splade/index.rs:350-440 (`save`)
- **Description:** The SPLADE save streams a blake3 hash through a `HashingWriter`, then seeks back to the checksum offset to stamp the digest, then `atomic_replace`s the temp into place. That part is sound. But unlike the HNSW save (which renames the existing `<basename>.<ext>` to `.<ext>.bak` *before* atomic_replace runs and rolls back on failure), SPLADE's atomic_replace overwrites the previous good `splade.index.bin` directly. If `atomic_replace` itself fails (e.g. cross-device fallback runs out of disk after the source file was already promoted but before the rename completes), the previous good index is gone and the temp may also be corrupt. The `splade_generation` counter then diverges from the on-disk file: SPLADE refuses to load (generation mismatch), so we fall back to no-SPLADE search. Recovery requires a full reindex. The HNSW pattern (rename to `.bak` first, restore from `.bak` on rollback) is precisely the missing piece.
- **Suggested fix:** Before `atomic_replace`, rename any existing `splade.index.bin` to `splade.index.bin.bak`. On atomic_replace failure, rename `.bak` back to the live name. On success, remove `.bak` after a parent fsync. Mirrors `src/hnsw/persist.rs:467-484`.

#### DS-V1.36-6: HNSW load `_lock_file` drop happens at end of function — but lock is taken without timeout, can hang load on stuck save
- **Difficulty:** medium
- **Location:** src/hnsw/persist.rs:736-758 (lock acquisition in `load_with_dim`)
- **Description:** The shared lock taken on `<basename>.hnsw.lock` during load is acquired via `lock_file.lock()` (the std file-lock API) with no timeout or `try_lock` fallback. If a concurrent `save()` from a different process holds the exclusive lock and that process is wedged (e.g. WSL 9P stall, paused under debugger), the load blocks indefinitely. `cqs search` then hangs and the user has no signal except "command never returns". Daemons that hit this on reconcile-time HNSW reload would silently lock up. The save path's lock is also unbounded but that's the writer's own choice; the reader path inheriting the same wait makes a single misbehaving writer take down all readers.
- **Suggested fix:** Use `try_lock_shared` with a bounded retry loop (e.g. 5 attempts × 200 ms with jitter) and surface a `HnswError::Internal("save in progress, try again")` when it expires. Daemons can retry; CLI users see a clear error instead of an opaque hang. Existing test infrastructure in `src/hnsw/persist.rs` tests at lines 1762+ already exercises lock contention — extend with a stuck-writer test.

#### DS-V1.36-7: `Store::close()` still uses `wal_checkpoint(TRUNCATE)` — can deadlock under V1.36.2 PR #1450 conditions
- **Difficulty:** easy
- **Location:** src/store/mod.rs:1289-1298 (`Store::close`)
- **Description:** PR #1450 (recent main commit `4b3fd41e`) replaced the `Store::drop` checkpoint from TRUNCATE → PASSIVE specifically because TRUNCATE acquires the SQLite EXCLUSIVE lock and can stall under WSL 9P when readers are present, occasionally blowing past the 30s busy_timeout (per `git log` and src/store/mod.rs:1440-1447). `Store::close()` — the explicit caller-controlled shutdown path — still issues `PRAGMA wal_checkpoint(TRUNCATE)` with no timeout. If a user calls `close()` while another reader handle (e.g. an explicit `cqs stats` invocation, or the watch daemon) holds the WAL, close blocks on the EXCLUSIVE lock. Under WSL 9P this is the same failure mode the drop-path fix addressed. close() is called from the `--force` rebuild path (after this audit's DS-V1.36-4 is applied) and from `cqs serve` shutdown — both places where a stuck checkpoint becomes operator-visible.
- **Suggested fix:** Wrap the close-path checkpoint in a `tokio::time::timeout` (e.g. 30s) and downgrade to PASSIVE on timeout, mirroring the drop-path treatment. Or accept that close is "best-effort fully-blocking" and document the timeout expectation; either way the unbounded TRUNCATE is the same hazard PR #1450 fixed elsewhere.

#### DS-V1.36-8: `migrate_v18_to_v19` orphan-drop threshold uses lossy `f64 → i64` cast
- **Difficulty:** easy
- **Location:** src/store/migrations.rs:738-754 (orphan threshold computation)
- **Description:** `let threshold = (before_rows as f64 * 0.10) as i64;`. For very large `before_rows` (≥ 2^53 ≈ 9 quadrillion) the f64 multiplication loses precision, producing an incorrect threshold. More relevantly: for `before_rows < 10`, threshold is `0` — meaning a single dropped orphan trips the `error!` log path, spamming the user with "more than 10% dropped" warnings on small DBs that have any orphan at all. The intent is "10% or more is suspicious, otherwise log routinely". Saturating-cast or integer arithmetic avoids both edge cases.
- **Suggested fix:** Use integer math: `let threshold = before_rows / 10;` and require `dropped > threshold` to fire the error path. The min-corpus case (before_rows=5, threshold=0, dropped=1) then falls through to the `warn` arm rather than `error`. Float arithmetic isn't justified here — the percentage is a single fixed constant.

#### DS-V1.36-9: `INSERT OR REPLACE` on `llm_summaries` triggers `bump_splade_on_chunks_delete` cascade if FK mistakenly added
- **Difficulty:** medium
- **Location:** src/store/chunks/crud.rs:520 (llm_summaries upsert) — preventive
- **Description:** Currently safe: `llm_summaries` has no FK to `chunks`. But the `INSERT OR REPLACE` form is well-known to issue an implicit DELETE-then-INSERT, which fires ON DELETE triggers and ON DELETE CASCADE foreign keys. If a future migration adds `FOREIGN KEY (content_hash) REFERENCES chunks(content_hash) ON DELETE CASCADE` (a tempting addition for cleanup), every summary refresh would silently delete and re-insert, firing the v20 `bump_splade_on_chunks_delete` trigger via the cascade chain — bumping `splade_generation` and invalidating the on-disk SPLADE index on every summary write. The latent footgun is the trigger's broad scope: any future FK to chunks gets caught. P4-6 in the v1.33 triage flagged this conceptually ("INSERT OR REPLACE cascade enforced only by call-site convention") but the v20 trigger added a new cascade target since.
- **Suggested fix:** Replace the `INSERT OR REPLACE` with `INSERT … ON CONFLICT(content_hash, purpose) DO UPDATE SET ...` so the upsert path is a true UPDATE on conflict and never fires DELETE triggers. Same change PR #1342 made for `chunks` (per the comment at src/store/chunks/crud.rs:215). Idempotent semantics, no behavior change today, no future-FK footgun.

#### DS-V1.36-10: Migration backup `prune_old_backups` skips on `read_dir` failure — orphan disk burn on permission error
- **Difficulty:** easy
- **Location:** src/store/backup.rs:222-243 (`prune_old_backups`)
- **Description:** The prune step swallows `std::fs::read_dir(dir)` errors with a `tracing::warn!` and returns `Ok(())`. So the next migration runs, drops another `.bak-v*-v*-*.db` (potentially several hundred MB on a large project), the prune step warns again and returns Ok, and the `.cqs/` directory accumulates one new backup per migrate forever. KEEP_BACKUPS=3 is enforced only when the read succeeds. The most likely cause of read_dir failure is a transient permission issue (CI containers, NFS mount glitch) — exactly the conditions where backups also accumulate fastest because migrations re-run repeatedly. After 50 runs the user has 50 × 200 MB = 10 GB of backups in `.cqs/` with no signal beyond a debug log line.
- **Suggested fix:** Surface read_dir failures as `StoreError::Io` to the caller, who already classifies prune failure as "non-fatal — the user's DB is at the correct version" (src/store/migrations.rs:122-126). The error can still be downgraded to a warn at the caller, but at least one log site at `error!` would let operators notice the accumulation. Alternative: emit a `tracing::error!` with the directory size when prune was skipped, so monitoring can alert.

DONE
