mirror of
https://github.com/safishamsi/graphify.git
synced 2026-09-13 00:55:53 +00:00
fix(build): form-insensitive prune + scan-root marker (#2012)
build_merge now prunes a deleted file's nodes, edges, and hyperedges regardless of whether their stored source_file is absolute or relative. When the caller passed no root (the --update runbook), a node that kept an absolute path slipped past the relative prune set and the deleted file's graph survived silently. Matching is now form-insensitive (raw, normalized-relative, then an absolute- identity fallback); a re-extracted file is still never pruned (#1796 preserved). `graphify extract` also writes the .graphify_root marker after every graph write so a later build_merge relativizes deleted-file paths correctly even under a custom --out (its grandparent-of-graph.json fallback pointed at the wrong dir). Regression tests in tests/test_build_merge_hyperedges_and_prune.py.
This commit is contained in:
+50
-4
@@ -151,6 +151,29 @@ def _norm_source_file(p: str | None, root: str | None = None) -> str | None:
|
||||
return p
|
||||
|
||||
|
||||
def _abs_identity(p: str | None, root: str | None = None) -> str | None:
|
||||
"""Return a form-insensitive absolute identity for a source_file.
|
||||
|
||||
prune/replace matching in build_merge otherwise compares raw strings against
|
||||
``_norm_source_file`` output, so a node whose source_file survived in a THIRD
|
||||
form — absolute where prune_sources is relative, or vice versa, or a symlinked
|
||||
root — slips past every equality check and its nodes/edges are never pruned
|
||||
(silent survival of a deleted file's graph, #2012). Anchoring relative paths
|
||||
at ``root`` and resolving both sides to a canonical absolute posix path gives
|
||||
a fallback that matches regardless of which form each side happens to hold.
|
||||
"""
|
||||
if not p:
|
||||
return None
|
||||
q = p.replace("\\", "/")
|
||||
pp = Path(q)
|
||||
if not pp.is_absolute() and root:
|
||||
pp = Path(root) / q
|
||||
try:
|
||||
return pp.resolve().as_posix()
|
||||
except OSError:
|
||||
return pp.as_posix()
|
||||
|
||||
|
||||
def _infer_merge_root(graph_path: Path) -> str | None:
|
||||
"""Best-effort scan root for relativizing paths in build_merge when the caller
|
||||
passes no ``root`` (#1571).
|
||||
@@ -1038,6 +1061,7 @@ def build_merge(
|
||||
# handles symlinked roots and ".." / "./" segments so Path.relative_to()
|
||||
# succeeds even when the scan root is a symlink. (#1007, #1571)
|
||||
prune_set: set[str] = set()
|
||||
prune_abs: set[str] = set()
|
||||
for p in (prune_sources or []):
|
||||
if not p:
|
||||
continue
|
||||
@@ -1045,13 +1069,35 @@ def build_merge(
|
||||
norm = _norm_source_file(p, _eff_root)
|
||||
if norm:
|
||||
prune_set.add(norm)
|
||||
a = _abs_identity(p, _eff_root)
|
||||
if a:
|
||||
prune_abs.add(a)
|
||||
# A file that was just re-extracted (present in new_chunks) is being REPLACED,
|
||||
# never deleted — so never prune it, even if the caller also lists it in
|
||||
# prune_sources. Otherwise its fresh, just-built nodes are silently removed
|
||||
# (data loss): common when an edit keeps a node's label and the caller follows
|
||||
# the old edit-workflow of passing the changed file in prune_sources (#1796).
|
||||
# "replace" wins over a contradictory "delete" of the same source.
|
||||
# "replace" wins over a contradictory "delete" of the same source. Applied in
|
||||
# both string and absolute-identity space so the third-form fallback below
|
||||
# can't resurrect the delete for a re-extracted file (#2012).
|
||||
prune_set -= new_sources
|
||||
new_abs = {_abs_identity(s, _eff_root) for s in new_sources}
|
||||
new_abs.discard(None)
|
||||
prune_abs -= new_abs
|
||||
|
||||
def _prune_match(sf: "str | None") -> bool:
|
||||
# Match a node/edge/hyperedge source_file against the prune set in a
|
||||
# form-insensitive way: exact string, normalised-relative, then the
|
||||
# absolute-identity fallback for the third-form case (#2012).
|
||||
if not sf:
|
||||
return False
|
||||
if sf in prune_set:
|
||||
return True
|
||||
norm = _norm_source_file(sf, _eff_root)
|
||||
if norm and norm in prune_set:
|
||||
return True
|
||||
a = _abs_identity(sf, _eff_root)
|
||||
return bool(a) and a in prune_abs
|
||||
|
||||
# Carry forward hyperedges from files that were neither re-extracted nor
|
||||
# deleted (#1574). build() only sees the new chunks' hyperedges, so without
|
||||
@@ -1070,7 +1116,7 @@ def build_merge(
|
||||
norm = _norm_source_file(sf, _eff_root)
|
||||
if sf in new_sources or norm in new_sources:
|
||||
continue # re-extracted — replaced by the new chunk's version
|
||||
if sf in prune_set or norm in prune_set:
|
||||
if _prune_match(sf):
|
||||
continue # deleted — pruned
|
||||
carried.append(he)
|
||||
if carried:
|
||||
@@ -1081,7 +1127,7 @@ def build_merge(
|
||||
if prune_sources:
|
||||
to_remove = [
|
||||
n for n, d in G.nodes(data=True)
|
||||
if d.get("source_file") in prune_set
|
||||
if _prune_match(d.get("source_file"))
|
||||
]
|
||||
G.remove_nodes_from(to_remove)
|
||||
n_files = len(prune_sources)
|
||||
@@ -1094,7 +1140,7 @@ def build_merge(
|
||||
|
||||
edges_to_remove = [
|
||||
(u, v) for u, v, d in G.edges(data=True)
|
||||
if d.get("source_file") in prune_set
|
||||
if _prune_match(d.get("source_file"))
|
||||
]
|
||||
if edges_to_remove:
|
||||
G.remove_edges_from(edges_to_remove)
|
||||
|
||||
@@ -3158,6 +3158,16 @@ def dispatch_command(cmd: str) -> None:
|
||||
_invalidate_file_manifest_for_db_graph()
|
||||
from graphify.paths import write_json_atomic as _write_json_atomic
|
||||
_write_json_atomic(graph_json_path, merged, indent=2)
|
||||
try:
|
||||
# Record the scan root so a later build_merge / update runbook can
|
||||
# relativize deleted-file paths correctly even for a custom --out
|
||||
# (its grandparent-of-graph.json fallback points at the wrong dir
|
||||
# otherwise, and deleted files never prune — #2012/#1571).
|
||||
(graphify_out / ".graphify_root").write_text(
|
||||
str(Path(target).resolve()), encoding="utf-8"
|
||||
)
|
||||
except OSError:
|
||||
pass
|
||||
stages.mark("write")
|
||||
cost = _estimate_cost(
|
||||
backend, merged["input_tokens"], merged["output_tokens"]
|
||||
@@ -3284,6 +3294,14 @@ def dispatch_command(cmd: str) -> None:
|
||||
file=sys.stderr,
|
||||
)
|
||||
sys.exit(1)
|
||||
try:
|
||||
# See the --no-cluster path above: persist the scan root so build_merge
|
||||
# can relativize deleted-file paths under a custom --out (#2012/#1571).
|
||||
(graphify_out / ".graphify_root").write_text(
|
||||
str(Path(target).resolve()), encoding="utf-8"
|
||||
)
|
||||
except OSError:
|
||||
pass
|
||||
stages.mark("export")
|
||||
if merged.get("output_tokens", 0) > 0:
|
||||
(graphify_out / ".graphify_semantic_marker").write_text(
|
||||
|
||||
@@ -207,3 +207,60 @@ def test_genuine_deletion_still_prunes(tmp_path):
|
||||
labels = {G.nodes[n].get("label") for n in G.nodes()}
|
||||
assert "Other" not in labels, "genuinely deleted file's node should be pruned"
|
||||
assert "Widget Cache Design" in labels
|
||||
|
||||
|
||||
# ── #2012: form-insensitive prune (absolute node vs relative prune, and back) ──
|
||||
|
||||
def test_prune_matches_node_stored_absolute_against_relative_delete(tmp_path):
|
||||
"""#2012: a node whose source_file survived in ABSOLUTE form must still be
|
||||
pruned when the deletion is expressed relative to root. The runbook calls
|
||||
build_merge WITHOUT root, so build() does not re-normalize the node's stored
|
||||
absolute source_file; the old prune membership test then compared that raw
|
||||
absolute string against a prune_set that only held the relative forms, so the
|
||||
node slipped through and a deleted file's graph survived silently. build_merge
|
||||
now normalizes the node side too + an absolute-identity fallback."""
|
||||
root = tmp_path / "corpus"
|
||||
(root / "graphify-out").mkdir(parents=True)
|
||||
graph_path = root / "graphify-out" / "graph.json"
|
||||
nodes = [
|
||||
# gone.py's node kept an ABSOLUTE source_file (a semantic subagent wrote
|
||||
# it that way, #932); keep.py's is relative.
|
||||
{"id": "g1", "label": "gone", "file_type": "code",
|
||||
"source_file": str(root / "gone.py")},
|
||||
{"id": "k1", "label": "keep", "file_type": "code", "source_file": "keep.py"},
|
||||
]
|
||||
edges = [
|
||||
{"source": "g1", "target": "k1", "type": "calls",
|
||||
"source_file": str(root / "gone.py")},
|
||||
]
|
||||
_write_graph(graph_path, nodes, edges, [])
|
||||
# Runbook-style: NO root passed (eff_root inferred from the graphify-out
|
||||
# grandparent), so build() leaves the absolute node form intact. Deletion is
|
||||
# expressed RELATIVE — a third form vs the stored absolute node.
|
||||
G = build_merge([], graph_path, prune_sources=["gone.py"], dedup=False)
|
||||
labels = {d["label"] for _, d in G.nodes(data=True)}
|
||||
assert "gone" not in labels, "absolute-stored node not pruned by relative delete (#2012)"
|
||||
assert "keep" in labels
|
||||
assert G.number_of_edges() == 0, "edge from the deleted file must be pruned too (#2012)"
|
||||
|
||||
|
||||
def test_prune_reextracted_absolute_node_not_deleted(tmp_path):
|
||||
"""#1796 protection must hold in absolute-identity space too: a file present
|
||||
in BOTH new_chunks and prune_sources (in mismatched forms) is REPLACED, not
|
||||
deleted — the #2012 form-insensitive match must not resurrect the delete for
|
||||
a re-extracted file."""
|
||||
root = tmp_path / "corpus"
|
||||
(root / "graphify-out").mkdir(parents=True)
|
||||
graph_path = root / "graphify-out" / "graph.json"
|
||||
_write_graph(graph_path, [
|
||||
{"id": "g1", "label": "gone", "file_type": "code",
|
||||
"source_file": str(root / "mod.py")},
|
||||
], [], [])
|
||||
# Re-extracted with a RELATIVE source_file; prune lists it RELATIVE too.
|
||||
# No root passed (runbook), so the stored absolute node is not re-normalized.
|
||||
new_chunk = {"nodes": [
|
||||
{"id": "g1", "label": "gone", "file_type": "code", "source_file": "mod.py"},
|
||||
], "edges": []}
|
||||
G = build_merge([new_chunk], graph_path, prune_sources=["mod.py"], dedup=False)
|
||||
labels = {d["label"] for _, d in G.nodes(data=True)}
|
||||
assert "gone" in labels, "re-extracted file wrongly pruned across mismatched forms (#2012/#1796)"
|
||||
|
||||
Reference in New Issue
Block a user