fix(cli): distinguish zero role matches from an unbuilt graph in roles --role - #2531
Merged
Conversation
…s --role (#2390) rolesData conflated "the graph has no classified symbols at all" with "this --role filter matched nothing" — both produced count:0, so a correctly built graph with e.g. no entry-classified symbols told the user to rebuild, which changes nothing. rolesData now also returns totalClassified (the classified count ignoring the role filter, respecting file/noTests), computed via one extra COUNT-style query only when a role filter is active. The CLI uses it to print "No symbols with role X. (N classified symbols in graph.)" instead of the rebuild suggestion whenever the graph is otherwise non-empty. docs check acknowledged Impact: 3 functions changed, 2 affected
Contributor
Greptile SummaryThe PR now distinguishes an unbuilt graph from valid role or file/test scopes with no matching classified symbols.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (3): Last reviewed commit: "fix(cli): label the role-specific classi..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
Codegraph Impact Analysis2 functions changed → 2 callers affected across 2 files
|
…verything (#2390) Greptile's review of the #2390 fix correctly flagged that totalClassified is itself scoped by --file/--no-tests, so a filter combination that excludes every classified symbol from that scope (an empty file, or a role that only exists in test files under -T) still fell through to the misleading rebuild message even though the graph is healthy outside that scope. Adds totalClassifiedUnscoped: a fully unfiltered (no role, no file, no noTests) count, computed only in the now-doubly-empty case. The CLI uses it to report which scope excluded everything instead of blaming the build. docs check acknowledged Impact: 3 functions changed, 2 affected
Contributor
Author
|
@greptileai please re-review — added totalClassifiedUnscoped to handle the file/no-tests scoped-zero case you flagged, so the CLI no longer recommends a rebuild when a --file/--no-tests scope excludes everything but the graph is healthy elsewhere. |
#2390) Greptile's second review round on the #2390 fix flagged that totalClassified stays scoped by --file/--no-tests even in the role-specific message branch, so combining --role with --file/--no-tests displayed that scoped count while calling it "classified symbols in graph" — inaccurate whenever those filters narrow the count below the true graph-wide total. The message now describes the actual scope ("in file X", "in non-test files", or both) instead of always claiming "in graph". docs check acknowledged Impact: 1 functions changed, 1 affected
Contributor
Author
|
@greptileai please re-review — fixed the scoped-count-mislabeled-as-graph-wide finding in d6afc74. Also replied inline on the other open comment: that one describes a scenario already fixed by the prior commit (e5de8c2), verified by 2 tests that fail without the fix — its diff_hunk shows it's anchored to the pre-e5de8c27 commit (a30efcd), so it looks like a re-surfaced round-1 finding rather than a new one. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2390
Problem
codegraph roles --role <X>with zero matches always saidNo classified symbols found. Run "codegraph build" first.— even on a fully and correctly built graph where the role simply has no members. Per the issue, this cost real time across 5 of 14 agents in an org rollout, each re-runningcodegraph buildfor nothing.rolesData'scountconflates two different states:--rolefilter matched nothing (the graph is fine).Fix
rolesData(src/domain/analysis/roles.ts) now also returnstotalClassified: the classified-symbol count ignoring the--rolefilter but still respecting the--file/-T(noTests) filters. It's computed via one extra lightweight query, only when a role filter is active (free — identical torows.length— in the common unfiltered case).The CLI (
src/presentation/queries-cli/overview.ts) uses this to print:instead of the misleading rebuild suggestion, whenever the graph is otherwise non-empty. The original message is preserved for the genuine zero-graph case.
No native/Rust changes — this is a TypeScript query/presentation-layer fix only;
domain/analysis/andpresentation/queries-cli/have no Rust mirror (the dual-engine parity requirement applies to graph extraction, not CLI query formatting).Scope note
While auditing
queries-cli/for the same conflation (per the issue's suggestion), found an identical bug incodegraph exports <file>(a file with legitimately zero exports gets told to rebuild). Filed separately as #2530 to keep this PR to one concern.Test plan
tests/integration/roles.test.ts: 3 new tests onrolesData—totalClassifiedmatches the unfiltered count when a role filter has zero matches, equalscountwhen no filter is applied, and respects thefilefilter.tests/presentation/queries-cli.test.ts: 2 new tests on the CLI'sroles()formatter — role-specific zero-match message when the graph is otherwise classified, and the original rebuild message preserved when the graph genuinely has none.npm test— 5313 passed.npm run lintclean.codegraph diff-impact --staged -T— bounded impact (2 files, 2 transitive callers, benchmark script + MCP passthrough only, both benign additive changes).