From e07a63c49bd7422ee2a1523c7bbbeb2639e33a9e Mon Sep 17 00:00:00 2001 From: mochan Date: Sat, 30 May 2026 19:05:20 +0530 Subject: [PATCH] feat(metrics): split unresolved edges into external vs unresolved-local MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `unresolved_edges` count conflated two very different things: 1. External imports — `fastapi`, `os`, `react`. Static analysis without a venv/site-packages parse can't resolve these and isn't supposed to. These are expected, not errors. 2. Local-unresolved — target's root module IS in the repo but the specific symbol couldn't be matched (rename drift, dynamic import, missed binding). These are the actionable ones. `compute_metrics` now classifies each unresolved edge by checking whether the target's root module appears as a MODULE node's qualname-root in the graph. New fields: `external_edges`, `unresolved_local_edges`. The `unresolved_edges` total is kept as the sum for back-compat. Surfaced + exposed in the markdown report and the MCP `metrics` tool. v0.1.2 backlog item #2. --- CHANGELOG.md | 16 ++++++++ codegraph/analysis/metrics.py | 66 ++++++++++++++++++++++++++++++-- codegraph/analysis/report.py | 7 +++- codegraph/mcp_server/server.py | 2 + tests/test_metrics_external.py | 69 ++++++++++++++++++++++++++++++++++ 5 files changed, 155 insertions(+), 5 deletions(-) create mode 100644 tests/test_metrics_external.py diff --git a/CHANGELOG.md b/CHANGELOG.md index f26dc99..9182b7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **Edge classification: `external` vs `unresolved-local`.** The + `unresolved_edges` metric used to lump together imports of external + packages (`fastapi`, `os`, `react`) — which static analysis without a + venv/site-packages parse cannot resolve and isn't supposed to — with + edges that pointed at in-repo symbols but couldn't be matched (rename + drift, dynamic import). One number, two very different meanings. + `compute_metrics` now also returns `external_edges` (root module not + in the repo) and `unresolved_local_edges` (root is in-repo, symbol + missed). The `unresolved_edges` total is kept as the sum for + back-compat. Markdown report + MCP `metrics` tool both expose the + split. (Surfaced by a user adding polycodegraph to a FastAPI project + who saw "1,423 unresolved edges" and asked whether the tool was + broken; almost all were external imports.) + ### Fixed - **`codegraph init` now writes a robust `.mcp.json`** — uses the absolute diff --git a/codegraph/analysis/metrics.py b/codegraph/analysis/metrics.py index 6ad678c..72c77cb 100644 --- a/codegraph/analysis/metrics.py +++ b/codegraph/analysis/metrics.py @@ -17,7 +17,55 @@ class GraphMetrics: edges_by_kind: dict[str, int] = field(default_factory=dict) languages: dict[str, int] = field(default_factory=dict) top_files_by_nodes: list[tuple[str, int]] = field(default_factory=list) + # Total of `external_edges + unresolved_local_edges`. Kept for back-compat + # with consumers that pre-date the 0.1.2 split. unresolved_edges: int = 0 + # Imports whose root module is not part of this repo (e.g. `fastapi`, + # `os`, `react`). These are expected, not errors — they're cross-boundary + # references that static analysis without a venv / site-packages parse + # cannot resolve and isn't supposed to. + external_edges: int = 0 + # Unresolved edges whose root module IS in the repo but couldn't be + # matched (rename drift, dynamic import, missed binding). These are the + # actionable ones. + unresolved_local_edges: int = 0 + + +_UNRESOLVED_PREFIX = "unresolved::" + + +def _module_root(qualname: str) -> str: + """First segment of a dotted qualname, e.g. ``foo.bar.baz`` -> ``foo``. + + Also handles TS path-style names (``pkg/sub/file``) and rare ``a::b`` + forms by stripping at the first separator of any flavour. + """ + if not qualname: + return "" + head = qualname.split(".", 1)[0] + for sep in ("/", "::"): + if sep in head: + head = head.split(sep, 1)[0] + return head + + +def _collect_repo_roots(graph: nx.MultiDiGraph) -> set[str]: + """Module-name roots that live in the repo. + + Used to classify each unresolved edge as either ``external`` (root not + in the repo — e.g. ``fastapi``, ``os``) or ``unresolved_local`` (root + is a repo package but the specific symbol couldn't be matched). + """ + roots: set[str] = set() + for _nid, attrs in graph.nodes(data=True): + if _kind_str(attrs.get("kind")) != "MODULE": + continue + qn = attrs.get("qualname") + if isinstance(qn, str): + root = _module_root(qn) + if root: + roots.add(root) + return roots def compute_metrics(graph: nx.MultiDiGraph, *, top_files: int = 10) -> GraphMetrics: @@ -37,16 +85,26 @@ def compute_metrics(graph: nx.MultiDiGraph, *, top_files: int = 10) -> GraphMetr metrics.languages = dict(sorted(lang_counter.items())) metrics.top_files_by_nodes = file_counter.most_common(top_files) + repo_roots = _collect_repo_roots(graph) + edge_counter: Counter[str] = Counter() - unresolved = 0 + external = 0 + unresolved_local = 0 total = 0 for _src, dst, _key, data in graph.edges(keys=True, data=True): total += 1 ek = _kind_str(data.get("kind")) or "UNKNOWN" edge_counter[ek] += 1 - if isinstance(dst, str) and dst.startswith("unresolved::"): - unresolved += 1 + if isinstance(dst, str) and dst.startswith(_UNRESOLVED_PREFIX): + target = dst[len(_UNRESOLVED_PREFIX):] + root = _module_root(target) + if root and root in repo_roots: + unresolved_local += 1 + else: + external += 1 metrics.total_edges = total metrics.edges_by_kind = dict(sorted(edge_counter.items())) - metrics.unresolved_edges = unresolved + metrics.external_edges = external + metrics.unresolved_local_edges = unresolved_local + metrics.unresolved_edges = external + unresolved_local return metrics diff --git a/codegraph/analysis/report.py b/codegraph/analysis/report.py index f34554e..cfb05b1 100644 --- a/codegraph/analysis/report.py +++ b/codegraph/analysis/report.py @@ -64,6 +64,8 @@ def _metrics_to_dict(m: GraphMetrics) -> dict[str, Any]: "languages": dict(m.languages), "top_files_by_nodes": [list(t) for t in m.top_files_by_nodes], "unresolved_edges": m.unresolved_edges, + "external_edges": m.external_edges, + "unresolved_local_edges": m.unresolved_local_edges, } @@ -90,7 +92,10 @@ def report_to_markdown(report: AnalyzeReport) -> str: lines.append("") lines.append(f"- Nodes: **{m.total_nodes}**") lines.append(f"- Edges: **{m.total_edges}**") - lines.append(f"- Unresolved edges: **{m.unresolved_edges}**") + lines.append( + f"- Unresolved edges: **{m.unresolved_edges}** " + f"(external: {m.external_edges}, local-unresolved: {m.unresolved_local_edges})" + ) if m.nodes_by_kind: lines.append("- Nodes by kind: " + ", ".join( f"{k}={v}" for k, v in m.nodes_by_kind.items() diff --git a/codegraph/mcp_server/server.py b/codegraph/mcp_server/server.py index 8160b6b..bed6015 100644 --- a/codegraph/mcp_server/server.py +++ b/codegraph/mcp_server/server.py @@ -363,6 +363,8 @@ def tool_metrics(graph: nx.MultiDiGraph) -> dict[str, Any]: "languages": m.languages, "top_files_by_nodes": m.top_files_by_nodes, "unresolved_edges": m.unresolved_edges, + "external_edges": m.external_edges, + "unresolved_local_edges": m.unresolved_local_edges, } diff --git a/tests/test_metrics_external.py b/tests/test_metrics_external.py new file mode 100644 index 0000000..f01e618 --- /dev/null +++ b/tests/test_metrics_external.py @@ -0,0 +1,69 @@ +"""Tests for the external/unresolved-local edge classification (v0.1.2 #2).""" +from __future__ import annotations + +import networkx as nx + +from codegraph.analysis.metrics import _module_root, compute_metrics + + +def test_module_root_handles_separators() -> None: + assert _module_root("foo.bar.baz") == "foo" + assert _module_root("pkg/sub/file") == "pkg" + assert _module_root("a::b") == "a" + assert _module_root("simple") == "simple" + assert _module_root("") == "" + + +def _make_graph_with_module(module_qualname: str) -> nx.MultiDiGraph: + g = nx.MultiDiGraph() + g.add_node( + "mod:" + module_qualname, + kind="MODULE", + qualname=module_qualname, + language="python", + ) + g.add_node( + "fn:caller", + kind="FUNCTION", + qualname=module_qualname + ".caller", + language="python", + ) + return g + + +def test_external_vs_unresolved_local_classification() -> None: + g = _make_graph_with_module("myrepo") + + # External: target root is `fastapi`, not in repo + g.add_edge("fn:caller", "unresolved::fastapi.FastAPI", kind="IMPORTS") + # External: target root is `os`, not in repo + g.add_edge("fn:caller", "unresolved::os.path.join", kind="CALLS") + # Local-unresolved: target root is `myrepo`, IS in repo + g.add_edge("fn:caller", "unresolved::myrepo.utils.missing_fn", kind="CALLS") + # Bare name with no dot: treated as external since root not in repo_roots + g.add_edge("fn:caller", "unresolved::randomBareName", kind="CALLS") + + m = compute_metrics(g) + assert m.external_edges == 3 + assert m.unresolved_local_edges == 1 + # Back-compat field is the sum. + assert m.unresolved_edges == 4 + + +def test_no_modules_means_everything_external() -> None: + g = nx.MultiDiGraph() + g.add_node("fn:x", kind="FUNCTION", qualname="x") + g.add_edge("fn:x", "unresolved::anything", kind="CALLS") + m = compute_metrics(g) + assert m.external_edges == 1 + assert m.unresolved_local_edges == 0 + + +def test_no_unresolved_edges_yields_zero_external() -> None: + g = _make_graph_with_module("myrepo") + g.add_node("fn:other", kind="FUNCTION", qualname="myrepo.other") + g.add_edge("fn:caller", "fn:other", kind="CALLS") + m = compute_metrics(g) + assert m.external_edges == 0 + assert m.unresolved_local_edges == 0 + assert m.unresolved_edges == 0