forked from Karylab-cklius/vllm
[Bugfix][Tool Parser] PoolsideV1: fix string whitespace and required named tool choice (#46486)
Signed-off-by: Joe Rowell <joerowell4@gmail.com>
This commit is contained in:
@@ -0,0 +1,217 @@
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
# SPDX-FileCopyrightText: Copyright contributors to the vLLM project
|
||||
"""Regression tests for ``PoolsideV1ToolParser``.
|
||||
|
||||
Covers two bugs:
|
||||
|
||||
1. ``adjust_request`` did not skip the forced ``structured_outputs`` JSON
|
||||
for ``required``/named tool choice. These models emit XML tool calls
|
||||
(``<tool_call>...<arg_value>...</arg_value></tool_call>``) per the chat
|
||||
template, so guided JSON decoding conflicts with the format: the call
|
||||
leaks as content with empty ``tool_calls``. ``adjust_request`` now skips
|
||||
the constraint for both ChatCompletion (``ChatCompletionNamedToolChoice``)
|
||||
and Responses (``ToolChoiceFunction``) named choices.
|
||||
|
||||
2. ``extract_tool_calls`` stripped string-typed argument values, corrupting
|
||||
content whose whitespace is significant (e.g. code/file bodies losing
|
||||
leading indent and trailing newline). String values are now kept verbatim;
|
||||
only non-string types are stripped/deserialized.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from typing import Any
|
||||
|
||||
from openai.types.responses.tool_param import FunctionToolParam
|
||||
|
||||
from vllm.entrypoints.openai.chat_completion.protocol import ChatCompletionRequest
|
||||
from vllm.entrypoints.openai.responses.protocol import ResponsesRequest
|
||||
from vllm.tool_parsers.poolside_v1_tool_parser import PoolsideV1ToolParser
|
||||
|
||||
|
||||
def _write_file_tool() -> dict[str, Any]:
|
||||
"""Tool with a string arg (``content``) and a non-string arg (``mode``)."""
|
||||
return {
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "write_file",
|
||||
"description": "Write content to a file",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"content": {"type": "string"},
|
||||
"mode": {"type": "integer"},
|
||||
},
|
||||
"required": ["content"],
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
def _responses_write_file_tool() -> FunctionToolParam:
|
||||
return FunctionToolParam(
|
||||
type="function",
|
||||
name="write_file",
|
||||
description="Write content to a file",
|
||||
parameters={
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"content": {"type": "string"},
|
||||
"mode": {"type": "integer"},
|
||||
},
|
||||
"required": ["content"],
|
||||
},
|
||||
strict=True,
|
||||
)
|
||||
|
||||
|
||||
def _build_chat_request(*, tool_choice: str | dict[str, Any]) -> ChatCompletionRequest:
|
||||
return ChatCompletionRequest.model_validate(
|
||||
{
|
||||
"model": "poolside-test",
|
||||
"messages": [{"role": "user", "content": "write the file"}],
|
||||
"tools": [_write_file_tool()],
|
||||
"tool_choice": tool_choice,
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def _build_responses_request(*, tool_choice: str | dict[str, Any]) -> ResponsesRequest:
|
||||
return ResponsesRequest(
|
||||
model="poolside-test",
|
||||
input=[{"role": "user", "content": "write the file"}],
|
||||
tools=[_responses_write_file_tool()],
|
||||
tool_choice=tool_choice,
|
||||
stream=True,
|
||||
max_output_tokens=200,
|
||||
)
|
||||
|
||||
|
||||
class _StubTokenizer:
|
||||
"""Minimal tokenizer stub to satisfy ``PoolsideV1ToolParser.__init__``."""
|
||||
|
||||
def get_vocab(self) -> dict[str, int]:
|
||||
return {"<tool_call>": 151_657, "</tool_call>": 151_658}
|
||||
|
||||
|
||||
def _make_parser(request: ChatCompletionRequest) -> PoolsideV1ToolParser:
|
||||
return PoolsideV1ToolParser(_StubTokenizer(), tools=request.tools)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Bug 1: required/named must skip forced structured_outputs (#39870 pattern)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_required_skips_structured_outputs_chatcompletion() -> None:
|
||||
request = _build_chat_request(tool_choice="required")
|
||||
_make_parser(request).adjust_request(request)
|
||||
|
||||
assert request.structured_outputs is None
|
||||
assert request.skip_special_tokens is False
|
||||
|
||||
|
||||
def test_named_skips_structured_outputs_chatcompletion() -> None:
|
||||
request = _build_chat_request(
|
||||
tool_choice={"type": "function", "function": {"name": "write_file"}}
|
||||
)
|
||||
_make_parser(request).adjust_request(request)
|
||||
|
||||
assert request.structured_outputs is None
|
||||
assert request.skip_special_tokens is False
|
||||
|
||||
|
||||
def test_required_skips_structured_outputs_responses() -> None:
|
||||
request = _build_responses_request(tool_choice="required")
|
||||
PoolsideV1ToolParser(_StubTokenizer()).adjust_request(request)
|
||||
|
||||
assert request.text is None
|
||||
assert request.skip_special_tokens is False
|
||||
|
||||
|
||||
def test_named_skips_structured_outputs_responses() -> None:
|
||||
# Responses-API named choice parses to ToolChoiceFunction, a different
|
||||
# type than the ChatCompletion named choice; both must be handled.
|
||||
request = _build_responses_request(
|
||||
tool_choice={"type": "function", "name": "write_file"}
|
||||
)
|
||||
PoolsideV1ToolParser(_StubTokenizer()).adjust_request(request)
|
||||
|
||||
assert request.text is None
|
||||
assert request.skip_special_tokens is False
|
||||
|
||||
|
||||
def test_auto_still_keeps_special_tokens() -> None:
|
||||
request = _build_chat_request(tool_choice="auto")
|
||||
_make_parser(request).adjust_request(request)
|
||||
|
||||
assert request.skip_special_tokens is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Bug 2: string arg whitespace must be preserved (#42026 pattern)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_string_arg_preserves_whitespace() -> None:
|
||||
request = _build_chat_request(tool_choice="auto")
|
||||
parser = _make_parser(request)
|
||||
|
||||
content = " def f():\n return 1\n"
|
||||
model_output = (
|
||||
"<tool_call>write_file\n"
|
||||
"<arg_key>content</arg_key>\n"
|
||||
f"<arg_value>{content}</arg_value>\n"
|
||||
"</tool_call>"
|
||||
)
|
||||
|
||||
result = parser.extract_tool_calls(model_output, request)
|
||||
|
||||
assert result.tools_called
|
||||
args = json.loads(result.tool_calls[0].function.arguments)
|
||||
# Leading indent and trailing newline must survive verbatim.
|
||||
assert args["content"] == content
|
||||
|
||||
|
||||
def test_non_string_arg_still_deserialized() -> None:
|
||||
request = _build_chat_request(tool_choice="auto")
|
||||
parser = _make_parser(request)
|
||||
|
||||
model_output = (
|
||||
"<tool_call>write_file\n"
|
||||
"<arg_key>content</arg_key>\n"
|
||||
"<arg_value>hi</arg_value>\n"
|
||||
"<arg_key>mode</arg_key>\n"
|
||||
"<arg_value> 420 </arg_value>\n"
|
||||
"</tool_call>"
|
||||
)
|
||||
|
||||
result = parser.extract_tool_calls(model_output, request)
|
||||
|
||||
assert result.tools_called
|
||||
args = json.loads(result.tool_calls[0].function.arguments)
|
||||
assert args["content"] == "hi"
|
||||
# Non-string value is stripped and parsed to its native type.
|
||||
assert args["mode"] == 420
|
||||
|
||||
|
||||
def test_responses_extract_tool_calls_with_flat_tools() -> None:
|
||||
# required/named Responses calls route into extract_tool_calls with flat
|
||||
# FunctionTool (.name); _is_string_type must not raise.
|
||||
request = _build_responses_request(tool_choice="required")
|
||||
parser = PoolsideV1ToolParser(_StubTokenizer(), tools=request.tools)
|
||||
|
||||
content = " x = 1\n"
|
||||
model_output = (
|
||||
"<tool_call>write_file\n"
|
||||
"<arg_key>content</arg_key>\n"
|
||||
f"<arg_value>{content}</arg_value>\n"
|
||||
"</tool_call>"
|
||||
)
|
||||
|
||||
result = parser.extract_tool_calls(model_output, request)
|
||||
|
||||
assert result.tools_called
|
||||
args = json.loads(result.tool_calls[0].function.arguments)
|
||||
assert args["content"] == content
|
||||
@@ -17,10 +17,12 @@ from typing import Any
|
||||
|
||||
import partial_json_parser.core.complete
|
||||
import regex as re
|
||||
from openai.types.responses import ToolChoiceFunction
|
||||
from partial_json_parser.core.options import Allow
|
||||
|
||||
from vllm.entrypoints.chat_utils import make_tool_call_id
|
||||
from vllm.entrypoints.openai.chat_completion.protocol import (
|
||||
ChatCompletionNamedToolChoiceParam,
|
||||
ChatCompletionRequest,
|
||||
)
|
||||
from vllm.entrypoints.openai.engine.protocol import (
|
||||
@@ -53,6 +55,8 @@ class PoolsideV1ToolParser(ToolParser):
|
||||
rather than waiting for the complete </arg_value> tag.
|
||||
"""
|
||||
|
||||
supports_required_and_named = False
|
||||
|
||||
def __init__(self, tokenizer: TokenizerLike, tools: list[Tool] | None = None):
|
||||
super().__init__(tokenizer, tools)
|
||||
# Stateful streaming fields
|
||||
@@ -132,15 +136,15 @@ class PoolsideV1ToolParser(ToolParser):
|
||||
if tools is None:
|
||||
return False
|
||||
for tool in tools:
|
||||
if tool.function.name != tool_name:
|
||||
# ChatCompletion tools nest under .function; Responses
|
||||
# FunctionTool is flat (.name/.parameters at the top level).
|
||||
fn = getattr(tool, "function", tool)
|
||||
if getattr(fn, "name", None) != tool_name:
|
||||
continue
|
||||
if tool.function.parameters is None:
|
||||
params = getattr(fn, "parameters", None)
|
||||
if params is None:
|
||||
return False
|
||||
arg_type = (
|
||||
tool.function.parameters.get("properties", {})
|
||||
.get(arg_name, {})
|
||||
.get("type", None)
|
||||
)
|
||||
arg_type = params.get("properties", {}).get(arg_name, {}).get("type", None)
|
||||
return arg_type == "string"
|
||||
logger.debug("No tool named '%s'.", tool_name)
|
||||
return False
|
||||
@@ -159,7 +163,19 @@ class PoolsideV1ToolParser(ToolParser):
|
||||
def adjust_request(
|
||||
self, request: ChatCompletionRequest | ResponsesRequest
|
||||
) -> ChatCompletionRequest | ResponsesRequest:
|
||||
"""Adjust request parameters for tool call token handling."""
|
||||
"""Adjust request parameters for tool call token handling.
|
||||
|
||||
For required/named tool_choice, skip super().adjust_request() so it
|
||||
does not install JSON guided decoding. These models emit XML tool
|
||||
calls (per the chat template), which JSON guidance would break.
|
||||
"""
|
||||
if request.tools:
|
||||
tc = request.tool_choice
|
||||
if tc == "required" or isinstance(
|
||||
tc, (ChatCompletionNamedToolChoiceParam, ToolChoiceFunction)
|
||||
):
|
||||
request.skip_special_tokens = False
|
||||
return request
|
||||
request = super().adjust_request(request)
|
||||
if request.tools and request.tool_choice != "none":
|
||||
# Ensure tool call tokens (<tool_call>, </tool_call>) are not skipped
|
||||
@@ -192,9 +208,12 @@ class PoolsideV1ToolParser(ToolParser):
|
||||
arg_dct: dict[str, Any] = {}
|
||||
for key, value in pairs:
|
||||
arg_key = key.strip()
|
||||
arg_val = value.strip()
|
||||
if not self._is_string_type(tc_name, arg_key, request.tools):
|
||||
arg_val = self._deserialize(arg_val)
|
||||
# Keep string values verbatim; whitespace is significant
|
||||
# (e.g. code/file content). Only strip non-string types.
|
||||
if self._is_string_type(tc_name, arg_key, request.tools):
|
||||
arg_val = value
|
||||
else:
|
||||
arg_val = self._deserialize(value.strip())
|
||||
logger.debug("arg_key = %s, arg_val = %s", arg_key, arg_val)
|
||||
arg_dct[arg_key] = arg_val
|
||||
tool_calls.append(
|
||||
|
||||
Reference in New Issue
Block a user