mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-02 02:11:58 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
parent
96a2e8b16d
commit
008787732f
3 changed files with 362 additions and 40 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue