perf: cache ChromaDB PersistentClient instead of re-instantiating per call by igorls · Pull Request #135 · MemPalace/mempalace

igorls · 2026-04-07T20:20:10Z

Problem

_get_collection() creates a new chromadb.PersistentClient(path=...) on every MCP tool call. This re-opens the SQLite metadata DB and reloads the HNSW index each time, adding unnecessary latency.

Finding:
HIGH — PersistentClient re-instantiation

Fix

Cache the client and collection at module level:

_client_cache = None
_collection_cache = None

def _get_collection(create=False):
    global _client_cache, _collection_cache
    try:
        if _client_cache is None:
            _client_cache = chromadb.PersistentClient(path=_config.palace_path)
        if create:
            _collection_cache = _client_cache.get_or_create_collection(...)
        elif _collection_cache is None:
            _collection_cache = _client_cache.get_collection(...)
        return _collection_cache
    except Exception:
        return None

The MCP server runs as a subprocess (one per session), so the cache lifetime matches the session. No concurrency concerns — MCP is single-threaded stdin/stdout.

92 tests pass (includes test infrastructure from PR #131 + a _reset_mcp_cache fixture for test isolation).

Copilot

Pull request overview

This PR reduces MCP tool-call latency by caching the ChromaDB PersistentClient/collection in the MCP server process, and updates the project’s packaging/test setup alongside a version bump.

Changes:

Cache chromadb.PersistentClient and the target collection in mempalace/mcp_server.py to avoid re-opening SQLite + reloading indexes on every request.
Add/expand pytest coverage for MCP server handlers, the programmatic search API, the dialect, and the temporal knowledge graph (with shared isolation fixtures).
Update packaging metadata/build configuration (move to hatchling, add uv-style dependency groups), remove requirements.txt, and bump __version__ to 3.0.0.

Reviewed changes

Copilot reviewed 8 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`mempalace/mcp_server.py`	Adds module-level caching for ChromaDB client/collection used by MCP tools.
`mempalace/__init__.py`	Updates package version constant to 3.0.0.
`pyproject.toml`	Switches build backend to hatchling and updates dev dependency definitions.
`requirements.txt`	Removed in favor of `pyproject.toml` / uv workflow.
`tests/conftest.py`	Adds isolation fixtures (HOME redirection) and MCP cache reset for test independence.
`tests/test_mcp_server.py`	Adds unit/integration tests for MCP protocol dispatch + tool handlers.
`tests/test_searcher.py`	Adds tests for `search_memories` programmatic API behavior.
`tests/test_knowledge_graph.py`	Adds tests for KnowledgeGraph CRUD + temporal queries + stats/timeline.
`tests/test_dialect.py`	Adds tests for dialect compression/encoding/decoding utilities.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-07T20:24:45Z

 """MemPalace — Give your AI a memory. No API key required."""

-__version__ = "2.0.0"
+__version__ = "3.0.0"

 from .cli import main


__version__ is bumped to 3.0.0 here, but the MCP protocol serverInfo.version reported by mempalace.mcp_server.handle_request() is still hardcoded to 2.0.0. Consider deriving the MCP server version from mempalace.__version__ (or importlib.metadata.version("mempalace")) so clients see a consistent version.

Copilot · 2026-04-07T20:24:45Z

+    yield
+    try:
+        from mempalace import mcp_server
+        mcp_server._client_cache = None
+        mcp_server._collection_cache = None
+    except (ImportError, AttributeError):
+        pass


_reset_mcp_cache only clears the MCP server caches in teardown. If a previous test populated the cache, the next test will still start with whatever state is left until teardown runs again. Resetting the cache in the fixture setup (before yield) as well would make isolation deterministic and reduce the chance of temp-dir cleanup issues due to lingering open ChromaDB handles.

Suggested change

yield

try:

from mempalace import mcp_server

mcp_server._client_cache = None

mcp_server._collection_cache = None

except (ImportError, AttributeError):

pass

def _clear_cache():

try:

from mempalace import mcp_server

mcp_server._client_cache = None

mcp_server._collection_cache = None

except (ImportError, AttributeError):

pass

_clear_cache()

yield

_clear_cache()

Copilot · 2026-04-07T20:24:45Z

+    def test_no_palace_returns_error(self):
+        result = search_memories("anything", "/nonexistent/path")


This test uses a hard-coded absolute path (/nonexistent/path) to simulate a missing palace. That can behave differently across OSes (and could even be creatable in privileged CI), making the test flaky and potentially leaving junk on the filesystem. Prefer using a per-test temp path (e.g. tmp_path / "missing") that is guaranteed not to exist.

Suggested change

def test_no_palace_returns_error(self):

result = search_memories("anything", "/nonexistent/path")

def test_no_palace_returns_error(self, tmp_path):

missing_palace_path = tmp_path / "missing"

result = search_memories("anything", str(missing_palace_path))

Copilot · 2026-04-07T20:24:46Z

+    def test_no_palace_returns_error(self, monkeypatch, config, kg):
+        _patch_mcp_server(monkeypatch, config, "/nonexistent/path", kg)
+        from mempalace.mcp_server import tool_status
+
+        result = tool_status()
+        assert "error" in result
+


This test uses a hard-coded absolute path (/nonexistent/path) to simulate a missing palace. That can behave differently across OSes (and could even be creatable in privileged CI), making the test flaky and potentially leaving junk on the filesystem. Prefer using a per-test temp path (e.g. tmp_path / "missing") that is guaranteed not to exist.

bensig · 2026-04-07T20:45:01Z

Hey Igor — CI is failing, looks like it needs a rebase on main (we've merged a bunch of changes recently). Can you rebase and push?

bensig · 2026-04-07T23:06:06Z

Merged your other 5 PRs — this one now has conflicts from those merges. Can you rebase on main one more time?

… call The MCP server previously created a new PersistentClient on every tool call via _get_collection(). This incurs HNSW index loading overhead on each request. Cache the client and collection at module level. The cache resets naturally on process restart (MCP runs as a subprocess). Also adds a _reset_mcp_cache fixture to conftest.py for test isolation. Includes test infrastructure from PR MemPalace#131. 92 tests pass.

…ion and portability

Audited PR MemPalace#135 (ChromaDB singleton cache): none of its 3 commits port to this fork. The client-caching perf gain is already achieved via the module-level get_db() singleton in db.py, and the companion test fixes target upstream-only test files (test_mcp_server.py, test_searcher.py).

Copilot AI review requested due to automatic review settings April 7, 2026 20:20

Copilot started reviewing on behalf of igorls April 7, 2026 20:20 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

bensig mentioned this pull request Apr 7, 2026

fix: close chroma clients after ingest #49

Closed

igorls force-pushed the fix/chromadb-singleton branch 4 times, most recently from 07f1f86 to e4d938c Compare April 7, 2026 21:59

igorls added 2 commits April 8, 2026 04:39

fix: address Copilot review — derive MCP version, improve test isolat…

47696be

…ion and portability

igorls force-pushed the fix/chromadb-singleton branch from e4d938c to 47696be Compare April 8, 2026 07:41

bensig merged commit 3489a09 into MemPalace:main Apr 8, 2026
4 checks passed

bensig mentioned this pull request Apr 8, 2026

fix: use upsert and deterministic IDs to prevent data stagnation #140

Merged

milla-jovovich mentioned this pull request Apr 9, 2026

chore: bump version to 3.1.0 #409

Merged

6 tasks

sinful1992 mentioned this pull request Apr 11, 2026

BUG: mempalace_search returns stale results after mid-session CLI mine — cached HNSW client not invalidated #608

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf: cache ChromaDB PersistentClient instead of re-instantiating per call#135

perf: cache ChromaDB PersistentClient instead of re-instantiating per call#135
bensig merged 2 commits intoMemPalace:mainfrom
igorls:fix/chromadb-singleton

igorls commented Apr 7, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

bensig commented Apr 7, 2026

Uh oh!

bensig commented Apr 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		def test_no_palace_returns_error(self):
		result = search_memories("anything", "/nonexistent/path")

-    def test_no_palace_returns_error(self):
-        result = search_memories("anything", "/nonexistent/path")
+    def test_no_palace_returns_error(self, tmp_path):
+        missing_palace_path = tmp_path / "missing"
+        result = search_memories("anything", str(missing_palace_path))

Conversation

igorls commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

bensig commented Apr 7, 2026

Uh oh!

bensig commented Apr 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

igorls commented Apr 7, 2026 •

edited

Loading