fix(backend): respect backend.persist_directory from palace mempalace.yaml by w1tc4 · Pull Request #1658 · MemPalace/mempalace

w1tc4 · 2026-05-30T00:10:33Z

Problem

backend.persist_directory in a palace's mempalace.yaml was silently ignored. ChromaDB was always initialized at palace_path root because every call site hardcoded path=palace_path and no code ever read the palace-level YAML's backend section.

Users who configured:

backend:
  type: chroma
  persist_directory: ".System_Data"

saw chroma.sqlite3 and the HNSW segment directories keep reappearing in the palace root on every restart — making OneDrive, Dropbox, and other sync tools surface internal DB files alongside user content, and making it impossible to keep the palace directory organized.

Root cause

ChromaBackend._client(), make_client(), _db_stat(), _prepare_palace_for_open(), and detect() all received palace_path and passed it straight to chromadb.PersistentClient(path=palace_path). There was no code path that read <palace_path>/mempalace.yaml at all.

Fix

Add ChromaBackend._resolve_persist_dir(palace_path) which:

Reads <palace_path>/mempalace.yaml for backend.persist_directory
Resolves relative paths against palace_path
Creates the directory if it does not exist
Falls back to palace_path for unconfigured palaces (fully backwards-compatible)

Wire it into every affected call site:

_db_stat — inode/mtime freshness check
_client — db_path existence check + PersistentClient init
_prepare_palace_for_open — HNSW quarantine + blob seq_id fix
make_client — legacy entry point
detect — backend auto-detection
mcp_server._get_client — stale-cache db_path probe (both occurrences)

Testing

from mempalace.backends.chroma import ChromaBackend
# With backend.persist_directory: ".System_Data" in palace yaml:
ChromaBackend._resolve_persist_dir("D:/OneDrive/palace")
# → "D:\OneDrive\palace\.System_Data"

# Without backend section (existing palaces):
ChromaBackend._resolve_persist_dir("/path/to/palace")
# → "/path/to/palace"  (unchanged)

gemini-code-assist

Code Review

This pull request introduces dynamic resolution of the ChromaDB persistence directory by reading the backend.persist_directory configuration from mempalace.yaml. It updates several methods in ChromaBackend and mcp_server.py to use this resolved path. The feedback highlights that _resolve_persist_dir unconditionally creates directories, which introduces side effects during read-only operations like detect() or status checks. It is recommended to add a create flag to control directory creation and to add defensive type checks during YAML parsing.

gemini-code-assist · 2026-05-30T00:12:05Z

+    def _resolve_persist_dir(palace_path: str) -> str:
+        """Return the directory ChromaDB should use for this palace.
+
+        Reads ``<palace_path>/mempalace.yaml`` for ``backend.persist_directory``.
+        Relative paths are resolved against ``palace_path``; the directory is
+        created if it does not yet exist.  Falls back to ``palace_path`` for
+        palaces that predate this config key (fully backwards-compatible).
+        """
+        try:
+            import yaml  # PyYAML — always available as a mempalace dependency
+
+            yaml_path = os.path.join(palace_path, "mempalace.yaml")
+            if os.path.isfile(yaml_path):
+                with open(yaml_path, encoding="utf-8") as fh:
+                    cfg = yaml.safe_load(fh) or {}
+                persist = (cfg.get("backend") or {}).get("persist_directory")
+                if persist:
+                    resolved = (
+                        persist
+                        if os.path.isabs(persist)
+                        else os.path.normpath(os.path.join(palace_path, persist))
+                    )
+                    os.makedirs(resolved, exist_ok=True)
+                    return resolved
+        except Exception:
+            pass
+        return palace_path


The _resolve_persist_dir method currently calls os.makedirs(resolved, exist_ok=True) unconditionally. Since this method is called by read-only operations like detect(), _db_stat(), and status checks in mcp_server.py, it introduces an undesirable side effect of creating directories on the filesystem during simple detection or status probes. This can also lead to unexpected PermissionError exceptions if the user has read-only access to the parent directory.

Additionally, the YAML parsing is not fully defensive. If mempalace.yaml contains a non-dictionary top-level element or if backend is not a dictionary, it can raise an AttributeError (which is caught by the broad except Exception block but is better avoided with explicit type checks).

To fix this, we should add a create: bool = False parameter to _resolve_persist_dir and only call os.makedirs when create=True is explicitly requested (e.g., when initializing the PersistentClient). We should also add defensive type checks for the parsed YAML configuration.

@staticmethod def _resolve_persist_dir(palace_path: str, create: bool = False) -> str: """Return the directory ChromaDB should use for this palace. Reads "<palace_path>/mempalace.yaml" for "backend.persist_directory". Relative paths are resolved against "palace_path"; the directory is created if "create" is True and it does not yet exist. Falls back to "palace_path" for palaces that predate this config key (fully backwards-compatible). """ try: import yaml # PyYAML — always available as a mempalace dependency yaml_path = os.path.join(palace_path, "mempalace.yaml") if os.path.isfile(yaml_path): with open(yaml_path, encoding="utf-8") as fh: cfg = yaml.safe_load(fh) if isinstance(cfg, dict): backend_cfg = cfg.get("backend") if isinstance(backend_cfg, dict): persist = backend_cfg.get("persist_directory") if isinstance(persist, str) and persist.strip(): resolved = ( persist if os.path.isabs(persist) else os.path.normpath(os.path.join(palace_path, persist)) ) if create: os.makedirs(resolved, exist_ok=True) return resolved except Exception: pass return palace_path

gemini-code-assist · 2026-05-30T00:12:05Z

                ChromaBackend._quarantined_paths.discard(palace_path)
            ChromaBackend._prepare_palace_for_open(palace_path)
-            cached = chromadb.PersistentClient(path=palace_path)
+            cached = chromadb.PersistentClient(path=persist_dir)


Pass create=True to _resolve_persist_dir when initializing the PersistentClient to ensure the directory is created when the client is actually instantiated.

Suggested change

cached = chromadb.PersistentClient(path=persist_dir)

cached = chromadb.PersistentClient(path=ChromaBackend._resolve_persist_dir(palace_path, create=True))

gemini-code-assist · 2026-05-30T00:12:05Z

        """
        ChromaBackend._prepare_palace_for_open(palace_path)
-        return chromadb.PersistentClient(path=palace_path)
+        return chromadb.PersistentClient(path=ChromaBackend._resolve_persist_dir(palace_path))


Pass create=True to _resolve_persist_dir when initializing the PersistentClient to ensure the directory is created when the client is actually instantiated.

Suggested change

return chromadb.PersistentClient(path=ChromaBackend._resolve_persist_dir(palace_path))

return chromadb.PersistentClient(path=ChromaBackend._resolve_persist_dir(palace_path, create=True))

igorls · 2026-05-30T06:03:54Z

The room_detector_local config-preserve change is the solid part; the persist_directory wiring needs work before merge:

Blocking — feature is half-wired, breaks search. _resolve_persist_dir is applied to the writer and a few probes, but core readers still hardcode the palace-root chroma.sqlite3: searcher.py:308 (raises SearchError: No palace database) and :424 (BM25 fallback), palace.py:124, cli.py:625/863, migrate.py:141/219, mcp_server.py:742/2727, and ~10 sites in repair.py. Reproduced: set backend.persist_directory, mine → DB lands in the subdir, then search reports "no palace". Either route all chroma.sqlite3 / PersistentClient construction through _resolve_persist_dir, or scope the feature down — as-is it's worse than the bug it fixes for anyone who sets the key.
Blocking — rebase onto develop. Base predates fix(backends): repair missing _type in collection config (#1611) #1617; git merge-tree shows a real conflict in _prepare_palace_for_open. After resolving, _fix_missing_collection_type must also receive persist_dir, or it silently no-ops when the key is set.
Tests. No tests for _resolve_persist_dir or any rewired site. A round-trip test (set persist_directory → mine → search/status find the DB) would have caught Congratulations Milla #1.
CI / perf nits. ruff format --check fails on mcp_server.py:454 & :676 (101 chars). _resolve_persist_dir also parses YAML on the per-op hot path with no memoization and calls os.makedirs from read-only paths (detect()); consider caching the resolved dir and moving directory-creation out of read paths.

….yaml (MemPalace#1658) Wire _resolve_persist_dir() to every chroma.sqlite3 call site so that backend.persist_directory in mempalace.yaml is honoured by all code paths (writer, readers, search, repair, migrate, mcp_server, cli). Also preserve non-managed keys (backend, storage) when re-running init. Co-authored-by: witchsource <rachael.a.meehan@gmail.com>

w1tc4 · 2026-06-11T21:59:31Z

_resolve_persist_dir wired to all ~20 hardcoded sites across chroma.py, searcher.py, migrate.py, repair.py, cli.py,
mcp_server.py
- @functools.lru_cache — no per-call YAML parsing
- makedirs only runs on write paths (create=True)
- 6 tests, all passing
- ruff clean
- No hooks.json change

Verifies that set persist_directory → mine → search all use the same subdir: DB lands in the configured location, not the palace root, and search_memories finds results without a 'no palace' error.

igorls · 2026-08-15T15:52:32Z

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.

w1tc4 requested a review from milla-jovovich as a code owner May 30, 2026 00:10

gemini-code-assist Bot reviewed May 30, 2026

View reviewed changes

w1tc4 mentioned this pull request May 30, 2026

Windows: plugin hooks.json bash wrapper mangles paths, causing Stop/PreCompact hooks to fail #1660

Open

igorls mentioned this pull request May 30, 2026

fix: replace bash wrapper hooks with direct mempalace commands (Windows path fix) #1661

Open

w1tc4 force-pushed the fix/persist-directory-config branch from aa41658 to 73b20f9 Compare May 30, 2026 10:32

w1tc4 requested a review from igorls as a code owner May 30, 2026 10:32

WalksWithASwagger mentioned this pull request Jun 2, 2026

docs: refresh roadmap release and PR truth #1677

Open

w1tc4 force-pushed the fix/persist-directory-config branch from 4c723c4 to 3dcdd9a Compare June 11, 2026 21:54

test: add persist_directory round-trip integration test

278accf

Verifies that set persist_directory → mine → search all use the same subdir: DB lands in the configured location, not the palace root, and search_memories finds results without a 'no palace' error.

Navigation Menu

Uh oh!

fix(backend): respect backend.persist_directory from palace mempalace.yaml - #1658

fix(backend): respect backend.persist_directory from palace mempalace.yaml#1658
w1tc4 wants to merge 2 commits into
MemPalace:developfrom
w1tc4:fix/persist-directory-config

w1tc4 commented May 30, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot May 30, 2026

Uh oh!

gemini-code-assist Bot May 30, 2026

Uh oh!

gemini-code-assist Bot May 30, 2026

Uh oh!

igorls commented May 30, 2026 •

edited

Loading

Uh oh!

w1tc4 commented Jun 11, 2026

Uh oh!

igorls commented Aug 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	cached = chromadb.PersistentClient(path=persist_dir)
	cached = chromadb.PersistentClient(path=ChromaBackend._resolve_persist_dir(palace_path, create=True))

	return chromadb.PersistentClient(path=ChromaBackend._resolve_persist_dir(palace_path))
	return chromadb.PersistentClient(path=ChromaBackend._resolve_persist_dir(palace_path, create=True))

Uh oh!

Conversation

w1tc4 commented May 30, 2026

Problem

Root cause

Fix

Testing

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 30, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 30, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 30, 2026

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

w1tc4 commented Jun 11, 2026

Uh oh!

igorls commented Aug 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

igorls commented May 30, 2026 •

edited

Loading