diff --git a/mempalace/cli.py b/mempalace/cli.py index b7eda8c..0eb6694 100644 --- a/mempalace/cli.py +++ b/mempalace/cli.py @@ -580,6 +580,8 @@ def cmd_mine(args): _submit_daemon_cli_job("mine", payload, args, background=getattr(args, "background", False)) return + from .palace import MineAlreadyRunning, MineValidationError + if source_adapter: try: drawers_written = mine_source_adapter( @@ -591,6 +593,9 @@ def cmd_mine(args): except (UnknownSourceAdapterError, UnsupportedSourceAdapterProtocolError) as exc: print(f"mempalace: {exc}", file=sys.stderr) sys.exit(2) + except MineAlreadyRunning as exc: + print(f"mempalace: {exc}", file=sys.stderr) + sys.exit(1) suffix = " would be written" if args.dry_run else " written" print(f" Source adapter {source_adapter!r}: {drawers_written} drawer(s){suffix}.") return @@ -605,8 +610,6 @@ def cmd_mine(args): llm_provider=None, ) - from .palace import MineAlreadyRunning, MineValidationError - try: if mode == "convos": from .convo_miner import mine_convos @@ -681,15 +684,14 @@ class UnsupportedSourceAdapterProtocolError(ValueError): class _DryRunCollectionProxy: - """Read-only collection facade that records, but never persists, writes. + """Empty collection facade that records, but never persists, writes. Source adapters are deliberately allowed to access ``drawer_collection`` - directly. Passing the live collection during a dry run would therefore - make ``--dry-run`` advisory rather than safe. + directly. A dry run must not open the real backend: even read-only-looking + opens can create or repair backend artifacts (for example SQLite WAL files). """ - def __init__(self, collection=None): - self._collection = collection + def __init__(self): self.operations = [] def add(self, **kwargs): @@ -705,19 +707,23 @@ class _DryRunCollectionProxy: self.operations.append(("update", kwargs)) def query(self, **kwargs): - if self._collection is None: - return {"ids": [[]], "documents": [[]], "metadatas": [[]], "distances": [[]]} - return self._collection.query(**kwargs) + from .backends import QueryResult + + query_input = kwargs.get("query_texts", kwargs.get("query_embeddings")) + num_queries = len(query_input) if isinstance(query_input, (list, tuple)) else 1 + include = kwargs.get("include") or [] + return QueryResult.empty( + num_queries=num_queries, + embeddings_requested="embeddings" in include, + ) def get(self, **kwargs): - if self._collection is None: - return {"ids": [], "documents": [], "metadatas": []} - return self._collection.get(**kwargs) + from .backends import GetResult + + return GetResult.empty() def count(self): - if self._collection is None: - return 0 - return self._collection.count() + return 0 class _DryRunKnowledgeGraphProxy: @@ -778,25 +784,16 @@ def mine_source_adapter( "mempalace mine does not support yet" ) - # A dry run must never create a collection: opening a fresh Chroma palace - # can create storage and persist embedder identity before write proxies are - # installed. It may safely retain a read-only view of an existing - # collection so adapters can produce an accurate preview. Non-dry runs - # hold one writer lease from handle creation through adapter iteration, - # including direct KG mutations by adapters. + # A dry run must never open a collection: backend opens can create or + # repair storage even when requested as read-only. Non-dry runs hold one + # writer lease from handle creation through adapter iteration, including + # direct KG mutations by adapters. lock = mine_palace_lock(palace_path) if not dry_run else contextlib.nullcontext() with lock: knowledge_graph = None try: if dry_run: - try: - drawer_collection = _DryRunCollectionProxy( - get_collection(palace_path, create=False) - ) - except FileNotFoundError: - # No palace or collection has been initialized yet. Use - # an empty recording facade rather than materializing one. - drawer_collection = _DryRunCollectionProxy() + drawer_collection = _DryRunCollectionProxy() knowledge_graph = _DryRunKnowledgeGraphProxy() else: drawer_collection = get_collection(palace_path) diff --git a/tests/test_cli_source_adapters.py b/tests/test_cli_source_adapters.py index b5c3a4e..9b78a01 100644 --- a/tests/test_cli_source_adapters.py +++ b/tests/test_cli_source_adapters.py @@ -197,11 +197,27 @@ def test_cmd_mine_source_rejects_unknown_adapter(capsys): assert "unknown source adapter 'not-installed'" in capsys.readouterr().err +def test_cmd_mine_source_reports_contention_without_traceback(monkeypatch, capsys): + from mempalace.palace import MineAlreadyRunning + + def raise_contention(**_kwargs): + raise MineAlreadyRunning("palace is held by pid=123") + + monkeypatch.setattr(cli, "mine_source_adapter", raise_contention) + + with pytest.raises(SystemExit) as excinfo: + cli.cmd_mine(_mine_args(source="fixture")) + + assert excinfo.value.code == 1 + assert capsys.readouterr().err == "mempalace: palace is held by pid=123\n" + + def test_mine_source_dry_run_prevents_direct_collection_and_kg_mutations(monkeypatch): from mempalace import knowledge_graph, palace register("direct-mutation", _DirectMutationAdapter) monkeypatch.setattr(cli, "MempalaceConfig", _FakeConfig) + def read_only_collection(_palace_path, *, create): assert create is False raise FileNotFoundError @@ -228,26 +244,41 @@ def test_mine_source_dry_run_prevents_direct_collection_and_kg_mutations(monkeyp ] -def test_mine_source_dry_run_reads_existing_collection_without_writing(monkeypatch): +def test_mine_source_dry_run_never_opens_existing_collection(monkeypatch): from mempalace import knowledge_graph, palace - collection = _FakeCollection() - collection.count = lambda: 7 register("read-aware", _ReadAwareAdapter) monkeypatch.setattr(cli, "MempalaceConfig", _FakeConfig) monkeypatch.setattr( - palace, - "get_collection", - lambda palace_path, *, create: collection if create is False else pytest.fail("must not create"), + palace, "get_collection", lambda *_args, **_kwargs: pytest.fail("must not open") ) monkeypatch.setattr(knowledge_graph, "KnowledgeGraph", _FakeKnowledgeGraph) - assert cli.mine_source_adapter( - source_name="read-aware", source_path="/source", palace_path="/fake/palace", dry_run=True - ) == 1 + assert ( + cli.mine_source_adapter( + source_name="read-aware", + source_path="/source", + palace_path="/fake/palace", + dry_run=True, + ) + == 1 + ) - assert _ReadAwareAdapter.observed_count == 7 - assert collection.upserts == [] + assert _ReadAwareAdapter.observed_count == 0 + + +def test_dry_run_collection_proxy_returns_backend_result_types(): + from mempalace.backends import GetResult, QueryResult + + collection = cli._DryRunCollectionProxy() + + get_result = collection.get() + query_result = collection.query(query_texts=["one", "two"], include=["embeddings"]) + + assert isinstance(get_result, GetResult) + assert isinstance(query_result, QueryResult) + assert query_result.ids == [[], []] + assert query_result.embeddings == [[], []] def test_mine_source_dry_run_fresh_palace_creates_no_backend_artifacts(tmp_path): @@ -255,12 +286,15 @@ def test_mine_source_dry_run_fresh_palace_creates_no_backend_artifacts(tmp_path) register("fixture", _FixtureAdapter) palace = tmp_path / "fresh-palace" - assert cli.mine_source_adapter( - source_name="fixture", - source_path="/source", - palace_path=str(palace), - dry_run=True, - ) == 1 + assert ( + cli.mine_source_adapter( + source_name="fixture", + source_path="/source", + palace_path=str(palace), + dry_run=True, + ) + == 1 + ) assert not palace.exists(), "dry run must not create backend storage" @@ -321,12 +355,17 @@ def test_mine_source_holds_writer_lease_before_opening_handles(monkeypatch): monkeypatch.setattr(palace, "get_collection", get_collection) monkeypatch.setattr(knowledge_graph, "KnowledgeGraph", _FakeKnowledgeGraph) - assert cli.mine_source_adapter( - source_name="fixture", source_path="/source", palace_path="/fake/palace" - ) == 1 + assert ( + cli.mine_source_adapter( + source_name="fixture", source_path="/source", palace_path="/fake/palace" + ) + == 1 + ) -@pytest.mark.skipif(sys.platform == "win32", reason="cross-process lock semantics differ on Windows") +@pytest.mark.skipif( + sys.platform == "win32", reason="cross-process lock semantics differ on Windows" +) def test_mine_source_refuses_held_writer_lease_before_opening_handles(tmp_path, monkeypatch): """A competing writer prevents adapter ingest and all handle creation.""" from mempalace import knowledge_graph, palace