fix tool handling in OpenAIServingChat (#18996)
Signed-off-by: Xinyuan Tong <xinyuantong.cs@gmail.com>
This commit is contained in:
@@ -329,12 +329,12 @@ class OpenAIServingChat(OpenAIServingBase):
|
||||
request.skip_special_tokens = False
|
||||
if not isinstance(request.tool_choice, str):
|
||||
tools = [
|
||||
item.function.model_dump()
|
||||
item.model_dump()
|
||||
for item in request.tools
|
||||
if item.function.name == request.tool_choice.function.name
|
||||
]
|
||||
else:
|
||||
tools = [item.function.model_dump() for item in request.tools]
|
||||
tools = [item.model_dump() for item in request.tools]
|
||||
if self.tool_call_parser:
|
||||
parser = FunctionCallParser(request.tools, self.tool_call_parser)
|
||||
tool_call_constraint = parser.get_structure_constraint(
|
||||
@@ -472,11 +472,10 @@ class OpenAIServingChat(OpenAIServingBase):
|
||||
return_dict=False,
|
||||
)
|
||||
except Exception as e:
|
||||
# If the first attempt fails, try transforming the tools format
|
||||
# This handles models like Mistral that have a different tools input format
|
||||
# that is not compatible with OpenAI's apply_chat_template tool_call format
|
||||
# If the first attempt fails, try with flat function-only format.
|
||||
# Some templates (e.g. Mistral) expect tools without the OpenAI wrapper.
|
||||
tools = (
|
||||
[t if "function" in t else {"function": t} for t in tools]
|
||||
[t["function"] if "function" in t else t for t in tools]
|
||||
if tools
|
||||
else None
|
||||
)
|
||||
|
||||
@@ -133,6 +133,84 @@ class ServingChatTestCase(unittest.TestCase):
|
||||
self.assertFalse(adapted.stream)
|
||||
self.assertEqual(processed, self.basic_req)
|
||||
|
||||
def test_jinja_uses_openai_tool_schema_first(self):
|
||||
"""Ensure Jinja chat templates receive OpenAI-shaped tools by default."""
|
||||
self.template_manager.chat_template_name = None
|
||||
self.template_manager.jinja_template_content_format = "string"
|
||||
|
||||
req = ChatCompletionRequest(
|
||||
model="x",
|
||||
messages=[{"role": "user", "content": "What is 2+2?"}],
|
||||
tools=[
|
||||
{
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "add",
|
||||
"description": "Add two numbers.",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"a": {"type": "integer"},
|
||||
"b": {"type": "integer"},
|
||||
},
|
||||
"required": ["a", "b"],
|
||||
},
|
||||
},
|
||||
}
|
||||
],
|
||||
)
|
||||
|
||||
self.chat._process_messages(req, is_multimodal=False)
|
||||
|
||||
expected_tools = [tool.model_dump() for tool in req.tools]
|
||||
kwargs = self.tm.tokenizer.apply_chat_template.call_args.kwargs
|
||||
self.assertEqual(kwargs["tools"], expected_tools)
|
||||
|
||||
def test_jinja_tool_schema_fallback_to_flat_function(self):
|
||||
"""Fallback to function-only schema when template rejects OpenAI wrapper."""
|
||||
self.template_manager.chat_template_name = None
|
||||
self.template_manager.jinja_template_content_format = "string"
|
||||
|
||||
req = ChatCompletionRequest(
|
||||
model="x",
|
||||
messages=[{"role": "user", "content": "What is 2+2?"}],
|
||||
tools=[
|
||||
{
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "add",
|
||||
"description": "Add two numbers.",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"a": {"type": "integer"},
|
||||
"b": {"type": "integer"},
|
||||
},
|
||||
"required": ["a", "b"],
|
||||
},
|
||||
},
|
||||
}
|
||||
],
|
||||
)
|
||||
|
||||
self.tm.tokenizer.apply_chat_template.side_effect = [
|
||||
RuntimeError("template expects flat tools format"),
|
||||
[1, 2, 3],
|
||||
]
|
||||
|
||||
self.chat._process_messages(req, is_multimodal=False)
|
||||
|
||||
first_tools = self.tm.tokenizer.apply_chat_template.call_args_list[0].kwargs[
|
||||
"tools"
|
||||
]
|
||||
second_tools = self.tm.tokenizer.apply_chat_template.call_args_list[1].kwargs[
|
||||
"tools"
|
||||
]
|
||||
self.assertEqual(first_tools, [tool.model_dump() for tool in req.tools])
|
||||
self.assertEqual(
|
||||
second_tools, [tool.function.model_dump() for tool in req.tools]
|
||||
)
|
||||
|
||||
def test_stop_str_isolation_between_requests(self):
|
||||
"""Test that stop strings from one request don't affect subsequent requests.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user