Skip to content

feat(explain): add --limit so connections past the top 20 are readable - #2563

Open
lucadepascale wants to merge 1 commit into
Graphify-Labs:v8from
lucadepascale:feat/explain-limit
Open

feat(explain): add --limit so connections past the top 20 are readable#2563
lucadepascale wants to merge 1 commit into
Graphify-Labs:v8from
lucadepascale:feat/explain-limit

Conversation

@lucadepascale

Copy link
Copy Markdown

What

graphify explain prints the 20 most connected neighbours and then stops. This adds --limit N (and --limit=N) to raise that cut, with --limit 0 printing every connection.

The default stays 20, so output is byte-identical when the flag is absent.

Why

#2009 already improved what happens after the cut, by grouping the remainder per file. That answers where the rest are, but not what they are — and on a hub node the callers themselves stay unreachable from the CLI, so the only way to read them is the repo-wide grep the tool exists to avoid.

The MCP server never had this problem: it has a per-call token_budget. So this is purely about interactive use.

Details

  • --limit also bounds the grouped-by-file list, which carried the second hardcoded 20.
  • Non-integer and negative values exit 1 with a message, matching how --top and --max-examples already behave.
  • The "... and N more" line now names the flag, so the next person does not have to find it in --help.

Tests

Six new tests in tests/test_explain_cli.py, on a fixture hub node with 25 (and 40) callers:

  • the default cut is unchanged at 20 with ... and 5 more
  • --limit 25 shows all 25 and stops summarizing
  • --limit=0 shows all 40 and prints no grouped section
  • --limit 3 shows 3 and still summarizes the remaining 22 by file
  • a non-integer and a negative value each exit 1 with the right message

tests/test_explain_cli.py is green (20 passed).

Note unrelated to this change

On Windows, tests/test_build.py::test_semantic_rekey_relative_vs_absolute_source_file fails on a clean checkout of v8 (verified by stashing this branch's changes). It looks like a path-separator assumption rather than anything to do with explain; happy to open a separate issue if it is not already known.

`graphify explain` prints the 20 most connected neighbours and then stops.
Graphify-Labs#2009 improved what happens next by grouping the remainder per file, which
answers "where are they" but not "what are they": on a hub node the callers
themselves are unreachable from the CLI, and the only way to read them is the
repo-wide grep the tool exists to avoid.

`--limit N` (and `--limit=N`) raises the cut; `--limit 0` prints every
connection. The default stays 20, so output is byte-identical when the flag is
absent, and the same value bounds the grouped-by-file list, which carried the
second hardcoded 20.

Non-integer and negative values exit 1 with a message, matching how `--top` and
`--max-examples` already behave.

The MCP server is unaffected: it has a per-call `token_budget` and never had
this cap.

Six tests in tests/test_explain_cli.py: the unchanged default, a raised limit,
`--limit=0`, a limit below the default (which must still summarize the rest),
and both error paths.

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.


Graphify review — findings

This PR adds a --limit N option (also accepting --limit=N) to the graphify explain CLI command, letting users control how many connections are printed before the grouped-by-file summary, with 0 meaning print all and a default of 20. The change introduces a limit-parsing helper with validation for non-integer and negative values, applies the limit to both the connection list and the grouped-by-file summary, and updates the usage string and "more" message. A new test suite covers the default behavior, raising/lowering the cut, the 0 case, and the error paths, and the CHANGELOG notes the addition.

Worth a look

  • --limit without a value is silently ignoredgraphify/cli.py:1465 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 416 functions depend on the 270 functions this change touches.

Health — this change adds coupling hotspots:

  • worse: dispatch_command() — 2 callers, 120 callees

Verification — 416 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 387 function(s) in the blast radius were not formally verified this run

· 1 grounded finding(s) anchored inline below; 1 more finding(s) on lines outside this diff (see the check run).

Comment thread graphify/cli.py
Comment on lines +1469 to +1471
elif a == "--limit" and i + 1 < len(args):
limit = _explain_limit(args[i + 1])
elif a.startswith("--limit="):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--limit without a value is silently ignored — agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review

Graphify suggests a fix:

Suggested change
elif a == "--limit" and i + 1 < len(args):
limit = _explain_limit(args[i + 1])
elif a.startswith("--limit="):
elif a == "--limit":
if i + 1 >= len(args):
print("error: --limit requires a value", file=sys.stderr)
sys.exit(1)
limit = _explain_limit(args[i + 1])
elif a.startswith("--limit="):

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