fix(cli): warn when a symbol name is ambiguous - #72
Merged
Conversation
find_definition returns every symbol matching a name and the commands acted on defs[0], so an ambiguous name was answered for a symbol the user did not choose -- and nothing in the output said which one. An empty result then reads as "this function has no blocks" rather than "I looked at the wrong function". $ ast-rag blocks main No blocks found. while another `main` in the same repo had 45 blocks. The issue reported this for `blocks`. It was six sites: callers, call-graph, symbol-impact, blocks, summarize, and the sandbox find_callers tool. All now route through one helper. $ ast-rag blocks main ambiguous: 'main' matched 3 symbols (3 python). Reporting on embedding_server.main. Re-run with a qualified name to pick another, for example server.main, watcher_service.main No blocks found. Per the discussion on the issue the note leads with "ambiguous" so an agent reading the output hits it first, carries a per-language breakdown, and uses no exclamation mark. Behaviour is otherwise unchanged: the unambiguous case stays a single command and defs[0] is still used, so nothing that worked before now fails. Left out: the --filter idea from the same comment. That thread carries an open "TODO research about needed filter params", so it wants a decision on the filter vocabulary first rather than a guess here.
10 tasks
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
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.
Description
find_definitionreturns every symbol matching a name; the commands tookdefs[0]and reported on that one. For names that are ambiguous by nature —main,run,parse,__init__— the answer is usually about a symbol theuser did not mean, and nothing in the output said which one was chosen.
Before:
…while another
mainin the same repo had 45 blocks.After:
Related Issue
Fixes #64
Type of Change
Following your direction on the issue
You asked for a stat and a message suggesting how to narrow the query, with
"ambiguous" at the beginning so an LLM reading the output hits it first, and
without the exclamation mark. That is what this does: leading
ambiguous:,the match count, a per-language breakdown, the symbol actually used, and up to
three alternatives to re-run with.
--filteris deliberately not here. Your comment carries an open"TODO research about needed filter params", so the filter vocabulary is a
decision rather than something to guess at in a bug fix. Happy to do it as a
follow-up once you have settled the shape.
It was six sites, not one
The issue reported
blocks. The same silentdefs[0]was in:callerscall-graphsymbol-impactfind_callerstoolblockssummarizeAll six now route through one
_warn_if_ambiguoushelper.Compatibility
Behaviour is otherwise unchanged. The unambiguous case stays a single command
with no extra output, and
defs[0]is still what gets used — so nothing thatworked before now fails or changes its result. The note is additive.
Checklist
pytest tests/ -v)ast-rag evaluate --all— needs the summarizer LLM, see belowTesting
Five unit tests on the note itself, covering the wording rules from your
comment (leads with "ambiguous", no
!), the count, the per-languagebreakdown, and that it names both the chosen symbol and an alternative. They
fail on
mainwithImportErrorsince the helper does not exist there.main@ 41e48af)+5, the new tests.
Verified live against Neo4j 5.18 with this repo indexed — that is where the
"3 symbols (3 python)" output above comes from.
ast-rag evaluate --allnot run: it needs the summarizer LLM and I have nomodel serving locally. This change only adds an advisory line to command
output and does not touch scoring.
Note on ordering
This sits on top of
main, so it is independent of #70 and #71 — merge in anyorder. If #71 lands first there will be a small context overlap in
cli.pyaround
_build_api; I am happy to rebase whichever comes second.