fix(build): make ghost-node merge deterministic across process runs (#1753)
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) <noreply@anthropic.com>
This commit is contained in:
committed by
safishamsi
co-authored by
Claude Opus 4.8
parent
9c27a52448
commit
29a6954f91
@@ -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)
|
||||
|
||||
+24
-4
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user