fix(palace): stratify state messages for empty/missing palace (#1498)
`mempalace status`, `search`, and `compress` printed the same misleading "No palace found / Run: mempalace init" output for three distinct palace states (no dir / no DB / no collection). Most user-visible on the first-run state where `init` had run but `mine` had not: the hint to re-run `init` is a no-op and wastes the user's time. Backend gets a new typed exception `CollectionNotInitializedError` (subclass of `PalaceNotFoundError`, transitively `FileNotFoundError`, so legacy callers keep working). `ChromaBackend.get_collection( create=False)` wraps chromadb's bare `NotFoundError` as the new typed exception instead of leaking the chromadb-specific class to callers. A new internal helper `_open_collection_or_explain` in `palace.py` runs filesystem-first state checks before the backend call to avoid chromadb's lazy `chroma.sqlite3` creation as a side-effect of a read-only inspection, then catches the typed exceptions and prints a state-specific actionable message. `BackendClosedError` is explicitly re-raised so a programmer error is not masked as a UX hint. Two CLI bug sites route through the helper: `miner.status` and `cli.cmd_compress`. `searcher.search` catches the typed exceptions directly so it can preserve the cause chain in `SearchError(...) from e` for programmatic search-API consumers. `cli.cmd_sync` gained an inline filesystem distinction (no helper needed: it does not use the collection handle). `repair.status` (capacity check, which by design must work on corrupted palaces without opening a chromadb client) got the same distinction via `sqlite_drawer_count`-based empty detection. The MCP `tool_status` is intentionally left alone: PR #831 already fixed it there with a `create=True` bootstrap strategy appropriate for programmatic clients.
This commit is contained in:
mvalentsev
parent
bb313961f3
commit
c933095a2f
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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()``."""
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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 <dir>")
|
||||
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 <dir> then mempalace mine <dir>")
|
||||
# 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
|
||||
|
|
|
|||
|
|
@ -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 <dir> then mempalace mine <dir>")
|
||||
col = _open_collection_or_explain(palace_path)
|
||||
if col is None:
|
||||
return
|
||||
|
||||
# Count by wing and room — paginate to avoid SQLite "too many SQL
|
||||
|
|
|
|||
|
|
@ -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 <dir> then mempalace mine <dir>")
|
||||
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 <dir>")
|
||||
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 <dir>")
|
||||
return None
|
||||
except PalaceNotFoundError:
|
||||
emit(f"\n No palace found at {palace_path}")
|
||||
emit(" Run: mempalace init <dir> then mempalace mine <dir>")
|
||||
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 <path>")
|
||||
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
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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 <dir>")
|
||||
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 <dir> then mempalace mine <dir>")
|
||||
raise SearchError(f"No palace found at {palace_path}") from e
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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}),
|
||||
):
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
@ -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 = [
|
||||
|
|
|
|||
Loading…
Reference in New Issue