Repository navigation
fix(tools): tell the model the retrieval query is required - #7678
Open
roydonsequeira wants to merge 1 commit into
Open
roydonsequeira wants to merge 1 commit into
roydonsequeira wants to merge 1 commit into
Conversation
BaseRetrievalTool declared its only parameter, query, without listing it as required, so LlamaIndexRetrieval, FilesRetrieval and VertexAiRagRetrieval on non-Gemini models told the model the query was optional. None of them can run without one, and a call that leaves it out fails the run with KeyError or ValueError. List query as required in both declaration forms, as load_memory does. Fixes google#7677
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.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
queryas optional, and a call without it fails the run #7677Problem:
BaseRetrievalTooldeclared its only parameter,query, without listing it inrequired, in both the JSON schema and thetypes.Schemaform. Every tool built on it told the model the query was optional:LlamaIndexRetrievalandFilesRetrievalwith any model, andVertexAiRagRetrievalwith non-Gemini models. None of them can run without one.LlamaIndexRetrievalraisesKeyError,VertexAiRagRetrievalraisesValueError, and the exception fails the run.Solution:
List
queryinrequiredin both forms, asload_memoryalready does for its ownqueryparameter. The model then knows it has to send a query, and the Gemini API'sVALIDATEDfunction calling mode, which keeps calls to the declared schema, no longer allows a call without one. What the tools do with a call that still leaves the query out is unchanged.Testing Plan
Unit Tests:
test_get_declaration_with_json_schema_feature_disabledandtest_get_declaration_with_json_schema_feature_enabledintests/unittests/tools/retrieval/test_base_retrieval_tool.pynow checkrequiredin each form. Both fail onmainand pass with the fix.Full suite on Windows 11, Python 3.11 (
pytest tests/unittests -n 4): 71 failed, 20782 passed, 107 skipped. Every one of the 71 also fails on unmodifiedmainin this environment, and none of them is intools/retrieval/.Manual End-to-End (E2E) Tests:
An
Agentwith aLlamaIndexRetrievaltool, run throughInMemoryRunnerwith a model that records each request. The run prints the tool parameters ADK sends, and the same declaration converted byLiteLlm:Checklist
Additional context
The formatters, linters and repository checks from
.pre-commit-config.yamlpass on both files, and mypy reports no errors inbase_retrieval_tool.py. The change does not overlap with #7675, which touchesllama_index_retrieval.py.