Follow-up: deferred review findings from PR #953

[philcunliffe]

Deferred, non-blocking findings from the review of PR #953 (LLP 0265 T6, sidecar builds at maintenance), triaged at head bf79490e527ab0dc9dd5... (bf79490e). Rounds 1 and 2 are recorded on that thread; four of these were seen and explicitly deferred by @bgmcmullen's decision comment there, and the fifth was recorded open in round 2. None is a production defect in the tree as merged: every gap below costs speed, completeness of the optimization, or observability, never correctness, because the scan tier serves any file without a usable sidecar and returns exact results.

1. Nothing ever rebuilds a missing sidecar

src/core/cache/maintenance.js (the maintenance.grep_index pass, gated on report.compacted). Compaction always publishes a fresh generation directory, so every pass sees only brand-new files. A partition that has converged (no longer compacting) never gets indexed at all, which is permanent for a detached source; a pass cut short by daemon shutdown leaves those files on the scan tier until new rows make compaction due again; and the ported MAX_INDEX_ATTEMPTS poison bound in src/core/search/sidecar_build.js can never fire in the daemon, because isQuarantined is always asked about a path it has never seen. The module doc in sidecar_build.js records this honestly ("a missing sidecar is repaired by the next compaction rewriting the rows into a new file"). Whether the pass should also run on a non-compacting tick is an LLP 0264 lifecycle decision.

2. A corrupt sidecar is permanent

src/core/search/sidecar_build.js (fs.existsSync(sidecarPath) is the whole presence test) and nothing ever replaces an existing sidecar. An unreadable sidecar costs a caught exception plus a full brute scan of its data file on every query, forever; the read path warns (grep_search.indexed_read_failed, naming both files) but only an operator deleting the file recovers the indexed tier. Deleting the sidecar from the read-side failure path, or verifying the footer before counting a file present at build time, would close it; both are read-path-writes-cache decisions worth stating in the LLP rather than assuming.

3. The sidecar build pass sits outside max_tick_ms

src/core/cache/maintenance.js. The tick budget is checked only at the top of the partition loop; the build pass then reads and indexes every live data file of the just-compacted generation (one at a time, so memory stays bounded, but seconds of CPU per file) with no deadline, per-file cap, or signal. A wide generation overruns the configured tick by the whole build. Background-daemon cost only, but the budget guard should at least see it.

4. Sidecar bytes are invisible to every disk-accounting surface

src/core/cache/maintenance.js (countDataFiles / measureDataDir exclude *.index.parquet, correctly, for the compaction heuristics). cacheStatus derives from those same helpers, so nothing reports the index footprint and the bounded-disk-growth acceptance gate would not see it grow. Growth is bounded by the generation lifecycle (sidecars die with their directory), but it should be visible; a separate sidecarBytes on CacheStatusPartition (T7 territory) rather than folding it back into the compaction inputs.

5. A sidecar freezes the allowlist it was built over

src/core/search/index_worker_thread.js (searchableStringColumns intersects with SEARCHABLE_COLUMNS at build time) and src/core/search/grep_service.js (the read side prunes to the stored hypgrep.text_columns and never compares it against today's constant; the index carries a hypgrep format version but no allowlist stamp). Combined with the no-rebuild lifecycle (finding 1), the first release that adds a column to the allowlist makes every already-indexed file silently answer zero for that column while uncompacted files answer correctly, for the life of the generation. Non-blocking today because the shipped allowlist is fully indexable, so both tiers agree as merged; it becomes live the moment the set grows. This is exactly the #977 scenario (restoring tool_args coverage once hypgrep can index VARIANT): #977 must invalidate existing sidecars, not merely start building new ones. Recorded in the sidecar_build.js module doc as well.

---

Findings 1, 2, and 5 share one design surface (the sidecar rebuild/invalidation lifecycle) and are best solved together; 3 and 4 are independent and small. Source: review rounds on PR #953.