mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-28 01:06:36 +00:00
fix(update): prune edges owned by a re-extracted file (#1521)
`graphify update` preserved old edges keyed only on endpoint-node membership, so a removed import's edge survived forever whenever both endpoint nodes still existed (e.g. a.py no longer imports b, but both a and b are still present). The stale edge drove phantom circular-dependency findings; `--force` didn't help (it only gates the node-count shrink guard), only a clean rebuild did. _rebuild_code (watch.py) now also drops preserved edges whose source_file was re-extracted this run — an edge is owned by the file it was extracted from, and the fresh extraction re-emits whichever ones still exist. Scoped to source_file ownership (and the full-rebuild case, where evict_sources lists only deleted files), so edges with no source_file or owned by a non-re-extracted file are kept — cross-file edges that merely point at a re-extracted file (#1402 sourceless stubs/rewire) are not over-pruned. Reported by @UltronOfSpace (#1521). Regression test: a removed import's edge is gone after update; verified no over-prune (still-present imports survive) and no regression to deleted-file pruning. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
faa9218907
commit
0080d8ac43
@@ -641,9 +641,40 @@ def _rebuild_code(
|
||||
and (not evict_sources or n.get("source_file") not in evict_sources)
|
||||
]
|
||||
all_ids = new_ast_ids | {n["id"] for n in preserved_nodes}
|
||||
# An edge is OWNED by the file it was extracted from (its
|
||||
# source_file). When that file is re-extracted, its prior edges must
|
||||
# not be carried forward — the fresh extraction re-emits whichever
|
||||
# ones still exist. Preserving by endpoint membership alone keeps a
|
||||
# removed import's edge alive forever whenever both endpoint nodes
|
||||
# survive (e.g. `a` no longer imports from `b`, but both `a` and `b`
|
||||
# are still present), producing phantom circular dependencies
|
||||
# (#1521). So drop preserved edges whose source_file was re-extracted
|
||||
# this run (or deleted). Unlike the node-level evict set, this MUST
|
||||
# cover the full-rebuild case too — there every file is re-extracted
|
||||
# but `evict_sources` only lists deleted files, so a removed import
|
||||
# in a surviving file would never be pruned. Edges with no
|
||||
# source_file, or owned by a file that was NOT re-extracted, are
|
||||
# kept exactly as before, so cross-file edges that merely point at a
|
||||
# re-extracted file (#1402 sourceless stubs / cross-file rewire) are
|
||||
# not over-pruned — only edges the re-extracted file itself produced.
|
||||
edge_evict_sources: set[str] = set(evict_sources)
|
||||
for p in extract_targets:
|
||||
for _root in (project_root, watch_root):
|
||||
edge_evict_sources.add(_nsf(str(p), str(_root)) or str(p))
|
||||
def _edge_evicted(e: dict) -> bool:
|
||||
if not edge_evict_sources:
|
||||
return False
|
||||
sf = e.get("source_file")
|
||||
if not sf:
|
||||
return False
|
||||
if sf in edge_evict_sources:
|
||||
return True
|
||||
norm = _nsf(sf, str(project_root))
|
||||
return bool(norm) and norm in edge_evict_sources
|
||||
preserved_edges = [
|
||||
e for e in existing.get("links", existing.get("edges", []))
|
||||
if e.get("source") in all_ids and e.get("target") in all_ids
|
||||
and not _edge_evicted(e)
|
||||
]
|
||||
result = {
|
||||
"nodes": result["nodes"] + preserved_nodes,
|
||||
|
||||
@@ -96,3 +96,40 @@ def test_extract_no_cluster_incremental_noop_preserves_existing_graph(tmp_path):
|
||||
after = json.loads(after_text)
|
||||
assert after.get("nodes"), "no-op incremental run must not empty the graph"
|
||||
assert after_text == before_text
|
||||
|
||||
|
||||
def _edges(graph_json: Path) -> list[dict]:
|
||||
g = json.loads(graph_json.read_text())
|
||||
return g.get("links", g.get("edges", []))
|
||||
|
||||
|
||||
def test_update_prunes_a_removed_imports_edge(tmp_path):
|
||||
"""#1521: when an import is deleted from a file, `graphify update` must prune
|
||||
the edge it produced — preserving it (keyed only on endpoint membership) left a
|
||||
stale edge that drove phantom circular-dependency findings."""
|
||||
proj = tmp_path / "proj"
|
||||
pkg = proj / "pkg"
|
||||
pkg.mkdir(parents=True)
|
||||
(pkg / "b.py").write_text("def helper():\n return 1\n")
|
||||
(pkg / "a.py").write_text("from pkg.b import helper\ndef use():\n return helper()\n")
|
||||
|
||||
# initial extract -> the import edge a -> b exists
|
||||
r1 = _run(["extract", str(proj), "--no-cluster"], tmp_path)
|
||||
assert r1.returncode == 0, r1.stderr
|
||||
gj = proj / "graphify-out" / "graph.json"
|
||||
before = _edges(gj)
|
||||
assert any(e.get("relation") in ("imports", "imports_from") and
|
||||
str(e.get("source_file", "")).endswith("a.py") for e in before), \
|
||||
f"expected an import edge from a.py initially: {before}"
|
||||
|
||||
# remove the import, then update
|
||||
(pkg / "a.py").write_text("def use():\n return 1\n")
|
||||
r2 = _run(["update", str(proj)], tmp_path)
|
||||
assert r2.returncode == 0, r2.stderr
|
||||
after = _edges(gj)
|
||||
|
||||
# the stale import edge owned by a.py must be gone
|
||||
stale = [e for e in after
|
||||
if e.get("relation") in ("imports", "imports_from")
|
||||
and str(e.get("source_file", "")).endswith("a.py")]
|
||||
assert not stale, f"removed import's edge survived update (stale): {stale}"
|
||||
|
||||
Reference in New Issue
Block a user