From 29a6954f91560352f9f89a0c98f868b3e8631b6b Mon Sep 17 00:00:00 2001 From: erasmust-dotcom Date: Thu, 9 Jul 2026 23:32:54 +0100 Subject: [PATCH] fix(build): make ghost-node merge deterministic across process runs (#1753) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit build_from_json's ghost-node merge iterated set(G.nodes()), so when two nodes shared a (basename, label) key the "canonical" survivor was chosen by CPython's per-process string-hash order — rebuilding the same extraction JSON in a fresh process could pick a different survivor, silently changing which node id represents a concept. That breaks any workflow persisting ids across a rebuild; concretely it broke the cluster->relabel step (community membership referenced an id that the second build merged away -> KeyError in report generation). Two changes: - Pass 1 and Pass 2 now iterate sorted(node_set), not set(node_set), the same deterministic-order fix the edge loop below already uses on purpose. - The #1257 ambiguity guard is extended to the case it did not cover: two NON-AST nodes sharing a key but from DIFFERENT source files are distinct concepts, not an AST ghost/canonical twin, so the key is marked ambiguous and both survive rather than one arbitrarily merging away (data loss). A genuine same-file duplicate (identical source_file) is not flagged and still collapses to one node. Reported with a precise root-cause, minimal repro, and real-world impact by @erasmust-dotcom. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 2 ++ graphify/build.py | 28 ++++++++++++++++++++++++---- tests/test_build.py | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c85a71..5fc023f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ Full release notes with details on each version: [GitHub Releases](https://githu ## 0.9.12 (unreleased) +- Fix: `build_from_json` is deterministic across process runs again (#1753, thanks @erasmust-dotcom). The ghost-node merge iterated `set(G.nodes())`, so which node survived a `(basename, label)` collision depended on CPython's per-process string-hash seed — rebuilding the same extraction JSON in a fresh process could silently pick a different canonical id (breaking the cluster→relabel workflow with a `KeyError` on an id that vanished). The Pass 1/Pass 2 loops now iterate in sorted order. Additionally, two non-AST (semantic) nodes sharing a key but from *different* files are now treated as distinct concepts and both survive (mirroring the AST/AST ambiguity guard #1257) instead of one arbitrarily merging away; a genuine same-file duplicate still collapses. + - Fix: a Java field/parameter/return-type reference to a class whose simple name is shared by two modules no longer dangles on a sourceless phantom node (#1744, thanks @aviciot). Both same-named classes already survive as distinct path-scoped nodes, but the cross-module `references` edge was left pointing at a bare no-source stub because `_resolve_java_type_references` re-pointed `implements`/`inherits`/`imports` but not `references` — so a query about the referenced class could miss it. The Java resolver now disambiguates `references` by the importing file's `import` statement (falling back to same-package), mirroring the C# resolver, and drops the orphaned phantom. ## 0.9.11 (2026-07-08) diff --git a/graphify/build.py b/graphify/build.py index f0676a8..8ae6e78 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -449,7 +449,11 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat # ghost would pick an arbitrary winner via set-iteration order (#1257). Track # those keys so Pass 2 skips them — same conservatism as # _rewire_unique_stub_nodes, which only merges when exactly one real def exists. - for nid in node_set: + # Iterate in a deterministic (sorted) order, not set-iteration order, so the + # canonical winner and the ambiguity decisions below don't flip run-to-run + # with CPython's per-process string-hash seed (#1753) — the same reason the + # edge-iteration loop further down sorts on purpose. + for nid in sorted(node_set): attrs = G.nodes[nid] label = str(attrs.get("label", "")).strip() sf = str(attrs.get("source_file", "")) @@ -465,11 +469,27 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat _loc_collisions.add(key) # AST-origin nodes always overwrite a prior non-AST entry. _loc_nodes[key] = nid - elif key not in _loc_nodes: - _loc_nodes[key] = nid + else: + existing = _loc_nodes.get(key) + if existing is None: + _loc_nodes[key] = nid + elif ( + G.nodes[existing].get("_origin") != "ast" + and str(G.nodes[existing].get("source_file", "")) != sf + ): + # Two NON-AST nodes sharing (basename, label) but coming from + # DIFFERENT files are distinct concepts (e.g. a same-named + # concept in dir_a/update.md and dir_b/update.md), not an AST + # ghost/canonical twin. Merging them would drop a real node + # and pick the survivor arbitrarily via iteration order + # (#1753). Mark the key ambiguous so Pass 2 leaves both, the + # same conservatism the AST/AST case uses (#1257). A genuine + # same-file duplicate (identical source_file) is not flagged + # and still collapses. + _loc_collisions.add(key) # Pass 2: find ghosts — non-AST nodes that have an AST canonical twin. - for nid in node_set: + for nid in sorted(node_set): attrs = G.nodes[nid] if attrs.get("_origin") == "ast": continue # AST nodes are never ghosts diff --git a/tests/test_build.py b/tests/test_build.py index e8c0309..5d2e2df 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -257,6 +257,41 @@ def test_ghost_merge_skipped_on_basename_collision(): assert not G.has_edge("caller", "b_render") +def test_ghost_merge_non_ast_different_files_both_survive(): + """#1753: two NON-AST (semantic) nodes sharing (basename, label) but from + DIFFERENT files are distinct concepts with no AST canonical twin. They must + not be merged into an arbitrary survivor (which flipped run-to-run with the + hash seed); both survive, mirroring the AST/AST guard (#1257).""" + ext = { + "nodes": [ + {"id": "dir_a_update_build_merge", "label": "build_merge() function", + "file_type": "concept", "source_file": "dir_a/update.md", "source_location": "L10"}, + {"id": "dir_b_update_build_merge", "label": "build_merge() function", + "file_type": "concept", "source_file": "dir_b/update.md", "source_location": "L12"}, + ], + "edges": [], + } + G = build_from_json(ext, directed=False) + assert sorted(G.nodes()) == ["dir_a_update_build_merge", "dir_b_update_build_merge"] + + +def test_ghost_merge_non_ast_same_file_still_merges(): + """A genuine duplicate — two non-AST nodes with the SAME source_file and + label — is a real ghost and still collapses to one node (deterministically), + so #1753's fix doesn't leave same-file LLM duplicates behind.""" + ext = { + "nodes": [ + {"id": "a_foo", "label": "Foo", "file_type": "concept", + "source_file": "x/doc.md", "source_location": "L1"}, + {"id": "b_foo", "label": "Foo", "file_type": "concept", + "source_file": "x/doc.md", "source_location": "L2"}, + ], + "edges": [], + } + G = build_from_json(ext, directed=False) + assert G.number_of_nodes() == 1 + + def test_build_merge_preserves_call_edge_direction(tmp_path): """Regression for #760.