fix(mine): make source adapter dry runs inert
This commit is contained in:
parent
141e16fb2b
commit
f414bb881f
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue