mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-16 00:49:37 +00:00
fix(build): mint typed external stubs for undeclared import endpoints (#2873)
graph.json shipped imports/imports_from/re_exports edges whose target was an external module (stdlib, a third-party dep) with no entry in nodes[]. Every loader (networkx node_link_graph) materialises such an endpoint as an attribute-less phantom, so declared node-count != loaded node-count, and merge-graphs fragmented the same module into one node per repo. - build_from_json now mints a typed external stub node (type=external, external=True) for an import-family edge whose target is otherwise undeclared, instead of dropping the edge. Non-import relations are unchanged, so a sourceless external call target stays suppressed (#3156, no phantom god-node). - The --no-cluster / incremental update path writes the raw merged extraction rather than a builder graph, so it applies the same stubbing via the shared mint_external_stubs_in_data helper. Idempotent; grows the node set so the #479 shrink guard is unaffected. - prefix_graph_for_global leaves external stubs unprefixed (no repo:: tag), so the same stdlib/dependency module unifies into one global node across a cross-repo merge instead of fragmenting per repo. Adds regression tests for build-level stubbing, serialized-graph consistency, the #3156 non-import drop, idempotency, cross-repo unification, and an end-to-end `update --no-cluster` integrity check through the real rebuild. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
e3535e942a
commit
778c04726d
3 files changed
+240
-2
No files matched your search
+102
-2
@@ -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")
|
||||
|
||||
@@ -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():
|
||||
|
||||
@@ -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
|
||||
Reference in new issue
Block a user