mirror of
https://github.com/safishamsi/graphify.git
synced 2026-07-14 03:17:28 +00:00
fix(build_merge): replace re-extracted files instead of accumulating stale edges (#1344)
build_merge: prune a re-extracted file's stale nodes/edges before merge instead of accumulating (fixes #1283, #1285). Validated: full suite 2107 passed. Thanks @RelywOo.
This commit is contained in:
+37
-4
@@ -443,8 +443,11 @@ def build_merge(
|
||||
) -> nx.Graph:
|
||||
"""Load existing graph.json, merge new chunks into it, and save back.
|
||||
|
||||
Never replaces - only grows (or prunes deleted-file nodes via prune_sources).
|
||||
Safe to call repeatedly: existing nodes and edges are preserved.
|
||||
Re-extracted files REPLACE their prior contribution: any source_file present
|
||||
in new_chunks is dropped from the loaded graph before merging, so a changed
|
||||
file's stale nodes/edges don't accumulate. Files absent from new_chunks are
|
||||
preserved unchanged; deleted files are removed via prune_sources.
|
||||
Safe to call repeatedly.
|
||||
root: if given, absolute source_file paths in new_chunks are made relative (#932).
|
||||
"""
|
||||
graph_path = Path(graph_path)
|
||||
@@ -462,10 +465,40 @@ def build_merge(
|
||||
links_key = "links" if "links" in data else "edges"
|
||||
existing_nodes = list(data.get("nodes", []))
|
||||
existing_edges = list(data.get(links_key, []))
|
||||
base = [{"nodes": existing_nodes, "edges": existing_edges}]
|
||||
had_graph = True
|
||||
else:
|
||||
existing_nodes = []
|
||||
base = []
|
||||
existing_edges = []
|
||||
had_graph = False
|
||||
|
||||
# Re-extracted files REPLACE their prior contribution. Every source_file
|
||||
# present in new_chunks is dropped from the loaded base before merging, so a
|
||||
# CHANGED file's stale nodes/edges don't accumulate across incremental
|
||||
# updates. Without this, build() merges old+new for the same file and only
|
||||
# exact-duplicate edges collapse — edges/nodes that disappeared from the new
|
||||
# version survive forever. Brand-new files aren't in base, so this is a no-op
|
||||
# for them; genuinely deleted files are still handled via prune_sources.
|
||||
# Matched in both raw and _norm_source_file form because new_chunks may carry
|
||||
# absolute win32 paths while the stored graph keeps relative posix (#1007).
|
||||
_replace_root = str(Path(root).resolve()) if root is not None else None
|
||||
new_sources: set[str] = set()
|
||||
for ch in new_chunks:
|
||||
for n in ch.get("nodes", []):
|
||||
sf = n.get("source_file")
|
||||
if not sf:
|
||||
continue
|
||||
new_sources.add(sf)
|
||||
norm = _norm_source_file(sf, _replace_root)
|
||||
if norm:
|
||||
new_sources.add(norm)
|
||||
if new_sources:
|
||||
def _kept(item: dict) -> bool:
|
||||
sf = item.get("source_file")
|
||||
return sf not in new_sources and _norm_source_file(sf, _replace_root) not in new_sources
|
||||
existing_nodes = [n for n in existing_nodes if _kept(n)]
|
||||
existing_edges = [e for e in existing_edges if _kept(e)]
|
||||
|
||||
base = [{"nodes": existing_nodes, "edges": existing_edges}] if had_graph else []
|
||||
|
||||
all_chunks = base + list(new_chunks)
|
||||
G = build(all_chunks, directed=directed, dedup=dedup, dedup_llm_backend=dedup_llm_backend, root=root)
|
||||
|
||||
@@ -549,6 +549,58 @@ def test_build_merge_prune_windows_backslash_paths(tmp_path):
|
||||
assert "parse_date" not in node_labels, "node should be pruned even with backslash path"
|
||||
|
||||
|
||||
def test_build_merge_replaces_changed_file_stale_edges(tmp_path):
|
||||
"""Re-extracting a CHANGED file must REPLACE its prior nodes/edges, not
|
||||
accumulate them. build_merge previously only grew the graph, so an edge that
|
||||
disappeared from a file's new version survived forever (only exact-duplicate
|
||||
edges collapsed). The new-chunk source_file may be an absolute win32 path
|
||||
while the stored graph keeps relative posix — both forms must match."""
|
||||
import networkx as nx
|
||||
|
||||
root = tmp_path / "corpus"
|
||||
root.mkdir()
|
||||
graph_path = tmp_path / "graph.json"
|
||||
|
||||
# First build: changed.md contributed A, B and edge A->B; keep.md is unrelated.
|
||||
chunk0 = {"nodes": [
|
||||
{"id": "A", "label": "A", "file_type": "document", "source_file": "changed.md"},
|
||||
{"id": "B", "label": "B", "file_type": "document", "source_file": "changed.md"},
|
||||
{"id": "K", "label": "K", "file_type": "document", "source_file": "keep.md"},
|
||||
], "edges": [
|
||||
{"source": "A", "target": "B", "relation": "references", "confidence": "EXTRACTED",
|
||||
"source_file": "changed.md", "weight": 1.0},
|
||||
{"source": "K", "target": "A", "relation": "references", "confidence": "EXTRACTED",
|
||||
"source_file": "keep.md", "weight": 1.0},
|
||||
]}
|
||||
G0 = build([chunk0], dedup=False)
|
||||
graph_path.write_text(json.dumps(nx.node_link_data(G0, edges="edges")), encoding="utf-8")
|
||||
|
||||
# changed.md edited: re-extraction now yields A, C and edge A->C (B dropped).
|
||||
# source_file arrives as an absolute win32-style path (as detect emits on Windows).
|
||||
abs_changed = str(root / "changed.md").replace("/", "\\")
|
||||
new_chunk = {"nodes": [
|
||||
{"id": "A", "label": "A", "file_type": "document", "source_file": abs_changed},
|
||||
{"id": "C", "label": "C", "file_type": "document", "source_file": abs_changed},
|
||||
], "edges": [
|
||||
{"source": "A", "target": "C", "relation": "references", "confidence": "EXTRACTED",
|
||||
"source_file": abs_changed, "weight": 1.0},
|
||||
]}
|
||||
G1 = build_merge([new_chunk], graph_path, dedup=False, root=root)
|
||||
|
||||
labels = {d["label"] for _, d in G1.nodes(data=True)}
|
||||
edges = {(u, v) for u, v in G1.edges()}
|
||||
|
||||
# Stale contribution from the old version of changed.md is gone.
|
||||
assert "B" not in labels, "stale node from changed file's old version must be dropped"
|
||||
assert ("A", "B") not in edges and ("B", "A") not in edges, "stale edge must be dropped"
|
||||
# Fresh contribution is present.
|
||||
assert "C" in labels, "re-extracted node must be present"
|
||||
assert ("A", "C") in edges, "re-extracted edge must be present"
|
||||
# An unchanged file is untouched.
|
||||
assert "K" in labels, "unchanged file's node must survive"
|
||||
assert ("K", "A") in edges, "unchanged file's edge must survive"
|
||||
|
||||
|
||||
def test_build_merge_rejects_oversized_existing_graph(monkeypatch, tmp_path):
|
||||
"""#F4: build_merge must refuse to read an existing graph.json that
|
||||
exceeds the size cap, rather than json.loads-ing it into memory."""
|
||||
|
||||
Reference in New Issue
Block a user