Skip to content

fix: group cut connections by file on explain for high-degree nodes (#2009)#2094

Closed
Yyunozor wants to merge 1 commit into
Graphify-Labs:v8from
Yyunozor:fix/explain-callers-aggregation
Closed

fix: group cut connections by file on explain for high-degree nodes (#2009)#2094
Yyunozor wants to merge 1 commit into
Graphify-Labs:v8from
Yyunozor:fix/explain-callers-aggregation

Conversation

@Yyunozor

Copy link
Copy Markdown
Contributor

Summary

graphify explain "<node>" on a high-degree node sorts connections by neighbor degree, shows the top 20, and appends a bare ... and N more for the rest — no file, no direction, no way to tell what was cut. The exact question explain exists to answer ("who calls this, what's the impact?") goes unanswered for the majority of callers on any node above degree ~20, and the fallback is a repo-wide grep — what the graph was meant to replace.

Reproduction

A node with 30 callers spread across 3 files (10 each):

$ graphify explain log_helper
...
Connections (30):
  <-- caller_0() [calls] [EXTRACTED] app/handlers/email.py:L10
  ... (19 more shown) ...
  ... and 10 more

The last 10 callers vanish with zero indication of where they live, even though all 3 files are still meaningfully represented in the cut set.

Root cause

cli.py's explain handler: the connections[:20] slice, followed by a bare ... and {len(connections) - 20} more count — no aggregation of any kind on the remainder.

Fix

Top-20 list and ordering unchanged (verified with a byte-for-byte regression test for nodes at/under the cutoff). Past it, cut connections are grouped by (direction, source_file) with counts, sorted by count descending:

  ... and 10 more
  Grouped by file:
    <-- lib/workers/queue.py: 4 connections
    <-- app/handlers/email.py: 3 connections
    <-- app/jobs/retry.py: 3 connections

The aggregation itself is capped at 20 files with its own ... and N more files line, so a pathological remainder (hundreds of distinct files) can't reproduce the same problem one level down.

Scope note

The issue also proposes --callers --all / --group-by=dir flags and untruncated output behind a flag — a larger, more opinionated surface (flag naming, grouping level, pagination) than the literal complaint needs. Left for a follow-up; the default output no longer hides the answer either way.

Validation

Four regression tests in tests/test_explain_cli.py: unchanged truncation notice, grouped-by-file counts summing back to the cut total (no silent loss), a no-op check at/under the cutoff, and a boundary check pinning it at exactly 20 vs. 21 connections.

pytest -q
3350 passed, 166 skipped, 13 failed

Parent commit (e32c9f4), same env: 3346 passed — delta is exactly these 4 tests, same 13 pre-existing failures both sides (missing optional tree-sitter-hcl/ollama extras, a built-wheel fixture — unrelated). ruff check clean. test_serve.py (#2080's tests) passes unchanged, 126/126.

`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 (Graphify-Labs#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.
@Yyunozor

Copy link
Copy Markdown
Contributor Author

Landed on v8 as 0ef452c, with the deterministic tie-break follow-up in 4c759b6 — closing as superseded. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant