Skip to content

Commit fe905e2

Browse files
seonghobaeclaude
andcommitted
fix: excuse dedup-explained node drops from the #479 shrink guard
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BXYRNTJhYLB2bYveuaFe74
1 parent c7ec108 commit fe905e2

4 files changed

Lines changed: 74 additions & 12 deletions

File tree

‎graphify/build.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1113,6 +1113,13 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat
11131113
G.remove_node(ghost_id)
11141114
node_set.discard(ghost_id)
11151115

1116+
# #1847: record how many nodes this canonicalization pass legitimately
1117+
# collapsed (AST/manifest duplicates sharing (source_file, label)), so a
1118+
# downstream shrink guard (export.to_json) can tell a merge-explained node
1119+
# count drop from an unexplained data-loss shrink instead of refusing both
1120+
# alike.
1121+
G.graph["_ghost_dedup_count"] = G.graph.get("_ghost_dedup_count", 0) + len(_ghost_remap)
1122+
11161123
# Normalized ID map: lets edges survive when the LLM generates IDs with
11171124
# slightly different casing or punctuation than the AST extractor.
11181125
# e.g. "Session_ValidateToken" maps to "session_validatetoken".

‎graphify/export.py‎

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -312,18 +312,31 @@ def to_json(G: nx.Graph, communities: dict[int, list[str]], output_path: str, *,
312312
return False
313313
new_n = G.number_of_nodes()
314314
if new_n < existing_n:
315-
import sys as _sys
316-
print(
317-
f"[graphify] WARNING: new graph has {new_n} nodes but existing "
318-
f"graph.json has {existing_n} (net -{existing_n - new_n}). "
319-
f"Refusing to overwrite. Possible causes: missing chunk files from "
320-
f"a previous session, or fuzzy dedup collapsed same-named symbols "
321-
f"across files during an --update on an already-current graph. "
322-
f"Run a full rebuild (/graphify .) to be safe, or pass force=True "
323-
f"only if you have verified the reduction is legitimate.",
324-
file=_sys.stderr,
325-
)
326-
return False
315+
# #1847: build_from_json legitimately collapses duplicate
316+
# nodes that resolve to the same (source_file, label) — e.g. a
317+
# manifest-derived package node and its AST-canonical twin
318+
# (crate:foo vs pkg_foo). That merge is recorded on the graph
319+
# as `_ghost_dedup_count`. When the observed drop is fully
320+
# explained by that count, it is a known-safe canonicalization,
321+
# not the unverified data-loss shrink #479 guards against —
322+
# proceed instead of refusing.
323+
dedup = int(getattr(G, "graph", {}).get("_ghost_dedup_count", 0) or 0)
324+
unexplained = (existing_n - new_n) - dedup
325+
if unexplained > 0:
326+
import sys as _sys
327+
print(
328+
f"[graphify] WARNING: new graph has {new_n} nodes but existing "
329+
f"graph.json has {existing_n} (net -{existing_n - new_n}, "
330+
f"{dedup} explained by duplicate-node canonicalization, "
331+
f"{unexplained} unexplained). "
332+
f"Refusing to overwrite. Possible causes: missing chunk files from "
333+
f"a previous session, or fuzzy dedup collapsed same-named symbols "
334+
f"across files during an --update on an already-current graph. "
335+
f"Run a full rebuild (/graphify .) to be safe, or pass force=True "
336+
f"only if you have verified the reduction is legitimate.",
337+
file=_sys.stderr,
338+
)
339+
return False
327340

328341
node_community = _node_community_map(communities)
329342
_labels: dict[int, str] = {int(k): v for k, v in (community_labels or {}).items()}

‎tests/test_build.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,24 @@ def test_ghost_merge_unique_located_node_still_merges():
488488
assert G.has_edge("caller", "ast_render")
489489

490490

491+
def test_ghost_merge_records_dedup_count():
492+
"""#1847: each ghost collapsed into its AST canonical twin is a legitimate
493+
node-count reduction, not data loss. build_from_json must record how many
494+
were merged so a downstream shrink guard can tell the two apart instead of
495+
refusing to overwrite a graph.json that only shrank via this dedup."""
496+
ext = {
497+
"nodes": [
498+
{"id": "ast_render", "label": "render", "file_type": "code",
499+
"source_file": "src/app/index.ts", "source_location": "L10", "_origin": "ast"},
500+
{"id": "ghost_render", "label": "render", "file_type": "code",
501+
"source_file": "src/app/index.ts"},
502+
],
503+
"edges": [], "input_tokens": 0, "output_tokens": 0,
504+
}
505+
G = build_from_json(ext)
506+
assert G.graph.get("_ghost_dedup_count") == 1
507+
508+
491509
def test_ghost_merge_uses_source_file_not_basename():
492510
"""#2068: the ghost-merge key is the full source_file, not the bare basename.
493511
A ghost from src/a/index.ts merges into THAT file's AST node (a_render), never

‎tests/test_export.py‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -960,6 +960,30 @@ def test_to_json_refuses_shrink(tmp_path):
960960
assert to_json(_mkG(2), {}, str(p), force=True) is True # force overrides
961961

962962

963+
def test_to_json_allows_dedup_explained_shrink(tmp_path):
964+
"""#1847: a cluster-only re-run that reloads graph.json through
965+
build_from_json legitimately collapses a duplicate node (e.g. a
966+
manifest package node merged into its AST-canonical twin, same
967+
(source_file, label)). build_from_json records that collapse count on
968+
`G.graph["_ghost_dedup_count"]`; the #479 shrink guard must let a drop
969+
fully explained by it through instead of refusing."""
970+
p = tmp_path / "graph.json"
971+
json.dump({"nodes": [{"id": f"n{i}"} for i in range(5)]}, p.open("w"))
972+
G = _mkG(4)
973+
G.graph["_ghost_dedup_count"] = 1 # exactly explains the 5 -> 4 drop
974+
assert to_json(G, {}, str(p), force=False) is True
975+
976+
977+
def test_to_json_still_refuses_shrink_beyond_dedup_count(tmp_path):
978+
"""The dedup allowance only excuses the EXPLAINED portion of a drop — a
979+
further, unexplained loss on top of it must still refuse."""
980+
p = tmp_path / "graph.json"
981+
json.dump({"nodes": [{"id": f"n{i}"} for i in range(5)]}, p.open("w"))
982+
G = _mkG(2) # drop of 3, but only 1 is dedup-explained
983+
G.graph["_ghost_dedup_count"] = 1
984+
assert to_json(G, {}, str(p), force=False) is False
985+
986+
963987
def test_to_json_fails_safe_on_corrupt_existing(tmp_path):
964988
"""A non-empty but unparseable existing graph.json (corrupt or mid-write)
965989
must NOT be silently overwritten — we can't verify the new graph isn't a

0 commit comments

Comments
 (0)