From 5cebe6608a1e468e90c1e2429cf1381c71ebc4a8 Mon Sep 17 00:00:00 2001 From: Sandro da Silva <55045047+fatkobra@users.noreply.github.com> Date: Sun, 2 Aug 2026 17:12:25 +0000 Subject: [PATCH] fix(convos): honor mined state during dry runs --- mempalace/convo_miner.py | 35 ++++++++++-- tests/test_convo_miner.py | 116 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 4 deletions(-) diff --git a/mempalace/convo_miner.py b/mempalace/convo_miner.py index b6b714f986..aaeee5b3ae 100644 --- a/mempalace/convo_miner.py +++ b/mempalace/convo_miner.py @@ -19,6 +19,7 @@ from collections import defaultdict from typing import Optional +from .backends import PalaceNotFoundError from .collision_scan import assert_no_collisions from .ids import ID_RECIPE, make_convo_drawer_id, make_convo_sentinel_id from .normalize import normalize_conversations @@ -830,6 +831,27 @@ def _normalize_convo_conversations( return conversations +def _open_convo_collection( + palace_path: str, + *, + dry_run: bool, +): + """Open the conversation collection without creating it during dry-run.""" + if not dry_run: + return get_collection(palace_path) + + try: + return get_collection( + palace_path, + create=False, + read_only=True, + ) + except PalaceNotFoundError: + # A missing palace or uninitialized collection represents empty + # prior state to a dry-run. Do not create either one. + return None + + def _mine_convos_impl( convo_dir: str, palace_path: str, @@ -871,7 +893,10 @@ def _mine_convos_impl( print(" DRY RUN — nothing will be filed") print(f"{'-' * 55}\n") - collection = get_collection(palace_path) if not dry_run else None + collection = _open_convo_collection( + palace_path, + dry_run=dry_run, + ) # Bulk pre-fetch already-mined source_file -> stored mtime in one # paginated pass instead of `len(files)` separate WHERE-source_file @@ -880,7 +905,7 @@ def _mine_convos_impl( # prefetch_mined_set() does the same decisions in a single scan; loop # body becomes an O(1) dict lookup + a cheap local mtime comparison. mined_mtimes: dict = ( - prefetch_mined_set(collection, extract_mode=extract_mode) if not dry_run else {} + prefetch_mined_set(collection, extract_mode=extract_mode) if collection is not None else {} ) # content_hash -> source_file for transcripts already filed. Repeated # exports from Claude/ChatGPT commonly land under a new filename each @@ -888,7 +913,9 @@ def _mine_convos_impl( # source_file-keyed skip above ("mined_mtimes") never recognizes them — # this catches the same conversation reappearing at a new path. mined_content_hashes: dict = ( - prefetch_content_hashes(collection, extract_mode=extract_mode) if not dry_run else {} + prefetch_content_hashes(collection, extract_mode=extract_mode) + if collection is not None + else {} ) total_drawers = 0 @@ -909,7 +936,7 @@ def _mine_convos_impl( # Falling through re-mines: _file_chunks_locked purges this # source_file's stale drawers before inserting fresh ones, so this # never leaves duplicates behind. - if not dry_run and _is_unchanged_since_last_mine(source_file, mined_mtimes): + if _is_unchanged_since_last_mine(source_file, mined_mtimes): files_skipped += 1 continue diff --git a/tests/test_convo_miner.py b/tests/test_convo_miner.py index 634e1d1ae3..ad6924ca65 100644 --- a/tests/test_convo_miner.py +++ b/tests/test_convo_miner.py @@ -834,3 +834,119 @@ def test_register_file_sentinel_includes_source_mtime(): assert abs(mined[str(tiny_file)] - os.path.getmtime(tiny_file)) < 0.001 finally: shutil.rmtree(tmpdir, ignore_errors=True) + + +def _write_dry_run_transcript(path: Path) -> None: + path.write_text( + "> What is the plan?\n" + "Start with the schema, then the API.\n\n" + "> Are there any risks?\n" + "Migration ordering is the main one.\n\n" + "> What comes next?\n" + "Run focused tests before the full suite.\n", + encoding="utf-8", + ) + + +def test_mine_convos_dry_run_skips_unchanged_mined_file( + tmp_path, + capsys, + monkeypatch, +): + monkeypatch.setenv("HOME", str(tmp_path)) + + convo_dir = tmp_path / "convos" + convo_dir.mkdir() + transcript = convo_dir / "session.txt" + _write_dry_run_transcript(transcript) + palace_path = str(tmp_path / "palace") + + mine_convos( + str(convo_dir), + palace_path, + wing="original", + ) + capsys.readouterr() + + mine_convos( + str(convo_dir), + palace_path, + wing="target", + dry_run=True, + ) + output = capsys.readouterr().out + + assert "[DRY RUN] session.txt" not in output + assert "Files processed: 0" in output + assert "Files skipped (already filed): 1" in output + assert "Drawers filed: 0" in output + + +def test_mine_convos_dry_run_keeps_modified_file_as_work( + tmp_path, + capsys, + monkeypatch, +): + monkeypatch.setenv("HOME", str(tmp_path)) + + convo_dir = tmp_path / "convos" + convo_dir.mkdir() + transcript = convo_dir / "session.txt" + _write_dry_run_transcript(transcript) + palace_path = str(tmp_path / "palace") + + mine_convos( + str(convo_dir), + palace_path, + wing="original", + ) + capsys.readouterr() + + transcript.write_text( + transcript.read_text(encoding="utf-8") + + "\n> Did the plan change?\n" + + "Yes, add a migration rollback test.\n", + encoding="utf-8", + ) + + future = time.time() + 60 + os.utime(transcript, (future, future)) + + mine_convos( + str(convo_dir), + palace_path, + wing="target", + dry_run=True, + ) + output = capsys.readouterr().out + + assert "[DRY RUN] session.txt" in output + assert "Files processed: 1" in output + assert "Files skipped (already filed): 0" in output + + +def test_mine_convos_dry_run_missing_palace_does_not_create_it( + tmp_path, + capsys, + monkeypatch, +): + monkeypatch.setenv("HOME", str(tmp_path)) + + convo_dir = tmp_path / "convos" + convo_dir.mkdir() + transcript = convo_dir / "session.txt" + _write_dry_run_transcript(transcript) + palace_path = tmp_path / "palace" + + mine_convos( + str(convo_dir), + str(palace_path), + wing="target", + dry_run=True, + ) + output = capsys.readouterr().out + + assert "[DRY RUN] session.txt" in output + assert "Files processed: 1" in output + assert "Files skipped (already filed): 0" in output + assert not palace_path.exists()