Skip to content

Commit 645ba20

Browse files
jpheinclaude
andcommitted
fix(hnsw): integrity gate in quarantine_stale_hnsw — corruption vs flush-lag
Previous: quarantine fired whenever sqlite_mtime - hnsw_mtime exceeded the (lowered, in MemPalace#1173) 300s threshold. ChromaDB 1.5.x flushes HNSW asynchronously and a clean shutdown does not force-flush, so the on- disk HNSW is *always* meaningfully older than chroma.sqlite3 — that's the steady state, not corruption. Quarantine renamed valid HNSW segments on every cold-start, chromadb created empty replacements, vector recall went to 0/N until rebuild. Confirmed in production on the disks daemon journal, 2026-04-26 06:56:45: three of three HNSW segments quarantined on cold-start with 538-557s mtime gaps (post-clean-shutdown flush lag), leaving a 151,478-drawer palace with vector_ranked=0. Drift directories at *.drift-20260426-065645/ each contained a complete 253MB data_level0.bin plus 18MB index_metadata.pickle — clearly healthy indexes, renamed by the false-positive heuristic. Fix: two-stage gate. 1. mtime gate (existing) — gap > stale_seconds is necessary. 2. integrity gate (new) — sniff index_metadata.pickle for chromadb's expected protocol/terminator bytes (PROTO 0x80 head, STOP 0x2e tail) and a non-trivial size, WITHOUT deserializing the file. Healthy segment with mtime drift → keep in place; truncated / zero-filled / partial-flush → quarantine. Format-sniff is deliberately non-deserializing — pickle deserialization can execute arbitrary code, and the PROTO+STOP byte presence + size floor is sufficient to distinguish a complete chromadb write from truncation, zero-fill, or a partial flush during process kill. Real load failures (the rare case where the bytes look right but chromadb fails to load) still surface to palace-daemon's _auto_repair, which calls quarantine_stale_hnsw directly on observed HNSW errors and bypasses this gate. The cold-start gate from 70c4bc6 (row 24) remains as a perf optimization — even with the integrity check, repeating the sniff on every reconnect is unnecessary work — but its load-bearing role is now covered by this deeper fix. 4 new tests in test_backends.py: - test_quarantine_stale_hnsw_renames_corrupt_segment (drift + bad meta) - test_quarantine_stale_hnsw_leaves_healthy_segment_with_drift_alone (drift + valid meta — the production case at 06:24) - test_quarantine_stale_hnsw_leaves_segment_without_metadata_alone (fresh / never-flushed, no meta file) - test_quarantine_stale_hnsw_renames_truncated_metadata (under-floor size, partial-flush shape) Existing test_quarantine_stale_hnsw_renames_drifted_segment renamed to renames_corrupt_segment with explicit corrupt meta_bytes — the old "renames any drift" contract is gone. Suite 1366/1366 pass. Coordinated cross-repo with palace-daemon's auto-repair-on-startup workaround (separate agent's commit ed3a892). With this fork-side fix the auto-repair becomes belt-and-suspenders; the structural cause of empty-HNSW-on-restart is addressed at the quarantine layer. CLAUDE.md row 26 + README fork-change-queue row + test count 1363→1366. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 8252025 commit 645ba20

4 files changed

Lines changed: 181 additions & 38 deletions

File tree

CLAUDE.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ Ruff for linting (`ruff check`), line length 100, target Python 3.9.
5555

5656
23. **feat: checkpoint collection split — phases A–C** (commit `e266365`, 2026-04-25) — Promoted from "future work" to "necessary" by 2026-04-25 Cat 9 A/B (`kind=all` 632 tokens/Q vs `kind=content` 3 tokens/Q on the canonical 151K palace; over-fetch=100 inadequate, structural fix non-optional). **Phase A:** new `_SESSION_RECOVERY_COLLECTION` constant + `get_session_recovery_collection()` in `palace.py` (mirrors `get_collection`'s shape — cosine, num_threads=1). **Phase B:** `tool_diary_write` routes `topic in _CHECKPOINT_TOPICS` to the dedicated `mempalace_session_recovery` collection, everything else stays in `mempalace_drawers`; new `_get_session_recovery_collection()` in `mcp_server.py` with parallel cache. **Phase C:** new `tool_session_recovery_read` MCP handler reads recovery collection only with optional filters `session_id`, `agent`, `since`, `until`, `wing`, `limit`; `session_id` added as optional metadata field on `tool_diary_write` so the new tool can filter by Claude Code session. Registered in `TOOLS` dict, documented in `website/reference/mcp-tools.md`. 12 new tests across `tests/test_session_recovery.py` + `TestCheckpointRouting` + `TestSessionRecoveryRead`. Design + plan at `docs/superpowers/specs/2026-04-25-checkpoint-collection-split.md` and `docs/superpowers/plans/2026-04-25-checkpoint-collection-split-impl.md`. **Phases D (data migration of ~640 existing checkpoints out of main collection) and E (palace-daemon `lifespan` auto-migrate + `mempalace repair --mode reorganize`) deferred** — multi-day work, gated on a separate go-ahead. Once D lands and the canonical-palace re-run shows the predicted `kind=all` ≈ `kind=content` token convergence, the `kind=` post-filter and over-fetch hack become deletable.
5757

58+
26. **fix: integrity gate in `quarantine_stale_hnsw` — distinguish corruption from chromadb async flush lag** (commit `<TBD>`, 2026-04-26) — The cold-start gate (row 24) prevented repeated firing but didn't fix the deeper bug: when quarantine fired (even once on cold start), it could destructively rename a *healthy* HNSW segment that just hadn't flushed in a while. **Production confirmation 2026-04-26 06:56:45:** disks daemon journal showed all three HNSW segments quarantined on cold start with 538–557s mtime gap (post-clean-shutdown flush lag, not corruption); chromadb created empty replacements, vector recall went to 0/151,478 until a 15-min `mempalace repair --mode rebuild`. ChromaDB 1.5.x flushes HNSW asynchronously and clean shutdown doesn't force-flush, so on-disk HNSW is always somewhat older than `chroma.sqlite3` — mtime threshold alone can't distinguish steady-state from corruption. New `_segment_appears_healthy()` helper sniffs the chromadb-written segment metadata file for its protocol/terminator bytes (`0x80` head, `0x2e` tail) and a non-trivial size, **without ever deserializing** (security-hook compliant; deserialization can execute arbitrary code). Quarantine now requires BOTH stages to fire: (1) mtime gap > 300s AND (2) integrity check fails. Production case from 06:24 had `data_level0.bin=253MB` + 18MB metadata file — clearly healthy, but renamed under old logic; under new logic, kept in place. 4 new tests in `test_backends.py` (renames-corrupt-segment, leaves-healthy-with-drift, leaves-no-metadata, renames-truncated-metadata). Coordinated cross-repo with palace-daemon agent's auto-repair-on-startup workaround — this fork-side fix is the structural complement; auto-repair becomes belt-and-suspenders rather than load-bearing.
59+
5860
25. **feat: surface `drawer_id` in search + diary + recovery payloads** (commit `9a8bb77`, 2026-04-26) — ChromaDB's primary key was always returned by `query()` and `get()` but never plumbed into result-building loops; consumers (e.g. `familiar.realm.watch`'s citation-popover loop) couldn't link a hit back to the underlying drawer. Three call sites updated for parity: `searcher.search_memories` (vector path + sqlite BM25 fallback), `mcp_server.tool_session_recovery_read`, `mcp_server.tool_diary_read`. Defensive zip with id-pad: production chromadb always returns ids, but several test mocks in `test_searcher.py` omit them — pad with `None` when absent so existing fixtures keep working without touching N tests. New integration test `test_results_include_drawer_id` (seeded-collection, asserts non-empty `drawer_id` on every hit and the `drawer_*` prefix shape from conftest); session-recovery test extended to assert `drawer_id` is present and starts with `diary_`. `website/reference/mcp-tools.md` Return-shape docs updated for `mempalace_search`, `mempalace_diary_read`, `mempalace_session_recovery_read`. Worth bringing back upstream as a small isolated PR after this lands.
5961

6062
24. **fix: gate `quarantine_stale_hnsw` to cold-start, not every reconnect** (commit `70c4bc6`, 2026-04-25) — `ChromaBackend._quarantined_paths` set tracks which palaces have already had the proactive drift check run in this process; `make_client()` skips `quarantine_stale_hnsw` on subsequent calls. **Symptom on canonical disks daemon:** `.drift-*` directories accumulating every 10–30 min throughout 2026-04-25 despite the daemon being the only writer (palace data is **not** Syncthing-replicated — only the source code under `~/Projects/` syncs; `/mnt/raid/projects/.stignore` excludes `mempalace-data`). **Root cause:** false-positive thrash. `chroma.sqlite3` mtime bumps per write (millisecond cadence) but HNSW segments flush on chromadb's internal batch cadence. Under steady write load the gap exceeds the 300s threshold (lowered from 3600s in PR #1173 after a real cross-machine drift segfault) even when nothing is corrupt — so the proactive check renames a valid HNSW segment, chromadb rebuilds, drift recurs as soon as the next batch lands. **Real cold-start drift still caught** — that's exactly when a fresh client opens a palace. **Real runtime drift still caught** — palace-daemon's `_auto_repair` calls `quarantine_stale_hnsw` directly on observed HNSW errors, bypassing this gate. 2 new tests in `test_backends.py` verify single-fire-per-palace and per-palace independence. Conftest clears the gate between tests. Worth bringing back upstream — false-positive shape applies to any high-write-rate deployment, not just daemon-strict.

README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ Fork of [MemPalace](https://github.com/milla-jovovich/mempalace), tracking `upst
1212

1313
What this fork adds that you won't get from upstream yet: a **deterministic silent-save hook architecture** (zero data loss, `systemMessage` notification, daemon-strict mode that skips local writes when `PALACE_DAEMON_URL` is set), **ChromaDB 1.5.x hardening** (`quarantine_stale_hnsw` drift recovery, segfault-trigger guards, 8-site `None`-metadata safety), **search that never silently misses** (`search_memories` returns warnings + sqlite BM25 top-up + `available_in_scope` so callers can see what they aren't getting), and **`kind=`-filtered search** that excludes Stop-hook auto-save checkpoints by default — discovered via the 2026-04-25 RLM smoke test, which surfaced that checkpoint diary entries (high word-density session summaries) were dominating retrieval and producing confident-but-misleading answers. Full list below.
1414

15-
1363 tests pass on `main` · [Discussion #1017](https://github.com/MemPalace/mempalace/discussions/1017) introduces the fork upstream · [Issues on this repo](https://github.com/jphein/mempalace/issues) for fork-specific feedback.
15+
1366 tests pass on `main` · [Discussion #1017](https://github.com/MemPalace/mempalace/discussions/1017) introduces the fork upstream · [Issues on this repo](https://github.com/jphein/mempalace/issues) for fork-specific feedback.
1616

1717
## Fork change queue
1818

@@ -36,6 +36,7 @@ Size (lines of diff) and Risk (maintainer-appetite + chance of a rework request)
3636
| **Search** | `kind=` filter on `search_memories` and `mempalace_search` MCP tool excludes Stop-hook auto-save checkpoints by default. CHECKPOINT-shaped diary entries (`topic=checkpoint`, text starting `"CHECKPOINT:"`) are short, word-dense session summaries that consistently outrank substantive content under cosine similarity — the actual conversations they summarize get buried. Three values: `"content"` (default, excludes), `"checkpoint"` (recovery/audit only), `"all"` (no filter). The exclusion is post-filter only (commits [`398f42f`](https://github.com/jphein/mempalace/commit/398f42f), [`f9f5cc4`](https://github.com/jphein/mempalace/commit/f9f5cc4)) — combining `$nin`/`$in` metadata operators with vector queries trips a ChromaDB 1.5.x filter-planner bug that returns `Internal error: Error finding id` on every query. Post-filter checks both `topic` metadata AND text-prefix shape; coverage equivalent to where-clause + post-filter belt-and-suspenders, no chromadb bug. With kind != "all", over-fetch pulls `max(n*20, 100)` candidates so substantive content survives the filter on a checkpoint-dominant corpus. Companion fix in [`jphein/palace-daemon` `b4b39fc`](https://github.com/jphein/palace-daemon/commit/b4b39fc) plumbs `kind=` through `/search` + `/context` HTTP routes (and fixes a coexisting bug where `limit=` had been silently ignored). | PR pending — fork commits [`8d02835`](https://github.com/jphein/mempalace/commit/8d02835) → [`3d85739`](https://github.com/jphein/mempalace/commit/3d85739) → [`398f42f`](https://github.com/jphein/mempalace/commit/398f42f) → [`f9f5cc4`](https://github.com/jphein/mempalace/commit/f9f5cc4), 9 regression tests in `TestCheckpointFilter`, end-to-end validated against the 151K-drawer canonical palace on 2026-04-25 | small | low | `searcher.py`, `mcp_server.py`, `tests/test_searcher.py` |
3737
| **Reliability** | Call `quarantine_stale_hnsw()` in `make_client()` itself + lower threshold 3600→300s — upstream's #1062 wires it at server startup but misses short-lived callers (hooks, CLI). Production 0.96h-drift segfault confirmed 1h threshold was too loose. | [#1173](https://github.com/milla-jovovich/mempalace/pull/1173) filed 2026-04-24, complementary to [#1062](https://github.com/MemPalace/mempalace/pull/1062) | small | low | `backends/chroma.py` |
3838
| **Reliability** | Gate `quarantine_stale_hnsw` in `make_client()` to once-per-palace-per-process. Symptom on canonical disks daemon (no Syncthing replication of palace data — confirmed `.stignore` excludes `mempalace-data`): `.drift-*` directories accumulating every 10–30 min throughout the day. Root cause: `chroma.sqlite3` mtime bumps per write but HNSW segments flush on chromadb's internal cadence; under steady write load the mtime gap exceeds the 300s threshold from #1173 even though nothing is corrupt. The proactive check renames a valid HNSW segment, chromadb rebuilds, drift recurs as soon as the next batch lands. **Real cold-start drift still caught** (fresh process opening replicated/restored palace); **real runtime errors still caught** via palace-daemon `_auto_repair` calling `quarantine_stale_hnsw` directly. | PR pending — fork commit [`70c4bc6`](https://github.com/jphein/mempalace/commit/70c4bc6), 2 new tests in `test_backends.py` (single-fire-per-palace, per-palace independence) | tiny | low | `backends/chroma.py`, `tests/test_backends.py` |
39+
| **Reliability** | Make `quarantine_stale_hnsw` non-destructive — add an integrity sniff-test that distinguishes corruption from chromadb's normal async flush lag. **Production root-cause** confirmed in disks daemon journal 2026-04-26 06:56:45: every cold-start renamed all three HNSW segment dirs (538-557s `chroma.sqlite3` mtime gap), chromadb created empty replacements, vector recall went to 0/N until a 15-min `mempalace repair --mode rebuild` repopulated. The mtime threshold alone can't tell flush-lag from corruption: chromadb 1.5.x flushes HNSW asynchronously and a clean shutdown does NOT force-flush, so on-disk HNSW is *always* somewhat older than `chroma.sqlite3`. Fix: stage 2 integrity gate sniffs `index_metadata.pickle` for its protocol/terminator bytes (no deserialization — security-hook compliant) before renaming. Healthy segment with mtime drift → keep in place; truncated/zero-filled metadata → quarantine. Production case from 06:24 had `data_level0.bin=253MB` + 18MB metadata file — clearly healthy, but renamed under old logic. | PR pending — 4 new tests in `test_backends.py` (renames-corrupt, leaves-healthy-with-drift, leaves-no-metadata, renames-truncated) | small | low | `backends/chroma.py`, `tests/test_backends.py` |
3940
| **Search** | Surface `drawer_id` in `mempalace_search` results, `mempalace_diary_read` entries, and the new `mempalace_session_recovery_read` payload. ChromaDB's primary key was always returned by `query()` / `get()` but never plumbed into the result-building loop, so callers couldn't link a hit back to a drawer (citation popovers, `mempalace_get_drawer` follow-ups, link-out with real target). Defensive zip-with-id-pad keeps existing test mocks working without touching N fixtures. | PR pending — fork commit [`9a8bb77`](https://github.com/jphein/mempalace/commit/9a8bb77), 1 new integration test + 1 inline assertion | tiny | none | `searcher.py`, `mcp_server.py`, `tests/test_searcher.py`, `tests/test_mcp_server.py`, `website/reference/mcp-tools.md` |
4041
| **Search** | Move Stop-hook auto-save checkpoints to a dedicated `mempalace_session_recovery` ChromaDB collection so they're physically absent from `mempalace_search` rather than post-filtered. Promoted from "future work" to "necessary" by 2026-04-25 Cat 9 A/B (`kind=all` 632 tokens/Q vs `kind=content` 3 tokens/Q on 151K palace — over-fetch=100 still wasn't enough on a checkpoint-dominant corpus, the structural change is non-optional). New MCP tool `mempalace_session_recovery_read` reads the recovery collection by session_id, agent, since/until, wing, limit. **Phases A–C shipped** (collection adapter, write routing, new read tool); Phase D (migrate the ~640 checkpoints already in main collection) and Phase E (palace-daemon startup migrate + `mempalace repair --mode reorganize`) deferred for a separate session. Once D lands, the kind= post-filter and over-fetch hack become deletable — checkpoints aren't there to filter. | PR pending — fork commit [`e266365`](https://github.com/jphein/mempalace/commit/e266365), 12 new tests across `test_session_recovery.py` + `TestCheckpointRouting` + `TestSessionRecoveryRead`, design doc at [`docs/superpowers/specs/2026-04-25-checkpoint-collection-split.md`](docs/superpowers/specs/2026-04-25-checkpoint-collection-split.md), 12-task TDD plan at [`docs/superpowers/plans/2026-04-25-checkpoint-collection-split-impl.md`](docs/superpowers/plans/2026-04-25-checkpoint-collection-split-impl.md) | medium | medium | `palace.py`, `mcp_server.py`, `tests/test_session_recovery.py`, `tests/test_mcp_server.py`, `website/reference/mcp-tools.md` |
4142
| **Performance** | L1 importance pre-filter — `importance >= 3` first, full scan fallback | [#660](https://github.com/milla-jovovich/mempalace/pull/660) | small | low | `layers.py` |

0 commit comments

Comments
 (0)