fix: fall back to FTS5 lexical search when chromadb HNSW segment segfaults on open

[flaviomartil]

Problem

When a palace's HNSW vector segment is corrupt, chromadb segfaults while loading the segment on get_collection(). SIGSEGV cannot be caught with try/except (it kills the interpreter), so mempalace search dies with exit 139 and returns nothing, even though the FTS5/sqlite lexical index in chroma.sqlite3 is intact and independent of the vector segment.

Fix

search() now probes the collection open in a throwaway subprocess (_chromadb_open_crashes) before touching chromadb in-process. The child runs get_collection(...).count(); a non-zero / signal exit or a timeout means opening in-process is unsafe.

• Open unsafe → serve read-only lexical results from the existing _bm25_only_via_sqlite path, rendered by a new _print_lexical_results with a clear lexical mode — vector index unavailable banner. The CLI stays up and answers the query.
• Open safe (healthy palace) → unchanged. The original vector/hybrid path runs exactly as before; the probe is the only added step.

The subprocess is the crux: SIGSEGV is uncatchable in-process, so the only safe way to know whether an open will crash is to let a disposable child try it and inspect its exit status.

Before / after

• Before: mempalace search "..." on a corrupt HNSW palace → core dump, exit 139, no output.
• After: same palace → BM25/FTS5 lexical results (read-only), exit 0, banner shown. Healthy palaces are byte-for-byte unchanged in behavior.

Tests

tests/test_lexical_fallback_on_segfault.py (hermetic, no corrupt palace needed):

• unsafe open routes to lexical, prints the banner, never touches the vector path
• empty lexical hit set prints "No results found"
• healthy open keeps the vector path and never calls the lexical fallback
• probe verdict follows the child's exit status (0 → safe, non-zero and -11/SIGSEGV → unsafe) and treats a timeout as unsafe

ruff check / ruff format --check clean; existing test_hybrid_search.py and test_empty_chromadb_results.py still pass.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[igorls]

Thanks for this contribution, and apologies for the slow turnaround.

develop has moved a fair way since this was opened and the branch no longer merges cleanly. If you're still interested in landing it, could you rebase onto current develop? Once it merges cleanly and CI is green I'll get it reviewed for the 3.8.0 cycle.

If you'd rather not pick it back up, no problem at all — just say so and I'll close it out, and thanks either way for taking the time to send it.

[flaviomartil]

Rebased onto current develop — merges cleanly now.

A few notes on how the conflicts were resolved, since develop grew its own HNSW-divergence fallback (_hnsw_capacity_diverged + _print_search_results_bm25_only) while this was open:

• Kept develop's divergence fence untouched and added the segfault probe as a second, complementary fence in search(). They cover different failure modes: divergence is detected cheaply in-process, while a corrupt HNSW segment segfaults on open and can only be detected via the subprocess probe.
• The segfault fence is gated on backend_name == "chroma" and forwards the resolved stop words and the [since, before) window to the BM25 reader, matching the semantics of the divergence fallback.
• Refined the probe verdict: it now treats only signal death (returncode < 0, e.g. -SIGSEGV), timeout, or OSError as unsafe. A clean non-zero exit is an ordinary Python exception that the in-process open re-raises as a catchable, diagnosable error — so palaces with a catchable open failure keep their normal error path (and _open_collection_or_explain's state-specific diagnostics) instead of being silently downgraded to lexical results. Updated the parametrized probe test accordingly (exit 1 -> safe).

Local verification: full suite green (4307 passed, 31 skipped), plus ruff check and ruff format --check clean with the pinned 0.16.1.

[igorls]

Thanks for rebasing this, and for the notes on how you resolved against develop's own HNSW-divergence fallback — that made the diff much easier to follow.

The core reasoning here is right, and I want to say so before the concern: probing in a throwaway subprocess because SIGSEGV can't be caught in-process is the correct call, and the returncode handling (negative means killed by signal, so unsafe; positive means an ordinary exception the in-process path can surface properly) is exactly the right distinction. That part I'd merge as-is.

The problem is the call site. _chromadb_open_crashes runs on every search, and each call spawns a fresh interpreter that imports chromadb and opens the collection. On a healthy palace — which is the overwhelmingly common case — that turns every query into a process spawn plus a full import. This project treats latency as a design constraint rather than a nice-to-have (CLAUDE.md puts hooks under 500ms and startup injection under 100ms, with "memory should feel instant" as the goal), and an unconditional subprocess probe won't fit inside that.

You already identified the fix in the comment you left:

# ponytail: re-probes chromadb every search. Cache on the HNSW segment's
# mtime/size if search latency on a healthy palace ever matters.

It does matter, so I'd like that cache before this lands. Keying on the HNSW segment's (mtime, size) is the right instinct — a healthy palace then pays the probe once and hits the cache afterwards, and the cache invalidates exactly when the segment changes underneath it, which is the only time the answer can differ. There's prior art for that shape in backends/chroma.py, which already fingerprints segment files for the capacity cache.

Two smaller things while you're in there:

• timeout=120 allows a two-minute stall on a single search. Since the probe only needs to answer "does opening this segfault", something far shorter would still be conclusive, and a timeout is already treated as unsafe.
• The # ponytail: tag doesn't appear anywhere else in the repo — I assume it's a personal marker. Worth converting to a plain TODO or dropping it once the cache is in.

Happy to look again as soon as the probe is cached.

[flaviomartil]

All three points addressed in fe0a747, pushed just now.

Probe is cached on the segment fingerprint. New hnsw_segment_signature() in backends/chroma.py, built on the same _stat_signature/_vector_segment_id primitives the capacity cache uses: (segment_id, ((name, inode, mtime_ns, size), ...)) over every file in the VECTOR segment directory, resolved fresh on each call. _chromadb_open_crashes caches the verdict per (palace_path, collection_name) keyed on that signature, so a healthy palace pays the probe once and hits the cache afterwards, and the cache invalidates exactly when the segment changes underneath it — repair, a re-mine, or the collection being re-pointed at a new segment all change the signature immediately.

I also carried over the capacity cache's safety properties, since they apply verbatim here: before/after snapshots so an external write landing mid-probe falls through uncached rather than pinning a verdict the disk no longer supports, a 10s max-age backstop for filesystems with coarse timestamps (the signature is the freshness mechanism, the ceiling never fires on ext4/APFS), and no caching when the segment can't be resolved — a None signature means there is nothing on disk to key a verdict on, so that case probes every call, identical to the pre-cache behavior.

One honest scoping note: the cache is in-process, so it eliminates the probe entirely for the long-lived MCP server. A one-shot mempalace search CLI invocation still pays exactly one probe per process — I considered persisting the verdict to disk, but that adds file-locking complexity for a case the signature can't make any fresher, so I kept the shape you pointed at.

Timeout is 15s now. You're right that 120s allowed a two-minute stall on a single search. The probe only needs to import chromadb and open the collection — seconds even cold — and a timeout is already treated as unsafe, so a wedged open now degrades to lexical quickly instead of hanging.

The # ponytail: tag is gone — personal marker, replaced by the cache itself.

Eleven new tests: cache hit, invalidation on segment change, unsafe-verdict caching, no-signature passthrough, TTL age-out, and hnsw_segment_signature unit tests over the seeded chroma.sqlite3 fixture. Full suite green (4316 passed), ruff check + format clean.

[flaviomartil]

Follow-up in d00b6ca: stripped the explanatory comments from the probe cache — the reasoning lives here in the conversation and in the commit message, the code stays bare. Docstrings are one-liners now, including on the new hnsw_segment_signature.

Nothing behavioural changed; 167 tests across the touched files still pass, ruff clean.