From 0d5bdfbf87dafb95adb18fa4815b0803dae2d82a Mon Sep 17 00:00:00 2001 From: Aromal Biju Date: Thu, 20 Aug 2026 00:01:51 +0530 Subject: [PATCH 1/2] =?UTF-8?q?fix(#2873):=20mint=20stub=20nodes=20for=20u?= =?UTF-8?q?ndeclared=20edge=20endpoints,=20prune=20manifest=20deps,=20fix?= =?UTF-8?q?=20merge=20repos-union=20clobbering=20-=20extract.py:=20mint=20?= =?UTF-8?q?external=20stub=20nodes=20for=20import=20edges=20pointing=20at?= =?UTF-8?q?=20=20=20undeclared=20endpoints=20(stdlib/third-party);=20manif?= =?UTF-8?q?est/package=20deps=20=20=20(depends=5Fon/requires,=20pkg=5F*=20?= =?UTF-8?q?ids)=20keep=20prior=20prune-not-fabricate=20behavior=20-=20extr?= =?UTF-8?q?act.py:=20include=20resolution=5Fcontext=5Fnodes=20when=20compu?= =?UTF-8?q?ting=20declared=20ids,=20=20=20so=20an=20incremental=20rebuild'?= =?UTF-8?q?s=20cross-file-resolved-but-not-locally-extracted=20=20=20targe?= =?UTF-8?q?ts=20(e.g.=20an=20unchanged=20file's=20function)=20aren't=20wro?= =?UTF-8?q?ngly=20treated=20as=20=20=20undeclared=20and=20stub-shadowed=20?= =?UTF-8?q?=E2=80=94=20this=20was=20silently=20corrupting=204=20=20=20incr?= =?UTF-8?q?emental-rebuild=20test=20cases=20(#2406/#2437/#2438=20symptoms,?= =?UTF-8?q?=20actually=20=20=20caused=20by=20this=20interaction)=20-=20cli?= =?UTF-8?q?.py:=20accumulate=20external=20node=20'repos'=20as=20a=20union?= =?UTF-8?q?=20across=20merge-graphs=20=20=20inputs=20instead=20of=20lettin?= =?UTF-8?q?g=20nx.compose's=20dict.update=20clobber=20it=20to=20the=20=20?= =?UTF-8?q?=20last=20repo's=20value=20-=20tests:=20strengthen=20csharp=20i?= =?UTF-8?q?mport-edge=20assertions=20now=20that=20stub=20nodes=20=20=20res?= =?UTF-8?q?olve;=20add=20coverage=20for=20the=20repos-union=20fix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- graphify/build.py | 25 +++++++++++++++--- graphify/cli.py | 13 ++++++++++ graphify/extract.py | 39 ++++++++++++++++++++++++++++ tests/test_csharp_type_resolution.py | 6 ++--- tests/test_merge_graphs_cli.py | 30 +++++++++++++++++++++ 5 files changed, 107 insertions(+), 6 deletions(-) diff --git a/graphify/build.py b/graphify/build.py index 8efdcbd6e2..b5fec12754 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -1608,6 +1608,8 @@ def _dropped(item: dict) -> bool: # re-extracted its source. Hyperedges are semantic-tier (no _origin, # null source_location), so an AST-only re-extract carries them. # Deletion pruning below stays tier-blind. + if item.get("external"): + return False # external stub nodes/edges (#2873): never carried-forward-drop, never replace-drop own = new_ast_sources if _is_ast_tier(item) else new_sem_sources if sf in own or _norm_source_file(sf, _eff_root) in own: return True # re-extracted this run — replaced by the new chunk @@ -1987,12 +1989,29 @@ def prefix_graph_for_global(G: nx.Graph, repo_tag: str) -> nx.Graph: is added to each node so the original ID can be recovered. Edges and their directional attributes (_src/_tgt) are rewritten to match the new prefixed IDs. The 'repo' attribute is set on every node. + + Nodes marked ``external`` (stdlib/third-party references materialized by + the #2873 dangling-edge-endpoint pass) are deliberately NOT namespaced: + they are not repo-local ids that happen to collide, they are external + identifiers that arrived through a repo-local path, so `typing` from repoA + and `typing` from repoB are the same node. Leaving the id bare lets + nx.compose unify them across inputs instead of manufacturing one + `repoX::typing` per repo (#2873). A 'repos' list (not the singular + 'repo') accumulates every repo tag that referenced the node, since an + external node can now be shared by several inputs. """ - relabel = {n: f"{repo_tag}::{n}" for n in G.nodes} + relabel = { + n: f"{repo_tag}::{n}" + for n, data in G.nodes(data=True) + if not data.get("external") + } H = nx.relabel_nodes(G, relabel, copy=True) for node, data in H.nodes(data=True): - data["repo"] = repo_tag - data.setdefault("local_id", node.split("::", 1)[1]) + if data.get("external"): + data["repos"] = sorted(set(data.get("repos", [])) | {repo_tag}) + else: + data["repo"] = repo_tag + data.setdefault("local_id", node.split("::", 1)[1]) for u, v, data in H.edges(data=True): if "_src" in data and data["_src"] in relabel: data["_src"] = relabel[data["_src"]] diff --git a/graphify/cli.py b/graphify/cli.py index cb30420473..c288d42853 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -2399,12 +2399,25 @@ def _to_simple(g: "_nx.Graph") -> "_nx.Graph": # diagnosis in PR #1691). Collect every input's prefixed hyperedges # and re-attach the union after composing. collected_hyperedges: list = [] + # Same clobbering hazard as the hyperedge list (#2484) hits the 'repos' + # attribute prefix_graph_for_global puts on shared, un-namespaced + # external nodes (#2873): nx.compose merges shared-node attrs with + # dict.update, so each pass's 'repos' list overwrites the previous + # one instead of unioning with it. Accumulate the union ourselves and + # reattach it after composing, same shape as collected_hyperedges. + external_repos: dict[str, set] = {} for G, repo_tag in zip(graphs, repo_tags): prefixed = _to_simple(_prefix(G, repo_tag)) hes = prefixed.graph.get("hyperedges") if isinstance(hes, list): collected_hyperedges.extend(h for h in hes if isinstance(h, dict)) + for nid, data in prefixed.nodes(data=True): + if data.get("external"): + external_repos.setdefault(nid, set()).update(data.get("repos", ())) merged = _nx.compose(merged, prefixed) + for nid, repos in external_repos.items(): + if nid in merged.nodes: + merged.nodes[nid]["repos"] = sorted(repos) # Drop whatever compose left behind (the last input's list, possibly # with internal duplicates) so attach_hyperedges dedups the full # collection by id from a clean slate. diff --git a/graphify/extract.py b/graphify/extract.py index 9afb543198..2d756fb7dd 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -6701,6 +6701,45 @@ def _canon(nid: str) -> str: if e.get("target"): e["target"] = _canon(e["target"]) + # Every resolution/repoint/rewire/disambiguation pass above has now had its + # chance to land an edge endpoint on a real node id. Anything still not in + # the node set at this point is not a bug in one particular resolver — it + # is a genuine reference to something outside the graph (an unimported + # stdlib/third-party module, an unresolved dotted path, ...). Leaving it + # as a bare edge endpoint is not a neutral encoding of "outside the + # graph": every consumer that builds a graph object from nodes+edges + # (networkx included) materializes it as an attribute-less phantom node, + # so the file's declared node set and its produced node set disagree + # (#2873). Mint a minimal, explicitly-marked stub for each so they agree. + # `external: True` distinguishes these from ordinary file/symbol nodes — + # merge-graphs reads it to merge same-named external references across + # repos instead of namespacing them like repo-local ids (#2873). + _declared_ids = {n.get("id") for n in all_nodes if isinstance(n, dict)} + if resolution_context_nodes: + _declared_ids |= { + n.get("id") for n in resolution_context_nodes if isinstance(n, dict) + } + _external_stubs: dict[str, dict] = {} + for e in all_edges: + if not isinstance(e, dict): + continue + tgt = e.get("target") + if not tgt or tgt in _declared_ids or tgt in _external_stubs: + continue + if e.get("relation") in ("depends_on", "requires") or str(tgt).startswith("pkg_"): + continue # manifest/package deps: keep prior prune-not-fabricate behavior (#2873) + _external_stubs[tgt] = { + "id": tgt, + "label": tgt, + "file_type": "code", + "type": "module", + "confidence": "INFERRED", + "external": True, + "source_file": None, + } + if _external_stubs: + all_nodes.extend(_external_stubs.values()) + # origin_file is an internal disambiguation hint (#1462): the colliding-id pass # above reads it to keep same-named cross-file stubs distinct, after which nothing # consumes it. Drop it from the returned nodes so it never ships into graph.json as diff --git a/tests/test_csharp_type_resolution.py b/tests/test_csharp_type_resolution.py index 694a491d20..b727a03f8f 100644 --- a/tests/test_csharp_type_resolution.py +++ b/tests/test_csharp_type_resolution.py @@ -252,7 +252,7 @@ def test_csharp_import_edges_resolve_internal_namespace_and_alias(tmp_path: Path (kind, fqn, target.get("type") if target else None) for kind, fqn, target in imports ] - assert ("namespace", "UnityEngine", None) in [ + assert ("namespace", "UnityEngine", "module") in [ (kind, fqn, target.get("type") if target else None) for kind, fqn, target in imports ] @@ -260,11 +260,11 @@ def test_csharp_import_edges_resolve_internal_namespace_and_alias(tmp_path: Path (kind, fqn, target.get("label") if target else None) for kind, fqn, target in imports ] - assert ("alias", "System.Math", None) in [ + assert ("alias", "System.Math", "system_math") in [ (kind, fqn, target.get("label") if target else None) for kind, fqn, target in imports ] - assert ("static", "Game.Core.Damage", None) in [ + assert ("static", "Game.Core.Damage", "game_core_damage") in [ (kind, fqn, target.get("label") if target else None) for kind, fqn, target in imports ] diff --git a/tests/test_merge_graphs_cli.py b/tests/test_merge_graphs_cli.py index e0203a5a39..990a48eb16 100644 --- a/tests/test_merge_graphs_cli.py +++ b/tests/test_merge_graphs_cli.py @@ -247,3 +247,33 @@ def test_merge_graphs_reads_top_level_only_hyperedges(tmp_path): assert [h["id"] for h in data["hyperedges"]] == ["alpha::h_top"] assert data["hyperedges"][0]["nodes"] == ["alpha::x"] +def test_merge_graphs_external_node_repos_union_not_clobbered(tmp_path): + # #2873: prefix_graph_for_global leaves `external` nodes (unresolved + # imports) unprefixed so nx.compose unifies them across repos, tagging + # each with a `repos` list. nx.compose merges shared-node attrs with + # dict.update, so each pass's `repos` list overwrote the previous one — + # only the LAST repo survived on a 3-way merge. merge-graphs now + # accumulates the union itself and reattaches it after composing. + a = tmp_path / "alpha" / "graphify-out" / "graph.json" + b = tmp_path / "beta" / "graphify-out" / "graph.json" + c = tmp_path / "gamma" / "graphify-out" / "graph.json" + for p, local_id in ((a, "a_mod"), (b, "b_mod"), (c, "c_mod")): + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(json.dumps({ + "directed": False, "multigraph": False, "graph": {}, + "nodes": [ + {"id": local_id}, + {"id": "typing", "label": "typing", "type": "module", + "external": True, "repos": []}, + ], + "links": [{"source": local_id, "target": "typing"}], + })) + out = tmp_path / "merged.json" + r = _run(["merge-graphs", str(a), str(b), str(c), "--out", str(out)], tmp_path) + assert r.returncode == 0, r.stderr + data = json.loads(out.read_text()) + ext_nodes = [n for n in data["nodes"] if n.get("id") == "typing"] + assert len(ext_nodes) == 1, f"external node fragmented: {ext_nodes}" + assert sorted(ext_nodes[0].get("repos", [])) == ["alpha", "beta", "gamma"], ( + f"repos union clobbered: {ext_nodes[0].get('repos')}" + ) \ No newline at end of file From 3c739a04f487c8aebfdc6695c4eb7ea891093b5e Mon Sep 17 00:00:00 2001 From: Aromal Biju Date: Thu, 20 Aug 2026 18:13:38 +0530 Subject: [PATCH 2/2] fix: prune_repo_from_graph now correctly handles multi-repo external nodes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bot review on #2878 flagged that external nodes (#2873) never carry a 'repo' attribute, only 'repos' — so prune_repo_from_graph's repo==repo_tag filter silently skipped them, leaving orphaned external stubs behind after the last referencing repo was pruned. Now removes the repo tag from 'repos' and only drops the node once no repo remains. --- graphify/build.py | 18 ++++++++++++++++-- tests/test_global_graph.py | 16 ++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/graphify/build.py b/graphify/build.py index b5fec12754..1bd75443e2 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -2071,7 +2071,21 @@ def distinct_repo_tags(graph_paths: "list[Path]") -> "list[str]": def prune_repo_from_graph(G: nx.Graph, repo_tag: str) -> int: - """Remove all nodes tagged with repo_tag from G in-place. Returns count removed.""" - to_remove = [n for n, d in G.nodes(data=True) if d.get("repo") == repo_tag] + """Remove all nodes tagged with repo_tag from G in-place. Returns count removed. + + External nodes (#2873) can be referenced by multiple repos via their + 'repos' list -- pruning one repo only removes the tag; the node itself + is removed only once no repo still references it. + """ + to_remove = [] + for n, d in G.nodes(data=True): + if d.get("repo") == repo_tag: + to_remove.append(n) + elif d.get("external") and repo_tag in d.get("repos", ()): + remaining = [r for r in d["repos"] if r != repo_tag] + if remaining: + d["repos"] = remaining + else: + to_remove.append(n) G.remove_nodes_from(to_remove) return len(to_remove) diff --git a/tests/test_global_graph.py b/tests/test_global_graph.py index 389c88d260..9baee522ee 100644 --- a/tests/test_global_graph.py +++ b/tests/test_global_graph.py @@ -104,6 +104,22 @@ def test_prune_repo_returns_zero_if_not_present(): assert G.number_of_nodes() == 1 +def test_prune_repo_preserves_shared_external_node_until_last_repo(): + from graphify.build import prune_repo_from_graph + G = nx.Graph() + G.add_node("repoA::userservice", repo="repoA", label="UserService") + G.add_node("typing", external=True, repos=["repoA", "repoB"], label="typing") + + removed = prune_repo_from_graph(G, "repoA") + assert removed == 1 # only repoA::userservice; typing still referenced by repoB + assert "typing" in G.nodes + assert G.nodes["typing"]["repos"] == ["repoB"] + + removed = prune_repo_from_graph(G, "repoB") + assert removed == 1 + assert "typing" not in G.nodes + + # ── global_graph.py ─────────────────────────────────────────────────────────── def test_global_add_creates_global_graph(tmp_path):