-
-
Notifications
You must be signed in to change notification settings - Fork 11.6k
fix: merge websearch tool params #32162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4b5d988
fcffb09
d6fefff
56b0610
dba92eb
48e581e
d1c51bd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -91,6 +91,7 @@ async def try_short_circuit_search( | |
| messages: List[Dict], | ||
| tools: Optional[List[Dict]], | ||
| custom_llm_provider: Optional[str], | ||
| kwargs: Optional[dict[str, Any]] = None, | ||
| ) -> Optional[Dict[str, Any]]: | ||
| """ | ||
| Short-circuit web-search-only requests by executing the search directly. | ||
|
|
@@ -176,7 +177,10 @@ async def try_short_circuit_search( | |
| # Execute search — keep the structured SearchResponse so the native | ||
| # block can carry per-result url/title/page_age. | ||
| try: | ||
| search_result_text, structured = await self._execute_search(query) | ||
| if kwargs is None: | ||
| search_result_text, structured = await self._execute_search(query) | ||
| else: | ||
| search_result_text, structured = await self._execute_search(query, kwargs=kwargs) | ||
| except Exception as e: | ||
| verbose_logger.error(f"WebSearchInterception: Short-circuit search failed: {e}") | ||
| search_result_text, structured = f"Search failed: {e}", None | ||
|
|
@@ -936,7 +940,7 @@ async def _build_anthropic_request_patch( | |
| query = tool_call["input"].get("query") | ||
| if query: | ||
| verbose_logger.debug(f"WebSearchInterception: Queuing search for query='{query}'") | ||
| search_tasks.append(self._execute_search(query)) | ||
| search_tasks.append(self._execute_search(query, kwargs=kwargs)) | ||
| else: | ||
| verbose_logger.debug(f"WebSearchInterception: Tool call {tool_call['id']} has no query") | ||
| # Add empty result for tools without query | ||
|
|
@@ -1009,7 +1013,9 @@ async def _build_anthropic_request_patch( | |
| ) | ||
| return patch, structured_results | ||
|
|
||
| async def _execute_search(self, query: str) -> Tuple[str, Optional[SearchResponse]]: | ||
| async def _execute_search( | ||
| self, query: str, kwargs: Optional[dict[str, Any]] = None | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm noticing many introduced kwargs. Are these necessary? Can we just introduce named params?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes - this fixes the issue where litellm params were not being passed through. since litellm params can be a broad dict, this is appropriate imo
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Also, I'm seeing: FAILED tests/test_litellm/proxy/common_utils/test_key_rotation_e2e.py::TestDeprecatedKeyLookupDbE2E::test_deprecated_key_grace_period_cache_hit_path - httpx.ConnectError: All connection attempts failed. Deterministic failure or flake? this seems like flake, since it's unrelated to the change here
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see the issue if the tests pass, LGTM |
||
| ) -> Tuple[str, Optional[SearchResponse]]: | ||
| """ | ||
| Execute a single web search using router's search tools. | ||
|
|
||
|
|
@@ -1031,36 +1037,13 @@ async def _execute_search(self, query: str) -> Tuple[str, Optional[SearchRespons | |
| ) | ||
| llm_router = None | ||
|
|
||
| # Determine search provider from router's search_tools | ||
| search_tool = self._select_search_tool_from_router(llm_router=llm_router) | ||
| search_provider: Optional[str] = None | ||
| if llm_router is not None and hasattr(llm_router, "search_tools"): | ||
| if self.search_tool_name: | ||
| # Find specific search tool by name | ||
| matching_tools = [ | ||
| tool | ||
| for tool in llm_router.search_tools | ||
| if tool.get("search_tool_name") == self.search_tool_name | ||
| ] | ||
| if matching_tools: | ||
| search_tool = matching_tools[0] | ||
| search_provider = search_tool.get("litellm_params", {}).get("search_provider") | ||
| verbose_logger.debug( | ||
| f"WebSearchInterception: Found search tool '{self.search_tool_name}' " | ||
| f"with provider '{search_provider}'" | ||
| ) | ||
| else: | ||
| verbose_logger.debug( | ||
| f"WebSearchInterception: Search tool '{self.search_tool_name}' not found in router, " | ||
| "falling back to first available or perplexity" | ||
| ) | ||
|
|
||
| # If no specific tool or not found, use first available | ||
| if not search_provider and llm_router.search_tools: | ||
| first_tool = llm_router.search_tools[0] | ||
| search_provider = first_tool.get("litellm_params", {}).get("search_provider") | ||
| verbose_logger.debug( | ||
| f"WebSearchInterception: Using first available search tool with provider '{search_provider}'" | ||
| ) | ||
| search_litellm_params: dict[str, Any] = {} | ||
| if search_tool is not None: | ||
| await self._authorize_search_tool(search_tool=search_tool, kwargs=kwargs) | ||
| search_litellm_params = dict(search_tool.get("litellm_params", {}) or {}) | ||
| search_provider = search_litellm_params.get("search_provider") | ||
|
|
||
| # Fallback to perplexity if no router or no search tools configured | ||
| if not search_provider: | ||
|
|
@@ -1073,7 +1056,12 @@ async def _execute_search(self, query: str) -> Tuple[str, Optional[SearchRespons | |
| verbose_logger.debug( | ||
| f"WebSearchInterception: Executing search for '{query}' using provider '{search_provider}'" | ||
| ) | ||
| result = await litellm.asearch(query=query, search_provider=search_provider) | ||
| search_kwargs = { | ||
| key: value | ||
| for key, value in search_litellm_params.items() | ||
| if key != "search_provider" and value is not None | ||
| } | ||
| result = await litellm.asearch(query=query, search_provider=search_provider, **search_kwargs) | ||
|
veria-ai[bot] marked this conversation as resolved.
|
||
|
|
||
| # Format using transformation function | ||
| search_result_text = WebSearchTransformation.format_search_response(result) | ||
|
|
@@ -1086,6 +1074,107 @@ async def _execute_search(self, query: str) -> Tuple[str, Optional[SearchRespons | |
| verbose_logger.error(f"WebSearchInterception: Search failed for '{query}': {str(e)}") | ||
| raise | ||
|
|
||
| async def _authorize_search_tool( | ||
| self, | ||
| search_tool: dict[str, Any], | ||
| kwargs: Optional[dict[str, Any]], | ||
| ) -> None: | ||
| search_tool_name = search_tool.get("search_tool_name") | ||
| if not isinstance(search_tool_name, str) or not search_tool_name: | ||
| return | ||
|
|
||
| user_api_key_auth = self._get_user_api_key_auth_from_kwargs(kwargs) | ||
| if user_api_key_auth is None: | ||
| return | ||
|
|
||
| from litellm.proxy.auth.auth_checks import ( | ||
| can_key_call_search_tool, | ||
| can_team_call_search_tool, | ||
| get_team_object, | ||
| ) | ||
|
|
||
| await can_key_call_search_tool( | ||
| search_tool_name=search_tool_name, | ||
| valid_token=user_api_key_auth, | ||
| ) | ||
|
|
||
| team_id = getattr(user_api_key_auth, "team_id", None) | ||
| if team_id: | ||
| from litellm.proxy.proxy_server import ( | ||
| prisma_client, | ||
| proxy_logging_obj, | ||
| user_api_key_cache, | ||
| ) | ||
|
|
||
| team_object = await get_team_object( | ||
| team_id=team_id, | ||
| prisma_client=prisma_client, | ||
| user_api_key_cache=user_api_key_cache, | ||
| parent_otel_span=getattr(user_api_key_auth, "parent_otel_span", None), | ||
| proxy_logging_obj=proxy_logging_obj, | ||
| ) | ||
| await can_team_call_search_tool( | ||
| search_tool_name=search_tool_name, | ||
| team_object=team_object, | ||
| ) | ||
|
|
||
| @staticmethod | ||
| def _get_user_api_key_auth_from_kwargs(kwargs: Optional[dict[str, Any]]) -> Any: | ||
| if not kwargs: | ||
| return None | ||
|
|
||
| for metadata_key in ("metadata", "litellm_metadata"): | ||
| metadata = kwargs.get(metadata_key) | ||
| if isinstance(metadata, dict) and metadata.get("user_api_key_auth") is not None: | ||
| return metadata["user_api_key_auth"] | ||
|
|
||
| litellm_params = kwargs.get("litellm_params") | ||
| if not isinstance(litellm_params, dict): | ||
| return None | ||
|
|
||
| for metadata_key in ("metadata", "litellm_metadata"): | ||
| metadata = litellm_params.get(metadata_key) | ||
| if isinstance(metadata, dict) and metadata.get("user_api_key_auth") is not None: | ||
| return metadata["user_api_key_auth"] | ||
|
|
||
| return None | ||
|
|
||
| def _select_search_tool_from_router(self, llm_router: Any) -> Optional[dict[str, Any]]: | ||
| if llm_router is None or not hasattr(llm_router, "search_tools"): | ||
| return None | ||
| search_tools = list(getattr(llm_router, "search_tools") or []) | ||
| return self._select_search_tool_from_list(search_tools=search_tools, source="router") | ||
|
|
||
| def _select_search_tool_from_list( | ||
| self, | ||
| search_tools: list[dict[str, Any]], | ||
| source: str, | ||
| ) -> Optional[dict[str, Any]]: | ||
| if self.search_tool_name: | ||
| matching_tools = [tool for tool in search_tools if tool.get("search_tool_name") == self.search_tool_name] | ||
| if matching_tools: | ||
| search_provider = (matching_tools[0].get("litellm_params", {}) or {}).get("search_provider") | ||
| verbose_logger.debug( | ||
| f"WebSearchInterception: Found search tool '{self.search_tool_name}' " | ||
| f"from {source} with provider '{search_provider}'" | ||
| ) | ||
| return matching_tools[0] | ||
| verbose_logger.debug( | ||
| f"WebSearchInterception: Search tool '{self.search_tool_name}' not found in {source}, " | ||
| "falling back to first available or perplexity" | ||
| ) | ||
|
|
||
| if search_tools: | ||
| first_tool = search_tools[0] | ||
| search_provider = (first_tool.get("litellm_params", {}) or {}).get("search_provider") | ||
| verbose_logger.debug( | ||
| f"WebSearchInterception: Using first available search tool from {source} " | ||
| f"with provider '{search_provider}'" | ||
| ) | ||
| return first_tool | ||
|
|
||
| return None | ||
|
|
||
| async def _execute_chat_completion_agentic_loop( | ||
| self, | ||
| model: str, | ||
|
|
@@ -1145,7 +1234,7 @@ async def _build_chat_completion_request_patch( | |
|
|
||
| if query: | ||
| verbose_logger.debug(f"WebSearchInterception: Queuing search for query='{query}'") | ||
| search_tasks.append(self._execute_search(query)) | ||
| search_tasks.append(self._execute_search(query, kwargs=kwargs)) | ||
| else: | ||
| verbose_logger.debug(f"WebSearchInterception: Tool call {tool_call.get('id')} has no query") | ||
| # Add empty result for tools without query | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_execute_searchcan now raiseProxyExceptionfrom_authorize_search_toolwhen a key or team lacks permission for a search tool. That exception is a subclass ofExceptionand therefore falls into the existing catch-all block, which converts it into a"Search failed: ..."text string. The client receives a 200 OK with a failed search message instead of a proper authorization error — and the proxy logs the rejection aterrorlevel rather than surfacing it to the caller. The agentic-loop path (gatheredsearch_tasks) lets the exception propagate normally, so there is already an inconsistency between the two paths.