From fe905e280bab93868da53c56dd33c12306097137 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 17 Sep 2026 09:19:42 +0900 Subject: [PATCH] fix: excuse dedup-explained node drops from the #479 shrink guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit build_from_json's #1145 ghost-merge pass collapses non-AST duplicate nodes (e.g. a manifest package node and its AST-canonical twin sharing the same source_file/label) into their AST canonical counterpart. That is a legitimate, data-preserving node-count reduction, but to_json's #479 shrink guard could not distinguish it from an unverified shrink and refused to overwrite graph.json even when nothing was actually lost — reproduced on a 23,934-node graph.json where reloading via build_from_json produced 23,933 NetworkX nodes (crate:mlsirm-core merged into its AST twin pkg_mlsirm_core, zero incident edges dropped) and cluster-only refused the write. build_from_json now records the collapsed-node count as G.graph["_ghost_dedup_count"]; to_json's shrink guard treats a node-count drop as safe when it is fully explained by that count, and still refuses when any part of the drop is unexplained. Fixes downstream report at ContextualWisdomLab/fast-mlsirm#1847. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BXYRNTJhYLB2bYveuaFe74 --- graphify/build.py | 7 +++++++ graphify/export.py | 37 +++++++++++++++++++++++++------------ tests/test_build.py | 18 ++++++++++++++++++ tests/test_export.py | 24 ++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 12 deletions(-) diff --git a/graphify/build.py b/graphify/build.py index 861a928553..889040777b 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -1113,6 +1113,13 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat G.remove_node(ghost_id) node_set.discard(ghost_id) + # #1847: record how many nodes this canonicalization pass legitimately + # collapsed (AST/manifest duplicates sharing (source_file, label)), so a + # downstream shrink guard (export.to_json) can tell a merge-explained node + # count drop from an unexplained data-loss shrink instead of refusing both + # alike. + G.graph["_ghost_dedup_count"] = G.graph.get("_ghost_dedup_count", 0) + len(_ghost_remap) + # Normalized ID map: lets edges survive when the LLM generates IDs with # slightly different casing or punctuation than the AST extractor. # e.g. "Session_ValidateToken" maps to "session_validatetoken". diff --git a/graphify/export.py b/graphify/export.py index 43c9b9adb7..0e24fdac53 100644 --- a/graphify/export.py +++ b/graphify/export.py @@ -312,18 +312,31 @@ def to_json(G: nx.Graph, communities: dict[int, list[str]], output_path: str, *, return False new_n = G.number_of_nodes() if new_n < existing_n: - import sys as _sys - print( - f"[graphify] WARNING: new graph has {new_n} nodes but existing " - f"graph.json has {existing_n} (net -{existing_n - new_n}). " - f"Refusing to overwrite. Possible causes: missing chunk files from " - f"a previous session, or fuzzy dedup collapsed same-named symbols " - f"across files during an --update on an already-current graph. " - f"Run a full rebuild (/graphify .) to be safe, or pass force=True " - f"only if you have verified the reduction is legitimate.", - file=_sys.stderr, - ) - return False + # #1847: build_from_json legitimately collapses duplicate + # nodes that resolve to the same (source_file, label) — e.g. a + # manifest-derived package node and its AST-canonical twin + # (crate:foo vs pkg_foo). That merge is recorded on the graph + # as `_ghost_dedup_count`. When the observed drop is fully + # explained by that count, it is a known-safe canonicalization, + # not the unverified data-loss shrink #479 guards against — + # proceed instead of refusing. + dedup = int(getattr(G, "graph", {}).get("_ghost_dedup_count", 0) or 0) + unexplained = (existing_n - new_n) - dedup + if unexplained > 0: + import sys as _sys + print( + f"[graphify] WARNING: new graph has {new_n} nodes but existing " + f"graph.json has {existing_n} (net -{existing_n - new_n}, " + f"{dedup} explained by duplicate-node canonicalization, " + f"{unexplained} unexplained). " + f"Refusing to overwrite. Possible causes: missing chunk files from " + f"a previous session, or fuzzy dedup collapsed same-named symbols " + f"across files during an --update on an already-current graph. " + f"Run a full rebuild (/graphify .) to be safe, or pass force=True " + f"only if you have verified the reduction is legitimate.", + file=_sys.stderr, + ) + return False node_community = _node_community_map(communities) _labels: dict[int, str] = {int(k): v for k, v in (community_labels or {}).items()} diff --git a/tests/test_build.py b/tests/test_build.py index c496873cbe..b4ef07e8e4 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -488,6 +488,24 @@ def test_ghost_merge_unique_located_node_still_merges(): assert G.has_edge("caller", "ast_render") +def test_ghost_merge_records_dedup_count(): + """#1847: each ghost collapsed into its AST canonical twin is a legitimate + node-count reduction, not data loss. build_from_json must record how many + were merged so a downstream shrink guard can tell the two apart instead of + refusing to overwrite a graph.json that only shrank via this dedup.""" + ext = { + "nodes": [ + {"id": "ast_render", "label": "render", "file_type": "code", + "source_file": "src/app/index.ts", "source_location": "L10", "_origin": "ast"}, + {"id": "ghost_render", "label": "render", "file_type": "code", + "source_file": "src/app/index.ts"}, + ], + "edges": [], "input_tokens": 0, "output_tokens": 0, + } + G = build_from_json(ext) + assert G.graph.get("_ghost_dedup_count") == 1 + + def test_ghost_merge_uses_source_file_not_basename(): """#2068: the ghost-merge key is the full source_file, not the bare basename. A ghost from src/a/index.ts merges into THAT file's AST node (a_render), never diff --git a/tests/test_export.py b/tests/test_export.py index d957b87957..7f344f684e 100644 --- a/tests/test_export.py +++ b/tests/test_export.py @@ -960,6 +960,30 @@ def test_to_json_refuses_shrink(tmp_path): assert to_json(_mkG(2), {}, str(p), force=True) is True # force overrides +def test_to_json_allows_dedup_explained_shrink(tmp_path): + """#1847: a cluster-only re-run that reloads graph.json through + build_from_json legitimately collapses a duplicate node (e.g. a + manifest package node merged into its AST-canonical twin, same + (source_file, label)). build_from_json records that collapse count on + `G.graph["_ghost_dedup_count"]`; the #479 shrink guard must let a drop + fully explained by it through instead of refusing.""" + p = tmp_path / "graph.json" + json.dump({"nodes": [{"id": f"n{i}"} for i in range(5)]}, p.open("w")) + G = _mkG(4) + G.graph["_ghost_dedup_count"] = 1 # exactly explains the 5 -> 4 drop + assert to_json(G, {}, str(p), force=False) is True + + +def test_to_json_still_refuses_shrink_beyond_dedup_count(tmp_path): + """The dedup allowance only excuses the EXPLAINED portion of a drop — a + further, unexplained loss on top of it must still refuse.""" + p = tmp_path / "graph.json" + json.dump({"nodes": [{"id": f"n{i}"} for i in range(5)]}, p.open("w")) + G = _mkG(2) # drop of 3, but only 1 is dedup-explained + G.graph["_ghost_dedup_count"] = 1 + assert to_json(G, {}, str(p), force=False) is False + + def test_to_json_fails_safe_on_corrupt_existing(tmp_path): """A non-empty but unparseable existing graph.json (corrupt or mid-write) must NOT be silently overwritten — we can't verify the new graph isn't a