diff --git a/CHANGELOG.md b/CHANGELOG.md index f26dc99..beb167d 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 @@ -19,6 +35,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 pre-0.1.2 default entries are migrated forward in place; user-customised entries are left alone. (Surfaced when a user added polycodegraph to a FastAPI project — Claude reported needing to fix all three.) +- **Type-annotation-only references no longer count as dead code.** The + Python parser now emits a reference edge from a function/method to + each capitalized type name appearing in its parameter and return type + annotations. Edges go through the existing `unresolved::` + resolver pipeline, so they rewrite to real `CLASS` ids when the type + is defined in the repo. The flagship case is FastAPI Pydantic + request-body models — `def create_defect(body: RetroDefect): ...` was + flagged dead because the only reference to `RetroDefect` came through + the type annotation; that loop is now closed. Stdlib typing + scaffolding (`Optional`, `Annotated`, `Union`, …) and FastAPI + dependency markers (`Body`, `Depends`, …) are blocklisted so the + graph stays clean. Framework-agnostic: any function with type + annotations benefits, not just route handlers. ## [0.1.1] — 2026-05-24 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/codegraph/parsers/python.py b/codegraph/parsers/python.py index fc7fd22..5e8c1d9 100644 --- a/codegraph/parsers/python.py +++ b/codegraph/parsers/python.py @@ -321,6 +321,58 @@ def _simplify_arg(node: tree_sitter.Node, src: bytes) -> str: return "" +# Names that look like classes by the capital-letter heuristic but are +# either builtins, stdlib, or typing scaffolding — emitting REFERENCES +# edges to these would be noise. The dead-code analyzer doesn't care +# about them anyway. +_ANNOTATION_NAME_BLOCKLIST: frozenset[str] = frozenset({ + # typing module scaffolding + "Any", "AnyStr", "Optional", "Union", "Literal", "Final", "ClassVar", + "Annotated", "TypedDict", "TypeVar", "ParamSpec", "Concatenate", + "Type", "Tuple", "List", "Dict", "Set", "FrozenSet", "Callable", + "Iterable", "Iterator", "Generator", "AsyncIterator", "AsyncGenerator", + "Awaitable", "Coroutine", "AsyncContextManager", "ContextManager", + "Sequence", "Mapping", "MutableMapping", "MutableSequence", "MutableSet", + "Hashable", "Sized", "Container", "Collection", "Reversible", + "NamedTuple", "Self", "Never", "NoReturn", "LiteralString", "NotRequired", + "Required", "Unpack", "TypeAlias", "TypeGuard", "TypeIs", + # Pydantic / FastAPI dependency annotations users wrap their types in. + # We don't want to emit references TO `Body` / `Depends` etc.; + # we want the inner type names that come along with them. + "Body", "Depends", "Path", "Query", "Header", "Cookie", "Form", "File", + "Security", "Request", "Response", + # Common Python builtins that pass the capital-letter heuristic. + "True", "False", "None", "Ellipsis", "NotImplemented", +}) + +_TYPE_NAME_RE = re.compile(r"\b([A-Z][A-Za-z0-9_]*)\b") + + +def _extract_type_references(annotation: str | None) -> list[str]: + """Return capitalized type names referenced in an annotation string. + + Splits ``Annotated[User, Body(...)]`` / ``list[User] | None`` / + ``Optional[User]`` etc. into the candidate names ``User``. Stdlib + typing scaffolding and FastAPI dependency markers are blocklisted. + + Returns names in source-order with duplicates removed; an empty list + when ``annotation`` is None or blank. + """ + if not annotation: + return [] + seen: set[str] = set() + out: list[str] = [] + for match in _TYPE_NAME_RE.finditer(annotation): + name = match.group(1) + if name in _ANNOTATION_NAME_BLOCKLIST: + continue + if name in seen: + continue + seen.add(name) + out.append(name) + return out + + def _extract_params( params_node: tree_sitter.Node, src: bytes, @@ -1153,6 +1205,40 @@ def _handle_function( self._emit_decorator_calls(node, rel, func_id, src, edges) + # Emit reference edges for each capitalized type name appearing in + # parameter annotations and the return type. The resolver rewrites + # ``unresolved::`` to the real CLASS id if the type is + # defined in the repo. This keeps dead-code detection honest for + # types that are only referenced via type annotations — most + # notably FastAPI Pydantic request-body models, which would + # otherwise look unreferenced because handler dispatch happens + # via parameter annotations rather than direct calls. + annotation_strs: list[str] = [] + params_meta = metadata.get("params") + if isinstance(params_meta, list): + for param in params_meta: + if isinstance(param, dict): + type_str = param.get("type") + if isinstance(type_str, str): + annotation_strs.append(type_str) + return_meta = metadata.get("returns") + if isinstance(return_meta, str): + annotation_strs.append(return_meta) + emitted: set[str] = set() + for ann in annotation_strs: + for type_name in _extract_type_references(ann): + if type_name in emitted: + continue + emitted.add(type_name) + edges.append(Edge( + src=func_id, + dst=f"unresolved::{type_name}", + kind=EdgeKind.CALLS, + file=rel, + line=node.start_point[0] + 1, + metadata={"target_name": type_name, "via": "annotation"}, + )) + # DF1 — HTTP route extraction. One ROUTE edge per (method, path); # Flask's ``methods=[...]`` expands to multiple edges. for spec in _extract_route_specs(decorators): 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 diff --git a/tests/test_param_annotation_refs.py b/tests/test_param_annotation_refs.py new file mode 100644 index 0000000..53b694a --- /dev/null +++ b/tests/test_param_annotation_refs.py @@ -0,0 +1,103 @@ +"""Tests for parameter-annotation REFERENCES (v0.1.2 #3). + +A FastAPI handler that takes a Pydantic model as a body parameter used +to look like dead code: the model class had no incoming CALLS / IMPORTS +/ INHERITS / IMPLEMENTS edges because the only "reference" was a type +annotation on the handler's parameter, which the parser didn't trace. +This module verifies the fix: handlers emit ``CALLS`` edges from the +handler to each capitalized type name in their parameter and return +type annotations, with ``metadata.via == "annotation"``. +""" +from __future__ import annotations + +from pathlib import Path + +import networkx as nx +import pytest + +from codegraph.analysis import find_dead_code +from codegraph.graph.builder import GraphBuilder +from codegraph.graph.store_networkx import to_digraph +from codegraph.graph.store_sqlite import SQLiteGraphStore +from codegraph.parsers.python import _extract_type_references + + +def test_extract_type_references_simple() -> None: + assert _extract_type_references("User") == ["User"] + assert _extract_type_references("str") == [] # not capitalized + assert _extract_type_references(None) == [] + assert _extract_type_references("") == [] + + +def test_extract_type_references_generic_and_union() -> None: + assert _extract_type_references("list[User]") == ["User"] + assert _extract_type_references("dict[str, User]") == ["User"] + assert _extract_type_references("User | None") == ["User"] + assert _extract_type_references("Optional[User]") == ["User"] + assert _extract_type_references("Annotated[User, Body(...)]") == ["User"] + + +def test_extract_type_references_drops_typing_scaffolding() -> None: + # All of these are blocklisted; only `User` should survive. + ann = "Annotated[Optional[Union[User, Admin]], Body(...)]" + assert set(_extract_type_references(ann)) == {"User", "Admin"} + + +def test_extract_type_references_deduplicates_in_order() -> None: + assert _extract_type_references("dict[User, User]") == ["User"] + assert _extract_type_references("tuple[A, B, A]") == ["A", "B"] + + +@pytest.fixture +def fastapi_repo_graph(tmp_path: Path) -> nx.MultiDiGraph: + """Tiny FastAPI-style fixture: a handler that takes a Pydantic body.""" + repo = tmp_path / "repo" + (repo / "myapp").mkdir(parents=True) + (repo / "myapp" / "__init__.py").write_text("") + (repo / "myapp" / "models.py").write_text( + "class RetroDefect:\n" + " name: str\n" + ) + (repo / "myapp" / "routes.py").write_text( + "from myapp.models import RetroDefect\n" + "\n" + "def app_get(path):\n" + " def deco(fn): return fn\n" + " return deco\n" + "\n" + "@app_get('/defect')\n" + "def create_defect(body: RetroDefect) -> RetroDefect:\n" + " return body\n" + ) + store = SQLiteGraphStore(tmp_path / "graph.db") + GraphBuilder(repo, store).build(incremental=False) + g = to_digraph(store) + store.close() + return g + + +def test_pydantic_model_not_dead_when_only_referenced_via_annotation( + fastapi_repo_graph: nx.MultiDiGraph, +) -> None: + dead_qns = {d.qualname for d in find_dead_code(fastapi_repo_graph)} + assert not any(qn.endswith("RetroDefect") for qn in dead_qns), ( + "RetroDefect is referenced by the handler's body+return annotations " + "and must not be flagged dead. dead_qns=" + repr(dead_qns) + ) + + +def test_handler_has_annotation_edge_to_model( + fastapi_repo_graph: nx.MultiDiGraph, +) -> None: + """The handler should have an outgoing edge tagged via=annotation.""" + g = fastapi_repo_graph + found_via_annotation = False + for src, _dst, _key, data in g.edges(keys=True, data=True): + attrs = g.nodes.get(src) or {} + if not str(attrs.get("qualname") or "").endswith("create_defect"): + continue + md = data.get("metadata") or {} + if isinstance(md, dict) and md.get("via") == "annotation": + found_via_annotation = True + break + assert found_via_annotation, "expected an annotation-via edge from handler"