From e5db07918277da5cfe9df325af18c46232abca28 Mon Sep 17 00:00:00 2001 From: colorpanda82 <160292664+colorpanda82@users.noreply.github.com> Date: Wed, 15 Jul 2026 19:21:21 +0300 Subject: [PATCH 1/2] fix(chroma): reset chromadb System cache in ChromaBackend._client() on inode/mtime reopen _client() reconstructs PersistentClient on an inode/mtime change but did not drop chromadb's process-global SharedSystemClient cache first, so the rebuilt client reused the stale path-keyed System (and its in-memory HNSW segment) and could persist an outdated index over on-disk changes -- the same class as #2002, reached via _client() instead of _get_client. Add SharedSystemClient.clear_system_cache() to the external-change branch of _client(), mirroring mcp_server._force_chroma_cache_reset (#2026) and repair._close_chroma_handles. Backend-level regression test asserts the reset fires on the change reopen, strictly before the reconstruct, and not on first open (chroma-core/chroma#2536, #5843). Fixes #2028. --- mempalace/backends/chroma.py | 35 +++++++++++++++++++ tests/test_backends.py | 65 ++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) diff --git a/mempalace/backends/chroma.py b/mempalace/backends/chroma.py index ee8f10b384..24d2ad6a48 100644 --- a/mempalace/backends/chroma.py +++ b/mempalace/backends/chroma.py @@ -1311,6 +1311,33 @@ def _close_client(client) -> None: logger.debug("client.close() unavailable or failed", exc_info=True) +def _clear_chroma_system_cache() -> None: + """Drop chromadb's process-global ``SharedSystemClient`` cache. + + chromadb caches its ``System`` (and the live HNSW segment) keyed by path. + A bare ``chromadb.PersistentClient(path=...)`` reopen reuses that cached + System, so after a peer/rebuild has changed ``chroma.sqlite3`` on disk we + would rebuild against the stale in-memory segment and persist an outdated + index over the on-disk changes -- the same data-loss class as #2002, + reached via :meth:`ChromaBackend._client` instead of + ``mcp_server._get_client``. This mirrors the reset already performed by + ``mcp_server._force_chroma_cache_reset`` and ``repair._close_chroma_handles``. + + The clear is process-global (it evicts every palace's cached System, not + just this path); chromadb exposes no per-path eviction. It only fires on the + inode/mtime-change branch of ``_client``, never the steady-state hot path, + so the redundant rebuild cost is bounded to genuine external-change reopens. + """ + try: + from chromadb.api.client import SharedSystemClient + + clear = getattr(SharedSystemClient, "clear_system_cache", None) + if callable(clear): + clear() + except Exception: + logger.debug("Failed to clear chromadb SharedSystemClient cache", exc_info=True) + + class ChromaCollection(BaseCollection): """Thin adapter translating ChromaDB dict returns into typed results. @@ -2054,6 +2081,14 @@ def _client(self, palace_path: str): or (mtime_appeared and palace_path in self._freshness) ): ChromaBackend._quarantined_paths.discard(palace_path) + # #2028: the same external change means chromadb's path-keyed + # System cache is now stale. Reconstructing PersistentClient + # below would reuse the cached System (and its in-memory HNSW + # segment), so drop the shared cache first -- otherwise the + # rebuilt client persists an outdated index over the on-disk + # change. Gated on genuine external change (not first open) so + # cold opens never pay the global-evict cost. + _clear_chroma_system_cache() ChromaBackend._prepare_palace_for_open(palace_path) cached = chromadb.PersistentClient(path=palace_path) self._clients[palace_path] = cached diff --git a/tests/test_backends.py b/tests/test_backends.py index 3168ec73dc..92a1b3610a 100644 --- a/tests/test_backends.py +++ b/tests/test_backends.py @@ -1827,6 +1827,71 @@ class DummyClient: ] +def test_chroma_backend_resets_system_cache_on_inode_change(tmp_path, monkeypatch): + """#2028: ``_client`` must drop chromadb's path-keyed ``SharedSystemClient`` + cache *before* reconstructing ``PersistentClient`` on an inode/mtime change. + + chromadb caches its ``System`` (and live HNSW segment) keyed by path, so a + bare reopen reuses the stale segment and persists an outdated index over a + peer/rebuild's on-disk changes -- the #2002 data-loss class reached via + ``_client`` instead of ``mcp_server._get_client``. The reset must fire only + on a genuine external change (not first open) and must precede the reopen. + """ + palace = tmp_path / "palace" + palace.mkdir() + (palace / "chroma.sqlite3").write_text("") + + events = [] + + # Neutralize the on-disk HNSW pre-checks so the test exercises only the + # cache-reset / client-rebuild ordering. + for _name in ( + "_fix_missing_collection_type", + "_fix_blob_seq_ids", + "quarantine_invalid_hnsw_metadata", + "quarantine_stale_hnsw", + ): + monkeypatch.setattr( + f"mempalace.backends.chroma.{_name}", lambda path, *a, **k: [] + ) + + monkeypatch.setattr(ChromaBackend, "_quarantined_paths", set()) + + class DummyClient: + pass + + def _record_open(path): + events.append(("open", path)) + return DummyClient() + + monkeypatch.setattr( + "mempalace.backends.chroma.chromadb.PersistentClient", _record_open + ) + + from chromadb.api.client import SharedSystemClient + + def _record_clear(*args, **kwargs): + events.append(("clear", None)) + + monkeypatch.setattr(SharedSystemClient, "clear_system_cache", _record_clear) + + backend = ChromaBackend() + # ``_db_stat`` is called twice per ``_client`` call (freshness check, then + # re-stat after reopen). Same inode on the first call (first open, no prior + # freshness -> no reset), changed inode on the second (external change). + stats = iter([(1, 1.0), (1, 1.0), (2, 2.0), (2, 2.0)]) + monkeypatch.setattr(backend, "_db_stat", lambda path: next(stats)) + + backend._client(str(palace)) # first open: no external change -> no clear + backend._client(str(palace)) # inode 1 -> 2: clear, then reopen + + assert events == [ + ("open", str(palace)), # first open, no cache reset + ("clear", None), # #2028: reset fires on the inode change... + ("open", str(palace)), # ...strictly before the PersistentClient reopen + ], events + + def test_explain_ef_mismatch_recognizes_chromadb_conflict(): """When ChromaDB rejects a collection read due to an EF-name mismatch (user changed MEMPALACE_EMBEDDING_MODEL on an existing palace), the From e672a3f7f098ea617d30ff288930ca286f364c7f Mon Sep 17 00:00:00 2001 From: Cristian Deheleanu <160292664+colorpanda82@users.noreply.github.com> Date: Fri, 17 Jul 2026 21:37:31 +0300 Subject: [PATCH 2/2] test(chroma): reformat test_backends.py to satisfy ruff format Two monkeypatch.setattr calls were wrapped across lines that fit within the line length; ruff format --check flagged them. Formatter-only, no behavior change. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Gg6g5efZ1rNbBTHqGz2Tjw --- tests/test_backends.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/test_backends.py b/tests/test_backends.py index 92a1b3610a..262d43d286 100644 --- a/tests/test_backends.py +++ b/tests/test_backends.py @@ -1851,9 +1851,7 @@ def test_chroma_backend_resets_system_cache_on_inode_change(tmp_path, monkeypatc "quarantine_invalid_hnsw_metadata", "quarantine_stale_hnsw", ): - monkeypatch.setattr( - f"mempalace.backends.chroma.{_name}", lambda path, *a, **k: [] - ) + monkeypatch.setattr(f"mempalace.backends.chroma.{_name}", lambda path, *a, **k: []) monkeypatch.setattr(ChromaBackend, "_quarantined_paths", set()) @@ -1864,9 +1862,7 @@ def _record_open(path): events.append(("open", path)) return DummyClient() - monkeypatch.setattr( - "mempalace.backends.chroma.chromadb.PersistentClient", _record_open - ) + monkeypatch.setattr("mempalace.backends.chroma.chromadb.PersistentClient", _record_open) from chromadb.api.client import SharedSystemClient