From 628f0d1d63a13f36e3223f026dca369c2f64ea57 Mon Sep 17 00:00:00 2001 From: Yyunozor Date: Wed, 22 Jul 2026 01:46:40 +0200 Subject: [PATCH] fix: group cut connections by file on explain for high-degree nodes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `graphify explain ""` sorts a node's connections by neighbor degree and shows only the top 20, appending a bare "... and N more" for the rest. On a high-degree node (a logging/error helper called from dozens of places is typical) that leaves the exact question explain is meant to answer - "who calls this, what's the impact?" - unanswered for 97% of the callers, with nothing pointing at where they live (#2009). The top-20 list and its ordering are unchanged (no behavior change for nodes at or under the cutoff). Past it, the cut connections are now grouped by (direction, source_file) with counts and printed under a "Grouped by file:" section, sorted by count descending, so the caller/callee distribution is visible without falling back to a repo-wide grep. The aggregation itself is capped at 20 files with its own "... and N more files" line for the pathological case where the remainder is spread across more files than that. Full per-caller detail behind a flag (--callers --all / --group-by=dir as proposed in the issue) is left for a follow-up: it's a larger, more opinionated surface (flag naming, pagination semantics) than the literal complaint needs, and the default output no longer hides the answer either way. Four regression tests in tests/test_explain_cli.py: the pre-existing truncation notice on a 30-connection node is unchanged, the grouped- by-file output carries real counts (3 files, one at 4 and two at 3) that sum back to exactly the cut total (no silent loss), a byte-for-byte no-op check for nodes at/under the cutoff, and a boundary check pinning the cutoff itself at exactly 20 connections (no section) vs exactly 21 (one grouped entry) — the earlier tests sit well clear of the edge and wouldn't catch a future off-by-one. --- graphify/cli.py | 20 ++++++++- tests/test_explain_cli.py | 86 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/graphify/cli.py b/graphify/cli.py index 816eb7941..6d52c3bbc 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -1283,7 +1283,25 @@ def dispatch_command(cmd: str) -> None: at = f" {sfile}:{loc}" if loc else "" print(f" {arrow} {G.nodes[nb].get('label', nb)} [{rel}] [{conf}]{at}") if len(connections) > 20: - print(f" ... and {len(connections) - 20} more") + remainder = connections[20:] + print(f" ... and {len(remainder)} more") + # #2009: a bare count silently hides the answer on high-degree + # nodes ("who calls this, what's the impact?"). Group the cut + # connections by direction + file so their shape is visible + # without falling back to a repo-wide grep. + by_file: dict[tuple[str, str], int] = {} + for direction, _nb, edata in remainder: + sfile = edata.get("source_file") or "(unknown file)" + key = (direction, sfile) + by_file[key] = by_file.get(key, 0) + 1 + grouped = sorted(by_file.items(), key=lambda kv: kv[1], reverse=True) + print(" Grouped by file:") + for (direction, sfile), count in grouped[:20]: + arrow = "-->" if direction == "out" else "<--" + noun = "connection" if count == 1 else "connections" + print(f" {arrow} {sfile}: {count} {noun}") + if len(grouped) > 20: + print(f" ... and {len(grouped) - 20} more files") from graphify import querylog querylog.log_query( kind="explain", diff --git a/tests/test_explain_cli.py b/tests/test_explain_cli.py index 5065913e6..94ba22b9c 100644 --- a/tests/test_explain_cli.py +++ b/tests/test_explain_cli.py @@ -150,3 +150,89 @@ def test_explain_connection_shows_call_site_line(monkeypatch, tmp_path, capsys): assert "apollo.py:L90" not in caller_line # never the caller's def line # The queried node's own header still shows its def line (correct). assert "state.py" in out and "L56" in out + + +# --- #2009: high-degree nodes must not silently hide the cut connections ------ + +def _write_high_degree_graph(tmp_path, n_callers=30, files=None): + """A node with n_callers callers, spread across `files` (default: 3 + files, so counts land above 1 per file and truncation kicks in — the + CLI shows the top 20 by neighbor degree, cutting the rest).""" + files = files or ["app/handlers/email.py", "app/jobs/retry.py", "lib/workers/queue.py"] + nodes = [{"id": "hub", "label": "hub()", + "source_file": "lib/hub.py", "community": 0}] + links = [] + for i in range(n_callers): + fpath = files[i % len(files)] + nid = f"caller_{i}" + nodes.append({"id": nid, "label": f"caller_{i}()", + "source_file": fpath, "community": 0}) + links.append({"source": nid, "target": "hub", "relation": "calls", + "confidence": "EXTRACTED", "source_file": fpath, + "source_location": f"L{10 + i}"}) + graph_data = {"directed": False, "multigraph": False, "graph": {}, + "nodes": nodes, "links": links} + p = tmp_path / "graph.json" + p.write_text(json.dumps(graph_data)) + return p + + +def test_explain_truncation_notice_present_for_high_degree_node(monkeypatch, tmp_path, capsys): + """Baseline: the cut count is still announced (pre-existing behavior).""" + p = _write_high_degree_graph(tmp_path, n_callers=30) + out = _run(monkeypatch, p, "hub", capsys) + assert "Connections (30):" in out + assert "... and 10 more" in out + + +def test_explain_groups_cut_callers_by_file_instead_of_dropping_them(monkeypatch, tmp_path, capsys): + """#2009: past the top-20 cutoff, the remaining callers must still be + accounted for — grouped by file with counts — instead of vanishing + behind a bare '... and N more'. No caller may be lost silently: the + per-file counts in the aggregation must sum back to the cut total.""" + p = _write_high_degree_graph( + tmp_path, n_callers=30, + files=["app/handlers/email.py", "app/jobs/retry.py", "lib/workers/queue.py"], + ) + out = _run(monkeypatch, p, "hub", capsys) + assert "Grouped by file:" in out + assert "<-- lib/workers/queue.py: 4 connections" in out + assert "<-- app/handlers/email.py: 3 connections" in out + assert "<-- app/jobs/retry.py: 3 connections" in out + # No silent loss: the aggregated counts must sum to the announced cut. + grouped_lines = [ + l for l in out.splitlines() if l.strip().startswith(("<--", "-->")) and "connection" in l + ] + total = sum(int(l.rsplit(":", 1)[1].split()[0]) for l in grouped_lines) + assert total == 10 # 30 connections - 20 shown = 10 cut, all accounted for + + +def test_explain_no_grouping_section_when_under_cutoff(monkeypatch, tmp_path, capsys): + """Regression guard: nodes at or below the 20-connection cutoff keep the + pre-#2009 output byte-for-byte (no new section, no behavior change).""" + p = _write_high_degree_graph(tmp_path, n_callers=5) + out = _run(monkeypatch, p, "hub", capsys) + assert "Grouped by file:" not in out + assert "more" not in out + + +def test_explain_grouping_boundary_at_exactly_21_vs_20_connections(monkeypatch, tmp_path, capsys): + """Pin the exact `> 20` cutoff itself. The other #2009 tests use 30 and 5 + connections, both comfortably clear of the edge — nothing here fails if a + future refactor shifts the boundary by one. One node at exactly 21 + connections (one past the cutoff) must show the grouped section with + exactly one grouped entry; one node at exactly 20 (at the cutoff, not + past it) must show neither.""" + p21 = _write_high_degree_graph(tmp_path, n_callers=21, files=["lib/only.py"]) + out21 = _run(monkeypatch, p21, "hub", capsys) + assert "Grouped by file:" in out21 + assert "<-- lib/only.py: 1 connection" in out21 + grouped_lines21 = [ + l for l in out21.splitlines() if l.strip().startswith(("<--", "-->")) and "connection" in l + ] + assert len(grouped_lines21) == 1 # exactly one grouped entry, not zero, not more + + p20 = _write_high_degree_graph(tmp_path, n_callers=20, files=["lib/only.py"]) + out20 = _run(monkeypatch, p20, "hub", capsys) + assert "Grouped by file:" not in out20 + assert "more" not in out20