From 7a97525ac50989fda9246e8d90d906300b213dc7 Mon Sep 17 00:00:00 2001 From: 2sumtech <2sumtech@gmail.com> Date: Mon, 24 Aug 2026 12:22:37 -0700 Subject: [PATCH] [Bugfix] Do not read falsy request values as unset in the API layer Three request parameters in the OpenAI-compatible layer are typed `X | None` but read through a truthiness test, so a caller-supplied falsy value takes the branch reserved for an omitted field. `chat_template=""` was replaced by the server template in `build_chat_params` for chat completions, `/tokenize` and pooling requests. `OnlineRenderer.validate_chat_template` already counts `""` as a caller-supplied template and refuses it unless `--trust-request-chat-template` is set, so the two paths disagreed on whether an empty template had been supplied. `stream_options={}` skipped the "Stream options can only be defined when `stream=True`" rejection in chat completions and completions, because an empty object is falsy while a populated one is not. `padding=False` and `truncation=False` were ignored by `TokenizeParams.with_kwargs`, leaving the parent's `pad_prompt_tokens` and `truncate_prompt_tokens` in place. The documented HuggingFace synonyms `"do_not_pad"` and `"do_not_truncate"` already worked, so the two spellings of the same option behaved differently. Each site now tests `is not None` instead of truthiness. Co-authored-by: Claude Fable 5 Signed-off-by: 2sumtech <2sumtech@gmail.com> --- .../unit_tests/test_falsy_request_params.py | 94 +++++++++++++++++++ tests/renderers/test_tokenize_params.py | 54 +++++++++++ .../openai/chat_completion/protocol.py | 6 +- .../entrypoints/openai/completion/protocol.py | 2 +- vllm/entrypoints/pooling/base/protocol.py | 4 +- vllm/entrypoints/serve/tokenize/protocol.py | 4 +- vllm/renderers/params.py | 4 +- 7 files changed, 161 insertions(+), 7 deletions(-) create mode 100644 tests/entrypoints/unit_tests/test_falsy_request_params.py create mode 100644 tests/renderers/test_tokenize_params.py diff --git a/tests/entrypoints/unit_tests/test_falsy_request_params.py b/tests/entrypoints/unit_tests/test_falsy_request_params.py new file mode 100644 index 000000000000..2a9dd0341dba --- /dev/null +++ b/tests/entrypoints/unit_tests/test_falsy_request_params.py @@ -0,0 +1,94 @@ +# SPDX-License-Identifier: Apache-2.0 +# SPDX-FileCopyrightText: Copyright contributors to the vLLM project + +"""Falsy-but-explicit request values must not be read as "field omitted". + +Each case below covers a request field typed ``X | None`` whose consumer used a +truthiness test, so a caller-supplied falsy value took the branch reserved for +an omitted field. + +This extends the falsy-field coverage of #48098 to the chat template and stream +option paths. +""" + +import pytest + +from vllm.entrypoints.openai.chat_completion.protocol import ChatCompletionRequest +from vllm.entrypoints.openai.completion.protocol import CompletionRequest +from vllm.entrypoints.pooling.embed.protocol import EmbeddingChatRequest +from vllm.entrypoints.serve.tokenize.protocol import TokenizeChatRequest +from vllm.exceptions import VLLMValidationError + +pytestmark = pytest.mark.skip_global_cleanup + +SERVER_TEMPLATE = "{{ 'server default' }}" + +CHAT_BODY = {"model": "qwen", "messages": [{"role": "user", "content": "hello"}]} + +CHAT_TEMPLATE_MODELS = [ + ChatCompletionRequest, + TokenizeChatRequest, + EmbeddingChatRequest, +] + +STREAM_OPTION_BODIES = [ + (ChatCompletionRequest, CHAT_BODY), + (CompletionRequest, {"model": "qwen", "prompt": "hello"}), +] +IDS = [model.__name__ for model, _ in STREAM_OPTION_BODIES] + + +@pytest.mark.parametrize( + "request_model", CHAT_TEMPLATE_MODELS, ids=lambda m: m.__name__ +) +def test_empty_chat_template_is_not_replaced_by_the_server_default(request_model): + """``chat_template: ""`` must reach the renderer as the caller wrote it. + + ``OnlineRenderer.validate_chat_template`` already counts ``""`` as a + caller-supplied template and refuses it unless + ``--trust-request-chat-template`` is set, so substituting the server + template afterwards renders a prompt the caller never asked for. + """ + request = request_model.model_validate({**CHAT_BODY, "chat_template": ""}) + + chat_params = request.build_chat_params(SERVER_TEMPLATE, "auto") + + assert chat_params.chat_template == "" + + +@pytest.mark.parametrize( + "request_model", CHAT_TEMPLATE_MODELS, ids=lambda m: m.__name__ +) +def test_omitted_chat_template_still_falls_back_to_the_server_default(request_model): + request = request_model.model_validate(dict(CHAT_BODY)) + + chat_params = request.build_chat_params(SERVER_TEMPLATE, "auto") + + assert chat_params.chat_template == SERVER_TEMPLATE + + +@pytest.mark.parametrize(("request_model", "body"), STREAM_OPTION_BODIES, ids=IDS) +def test_empty_stream_options_are_refused_without_streaming(request_model, body): + """``stream_options: {}`` is still ``stream_options``. + + The field is meaningless outside a streaming request, and an empty object + is no more meaningful there than a populated one. + """ + with pytest.raises(VLLMValidationError, match="stream_options"): + request_model.model_validate({**body, "stream": False, "stream_options": {}}) + + +@pytest.mark.parametrize(("request_model", "body"), STREAM_OPTION_BODIES, ids=IDS) +def test_empty_stream_options_are_accepted_with_streaming(request_model, body): + request = request_model.model_validate( + {**body, "stream": True, "stream_options": {}} + ) + + assert request.stream_options is not None + + +@pytest.mark.parametrize(("request_model", "body"), STREAM_OPTION_BODIES, ids=IDS) +def test_omitted_stream_options_are_accepted_without_streaming(request_model, body): + request = request_model.model_validate({**body, "stream": False}) + + assert request.stream_options is None diff --git a/tests/renderers/test_tokenize_params.py b/tests/renderers/test_tokenize_params.py new file mode 100644 index 000000000000..08d64b4c92fe --- /dev/null +++ b/tests/renderers/test_tokenize_params.py @@ -0,0 +1,54 @@ +# SPDX-License-Identifier: Apache-2.0 +# SPDX-FileCopyrightText: Copyright contributors to the vLLM project +"""``TokenizeParams.with_kwargs`` overrides that are spelled as ``False``. + +``padding`` and ``truncation`` accept the HuggingFace spellings documented at +https://huggingface.co/docs/transformers/en/pad_truncation, where ``False`` and +``"do_not_pad"`` / ``"do_not_truncate"`` mean the same thing. The two spellings +must therefore produce the same ``TokenizeParams``. +""" + +import pytest + +from vllm.renderers import TokenizeParams + +pytestmark = pytest.mark.skip_global_cleanup + + +@pytest.fixture +def params() -> TokenizeParams: + return TokenizeParams( + max_total_tokens=1024, + max_output_tokens=16, + pad_prompt_tokens=64, + truncate_prompt_tokens=128, + ) + + +@pytest.mark.parametrize("truncation", [False, "do_not_truncate"]) +def test_disabling_truncation_clears_truncate_prompt_tokens(params, truncation): + assert params.with_kwargs(truncation=truncation).truncate_prompt_tokens is None + + +@pytest.mark.parametrize("padding", [False, "do_not_pad"]) +def test_disabling_padding_clears_pad_prompt_tokens(params, padding): + assert params.with_kwargs(padding=padding).pad_prompt_tokens is None + + +def test_enabling_truncation_still_uses_the_input_budget(params): + child = params.with_kwargs(truncation=True) + + assert child.truncate_prompt_tokens == params.max_input_tokens + + +def test_padding_to_max_length_still_uses_the_input_budget(params): + child = params.with_kwargs(padding="max_length") + + assert child.pad_prompt_tokens == params.max_input_tokens + + +def test_omitting_the_overrides_preserves_the_parent_values(params): + child = params.with_kwargs(add_special_tokens=False) + + assert child.truncate_prompt_tokens == 128 + assert child.pad_prompt_tokens == 64 diff --git a/vllm/entrypoints/openai/chat_completion/protocol.py b/vllm/entrypoints/openai/chat_completion/protocol.py index f3a3b398d6d9..8b67ad2da8c0 100644 --- a/vllm/entrypoints/openai/chat_completion/protocol.py +++ b/vllm/entrypoints/openai/chat_completion/protocol.py @@ -588,7 +588,9 @@ def build_chat_params( extra_kwargs["enable_thinking"] = self.reasoning_effort != "none" return ChatParams( - chat_template=self.chat_template or default_template, + chat_template=( + default_template if self.chat_template is None else self.chat_template + ), chat_template_content_format=default_template_content_format, chat_template_kwargs=merge_kwargs( self.chat_template_kwargs, @@ -787,7 +789,7 @@ def validate_response_format(cls, data): def validate_stream_options(cls, data): if not isinstance(data, dict): return data - if data.get("stream_options") and not data.get("stream"): + if data.get("stream_options") is not None and not data.get("stream"): raise VLLMValidationError( "Stream options can only be defined when `stream=True`.", parameter="stream_options", diff --git a/vllm/entrypoints/openai/completion/protocol.py b/vllm/entrypoints/openai/completion/protocol.py index 4b9ec1827e9d..2c6abef1b321 100644 --- a/vllm/entrypoints/openai/completion/protocol.py +++ b/vllm/entrypoints/openai/completion/protocol.py @@ -544,7 +544,7 @@ def check_logprobs(cls, data): def validate_stream_options(cls, data): if not isinstance(data, dict): return data - if data.get("stream_options") and not data.get("stream"): + if data.get("stream_options") is not None and not data.get("stream"): raise VLLMValidationError( "Stream options can only be defined when `stream=True`.", parameter="stream_options", diff --git a/vllm/entrypoints/pooling/base/protocol.py b/vllm/entrypoints/pooling/base/protocol.py index 6e592e30f88c..eb7ce9ca0ac3 100644 --- a/vllm/entrypoints/pooling/base/protocol.py +++ b/vllm/entrypoints/pooling/base/protocol.py @@ -254,7 +254,9 @@ def build_chat_params( default_template_content_format: ChatTemplateContentFormatOption, ) -> ChatParams: return ChatParams( - chat_template=self.chat_template or default_template, + chat_template=( + default_template if self.chat_template is None else self.chat_template + ), chat_template_content_format=default_template_content_format, chat_template_kwargs=merge_kwargs( self.chat_template_kwargs, diff --git a/vllm/entrypoints/serve/tokenize/protocol.py b/vllm/entrypoints/serve/tokenize/protocol.py index e68302d1f95b..7d49dabddddc 100644 --- a/vllm/entrypoints/serve/tokenize/protocol.py +++ b/vllm/entrypoints/serve/tokenize/protocol.py @@ -135,7 +135,9 @@ def build_chat_params( default_template_content_format: ChatTemplateContentFormatOption, ) -> ChatParams: return ChatParams( - chat_template=self.chat_template or default_template, + chat_template=( + default_template if self.chat_template is None else self.chat_template + ), chat_template_content_format=default_template_content_format, chat_template_kwargs=merge_kwargs( self.chat_template_kwargs, diff --git a/vllm/renderers/params.py b/vllm/renderers/params.py index c0e2d2129e54..0f3fa4a037c1 100644 --- a/vllm/renderers/params.py +++ b/vllm/renderers/params.py @@ -270,7 +270,7 @@ def with_kwargs(self, **tokenization_kwargs: Any): ) # https://huggingface.co/docs/transformers/en/pad_truncation - if padding := tokenization_kwargs.pop("padding", None): + if (padding := tokenization_kwargs.pop("padding", None)) is not None: if padding == "max_length": pad_prompt_tokens = max_length elif padding in (False, "do_not_pad"): @@ -279,7 +279,7 @@ def with_kwargs(self, **tokenization_kwargs: Any): # To emit the below warning tokenization_kwargs["padding"] = padding - if truncation := tokenization_kwargs.pop("truncation", None): + if (truncation := tokenization_kwargs.pop("truncation", None)) is not None: if truncation in (True, "longest_first"): truncate_prompt_tokens = max_length elif truncation in (False, "do_not_truncate"):