diff --git a/mempalace/backends/__init__.py b/mempalace/backends/__init__.py index ab22ec6..4e8fdce 100644 --- a/mempalace/backends/__init__.py +++ b/mempalace/backends/__init__.py @@ -19,6 +19,7 @@ from .base import ( BackendError, BaseBackend, BaseCollection, + CollectionNotInitializedError, DimensionMismatchError, EmbedderIdentityMismatchError, GetResult, @@ -46,6 +47,7 @@ __all__ = [ "BaseCollection", "ChromaBackend", "ChromaCollection", + "CollectionNotInitializedError", "DimensionMismatchError", "EmbedderIdentityMismatchError", "GetResult", diff --git a/mempalace/backends/base.py b/mempalace/backends/base.py index 2ff9b87..a32f0a8 100644 --- a/mempalace/backends/base.py +++ b/mempalace/backends/base.py @@ -35,6 +35,18 @@ class PalaceNotFoundError(BackendError, FileNotFoundError): """ +class CollectionNotInitializedError(PalaceNotFoundError): + """Raised when the palace exists on disk but the requested collection has + never been created (e.g. ``init`` ran but ``mine`` has not). + + Distinct from :class:`PalaceNotFoundError`: the palace dir and DB are + present and valid, only the collection has not been bootstrapped yet. + Subclass of :class:`PalaceNotFoundError` (and therefore + :class:`FileNotFoundError`) so legacy callers catching either parent + keep working unchanged. + """ + + class BackendClosedError(BackendError): """Raised when a backend method is called after ``close()``.""" diff --git a/mempalace/backends/chroma.py b/mempalace/backends/chroma.py index 581a313..387a2b9 100644 --- a/mempalace/backends/chroma.py +++ b/mempalace/backends/chroma.py @@ -16,6 +16,7 @@ from chromadb.errors import NotFoundError as _ChromaNotFoundError from .base import ( BaseBackend, BaseCollection, + CollectionNotInitializedError, GetResult, HealthStatus, PalaceNotFoundError, @@ -1380,7 +1381,10 @@ class ChromaBackend(BaseBackend): **ef_kwargs, ) else: - collection = client.get_collection(collection_name, **ef_kwargs) + try: + collection = client.get_collection(collection_name, **ef_kwargs) + except _ChromaNotFoundError as e: + raise CollectionNotInitializedError(palace_path) from e _pin_hnsw_threads(collection) return ChromaCollection(collection, palace_path=palace_path) diff --git a/mempalace/cli.py b/mempalace/cli.py index bd34699..48b6af4 100644 --- a/mempalace/cli.py +++ b/mempalace/cli.py @@ -590,6 +590,10 @@ def cmd_sync(args): if not os.path.isdir(palace_path): print(f"\n No palace found at {palace_path}") return + if not os.path.isfile(os.path.join(palace_path, "chroma.sqlite3")): + print(f"\n Palace dir at {palace_path} exists but has no chroma.sqlite3 yet.") + print(" Run: mempalace mine ") + return project_dirs = [] if args.dir: @@ -1007,15 +1011,20 @@ def cmd_compress(args): else: dialect = Dialect() - # Connect to palace - backend = ChromaBackend() - try: - col = backend.get_collection(palace_path, "mempalace_drawers") - except Exception: - print(f"\n No palace found at {palace_path}") - print(" Run: mempalace init then mempalace mine ") + # State-aware open: distinguish "no palace" from "initialized but empty" + # from "corrupt" via the shared helper (#1498). MCP and library callers + # catch the backend exceptions directly; CLI gets the friendly print. + from .palace import _open_collection_or_explain + + col = _open_collection_or_explain(palace_path, collection_name="mempalace_drawers") + if col is None: sys.exit(1) + # Backend instance for the closets write below. chromadb's + # PersistentClient is cached per palace path internally, so this second + # ChromaBackend instance shares the underlying client with the helper. + backend = ChromaBackend() + # Query drawers in batches to avoid SQLite variable limit (~999) where = {"wing": args.wing} if args.wing else None _BATCH = 500 diff --git a/mempalace/miner.py b/mempalace/miner.py index 0b55b7b..0eab9dc 100644 --- a/mempalace/miner.py +++ b/mempalace/miner.py @@ -22,6 +22,7 @@ from typing import Optional from .palace import ( NORMALIZE_VERSION, SKIP_DIRS, + _open_collection_or_explain, build_closet_lines, file_already_mined, get_closets_collection, @@ -1355,11 +1356,8 @@ def _compute_topic_tunnels_for_wing(wing: str) -> int: def status(palace_path: str): """Show what's been filed in the palace.""" - try: - col = get_collection(palace_path, create=False) - except Exception: - print(f"\n No palace found at {palace_path}") - print(" Run: mempalace init then mempalace mine ") + col = _open_collection_or_explain(palace_path) + if col is None: return # Count by wing and room — paginate to avoid SQLite "too many SQL diff --git a/mempalace/palace.py b/mempalace/palace.py index 38c3979..8df68e5 100644 --- a/mempalace/palace.py +++ b/mempalace/palace.py @@ -13,6 +13,7 @@ import sys import threading from typing import Optional +from .backends import BackendClosedError, CollectionNotInitializedError, PalaceNotFoundError from .backends.chroma import ChromaBackend logger = logging.getLogger("mempalace_mcp") @@ -78,6 +79,72 @@ def get_closets_collection(palace_path: str, create: bool = True): return get_collection(palace_path, collection_name="mempalace_closets", create=create) +def _open_collection_or_explain( + palace_path: str, + *, + collection_name: Optional[str] = None, + out=None, +): + """Open the palace collection or print a state-specific message and return ``None``. + + For CLI and repair commands that want consistent, actionable user-facing + messages distinguishing four "not-healthy" states from one another. MCP + and library callers should catch + :class:`mempalace.backends.PalaceNotFoundError` / + :class:`mempalace.backends.CollectionNotInitializedError` directly. + + The MCP server (``mcp_server.tool_status``) deliberately does NOT use + this helper: it uses ``_get_collection(create=db_exists)`` so a valid + palace whose collection was never bootstrapped lazily gets one on the + first status call, and a corruption-detection sqlite-only probe fires + first when the vector path is disabled (see PR #831 / issue #830). + + State A: palace dir is absent. + State B: dir is present but ``chroma.sqlite3`` is absent. The helper + short-circuits to a message before reaching the backend, because + ``chromadb.PersistentClient`` lazily creates the DB file on first + open — calling the backend on this state would silently mutate + the filesystem for what should be a read-only inspection. + State C: DB is present but the ``mempalace_drawers`` collection has + never been bootstrapped (``init`` ran, ``mine`` has not). + State D: healthy — returns the opened collection. + State E: an unexpected error opens the backend — message points the + user at ``repair-status`` for further diagnosis. + + ``out`` is the message sink; defaults to the builtin ``print``. Pass a + callable (e.g. a repair progress emitter) to route messages through it. + """ + emit = out if out is not None else print + + if not os.path.isdir(palace_path): + emit(f"\n No palace found at {palace_path}") + emit(" Run: mempalace init then mempalace mine ") + return None + if not os.path.isfile(os.path.join(palace_path, "chroma.sqlite3")): + emit(f"\n Palace dir at {palace_path} exists but has no chroma.sqlite3 yet.") + emit(" Run: mempalace mine ") + return None + try: + return get_collection(palace_path, collection_name=collection_name, create=False) + except CollectionNotInitializedError: + emit(f"\n Palace at {palace_path} is initialized but empty (no drawers yet).") + emit(" Run: mempalace mine ") + return None + except PalaceNotFoundError: + emit(f"\n No palace found at {palace_path}") + emit(" Run: mempalace init then mempalace mine ") + return None + except BackendClosedError: + # Surface this as a programmer error, not a palace-state UX message: + # a closed backend means the caller violated the backend lifecycle, + # not that the palace on disk is in a recoverable state. + raise + except Exception as e: # noqa: BLE001 — backend exceptions vary (chromadb, OSError, lock errors) + emit(f"\n Error opening palace at {palace_path}: {e!r}") + emit(" Try: mempalace repair-status --palace ") + return None + + CLOSET_CHAR_LIMIT = 1500 # fill closet until ~1500 chars, then start a new one CLOSET_EXTRACT_WINDOW = 5000 # how many chars of source content to scan for entities/topics diff --git a/mempalace/repair.py b/mempalace/repair.py index c5ad068..778a354 100644 --- a/mempalace/repair.py +++ b/mempalace/repair.py @@ -1222,6 +1222,21 @@ def status(palace_path=None, collection_name: Optional[str] = None) -> dict: print(" No palace found.\n") return {"status": "unknown", "message": "no palace at path"} + db_path = os.path.join(palace_path, "chroma.sqlite3") + if not os.path.isfile(db_path): + print(f" Palace dir at {palace_path} exists but has no chroma.sqlite3 yet.\n") + return {"status": "uninitialized", "message": "palace has no chroma.sqlite3 yet"} + + # Cheap collection-existence check via sqlite. By design this function + # never opens a chromadb client (see the docstring); sqlite_drawer_count + # reads chroma.sqlite3 directly and returns None on any schema/lock error + # (chromadb version drift, missing tables, locked file). None means fall + # through and let hnsw_capacity_status report "unknown". + drawer_row_count = sqlite_drawer_count(palace_path, collection_name) + if isinstance(drawer_row_count, int) and drawer_row_count == 0: + print(" Palace is initialized but empty (no drawers yet).\n") + return {"status": "empty", "message": "palace has no drawers yet"} + drawers = hnsw_capacity_status(palace_path, collection_name) closets = hnsw_capacity_status(palace_path, CLOSETS_COLLECTION_NAME) diff --git a/mempalace/searcher.py b/mempalace/searcher.py index f644fda..7f7e2e2 100644 --- a/mempalace/searcher.py +++ b/mempalace/searcher.py @@ -16,6 +16,7 @@ import re import sqlite3 from pathlib import Path +from .backends import CollectionNotInitializedError, PalaceNotFoundError from .palace import get_closets_collection, get_collection # Closet pointer line format: "topic|entities|→drawer_id_a,drawer_id_b" @@ -297,7 +298,13 @@ def search(query: str, palace_path: str, wing: str = None, room: str = None, n_r """ try: col = get_collection(palace_path, create=False) - except Exception as e: + except CollectionNotInitializedError as e: + # State C from #1498: palace initialized but never mined. + print(f"\n Palace at {palace_path} is initialized but empty (no drawers yet).") + print(" Run: mempalace mine ") + raise SearchError(f"Palace at {palace_path} is initialized but empty") from e + except PalaceNotFoundError as e: + # State A from #1498: palace dir missing. print(f"\n No palace found at {palace_path}") print(" Run: mempalace init then mempalace mine ") raise SearchError(f"No palace found at {palace_path}") from e diff --git a/tests/test_backends.py b/tests/test_backends.py index 90cf128..8f36126 100644 --- a/tests/test_backends.py +++ b/tests/test_backends.py @@ -9,7 +9,9 @@ import chromadb import pytest from mempalace.backends import ( + CollectionNotInitializedError, GetResult, + PalaceNotFoundError, PalaceRef, QueryResult, UnsupportedFilterError, @@ -993,6 +995,43 @@ def test_get_collection_applies_retrofit_on_existing_palace(tmp_path): assert wrapper._collection.configuration_json["hnsw"]["num_threads"] == 1 +def test_get_collection_raises_palace_not_found_when_dir_missing(tmp_path): + """create=False on a missing dir raises PalaceNotFoundError, not the + new CollectionNotInitializedError. The two states must be distinguishable + so callers can render state-specific messages (#1498).""" + missing = tmp_path / "no-such-dir" + with pytest.raises(PalaceNotFoundError) as excinfo: + ChromaBackend().get_collection( + str(missing), + collection_name="mempalace_drawers", + create=False, + ) + # Must be the parent class, not the new subclass: dir is genuinely absent. + assert not isinstance(excinfo.value, CollectionNotInitializedError) + + +def test_get_collection_raises_collection_not_initialized_on_empty_palace(tmp_path): + """When the palace dir + DB exist but the collection has never been + created, ChromaBackend.get_collection(create=False) raises the new + CollectionNotInitializedError instead of leaking chromadb.NotFoundError + (#1498).""" + palace_path = tmp_path / "palace" + palace_path.mkdir() + # PersistentClient lazily creates chroma.sqlite3 — no collection yet. + chromadb.PersistentClient(path=str(palace_path)) + assert (palace_path / "chroma.sqlite3").is_file() + + with pytest.raises(CollectionNotInitializedError) as excinfo: + ChromaBackend().get_collection( + str(palace_path), + collection_name="mempalace_drawers", + create=False, + ) + # Backward-compat: subclass of PalaceNotFoundError (and FileNotFoundError). + assert isinstance(excinfo.value, PalaceNotFoundError) + assert isinstance(excinfo.value, FileNotFoundError) + + def test_quarantine_invalid_hnsw_metadata_renames_missing_dimensionality(tmp_path): palace = tmp_path / "palace" palace.mkdir() diff --git a/tests/test_cli.py b/tests/test_cli.py index 9894358..788d295 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1041,16 +1041,45 @@ def test_cmd_repair_aborts_without_confirmation(mock_config_cls, tmp_path, capsy @patch("mempalace.cli.MempalaceConfig") -def test_cmd_compress_no_palace(mock_config_cls, capsys): - mock_config_cls.return_value.palace_path = "/fake/palace" +def test_cmd_sync_no_palace_dir(mock_config_cls, tmp_path, capsys): + """cmd_sync on a missing palace dir prints the State A message (#1498).""" + from mempalace.cli import cmd_sync + + palace_path = tmp_path / "nonexistent" + mock_config_cls.return_value.palace_path = str(palace_path) + args = argparse.Namespace(palace=None, dir=None, root=[], wing=None, dry_run=False) + cmd_sync(args) + captured = capsys.readouterr() + assert "No palace found" in captured.out + captured.err + + +@patch("mempalace.cli.MempalaceConfig") +def test_cmd_sync_palace_dir_no_db(mock_config_cls, tmp_path, capsys): + """cmd_sync on a palace dir without chroma.sqlite3 prints the State B + message and does NOT trigger chromadb's lazy DB creation (#1498).""" + from mempalace.cli import cmd_sync + + mock_config_cls.return_value.palace_path = str(tmp_path) + args = argparse.Namespace(palace=None, dir=None, root=[], wing=None, dry_run=False) + cmd_sync(args) + captured = capsys.readouterr() + assert "has no chroma.sqlite3 yet" in captured.out + captured.err + # Side-effect-free: backend not invoked. + assert list(tmp_path.iterdir()) == [] + + +@patch("mempalace.cli.MempalaceConfig") +def test_cmd_compress_no_palace(mock_config_cls, tmp_path, capsys): + """cmd_compress exits non-zero with a 'No palace found' message on a missing dir. + + Uses a real non-existent tmp_path so the stratified state helper (#1498) + walks the State A branch instead of hitting the chromadb backend. + """ + mock_config_cls.return_value.palace_path = str(tmp_path / "nonexistent") args = argparse.Namespace(palace=None, wing=None, dry_run=False, config=None) - mock_backend = MagicMock() - mock_backend.get_collection.side_effect = Exception("no palace") - with ( - patch("mempalace.backends.chroma.ChromaBackend", return_value=mock_backend), - pytest.raises(SystemExit), - ): + with pytest.raises(SystemExit): cmd_compress(args) + assert "No palace found" in capsys.readouterr().out @patch("mempalace.cli.MempalaceConfig") @@ -1060,7 +1089,10 @@ def test_cmd_compress_no_drawers(mock_config_cls, capsys): mock_col = MagicMock() mock_col.get.return_value = {"documents": [], "metadatas": [], "ids": []} mock_backend = _mock_backend_for(col=mock_col) - with patch("mempalace.backends.chroma.ChromaBackend", return_value=mock_backend): + with ( + patch("mempalace.palace._open_collection_or_explain", return_value=mock_col), + patch("mempalace.backends.chroma.ChromaBackend", return_value=mock_backend), + ): cmd_compress(args) out = capsys.readouterr().out assert "No drawers found" in out @@ -1103,6 +1135,7 @@ def test_cmd_compress_dry_run(mock_config_cls, capsys): mock_dialect_mod = _make_mock_dialect_module(mock_dialect) with ( + patch("mempalace.palace._open_collection_or_explain", return_value=mock_col), patch("mempalace.backends.chroma.ChromaBackend", return_value=mock_backend), patch.dict("sys.modules", {"mempalace.dialect": mock_dialect_mod}), ): @@ -1127,6 +1160,7 @@ def test_cmd_compress_with_config(mock_config_cls, tmp_path, capsys): mock_dialect_mod = _make_mock_dialect_module(mock_dialect) with ( + patch("mempalace.palace._open_collection_or_explain", return_value=mock_col), patch("mempalace.backends.chroma.ChromaBackend", return_value=mock_backend), patch.dict("sys.modules", {"mempalace.dialect": mock_dialect_mod}), ): @@ -1167,6 +1201,7 @@ def test_cmd_compress_stores_results(mock_config_cls, capsys): mock_dialect_mod = _make_mock_dialect_module(mock_dialect) with ( + patch("mempalace.palace._open_collection_or_explain", return_value=mock_col), patch("mempalace.backends.chroma.ChromaBackend", return_value=mock_backend), patch.dict("sys.modules", {"mempalace.dialect": mock_dialect_mod}), ): diff --git a/tests/test_miner.py b/tests/test_miner.py index d58ef92..2c62de3 100644 --- a/tests/test_miner.py +++ b/tests/test_miner.py @@ -442,6 +442,41 @@ def test_status_missing_palace_does_not_create_empty_collection(tmp_path, capsys assert not palace_path.exists() +def test_status_initialized_but_empty_palace_reports_empty(tmp_path, capsys): + """State C from #1498: palace dir + chroma.sqlite3 exist but no drawers + have been mined yet. status() must print the 'initialized but empty' + message and suggest `mempalace mine`, not the misleading 'No palace + found' / 'Run init' message.""" + import chromadb + + palace_path = tmp_path / "empty-palace" + palace_path.mkdir() + chromadb.PersistentClient(path=str(palace_path)) # creates chroma.sqlite3 + assert (palace_path / "chroma.sqlite3").is_file() + + status(str(palace_path)) + + out = capsys.readouterr().out + assert "initialized but empty" in out + assert "mempalace mine" in out + assert "No palace found" not in out + + +def test_status_palace_dir_without_db_reports_uninitialized(tmp_path, capsys): + """State B from #1498: palace dir exists but chroma.sqlite3 is absent. + Helper must short-circuit before invoking chromadb (which would lazily + create the DB file as a side effect of a read-only inspection).""" + palace_path = tmp_path / "no-db-palace" + palace_path.mkdir() + + status(str(palace_path)) + + out = capsys.readouterr().out + assert "has no chroma.sqlite3 yet" in out + # Side-effect check: chromadb was never touched. + assert list(palace_path.iterdir()) == [] + + def test_status_handles_none_metadata_without_crash(tmp_path, capsys): """status must not crash when col.get returns a None entry in metadatas. @@ -462,7 +497,7 @@ def test_status_handles_none_metadata_without_crash(tmp_path, capsys): "metadatas": [{"wing": "proj", "room": "r"}, None], } - with patch("mempalace.miner.get_collection", return_value=FakeCol()): + with patch("mempalace.miner._open_collection_or_explain", return_value=FakeCol()): status(str(tmp_path)) out = capsys.readouterr().out diff --git a/tests/test_palace.py b/tests/test_palace.py new file mode 100644 index 0000000..1e2325f --- /dev/null +++ b/tests/test_palace.py @@ -0,0 +1,170 @@ +"""Tests for mempalace.palace shared helpers.""" + +import chromadb + +from mempalace.backends import CollectionNotInitializedError, PalaceNotFoundError +from mempalace.palace import _open_collection_or_explain, get_collection + + +def _capture(): + """Return (emit, lines) — emit appends to lines for inspection.""" + lines: list[str] = [] + return lines.append, lines + + +def test_open_collection_or_explain_state_a_missing_dir(tmp_path): + """State A: palace dir does not exist.""" + emit, lines = _capture() + missing = tmp_path / "no-such-palace" + + result = _open_collection_or_explain(str(missing), out=emit) + + assert result is None + assert any("No palace found" in line for line in lines) + assert any("mempalace init" in line for line in lines) + # Helper must not create the directory. + assert not missing.exists() + + +def test_open_collection_or_explain_state_b_no_db(tmp_path): + """State B: dir exists but chroma.sqlite3 does not. + + Critical invariant: the helper must NOT trigger chromadb's lazy DB + creation by reaching the backend. The dir must remain empty after + the call so a read-only inspection stays read-only. + """ + emit, lines = _capture() + palace = tmp_path / "palace" + palace.mkdir() + assert not (palace / "chroma.sqlite3").exists() + + result = _open_collection_or_explain(str(palace), out=emit) + + assert result is None + assert any("has no chroma.sqlite3 yet" in line for line in lines) + # No side-effect: backend was not invoked. + assert list(palace.iterdir()) == [] + + +def test_open_collection_or_explain_state_c_no_collection(tmp_path): + """State C: DB file exists but the collection has never been created.""" + emit, lines = _capture() + palace = tmp_path / "palace" + palace.mkdir() + chromadb.PersistentClient(path=str(palace)) # creates DB, no collection + assert (palace / "chroma.sqlite3").is_file() + + result = _open_collection_or_explain(str(palace), out=emit) + + assert result is None + assert any("initialized but empty" in line for line in lines) + assert any("mempalace mine" in line for line in lines) + + +def test_open_collection_or_explain_state_d_healthy(tmp_path): + """State D: healthy palace — returns the opened collection silently.""" + emit, lines = _capture() + palace = tmp_path / "palace" + palace.mkdir() + get_collection(str(palace), create=True) # bootstrap collection + + result = _open_collection_or_explain(str(palace), out=emit) + + assert result is not None + assert lines == [] # healthy path is silent + + +def test_open_collection_or_explain_state_e_unexpected_error(tmp_path, monkeypatch): + """State E: unexpected error opening the backend routes to repair hint.""" + emit, lines = _capture() + palace = tmp_path / "palace" + palace.mkdir() + (palace / "chroma.sqlite3").touch() # pass the isfile guard + + def boom(*args, **kwargs): + raise RuntimeError("disk on fire") + + monkeypatch.setattr("mempalace.palace.get_collection", boom) + + result = _open_collection_or_explain(str(palace), out=emit) + + assert result is None + assert any("Error opening palace" in line for line in lines) + assert any("repair-status" in line for line in lines) + + +def test_open_collection_or_explain_default_sink_is_print(tmp_path, capsys): + """When out is None, messages go through builtin print → stdout.""" + missing = tmp_path / "no-such-palace" + + result = _open_collection_or_explain(str(missing)) + + assert result is None + assert "No palace found" in capsys.readouterr().out + + +def test_open_collection_or_explain_propagates_palace_not_found_from_backend(tmp_path, monkeypatch): + """If the backend raises bare PalaceNotFoundError after our filesystem + guards (rare race or backend-internal "not found"), the helper still + prints the State A message and returns None.""" + emit, lines = _capture() + palace = tmp_path / "palace" + palace.mkdir() + (palace / "chroma.sqlite3").touch() + + def raise_pnf(*args, **kwargs): + raise PalaceNotFoundError(str(palace)) + + monkeypatch.setattr("mempalace.palace.get_collection", raise_pnf) + + result = _open_collection_or_explain(str(palace), out=emit) + + assert result is None + assert any("No palace found" in line for line in lines) + + +def test_open_collection_or_explain_reraises_backend_closed_error(tmp_path, monkeypatch): + """BackendClosedError is a programmer error (caller violated the backend + lifecycle), not a palace-state UX condition. The helper must propagate + it instead of swallowing it into the State E "repair-status" hint. + + Without this re-raise, a closed default backend would silently mask + every call site as "Error opening palace ... Try: repair-status" + even when the actual fix is to stop using a closed backend handle. + """ + from mempalace.backends import BackendClosedError + + palace = tmp_path / "palace" + palace.mkdir() + (palace / "chroma.sqlite3").touch() + + def raise_closed(*args, **kwargs): + raise BackendClosedError("ChromaBackend has been closed") + + monkeypatch.setattr("mempalace.palace.get_collection", raise_closed) + + import pytest + + with pytest.raises(BackendClosedError): + _open_collection_or_explain(str(palace)) + + +def test_open_collection_or_explain_distinguishes_collection_subclass(tmp_path, monkeypatch): + """The helper must surface CollectionNotInitializedError as the + 'empty' message rather than the broader 'No palace found' message, + even though the former subclasses the latter.""" + emit, lines = _capture() + palace = tmp_path / "palace" + palace.mkdir() + (palace / "chroma.sqlite3").touch() + + def raise_cnie(*args, **kwargs): + raise CollectionNotInitializedError(str(palace)) + + monkeypatch.setattr("mempalace.palace.get_collection", raise_cnie) + + result = _open_collection_or_explain(str(palace), out=emit) + + assert result is None + assert any("initialized but empty" in line for line in lines) + assert not any("No palace found" in line for line in lines) diff --git a/tests/test_repair.py b/tests/test_repair.py index 144d907..d8dfa94 100644 --- a/tests/test_repair.py +++ b/tests/test_repair.py @@ -538,9 +538,104 @@ def test_rebuild_index_default_uses_configured_collection(mock_backend_cls, mock ] +def test_status_returns_uninitialized_when_db_missing(tmp_path, capsys): + """repair.status on a palace dir without chroma.sqlite3 returns a + structured status (no chromadb client opened, per the design that + repair-status must work even on corrupted palaces — #1498).""" + # tmp_path exists, no chroma.sqlite3 + result = repair.status(palace_path=str(tmp_path)) + + assert result["status"] == "uninitialized" + assert "no chroma.sqlite3" in result["message"] + captured = capsys.readouterr() + assert "has no chroma.sqlite3 yet" in captured.out + captured.err + + +def test_status_returns_empty_when_db_present_no_drawers(tmp_path, capsys): + """repair.status on a palace with chroma.sqlite3 but zero drawer rows + returns a structured 'empty' status, distinguishable from 'unknown' / + 'uninitialized' (#1498). Mocks sqlite_drawer_count to assert the + return-shape contract; see the real-disk sibling below for the + no-chromadb-client invariant.""" + (tmp_path / "chroma.sqlite3").touch() + with patch("mempalace.repair.sqlite_drawer_count", return_value=0): + result = repair.status(palace_path=str(tmp_path)) + + assert result["status"] == "empty" + assert "no drawers yet" in result["message"] + captured = capsys.readouterr() + assert "initialized but empty" in captured.out + captured.err + + +def test_status_empty_palace_never_opens_chromadb_client(tmp_path): + """Design invariant from #1498: repair.status on an initialized-but-empty + palace must NOT open a chromadb client. Opening would materialize HNSW + segment state files on disk, breaking the promise that repair-status is + safe to run on corrupted palaces. + + Real-disk sibling of test_status_returns_empty_when_db_present_no_drawers: + bootstrap a real chroma.sqlite3 via PersistentClient (creates the DB + file but no collection), then assert repair.status returns 'empty' and + no chromadb segment artifacts appeared in the dir.""" + import chromadb + + chromadb.PersistentClient(path=str(tmp_path)) + before = sorted(p.name for p in tmp_path.iterdir()) + + result = repair.status(palace_path=str(tmp_path)) + + after = sorted(p.name for p in tmp_path.iterdir()) + assert result["status"] == "empty", result + # repair.status must not create new files; chromadb writes HNSW segment + # state and *.bin payloads on collection open — none of those should + # appear here. + assert before == after, f"repair.status mutated palace on disk: before={before} after={after}" + + +def test_status_falls_through_to_capacity_when_sqlite_count_unreadable(tmp_path): + """When sqlite_drawer_count returns None (schema drift / locked file), + repair.status must fall through to hnsw_capacity_status instead of + short-circuiting on 'empty' (#1498).""" + (tmp_path / "chroma.sqlite3").touch() + with ( + patch("mempalace.repair.sqlite_drawer_count", return_value=None), + patch("mempalace.repair.hnsw_capacity_status") as capacity_status, + ): + capacity_status.side_effect = [ + { + "sqlite_count": None, + "hnsw_count": None, + "divergence": None, + "diverged": False, + "status": "unknown", + "message": "", + }, + { + "sqlite_count": None, + "hnsw_count": None, + "divergence": None, + "diverged": False, + "status": "unknown", + "message": "", + }, + ] + result = repair.status(palace_path=str(tmp_path)) + + # Did not short-circuit on 'empty': fell through to capacity check. + # The healthy/fall-through path returns {drawers, closets} dicts, no top-level "status" key. + assert "status" not in result or result["status"] != "empty" + assert "drawers" in result and "closets" in result + assert capacity_status.called + + def test_status_default_uses_configured_drawer_collection(tmp_path): + # Provide the on-disk preconditions the stratified state helper (#1498) + # checks before reaching the capacity probe: chroma.sqlite3 file exists + # and sqlite_drawer_count returns a positive number (palace not empty). + (tmp_path / "chroma.sqlite3").touch() with ( patch("mempalace.repair._drawers_collection_name", return_value="custom_drawers"), + patch("mempalace.repair.sqlite_drawer_count", return_value=1), patch("mempalace.repair.hnsw_capacity_status") as capacity_status, ): capacity_status.side_effect = [