-
-
Notifications
You must be signed in to change notification settings - Fork 11.2k
fix: short-circuit websearch for github_copilot provider #24143
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
6d0763b
b5a775d
3b12926
141ad04
32cb6f0
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 |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |
|
|
||
| import asyncio | ||
| import math | ||
| import uuid | ||
| from typing import Any, Dict, List, Optional, Tuple, Union, cast | ||
|
|
||
| import litellm | ||
|
|
@@ -28,6 +29,7 @@ | |
| WebSearchInterceptionConfig, | ||
| ) | ||
| from litellm.types.utils import LlmProviders | ||
| from litellm.utils import ProviderConfigManager | ||
|
|
||
|
|
||
| class WebSearchInterceptionLogger(CustomLogger): | ||
|
|
@@ -67,6 +69,111 @@ def __init__( | |
| self.search_tool_name = search_tool_name | ||
| self._request_has_websearch = False # Track if current request has web search | ||
|
|
||
| async def try_short_circuit_search( | ||
| self, | ||
| model: str, | ||
| messages: List[Dict], | ||
| tools: Optional[List[Dict]], | ||
| custom_llm_provider: Optional[str], | ||
| ) -> Optional[Dict[str, Any]]: | ||
| """ | ||
| Short-circuit web-search-only requests by executing the search directly. | ||
|
|
||
| Claude Code sends web search as a separate, standalone /v1/messages | ||
| request with a simple prompt and only web_search tool(s). For providers | ||
| that don't natively support web search (e.g. github_copilot), there is | ||
| no need to route this through the backend LLM — we can detect the | ||
| pattern, execute the search via Tavily/Perplexity, and return a | ||
| synthetic Anthropic response immediately. | ||
|
|
||
| Args: | ||
| model: Model name from the request | ||
| messages: Messages list from the request | ||
| tools: Tools list from the request | ||
| custom_llm_provider: Provider name | ||
|
|
||
| Returns: | ||
| An AnthropicMessagesResponse dict if short-circuited, or None to | ||
| continue normal processing. | ||
| """ | ||
| if not tools: | ||
| return None | ||
|
|
||
| # Check if provider is in enabled list | ||
| provider_str = custom_llm_provider or "" | ||
| if ( | ||
| self.enabled_providers is not None | ||
| and provider_str not in self.enabled_providers | ||
| ): | ||
| return None | ||
|
|
||
| # Only short-circuit for providers without native Anthropic Messages | ||
| # support. Providers that have a BaseAnthropicMessagesConfig (bedrock, | ||
| # vertex_ai, azure_ai, anthropic) already use the agentic loop, which | ||
| # includes a follow-up LLM call to synthesize the answer from search | ||
| # results. Short-circuiting those would skip that synthesis step and | ||
| # return raw search text — a regression for existing users. | ||
| try: | ||
| provider_enum = LlmProviders(provider_str) | ||
| anthropic_config = ( | ||
| ProviderConfigManager.get_provider_anthropic_messages_config( | ||
| model=model, provider=provider_enum | ||
| ) | ||
| ) | ||
| if anthropic_config is not None: | ||
| verbose_logger.debug( | ||
| f"WebSearchInterception: Skipping short-circuit for {provider_str} " | ||
| "(provider has native Anthropic Messages support, using agentic loop)" | ||
| ) | ||
| return None | ||
| except (ValueError, Exception): | ||
| pass # unknown provider enum → safe to short-circuit | ||
|
|
||
| # All tools must be web search tools | ||
| if not all(is_web_search_tool(t) for t in tools): | ||
| return None | ||
|
Comment on lines
+132
to
+134
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.
The root of the issue is that Consider introducing a separate
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. Fixed — the short-circuit now checks |
||
|
|
||
| # Extract search query from the last user message | ||
| from litellm.litellm_core_utils.prompt_templates.common_utils import ( | ||
| get_last_user_message, | ||
| ) | ||
|
|
||
| query = get_last_user_message(messages) | ||
| if not query: | ||
| return None | ||
|
Comment on lines
+99
to
+143
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.
The The subtle risk is that on a non- Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
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. Acknowledged — |
||
|
|
||
| verbose_logger.debug( | ||
| "WebSearchInterception: Short-circuit search detected " | ||
| f"(provider={provider_str}, query='{query}')" | ||
| ) | ||
|
|
||
| # Execute search | ||
| try: | ||
| search_result_text = await self._execute_search(query) | ||
| except Exception as e: | ||
| verbose_logger.error( | ||
| f"WebSearchInterception: Short-circuit search failed: {e}" | ||
| ) | ||
| search_result_text = f"Search failed: {e}" | ||
|
|
||
| # Build synthetic Anthropic response | ||
| response: Dict[str, Any] = { | ||
| "id": f"msg_{str(uuid.uuid4())}", | ||
| "type": "message", | ||
| "role": "assistant", | ||
| "model": model, | ||
| "content": [{"type": "text", "text": search_result_text}], | ||
| "stop_reason": "end_turn", | ||
| "stop_sequence": None, | ||
| "usage": {"input_tokens": 0, "output_tokens": 0}, | ||
| } | ||
|
Comment on lines
+159
to
+169
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.
The synthetic response hardcodes The existing # Rough token estimate: 1 token ≈ 4 chars
estimated_input = max(1, len(query) // 4)
estimated_output = max(1, len(search_result_text) // 4)
response: Dict[str, Any] = {
...
"usage": {"input_tokens": estimated_input, "output_tokens": estimated_output},
}
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. Intentional — no LLM call is made, so 0 tokens is accurate. Search provider costs are tracked separately. |
||
|
|
||
| verbose_logger.debug( | ||
| "WebSearchInterception: Short-circuit search completed, " | ||
| f"returning synthetic response ({len(search_result_text)} chars)" | ||
| ) | ||
| return response | ||
|
|
||
| async def async_pre_call_deployment_hook( | ||
| self, kwargs: Dict[str, Any], call_type: Optional[Any] | ||
| ) -> Optional[dict]: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -114,6 +114,54 @@ async def _execute_pre_request_hooks( | |
| return request_kwargs | ||
|
|
||
|
|
||
| async def _try_websearch_short_circuit( | ||
| model: str, | ||
| messages: List[Dict], | ||
| tools: Optional[List[Dict]], | ||
| custom_llm_provider: Optional[str], | ||
| stream: Optional[bool], | ||
| ) -> Optional[Union[AnthropicMessagesResponse, AsyncIterator]]: | ||
| """ | ||
| Attempt to short-circuit a web-search-only request. | ||
|
|
||
| Claude Code sends web search as a separate, standalone /v1/messages | ||
| request. For providers that don't natively support web search (e.g. | ||
| github_copilot), we detect this pattern, execute the search via | ||
| Tavily/Perplexity, and return a synthetic Anthropic response — bypassing | ||
| the backend LLM entirely. | ||
|
|
||
| Returns the synthetic response if short-circuited, or None to continue | ||
| normal processing. | ||
| """ | ||
| if not litellm.callbacks: | ||
| return None | ||
|
|
||
| from litellm.integrations.websearch_interception.handler import ( | ||
| WebSearchInterceptionLogger, | ||
| ) | ||
|
|
||
| for callback in litellm.callbacks: | ||
| if not isinstance(callback, WebSearchInterceptionLogger): | ||
| continue | ||
|
|
||
| response = await callback.try_short_circuit_search( | ||
| model=model, | ||
| messages=messages, | ||
| tools=tools, | ||
| custom_llm_provider=custom_llm_provider, | ||
| ) | ||
| if response is not None: | ||
| if stream: | ||
| from litellm.llms.anthropic.experimental_pass_through.messages.fake_stream_iterator import ( | ||
| FakeAnthropicMessagesStreamIterator, | ||
| ) | ||
|
|
||
| return FakeAnthropicMessagesStreamIterator(response) | ||
| return response | ||
|
|
||
| return None | ||
|
|
||
|
|
||
| @client | ||
| async def anthropic_messages( | ||
| max_tokens: int, | ||
|
|
@@ -138,6 +186,12 @@ async def anthropic_messages( | |
| """ | ||
| Async: Make llm api request in Anthropic /messages API spec | ||
| """ | ||
| # Save original stream flag before pre-request hooks can convert it. | ||
| # The websearch interception hook converts stream=True → stream=False | ||
| # for the agentic loop, but the short-circuit path needs to know | ||
| # whether the caller originally requested streaming. | ||
| original_stream = stream | ||
|
|
||
| # Execute pre-request hooks to allow CustomLoggers to modify request | ||
| request_kwargs = await _execute_pre_request_hooks( | ||
| model=model, | ||
|
|
@@ -151,11 +205,38 @@ async def anthropic_messages( | |
| # Extract modified parameters | ||
| tools = request_kwargs.pop("tools", tools) | ||
| stream = request_kwargs.pop("stream", stream) | ||
| # Propagate the provider derived inside pre-request hooks, if not already set. | ||
| # The litellm_params dict may have been overwritten by **kwargs in | ||
| # _execute_pre_request_hooks, so fall back to get_llm_provider() if needed. | ||
| if not custom_llm_provider: | ||
| custom_llm_provider = request_kwargs.get("litellm_params", {}).get( | ||
| "custom_llm_provider" | ||
| ) | ||
| if not custom_llm_provider: | ||
| try: | ||
| _, custom_llm_provider, _, _ = litellm.get_llm_provider(model=model) | ||
| except Exception: | ||
| pass | ||
| # Remove litellm_params from kwargs (only needed for hooks) | ||
| request_kwargs.pop("litellm_params", None) | ||
| # Merge back any other modifications | ||
| kwargs.update(request_kwargs) | ||
|
|
||
| # Short-circuit web-search-only requests: detect the pattern, execute | ||
| # search directly via Tavily/Perplexity, and return a synthetic response | ||
| # without ever touching the backend LLM or the adapter path. | ||
| # Use original_stream (not the hook-converted stream) so streaming | ||
| # callers get SSE events instead of a plain dict. | ||
| short_circuit_response = await _try_websearch_short_circuit( | ||
| model=model, | ||
| messages=messages, | ||
| tools=tools, | ||
| custom_llm_provider=custom_llm_provider, | ||
| stream=original_stream, | ||
| ) | ||
| if short_circuit_response is not None: | ||
| return short_circuit_response | ||
|
Comment on lines
+230
to
+238
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.
The As a result, by the time
The # Save original stream flag before hooks can convert it
original_stream = stream
request_kwargs = await _execute_pre_request_hooks(
model=model,
messages=messages,
tools=tools,
stream=stream,
custom_llm_provider=custom_llm_provider,
**kwargs,
)
tools = request_kwargs.pop("tools", tools)
stream = request_kwargs.pop("stream", stream)
request_kwargs.pop("litellm_params", None)
kwargs.update(request_kwargs)
short_circuit_response = await _try_websearch_short_circuit(
model=model,
messages=messages,
tools=tools,
custom_llm_provider=custom_llm_provider,
stream=original_stream, # use original, not the hook-converted value
)This bug is not covered by the new tests because
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. Fixed — we now save |
||
|
|
||
| loop = asyncio.get_event_loop() | ||
| kwargs["is_async"] = True | ||
|
|
||
|
|
||
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.
enabled_providers), bypassing the agentic loopThe provider guard uses
enabled_providers, which defaults to["bedrock"]. If a user hasenabled_providers=["bedrock"](the default) orenabled_providers=["bedrock", "github_copilot"], any bedrock request whose tools consist solely of web-search tools will hit this short-circuit and receive raw search text — skipping the follow-up LLM call that the existing agentic loop produces.For bedrock the expected flow is:
The short-circuit skips step 3 entirely, so callers get raw Tavily/Perplexity text instead of a synthesized answer. This is a behavioural regression for any existing bedrock user who has deployed
WebSearchInterceptionLogger.The PR description says "zero regression risk for Bedrock/Vertex" but the code does not include a guard to restrict short-circuiting to providers that genuinely lack native web-search support (e.g., a separate
short_circuit_providerslist or a capability flag inmodel_prices_and_context_window.json).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.
Fixed — added a guard that checks
ProviderConfigManager.get_provider_anthropic_messages_config(). Providers with native support (bedrock, vertex_ai, etc.) skip the short-circuit and use the existing agentic loop.