mirror of
https://github.com/safishamsi/graphify.git
synced 2026-09-13 00:55:53 +00:00
fix(detect,cli): excluded files are pruned from graph and manifest, not misreported as deleted (#1908 #1909)
Two coupled excluded-vs-deleted fixes: #1909 — incremental extract's prune set was derived from the manifest alone (manifest - corpus), so a file that became excluded without ever being manifest-listed (every pre-#1897 graph) kept its stale nodes in graph.json forever. The prune set is now also derived from the existing graph's own node source_files reconciled against the post-exclude detect corpus (_stale_graph_sources), restricted to in-root paths; out-of-root --include/symlinked entries and remote (://) sources are never pruned. Relative source_files are anchored against both the scan root and the --out root (the #555/#1899 relativized form). The --no-cluster incremental early exit never runs build_merge, so an exclusion-only change now prunes the raw graph.json in place instead. #1908 — save_manifest retained any prior row whose file still existed on disk, so an excluded-but-alive file survived as a permanent phantom that detect_incremental reported as deleted on every run. Full-scan callers (extract's saves, watch._rebuild_code's saves) now pass the RAW detect corpus via a new scan_corpus parameter and in-root rows outside it are dropped; the corpus is deliberately not the #933 stamp-filtered files dict, so failed-chunk/omitted-doc rows and --code-only doc rows survive. Subset saves (changed_paths hooks, #917) keep the seeding default. detect_incremental now splits manifest rows that left the scan into deleted_files (gone from disk) and excluded_files (alive but out of scan), mirroring the watch-side #1795 distinction, and the extract summaries report the two separately. Ordering matters: extract's cleanup of newly-excluded nodes previously worked only through the #1908 conflation, so the graph-source prune lands together with the manifest split to avoid regressing #1909.
This commit is contained in:
safishamsi
@@ -1851,3 +1851,162 @@ def test_nested_gitignore_patterns_still_apply_inside_their_dir(tmp_path):
|
||||
result = detect(tmp_path)
|
||||
|
||||
assert result["total_files"] == 2 # main.py + sub/keep.py; sub/noise.log ignored
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# #1908: manifest must not retain scan-excluded files as permanent
|
||||
# "deleted" entries. Full-scan saves prune excluded-but-alive rows; subset
|
||||
# saves keep preserving untouched rows (#917); out-of-root rows never prune.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_save_manifest_full_scan_prunes_excluded_but_alive_row(tmp_path):
|
||||
"""A row for a file that still exists on disk but left the scan corpus
|
||||
(newly excluded) is dropped when the caller passes the full corpus."""
|
||||
import json
|
||||
a = tmp_path / "a.py"
|
||||
b = tmp_path / "b.py"
|
||||
a.write_text("x = 1\n")
|
||||
b.write_text("y = 2\n")
|
||||
manifest_path = str(tmp_path / "graphify-out" / "manifest.json")
|
||||
|
||||
save_manifest({"code": [str(a), str(b)]}, manifest_path, root=tmp_path)
|
||||
raw = json.loads(Path(manifest_path).read_text(encoding="utf-8"))
|
||||
assert set(raw) == {"a.py", "b.py"}
|
||||
|
||||
# Second full scan no longer covers b.py (excluded), yet b.py is alive.
|
||||
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 set(raw) == {"a.py"}, (
|
||||
f"excluded-but-alive row must be pruned on a full-scan save, got {set(raw)}"
|
||||
)
|
||||
|
||||
|
||||
def test_save_manifest_full_scan_still_prunes_missing_file(tmp_path):
|
||||
"""Genuine deletions keep being pruned when scan_corpus is passed."""
|
||||
import json
|
||||
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()
|
||||
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 set(raw) == {"a.py"}
|
||||
|
||||
|
||||
def test_save_manifest_subset_save_preserves_untouched_rows(tmp_path):
|
||||
"""Without scan_corpus (changed_paths hooks, skill runbooks, #917) a
|
||||
subset save must keep seeding rows for files it wasn't given."""
|
||||
import json
|
||||
a = tmp_path / "a.py"
|
||||
b = tmp_path / "b.py"
|
||||
a.write_text("x = 1\n")
|
||||
b.write_text("y = 2\n")
|
||||
manifest_path = str(tmp_path / "graphify-out" / "manifest.json")
|
||||
save_manifest({"code": [str(a), str(b)]}, manifest_path, root=tmp_path)
|
||||
|
||||
# Incremental hook re-stamps only a.py; b.py's row must survive.
|
||||
save_manifest({"code": [str(a)]}, manifest_path, root=tmp_path)
|
||||
raw = json.loads(Path(manifest_path).read_text(encoding="utf-8"))
|
||||
assert set(raw) == {"a.py", "b.py"}, (
|
||||
f"subset saves must preserve untouched rows (#917), got {set(raw)}"
|
||||
)
|
||||
|
||||
|
||||
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
|
||||
evidence — a full-scan save must keep them."""
|
||||
import json
|
||||
a = tmp_path / "a.py"
|
||||
a.write_text("x = 1\n")
|
||||
outside = tmp_path.parent / f"{tmp_path.name}-extern.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
|
||||
)
|
||||
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"out-of-root rows must never be pruned to the scan, got {set(raw)}"
|
||||
)
|
||||
finally:
|
||||
outside.unlink(missing_ok=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)."""
|
||||
a = tmp_path / "a.py"
|
||||
b = tmp_path / "b.py"
|
||||
a.write_text("x = 1\n")
|
||||
b.write_text("y = 2\n")
|
||||
manifest_path = str(tmp_path / "graphify-out" / "manifest.json")
|
||||
full = detect(tmp_path)
|
||||
save_manifest(full["files"], manifest_path, root=tmp_path)
|
||||
|
||||
inc = detect_incremental(
|
||||
tmp_path, manifest_path, extra_excludes=["b.py"]
|
||||
)
|
||||
assert inc["deleted_files"] == [], (
|
||||
f"excluded-but-alive file misreported as deleted: {inc['deleted_files']}"
|
||||
)
|
||||
assert [Path(f).name for f in inc["excluded_files"]] == ["b.py"]
|
||||
|
||||
|
||||
def test_detect_incremental_still_reports_real_deletions(tmp_path):
|
||||
"""Counterpart: a manifest row whose file is gone from disk stays in
|
||||
deleted_files."""
|
||||
a = tmp_path / "a.py"
|
||||
b = tmp_path / "b.py"
|
||||
a.write_text("x = 1\n")
|
||||
b.write_text("y = 2\n")
|
||||
manifest_path = str(tmp_path / "graphify-out" / "manifest.json")
|
||||
full = detect(tmp_path)
|
||||
save_manifest(full["files"], manifest_path, root=tmp_path)
|
||||
|
||||
b.unlink()
|
||||
inc = detect_incremental(tmp_path, manifest_path)
|
||||
assert [Path(f).name for f in inc["deleted_files"]] == ["b.py"]
|
||||
assert inc["excluded_files"] == []
|
||||
|
||||
|
||||
def test_detect_incremental_exclusion_stable_across_runs(tmp_path):
|
||||
"""After a full-scan save prunes the excluded row, later incremental runs
|
||||
report the file neither as deleted nor as excluded — the exclusion has
|
||||
fully settled instead of resurfacing forever."""
|
||||
a = tmp_path / "a.py"
|
||||
b = tmp_path / "b.py"
|
||||
a.write_text("x = 1\n")
|
||||
b.write_text("y = 2\n")
|
||||
manifest_path = str(tmp_path / "graphify-out" / "manifest.json")
|
||||
full = detect(tmp_path)
|
||||
save_manifest(full["files"], manifest_path, root=tmp_path)
|
||||
|
||||
# Run 1: b.py newly excluded — reported as excluded, then the full-scan
|
||||
# save (what extract does at the end of the run) prunes its row.
|
||||
inc1 = detect_incremental(tmp_path, manifest_path, extra_excludes=["b.py"])
|
||||
assert [Path(f).name for f in inc1["excluded_files"]] == ["b.py"]
|
||||
assert inc1["deleted_files"] == []
|
||||
corpus = {f for flist in inc1["files"].values() for f in flist}
|
||||
save_manifest(inc1["files"], manifest_path, root=tmp_path, scan_corpus=corpus)
|
||||
|
||||
# Run 2 (and beyond): steady state — nothing deleted, nothing excluded.
|
||||
inc2 = detect_incremental(tmp_path, manifest_path, extra_excludes=["b.py"])
|
||||
assert inc2["deleted_files"] == []
|
||||
assert inc2["excluded_files"] == []
|
||||
|
||||
@@ -364,3 +364,170 @@ def test_extract_timing_flag_emits_stage_timings(monkeypatch, tmp_path, capsys):
|
||||
mainmod.main()
|
||||
assert exc2.value.code == 0
|
||||
assert "graphify timing" not in capsys.readouterr().err
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# #1909: a newly-excluded file's nodes must be pruned from graph.json on the
|
||||
# next incremental extract even when the manifest never listed the file (the
|
||||
# pre-#1897 state every 0.9.16 graph is in), so the manifest-diff prune set
|
||||
# (`manifest - corpus`) can never see it.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _two_file_corpus(tmp_path):
|
||||
project = tmp_path / "project"
|
||||
project.mkdir()
|
||||
(project / "x.py").write_text(
|
||||
"def secret_helper():\n return 42\n\n"
|
||||
"def secret_caller():\n return secret_helper()\n"
|
||||
)
|
||||
(project / "keep.py").write_text(
|
||||
"def kept():\n return still_here()\n\n"
|
||||
"def still_here():\n return 1\n"
|
||||
)
|
||||
return project
|
||||
|
||||
|
||||
def _node_sources(graph_path):
|
||||
import json
|
||||
data = json.loads(graph_path.read_text(encoding="utf-8"))
|
||||
return {n.get("source_file", "") for n in data.get("nodes", [])}
|
||||
|
||||
|
||||
def _run_extract(monkeypatch, argv):
|
||||
monkeypatch.setattr(mainmod.sys, "argv", argv)
|
||||
try:
|
||||
mainmod.main()
|
||||
except SystemExit as exc:
|
||||
assert exc.code in (None, 0), f"unexpected exit code {exc.code}"
|
||||
|
||||
|
||||
def test_incremental_extract_prunes_newly_excluded_file_not_in_manifest(
|
||||
monkeypatch, tmp_path
|
||||
):
|
||||
"""Seed a graph with nodes for x.py, drop x.py from the manifest (pre-#1897
|
||||
manifests never listed excluded/omitted files), exclude x.py via
|
||||
.graphifyignore, re-run extract: x.py's nodes must be gone even though it
|
||||
was never on the deleted list."""
|
||||
import json
|
||||
project = _two_file_corpus(tmp_path)
|
||||
out_dir = tmp_path / "out"
|
||||
_clear_backend_keys(monkeypatch)
|
||||
monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None)
|
||||
|
||||
_run_extract(
|
||||
monkeypatch,
|
||||
["graphify", "extract", str(project), "--out", str(out_dir)],
|
||||
)
|
||||
graph_path = out_dir / "graphify-out" / "graph.json"
|
||||
manifest_path = out_dir / "graphify-out" / "manifest.json"
|
||||
assert any("x.py" in s for s in _node_sources(graph_path)), (
|
||||
"seed extract must produce nodes for x.py"
|
||||
)
|
||||
|
||||
# Simulate the pre-#1897 manifest state: x.py was never manifest-listed,
|
||||
# so `manifest - corpus` can never flag it.
|
||||
manifest = json.loads(manifest_path.read_text(encoding="utf-8"))
|
||||
manifest = {k: v for k, v in manifest.items() if "x.py" not in k}
|
||||
manifest_path.write_text(json.dumps(manifest), encoding="utf-8")
|
||||
|
||||
(project / ".graphifyignore").write_text("x.py\n")
|
||||
_run_extract(
|
||||
monkeypatch,
|
||||
["graphify", "extract", str(project), "--out", str(out_dir)],
|
||||
)
|
||||
|
||||
sources = _node_sources(graph_path)
|
||||
assert not any("x.py" in s for s in sources), (
|
||||
f"newly-excluded x.py must be pruned from graph.json, still see {sources}"
|
||||
)
|
||||
assert any("keep.py" in s for s in sources), (
|
||||
"unchanged keep.py nodes must survive the incremental merge"
|
||||
)
|
||||
# x.py exists on disk, is excluded, and must not creep into the manifest.
|
||||
manifest = json.loads(manifest_path.read_text(encoding="utf-8"))
|
||||
assert not any("x.py" in k for k in manifest), (
|
||||
f"excluded x.py must not be (re)listed in the manifest: {set(manifest)}"
|
||||
)
|
||||
|
||||
|
||||
def test_incremental_extract_prunes_excluded_file_listed_in_manifest(
|
||||
monkeypatch, tmp_path
|
||||
):
|
||||
"""Post-#1897 state: the excluded file IS manifest-listed. It must be
|
||||
pruned from graph.json AND dropped from the manifest (#1908), and stay
|
||||
settled on a further run."""
|
||||
import json
|
||||
project = _two_file_corpus(tmp_path)
|
||||
out_dir = tmp_path / "out"
|
||||
_clear_backend_keys(monkeypatch)
|
||||
monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None)
|
||||
|
||||
_run_extract(
|
||||
monkeypatch,
|
||||
["graphify", "extract", str(project), "--out", str(out_dir)],
|
||||
)
|
||||
graph_path = out_dir / "graphify-out" / "graph.json"
|
||||
manifest_path = out_dir / "graphify-out" / "manifest.json"
|
||||
assert any("x.py" in k for k in json.loads(manifest_path.read_text()))
|
||||
|
||||
(project / ".graphifyignore").write_text("x.py\n")
|
||||
_run_extract(
|
||||
monkeypatch,
|
||||
["graphify", "extract", str(project), "--out", str(out_dir)],
|
||||
)
|
||||
|
||||
sources = _node_sources(graph_path)
|
||||
assert not any("x.py" in s for s in sources)
|
||||
assert any("keep.py" in s for s in sources)
|
||||
manifest = json.loads(manifest_path.read_text(encoding="utf-8"))
|
||||
assert not any("x.py" in k for k in manifest), (
|
||||
"excluded-but-alive manifest row must be pruned (#1908)"
|
||||
)
|
||||
|
||||
# Steady state: a third run neither resurrects x.py nor loses keep.py.
|
||||
_run_extract(
|
||||
monkeypatch,
|
||||
["graphify", "extract", str(project), "--out", str(out_dir)],
|
||||
)
|
||||
sources = _node_sources(graph_path)
|
||||
assert not any("x.py" in s for s in sources)
|
||||
assert any("keep.py" in s for s in sources)
|
||||
|
||||
|
||||
def test_no_cluster_incremental_prunes_newly_excluded_file(
|
||||
monkeypatch, tmp_path, capsys
|
||||
):
|
||||
"""--no-cluster's exclusion-only early exit must still scrub the excluded
|
||||
file's nodes from the raw graph.json (that path never runs build_merge),
|
||||
and must not report the alive file as deleted."""
|
||||
import json
|
||||
project = _two_file_corpus(tmp_path)
|
||||
out_dir = tmp_path / "out"
|
||||
_clear_backend_keys(monkeypatch)
|
||||
monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None)
|
||||
|
||||
monkeypatch.setattr(
|
||||
mainmod.sys, "argv",
|
||||
["graphify", "extract", str(project), "--no-cluster", "--out", str(out_dir)],
|
||||
)
|
||||
with pytest.raises(SystemExit) as exc:
|
||||
mainmod.main()
|
||||
assert exc.value.code == 0
|
||||
graph_path = out_dir / "graphify-out" / "graph.json"
|
||||
assert any("x.py" in s for s in _node_sources(graph_path))
|
||||
capsys.readouterr()
|
||||
|
||||
(project / ".graphifyignore").write_text("x.py\n")
|
||||
with pytest.raises(SystemExit) as exc:
|
||||
mainmod.main()
|
||||
assert exc.value.code == 0
|
||||
out_text = capsys.readouterr().out
|
||||
assert "1 deleted" not in out_text, (
|
||||
"excluded-but-alive file must not be reported as deleted"
|
||||
)
|
||||
|
||||
sources = _node_sources(graph_path)
|
||||
assert not any("x.py" in s for s in sources), (
|
||||
f"--no-cluster early exit must prune excluded sources, still see {sources}"
|
||||
)
|
||||
assert any("keep.py" in s for s in sources)
|
||||
|
||||
Reference in New Issue
Block a user