Skip to content

[Parser] Pass request.tools to tool parser#38860

Merged
DarkLight1337 merged 2 commits intovllm-project:mainfrom
sfeng33:tool-init-parser
Apr 7, 2026
Merged

[Parser] Pass request.tools to tool parser#38860
DarkLight1337 merged 2 commits intovllm-project:mainfrom
sfeng33:tool-init-parser

Conversation

@sfeng33
Copy link
Copy Markdown
Contributor

@sfeng33 sfeng33 commented Apr 3, 2026

Purpose

Pass request.tools to tool parser in _WrappedParser so non-streaming Responses API tool calls work correctly for parsers that depend on self.tools. Follow up to comment in #38189

Test plan

Triggered response api on non-stream path and validated parser.tool_parser.tools is set

vllm serve Qwen/Qwen3-0.6B --enable-auto-tool-choice --tool-call-parser hermes

curl -s http://localhost:8000/v1/responses \
  -H "Content-Type: application/json" \
  -d '{
    "model": "Qwen/Qwen3-0.6B",
    "stream": false,
    "input": "What is the weather in San Francisco?",
    "tools": [
      {
        "type": "function",
        "name": "get_weather",
        "description": "Get the current weather for a location",
        "parameters": {
          "type": "object",
          "properties": {
            "location": {"type": "string", "description": "City name"}
          },
          "required": ["location"]
        }
      }
    ]
  }'

Signed-off-by: sfeng33 <4florafeng@gmail.com>
@sfeng33 sfeng33 marked this pull request as ready for review April 3, 2026 00:06
@mergify mergify bot added the frontend label Apr 3, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the parser instantiation logic to support tool definitions by passing request.tools from the serving layer to the parser. Specifically, _WrappedParser in vllm/parser/abstract_parser.py now accepts an optional list of tools and forwards them to the underlying tool parser. A review comment suggests adding *args and **kwargs to the _WrappedParser constructor to maintain compatibility with the base class and prevent potential TypeError issues in the future.

@bbrowning
Copy link
Copy Markdown
Contributor

The change is small and definitely looks right on the surface. Is there any unit test or manual test you ran that verifies self.tools is now getting wired in properly for non-stream Responses API requests (I believe that's the code path that goes through the changed code)?

@sfeng33
Copy link
Copy Markdown
Contributor Author

sfeng33 commented Apr 6, 2026

The change is small and definitely looks right on the surface. Is there any unit test or manual test you ran that verifies self.tools is now getting wired in properly for non-stream Responses API requests (I believe that's the code path that goes through the changed code)?

Thanks, updated the test plan with steps to trigger the response api non-stream path.

@sfeng33
Copy link
Copy Markdown
Contributor Author

sfeng33 commented Apr 6, 2026

@aarnphm can you please add the ready tag?

@DarkLight1337 DarkLight1337 added the ready ONLY add when PR is ready to merge/full CI is needed label Apr 7, 2026
@DarkLight1337 DarkLight1337 merged commit 0102bd2 into vllm-project:main Apr 7, 2026
50 checks passed
@sfeng33 sfeng33 deleted the tool-init-parser branch April 7, 2026 17:37
puririshi98 pushed a commit to puririshi98/vllm that referenced this pull request Apr 7, 2026
Signed-off-by: sfeng33 <4florafeng@gmail.com>
Signed-off-by: Rishi Puri <riship@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants