diff --git a/graphify/build.py b/graphify/build.py index 8375eecd..9acf342b 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -625,9 +625,9 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat # populates source_location, so those ghosts survived. Extended fix: use # _origin=="ast" as the canonical signal. AST nodes always win; any non-AST # node sharing (basename, label) with an AST node is a ghost. - _loc_nodes: dict[tuple[str, str], str] = {} # (basename, label) -> canonical node id + _loc_nodes: dict[tuple[str, str], str] = {} # (source_file, label) -> canonical node id _loc_collisions: set[tuple[str, str]] = set() # keys shared by 2+ AST nodes - _noloc_nodes: dict[tuple[str, str], str] = {} # (basename, label) -> ghost node id + _noloc_nodes: dict[tuple[str, str], str] = {} # (source_file, label) -> ghost node id # Pass 1: collect canonical nodes — AST-origin nodes take precedence over LLM nodes. # When 2+ AST nodes share a key (same-named symbols in same-named files across @@ -643,36 +643,31 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat attrs = G.nodes[nid] label = str(attrs.get("label", "")).strip() sf = str(attrs.get("source_file", "")) - basename = Path(sf).name if sf else "" - if not label or not basename: + if not label or not sf: continue is_ast = attrs.get("_origin") == "ast" if attrs.get("source_location") or is_ast: - key = (basename, label) + # Key on the FULL normalized source_file, not the bare basename + # (#2068): the AST/LLM ghost twins of #1145 always share the same + # source_file (different ids, same file), so full-path keying still + # collapses them, while unrelated same-basename nodes in DIFFERENT + # directories (docs/a/index.md vs docs/b/index.md) now get distinct + # keys and are never falsely merged. This subsumes the #1753/#1257 + # cross-file ambiguity guard, which is why the non-AST branch below + # no longer needs it. + key = (sf, label) if is_ast: - # Two AST nodes on the same key is an ambiguous collision. + # Two AST nodes on the same key (same file, same label) is an + # ambiguous collision. if key in _loc_nodes and G.nodes[_loc_nodes[key]].get("_origin") == "ast": _loc_collisions.add(key) # AST-origin nodes always overwrite a prior non-AST entry. _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) + # First non-AST node for this (file, label) wins as canonical; a + # later same-key node is a genuine same-file duplicate and still + # collapses in Pass 2. + _loc_nodes.setdefault(key, nid) # Pass 2: find ghosts — non-AST nodes that have an AST canonical twin. for nid in sorted(node_set): @@ -681,10 +676,9 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat continue # AST nodes are never ghosts label = str(attrs.get("label", "")).strip() sf = str(attrs.get("source_file", "")) - basename = Path(sf).name if sf else "" - if not label or not basename: + if not label or not sf: continue - key = (basename, label) + key = (sf, label) if key in _loc_collisions: continue # ambiguous key: no safe canonical winner, leave ghost intact if key in _loc_nodes and _loc_nodes[key] != nid: diff --git a/tests/test_build.py b/tests/test_build.py index 9e470ad6..3d8582c3 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -270,10 +270,12 @@ def test_ghost_merge_unique_located_node_still_merges(): assert G.has_edge("caller", "ast_render") -def test_ghost_merge_skipped_on_basename_collision(): - """#1257: when two files with the same basename both define a symbol with the - same label, the (basename, label) key is ambiguous and the semantic ghost - must not be merged into an arbitrary one of them.""" +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 + the unrelated same-basename b_render in src/b/index.ts. (Pre-#2068 the + (basename, label) key made ('index.ts','render') ambiguous and skipped the + merge; the directory-aware key resolves it precisely.)""" ext = { "nodes": [ {"id": "a_render", "label": "render", "file_type": "code", @@ -290,13 +292,36 @@ def test_ghost_merge_skipped_on_basename_collision(): "input_tokens": 0, "output_tokens": 0, } G = build_from_json(ext) - # The ghost survives: merging it into either a_render or b_render would - # pick an arbitrary winner (set iteration order over node_set). - assert "ghost_render" in G.nodes() - assert G.number_of_nodes() == 4 - assert G.has_edge("caller", "ghost_render") - assert not G.has_edge("caller", "a_render") + # Ghost merges into its same-file twin; the edge re-points to a_render only. + assert "ghost_render" not in G.nodes() + assert G.has_edge("caller", "a_render") assert not G.has_edge("caller", "b_render") + # The unrelated same-basename node in another directory is untouched. + assert "b_render" in G.nodes() + + +def test_ghost_merge_not_across_directories_same_basename(): + """#2068: two unrelated non-AST nodes with the same basename+label in + DIFFERENT directories must NOT be merged onto one survivor (the bug: bare + basename collapsed docs/product_a/index.md and docs/product_b/index.md).""" + ext = { + "nodes": [ + {"id": "docs_a_index", "label": "Quickstart", "file_type": "document", + "source_file": "docs/product_a/index.md", "source_location": "L1"}, + {"id": "docs_b_index", "label": "Quickstart", "file_type": "document", + "source_file": "docs/product_b/index.md"}, + {"id": "docs_hub", "label": "Docs", "file_type": "concept", + "source_file": "docs/hub.md", "source_location": "L1"}, + ], + "edges": [{"source": "docs_hub", "target": "docs_b_index", "relation": "links_to", + "confidence": "INFERRED", "source_file": "docs/hub.md"}], + "input_tokens": 0, "output_tokens": 0, + } + G = build_from_json(ext, directed=False) + # Both docs survive; the edge stays on the file it was authored against. + assert "docs_a_index" in G.nodes() and "docs_b_index" in G.nodes() + assert G.has_edge("docs_hub", "docs_b_index") + assert not G.has_edge("docs_hub", "docs_a_index") def test_ghost_merge_non_ast_different_files_both_survive():