mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-14 07:59:40 +00:00
fix(detect): drop a previous checkout's absolute manifest keys (#4175)
A re-clone that copies graphify-out/ while the old checkout is still on disk left absolute manifest keys pointing at the old path, which survived the exists()/in-root prune and persisted alongside the new relative keys. On a full scan, drop stale absolute keys that duplicate a freshly-written relative key under a real shared directory prefix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
1 parent
1292639344
commit
00342989e4
2 files changed
+311
-5
No files matched your search
+80
-5
@@ -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))
|
||||
|
||||
@@ -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)."""
|
||||
|
||||
Reference in new issue
Block a user