Skip to content

[Bugfix][Tool Parser] PoolsideV1: fix string whitespace and required named tool choice - #46486

Merged
sfeng33 merged 3 commits into
vllm-project:mainfrom
joerowell:joerowell/fix-poolside-v1-toolparser
Jun 26, 2026
Merged

sfeng33 merged 3 commits into
vllm-project:mainfrom
joerowell:joerowell/fix-poolside-v1-toolparser

Conversation

@joerowell

@joerowell joerowell commented Jun 23, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

This PR brings some internal fixes for the PoolsideV1 tool parser into mainline vLLM. The main differences are that we handle the XML tags properly even if JSON is requested, and we handle whitespace more intelligently.

Test Plan

Test Result


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.

…named tool choice

extract_tool_calls stripped string-typed args, corrupting content whose
whitespace matters (e.g. code/file bodies). required/named tool choice
forced JSON guided decoding that conflicts with the XML tool-call format,
leaking the call as content with empty tool_calls.

Keep string values verbatim, and skip the forced JSON for required/named
so the model emits its native XML. Adds tests/tool_parsers/test_poolside_v1_tool_parser.py.

Signed-off-by: Joe Rowell <joerowell4@gmail.com>
Comment thread vllm/tool_parsers/poolside_v1_tool_parser.py
@sfeng33

sfeng33 commented Jun 23, 2026

Copy link
Copy Markdown
Member

Not a blocking comment for this PR, but sth to consider is:
It'd be great if poolside can be migrated to use the new parser engine - the format is very similar to glm and can benefit from the many bugfix there especially on the incremental string streaming and schema-aware type coercion (covers this PR's whitespace and typing fixes).

It can be done via adding a Poolside ParserEngineConfig + register an adapter (mirror glm47_moe_config).

With supports_required_and_named=False, required/named Responses tool calls route into extract_tool_calls, where tools are flat FunctionTool (.name) not nested (.function.name). Resolve via getattr so both shapes work. Adds a Responses extraction regression test.

Signed-off-by: Joe Rowell <joerowell4@gmail.com>
@sfeng33
sfeng33 enabled auto-merge (squash) June 24, 2026 18:08
@github-actions github-actions Bot added the ready ONLY add when PR is ready to merge/full CI is needed label Jun 24, 2026
@sfeng33
sfeng33 merged commit 63e161f into vllm-project:main Jun 26, 2026
54 of 55 checks passed
wincent8 pushed a commit to wincent8/vllm that referenced this pull request Jun 29, 2026
…named tool choice (vllm-project#46486)

Signed-off-by: Joe Rowell <joerowell4@gmail.com>
philippesic pushed a commit to philippesic/vllm-semantic-cache that referenced this pull request Jul 19, 2026
…named tool choice (vllm-project#46486)

Signed-off-by: Joe Rowell <joerowell4@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready ONLY add when PR is ready to merge/full CI is needed tool-calling

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants