diff --git a/CHANGELOG.md b/CHANGELOG.md index e03bf3ebfa..7b1254be96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -323,6 +323,7 @@ Intra-class call binding across five languages, new language-structure coverage, ## 0.9.63 (2026-09-16) +- Fix: `save_manifest` no longer erases a deleted file's manifest row before `detect_incremental` gets to report it. The row was pruned unconditionally whenever the file no longer existed on disk, which could erase it before a caller that saves without also pruning the graph (a scan-only run, an interrupted pipeline) ever acted on the report — making a genuine deletion permanently unreportable from the very next run onward. A full-scan caller (passing the complete scan corpus) still cleans the row up, now correctly sequenced after the deletion had a chance to be reported on that same scan; only a partial save now leaves a dead row in place, reconciled by the next full scan. That reconciliation on a full scan now also covers an out-of-root row whose file is genuinely gone, which the in-root-only check left permanently unprunable (#3426, thanks @John-kibe). - Feature: Elixir `alias`/`import`/`require`/`use` targets now resolve onto the module's `defmodule` node across files, so the internal module dependency graph is no longer dropped as dangling. Only top-level modules are indexed (a nested `defmodule`, labeled with its bare inner name, cannot capture an unrelated `use ` from another file), and a same-file reference is left unresolved so it cannot clobber the structural `contains` edge (#3603, thanks @ayushcodes10). - Feature: a Rust `self.method()` call now resolves to a method defined on the same type in another file (the common split-`impl`-block layout), pooling methods across every `impl` of one type and refusing to link when two unrelated types share a bare name (#3602, thanks @ayushcodes10). - Feature: a Ruby member call `obj.foo` on a known-type receiver now resolves to a method `foo` inherited from a superclass, including across files, using the same conservative promotion as the implicit-self resolver — a single owning class, matching method kind, and one unambiguous ancestry chain, or it stays dangling (#3585, thanks @oleksii-tumanov). diff --git a/graphify/detect.py b/graphify/detect.py index 73b573a722..5e945a13f8 100644 --- a/graphify/detect.py +++ b/graphify/detect.py @@ -2500,12 +2500,15 @@ 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). 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. + --code-only doc rows). Out-of-root entries are never pruned merely for + being outside the current scan (they were never walked by detect, so + their absence from the corpus is not exclusion evidence) — only when + the file no longer exists on disk at all (#3426). 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 @@ -2601,16 +2604,44 @@ def _normalise_entry(entry): # Seed from the existing manifest so incremental callers passing a subset # of files don't silently erase entries for untouched files (#917). - # Prune entries whose file no longer exists on disk — those are genuine - # deletions that detect_incremental() should treat as gone. When the - # 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). + # + # A row for a file no longer on disk is NOT pruned here (#3426): that was + # this function's own doing until this fix, on the theory that a + # genuinely deleted file's row is dead weight. But detect_incremental() + # is what REPORTS a deletion to callers via deleted_files, and it runs + # before this function is asked to save again — pruning the row here, + # unconditionally, could erase it before a caller that saves without + # also pruning the graph (a scan-only run, an interrupted pipeline) ever + # gets to act on the report, making that genuine deletion permanently + # unreportable from the very next run onward. When the caller supplied + # the full scan corpus, the check below already prunes an in-root row + # the scan no longer covers — which a deleted file always satisfies, on + # path alone, regardless of whether it still exists — so a full-scan + # caller still cleans the row up, just sequenced after + # detect_incremental() has had the chance to report it on that same + # scan. Only a partial/subset caller (no scan_corpus) now leaves a dead + # row in place; the next full scan reconciles it. + # + # A review finding pointed out that _in_root(f) alone leaves an + # out-of-root row (a merged/foreign corpus entry, or a root that could + # not be resolved) permanently unpruned even on a full scan, since + # _in_root() fails open for exactly those paths. A full scan is still + # the same safe reconciliation point detect_incremental() already had + # its chance to report against this run, so an out-of-root row whose + # file no longer exists on disk is pruned here too — never on a + # partial/subset save, matching the in-root case above. + # + # A previous checkout's manifest, copied along with graphify-out/ into a + # new checkout location, leaves behind absolute-path rows that duplicate + # the relative rows this save now writes for the same files (#3581). + # Those are not a deletion-reporting concern at all — the file is right + # there, just double-counted under its old absolute key — so dropping + # them is safe unconditionally, independent of the deletion sequencing + # above, and only runs on a full scan since a subset save never has + # enough of the new corpus to tell a duplicate from a genuinely + # untouched out-of-root row (#917). 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() @@ -2626,15 +2657,19 @@ def _normalise_entry(entry): normalised = _normalise_entry(entry) if normalised is None: continue - try: - if not Path(f).exists(): - 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 scan_set is not None and not _in_scan(f): + if _in_root(f): + continue # excluded-or-deleted, not in this scan: drop the stale row (#1908) + try: + out_of_root_gone = not Path(f).exists() + except OSError: + # Cannot tell: fail open and keep the row, matching _in_root's + # own "cannot tell in-root from out-of-root" fail-open rule. + out_of_root_gone = False + if out_of_root_gone: + continue # out-of-root deletion: safe to prune on a full scan if clear_ast_set is not None and _in_clear_ast(f): # AST failure this run (missing extra / zero nodes, #2543): blank # both hashes so either detect_incremental kind re-queues. diff --git a/tests/test_detect.py b/tests/test_detect.py index 8aa09ae21f..98037ba6db 100644 --- a/tests/test_detect.py +++ b/tests/test_detect.py @@ -3733,6 +3733,41 @@ def test_save_manifest_subset_save_preserves_untouched_rows(tmp_path): ) +def test_save_manifest_subset_save_keeps_a_deleted_files_row(tmp_path): + """#3426: without scan_corpus, a row for a file no longer on disk must + survive a save too -- not just an untouched one. detect_incremental() + is what REPORTS a deletion via deleted_files, and a save that runs + without also pruning the graph (a scan-only run, an interrupted + pipeline) must not erase the row before any caller gets a chance to + act on that report, or the deletion becomes unreportable from the very + next run onward.""" + import json + from graphify.detect import detect_incremental + + a = tmp_path / "a.py" + gone = tmp_path / "gone.py" + a.write_text("x = 1\n") + gone.write_text("y = 2\n") + manifest_path = str(tmp_path / "graphify-out" / "manifest.json") + save_manifest({"code": [str(a), str(gone)]}, manifest_path, root=tmp_path) + + gone.unlink() + detection = detect_incremental(tmp_path, manifest_path=manifest_path) + assert str(gone) in detection["deleted_files"], "must be reported deleted on this pass" + + # A save that is NOT given the full scan corpus (no scan_corpus) -- + # the row must not disappear before the report above was acted on. + save_manifest({"code": [str(a)]}, manifest_path, root=tmp_path) + raw = json.loads(Path(manifest_path).read_text(encoding="utf-8")) + assert "gone.py" in raw, ( + f"a deleted file's row must survive a subset save so it stays reportable, got {set(raw)}" + ) + + detection2 = detect_incremental(tmp_path, manifest_path=manifest_path) + assert str(gone) in detection2["deleted_files"], \ + "the deletion must still be reportable on the next pass" + + def test_save_manifest_full_scan_keeps_out_of_root_rows(tmp_path): """Out-of-root entries (--include sources, symlinked corpora) are never walked by detect, so their absence from the corpus is not exclusion @@ -3760,6 +3795,65 @@ def test_save_manifest_full_scan_keeps_out_of_root_rows(tmp_path): outside.unlink(missing_ok=True) +def test_save_manifest_full_scan_prunes_genuinely_deleted_out_of_root_row(tmp_path): + """Review finding on #3426: an out-of-root row must never be pruned + merely for being outside the current scan (the test above), but that is + different from the file actually being gone from disk. #3426 removed the + old unconditional exists() check that used to prune ANY missing file's + row regardless of root, and preserved reconciliation only for in-root + rows via the scan-exclusion check -- silently leaving a genuinely + deleted out-of-root row unprunable forever, even across full scans. A + full scan is still detect_incremental()'s chance to report the + deletion first, so it is the same safe reconciliation point as the + in-root case.""" + import json + a = tmp_path / "a.py" + a.write_text("x = 1\n") + outside = tmp_path.parent / f"{tmp_path.name}-extern-gone.py" + outside.write_text("z = 3\n") + try: + manifest_path = str(tmp_path / "graphify-out" / "manifest.json") + save_manifest( + {"code": [str(a), str(outside)]}, manifest_path, root=tmp_path + ) + outside.unlink() + 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()) not in raw, ( + f"a genuinely deleted out-of-root row must be pruned on a full scan, got {set(raw)}" + ) + finally: + outside.unlink(missing_ok=True) + + +def test_save_manifest_subset_save_keeps_out_of_root_deleted_row(tmp_path): + """The fix above must stay scoped to full-scan saves only -- a partial + save (no scan_corpus) must still preserve a deleted out-of-root row, the + same as it already does for an in-root one (#3426).""" + import json + a = tmp_path / "a.py" + a.write_text("x = 1\n") + outside = tmp_path.parent / f"{tmp_path.name}-extern-gone2.py" + outside.write_text("z = 3\n") + try: + manifest_path = str(tmp_path / "graphify-out" / "manifest.json") + save_manifest( + {"code": [str(a), str(outside)]}, manifest_path, root=tmp_path + ) + outside.unlink() + save_manifest({"code": [str(a)]}, manifest_path, root=tmp_path) + raw = json.loads(Path(manifest_path).read_text(encoding="utf-8")) + assert str(outside.resolve()) in raw, ( + f"a subset save must not prune a deleted out-of-root row, got {set(raw)}" + ) + finally: + 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"] @@ -3881,9 +3975,19 @@ def test_save_manifest_move_has_no_absolute_key_when_scan_corpus_is_set(tmp_path 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).""" +def test_save_manifest_subset_save_keeps_moved_checkout_keys_even_when_old_root_is_gone(tmp_path): + """#3426 superseded the unconditional ``exists()`` prune this test + originally relied on: a subset save (no ``scan_corpus``) now preserves + every untouched row regardless of disk existence, the same promise #917 + already makes for an ordinary out-of-root include row, since pruning on + a subset save can erase a genuine deletion before detect_incremental() + ever gets to report it (there is no scan corpus here to reconcile + against the way a full scan does). The old checkout's absolute keys are + no exception just because their directory happens to be gone too; the + stale-checkout dedup (#3581) that WOULD drop them needs the full scan + corpus to tell a duplicate from an untouched row, so it does not run + here either. A full scan is what actually reconciles this, exactly like + the sibling test above.""" import json import shutil checkout_a = tmp_path / "checkout-a" @@ -3899,8 +4003,9 @@ def test_save_manifest_move_has_no_absolute_key_when_scan_corpus_is_unset(tmp_pa 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) + 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_drops_other_checkout_keys_mixed_with_relative_keys(tmp_path):