From 63e161f2965e77b2c3ffcd159ce45b2157a21b43 Mon Sep 17 00:00:00 2001 From: Joe Rowell Date: Fri, 26 Jun 2026 08:05:16 +0200 Subject: [PATCH] [Bugfix][Tool Parser] PoolsideV1: fix string whitespace and required named tool choice (#46486) Signed-off-by: Joe Rowell --- .../test_poolside_v1_tool_parser.py | 217 ++++++++++++++++++ vllm/tool_parsers/poolside_v1_tool_parser.py | 41 +++- 2 files changed, 247 insertions(+), 11 deletions(-) create mode 100644 tests/tool_parsers/test_poolside_v1_tool_parser.py diff --git a/tests/tool_parsers/test_poolside_v1_tool_parser.py b/tests/tool_parsers/test_poolside_v1_tool_parser.py new file mode 100644 index 00000000000..68342e2763b --- /dev/null +++ b/tests/tool_parsers/test_poolside_v1_tool_parser.py @@ -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 + (``......``) 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 {"": 151_657, "": 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 = ( + "write_file\n" + "content\n" + f"{content}\n" + "" + ) + + 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 = ( + "write_file\n" + "content\n" + "hi\n" + "mode\n" + " 420 \n" + "" + ) + + 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 = ( + "write_file\n" + "content\n" + f"{content}\n" + "" + ) + + 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 diff --git a/vllm/tool_parsers/poolside_v1_tool_parser.py b/vllm/tool_parsers/poolside_v1_tool_parser.py index e515e1ce637..f5d996176b8 100644 --- a/vllm/tool_parsers/poolside_v1_tool_parser.py +++ b/vllm/tool_parsers/poolside_v1_tool_parser.py @@ -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 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 (, ) 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(