-
-
Notifications
You must be signed in to change notification settings - Fork 19.6k
[Frontend] Fix default_chat_template_kwargs handling in Responses API #37739
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| # SPDX-FileCopyrightText: Copyright contributors to the vLLM project | ||
|
|
||
| import pytest | ||
| import pytest_asyncio | ||
| from openai import OpenAI | ||
|
|
||
| from tests.utils import RemoteOpenAIServer | ||
|
|
||
| from .conftest import BASE_TEST_ENV | ||
|
|
||
| MODEL_NAME = "Qwen/Qwen3-0.6B" | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| def server(): | ||
| args = [ | ||
| "--reasoning-parser", | ||
| "qwen3", | ||
| "--dtype", | ||
| "bfloat16", | ||
| "--enforce-eager", | ||
| "--max-model-len", | ||
| "4096", | ||
| "--default-chat-template-kwargs", | ||
| '{"enable_thinking": false}', | ||
| ] | ||
| env_dict = { | ||
| **BASE_TEST_ENV, | ||
| "VLLM_ENABLE_RESPONSES_API_STORE": "1", | ||
| } | ||
| with RemoteOpenAIServer(MODEL_NAME, args, env_dict=env_dict) as remote_server: | ||
| yield remote_server | ||
|
|
||
|
|
||
| @pytest_asyncio.fixture | ||
| async def client(server): | ||
| async with server.get_async_client() as async_client: | ||
| yield async_client | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| @pytest.mark.parametrize("model_name", [MODEL_NAME]) | ||
| async def test_responses_honors_default_chat_template_kwargs( | ||
| client: OpenAI, model_name: str | ||
| ): | ||
| response = await client.responses.create( | ||
| model=model_name, | ||
| input="Compute 17 * 19 and explain briefly.", | ||
| reasoning={"effort": "low"}, | ||
| temperature=0.0, | ||
| ) | ||
|
|
||
| reasoning_items = [item for item in response.output if item.type == "reasoning"] | ||
|
|
||
| assert response.status == "completed" | ||
| assert response.output_text | ||
| assert not reasoning_items | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| @pytest.mark.parametrize("model_name", [MODEL_NAME]) | ||
| async def test_responses_request_chat_template_kwargs_override_server_default( | ||
| client: OpenAI, model_name: str | ||
| ): | ||
| response = await client.responses.create( | ||
| model=model_name, | ||
| input="Compute 23 * 17 and explain briefly.", | ||
| reasoning={"effort": "low"}, | ||
| temperature=0.0, | ||
| extra_body={"chat_template_kwargs": {"enable_thinking": True}}, | ||
| ) | ||
|
|
||
| reasoning_items = [item for item in response.output if item.type == "reasoning"] | ||
|
|
||
| assert response.status == "completed" | ||
| assert response.usage is not None | ||
| assert response.usage.output_tokens_details.reasoning_tokens > 0 | ||
| assert reasoning_items | ||
| assert reasoning_items[0].content |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -181,6 +181,13 @@ class ResponsesRequest(OpenAIBaseModel): | |
| "and vLLM will ignore it." | ||
| ), | ||
| ) | ||
| chat_template_kwargs: dict[str, Any] | None = Field( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks~ @sidsaha-ai This is a known issue. The reason we haven’t implemented it so far is that we wanted to wait and see whether OpenAI would introduce a similar field. Otherwise, introducing these fields would cause the Responses API to overlap with chat completions.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool. Should we then wait and close this PR? Or should I go ahead with rebasing and can get approval. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If OpenAI won't settle on this anytime soon, would a change that only takes |
||
| default=None, | ||
| description=( | ||
| "Additional keyword args to pass to the template renderer. " | ||
| "Will be accessible by the chat template." | ||
| ), | ||
| ) | ||
|
|
||
| # --8<-- [start:responses-extra-params] | ||
| request_id: str = Field( | ||
|
|
@@ -276,7 +283,7 @@ def build_chat_params( | |
| chat_template=default_template, | ||
| chat_template_content_format=default_template_content_format, | ||
| chat_template_kwargs=merge_kwargs( # To remove unset values | ||
| {}, | ||
| self.chat_template_kwargs, | ||
| dict( | ||
| add_generation_prompt=not continue_final, | ||
| continue_final_message=continue_final, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This documentation appears to be outdated with the changes in this PR. While this PR adds support for reasoning outputs in the
/v1/responsesendpoint, this line still states that reasoning content is only available for/v1/chat/completions. This should be updated to include/v1/responsesto reflect the new capability.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in fac98b1 by updating the limitations section to include
/v1/responsesalongside/v1/chat/completions.