diff --git a/graphify/detect.py b/graphify/detect.py index 59ae39b22..4a45b8c56 100644 --- a/graphify/detect.py +++ b/graphify/detect.py @@ -2448,6 +2448,63 @@ def load_manifest( ) +def _checkout_prefix(abs_key: str, relative_keys: set[str]) -> str | None: + """Longest directory prefix of ``abs_key`` whose suffix is in ``relative_keys``. + + The suffix is forward-slash and NFC. ``src/c.py`` beats a shorter ``c.py``. + Prefixes ``""``, ``"/"``, and ``"\\"`` are skipped so a shorter suffix can + still match. The prefix that remains must be a real directory. + """ + parts = _nfc(abs_key).replace("\\", "/").split("/") + best: tuple[int, str] | None = None + for i in range(1, len(parts)): + suffix = "/".join(parts[i:]) + if suffix not in relative_keys: + continue + prefix = "/".join(parts[:i]) + if prefix in ("", "/", "\\"): + continue + if not Path(prefix).is_dir(): + continue + if best is None or len(suffix) > best[0]: + best = (len(suffix), prefix) + return None if best is None else best[1] + + +def _previous_checkout_absolute_keys( + existing_keys: Iterable[Any], + storage_root: Path | None, + relative_keys: set[str], +) -> set[str]: + """Absolute keys that still name a previous checkout (#3581). + + A key is a candidate only when :func:`_to_relative_for_storage` keeps it + absolute against ``storage_root`` (a sibling row that becomes relative, + including #3785, is not a candidate). Candidates that share one directory + prefix are a previous checkout when that group has at least two keys. One + absolute key is an include row and stays. ``storage_root is None`` yields + an empty set. + """ + if storage_root is None: + return set() + groups: dict[str, set[str]] = {} + for key in existing_keys: + if not isinstance(key, str) or not _looks_absolute(key): + continue + stored = _nfc(_to_relative_for_storage(key, storage_root)) + if not _looks_absolute(stored): + continue + prefix = _checkout_prefix(stored, relative_keys) + if prefix is None: + continue + groups.setdefault(prefix, set()).add(key) + stale: set[str] = set() + for keys in groups.values(): + if len(keys) >= 2: + stale.update(keys) + return stale + + def save_manifest( files: dict[str, list[str]], manifest_path: str = _MANIFEST_PATH, @@ -2480,9 +2537,12 @@ def save_manifest( forever and masquerading as deletions in detect_incremental. It must be the RAW detect output, not a stamp-filtered subset — pruning to a filtered set would erase rows the filter merely omitted (failed chunks, - --code-only doc rows). Out-of-root entries are never pruned. Callers - saving a SUBSET of files (changed_paths hooks, skill runbooks, #917) - must leave this None so their untouched rows are preserved. + --code-only doc rows). When this argument is set, two or more absolute + keys that share a directory prefix and match relative keys this save writes + are a previous checkout and are dropped (#3581). A single include row stays. + Callers saving a SUBSET of files (changed_paths hooks, skill runbooks, #917) + must leave this None so their untouched rows, including out-of-root rows, + are preserved. ``clear_semantic`` (#1948): files that were dispatched this run but produced no stamped output (e.g. the LLM omitted their chunk on a @@ -2583,6 +2643,21 @@ def save_manifest( # caller supplied the full scan corpus, additionally prune in-root rows # the scan no longer covers: those files were excluded, not deleted, and # keeping the row makes them look deleted on every future run (#1908). + all_files = [f for file_list in files.values() for f in file_list] + storage_root = _manifest_storage_anchor(manifest_path, root) if root is not None else None + # A subset save omits scan_corpus and must keep every live row it was not + # given (#917), including an include row and a previous checkout. The + # checkout drop runs only on a full scan. + stale_checkout: set[str] = set() + if scan_set is not None and storage_root is not None: + new_rels: set[str] = set() + for f in all_files: + stored = _nfc(_to_relative_for_storage(_nfc(f), storage_root)) + if not _looks_absolute(stored): + new_rels.add(stored) + stale_checkout = _previous_checkout_absolute_keys( + existing.keys(), storage_root, new_rels + ) manifest: dict[str, dict] = {} for f, entry in existing.items(): normalised = _normalise_entry(entry) @@ -2593,6 +2668,8 @@ def save_manifest( continue except OSError: continue + if f in stale_checkout: + continue # previous checkout copied with graphify-out (#3581) if scan_set is not None and not _in_scan(f) and _in_root(f): continue # excluded-but-alive: drop the stale row (#1908) if clear_ast_set is not None and _in_clear_ast(f): @@ -2605,7 +2682,6 @@ def save_manifest( normalised = {**normalised, "semantic_hash": ""} manifest[f] = normalised - all_files = [f for file_list in files.values() for f in file_list] with ThreadPoolExecutor() as pool: raw = pool.map(_stat_and_hash, all_files) hashed: dict[str, tuple[float, str]] = { @@ -2646,7 +2722,6 @@ def save_manifest( "semantic_hash": sem_h, } manifest[key] = entry - storage_root = _manifest_storage_anchor(manifest_path, root) if root is not None else None if storage_root is not None: manifest = _collapse_manifest_duplicates( manifest.items(), lambda k: _nfc(_to_relative_for_storage(k, storage_root)) diff --git a/tests/test_detect.py b/tests/test_detect.py index 0994738f7..44befbe04 100644 --- a/tests/test_detect.py +++ b/tests/test_detect.py @@ -3589,6 +3589,237 @@ def test_save_manifest_full_scan_keeps_out_of_root_rows(tmp_path): outside.unlink(missing_ok=True) +def _write_checkout_files(checkout: Path) -> list[Path]: + """Three files under one checkout: a.py, b.py, and src/c.py (#3581).""" + files = [checkout / "a.py", checkout / "b.py", checkout / "src" / "c.py"] + (checkout / "src").mkdir(parents=True) + files[0].write_text("a = 1\n", encoding="utf-8") + files[1].write_text("b = 2\n", encoding="utf-8") + files[2].write_text("c = 3\n", encoding="utf-8") + return files + + +def _save_checkout_manifest(checkout: Path, files: list[Path], *, root, scan_corpus): + manifest_path = str(checkout / "graphify-out" / "manifest.json") + save_manifest( + {"code": [str(p) for p in files]}, + manifest_path, + kind="both", + root=root, + scan_corpus=scan_corpus, + ) + return manifest_path + + +def test_save_manifest_reclone_drops_other_checkout_when_scan_corpus_is_set(tmp_path): + """A copied graphify-out must not keep the old checkout's absolute keys + when the new save passes the full scan corpus (#3581).""" + import json + import shutil + checkout_a = tmp_path / "checkout-a" + checkout_b = tmp_path / "checkout-b" + files_a = _write_checkout_files(checkout_a) + _save_checkout_manifest(checkout_a, files_a, root=None, scan_corpus=None) + shutil.copytree(checkout_a, checkout_b) + files_b = [ + checkout_b / "a.py", checkout_b / "b.py", checkout_b / "src" / "c.py", + ] + manifest_b = _save_checkout_manifest( + checkout_b, files_b, root=checkout_b, + scan_corpus={str(p) for p in files_b}, + ) + raw = json.loads(Path(manifest_b).read_text(encoding="utf-8")) + assert set(raw) == {"a.py", "b.py", "src/c.py"} + assert not any(k.startswith(str(checkout_a)) for k in raw) + + +def test_save_manifest_subset_save_keeps_other_checkout_absolute_keys(tmp_path): + """Omitting scan_corpus is a subset save (#917). It must keep absolute + keys that point at the other checkout.""" + import json + import shutil + checkout_a = tmp_path / "checkout-a" + checkout_b = tmp_path / "checkout-b" + files_a = _write_checkout_files(checkout_a) + _save_checkout_manifest(checkout_a, files_a, root=None, scan_corpus=None) + shutil.copytree(checkout_a, checkout_b) + files_b = [ + checkout_b / "a.py", checkout_b / "b.py", checkout_b / "src" / "c.py", + ] + manifest_b = _save_checkout_manifest( + checkout_b, files_b, root=checkout_b, scan_corpus=None, + ) + raw = json.loads(Path(manifest_b).read_text(encoding="utf-8")) + assert {"a.py", "b.py", "src/c.py"} <= set(raw) + old = [k for k in raw if k.startswith(str(checkout_a))] + assert len(old) == 3, f"a subset save must keep the other checkout, got {set(raw)}" + + +def test_save_manifest_subset_save_keeps_includes_that_share_basenames(tmp_path): + """A subset save must keep two include rows whose file names match the + files it restamps.""" + import json + import shutil + a = tmp_path / "a.py" + b = tmp_path / "b.py" + a.write_text("x = 1\n", encoding="utf-8") + b.write_text("y = 2\n", encoding="utf-8") + outside_dir = tmp_path.parent / f"{tmp_path.name}-includes" + outside_dir.mkdir() + outside_a = outside_dir / "a.py" + outside_b = outside_dir / "b.py" + outside_a.write_text("z = 3\n", encoding="utf-8") + outside_b.write_text("w = 4\n", encoding="utf-8") + try: + manifest_path = str(tmp_path / "graphify-out" / "manifest.json") + save_manifest( + {"code": [str(a), str(b), str(outside_a), str(outside_b)]}, + manifest_path, root=tmp_path, + ) + save_manifest( + {"code": [str(a), str(b)]}, manifest_path, root=tmp_path, + ) + raw = json.loads(Path(manifest_path).read_text(encoding="utf-8")) + assert {"a.py", "b.py"} <= set(raw) + assert str(outside_a.resolve()) in raw, set(raw) + assert str(outside_b.resolve()) in raw, set(raw) + finally: + shutil.rmtree(outside_dir, ignore_errors=True) + + +def test_save_manifest_move_has_no_absolute_key_when_scan_corpus_is_set(tmp_path): + """Deleting the old checkout before the save already yields relative keys + when scan_corpus is set (#3581).""" + import json + import shutil + checkout_a = tmp_path / "checkout-a" + checkout_b = tmp_path / "checkout-b" + files_a = _write_checkout_files(checkout_a) + _save_checkout_manifest(checkout_a, files_a, root=None, scan_corpus=None) + shutil.copytree(checkout_a, checkout_b) + shutil.rmtree(checkout_a) + files_b = [ + checkout_b / "a.py", checkout_b / "b.py", checkout_b / "src" / "c.py", + ] + manifest_b = _save_checkout_manifest( + checkout_b, files_b, root=checkout_b, + scan_corpus={str(p) for p in files_b}, + ) + raw = json.loads(Path(manifest_b).read_text(encoding="utf-8")) + assert set(raw) == {"a.py", "b.py", "src/c.py"} + assert not any(k.startswith(str(checkout_a)) for k in raw) + + +def test_save_manifest_move_has_no_absolute_key_when_scan_corpus_is_unset(tmp_path): + """Deleting the old checkout before the save already yields relative keys + when scan_corpus is omitted (#3581).""" + import json + import shutil + checkout_a = tmp_path / "checkout-a" + checkout_b = tmp_path / "checkout-b" + files_a = _write_checkout_files(checkout_a) + _save_checkout_manifest(checkout_a, files_a, root=None, scan_corpus=None) + shutil.copytree(checkout_a, checkout_b) + shutil.rmtree(checkout_a) + files_b = [ + checkout_b / "a.py", checkout_b / "b.py", checkout_b / "src" / "c.py", + ] + manifest_b = _save_checkout_manifest( + checkout_b, files_b, root=checkout_b, scan_corpus=None, + ) + raw = json.loads(Path(manifest_b).read_text(encoding="utf-8")) + assert set(raw) == {"a.py", "b.py", "src/c.py"} + assert not any(k.startswith(str(checkout_a)) for k in raw) + + +def test_save_manifest_drops_other_checkout_keys_mixed_with_relative_keys(tmp_path): + """A manifest that already holds the new relative keys plus the old + absolute keys drops the old checkout on the next save (#3581).""" + import json + import shutil + checkout_a = tmp_path / "checkout-a" + checkout_b = tmp_path / "checkout-b" + files_a = _write_checkout_files(checkout_a) + manifest_a = _save_checkout_manifest(checkout_a, files_a, root=None, scan_corpus=None) + shutil.copytree(checkout_a, checkout_b) + files_b = [ + checkout_b / "a.py", checkout_b / "b.py", checkout_b / "src" / "c.py", + ] + manifest_b = _save_checkout_manifest( + checkout_b, files_b, root=checkout_b, + scan_corpus={str(p) for p in files_b}, + ) + mixed = json.loads(Path(manifest_b).read_text(encoding="utf-8")) + stale = json.loads(Path(manifest_a).read_text(encoding="utf-8")) + mixed.update(stale) + Path(manifest_b).write_text(json.dumps(mixed), encoding="utf-8") + manifest_b = _save_checkout_manifest( + checkout_b, files_b, root=checkout_b, + scan_corpus={str(p) for p in files_b}, + ) + raw = json.loads(Path(manifest_b).read_text(encoding="utf-8")) + assert set(raw) == {"a.py", "b.py", "src/c.py"} + assert not any(k.startswith(str(checkout_a)) for k in raw) + + +def test_save_manifest_keeps_one_include_that_shares_a_basename(tmp_path): + """One include row stays even when its file name matches a project file.""" + import json + import shutil + a = tmp_path / "a.py" + a.write_text("x = 1\n", encoding="utf-8") + outside_dir = tmp_path.parent / f"{tmp_path.name}-include-a" + outside_dir.mkdir() + outside = outside_dir / "a.py" + outside.write_text("z = 3\n", encoding="utf-8") + try: + manifest_path = str(tmp_path / "graphify-out" / "manifest.json") + save_manifest( + {"code": [str(a), str(outside)]}, manifest_path, root=tmp_path, + ) + save_manifest( + {"code": [str(a)]}, manifest_path, root=tmp_path, + scan_corpus={str(a)}, + ) + raw = json.loads(Path(manifest_path).read_text(encoding="utf-8")) + assert "a.py" in raw + assert str(outside.resolve()) in raw, ( + f"a single include row must stay, got {set(raw)}" + ) + finally: + shutil.rmtree(outside_dir, ignore_errors=True) + + +def test_save_manifest_keeps_includes_whose_paths_are_not_this_tree(tmp_path): + """Two include rows stay when their suffixes are not relative keys this save writes.""" + import json + import shutil + a = tmp_path / "a.py" + a.write_text("x = 1\n", encoding="utf-8") + outside_dir = tmp_path.parent / f"{tmp_path.name}-includes" + outside_dir.mkdir() + notes = outside_dir / "notes.md" + extra = outside_dir / "extra.md" + notes.write_text("notes\n", encoding="utf-8") + extra.write_text("extra\n", encoding="utf-8") + try: + manifest_path = str(tmp_path / "graphify-out" / "manifest.json") + save_manifest( + {"code": [str(a), str(notes), str(extra)]}, + manifest_path, root=tmp_path, + ) + save_manifest( + {"code": [str(a)]}, manifest_path, root=tmp_path, + scan_corpus={str(a)}, + ) + raw = json.loads(Path(manifest_path).read_text(encoding="utf-8")) + assert "a.py" in raw + assert str(notes.resolve()) in raw + assert str(extra.resolve()) in raw + finally: + shutil.rmtree(outside_dir, ignore_errors=True) + + def test_detect_incremental_reports_excluded_not_deleted(tmp_path): """A previously-indexed file that becomes excluded (still on disk) must land in excluded_files, not deleted_files (#1908)."""