Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 101 additions & 0 deletions tests/renderers/test_hf.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: Apache-2.0
# SPDX-FileCopyrightText: Copyright contributors to the vLLM project

import jinja2
import pytest

from vllm.config import ModelConfig
Expand All @@ -12,6 +13,7 @@
_convert_developer_to_system,
_detect_developer_role_support,
_get_hf_base_chat_template_params,
_template_error_reason,
_try_extract_ast,
resolve_chat_template,
resolve_chat_template_content_format,
Expand Down Expand Up @@ -844,6 +846,105 @@ def test_developer_only_no_prior_system(self, model_config, tokenizer):
)


EFFORT_VALIDATING_TEMPLATE = (
"{% if reasoning_effort is defined and "
"reasoning_effort not in ['xhigh', 'medium', 'low'] %}"
"{{ raise_exception('Unexpected reasoning effort ' + reasoning_effort + "
"'. Supported types are xhigh (default), medium, and low.') }}"
"{% endif %}"
"{% for message in messages %}"
"{{ message['role'] }}: {{ message['content'] }}\n"
"{% endfor %}"
)

EFFORT_BREAKING_TEMPLATE = (
"{% if reasoning_effort is defined %}"
"{{ raise_exception('Template broke for an unrelated reason') }}"
"{% endif %}"
"{% for message in messages %}"
"{{ message['role'] }}: {{ message['content'] }}\n"
"{% endfor %}"
)


class TestApplyChatTemplateEffortTolerant:
"""Chat templates that reject unsupported reasoning_effort values should
surface a 400-style client error instead of crashing the request."""

@pytest.fixture
def model_config(self):
return ModelConfig(
"facebook/opt-125m",
tokenizer="facebook/opt-125m",
tokenizer_mode="auto",
trust_remote_code=False,
dtype="float16",
)

@pytest.fixture
def tokenizer(self):
return get_tokenizer("facebook/opt-125m")

def test_unsupported_effort_raises_bad_request(self, model_config, tokenizer):
conversation = [{"role": "user", "content": "Hello"}]
with pytest.raises(
VLLMValidationError,
match="Unexpected reasoning effort high",
) as excinfo:
safe_apply_chat_template(
model_config,
tokenizer,
conversation,
chat_template=EFFORT_VALIDATING_TEMPLATE,
tokenize=False,
reasoning_effort="high",
)
# The client sees exactly the template's own reason, with no wrapper
# noise. It tells them which values are supported.
assert str(excinfo.value) == (
"Unexpected reasoning effort high. Supported types are xhigh "
"(default), medium, and low."
)

def test_supported_effort_accepted(self, model_config, tokenizer):
conversation = [{"role": "user", "content": "Hello"}]
result = safe_apply_chat_template(
model_config,
tokenizer,
conversation,
chat_template=EFFORT_VALIDATING_TEMPLATE,
tokenize=False,
reasoning_effort="medium",
)
assert result == "user: Hello\n"

def test_non_effort_template_error_is_bad_request(self, model_config, tokenizer):
conversation = [{"role": "user", "content": "Hello"}]
with pytest.raises(VLLMValidationError, match="unrelated reason"):
safe_apply_chat_template(
model_config,
tokenizer,
conversation,
chat_template=EFFORT_BREAKING_TEMPLATE,
tokenize=False,
reasoning_effort="high",
)


def test_template_error_reason_prefers_template_error():
# Upstream wrappers must not hide the template's own reason from clients.
reason = jinja2.TemplateError("Unexpected reasoning effort high")
wrapper = ValueError(f"An error occurred while rendering the template: {reason}")
wrapper.__cause__ = reason
assert _template_error_reason(wrapper) == str(reason)
assert _template_error_reason(reason) == str(reason)


def test_template_error_reason_falls_back_to_message():
err = ValueError("plain failure")
assert _template_error_reason(err) == "plain failure"


class TestConsolidateSystemMessages:
def test_no_system_messages_unchanged(self):
conversation = [
Expand Down
24 changes: 20 additions & 4 deletions vllm/renderers/hf.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
parse_chat_messages,
parse_chat_messages_async,
)
from vllm.exceptions import VLLMValidationError
from vllm.inputs import EmbedsPrompt
from vllm.inputs.engine import MultiModalInput
from vllm.logger import init_logger
Expand Down Expand Up @@ -684,6 +685,18 @@ def resolve_chat_template_kwargs(
return {k: v for k, v in chat_template_kwargs.items() if k in accept_vars}


def _template_error_reason(exc: BaseException) -> str:
# Extract the most specific reason from a chat template error chain.
seen: set[int] = set()
current: BaseException | None = exc
while current is not None and id(current) not in seen:
seen.add(id(current))
if isinstance(current, jinja2.TemplateError):
return str(current)
current = current.__cause__ or current.__context__
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return str(exc)


@overload
def safe_apply_chat_template(
model_config: ModelConfig,
Expand Down Expand Up @@ -806,10 +819,13 @@ def safe_apply_chat_template(
**resolved_kwargs,
)
except Exception as e:
logger.exception(
"An error occurred in `transformers` while applying chat template"
)
raise ValueError(str(e)) from e
# Chat templates reject invalid user input (e.g. an unsupported

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should try to extract the reason for the error if possible, instead of just copying the whole error message

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We should try to extract the reason for the error if possible, instead of just copying the whole error message

OK,Good point. Two things:

  • With transformers v5 (vLLM requires >= 5.10.4), raise_exception(...) inside
    a template propagates as an unwrapped jinja2.TemplateError (nothing in
    apply_chat_template wraps it), so the 400 message was already the
    template's own reason — e.g. "Unexpected reasoning effort high. Supported
    types are xhigh (default), medium, and low."
  • Still, to be robust against upstream wrapping, the PR now adds
    _template_error_reason(), which walks the exception chain and prefers the
    jinja2.TemplateError message (exactly what the template passed to
    raise_exception), falling back to the outer message otherwise. The tests
    now also assert that the 400 message equals the template's reason verbatim.

# `reasoning_effort` value) by raising from within the template.
# Surface those as a 400 Bad Request carrying the template's own
# reason (which typically lists the supported values) instead of a
# 500 or any generic upstream wrapper message.
logger.warning("Chat template rejected the request: %s", e)
raise VLLMValidationError(_template_error_reason(e)) from e

if return_assistant_tokens_mask:
assert isinstance(plain, list), f"Expected list[int], got {type(plain)}"
Expand Down
Loading