fix(repair): preserve embeddings on rebuild + cap divergence-floor

[zhapostolski]

Problem

Two bugs discovered when a 94k-drawer palace required emergency manual repair after HNSW drift:

1. _extract_drawers omits embeddings → hours of unnecessary recomputation

_extract_drawers only fetched documents and metadatas, then called upsert() without embeddings. ChromaDB silently recomputed all embeddings from scratch on every rebuild. For a 94k-drawer palace on CPU this took 4+ hours and caused heavy swap pressure (process peaked at 1.9 GB RSS, 625% CPU, swap nearly exhausted).

Fix: Include "embeddings" in the col.get() call and pass them to upsert(). Falls back to recomputation when the source HNSW is unreadable (the exact scenario being repaired).

2. Divergence-floor exceeds collection size when hnsw:sync_threshold is large → repair-status reports OK on 47% drift

PR #1191 set hnsw:sync_threshold = 50_000. This made divergence_floor = 2 × 50_000 = 100_000. For a 94k-drawer palace the 10%-fraction cap (9_459) is never reached since max(100_000, 9_459) = 100_000, so repair-status reported OK even with 44,592 drawers (47%) missing from the HNSW.

Fix: Cap divergence_floor at _HNSW_DIVERGENCE_FLOOR_CAP = 10_000 regardless of sync_threshold. Also tighten _HNSW_DIVERGENCE_FRACTION from 0.10 → 0.05 so that a 5%-or-greater drift is always flagged.

Behavior after fix

Scenario	Before	After
Rebuild 94k drawers (HNSW readable)	~4h CPU (re-embeds all)	~2min (copies stored embeddings)
Rebuild 94k drawers (HNSW unreadable)	~4h CPU	~4h CPU (expected fallback)
repair-status with 47% drift, sync_threshold=50k	OK ❌	DIVERGED ✓
repair-status with normal flush-lag (<10k), sync_threshold=50k	OK ✓	OK ✓

Test plan

• pytest tests/test_repair.py tests/test_backends.py — no new failures introduced (10 pre-existing failures unrelated to these changes remain)
• Rebuild smoke: mempalace repair on a small test palace — should print "Re-using stored embeddings" and complete in seconds
• repair-status on a palace where HNSW < SQLite by >10k — should report DIVERGED

🤖 Generated with Claude Code

[messelink]

Thanks for catching this — the 94K-with-100K-floor blind spot is real and your reproducer is convincing. The _extract_drawers embeddings fix is also clearly correct and independent of the threshold question; that part I'd land on its own without hesitation.

On the threshold change, one concern worth raising about the opposite-direction blind spot the cap re-creates for sync_threshold=50_000 palaces under steady write load:

I hit this from the other side back on a 98K palace right after rebuilding via the same extract-from-sqlite path you're now formalizing. Right after rebuild, chromadb had flushed exactly one window (50K of 98K) to HNSW; the remaining 48K sat in embeddings_queue waiting for the next flush trigger. Under #1287's floor = 2 × sync_threshold rule, the 48K stayed under the 100K floor → OK. Under your proposed floor = min(2 × sync_threshold, 10_000) = 10_000, the same 48K → DIVERGED. That's the original _refresh_vector_disabled_flag BM25-fallback regime PR #1287 was specifically trying to escape. The precondition (sqlite well past one flush window, queue holding up to one full window of pending writes) is the normal case for any actively-mined palace on the bloat-guard config — not just immediately post-rebuild.

The hard part is that your 47%-actually-missing case and the 48%-queued-for-flush case look identical to a count-only probe. Both have ~half the drawers absent from the HNSW pickle. The signal that distinguishes them lives in embeddings_queue: real corruption usually shows queue depth that's stuck (not draining), or gaps that don't correspond to missing flush events; flush-lag shows queue depth that fluctuates and resets after each flush.

Couple of shapes worth considering instead of a hard 10K cap:

• Cap relative to palace size, not absolute: floor = min(2 × sync_threshold, sqlite_count // 2) — the floor can never exceed half the palace, so a 94K palace never accepts a 100K floor; but a 500K palace with sync_threshold=50_000 still tolerates one full flush window of lag.
• Queue-state probe: read MAX(seq_id) FROM embeddings_queue against the segment's max_seq_id row. The difference is the actual unflushed-write count, distinct from sqlite_count - hnsw_count which conflates lag with corruption. More targeted, but pulls in another file path.
• Time-bounded floor: record a "last successful flush" timestamp; tolerate divergence ≤ sync_threshold only when that timestamp is recent (e.g. < N minutes). After that, the queue is suspect even at moderate sizes. Adds state, requires a place to store it.

Would any of these address the case you hit while leaving the 50K-window steady-state alone? Happy to test against the 98K palace if it helps — both the pre-rescue corrupted snapshot and the post-rebuild healthy one are still on disk, which gives concrete inputs for both sides of the boundary.

(As the person who hit the original false-positive that led to #1287, I have a vested interest in the floor not collapsing back to 10K — but the underlying concern about false positives during normal flush cycles is independent of that history.)

[jphein]

Hi @zhapostolski — pulled the embeddings-preserve portion of this PR into our fork at jphein/mempalace@aa84fa4 after the cherry-pick evaluation at jphein/mempalace#31.

Followed @messelink's split-recommendation: take the _extract_drawers embeddings fix (clearly correct, independent), skip the divergence-floor cap (would re-create the false-positive _refresh_vector_disabled_flag BM25-fallback regime that #1287 was added to fix — on our sync_threshold=50000 palace, divergence_floor=10000 flags DIVERGED during routine flush-window operation).

Field data on the embeddings fix (151K-drawer palace, hook-driven writes, daemon-served chromadb 1.5.9):

• Pre-fix: _extract_drawers omitted embeddings → rebuild had to recompute every vector through ONNX. Observed multi-hour rebuild times even before today's blocker.
• Post-fix (with your change): rebuild reuses stored vectors when available; falls back to recompute when any embedding is None (the actual bug-recovery scenario).

The fix is clean and the metadata-sanitization fork-side addition (#1458) composes cleanly with it. Looking forward to seeing this merge upstream once the divergence-cap question settles separately.