# Open Issue Fixes Implementation Plan

> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.

**Goal:** Fix 5 open GitHub issues (#645, #650, #647, #648, #646) — ordered by impact × ease.

**Architecture:** Each issue is independent. Tasks can be executed in order or parallelized (except #648 which touches files modified by #645). Commit after each task.

**Tech Stack:** Rust, SQLite (sqlx), tokio, clap

---

## File Map

| Task | Issue | Creates | Modifies |
|------|-------|---------|----------|
| 1 | #645 | — | `src/store/mod.rs`, `src/store/types.rs`, `src/store/calls.rs`, `src/store/helpers.rs` |
| 2 | #650 | — | `src/cli/mod.rs` |
| 3 | #647 | — | `src/cli/mod.rs` |
| 4 | #648 | `src/store/chunks/mod.rs`, `src/store/chunks/crud.rs`, `src/store/chunks/staleness.rs`, `src/store/chunks/embeddings.rs`, `src/store/chunks/query.rs`, `src/store/chunks/async_helpers.rs` | Removes `src/store/chunks.rs` |
| 5 | #646 | — | `src/cli/batch/mod.rs` |

---

### Task 1: #645 — Fix positional ChunkRow construction (High impact, Easy)

**Problem:** 3 SQL-row sites construct `ChunkRow` with `row.get(N)` integer indexes instead of `ChunkRow::from_row()`. Silent data corruption if column order changes. One site (calls.rs:989) maps from a `LightChunk` struct — needs a separate constructor.

**Files:**
- Modify: `src/store/helpers.rs` — add variant `from_row()` helpers for non-standard SELECT shapes
- Modify: `src/store/mod.rs:704-719` — replace positional construction in `search_by_name()`
- Modify: `src/store/types.rs:321-336` — replace positional construction in type-deps query
- Modify: `src/store/calls.rs:1036-1051` — replace positional construction in `find_test_chunks_async()`
- Modify: `src/store/calls.rs:989-1004` — add `ChunkRow::from_light_chunk()` constructor

**Approach:** The 3 SQL sites each SELECT different column sets. Rather than forcing identical SELECTs, add named column access to each query. The simplest fix: change each `SELECT col1, col2, ...` to `SELECT id, origin, name, ...` (canonical order matching `from_row()`) and call `from_row()`. Where that's not possible (extra columns like `type_name`), use named `row.get("column_name")`.

- [ ] **Step 1: Write a test that would catch column reordering**

Add a test in `src/store/helpers.rs` tests that constructs a ChunkRow via `from_row()` and verifies all fields match expected values. This validates the named-column approach works.

- [ ] **Step 2: Fix `search_by_name()` in `src/store/mod.rs:704-719`**

Change the SELECT to use canonical column order matching `from_row()`, then call `ChunkRow::from_row(&row)`. Verify the SQL query selects the same columns as the standard chunks SELECT.

- [ ] **Step 3: Fix type-deps query in `src/store/types.rs:321-336`**

This query has an extra `type_name` column at position 0. Use named access: `row.get::<String, _>("id")`, etc. Or restructure the query to SELECT standard chunk columns and get type_name separately.

- [ ] **Step 4: Fix `find_test_chunks_async()` in `src/store/calls.rs:1036-1051`**

**WARNING: `from_row()` CANNOT be used here** — the SQL omits `content` and `doc` columns. Calling `from_row()` would panic at runtime. Create a `ChunkRow::from_row_lightweight(&row)` constructor that uses named access for available columns and defaults the rest: `content: String::new()`, `doc: None`, `content_hash: String::new()`, `window_idx: None`.

- [ ] **Step 4b: Also fix `src/store/chunks.rs:885`**

Uses positional `row.get(1)` for `origin_key` right after `from_row()`. Switch to named access for consistency.

- [ ] **Step 5: Add `ChunkRow::from_light_chunk()` in `src/store/calls.rs:989-1004`**

This isn't a SQL row — it maps from a `LightChunk` struct. Add a named constructor method on `ChunkRow` to make the mapping explicit and future-proof.

- [ ] **Step 6: Run tests**

```bash
cargo test --features gpu-index 2>&1 | grep "^test result:"
```
Expected: all pass, no regressions.

- [ ] **Step 7: Commit**

```bash
git add src/store/mod.rs src/store/types.rs src/store/calls.rs src/store/helpers.rs
git commit -m "fix(#645): replace positional ChunkRow construction with named access"
```

---

### Task 2: #650 — Unify --json and --format flags (Medium impact, Easy)

**Problem:** 27 commands use `--json` (boolean), 4 commands (impact, review, ci, trace) use `--format json`. Users must remember which commands use which flag.

**Files:**
- Modify: `src/cli/mod.rs` — add `--json` alias to the 4 `--format` commands

**Approach:** Add `--json` as a boolean flag to Impact, Review, CI, and Trace. When `--json` is set, override `--format` to `json`. This is backward-compatible — existing `--format json` still works. Don't remove `--format` (it supports `mermaid` for Impact/Trace).

- [ ] **Step 1: Add `--json` flag to Impact command struct**

In `src/cli/mod.rs`, add `#[arg(long)]` `json: bool` to the Impact variant. In the dispatch, if `json` is true, set format to `OutputFormat::Json`.

- [ ] **Step 2: Add `--json` flag to Review, CI, Trace**

Same pattern for the other 3 commands.

- [ ] **Step 3: Test that `--json` and `--format json` both work**

```bash
cargo run --features gpu-index -- impact parse_file --json 2>&1 | head -1
cargo run --features gpu-index -- impact parse_file --format json 2>&1 | head -1
```
Both should produce JSON output.

- [ ] **Step 4: Add conflict handling and update help text**

Use clap's `#[arg(long, conflicts_with = "format")]` on the `json` field to prevent ambiguous `--json --format mermaid`. Add help text to `--format`: "Use --json as shorthand for --format json".

- [ ] **Step 5: Commit**

```bash
git add src/cli/mod.rs
git commit -m "fix(#650): add --json alias to impact/review/ci/trace commands"
```

---

### Task 3: #647 — Use single-thread runtime for read-only commands (Low impact, Easy)

**Problem:** `open_project_store()` always calls `Store::open()` (4-thread runtime). Read-only commands waste 4 OS threads.

**Files:**
- Modify: `src/store/mod.rs` — add `Store::open_light()` method (current_thread runtime + full mmap/cache)
- Modify: `src/cli/mod.rs:27-40` — add `open_project_store_readonly()` using `open_light()`
- Modify: 25+ `src/cli/cmd_*.rs` files — change `open_project_store()` to `open_project_store_readonly()` for read-only commands

**IMPORTANT:** Do NOT use `Store::open_readonly()` — it uses 64MB mmap and 4MB cache (designed for small reference stores). Primary index needs full 256MB mmap and 16MB cache. Create a new `Store::open_light()` that uses `current_thread` runtime but keeps full memory allocations.

**Approach:** Add `Store::open_light()` to `src/store/mod.rs` that calls `open_with_config` with `use_current_thread: true` but `max_connections: 4`, full mmap (256MB) and cache (16MB). Then add `open_project_store_readonly()` in `src/cli/mod.rs` that calls `open_light()`. Route read-only commands through it — note that many commands call `open_project_store()` in their own `cmd_` functions, not in central dispatch. Grep for all call sites.

- [ ] **Step 1: Add `Store::open_light()` to `src/store/mod.rs`**

```rust
pub fn open_light(db_path: &Path) -> Result<Self, StoreError> {
    Self::open_with_config(db_path, true, 4, "268435456", "-16384")
    //                              ^^^^ current_thread but full mmap/cache
}
```

- [ ] **Step 2: Add `open_project_store_readonly()` to `src/cli/mod.rs`**

```rust
pub(crate) fn open_project_store_readonly(root: &Path) -> Result<Store> {
    let cqs_dir = cqs::resolve_index_dir(root);
    Store::open_light(&cqs_dir.join("index.db"))
}
```

- [ ] **Step 3: Find all `open_project_store()` call sites**

```bash
grep -rn "open_project_store()" src/cli/ | grep -v "fn open_project_store"
```
Route read-only ones through `open_project_store_readonly()`.

Write commands (keep `open_project_store()`): index, improve-docs, audit-mode, notes add/update/remove, watch, gc --clean.

- [ ] **Step 4: Test**

```bash
cargo test --features gpu-index 2>&1 | grep "^test result:"
```
Run a read-only command and verify it works: `cqs stats --json`

- [ ] **Step 5: Commit**

```bash
git add src/store/mod.rs src/cli/
git commit -m "perf(#647): use single-thread runtime for read-only CLI commands"
```

---

### Task 4: #648 — Split store/chunks.rs into submodules (Medium impact, Medium difficulty)

**Problem:** `store/chunks.rs` is 2456 lines — the largest file in the codebase. Hard to navigate and edit.

**Files:**
- Create: `src/store/chunks/mod.rs` — re-exports
- Create: `src/store/chunks/crud.rs` — upsert, metadata, delete (~400 lines)
- Create: `src/store/chunks/staleness.rs` — prune, stale checks (~215 lines)
- Create: `src/store/chunks/embeddings.rs` — embedding retrieval, stats (~105 lines)
- Create: `src/store/chunks/query.rs` — chunk retrieval, search, identity (~500 lines)
- Create: `src/store/chunks/async_helpers.rs` — async fetch, batch insert (~350 lines)
- Remove: `src/store/chunks.rs`

**Approach:** Pure mechanical refactor. Move functions to submodules, add `pub(crate)` re-exports. Tests move with their code. No logic changes.

- [ ] **Step 1: Create `src/store/chunks/` directory and `mod.rs`**

```rust
mod crud;
mod staleness;
mod embeddings;
mod query;
mod async_helpers;

pub(crate) use crud::*;
pub(crate) use staleness::*;
pub(crate) use embeddings::*;
pub(crate) use query::*;
pub(crate) use async_helpers::*;
```

- [ ] **Step 2: Move CRUD functions (lines 17-410)**

Functions: `get_metadata`, `upsert_chunks_batch`, `upsert_chunk`, `update_embeddings_batch`, `update_embeddings_with_hashes_batch`, `get_enrichment_hashes_batch`, `get_summaries_by_hashes`, `upsert_summaries_batch`, `get_all_summaries`, `prune_orphan_summaries`, `needs_reindex`, `delete_by_origin`, `upsert_chunks_and_calls`.

- [ ] **Step 3: Move staleness functions (lines 410-625)**

Functions: `prune_missing`, `count_stale_files`, `list_stale_files`, `check_origins_stale`.

- [ ] **Step 4: Move embedding retrieval (lines 625-730)**

Functions: `get_embeddings_by_hashes`, `get_chunk_ids_and_embeddings_by_hashes`, `chunk_count`, `stats`.

- [ ] **Step 5: Move query/search functions (lines 730-1230)**

Functions: `get_chunks_by_origin`, `get_chunks_by_origins_batch`, `get_chunks_by_names_batch`, `get_chunk_with_embedding`, `get_chunks_by_ids`, `get_embeddings_by_ids`, `search_by_names_batch`, `all_chunk_identities`, `chunks_paged`, `all_chunk_identities_filtered`.

- [ ] **Step 6: Move async helpers (lines 1235-1580)**

Functions: `fetch_chunks_by_ids_async`, `fetch_candidates_by_ids_async`, `fetch_chunks_with_embeddings_by_ids_async`, `embedding_batches`, `snapshot_content_hashes`, `batch_insert_chunks`, `upsert_fts_conditional`, `EmbeddingBatchIterator`.

- [ ] **Step 7: Move tests alongside their code**

Each submodule gets its relevant `#[cfg(test)] mod tests { ... }` block. Shared test helpers (`make_chunk()`, `setup_store()`) go in `mod.rs` as `#[cfg(test)] pub(super) mod test_utils { ... }` so all submodule tests can import them. Note: each submodule file needs its own imports (`use super::*`, `use sqlx::Row`, `use std::collections::HashMap`, etc.) since `impl Store` blocks work across files in Rust.

- [ ] **Step 8: Delete `src/store/chunks.rs`, update `src/store/mod.rs`**

Change `mod chunks;` — it now resolves to `chunks/mod.rs`.

- [ ] **Step 9: Run full test suite**

```bash
cargo test --features gpu-index 2>&1 | grep "^test result:"
```

- [ ] **Step 10: Commit**

```bash
git add src/store/chunks/ src/store/mod.rs
git rm src/store/chunks.rs
git commit -m "refactor(#648): split store/chunks.rs into 5 submodules"
```

---

### Task 5: #646 — Fix stale caches in batch/chat mode (Medium impact, Medium difficulty)

**Problem:** `BatchContext` uses `OnceLock` caches that never invalidate. Long `cqs chat` sessions use stale call graphs, file sets, and HNSW indexes after concurrent `cqs index` runs.

**Files:**
- Modify: `src/cli/batch/mod.rs` — replace `OnceLock` with invalidatable cache for mutable data

**Approach:** Replace `OnceLock` with `RefCell<Option<T>>` for caches that go stale (matching the existing `refs: RefCell<LruCache>` pattern — BatchContext is single-threaded, `RwLock` is unnecessary overhead). Keep `OnceLock` for stable caches (embedder, reranker, config, audit_state). Auto-detect staleness by checking index mtime before returning cached values. On invalidation, also re-open the `Store` since it has its own internal `OnceLock` caches (`call_graph_cache`, `test_chunks_cache` in store/mod.rs).

- [ ] **Step 1: Define which caches are stable vs mutable**

Stable (keep OnceLock): `embedder`, `reranker`, `config`, `audit_state` (time-bounded, not index-derived)
Mutable (switch to RefCell<Option<T>>): `hnsw`, `call_graph`, `test_chunks`, `file_set`, `notes_cache`

- [ ] **Step 2: Add index mtime tracking**

Add `index_mtime: Cell<Option<SystemTime>>` to BatchContext. On cache access, check if `.cqs/index.db` mtime changed since last read. If so, clear all mutable caches AND re-open the Store (which resets its internal OnceLock caches).

- [ ] **Step 3: Replace OnceLock with RefCell<Option<T>> for mutable caches**

```rust
hnsw: RefCell<Option<Box<dyn VectorIndex>>>,
call_graph: RefCell<Option<Arc<CallGraph>>>,
// etc.
```

Update all accessor methods to check mtime before returning cached value. On mtime change, `take()` from all RefCells and re-open Store.

- [ ] **Step 4: Add `invalidate` / `refresh` batch command**

In the batch command parser, handle "refresh" or "invalidate" to clear all mutable caches manually.

- [ ] **Step 5: Test manual invalidation**

Write a test that creates BatchContext, caches a value, calls invalidate, verifies cache is empty.

- [ ] **Step 6: Test mtime-based invalidation**

Write a test that creates BatchContext, caches a value, touches the index.db file, verifies next access returns fresh data.

- [ ] **Step 7: Run full test suite**

```bash
cargo test --features gpu-index 2>&1 | grep "^test result:"
```

- [ ] **Step 8: Commit**

```bash
git add src/cli/batch/mod.rs
git commit -m "fix(#646): auto-invalidate stale caches in batch/chat mode"
```
