fix: group cut connections by file on explain for high-degree nodes
`graphify explain "<node>"` 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.
This commit is contained in:
Yyunozor
safishamsi
+19
-1
@@ -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",
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user