diff --git a/src/cgis/api/mcp_server.py b/src/cgis/api/mcp_server.py index d10f20b0..0cf5eb73 100644 --- a/src/cgis/api/mcp_server.py +++ b/src/cgis/api/mcp_server.py @@ -958,7 +958,11 @@ def cgis_module_graph( shows; an edge that closes a cycle is never dropped. Dependencies on code outside the package are summarised per module under the diagram, not drawn. Test modules and type-only imports are left out. The json form is - ``{prefix, nodes, subpackages, edges:[{source,target,kind}], dropped, outside}``. + ``{prefix, nodes, subpackages, edges:[{source,target,kind}], dropped, covered, outside}``; + ``covered`` counts the imports not drawn because a drawn ``==>`` joins the + same pair: that arrow then also stands for the import, so read an import + closure over imports and extends together. An import that closes an import + cycle is always drawn. Run ``cgis_ingest`` first. """ if not prefix.strip(): diff --git a/src/cgis/query/analysis/module_graph.py b/src/cgis/query/analysis/module_graph.py index 4dbf29af..8303b663 100644 --- a/src/cgis/query/analysis/module_graph.py +++ b/src/cgis/query/analysis/module_graph.py @@ -57,6 +57,8 @@ class ModuleGraph: """A package's module graph. ``dropped`` counts, per kind, the edges the transitive reduction removed; + ``covered`` the imports left out because a drawn inheritance edge joins the + same pair, so that ``==>`` also stands for the import; ``outside`` maps a module to the packages outside ``prefix`` it depends on. """ @@ -66,6 +68,7 @@ class ModuleGraph: edges: list[ModuleEdge] dropped: dict[str, int] outside: dict[str, list[str]] + covered: int = 0 def _under(fqn: str, prefix: str) -> bool: @@ -213,6 +216,29 @@ def _ends(e: Edge, scope: _Scope) -> tuple[str, str] | None: return _group(src_file, scope.prefix, scope.expand), dst_file +def _uncovered(kept: list[ModuleEdge]) -> tuple[list[ModuleEdge], int]: + """``kept`` without the imports that an inheritance arrow on screen stands for. + + An import of a pair is left out when the same pair has a kept ``==>``, + which then stands for the import too. It stays when that inheritance edge + was itself reduced away (nothing would be drawn on the pair), and when it + closes an import cycle (the cycle must stay readable as imports). Applied + after the reduction, so which imports are implied by a longer path is + decided over all of them. + """ + extends = {(e.source, e.target) for e in kept if e.kind == "extends"} + imports = [e for e in kept if e.kind == "imports"] + comp, _ = _condense(imports) + shown = [ + e + for e in kept + if e.kind != "imports" + or (e.source, e.target) not in extends + or comp[e.source] == comp[e.target] + ] + return shown, len(kept) - len(shown) + + def _reduced( found: Iterable[ModuleEdge], layers: list[str], reduce: bool ) -> tuple[list[ModuleEdge], dict[str, int]]: @@ -280,6 +306,7 @@ def build_module_graph( ) found, outside = _collect(edges, scope, chosen) kept, dropped = _reduced(found, chosen, reduce) + kept, covered = _uncovered(kept) return ModuleGraph( prefix=prefix, nodes=members, @@ -289,6 +316,7 @@ def build_module_graph( edges=kept, dropped=dropped, outside={m: sorted(pkgs) for m, pkgs in sorted(outside.items())}, + covered=covered, ) @@ -366,7 +394,11 @@ def render_mermaid(graph: ModuleGraph) -> str: kinds = ", ".join(f"{k} {_ARROWS[k]}" for k in graph.dropped) removed = ", ".join(f"{k} {n}" for k, n in graph.dropped.items() if n) lines.append("") - lines.append(f"Edges: {kinds}." + (f" Implied edges dropped: {removed}." if removed else "")) + lines.append( + f"Edges: {kinds}." + + (f" Implied edges dropped: {removed}." if removed else "") + + (f" Imports drawn as the ==> of the same pair: {graph.covered}." if graph.covered else "") + ) if graph.outside: lines += ["", f"| module | reaches outside `{graph.prefix}` |", "|---|---|"] for module, packages in graph.outside.items(): @@ -375,12 +407,16 @@ def render_mermaid(graph: ModuleGraph) -> str: def graph_to_dict(graph: ModuleGraph) -> dict[str, object]: - """The graph as plain data: ``{prefix, nodes, subpackages, edges, dropped, outside}``.""" + """The graph as plain data. + + ``{prefix, nodes, subpackages, edges, dropped, covered, outside}``. + """ return { "prefix": graph.prefix, "nodes": graph.nodes, "subpackages": graph.subpackages, "edges": [{"source": e.source, "target": e.target, "kind": e.kind} for e in graph.edges], "dropped": graph.dropped, + "covered": graph.covered, "outside": graph.outside, } diff --git a/tests/unit/test_module_graph.py b/tests/unit/test_module_graph.py index 838d4a44..2869571e 100644 --- a/tests/unit/test_module_graph.py +++ b/tests/unit/test_module_graph.py @@ -14,6 +14,7 @@ from cgis.query.analysis.module_graph import ( ModuleEdge, ModuleGraph, + graph_to_dict, module_graph, render_mermaid, transitive_reduction, @@ -217,8 +218,10 @@ def test_cli_prints_a_mermaid_diagram(db: str) -> None: result = CliRunner().invoke(app, ["module-graph", "pkg", "--db", db]) assert result.exit_code == 0, result.output assert "flowchart TD" in result.stdout - assert "m_creation --> m_core" in result.stdout - assert "m_creation --> m_effects" not in result.stdout + assert "m_cyc1 --> m_cyc2" in result.stdout + assert "m_creation ==> m_core" in result.stdout + assert "m_creation --> m_core" not in result.stdout # drawn as its inheritance + assert "m_creation --> m_effects" not in result.stdout # implied def test_cli_json_with_calls_and_an_unknown_layer(db: str) -> None: @@ -372,3 +375,71 @@ def test_type_only_imports_are_not_dependencies(tmp_path: Path) -> None: with store: IngestionPipeline(extractors([])).run(str(root), store=store, rebuild=True) assert module_graph(db_path, "pkg", layers=("imports",)).edges == [] + + +def test_inheritance_covers_the_import_of_the_same_pair(db: str) -> None: + """core imports and extends effects: one ==>, no --> beside it, and the count says so.""" + graph = module_graph(db, "pkg") + assert ("pkg.core", "pkg.effects") in _pairs(graph, "extends") + assert ("pkg.core", "pkg.effects") not in _pairs(graph, "imports") + # core, creation, service and sub each import what they extend + assert graph.covered == 4 + assert "Imports drawn as the ==> of the same pair: 4." in render_mermaid(graph) + assert graph_to_dict(graph)["covered"] == 4 + + +def test_imports_alone_keep_every_import(db: str) -> None: + """Without the extends layer nothing covers an import.""" + graph = module_graph(db, "pkg", layers=("imports",)) + assert ("pkg.core", "pkg.effects") in _pairs(graph, "imports") + assert graph.covered == 0 + + +def _graph_of(tmp_path: Path, files: dict[str, str]) -> str: + root = tmp_path / "repo" + for rel, text in files.items(): + (root / rel).parent.mkdir(parents=True, exist_ok=True) + (root / rel).write_text(text) + db_path = str(tmp_path / "g.db") + store = SQLiteStore(db_path) + store.connect() + with store: + IngestionPipeline(extractors([])).run(str(root), store=store, rebuild=True) + return db_path + + +def test_an_import_is_hidden_only_behind_an_inheritance_arrow_on_screen(tmp_path: Path) -> None: + """a extends b only through c, so a ==> b is reduced away; its import a --> b must stay.""" + db_path = _graph_of( + tmp_path, + { + "pkg/__init__.py": "from pkg.b import B\n", + "pkg/b.py": "class B:\n pass\n", + "pkg/c.py": "import pkg\n\n\nclass C(pkg.B):\n pass\n", + "pkg/a.py": ( + "from pkg.b import B\nfrom pkg.c import C\n\n\n" + "class A1(C):\n pass\n\n\nclass A2(B):\n pass\n" + ), + }, + ) + graph = module_graph(db_path, "pkg") + assert ("pkg.a", "pkg.b") not in _pairs(graph, "extends") + assert ("pkg.a", "pkg.b") in _pairs(graph, "imports") + assert ("pkg.a", "pkg.c") in _pairs(graph, "extends") + assert graph.covered == 1 + + +def test_an_import_that_closes_a_cycle_is_never_covered(tmp_path: Path) -> None: + """a imports and extends b, b imports a: the import cycle stays an import cycle.""" + db_path = _graph_of( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/b.py": "from pkg import a\n\n\nclass B:\n def f(self):\n return a\n", + "pkg/a.py": "from pkg.b import B\n\n\nclass A(B):\n pass\n", + }, + ) + graph = module_graph(db_path, "pkg") + assert {("pkg.a", "pkg.b"), ("pkg.b", "pkg.a")} <= _pairs(graph, "imports") + assert ("pkg.a", "pkg.b") in _pairs(graph, "extends") + assert graph.covered == 0