diff --git a/graphify/build.py b/graphify/build.py index 861a92855..e5ca2cdef 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -63,6 +63,79 @@ def _is_ast_tier(item: dict) -> bool: # this collapse actually needs. _GENERIC_RELATIONS: frozenset[str] = frozenset({"references", "uses", "mentions"}) +# Import-family relations whose target may legitimately be a module OUTSIDE the +# graph (stdlib, a third-party dependency, another repo). Historically the edge +# to such a target was dropped, which left the in-memory graph clean but let the +# on-disk graph.json (written by the incremental update path) keep the edge with +# no matching node — an undeclared endpoint every loader materialises as an +# attribute-less phantom (#2873). For these relations we instead mint a typed +# external stub node so every edge endpoint resolves. Deliberately NOT `calls`: +# a sourceless external call target is suppressed on purpose (#3156) to avoid a +# phantom god-node, and that policy is unchanged here. +_EXTERNAL_STUB_RELATIONS: frozenset[str] = frozenset( + {"imports", "imports_from", "re_exports"} +) + + +def _mint_external_stub(G: "nx.Graph", node_set: set, nid: str) -> None: + """Add a leaf node for an external import target so the edge is not dangling. + + The node is tagged ``external`` (and ``file_type='concept'`` so the schema + validator and community/report code treat it as a non-source concept rather + than warning on a missing ``file_type``). ``merge-graphs`` reads the + ``external`` flag to keep these ids global instead of namespacing them per + repo, so the same stdlib/dependency module unifies across repos (#2873). + """ + if nid in node_set: + return + G.add_node( + nid, + label=nid, + file_type="concept", + type="external", + external=True, + source_file="", + ) + node_set.add(nid) + + +def mint_external_stubs_in_data(data: dict) -> None: + """Mint external stub nodes for import-family links with an undeclared target. + + The ``--no-cluster`` / incremental write path serializes the raw merged + extraction directly rather than going through :func:`build_from_json`, so it + needs the same stubbing to keep the on-disk graph.json free of undeclared + edge endpoints (#2873). Idempotent: a graph already carrying its stubs (from + a prior run or the clustered path) is left unchanged. + """ + nodes = data.get("nodes") + links = data.get("links") + if not isinstance(links, list): + links = data.get("edges") + if not isinstance(nodes, list) or not isinstance(links, list): + return + declared = {n.get("id") for n in nodes if isinstance(n, dict)} + minted: set[str] = set() + for e in links: + if not isinstance(e, dict) or e.get("relation") not in _EXTERNAL_STUB_RELATIONS: + continue + src, tgt = e.get("source"), e.get("target") + if ( + src in declared + and isinstance(tgt, str) + and tgt not in declared + and tgt not in minted + ): + nodes.append({ + "id": tgt, + "label": tgt, + "file_type": "concept", + "type": "external", + "external": True, + "source_file": "", + }) + minted.add(tgt) + # Language interop families, keyed by extension, for the cross-language phantom-edge # guard in the edge loop below. Families group by REAL interop (JS/TS share a module # graph; C/C++/ObjC share a compilation unit via headers; JVM langs share bytecode), @@ -1214,7 +1287,23 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat if tgt not in node_set: tgt = norm_to_id.get(_normalize_id(tgt), tgt) if src not in node_set or tgt not in node_set: - continue # skip edges to external/stdlib nodes - expected, not an error + # An import/re-export whose target is not a declared node points at + # an external module (stdlib / third-party / another repo). Mint a + # typed external stub so the on-disk graph.json has no undeclared + # endpoint (#2873), rather than dropping the edge and leaving loaders + # to materialise an attribute-less phantom. Only the missing *target* + # of an import-family edge is stubbed; a dangling source, or a + # missing endpoint on any other relation (e.g. a sourceless external + # call target, suppressed by #3156), is still dropped. + if ( + edge.get("relation") in _EXTERNAL_STUB_RELATIONS + and src in node_set + and tgt not in node_set + and isinstance(tgt, str) + ): + _mint_external_stub(G, node_set, tgt) + else: + continue # external/stdlib target on a non-import relation - expected # `target_file` is a transient import-disambiguation salt hint (#1814) # with no downstream reader; it holds an absolute path, so it must never # be persisted. Disambiguation already pops it off fresh extractions — @@ -2240,9 +2329,20 @@ def prefix_graph_for_global( collide and the aggregated community view fuses unrelated communities into one meta-node (#3014). 0 (the default) leaves communities untouched. """ - relabel = {n: f"{repo_tag}::{n}" for n in G.nodes} + # External stubs (stdlib / third-party modules, #2873) are GLOBAL + # identifiers, not repo-local: leaving their id unprefixed lets the same + # module unify into one node across repos instead of fragmenting into + # repoA::typing / repoB::typing, which is exactly the "which repos depend on + # X" question a cross-repo merge exists to answer. + relabel = { + n: f"{repo_tag}::{n}" + for n, d in G.nodes(data=True) + if not d.get("external") + } H = nx.relabel_nodes(G, relabel, copy=True) for node, data in H.nodes(data=True): + if data.get("external"): + continue # global id, belongs to no single repo — no repo/local_id tag data["repo"] = repo_tag data.setdefault("local_id", node.split("::", 1)[1]) cid = data.get("community") diff --git a/graphify/watch.py b/graphify/watch.py index aef0c2569..f5fd10c2f 100644 --- a/graphify/watch.py +++ b/graphify/watch.py @@ -1863,6 +1863,7 @@ def _rebuild_code( dedupe_edges as _dedupe_edges, dedupe_nodes as _dedupe_nodes, disambiguate_file_labels_in_nodes as _disamb_labels, + mint_external_stubs_in_data as _mint_external_stubs_in_data, ) raw_nodes = _dedupe_nodes(result.get("nodes", [])) _disamb_labels(raw_nodes) @@ -1875,6 +1876,12 @@ def _rebuild_code( # `result` (the raw merged extraction) never carries one. "directed": bool((existing_graph_data or {}).get("directed", False)), } + # This path writes the raw merged extraction, not a build_from_json + # graph, so mint the same external stubs the builder does — otherwise + # an import to stdlib / a third-party module leaves an undeclared + # endpoint in graph.json that every loader materialises as an + # attribute-less phantom (#2873). + _mint_external_stubs_in_data(candidate_graph_data) candidate_graph_text = _json_text(candidate_graph_data) same_graph = False if existing_graph.exists(): diff --git a/tests/test_external_stub_endpoints.py b/tests/test_external_stub_endpoints.py new file mode 100644 index 000000000..1160c399a --- /dev/null +++ b/tests/test_external_stub_endpoints.py @@ -0,0 +1,131 @@ +"""Every edge endpoint in graph.json must be a declared node (#2873). + +`graph.json` used to ship `imports`/`imports_from`/`re_exports` edges whose +target was an external module (stdlib, a third-party dependency) with no +matching entry in `nodes[]`. Every loader (networkx `node_link_graph`) +materialises such an endpoint as an attribute-less phantom node, so the node +set the file declares and the node set it produces disagree, and `merge-graphs` +fragments the same external module into one node per repo. These tests pin the +fix: mint a typed external stub node instead, keep it global across repos, and +never stub a non-import relation (a sourceless external call target stays +suppressed per #3156). +""" +from __future__ import annotations + +import json +from pathlib import Path + +import networkx as nx + +from graphify.build import ( + build_from_json, + mint_external_stubs_in_data, + prefix_graph_for_global, +) + + +def _import_extraction(target: str, relation: str = "imports") -> dict: + return { + "nodes": [ + {"id": "pkg_a", "label": "a.py", "file_type": "code", "source_file": "pkg/a.py"}, + ], + "links": [ + {"source": "pkg_a", "target": target, "relation": relation, + "source_file": "pkg/a.py", "confidence": "EXTRACTED", "weight": 1.0}, + ], + } + + +def test_build_from_json_mints_external_import_stub(): + G = build_from_json(_import_extraction("typing")) + assert "typing" in G.nodes, "external import target must be a declared node, not dangling" + assert G.nodes["typing"].get("external") is True + assert G.nodes["typing"].get("type") == "external" + assert G.has_edge("pkg_a", "typing"), "the import edge must survive" + + +def test_external_stub_makes_serialized_graph_self_consistent(): + from networkx.readwrite import json_graph + + G = build_from_json(_import_extraction("pathlib", relation="imports_from")) + data = json_graph.node_link_data(G, edges="links") + declared = {n["id"] for n in data["nodes"]} + for e in data["links"]: + assert e["source"] in declared and e["target"] in declared, ( + "serialized graph must have no undeclared edge endpoint" + ) + + +def test_non_import_dangling_edge_is_still_dropped(): + """A sourceless external CALL target is deliberately suppressed (#3156) to + avoid a phantom god-node: only import-family relations get a stub.""" + G = build_from_json(_import_extraction("some_external_fn", relation="calls")) + assert "some_external_fn" not in G.nodes + assert not G.has_edge("pkg_a", "some_external_fn") + + +def test_mint_external_stubs_in_data_is_idempotent(): + data = { + "nodes": [{"id": "pkg_a", "label": "a.py", "file_type": "code", + "source_file": "pkg/a.py"}], + "links": [{"source": "pkg_a", "target": "typing", "relation": "imports"}], + } + mint_external_stubs_in_data(data) + mint_external_stubs_in_data(data) # second pass must not duplicate + stubs = [n for n in data["nodes"] if n.get("external")] + assert [n["id"] for n in stubs] == ["typing"] + + +def test_external_stub_unifies_across_repos_in_merge(): + def mk(file_id: str) -> nx.Graph: + return build_from_json({ + "nodes": [{"id": file_id, "label": f"{file_id}.py", "file_type": "code", + "source_file": f"{file_id}.py"}], + "links": [{"source": file_id, "target": "typing", "relation": "imports", + "source_file": f"{file_id}.py", "confidence": "EXTRACTED", "weight": 1.0}], + }) + + merged = nx.compose( + prefix_graph_for_global(mk("a"), "repoA"), + prefix_graph_for_global(mk("b"), "repoB"), + ) + assert "typing" in merged.nodes, "external module must stay a single global id" + assert "repoA::typing" not in merged.nodes and "repoB::typing" not in merged.nodes + assert merged.nodes["typing"].get("repo") is None, "an external node belongs to no repo" + # both repos' import edges land on the one shared node + assert merged.degree("typing") == 2 + + +def test_no_cluster_update_leaves_no_undeclared_endpoint(tmp_path: Path): + """End-to-end: `graphify update --no-cluster` writes the raw merged + extraction, not a build_from_json graph, so it must stub external endpoints + on that path too. Routes through the real watch rebuild.""" + from graphify.watch import _rebuild_code + + corpus = tmp_path / "corpus" + (corpus / "pkg").mkdir(parents=True) + (corpus / "pkg" / "a.py").write_text( + "import typing\nfrom pathlib import Path\nfrom .b import helper\n\n" + "def go(x: 'typing.Any') -> None:\n helper(Path(str(x)))\n", + encoding="utf-8", + ) + (corpus / "pkg" / "b.py").write_text( + "import re\n\ndef helper(x):\n return re.match(r'.', str(x))\n", encoding="utf-8" + ) + (corpus / "pkg" / "__init__.py").write_text("from .a import go\n", encoding="utf-8") + + assert _rebuild_code(corpus, no_cluster=True, acquire_lock=False) is True + data = json.loads((corpus / "graphify-out" / "graph.json").read_text(encoding="utf-8")) + declared = {n["id"] for n in data["nodes"]} + dangling = [ + (e.get("source"), e.get("target")) + for e in data["links"] + if e.get("source") not in declared or e.get("target") not in declared + ] + assert not dangling, f"graph.json has undeclared edge endpoints: {dangling}" + # networkx must not have to materialise any phantom node + G = nx.node_link_graph(data, edges="links") + assert G.number_of_nodes() == len(data["nodes"]) + # the stdlib imports resolved to typed external stubs + externals = {n["id"] for n in data["nodes"] if n.get("external")} + assert {"typing", "pathlib", "re"} <= externals