From 008787732f254ae5669a14f5f2752d4bf184fc79 Mon Sep 17 00:00:00 2001 From: Jonathan Wrede Date: Tue, 26 May 2026 08:24:03 +0000 Subject: [PATCH] fix(proxy): emit correct OpenAI tool schema from semantic MCP filter (#28766) The semantic MCP tool filter called _process_mcp_tools_to_openai_format with the default target_format="responses", producing flat tool dicts. Chat completions requires the nested {type: "function", function: {...}} wrapper. Pass call_type through so _expand_mcp_tools selects the correct format, and update _extract_tool_info / _get_tool_names_csv to handle both shapes. Co-Authored-By: Claude Opus 4.6 --- .../mcp_server/semantic_tool_filter.py | 19 +- .../proxy/hooks/mcp_semantic_filter/hook.py | 52 ++- .../mcp_server/test_semantic_tool_filter.py | 331 ++++++++++++++++-- 3 files changed, 362 insertions(+), 40 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/semantic_tool_filter.py b/litellm/proxy/_experimental/mcp_server/semantic_tool_filter.py index a9c4d2ece46..504fd563088 100644 --- a/litellm/proxy/_experimental/mcp_server/semantic_tool_filter.py +++ b/litellm/proxy/_experimental/mcp_server/semantic_tool_filter.py @@ -88,14 +88,25 @@ class SemanticMCPToolFilter: raise def _extract_tool_info(self, tool) -> tuple[str, str]: - """Extract name and description from MCP tool or OpenAI function dict.""" + """Extract name and description from MCP tool or OpenAI tool dict. + + Handles both OpenAI formats: + - Chat format: {"type": "function", "function": {"name": ..., "description": ...}} + - Responses format: {"name": ..., "description": ..., "type": "function"} + """ name: str description: str if isinstance(tool, dict): - # OpenAI function format - name = tool.get("name", "") - description = tool.get("description", name) + func = tool.get("function") + if isinstance(func, dict): + # Chat completions format with nested "function" key + name = func.get("name", "") + description = func.get("description", name) + else: + # Responses API flat format + name = tool.get("name", "") + description = tool.get("description", name) else: # MCPTool object name = str(tool.name) diff --git a/litellm/proxy/hooks/mcp_semantic_filter/hook.py b/litellm/proxy/hooks/mcp_semantic_filter/hook.py index 6343faaa965..102577ba582 100644 --- a/litellm/proxy/hooks/mcp_semantic_filter/hook.py +++ b/litellm/proxy/hooks/mcp_semantic_filter/hook.py @@ -5,7 +5,7 @@ Pre-call hook that filters MCP tools semantically before LLM inference. Reduces context window size and improves tool selection accuracy. """ -from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union +from typing import TYPE_CHECKING, Any, Dict, List, Literal, Optional, Union from litellm._logging import verbose_proxy_logger from litellm.constants import ( @@ -65,12 +65,14 @@ class SemanticToolFilterHook(CustomLogger): self, tools: List[Any], user_api_key_dict: "UserAPIKeyAuth", + call_type: str = "acompletion", ) -> List[Dict[str, Any]]: """ Expand MCP references to actual tool definitions. - Reuses LiteLLM_Proxy_MCP_Handler._process_mcp_tools_to_openai_format - which internally does: parse -> fetch -> filter -> deduplicate -> transform + Uses the correct OpenAI format based on call_type: + - "completion"/"acompletion" -> chat format with {type: "function", function: {...}} wrapper + - "aresponses" -> responses format with flat {name, parameters, type: "function", ...} """ from litellm.responses.mcp.litellm_proxy_mcp_handler import ( LiteLLM_Proxy_MCP_Handler, @@ -82,13 +84,23 @@ class SemanticToolFilterHook(CustomLogger): if not mcp_tools: return [] - # Use single combined method instead of 3 separate calls - # This already handles: fetch -> filter by allowed_tools -> deduplicate -> transform + # Fetch and filter MCP tools without format transformation ( - openai_tools, + deduplicated_mcp_tools, _, - ) = await LiteLLM_Proxy_MCP_Handler._process_mcp_tools_to_openai_format( - user_api_key_auth=user_api_key_dict, mcp_tools_with_litellm_proxy=mcp_tools + ) = await LiteLLM_Proxy_MCP_Handler._process_mcp_tools_without_openai_transform( + user_api_key_auth=user_api_key_dict, + mcp_tools_with_litellm_proxy=mcp_tools, + ) + + # Select the correct format for the endpoint + target_format: Literal["responses", "chat"] = "responses" + if call_type in ("completion", "acompletion"): + target_format = "chat" + + openai_tools = LiteLLM_Proxy_MCP_Handler._transform_mcp_tools_to_openai( + deduplicated_mcp_tools, + target_format=target_format, ) # Convert Pydantic models to dicts for compatibility @@ -172,7 +184,9 @@ class SemanticToolFilterHook(CustomLogger): ) try: - expanded_tools = await self._expand_mcp_tools(tools, user_api_key_dict) + expanded_tools = await self._expand_mcp_tools( + tools, user_api_key_dict, call_type=call_type + ) if not expanded_tools: verbose_proxy_logger.warning( @@ -288,17 +302,25 @@ class SemanticToolFilterHook(CustomLogger): return headers def _get_tool_names_csv(self, tools: List[Any]) -> str: - """Extract tool names and return as CSV string.""" + """Extract tool names and return as CSV string. + + Handles both OpenAI formats: + - Chat format: {"type": "function", "function": {"name": ...}} + - Responses format: {"name": ..., "type": "function"} + """ if not tools: return "" tool_names = [] for tool in tools: - name = ( - tool.get("name", "") - if isinstance(tool, dict) - else getattr(tool, "name", "") - ) + if isinstance(tool, dict): + func = tool.get("function") + if isinstance(func, dict): + name = func.get("name", "") + else: + name = tool.get("name", "") + else: + name = getattr(tool, "name", "") if name: tool_names.append(name) diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_semantic_tool_filter.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_semantic_tool_filter.py index 2558df8533b..121ff85aa75 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_semantic_tool_filter.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_semantic_tool_filter.py @@ -489,9 +489,7 @@ class TestGetToolsByNames: {"name": "send_email", "description": "send mail"}, ] - matched = filter_instance._get_tools_by_names( - ["send_email"], available_tools - ) + matched = filter_instance._get_tools_by_names(["send_email"], available_tools) assert len(matched) == 1 assert matched[0]["name"] == "send_email" @@ -503,9 +501,7 @@ class TestGetToolsByNames: client_name = "litellm_" + canonical available_tools = [{"name": client_name, "description": "scrape"}] - matched = filter_instance._get_tools_by_names( - [canonical], available_tools - ) + matched = filter_instance._get_tools_by_names([canonical], available_tools) assert len(matched) == 1 # Must return the incoming tool unchanged so the client-facing @@ -516,13 +512,9 @@ class TestGetToolsByNames: """Some clients use dash as alias separator; accept that too.""" filter_instance = self._make_filter() canonical = "weather_svc-get_weather" - available_tools = [ - {"name": "mcp-" + canonical, "description": "weather"} - ] + available_tools = [{"name": "mcp-" + canonical, "description": "weather"}] - matched = filter_instance._get_tools_by_names( - [canonical], available_tools - ) + matched = filter_instance._get_tools_by_names([canonical], available_tools) assert len(matched) == 1 assert matched[0]["name"] == "mcp-" + canonical @@ -552,9 +544,7 @@ class TestGetToolsByNames: {"name": "litellm_" + canonical, "description": "wrapped"}, ] - matched = filter_instance._get_tools_by_names( - [canonical], available_tools - ) + matched = filter_instance._get_tools_by_names([canonical], available_tools) assert len(matched) == 1 assert matched[0]["name"] == canonical @@ -567,9 +557,7 @@ class TestGetToolsByNames: separator-anchored suffixes of ``litellm_api-fs-read_file``. """ filter_instance = self._make_filter() - available_tools = [ - {"name": "litellm_api-fs-read_file", "description": "read"} - ] + available_tools = [{"name": "litellm_api-fs-read_file", "description": "read"}] matched = filter_instance._get_tools_by_names( ["fs-read_file", "api-fs-read_file"], available_tools @@ -590,9 +578,7 @@ class TestGetToolsByNames: {"name": "my_" + canonical, "description": "plain search"}, ] - matched = filter_instance._get_tools_by_names( - [canonical], available_tools - ) + matched = filter_instance._get_tools_by_names([canonical], available_tools) assert len(matched) == 1 assert matched[0]["name"] == "my_" + canonical @@ -640,3 +626,306 @@ class TestGetToolsByNames: ) assert matched == [] + + def test_chat_format_tools_matched_correctly(self): + """ + Tools in OpenAI chat completions format (wrapped in + {"type": "function", "function": {...}}) must be matched by + the name inside the "function" dict. + + Regression test for #28766: semantic filter could not extract + names from the chat format wrapper, causing empty matches. + """ + filter_instance = self._make_filter() + available_tools = [ + { + "type": "function", + "function": { + "name": "searxng-search", + "description": "Search the web", + "parameters": {"type": "object", "properties": {}}, + }, + }, + { + "type": "function", + "function": { + "name": "tavily-tavily_search", + "description": "Tavily search", + "parameters": {"type": "object", "properties": {}}, + }, + }, + ] + + matched = filter_instance._get_tools_by_names( + ["searxng-search"], available_tools + ) + + assert len(matched) == 1 + assert matched[0]["function"]["name"] == "searxng-search" + + +class TestExtractToolInfo: + """ + Tests for _extract_tool_info handling both OpenAI tool formats. + + Regression coverage for #28766: the semantic filter must correctly + extract name/description from both the chat completions format + (nested "function" key) and the responses API format (flat dict). + """ + + def _make_filter(self): + from litellm.proxy._experimental.mcp_server.semantic_tool_filter import ( + SemanticMCPToolFilter, + ) + + return SemanticMCPToolFilter( + embedding_model="text-embedding-3-small", + litellm_router_instance=Mock(), + top_k=5, + similarity_threshold=0.3, + enabled=True, + ) + + def test_responses_api_format(self): + """Flat dict with name/description at top level (responses API).""" + f = self._make_filter() + tool = { + "name": "web_search", + "description": "Search the web", + "parameters": {"type": "object"}, + "type": "function", + "strict": False, + } + + name, desc = f._extract_tool_info(tool) + + assert name == "web_search" + assert desc == "Search the web" + + def test_chat_completions_format(self): + """Nested dict with {type: "function", function: {name, description, ...}}.""" + f = self._make_filter() + tool = { + "type": "function", + "function": { + "name": "searxng-search", + "description": "Search via SearXNG", + "parameters": {"type": "object", "properties": {}}, + }, + } + + name, desc = f._extract_tool_info(tool) + + assert name == "searxng-search" + assert desc == "Search via SearXNG" + + def test_chat_format_missing_description_falls_back_to_name(self): + """Chat format tool without description uses name as fallback.""" + f = self._make_filter() + tool = { + "type": "function", + "function": { + "name": "my_tool", + "parameters": {"type": "object"}, + }, + } + + name, desc = f._extract_tool_info(tool) + + assert name == "my_tool" + assert desc == "my_tool" + + def test_mcp_tool_object(self): + """Native MCPTool object.""" + f = self._make_filter() + tool = MCPTool( + name="gmail_send", + description="Send an email", + inputSchema={"type": "object"}, + ) + + name, desc = f._extract_tool_info(tool) + + assert name == "gmail_send" + assert desc == "Send an email" + + +class TestGetToolNamesCsv: + """ + Tests for SemanticToolFilterHook._get_tool_names_csv handling both + OpenAI tool formats. + + Regression coverage for #28766. + """ + + def _make_hook(self): + from litellm.proxy._experimental.mcp_server.semantic_tool_filter import ( + SemanticMCPToolFilter, + ) + from litellm.proxy.hooks.mcp_semantic_filter import SemanticToolFilterHook + + filter_instance = SemanticMCPToolFilter( + embedding_model="text-embedding-3-small", + litellm_router_instance=Mock(), + top_k=5, + similarity_threshold=0.3, + enabled=True, + ) + return SemanticToolFilterHook(filter_instance) + + def test_responses_format(self): + """Flat dicts produce correct CSV.""" + hook = self._make_hook() + tools = [ + {"name": "tool_a", "description": "A"}, + {"name": "tool_b", "description": "B"}, + ] + + csv = hook._get_tool_names_csv(tools) + + assert csv == "tool_a,tool_b" + + def test_chat_completions_format(self): + """Nested function dicts produce correct CSV.""" + hook = self._make_hook() + tools = [ + { + "type": "function", + "function": {"name": "search", "description": "Search"}, + }, + { + "type": "function", + "function": {"name": "crawl", "description": "Crawl"}, + }, + ] + + csv = hook._get_tool_names_csv(tools) + + assert csv == "search,crawl" + + def test_empty_list(self): + hook = self._make_hook() + assert hook._get_tool_names_csv([]) == "" + + +class TestExpandMcpToolsFormat: + """ + Tests that _expand_mcp_tools produces the correct OpenAI format + based on call_type. + + Regression test for #28766: _expand_mcp_tools always produced + responses-API format (flat dicts), even for /chat/completions + requests that require the nested {type: "function", function: {...}} + wrapper. + """ + + @pytest.mark.asyncio + async def test_chat_completion_produces_chat_format(self): + """ + For call_type="acompletion", expanded tools must have the + chat completions wrapper: {type: "function", function: {name, ...}}. + """ + from litellm.proxy._experimental.mcp_server.semantic_tool_filter import ( + SemanticMCPToolFilter, + ) + from litellm.proxy.hooks.mcp_semantic_filter import SemanticToolFilterHook + + filter_instance = SemanticMCPToolFilter( + embedding_model="text-embedding-3-small", + litellm_router_instance=Mock(), + top_k=5, + similarity_threshold=0.3, + enabled=True, + ) + hook = SemanticToolFilterHook(filter_instance) + + mock_mcp_tools = [ + MCPTool( + name="web_search", + description="Search the web", + inputSchema={"type": "object", "properties": {}}, + ), + ] + + mock_user_api_key_dict = Mock() + + with ( + patch( + "litellm.responses.mcp.litellm_proxy_mcp_handler.LiteLLM_Proxy_MCP_Handler._parse_mcp_tools" + ) as mock_parse, + patch( + "litellm.responses.mcp.litellm_proxy_mcp_handler.LiteLLM_Proxy_MCP_Handler._process_mcp_tools_without_openai_transform" + ) as mock_process, + ): + mock_parse.return_value = (["mcp_ref"], []) + mock_process.return_value = (mock_mcp_tools, {}) + + result = await hook._expand_mcp_tools( + tools=["mcp_ref"], + user_api_key_dict=mock_user_api_key_dict, + call_type="acompletion", + ) + + assert len(result) == 1 + tool = result[0] + assert "function" in tool, "Chat format must have 'function' key, got: " + str( + list(tool.keys()) + ) + assert tool["type"] == "function" + assert tool["function"]["name"] == "web_search" + + @pytest.mark.asyncio + async def test_responses_api_produces_flat_format(self): + """ + For call_type="aresponses", expanded tools must use the flat + responses-API format: {name, parameters, type: "function", ...}. + """ + from litellm.proxy._experimental.mcp_server.semantic_tool_filter import ( + SemanticMCPToolFilter, + ) + from litellm.proxy.hooks.mcp_semantic_filter import SemanticToolFilterHook + + filter_instance = SemanticMCPToolFilter( + embedding_model="text-embedding-3-small", + litellm_router_instance=Mock(), + top_k=5, + similarity_threshold=0.3, + enabled=True, + ) + hook = SemanticToolFilterHook(filter_instance) + + mock_mcp_tools = [ + MCPTool( + name="web_search", + description="Search the web", + inputSchema={"type": "object", "properties": {}}, + ), + ] + + mock_user_api_key_dict = Mock() + + with ( + patch( + "litellm.responses.mcp.litellm_proxy_mcp_handler.LiteLLM_Proxy_MCP_Handler._parse_mcp_tools" + ) as mock_parse, + patch( + "litellm.responses.mcp.litellm_proxy_mcp_handler.LiteLLM_Proxy_MCP_Handler._process_mcp_tools_without_openai_transform" + ) as mock_process, + ): + mock_parse.return_value = (["mcp_ref"], []) + mock_process.return_value = (mock_mcp_tools, {}) + + result = await hook._expand_mcp_tools( + tools=["mcp_ref"], + user_api_key_dict=mock_user_api_key_dict, + call_type="aresponses", + ) + + assert len(result) == 1 + tool = result[0] + assert ( + tool.get("name") == "web_search" + ), "Responses format must have 'name' at top level" + assert ( + "function" not in tool + ), "Responses format must not have nested 'function' key"