fix(cli): route callers/callees/impact through one symbol resolver - #1656
Open
ferrine wants to merge 1 commit into
Open
fix(cli): route callers/callees/impact through one symbol resolver#1656ferrine wants to merge 1 commit into
ferrine wants to merge 1 commit into
Conversation
`callers`, `callees` and `impact` each carried their own symbol filter,
comparing the user's query against the BARE node name only:
node.name === symbol || node.name.endsWith('.' + symbol)
That fails in two opposite directions in the same repository.
A bare name OVER-reports. Every same-named definition passes the filter
and their results are unioned under one heading — "Callers of group" can
list callers that belong to an entirely different `group`, with nothing
saying the name was ambiguous. Collisions cluster on short generic names
(`group`, `num`, `parse`), so this bites hardest exactly where the verbs
would otherwise be most useful.
A qualified name UNDER-reports. `Foo.Bar.baz` can never equal a bare
`baz`, so every candidate fails the filter, and the guarded fallback
takes whichever node full-text search ranked first — or reports "not
found" for a symbol that plainly exists. It only ever appeared to work
when FTS happened to return exactly one hit.
All three now resolve through graph/symbol-lookup, which the MCP tools
share, so a verb cannot drift from the matcher again:
- the exact-name index is consulted first and is authoritative. It is
complete and uncapped, whereas FTS ranks, truncates, and tokenises
`::` away — so resolution no longer depends on search ranking. FTS
stays as the fallback for the fuzzy cases it is good at.
- `matchesSymbol` gains a boundary-aligned suffix match under a
canonical separator. Splitting on every separator assumes no scope
component contains one, which is false for any language whose module
names are themselves dotted: the stored `A.B::c` can never equal the
split-and-rejoined `A::B::c`, so a precise query resolved to nothing.
- an ambiguous bare name still aggregates (an interface method and its
overrides are usually all wanted) but the union is now disclosed, with
the matched definitions named and a qualified spelling that narrows
it. `--json` gains a `targets` array and an `ambiguous` flag.
`matchesSymbol` moves out of mcp/tools.ts unchanged apart from the new
stage; the tool path delegates to it, so its existing coverage applies.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013Bsi9EH64kMisnik4E1gf7
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Branch:
ferrine:fix/symbol-lookup-consistency→mainSize: 5 files, +453/−110 · Tests:
__tests__/symbol-lookup.test.ts(28 new)Why
codegraph callers,calleesandimpacteach resolved the symbol name theirown way, and none of them understood a qualified name. Asking for something like
Accounts.Format.groupmatched nothing, so the command quietly fell back towhichever symbol full-text search happened to rank first — or reported "not
found" for a symbol that plainly exists.
Two root causes, and the second is the one that made it look intermittent:
matched.
::, so a language whose module names containdots (Elixir, protobuf FQNs, C# namespaces) could not be addressed at all —
A.B.Cis genuinely ambiguous between "containerA.B, memberC" and amodule literally named
A.B.C.Because the fallback was FTS ranking, the wrong answer arrived with no signal
that it was a guess.
What changed
src/graph/symbol-lookup.ts— one "what did the user mean by this name?"path shared by all three commands.
uses FTS only as a fallback, so a name that exists is never missed because
ranking put it past a cap.
written
A.B.CorA.B::C.so and print the qualified spelling that narrows it, instead of silently
merging unrelated symbols into one list.
Notes for review
cover the previously-broken shapes.
matching, and the multi-definition diversity fix builds on the same path).