Repository navigation
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
…ypass path to DiscoveryEngineSearchTool When bypass_multi_tools_limit=True converts VertexAiSearchTool to a client-side DiscoveryEngineSearchTool, the resulting tool was always named 'discovery_engine_search' — an internal implementation name. Developers had no way to express a domain-specific tool name, forcing them to leak GCP internals into agent instructions or accept runtime ValueError: Tool 'search' not found crashes. DiscoveryEngineSearchTool already accepted name/description params but they were unreachable through the conversion path. - Add optional name/description kwargs to VertexAiSearchTool.__init__ stored as _bypass_tool_name/_bypass_tool_description to never shadow the base-class 'vertex_ai_search' name used in the grounding path - Forward them in llm_agent.py during the bypass conversion - Add 5 unit tests for the agent conversion path - Add 4 unit tests for DiscoveryEngineSearchTool constructor Fixes: google#7100 (partial — addresses problem 3, hardcoded tool naming) Related: google#7101
18b4a3c to
a25cde5
Compare
|
@googlebot I signed it! |
|
Just to clarify the scope of this PR: it intentionally addresses only Problem 3 from #7100—the hardcoded name and description used when The change allows developers to provide a domain-specific tool name and description through I kept this PR deliberately focused. It does not change the underlying retrieval architecture, add fuzzy tool-name matching, or address the optional GCP dependency behavior. I also noticed that #7121 overlaps with this area. I’m happy to adjust, rebase, or narrow this PR further depending on which implementation direction the maintainers prefer. The focused conversion and constructor tests are included and passing. Please let me know if any additional clarification or test coverage would help with the review. |
Description
Addresses Problem 3 identified in #7100.
When
bypass_multi_tools_limit=Trueis enabled onVertexAiSearchTool, ADK converts the tool to a client-sideDiscoveryEngineSearchToolduring canonical tool resolution. Previously, this conversion always named the resulting tool'discovery_engine_search'-- an internal implementation detail.This caused two key problems for developers:
'discovery_engine_search'explicitly in prompts rather than domain concepts like'product_search'or'knowledge_base'.searchorquery) instead ofdiscovery_engine_search, leading toValueError: Tool '...' not found.While
DiscoveryEngineSearchToolalready supportednameanddescriptionparameters, they were unreachable through theVertexAiSearchToolconversion path.Changes
nameanddescriptionparameters toVertexAiSearchTool.__init__._bypass_tool_nameand_bypass_tool_descriptionto avoid shadowingBaseTool.name = 'vertex_ai_search'(preserving backward compatibility with the built-in grounding path).nameanddescriptionduring bypass conversion inllm_agent.py.tests/unittests/agents/test_llm_agent_fields.py.tests/unittests/tools/test_discovery_engine_search_tool.py.Related Issues
bypass_multi_tools_limitonVertexAiSearchToolis a leaky abstraction: unexpected dependencies, silent shift from inbuilt RAG to tool calling, and prompt fragility #7100 (Problem 3: Hardcoded tool name)google-cloud-discoveryengineimport errors)Testing Plan
DiscoveryEngineSearchTool(pytest tests/unittests/tools/test_discovery_engine_search_tool.py) -- 31/31 passed.-[x] Ran unit tests for VAIS tool conversion (
pytest tests/unittests/agents/test_llm_agent_fields.py -k "vais") -- 10/10 passed.