diff --git a/components/src/dynamo/frontend/prepost.py b/components/src/dynamo/frontend/prepost.py index 512450a987be..65e650e9beaa 100644 --- a/components/src/dynamo/frontend/prepost.py +++ b/components/src/dynamo/frontend/prepost.py @@ -5,11 +5,13 @@ import json import os +import re from collections.abc import Awaitable, Callable, Sequence from concurrent.futures import ThreadPoolExecutor from copy import deepcopy from dataclasses import dataclass -from typing import Any, Protocol, cast +from functools import lru_cache +from typing import TYPE_CHECKING, Any, Protocol, TypeGuard, cast from vllm.entrypoints.chat_utils import make_tool_call_id from vllm.entrypoints.openai.chat_completion.protocol import ( @@ -34,6 +36,9 @@ from .thinking import apply_default_thinking_mode_to_template_kwargs +if TYPE_CHECKING: + from vllm.config import ModelConfig + class _Renderer(Protocol): """Structural type for vLLM's chat-template renderer.""" @@ -414,16 +419,41 @@ def _validate_chat_completion_request( return validated_request +def _reasoning_parser_enabled( + reasoning_parser_class: type[ReasoningParser] | None, + chat_template_kwargs: dict[str, Any], +) -> TypeGuard[type[ReasoningParser]]: + """Whether a reasoning parser runs for this request. + + Single-sourced because three call sites must agree: _prepare_request (which + lets the parser adjust the sampling request), preprocess_chat_request (which + must know whether that parser could have rewritten the request's guidance), + and StreamingPostProcessor (which builds the parser that consumes the result). + If they drift, the sampling params get adjusted for a parser that then does not + exist and the model's control markers reach the client as visible text. + + See https://github.com/ai-dynamo/dynamo/issues/8636 for the enable_thinking + half: when the chat template runs with enable_thinking=False the reasoning + tags live in the prompt and the generated output carries none. + """ + return reasoning_parser_class is not None and ( + chat_template_kwargs.get("enable_thinking") is not False + ) + + def _prepare_request( request: dict[str, Any] | ChatCompletionRequest, *, tokenizer: TokenizerLike, tool_parser_class: type[ToolParser] | None, + reasoning_parser_class: type[ReasoningParser] | None = None, + model_config: ModelConfig | None = None, exclude_tools_when_tool_choice_none: bool = True, enable_auto_tool_choice: bool = False, default_chat_template_kwargs: dict[str, Any] | None = None, default_thinking_mode: str | None = None, validated_request: ChatCompletionRequest | None = None, + guidance_snapshots: dict[str, Any] | None = None, ) -> tuple[ChatCompletionRequest, ToolParser | None, dict[str, Any], Any, ChatParams]: """Validate request and build arguments for template rendering. @@ -495,6 +525,50 @@ def _prepare_request( ), ) + # Let the reasoning parser adjust the sampling request, mirroring the tool-parser + # path above. Parsers that split thinking from the answer on text markers need + # `skip_special_tokens=False` so those markers survive detokenisation, and they + # declare that through adjust_request(). Without this call the reasoning channel + # only works when the *client* happens to send skip_special_tokens=false. + # + # Gated identically to StreamingPostProcessor's own parser construction below: if + # the sampling params were adjusted for a parser that then does not exist, the + # markers reach the client as visible text. + # + # ReasoningParser.adjust_request is `return request` by default, so this is a no-op + # for parsers that do not need it. + if _reasoning_parser_enabled(reasoning_parser_class, chat_template_kwargs): + # Both parsers rewrite the same request, tool first and reasoning here, so a + # single before/after diff cannot say which one moved the guidance. Splitting + # it here is the only reason the caller can apply one precedence rule to a + # reasoning rewrite and a different one to a tool rewrite. + # + # Taken inside this gate, not above it: it is read only behind the same + # _reasoning_parser_enabled() test in the caller, so on the tool-only and + # no-parser paths it was pure cost. No copy either -- adjust_request() + # REPLACES request.structured_outputs with a fresh object rather than + # mutating the one this names, so the plain reference still denotes the + # pre-adjustment value afterwards. That also makes the caller's `!=` O(1) + # by identity in the common unchanged case instead of a deep structural walk. + if guidance_snapshots is not None: + # Same accessor as adjusted_structured_guidance in the caller, + # deliberately: this value is only ever compared against that one, and + # extract_structured_outputs() is a different view. + guidance_snapshots[ + "after_tool_parser" + ] = _guided_decoding_from_structured_outputs( + request_for_sampling.structured_outputs + ) + # model_config is required, not decorative: the Cohere parsers gate their + # response_format -> structural_tag rewrite on _model_config.architecture and + # return the request untouched without it, which made this whole call a no-op + # for the only shipped parser that overrides adjust_request. + request_for_sampling = reasoning_parser_class( + tokenizer, + chat_template_kwargs=chat_template_kwargs, + model_config=model_config, + ).adjust_request(request_for_sampling) + # Mistral warns that tokenize=False is unsafe for chat templates. is_mistral_tokenizer = ( tokenizer.__class__.__name__ == "MistralTokenizer" @@ -536,6 +610,8 @@ async def preprocess_chat_request( tokenizer: TokenizerLike, renderer: _Renderer, tool_parser_class: type[ToolParser] | None, + reasoning_parser_class: type[ReasoningParser] | None = None, + model_config: ModelConfig | None = None, exclude_tools_when_tool_choice_none: bool = True, enable_auto_tool_choice: bool = False, default_chat_template_kwargs: dict[str, Any] | None = None, @@ -557,6 +633,7 @@ async def preprocess_chat_request( # structured_outputs itself. Reading it afterwards would treat a # parser-generated constraint as one the caller sent and reject the request. has_explicit_output_constraint = _has_explicit_output_constraint(validated_request) + guidance_snapshots: dict[str, Any] = {} ( request_for_sampling, tool_parser, @@ -567,19 +644,31 @@ async def preprocess_chat_request( request, tokenizer=tokenizer, tool_parser_class=tool_parser_class, + reasoning_parser_class=reasoning_parser_class, + model_config=model_config, exclude_tools_when_tool_choice_none=exclude_tools_when_tool_choice_none, enable_auto_tool_choice=enable_auto_tool_choice, default_chat_template_kwargs=default_chat_template_kwargs, default_thinking_mode=default_thinking_mode, validated_request=validated_request, + guidance_snapshots=guidance_snapshots, ) adjusted_structured_guidance = _guided_decoding_from_structured_outputs( request_for_sampling.structured_outputs ) + # Either parser's adjust_request() may rewrite the caller's constraint into the + # form the model actually speaks -- a reasoning parser converting + # response_format into a structural tag, say. Gating this recompute on + # tool_parser alone dropped a reasoning parser's rewrite on the floor: the + # request carried the adjusted constraint but the pre-adjustment copy was + # forwarded to the engine. parser_guided_decoding = ( adjusted_structured_guidance - if tool_parser is not None + if ( + tool_parser is not None + or _reasoning_parser_enabled(reasoning_parser_class, chat_template_kwargs) + ) and adjusted_structured_guidance != client_structured_guidance else None ) @@ -608,11 +697,45 @@ async def preprocess_chat_request( "tool_choice forces a tool call and cannot be combined with an " "explicit guided_* or structured_outputs constraint." ) - guided_decoding = ( - assistant_guided_decoding - if assistant_guided_decoding is not None and not is_forced_tool_choice - else tool_guided_decoding + # Which parser moved the guidance, measured rather than inferred. Both run + # inside _prepare_request against the same request -- tool first, reasoning + # second -- so the snapshot taken between them turns one ambiguous diff into + # two attributable ones: + # + # client -> after_tool_parser changed by the tool parser + # after_tool_parser -> final changed by the reasoning parser + # + # This matters because the two rewrites carry different precedence. A tool + # parser's rewrite loses to the caller's explicit constraint -- deliberate, + # pinned by test_assistant_guidance_takes_precedence_over_auto_tool_guidance + # and the response-format-precedence row of TOOL_GUIDANCE_PARITY_CASES. A + # reasoning parser's rewrite has to win, or the request carries the parser's + # structural tag while the engine is handed the pre-adjustment copy, which is + # the bug this change exists to fix. + # + # When both rewrote it there is nothing to arbitrate: the reasoning parser ran + # last, on the already-tool-adjusted request, so the final value IS the + # composition. Forwarding it is just forwarding what the request holds. + # Absent only when no tool parser ran, in which case nothing has touched the + # request yet and the caller's own constraint is the correct baseline. + # Absent whenever no reasoning parser ran, because _prepare_request only takes + # the snapshot when it is about to run one. The default is then a don't-care: + # the comparison below sits behind the same _reasoning_parser_enabled() test and + # short-circuits before reading it. + guidance_after_tool_parser = guidance_snapshots.get( + "after_tool_parser", client_structured_guidance ) + reasoning_rewrote_guidance = ( + _reasoning_parser_enabled(reasoning_parser_class, chat_template_kwargs) + and adjusted_structured_guidance != guidance_after_tool_parser + ) + guided_decoding: dict[str, Any] | None + if reasoning_rewrote_guidance and not is_forced_tool_choice: + guided_decoding = adjusted_structured_guidance + elif assistant_guided_decoding is not None and not is_forced_tool_choice: + guided_decoding = assistant_guided_decoding + else: + guided_decoding = tool_guided_decoding # build_tool_call_guided_decoding falls back to Dynamo's generic JSON schema # when a forced tool choice has no parser-provided grammar. That can happen # both without a parser and with parsers (for example Hermes) whose @@ -650,6 +773,22 @@ async def preprocess_chat_request( ) +@lru_cache(maxsize=64) +def _compile_marker_strip_re(strippable: tuple[str, ...]) -> "re.Pattern[str] | None": + """Alternation over the literals to remove, or None if there are none. + + Memoised: the set is a property of (tokenizer, active parser), so a server + sees a handful of distinct values, but a StreamingPostProcessor is built per + request and re.compile over ~15 escaped literals is ~12 us -- worth paying + once per process rather than once per request. + """ + if not strippable: + return None + # Longest-first so a marker that prefixes another cannot win. + ordered = sorted(strippable, key=len, reverse=True) + return re.compile("|".join(re.escape(m) for m in ordered)) + + class StreamingPostProcessor: def __init__( self, @@ -661,6 +800,7 @@ def __init__( tool_parser: ToolParser | None, reasoning_parser_class: type[ReasoningParser] | None, chat_template_kwargs: dict[str, Any], + model_config: ModelConfig | None = None, response_reasoning_ended: bool | None = None, stream_response: bool = True, uses_dynamo_json_tool_call_fallback: bool = False, @@ -681,14 +821,13 @@ def __init__( # `enable_thinking` is the convention adopted across the modern # reasoning-capable model families that vLLM supports; templates # that don't honor it simply leave it unset (no effect here). - thinking_disabled = chat_template_kwargs.get("enable_thinking") is False self.reasoning_parser = ( reasoning_parser_class( tokenizer, chat_template_kwargs=chat_template_kwargs, + model_config=model_config, ) - if reasoning_parser_class - and not thinking_disabled + if _reasoning_parser_enabled(reasoning_parser_class, chat_template_kwargs) and response_reasoning_ended is not True else None ) @@ -714,10 +853,41 @@ def __init__( self._reasoning_parser_streaming_started = False self._tool_parser_streaming_started = False + # Every ORIGINAL generated token id for this choice (NVBug 6678449b). + # The count is delegated to the pinned vLLM parser's own + # count_reasoning_tokens, never re-derived from parser output: the + # response projections drop or defer it, a per-chunk classifier + # over-counts a mixed chunk, and re-encoding decoded text drifts from + # the original ids (detokenisation is not injective). + self._reasoning_token_ids: list[int] = [] + self._control_markers = tuple( t for t in getattr(tokenizer, "all_special_tokens", ()) if t ) + # Markers that may be removed from user-visible content: the tokenizer's + # actual special tokens, minus any the active parser owns. Membership in + # all_special_tokens is the test -- a literal that merely looks like a + # control token (`<|not_a_real_special_token|>`) is ordinary generated + # text and must survive. Built once: it cannot change for the lifetime of + # this postprocessor, and compiled once per process (see the memo above). + # + # Gate on the CONFIGURED parser, not on `self.reasoning_parser`. What forces + # skip_special_tokens=false is the deployment's parser NAME -- + # preprocessor.rs::parser_requires_special_tokens (PRE.1) matches static + # names only, and vLLM's ParserEngine.adjust_request likewise -- so neither + # sees this request's enable_thinking or response_reasoning_ended. Those are + # exactly what null self.reasoning_parser just above, so an instance-keyed + # gate is a no-op precisely on the requests that still receive raw markers. + self._marker_strip_re: re.Pattern[str] | None = None + if reasoning_parser_class is not None or self.tool_parser is not None: + # A tool parser's own markers are its wire format, consumed + # downstream; removing them here would blind the tool-start scan. + owned = set(self._tool_start_markers()) | set(self._tool_end_markers()) + self._marker_strip_re = _compile_marker_strip_re( + tuple(m for m in self._control_markers if m not in owned) + ) + self.previous_text = "" self.previous_token_ids: list[int] = [] self.in_progress_tool_calls: dict[int, DeltaToolCall] = {} @@ -733,6 +903,13 @@ def __init__( self._tool_text_buffer: str | None = None self._dynamo_json_fallback_chunks: dict[int, list[str]] = {} + @property + def reasoning_token_total(self) -> int: + """Reasoning tokens for this choice, per the parser's own counter.""" + if self.reasoning_parser is None: + return 0 + return self.reasoning_parser.count_reasoning_tokens(self._reasoning_token_ids) + def _decode_dynamo_json_fallback_tool_calls( self, text: str ) -> list[dict[str, Any]]: @@ -915,6 +1092,27 @@ def _tool_end_markers(self) -> tuple[str, ...]: ) ) + # A text-grammar reasoning parser can only split the stream if the model's + # content-kind control markers survive detokenisation, which is why + # ParserEngine.adjust_request forces `skip_special_tokens = False`. But nothing + # then removes the markers the parser did NOT consume, so terminal ones leak + # into user-visible content, e.g.: + # content = "<|end_message|>408<|end_message|>" + # As shipped the two settings are mutually exclusive: the default strips the + # markers before the parser can split (reasoning is silently dropped), and + # False leaks them into the answer. Keep the markers for the parser, then strip + # the residue afterwards. + # + # Scope is deliberately narrow: only when a parser is actually active (the only + # case where skip_special_tokens was forced off), and only over literals that + # are genuinely in `tokenizer.all_special_tokens`. `reasoning` is left untouched + # -- it is already clean, and rewriting it would risk mangling legitimate text + # the model quoted. + def _strip_control_markers(self, text: str | None) -> str | None: + if not text or self._marker_strip_re is None: + return text + return self._marker_strip_re.sub("", text) + @staticmethod def _compose_delta_message( reasoning: str | None, content: str | None @@ -1045,6 +1243,16 @@ def _emit_tool_calls_choice(self, output: Any) -> dict[str, Any]: def _build_choice(self, output: Any, delta: dict[str, Any]) -> dict[str, Any]: if delta.get("tool_calls"): self._tool_call_choices_emitted.add(output.index) + # The single strip point. Every choice this class emits passes through here, + # so the invariant "no unconsumed control marker reaches the client" is + # enforced once at the funnel rather than at each producer -- stripping at + # the producers as well only re-scans text that is about to be scanned. + if isinstance(delta.get("content"), str): + stripped = self._strip_control_markers(delta["content"]) + if stripped: + delta["content"] = stripped + else: + delta.pop("content", None) return { "index": output.index, "delta": delta, @@ -1097,6 +1305,10 @@ def _process_non_streaming_tool_output(self, output: Any) -> dict[str, Any] | No return self._build_choice(output, delta) def process_output(self, output: Any) -> dict[str, Any] | None: + # BEFORE any branch: every path must feed the ledger, including the ones + # that emit nothing for this chunk. + if self.reasoning_parser is not None: + self._reasoning_token_ids.extend(output.token_ids or []) if self._uses_dynamo_json_tool_call_fallback: return self._process_dynamo_json_fallback_tool_calls(output) if self._should_buffer_for_non_streaming_tool_parse(): diff --git a/components/src/dynamo/frontend/tests/test_frontend_utils.py b/components/src/dynamo/frontend/tests/test_frontend_utils.py index 4c095b456fcc..24c68fc4e5cd 100644 --- a/components/src/dynamo/frontend/tests/test_frontend_utils.py +++ b/components/src/dynamo/frontend/tests/test_frontend_utils.py @@ -6,6 +6,7 @@ import pytest from dynamo.frontend.utils import ( + backend_invalid_argument_to_http_error, handle_engine_error, make_backend_error, make_internal_error, @@ -85,3 +86,73 @@ def test_none_response(self): def test_missing_token_ids(self): err = handle_engine_error({"other": "data"}, "req-1", logging.getLogger("test")) assert err["error"]["type"] == "internal_error" + + +class TestBackendInvalidArgumentToHttpError: # FRONTEND.8 — backend 4xx must not surface as 500 + def test_parses_code_and_message(self): + err = backend_invalid_argument_to_http_error( + ValueError( + 'BackendInvalidArgument: {"message":"The min_p and logit_bias ' + "sampling parameters are not yet supported with speculative " + 'decoding.","code":400}' + ) + ) + assert err is not None + assert err.code == 400 + assert "min_p" in err.message + # The serialized envelope must not leak into the client-visible text. + assert "BackendInvalidArgument" not in err.message + + def test_honours_a_non_400_status(self): + err = backend_invalid_argument_to_http_error( + ValueError('BackendInvalidArgument: {"message":"bad media","code":415}') + ) + assert err is not None and err.code == 415 + + def test_prefix_after_an_outer_wrapper(self): + err = backend_invalid_argument_to_http_error( + ValueError('Unknown: BackendInvalidArgument: {"message":"nope","code":400}') + ) + assert err is not None and err.message == "nope" + + def test_unstructured_payload_still_becomes_400(self): + err = backend_invalid_argument_to_http_error( + ValueError("BackendInvalidArgument: grammar is not valid") + ) + assert err is not None + assert err.code == 400 and err.message == "grammar is not valid" + + def test_missing_code_defaults_to_400(self): + err = backend_invalid_argument_to_http_error( + ValueError('BackendInvalidArgument: {"message":"nope"}') + ) + assert err is not None and err.code == 400 + + @pytest.mark.parametrize("code", [0, 99, 600, 1000, -1]) + def test_out_of_range_code_defers_to_the_caller(self, code): + # Rust degrades these to a 500, which is what the generic handler + # already produces -- returning None is how we defer to it. + assert ( + backend_invalid_argument_to_http_error( + ValueError(f'BackendInvalidArgument: {{"message":"x","code":{code}}}') + ) + is None + ) + + def test_boolean_code_is_not_a_status(self): + err = backend_invalid_argument_to_http_error( + ValueError('BackendInvalidArgument: {"message":"x","code":true}') + ) + assert err is not None and err.code == 400 + + @pytest.mark.parametrize( + "text", + [ + "CUDA out of memory", + "BackendUnknown: something broke", + '{"message":"no prefix","code":400}', + "", + ], + ) + def test_unrelated_errors_are_left_alone(self, text): + assert backend_invalid_argument_to_http_error(ValueError(text)) is None diff --git a/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py b/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py index b58888b4e878..5220518e46b3 100644 --- a/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py +++ b/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py @@ -24,6 +24,7 @@ ) from transformers import AutoTokenizer from vllm.sampling_params import SamplingParams, StructuredOutputsParams +from vllm.tool_parsers import ToolParser from dynamo.frontend import prepost as prepost_module from dynamo.frontend.prepost import ( @@ -31,7 +32,7 @@ _prepare_request, build_tool_call_guided_decoding, ) -from dynamo.llm.exceptions import InvalidArgument +from dynamo.llm.exceptions import HttpError, InvalidArgument # NOTE: dynamo.frontend.vllm_processor is imported lazily inside the tests that # need it (and via the vllm_processor_module fixture). Importing it at module @@ -936,7 +937,7 @@ def test_include_reasoning_false_only_marks_engine_reasoning_ended(self): from dynamo.frontend.vllm_processor import _build_reasoning_parser_metadata class FakeReasoningParser: - def __init__(self, tokenizer, *, chat_template_kwargs): + def __init__(self, tokenizer, *, chat_template_kwargs, model_config=None): self.tokenizer = tokenizer self.chat_template_kwargs = chat_template_kwargs @@ -961,7 +962,7 @@ def test_parser_receives_chat_template_kwargs(self): from dynamo.frontend.vllm_processor import _build_reasoning_parser_metadata class FakeReasoningParser: - def __init__(self, tokenizer, *, chat_template_kwargs): + def __init__(self, tokenizer, *, chat_template_kwargs, model_config=None): self.tokenizer = tokenizer self.chat_template_kwargs = chat_template_kwargs @@ -1150,10 +1151,9 @@ def __init__(self, **kwargs): @pytest.mark.asyncio -async def test_generator_preserves_zero_top_logprobs( +async def test_generator_rejects_logprobs_including_zero_top_logprobs( vllm_processor_module, monkeypatch, - caplog, ): class RequestForSampling(SimpleNamespace): model_fields = frozenset() @@ -1184,13 +1184,16 @@ class ProjectionObserved(Exception): pass def process_inputs(request_id, engine_inputs, sampling_params, supported_tasks): - assert sampling_params.logprobs == 0 + # logprobs are rejected upstream of here; reaching this is the failure. raise ProjectionObserved input_processor = SimpleNamespace( generation_config_fields={}, renderer=SimpleNamespace(process_for_engine_async=AsyncMock(return_value={})), process_inputs=process_inputs, + # Real InputProcessor always carries this (vllm_config.model_config); + # _generator_inner forwards it to the reasoning parser. + model_config=None, ) processor = vllm_processor_module.VllmProcessor( @@ -1202,7 +1205,11 @@ def process_inputs(request_id, engine_inputs, sampling_params, supported_tasks): routed_engine=object(), ) - with pytest.raises(ProjectionObserved): + # `logprobs=True` with `top_logprobs=0` projects to sampling_params.logprobs + # == 0, so a truthiness guard would let exactly this request through and it + # would fail later as an undeserializable empty logprobs struct (a 500). The + # guard tests `is not None`; this pins that distinction. + with pytest.raises(HttpError) as excinfo: await anext( processor._generator_inner( { @@ -1213,10 +1220,9 @@ def process_inputs(request_id, engine_inputs, sampling_params, supported_tasks): } ) ) - assert ( - "Logprobs requested but not supported in distributed inference mode" - in caplog.messages - ) + + assert excinfo.value.code == 400 + assert "logprobs" in excinfo.value.message @pytest.mark.asyncio @@ -1230,7 +1236,7 @@ class RequestForSampling(SimpleNamespace): class FakeReasoningParser: engine_based_streaming = False - def __init__(self, tokenizer, *, chat_template_kwargs): + def __init__(self, tokenizer, *, chat_template_kwargs, model_config=None): self.tokenizer = tokenizer self.chat_template_kwargs = chat_template_kwargs self.adjusted_prompt_token_ids = None @@ -1296,6 +1302,9 @@ def process_inputs(request_id, engine_inputs, sampling_params, supported_tasks): generation_config_fields={}, renderer=SimpleNamespace(process_for_engine_async=AsyncMock(return_value={})), process_inputs=process_inputs, + # Real InputProcessor always carries this (vllm_config.model_config); + # _generator_inner forwards it to the reasoning parser. + model_config=None, ) processor = vllm_processor_module.VllmProcessor( tokenizer=SimpleNamespace(eos_token_id=2, all_special_tokens=[]), @@ -1379,6 +1388,68 @@ async def capture_generate_and_stream( } +@pytest.mark.asyncio +async def test_generator_inner_forwards_reasoning_parser_and_model_config( + vllm_processor_module, + monkeypatch, +): + """The processor must hand its own parser AND model config to preprocessing. + + Every other reasoning-parser test calls preprocess_chat_request or + _prepare_request directly with an explicit reasoning_parser_class, so all of + them keep passing if the production call in _generator_inner stops forwarding + them. This is the boundary check that does not: it reads the arguments off the + real call site. Deleting either wiring line makes exactly this test fail. + + model_config is checked alongside because it is equally load-bearing and + equally invisible: without it the shipped Cohere parsers return the request + untouched and adjust_request() is a silent no-op. + """ + + class _SentinelReasoningParser: + """Identity is the whole point -- this is never instantiated.""" + + class _StopAfterPreprocess(Exception): + """Ends the generator at the boundary under test.""" + + sentinel_model_config = SimpleNamespace(architecture="SentinelForCausalLM") + captured: dict = {} + + async def spy_preprocess_chat_request(request, **kwargs): + captured.update(kwargs) + raise _StopAfterPreprocess + + monkeypatch.setattr( + vllm_processor_module, + "preprocess_chat_request", + spy_preprocess_chat_request, + ) + + processor = vllm_processor_module.VllmProcessor( + tokenizer=SimpleNamespace(eos_token_id=2), + input_processor=SimpleNamespace( + renderer=object(), model_config=sentinel_model_config + ), + output_processor=object(), + tool_parser_class=None, + reasoning_parser_class=_SentinelReasoningParser, + routed_engine=object(), + ) + + with pytest.raises(_StopAfterPreprocess): + await anext( + processor._generator_inner( + { + "model": "test", + "messages": [{"role": "user", "content": "Hello"}], + } + ) + ) + + assert captured.get("reasoning_parser_class") is _SentinelReasoningParser + assert captured.get("model_config") is sentinel_model_config + + def _make_processor(module, routed_engine): processor = module.VllmProcessor.__new__(module.VllmProcessor) processor.routed_engine = routed_engine @@ -1423,6 +1494,52 @@ async def _run_generate(processor, preproc, *, mm_routing_info=None, context=Non class TestRoutedEnginePath: + @pytest.mark.asyncio + async def test_backend_rejection_keeps_the_backend_status( + self, vllm_processor_module + ): + # A worker-side rejection crosses the Rust boundary as a plain ValueError + # whose text carries the real status. It must reach the client as that + # status with that message -- not as a generic 500 with both discarded. + class _RejectingEngine(_FakeRoutedEngine): + async def generate(self, preprocessed, **kwargs): + raise ValueError( + 'BackendInvalidArgument: {"message":"The min_p and logit_bias ' + "sampling parameters are not yet supported with speculative " + 'decoding.","code":400}' + ) + + processor = _make_processor(vllm_processor_module, _RejectingEngine()) + + with pytest.raises(HttpError) as excinfo: + await _run_generate(processor, _base_preproc()) + + assert excinfo.value.code == 400 + assert "min_p" in excinfo.value.message + # The serialized envelope must not leak to the client. + assert "BackendInvalidArgument" not in excinfo.value.message + + @pytest.mark.asyncio + async def test_genuine_internal_failure_is_still_internal( + self, vllm_processor_module + ): + """A real engine fault stays internal, and is sent as a tagged frame.""" + + class _BrokenEngine(_FakeRoutedEngine): + async def generate(self, preprocessed, **kwargs): + raise RuntimeError("CUDA out of memory") + + processor = _make_processor(vllm_processor_module, _BrokenEngine()) + + chunks = await _run_generate(processor, _base_preproc()) + + # Yielded, not raised: an error without the discriminator is not a 4xx. + last = chunks[-1] + # It must also be tagged. An untagged dict fails to parse and becomes a 500. + assert last["_dynamo_annotated"] is True + assert last["event"] == "error" + assert "CUDA out of memory" in last["comment"][0] + @pytest.mark.asyncio async def test_routed_engine_gets_extra_args_metadata(self, vllm_processor_module): routed_engine = _FakeRoutedEngine() @@ -1708,6 +1825,338 @@ async def test_root_thinking_suppresses_deployment_default(self, tokenizer): assert "thinking_mode" not in result.chat_template_kwargs +class _FakeStructuralTagReasoningParser: + """A reasoning parser that rewrites the caller's constraint. + + Mirrors a real text-grammar parser converting response_format into the + structural tag its model actually speaks. + """ + + def __init__(self, tokenizer, chat_template_kwargs=None, model_config=None): + del tokenizer, chat_template_kwargs + self._model_config = model_config + + def adjust_request(self, request): + request.skip_special_tokens = False + request.structured_outputs = StructuredOutputsParams( + structural_tag='{"format": "reasoning"}' + ) + return request + + +class _FakePassthroughReasoningParser(_FakeStructuralTagReasoningParser): + def adjust_request(self, request): + return request + + +class _ModelConfigGatedReasoningParser(_FakeStructuralTagReasoningParser): + """Rewrites the constraint only when it was given a model_config. + + This is the shipped Cohere parsers' actual contract: + BaseCohereCommandReasoningParser.adjust_request reads + ``self._model_config.architecture`` and returns the request untouched when the + config is absent. A fake that ignores model_config cannot tell a working call + from a no-op, which is how the missing kwarg went unnoticed. + """ + + def adjust_request(self, request): + if self._model_config is None: + return request + return super().adjust_request(request) + + +class TestReasoningParserGuidanceForwarding: + """A reasoning parser's guidance must be recomputed and forwarded. + + Regression: parser_guided_decoding was gated on `tool_parser is not None`, and + build_tool_call_guided_decoding returns early when there is no tool-call + guidance to build. A reasoning parser that rewrote the caller's constraint + therefore never surfaced: the request carried the adjusted constraint while the + engine received the pre-adjustment copy captured before _prepare_request ran. + """ + + @staticmethod + def _request(**overrides): + """A fresh request each call, so no test can perturb another's input.""" + return { + "model": MODEL, + "messages": [{"role": "user", "content": "Hello"}], + "response_format": {"type": "json_object"}, + **overrides, + } + + @staticmethod + def _renderer(): + return SimpleNamespace( + render_messages_async=AsyncMock( + return_value=(None, {"prompt_token_ids": [1]}) + ) + ) + + async def _preprocess(self, tokenizer, *, request=None, **kwargs): + kwargs.setdefault("tool_parser_class", None) + return await prepost_module.preprocess_chat_request( + request if request is not None else self._request(), + tokenizer=tokenizer, + renderer=self._renderer(), + **kwargs, + ) + + @pytest.mark.asyncio + async def test_rewrite_is_forwarded_without_a_tool_parser(self, tokenizer): + """The whole point: no tool parser anywhere in the request.""" + result = await self._preprocess( + tokenizer, + reasoning_parser_class=_FakeStructuralTagReasoningParser, + ) + + assert result.guided_decoding == {"structural_tag": '{"format": "reasoning"}'} + # The pre-adjustment copy must not win. + assert result.guided_decoding != {"json": {"type": "object"}} + + @pytest.mark.asyncio + async def test_passthrough_parser_keeps_the_caller_constraint(self, tokenizer): + """adjust_request that changes nothing must not disturb the caller.""" + result = await self._preprocess( + tokenizer, + reasoning_parser_class=_FakePassthroughReasoningParser, + ) + + assert result.guided_decoding == {"json": {"type": "object"}} + + @pytest.mark.asyncio + async def test_thinking_disabled_does_not_recompute(self, tokenizer): + """The gate is off, so no parser runs and nothing may be recomputed. + + Recomputing here would forward a constraint from a parser that the + postprocessor will not build. + """ + result = await self._preprocess( + tokenizer, + request=self._request(chat_template_kwargs={"enable_thinking": False}), + reasoning_parser_class=_FakeStructuralTagReasoningParser, + ) + + assert result.guided_decoding == {"json": {"type": "object"}} + assert result.request_for_sampling.skip_special_tokens is not False + + @pytest.mark.asyncio + async def test_rewrite_is_forwarded_with_a_tool_parser_also_active(self, tokenizer): + """Attribution is measured, so a tool parser being present is irrelevant. + + Both parsers run against the same request inside _prepare_request. The + snapshot taken between them shows the tool parser left the guidance alone + and the reasoning parser rewrote it, so the rewrite is attributable and + wins -- exactly as it does with no tool parser at all. + """ + result = await self._preprocess( + tokenizer, + request=self._request(tools=TOOL_REQUEST["tools"], tool_choice="auto"), + tool_parser_class=_FakePassthroughToolParser, + reasoning_parser_class=_FakeStructuralTagReasoningParser, + ) + + assert result.guided_decoding == {"structural_tag": '{"format": "reasoning"}'} + + @pytest.mark.asyncio + async def test_tool_parser_rewrite_still_loses_to_the_caller(self, tokenizer): + """The other half: tool-path precedence is deliberately unchanged. + + Same measurement, opposite attribution -- the tool parser moved the + guidance and the reasoning parser did not -- so the caller's explicit + constraint still wins, as pinned by + test_assistant_guidance_takes_precedence_over_auto_tool_guidance and the + response-format-precedence row of TOOL_GUIDANCE_PARITY_CASES. + """ + result = await self._preprocess( + tokenizer, + request=self._request(tools=TOOL_REQUEST["tools"], tool_choice="auto"), + tool_parser_class=_FakeAdjustRequestGrammarToolParser, + reasoning_parser_class=_FakePassthroughReasoningParser, + ) + + assert result.guided_decoding == {"json": {"type": "object"}} + + @pytest.mark.asyncio + async def test_model_config_reaches_the_parser(self, tokenizer): + """A parser that needs model_config must actually receive it. + + Without it the shipped Cohere parsers return the request untouched, which + made the whole adjust_request() call a no-op for the only parser in vLLM + that overrides it. + """ + result = await self._preprocess( + tokenizer, + reasoning_parser_class=_ModelConfigGatedReasoningParser, + model_config=SimpleNamespace(architecture="CohereForCausalLM"), + ) + + assert result.guided_decoding == {"structural_tag": '{"format": "reasoning"}'} + + @pytest.mark.asyncio + async def test_without_model_config_the_gated_parser_is_a_no_op(self, tokenizer): + """The negative control: same parser, no config, caller's constraint stands. + + This is the behaviour the PR shipped before model_config was threaded + through -- kept so the pair documents the difference. + """ + result = await self._preprocess( + tokenizer, + reasoning_parser_class=_ModelConfigGatedReasoningParser, + ) + + assert result.guided_decoding == {"json": {"type": "object"}} + + +class _PassthroughStreamingToolParser(ToolParser): + """A tool parser that finds no tool call and passes the raw delta through. + + Subclasses the real ToolParser so every attribute the postprocessor touches + exists, and returns the unstripped `delta_text` the way a real parser does. + """ + + def extract_tool_calls_streaming(self, **kwargs): + return prepost_module.DeltaMessage(content=kwargs["delta_text"]) + + def get_structural_tag(self, request, *, reasoning=False): + return None + + +class _InactiveReasoningParser: + """A configured reasoning parser that never becomes active on a request. + + `enable_thinking=False` and `response_reasoning_ended=True` both leave + StreamingPostProcessor.reasoning_parser None, so this is only ever passed as a + class -- constructing it is the bug this fake exists to catch. + """ + + def __init__(self, tokenizer, *, chat_template_kwargs): + raise AssertionError("must not be constructed for an inactive request") + + +class _MarkerOwningToolParser(_PassthroughStreamingToolParser): + """A tool parser that owns two of the tokenizer's special tokens.""" + + tool_call_start_token = "<|im_start|>" + tool_call_end_token = "<|im_end|>" + + +class TestControlMarkerStrip: + """Unconsumed special tokens must not reach the client. + + A text-grammar reasoning parser can only split the stream if the model's + content-kind markers survive detokenisation, which is why + ParserEngine.adjust_request forces skip_special_tokens=False. Nothing then + removes the markers the parser did not consume, so terminal ones leak into + visible content -- the report was content = "<|end_message|>408<|end_message|>", + a special token on that model's tokenizer. These use `<|im_end|>`, which is + the equivalent on Qwen3-0.6B. + """ + + @staticmethod + def _post( + tokenizer, + *, + tool_parser=None, + reasoning_parser_class=None, + response_reasoning_ended=None, + chat_template_kwargs=None, + ): + request = json.loads(json.dumps(TOOL_REQUEST)) + request.pop("tools", None) + request, _, _, _, _ = _prepare_request( + request, tokenizer=tokenizer, tool_parser_class=None + ) + return StreamingPostProcessor( + tokenizer=tokenizer, + request_for_sampling=request, + sampling_params=SamplingParams(), + prompt_token_ids=[], + tool_parser=tool_parser, + reasoning_parser_class=reasoning_parser_class, + chat_template_kwargs=chat_template_kwargs or {}, + response_reasoning_ended=response_reasoning_ended, + stream_response=True, + ) + + @staticmethod + def _content(post, text): + choice = post.process_output( + SimpleNamespace( + index=0, + text=text, + token_ids=[], + finish_reason="stop", + logprobs=None, + ) + ) + assert choice is not None + return choice["delta"].get("content") + + def test_reported_leak_is_removed(self, tokenizer): + post = self._post( + tokenizer, tool_parser=_PassthroughStreamingToolParser(tokenizer) + ) + assert self._content(post, "<|im_end|>408<|im_end|>") == "408" + + def test_non_special_literal_of_the_same_shape_survives(self, tokenizer): + """Looking like a control token is not proof of being one. + + `<|not_a_real_special_token|>` is absent from all_special_tokens and + encodes to ordinary token IDs, so it is generated text and must reach the + client intact. + """ + post = self._post( + tokenizer, tool_parser=_PassthroughStreamingToolParser(tokenizer) + ) + literal = "The literal is <|not_a_real_special_token|>." + assert self._content(post, literal) == literal + + def test_parser_owned_markers_are_preserved(self, tokenizer): + """A marker the parser owns is wire format, consumed downstream.""" + post = self._post(tokenizer, tool_parser=_MarkerOwningToolParser(tokenizer)) + assert ( + post._strip_control_markers("<|im_start|>{}<|im_end|>") + == "<|im_start|>{}<|im_end|>" + ) + # A special token it does not own is still removed. + assert post._strip_control_markers("<|endoftext|>x") == "x" + + def test_no_parser_configured_is_passthrough(self, tokenizer): + """Only a *configured* parser forces skip_special_tokens off.""" + post = self._post(tokenizer) + assert post.tool_parser is None and post.reasoning_parser is None + assert self._content(post, "<|im_end|>408") == "<|im_end|>408" + + @pytest.mark.parametrize( + "inactive_kwargs", + [ + {"response_reasoning_ended": True}, + {"chat_template_kwargs": {"enable_thinking": False}}, + ], + ids=["reasoning-already-ended", "thinking-disabled"], + ) + def test_configured_but_inactive_reasoning_parser_still_strips( + self, tokenizer, inactive_kwargs + ): + """The CONFIGURED parser forces skip_special_tokens off, not the instance. + + preprocessor.rs::parser_requires_special_tokens keys the + skip_special_tokens=false default off the deployment's configured + reasoning_parser NAME; it never sees this request's enable_thinking or + response_reasoning_ended. But this class leaves self.reasoning_parser None on + exactly those requests, so gating the strip on the instance made it a no-op + precisely where markers still arrive undecoded. + """ + post = self._post( + tokenizer, + reasoning_parser_class=_InactiveReasoningParser, + **inactive_kwargs, + ) + assert post.reasoning_parser is None + assert self._content(post, "<|im_end|>408<|im_end|>") == "408" + + class TestToolCallGuidedDecoding: def _request(self, tokenizer, **overrides): raw_request = {**TOOL_REQUEST, "tool_choice": "auto", **overrides} @@ -2539,3 +2988,316 @@ def test_shared_thinking_policy(self, tokenizer, case): tool_parser_class=None, ) assert kwargs == case.expected + + +class _AccountingReasoningParser: + """Emits reasoning until a chunk contains "", then visible content.""" + + engine_based_streaming = False + + def __init__(self, tokenizer, *, chat_template_kwargs=None, model_config=None): + self.tokenizer = tokenizer + self.chat_template_kwargs = chat_template_kwargs or {} + self.model_config = model_config + + def is_reasoning_end(self, prompt_token_ids): + return False + + def adjust_initial_state_from_prompt(self, prompt_token_ids): + return None + + def extract_reasoning_streaming( + self, + previous_text, + current_text, + delta_text, + previous_token_ids, + current_token_ids, + delta_token_ids, + ): + if "" in delta_text: + return prepost_module.DeltaMessage(content=delta_text.split("")[-1]) + return prepost_module.DeltaMessage(reasoning=delta_text) + + def is_reasoning_end_streaming(self, current_token_ids, delta_token_ids): + return False + + def extract_reasoning(self, text, request=None): + if "" in text: + reasoning, _, content = text.partition("") + return reasoning, content + return text, "" + + def count_reasoning_tokens(self, token_ids): + """Everything up to and including the end marker counts as reasoning. + + The production code delegates to this, so the fake has to provide it -- + a fake that omits it inherits ReasoningParser's base, which returns 0. + """ + end = self.tokenizer.encode("", add_special_tokens=False) + end_id = end[-1] if end else None + count = 0 + for tid in token_ids: + count += 1 + if tid == end_id: + break + return count + + +class _PassthroughToolParser: + """Finds no tool calls; returns the text as content. + + The buffered non-streaming path only needs `tool_parser is not None` to engage, + so this keeps the reasoning-accounting test focused on reasoning. + """ + + def __init__(self, tokenizer, *args, **kwargs): + self.tokenizer = tokenizer + + def extract_tool_calls(self, text, request=None): + return SimpleNamespace(tools_called=False, tool_calls=[], content=text) + + +class TestReasoningTokenAccounting: + """Reasoning-token usage on the Python chat-processor path (NVBug 6678449b). + + dynamo #12181 added this only to the Rust OpenAIPreprocessor, which + `--dyn-chat-processor vllm` bypasses, so `reasoning_tokens` was absent from usage + entirely (the worker's _build_completion_usage emits no completion_tokens_details). + + The count is accumulated where the reasoning parser CLASSIFIES output, NOT where + the response is projected. The two regression tests below both reported 0 when it + was derived from the emitted choices. + """ + + @staticmethod + def _post(tokenizer, *, include_reasoning=True, stream_response=True, tools=False): + request = SimpleNamespace( + include_reasoning=include_reasoning, + tool_choice="auto" if tools else "none", + model_fields=frozenset(), + ) + return StreamingPostProcessor( + tokenizer=tokenizer, + request_for_sampling=request, + sampling_params=SamplingParams(), + prompt_token_ids=[], + tool_parser=_PassthroughToolParser(tokenizer) if tools else None, + reasoning_parser_class=_AccountingReasoningParser, + chat_template_kwargs={}, + stream_response=stream_response, + ) + + @staticmethod + def _out(text, token_ids, finish_reason=None, index=0): + return SimpleNamespace( + index=index, + text=text, + token_ids=token_ids, + finish_reason=finish_reason, + logprobs=None, + ) + + @staticmethod + def _annotator(*posts): + from dynamo.frontend.vllm_processor import _ReasoningUsageAnnotator + + return _ReasoningUsageAnnotator({i: p for i, p in enumerate(posts)}) + + # -- streaming classification ------------------------------------------ + + def test_streaming_counts_reasoning_only_not_the_answer(self, tokenizer): + """Real parser, real ids: the visible answer must not be counted.""" + from vllm.reasoning import ReasoningParserManager + + reasoning = tokenizer.encode("abc", add_special_tokens=False) + answer = tokenizer.encode("the answer", add_special_tokens=False) + native = ReasoningParserManager.get_reasoning_parser("qwen3")( + tokenizer + ).count_reasoning_tokens(reasoning + answer) + + post = self._qwen3_post(tokenizer) + for tid in reasoning + answer: + post.process_output(self._out(tokenizer.decode([tid]), [tid])) + assert post.reasoning_token_total == native + assert 0 < native < len(reasoning + answer) + + # -- P1 #1: account before the include_reasoning projection ------------ + + def test_hidden_reasoning_is_counted_when_include_reasoning_false(self, tokenizer): + """include_reasoning=false suppresses the reasoning DELTA, not the tokens. + + Observing emitted choices saw no reasoning_content here and reported 0 for a + response that really did spend tokens reasoning. + """ + post = self._post(tokenizer, include_reasoning=False) + choice = post.process_output(self._out("hidden", [1, 2, 3])) + + # The projection still hides it -- that part is intended... + assert choice is None or "reasoning_content" not in (choice.get("delta") or {}) + # ...but the tokens are accounted anyway. + assert post.reasoning_token_total == 3 + annotated = self._annotator(post).annotate({"completion_tokens": 10}) + assert annotated["completion_tokens_details"]["reasoning_tokens"] == 3 + + # -- P1 #2: survive non-streaming tool buffering ------------------------ + + def test_non_streaming_tool_buffering_preserves_count(self, tokenizer): + """The non-streaming tool path emits nothing per chunk and releases on the + terminal delta, where chunk_tokens is 0. Per-chunk observation of emitted + choices therefore saw only zeros and reported 0. + """ + post = self._post(tokenizer, stream_response=False, tools=True) + for chunk in ("think a", "think b", "think c"): + assert post.process_output(self._out(chunk, [1, 2])) is None + post.process_output(self._out("answer", [3], finish_reason="stop")) + + assert post.reasoning_token_total > 0 + annotated = self._annotator(post).annotate({"completion_tokens": 20}) + assert annotated["completion_tokens_details"]["reasoning_tokens"] > 0 + + def test_non_streaming_buffering_counts_even_when_suppressed(self, tokenizer): + """Both projections at once: buffered AND include_reasoning=false.""" + post = self._post( + tokenizer, stream_response=False, tools=True, include_reasoning=False + ) + post.process_output(self._out("thinking hard", [1, 2])) + post.process_output(self._out("answer", [3], finish_reason="stop")) + assert post.reasoning_token_total > 0 + + def test_no_reasoning_parser_counts_nothing(self, tokenizer): + """Without a reasoning parser there is nothing to delegate to -> 0. + + Note the counterpart is NOT true: with a parser, plain content is not + automatically zero. Qwen3's parser counts an unopened span as reasoning + (the model is trained to reason first), so delegating means inheriting + that contract -- which is the point, since it is what vLLM itself + reports. + """ + request = SimpleNamespace( + include_reasoning=True, tool_choice="none", model_fields=frozenset() + ) + post = StreamingPostProcessor( + tokenizer=tokenizer, + request_for_sampling=request, + sampling_params=SamplingParams(), + prompt_token_ids=[], + tool_parser=None, + reasoning_parser_class=None, + chat_template_kwargs={}, + stream_response=True, + ) + for tid in tokenizer.encode("just an answer", add_special_tokens=False): + post.process_output(self._out(tokenizer.decode([tid]), [tid])) + assert post.reasoning_token_total == 0 + + # -- the parser's OWN counter, not a re-derivation ----------------------- + + @staticmethod + def _qwen3_post(tokenizer): + from vllm.reasoning import ReasoningParserManager + + request = SimpleNamespace( + include_reasoning=True, tool_choice="none", model_fields=frozenset() + ) + return StreamingPostProcessor( + tokenizer=tokenizer, + request_for_sampling=request, + sampling_params=SamplingParams(), + prompt_token_ids=[], + tool_parser=None, + reasoning_parser_class=ReasoningParserManager.get_reasoning_parser("qwen3"), + chat_template_kwargs={}, + stream_response=True, + ) + + def test_count_matches_the_parser_for_split_marker_tokens(self, tokenizer): + """``, `abc`, `<`, `x` -- the parser counts 3. + + A per-chunk classifier that adds len(delta_token_ids) whenever the chunk + carried reasoning reported 2 here. Delegating to the parser's own + count_reasoning_tokens is what makes this exact. + """ + from vllm.reasoning import ReasoningParserManager + + ids = [ + i + for piece in ("", "abc", "<", "x") + for i in tokenizer.encode(piece, add_special_tokens=False) + ] + native = ReasoningParserManager.get_reasoning_parser("qwen3")( + tokenizer + ).count_reasoning_tokens(ids) + assert native == 3, f"fixture drift: parser now counts {native}" + + post = self._qwen3_post(tokenizer) + for tid in ids: + post.process_output(self._out(tokenizer.decode([tid]), [tid])) + assert post.reasoning_token_total == native == 3 + + def test_count_survives_non_injective_detokenisation(self, tokenizer): + """Generated id 77150 decodes to '10', which RE-ENCODES to two tokens. + + Counting by re-encoding the decoded reasoning text therefore reported 2 + for a single generated token. The ledger keeps the original ids, so the + parser sees exactly what was generated. + """ + think = tokenizer.encode("", add_special_tokens=False) + ids = think + [77150] + assert ( + len(tokenizer.encode(tokenizer.decode([77150]), add_special_tokens=False)) + == 2 + ), "fixture drift: 77150 no longer re-encodes to 2 tokens" + + from vllm.reasoning import ReasoningParserManager + + native = ReasoningParserManager.get_reasoning_parser("qwen3")( + tokenizer + ).count_reasoning_tokens(ids) + post = self._qwen3_post(tokenizer) + for tid in ids: + post.process_output(self._out(tokenizer.decode([tid]), [tid])) + assert post.reasoning_token_total == native == 1 + + # -- annotation --------------------------------------------------------- + + def test_total_sums_across_choices(self, tokenizer): + a, b = self._post(tokenizer), self._post(tokenizer) + a.process_output(self._out("x", [1, 2])) + b.process_output(self._out("y", [3, 4, 5])) + assert self._annotator(a, b).total == 5 + + def test_annotate_fills_when_absent_and_does_not_mutate_input(self, tokenizer): + post = self._post(tokenizer) + post.process_output(self._out("x", [1, 2])) + usage = {"completion_tokens": 100} + out = self._annotator(post).annotate(usage) + assert out["completion_tokens_details"]["reasoning_tokens"] == 2 + assert "completion_tokens_details" not in usage + + def test_annotate_preserves_a_positive_backend_value(self, tokenizer): + post = self._post(tokenizer) + post.process_output(self._out("x", [1, 2])) + usage = {"completion_tokens_details": {"reasoning_tokens": 7}} + annotated = self._annotator(post).annotate(usage) + assert annotated["completion_tokens_details"]["reasoning_tokens"] == 7 + + def test_annotate_replaces_a_zero_backend_value(self, tokenizer): + post = self._post(tokenizer) + post.process_output(self._out("x", [1, 2])) + usage = {"completion_tokens_details": {"reasoning_tokens": 0}} + annotated = self._annotator(post).annotate(usage) + assert annotated["completion_tokens_details"]["reasoning_tokens"] == 2 + + @pytest.mark.parametrize("backend", [-1, -100, None, "7"]) + def test_annotate_replaces_a_non_positive_backend_value(self, tokenizer, backend): + """Only a POSITIVE backend count is authoritative. + + -1 is truthy, so a plain falsiness check preserved it and reported a + negative reasoning_tokens to the client. + """ + post = self._post(tokenizer) + post.process_output(self._out("x", [1, 2])) + usage = {"completion_tokens_details": {"reasoning_tokens": backend}} + annotated = self._annotator(post).annotate(usage) + assert annotated["completion_tokens_details"]["reasoning_tokens"] == 2 diff --git a/components/src/dynamo/frontend/utils.py b/components/src/dynamo/frontend/utils.py index 88e1d24e2242..e79e3164efac 100644 --- a/components/src/dynamo/frontend/utils.py +++ b/components/src/dynamo/frontend/utils.py @@ -9,6 +9,8 @@ import uuid from typing import Any, Literal +from dynamo.llm.exceptions import HttpError + _MASK_64_BITS = (1 << 64) - 1 @@ -181,6 +183,67 @@ def make_internal_error(request_id: str, detail: str | None = None) -> dict[str, } +def as_error_envelope(error_payload: dict[str, Any]) -> dict[str, Any]: + """Tag an error dict so the binding reads it as an error frame. + + Without ``_dynamo_annotated`` the binding reads the dict as a completion + chunk and fails with ``missing field `id```. The client then gets a 500 and + never sees the message. The HTTP layer reads the text back off ``comment``. + See ``depythonize_annotated`` in ``lib/bindings/python/rust/engine.rs``. + """ + message = (error_payload.get("error") or {}).get("message") or "unknown error" + return {"_dynamo_annotated": True, "event": "error", "comment": [message]} + + +_SERIALIZED_BACKEND_INVALID_ARGUMENT_PREFIX = "BackendInvalidArgument: " + + +def backend_invalid_argument_to_http_error(exc: BaseException) -> HttpError | None: + """Recover a worker's own HTTP status from a serialized backend error. + + A worker that rejects a request -- an unsupported sampling parameter, an + unparseable grammar -- fails it with ``{"message": ..., "code": 4xx}``. + Crossing the Rust boundary turns that into a plain Python exception whose + text is ``BackendInvalidArgument: {json}``, so a generic ``except + Exception`` reports a 500 and discards both the status and the reason. + + Rust's HTTP service already performs this recovery + (``extract_backend_error_if_present`` in + ``lib/llm/src/http/service/openai.rs``), but the Python chat processors do + not go through it, so they need their own. Mirror its status rules: honour + an explicit code, and otherwise fall back to 400 -- the prefix itself is + the discriminator proving the argument was invalid. + + Returns None when ``exc`` is not that shape, so callers fall through to + their existing handling instead of inventing a status. + """ + # Not ``removeprefix``: adapter paths wrap the error in an outer + # ``: `` before it reaches Python, so the discriminator is not + # always at position 0. + _, prefix, payload = str(exc).partition(_SERIALIZED_BACKEND_INVALID_ARGUMENT_PREFIX) + if not prefix: + return None + + try: + parsed = json.loads(payload) + except ValueError: + parsed = None + if not isinstance(parsed, dict): + # Prefix but no payload: still an invalid argument, just unstructured. + return HttpError(400, payload) + + code = parsed.get("code") + if isinstance(code, bool) or not isinstance(code, int): + code = 400 + elif not 100 <= code < 600: + # Rust degrades an out-of-range code to a 500, which is exactly what + # the caller's existing handler already produces -- defer to it. + return None + + message = parsed.get("message") + return HttpError(code, message if isinstance(message, str) else payload) + + def handle_engine_error( engine_response: Any, request_id: str, diff --git a/components/src/dynamo/frontend/vllm_processor.py b/components/src/dynamo/frontend/vllm_processor.py index cb5ded3fe439..83cf26fa7241 100644 --- a/components/src/dynamo/frontend/vllm_processor.py +++ b/components/src/dynamo/frontend/vllm_processor.py @@ -39,11 +39,14 @@ from dynamo.common.utils import nvtx_utils as _nvtx from dynamo.frontend.frontend_args import FrontendConfig from dynamo.llm import ModelCardInstanceId, PythonAsyncEngine, RoutedEngine +from dynamo.llm.exceptions import HttpError from dynamo.vllm.errors import vllm_client_error_to_http_error from .prepost import StreamingPostProcessor, preprocess_chat_request from .thinking import runtime_default_thinking_mode from .utils import ( + as_error_envelope, + backend_invalid_argument_to_http_error, extract_mm_urls, handle_engine_error, make_internal_error, @@ -64,6 +67,54 @@ } +class _ReasoningUsageAnnotator: + """Report reasoning-token usage on the PYTHON chat-processor path. + + NVBug 6678449b. dynamo #12181 added this, but only to the RUST OpenAIPreprocessor + (lib/llm/src/preprocessor.rs). `--dyn-chat-processor vllm` selects this Python path, + which bypasses that code entirely -- and the worker's _build_completion_usage() + emits no `completion_tokens_details` key at all, so `reasoning_tokens` is ABSENT + (not null) from usage. + + The count is NOT derived here. Each StreamingPostProcessor accumulates + `reasoning_token_total` where the reasoning parser CLASSIFIES its output; this + class only sums those totals and annotates usage. Deriving it here -- from the + choices this path emits -- is what reported zero whenever the response projection + dropped or deferred the reasoning (include_reasoning=false, and the non-streaming + tool path's terminal-delta buffering). + + Streaming classification stays chunk-granular, matching the Rust: a chunk carrying + both reasoning and visible content counts entirely as reasoning. The non-streaming + tool path has no per-chunk classification, so it counts the parsed reasoning text + exactly; that is more precise than the Rust rather than divergent from it. + A positive backend-supplied count stays authoritative. + """ + + __slots__ = ("_post_processors",) + + def __init__(self, post_processors: dict[int, StreamingPostProcessor]) -> None: + self._post_processors = post_processors + + @property + def total(self) -> int: + return sum( + post.reasoning_token_total for post in self._post_processors.values() + ) + + def annotate(self, usage: dict[str, Any]) -> dict[str, Any]: + """Return a COPY of usage carrying reasoning_tokens; never mutates the input.""" + annotated = dict(usage) + details = dict(annotated.get("completion_tokens_details") or {}) + # Only a POSITIVE backend count is authoritative. Missing, zero and + # negative all fall back to our own tally -- a negative would otherwise + # survive, since -1 is truthy. + backend = details.get("reasoning_tokens") + if not isinstance(backend, int) or backend <= 0: + details["reasoning_tokens"] = self.total + annotated["completion_tokens_details"] = details + return annotated + + def map_finish_reason(raw_reason: str | None) -> FinishReason | None: if raw_reason is None: return None @@ -193,6 +244,7 @@ def _build_reasoning_parser_metadata( chat_template_kwargs: dict[str, Any], request_for_sampling: Any, prompt_token_ids: list[int], + model_config: Any = None, ) -> _ReasoningParserMetadata: if reasoning_parser_class is None: return _ReasoningParserMetadata(None, None, None) @@ -203,9 +255,17 @@ def _build_reasoning_parser_metadata( if getattr(request_for_sampling, "_grammar_from_tool_parser", False): return _ReasoningParserMetadata(True, True, parser_kwargs) + # Same construction as the other two sites. is_reasoning_end() does not read + # model_config, but constructing this one differently is exactly the drift that + # let the missing model_config go unnoticed at the adjust_request() site. + # + # Deliberately NOT added to parser_kwargs: that dict goes on the wire as + # dynamo_preproc["reasoning_parser_kwargs"], and ModelConfig is not + # serializable -- the worker builds its own. reasoning_parser = reasoning_parser_class( tokenizer, chat_template_kwargs=chat_template_kwargs, + model_config=model_config, ) response_reasoning_ended = reasoning_parser.is_reasoning_end(prompt_token_ids) # include_reasoning controls response projection, not whether the model may @@ -542,6 +602,8 @@ async def _generator_inner( tokenizer=self.tokenizer, renderer=self.input_processor.renderer, tool_parser_class=self.tool_parser_class, + reasoning_parser_class=self.reasoning_parser_class, + model_config=self.input_processor.model_config, exclude_tools_when_tool_choice_none=self.exclude_tools_when_tool_choice_none, enable_auto_tool_choice=self.enable_auto_tool_choice, default_chat_template_kwargs=self.default_chat_template_kwargs, @@ -608,9 +670,26 @@ async def _generator_inner( sampling_params.logprobs = top_logprobs # TODO: Support logprobs in the distributed vLLM chat processor by # converting worker log_probs/top_logprobs into EngineCoreOutput.new_logprobs. + # + # Until then, REJECT rather than warn-and-continue. Proceeding emits a + # response whose logprobs structure is empty, which the HTTP layer cannot + # deserialize ("invalid length 0, expected struct ChatChoiceLogprobs with + # 2 elements") -- so the client saw a 500 for a request the server simply + # does not implement, with no usable explanation. + # + # 400, NOT the 501 this API uses for other unimplemented fields + # (`previous_response_id`, `background`, `prompt`). Those are raised inside + # the Rust HTTP service; a status asserted by a Python engine is triaged by + # SanitizedError::for_backend_status (lib/llm/src/http/service/error.rs), + # where only 4xx is forwarded verbatim -- a 5xx has its body replaced with + # the generic internal-error message, so a 501 here reaches the client as an + # opaque 500 and the reason is lost. Verified live: 501 -> "Internal server + # error" with backend_status=501 buried in the log. if sampling_params.logprobs is not None: - logger.warning( - "Logprobs requested but not supported in distributed inference mode" + raise HttpError( + 400, + "Validation: `logprobs` and `top_logprobs` are not supported by the " + "vLLM chat processor (--dyn-chat-processor vllm).", ) with _nvtx.annotate("mm_frontend:process_inputs", color="orange"): @@ -644,6 +723,7 @@ async def _generator_inner( chat_template_kwargs, request_for_sampling, tokens, + self.input_processor.model_config, ) # Convert to a Python object that has fields that match our PreprocessedRequest @@ -745,6 +825,7 @@ def new_post_processor() -> StreamingPostProcessor: tool_parser=choice_tool_parser, reasoning_parser_class=self.reasoning_parser_class, chat_template_kwargs=chat_template_kwargs, + model_config=self.input_processor.model_config, response_reasoning_ended=( reasoning_metadata.response_reasoning_ended ), @@ -838,6 +919,10 @@ async def _generate_and_stream( # content-part counts here too (else frontend metrics report zero media). input_tokens = len(tokens) cumulative_output_tokens = 0 + # Per-request reasoning-token usage (NVBug 6678449b); see + # _ReasoningUsageAnnotator. Must be per-request, never module-level. + # The counts live on the post-processors, which are per-request too. + reasoning_usage = _ReasoningUsageAnnotator(post_processors) _mm_counts, _ = extract_mm_urls(request.get("messages") or []) _mm_counts = _mm_counts or {} image_count = len(_mm_counts.get("image_url", [])) @@ -863,20 +948,24 @@ async def _generate_and_stream( request_id, message, ) - yield make_internal_error(request_id, message) + yield as_error_envelope(make_internal_error(request_id, message)) break engine_response = dynamo_response.data() if engine_response is None: if dynamo_response.is_error(): - yield handle_engine_error(engine_response, request_id, logger) + yield as_error_envelope( + handle_engine_error(engine_response, request_id, logger) + ) break # No data or error fields, means we may have a comment or other kind of event. # I'm not sure what those are used for, so TODO. Skip for now. continue if "token_ids" not in engine_response: - yield handle_engine_error(engine_response, request_id, logger) + yield as_error_envelope( + handle_engine_error(engine_response, request_id, logger) + ) break # Count before any choice gate — tool/reasoning parsers may @@ -887,15 +976,17 @@ async def _generate_and_stream( output_idx = engine_response.get("index", 0) or 0 output_request_id = output_request_ids.get(output_idx) if output_request_id is None: - yield { - "error": { - "message": ( - f"Invalid engine choice index {output_idx} " - f"for request {request_id}" - ), - "type": "internal_error", + yield as_error_envelope( + { + "error": { + "message": ( + f"Invalid engine choice index {output_idx} " + f"for request {request_id}" + ), + "type": "internal_error", + } } - } + ) break raw_finish_reason = engine_response.get("finish_reason") @@ -932,15 +1023,17 @@ async def _generate_and_stream( for output in vllm_out.request_outputs[0].outputs: post = post_processors.get(output.index) if post is None: - yield { - "error": { - "message": ( - f"Invalid postprocessor choice index {output.index} " - f"for request {request_id}" - ), - "type": "internal_error", + yield as_error_envelope( + { + "error": { + "message": ( + f"Invalid postprocessor choice index " + f"{output.index} for request {request_id}" + ), + "type": "internal_error", + } } - } + ) postprocess_error = True break choice = post.process_output(output) @@ -962,7 +1055,7 @@ async def _generate_and_stream( "object": "chat.completion.chunk", } if usage := engine_response.get("completion_usage"): - dynamo_out["usage"] = usage + dynamo_out["usage"] = reasoning_usage.annotate(usage) envelope["data"] = dynamo_out metrics = { @@ -988,8 +1081,20 @@ async def _generate_and_stream( # below is reserved for genuine internal failures. raise except Exception as e: + backend_error = backend_invalid_argument_to_http_error(e) + if backend_error is not None: + # The worker already judged the request invalid and said so with + # its own status. Reporting that as a 500 blames the server for a + # client error and drops the only text explaining the rejection. + logger.warning( + "Backend rejected request %s with %d: %s", + request_id, + backend_error.code, + backend_error.message, + ) + raise backend_error from e logger.exception("Error generating response for request %s", request_id) - yield make_internal_error(request_id, str(e)) + yield as_error_envelope(make_internal_error(request_id, str(e))) finally: for output_request_id in registered_request_ids: if output_request_id in self.output_processor.request_states: diff --git a/components/src/dynamo/vllm/tests/test_vllm_renderer_api.py b/components/src/dynamo/vllm/tests/test_vllm_renderer_api.py index 6542ef4be5d7..357d17cd9811 100755 --- a/components/src/dynamo/vllm/tests/test_vllm_renderer_api.py +++ b/components/src/dynamo/vllm/tests/test_vllm_renderer_api.py @@ -380,10 +380,16 @@ def test_engine_core_struct_contract(self): ) reasoning_request_fields = (*base_request_fields, "reasoning_parser_kwargs") abort_request_fields = (*reasoning_request_fields, "abort_immediately") + # vLLM 0.28.0 appends session_id after abort_immediately. Dynamo reads + # EngineCoreRequest by NAME (vllm_processor.py uses attribute access and + # builds EngineCoreOutput by keyword), so a trailing append is compatible + # with every shape below and needs no production change. + session_request_fields = (*abort_request_fields, "session_id") # vllm-omni monkey-patches EngineCoreRequest with an extra field # (only installed on amd64, not arm64) omni_fields = (*reasoning_request_fields, "additional_information") abort_omni_fields = (*abort_request_fields, "additional_information") + session_omni_fields = (*session_request_fields, "additional_information") valid_request_fields = ( base_request_fields, reasoning_request_fields, @@ -391,6 +397,8 @@ def test_engine_core_struct_contract(self): (*base_request_fields, "additional_information"), omni_fields, abort_omni_fields, + session_request_fields, + session_omni_fields, ) # vLLM 0.26 adds a trailing model_intermediate_buffer field. Dynamo # reads EngineCoreRequest fields by name, so the append is compatible @@ -447,15 +455,22 @@ def test_engine_core_struct_contract(self): "is_segment_finished", "new_prompt_len_snapshot", ) - omni_output_fields = base_output_fields + omni_output_extra_fields - omni_cached_token_output_fields = ( - cached_token_output_fields + omni_output_extra_fields - ) - valid_output_fields = ( - base_output_fields, - cached_token_output_fields, - omni_output_fields, - omni_cached_token_output_fields, + # vLLM 0.28.0 appends two fields after num_nans_in_logits, i.e. after + # vLLM's own fields but before anything vllm-omni adds. Dynamo builds + # EngineCoreOutput BY KEYWORD and feature-detects optional fields via + # __struct_fields__, so appended fields take their msgspec defaults and + # need no output mapping -- validated by ~7000 requests through this exact + # path on vLLM 0.28.0 (p0 + moontrace, 2-replica AGG and 1P1D). + vllm_0280_output_extra_fields = ( + "mm_cache_miss_hashes", + "new_sampling_mask", + ) + _output_bases = (base_output_fields, cached_token_output_fields) + valid_output_fields = tuple( + base + vllm_extra + omni_extra + for base in _output_bases + for vllm_extra in ((), vllm_0280_output_extra_fields) + for omni_extra in ((), omni_output_extra_fields) ) actual_output_fields = EngineCoreOutput.__struct_fields__ assert actual_output_fields in valid_output_fields, ( diff --git a/container/context.yaml b/container/context.yaml index 1d88711f2d46..00a138dcc8b5 100644 --- a/container/context.yaml +++ b/container/context.yaml @@ -71,7 +71,35 @@ vllm: base_image: nvcr.io/nvidia/cuda-dl-base runtime_image: vllm/vllm-openai base_image_tag: 25.11-cuda13.0-devel-ubuntu24.04 - runtime_image_tag: v0.27.1-ubuntu2404 + # Inkling requires 0.28.0. The Inkling NVIDIA path moved substantially after + # 0.27.1, and the MTP support this recipe runs on (MTP 8) is part of it: + # * models/inkling/nvidia/mtp.py -- InklingMTP gains + # SupportsMultiModalEmbeddings, from "Restore multimodal draft capability + # detection" (vllm#50417), which also reworks v1/spec_decode/llm_base_proposer + # * models/inkling/nvidia/ops/fa4_rel_attention.py (+400) and attention.py -- + # FA4 warmup migration (vllm#49315) + # * model.py / moe.py / ops/lamport.py -- shared-expert partial addition fused + # into the Lamport collective (vllm#50697) + # * configs.py -- HF-config compat for Inkling (vllm#51850) + # Three Inkling parser fixes also land in this range (vllm#49876, #50528, #51391). + # Multi-arch manifest list (linux/arm64 + linux/amd64), so GB300 aarch64 is covered. + # xpu/cpu below stay at 0.27.1 -- untested at 0.28.0, and Inkling does not use them. + runtime_image_tag: v0.28.0-ubuntu2404 + # PER-DEVICE, and it must stay that way: vllm-omni asserts matching + # major/minor against the installed vLLM at import, so this is pinned to the + # runtime_image_tag directly above it. A single global ref would install + # 0.28.0rc1 into the xpu/cpu images, which are still on vLLM 0.27.1, and + # reproduce the very mismatch this pins away from. + vllm_omni_ref: "v0.28.0" + # MUST stay <5.15 while vLLM-Omni is pinned to 0.28.0: that release + # declares `transformers>=5.10.1,<5.15`, so 5.15.x makes the image + # unbuildable -- "No solution found when resolving dependencies". + # vllm#49797 (the Gemma 4 heterogeneous-config fix) IS in 0.28.0, which makes + # 5.15 permissible for vLLM itself (it only needs >=5.5.3) but NOT required. + # OMNI is the binding constraint here, not vLLM. Revisit when an omni release + # relaxes the bound. Note the thin dev image that carries 5.15.1 does not + # install omni at all, so it is not evidence that this combination resolves. + transformers_version: "5.14.1" # Baseline is vllm-openai's TRUE base, nvidia/cuda:13.0.2-base-ubuntu24.04 # (Docker Hub), NOT a self-baseline of the third-party vllm/vllm-openai image. # vllm-openai is built FROM nvidia/cuda, so this attributes everything vLLM @@ -85,18 +113,19 @@ vllm: runtime_image: vllm/vllm-openai-xpu base_image_tag: v0.27.1 runtime_image_tag: v0.27.1 + vllm_omni_ref: "v0.27.0rc1" # matches this stanza's vLLM 0.27.1 + # vLLM 0.27.1 here lacks vllm#49797, so it still needs the 5.14.x pin. + transformers_version: "5.14.1" baseline_sbom: vllm-openai-xpu@3309c2e1 cpu: base_image: ubuntu runtime_image: vllm/vllm-openai-cpu base_image_tag: 22.04 runtime_image_tag: v0.27.1 + vllm_omni_ref: "v0.27.0rc1" # matches this stanza's vLLM 0.27.1 + # vLLM 0.27.1 here lacks vllm#49797, so it still needs the 5.14.x pin. + transformers_version: "5.14.1" # baseline_sbom: not yet captured for cpu — runtime build runs without subtraction - vllm_omni_ref: "v0.27.0rc1" - # vLLM 0.27.1 lacks the Transformers 5.15 heterogeneous Gemma 4 config - # support that landed upstream after the release. Remove this pin when the - # vLLM base images include https://github.com/vllm-project/vllm/pull/49797. - transformers_version: "5.14.1" nixl_ref: v1.3.2 max_jobs: "10" enable_media_ffmpeg: "true" diff --git a/container/deps/vllm/patches/v0.28.0/inkling/0001-inkling-structural-tag.patch b/container/deps/vllm/patches/v0.28.0/inkling/0001-inkling-structural-tag.patch new file mode 100644 index 000000000000..af4b76b58ef6 --- /dev/null +++ b/container/deps/vllm/patches/v0.28.0/inkling/0001-inkling-structural-tag.patch @@ -0,0 +1,185 @@ +diff --git a/vllm/tool_parsers/inkling_tool_parser.py b/vllm/tool_parsers/inkling_tool_parser.py +index ab760015af7..517854500bc 100644 +--- a/vllm/tool_parsers/inkling_tool_parser.py ++++ b/vllm/tool_parsers/inkling_tool_parser.py +@@ -5,7 +5,14 @@ from vllm.parser.engine.registered_adapters import InklingParserToolAdapter + + + class InklingEngineToolParser(InklingParserToolAdapter): # type: ignore[valid-type, misc] +- # No Inkling structural-tag grammar is wired up yet; fall back to auto +- # parsing for named/required tool choice. +- structural_tag_model = None +- supports_required_and_named = False ++ # An Inkling structural tag is registered in structural_tag_registry.py, so ++ # named/required tool choice is constrained on the model's real wire format ++ # (<|message_model|><|content_invoke_tool_json|>{...}<|end_message|>) instead ++ # of silently falling back to "auto". ++ # ++ # Do NOT also set supports_required_and_named: AbstractToolParser's ++ # __init_subclass__ forces it to False whenever structural_tag_model is set ++ # and VLLM_ENFORCE_STRICT_TOOL_CALLING is on. That is the intended design -- ++ # the structural tag supersedes the generic required/named JSON path -- so ++ # setting it here is both redundant and silently overwritten. ++ structural_tag_model = "inkling" +diff --git a/vllm/tool_parsers/structural_tag_registry.py b/vllm/tool_parsers/structural_tag_registry.py +index 1f9a651ee46..bc70a07c249 100644 +--- a/vllm/tool_parsers/structural_tag_registry.py ++++ b/vllm/tool_parsers/structural_tag_registry.py +@@ -70,7 +70,7 @@ XGRAMMAR_BUILTIN_STRUCTURAL_TAG_MODELS = frozenset( + "deepseek_v4", + } + ) +-VLLM_BUILTIN_STRUCTURAL_TAG_MODELS = frozenset({"hermes", "kimi_k3"}) ++VLLM_BUILTIN_STRUCTURAL_TAG_MODELS = frozenset({"hermes", "inkling", "kimi_k3"}) + SUPPORTED_STRUCTURAL_TAG_MODELS = ( + XGRAMMAR_BUILTIN_STRUCTURAL_TAG_MODELS | VLLM_BUILTIN_STRUCTURAL_TAG_MODELS + ) +@@ -376,6 +376,7 @@ _K3_ARG_CLOSE = f"{_K3_CLOSE}argument{_K3_SEP}" + # the end-of-message token. It is generated (not part of the prompt prefix), so + # the tag must permit it or the FSM would mask the model's natural terminator. + _K3_MESSAGE_CLOSE = f"{_K3_CLOSE}message{_K3_SEP}" ++_K3_END_OF_MSG = "<|end_of_msg|>" + + # JSON-schema type -> K3 XTML ``type=`` attribute value. Mirrors + # ``encoding_k3._xtml_type`` (integer collapses onto number). +@@ -556,12 +557,17 @@ def _k3_response_prefix() -> list[Any]: + + ``response`` is generated in thinking mode (prefix ends at + ``<|open|>think<|sep|>``) but is part of the generation prefix in +- non-thinking mode, so its open marker is optional. The body is bounded by +- the response close marker. ++ non-thinking mode, so its open marker is optional. Excluding the reserved ++ marker heads commits ``<|close|>`` to the response-close branch immediately ++ and prevents a nested channel from starting inside response text. + """ + return [ + OptionalFormat(content=ConstStringFormat(value=_K3_RESPONSE_OPEN)), +- TagFormat(begin="", content=AnyTextFormat(), end=_K3_RESPONSE_CLOSE), ++ TagFormat( ++ begin="", ++ content=AnyTextFormat(excludes=[_K3_OPEN, _K3_CLOSE, _K3_END_OF_MSG]), ++ end=_K3_RESPONSE_CLOSE, ++ ), + ] + + +@@ -577,6 +583,119 @@ def _k3_tools_channel(tools: list[FunctionToolParam]) -> TagFormat: + ) + + ++# --- Inkling ------------------------------------------------------------------- ++# Wire format (vllm/parser/inkling.py): ++# tool call: <|message_model|><|content_invoke_tool_json|> ++# {"name": "get_weather", "args": {"city": "SF"}}<|end_message|> ++# text: <|message_model|><|content_text|>...<|end_message|> ++# ++# Inkling wraps its tool JSON in content markers, which is exactly why a BARE ++# JSON-schema constraint is wrong for it -- Dynamo's prepost.py gates that ++# fallback off for this parser because forcing raw JSON conflicts with the wire ++# format and can crash EngineCore under speculative decoding. The structural tag ++# constrains markers AND payload together, which is the whole point. ++# ++# The tag constrains the GENERATED SUFFIX, so it begins at the content-kind ++# marker, NOT at `<|message_model|>`. Inkling's chat template prefills the role ++# marker into the generation prompt -- it ends ++# `...<|end_message|><|message_model|>` -- so generation resumes inside the ++# model message and never re-emits it. Anchoring the tag at `<|message_model|>` ++# forces a DUPLICATE role marker: verified with xgrammar, that grammar rejects ++# the correct suffix and accepts only the doubled form. It appears to work ++# because the parser absorbs the duplicate, but it manufactures exactly the ++# unconsumed-control-marker residue that dynamo#12510 exists to strip. ++_INKLING_CONTENT_TEXT = "<|content_text|>" ++_INKLING_TOOL_JSON = "<|content_invoke_tool_json|>" ++_INKLING_END_MESSAGE = "<|end_message|>" ++_INKLING_END_SAMPLING = "<|content_model_end_sampling|>" ++_INKLING_TOOL_OPEN = _INKLING_TOOL_JSON ++_INKLING_TEXT_OPEN = _INKLING_CONTENT_TEXT ++# BOTH terminators, because the parser treats them as co-equal: it documents ++# that "sampling may also end a block with the standalone ++# <|content_model_end_sampling|> token", `is_reasoning_end` returns True on it, ++# and `count_reasoning_tokens` closes a reasoning block on either. Permitting ++# only <|end_message|> would mask a valid natural terminator -- and would remove ++# one of the two paths that flip is_reasoning_end. ++_INKLING_ENDS = [_INKLING_END_MESSAGE, _INKLING_END_SAMPLING] ++ ++ ++def _inkling_call_schema(tool: FunctionToolParam) -> dict[str, Any]: ++ """`{"name": "", "args": {...}}` -- name pinned to ONE tool.""" ++ parameters = get_function_parameters(tool.function) ++ args_schema: Any = ( ++ parameters if isinstance(parameters, dict) else {"type": "object"} ++ ) ++ return { ++ "type": "object", ++ "properties": { ++ "name": {"const": tool.function.name}, ++ "args": args_schema, ++ }, ++ "required": ["name", "args"], ++ "additionalProperties": False, ++ } ++ ++ ++def _inkling_call_tag(tool: FunctionToolParam) -> TagFormat: ++ return TagFormat( ++ begin=_INKLING_TOOL_OPEN, ++ content=JSONSchemaFormat(json_schema=_inkling_call_schema(tool)), ++ end=_INKLING_ENDS, ++ ) ++ ++ ++def _inkling_tool_calls(tools: list[FunctionToolParam]) -> Any: ++ """One or more tool-call blocks, in any order, at least one.""" ++ return TagsWithSeparatorFormat( ++ tags=[_inkling_call_tag(tool) for tool in tools], ++ separator="", ++ at_least_one=True, ++ ) ++ ++ ++def _inkling_text_block() -> TagFormat: ++ return TagFormat( ++ begin=_INKLING_TEXT_OPEN, ++ content=AnyTextFormat(excludes=_INKLING_ENDS), ++ end=_INKLING_ENDS, ++ ) ++ ++ ++@register_vllm_structural_tag("inkling") ++def get_inkling_structural_tag( ++ tools: list[FunctionToolParam], ++ builtin_tools: list[BuiltinToolParam], ++ tool_choice: SimplifiedToolChoice, ++ reasoning: bool, ++) -> StructuralTag: ++ # `reasoning` is deliberately unused: an Inkling thinking block is a separate ++ # <|content_thinking|> message that precedes this one, and constraining it here ++ # would forbid the model from thinking before it calls a tool. ++ del builtin_tools, reasoning ++ ++ if not tools: ++ return StructuralTag(format=_inkling_text_block()) ++ ++ if tool_choice == "auto": ++ # The model may answer in text INSTEAD of calling a tool. ++ return StructuralTag( ++ format=OrFormat( ++ elements=[_inkling_tool_calls(tools), _inkling_text_block()] ++ ) ++ ) ++ if tool_choice == "forced": ++ # A named tool_choice. normalize_tool_choice() has ALREADY filtered ++ # `tools` to the named one, so tools[:1] IS that tool. ++ # Use the same CONTAINER format as the required branch, never a bare ++ # call tag: a bare TagFormat at the root is a TRIGGERED constraint ++ # (xgrammar only enforces after `begin` matches), so it never anchors ++ # and the model stays free to call a different tool. kimi_k3 wraps ++ # forced the same way, for the same reason. ++ return StructuralTag(format=_inkling_tool_calls(tools[:1])) ++ # required: at least one call from the offered set, no text alternative. ++ return StructuralTag(format=_inkling_tool_calls(tools)) ++ ++ + @register_vllm_structural_tag("kimi_k3") + def get_kimi_k3_structural_tag( + tools: list[FunctionToolParam], diff --git a/container/deps/vllm/validate_inkling_runtime.py b/container/deps/vllm/validate_inkling_runtime.py new file mode 100644 index 000000000000..f03f6219dc72 --- /dev/null +++ b/container/deps/vllm/validate_inkling_runtime.py @@ -0,0 +1,91 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Validate the Inkling vLLM runtime markers baked into this image. + +Asserts the POSTCONDITIONS of +`container/deps/vllm/patches/v0.28.0/inkling/*.patch`, not merely that `patch` +exited zero. The failure this guards against is silent: without a registered +structural tag, Dynamo installs no constraint for a tool request and degrades to +`tool_choice="auto"` with HTTP 200 and no warning, so a forced tool choice is +simply not honoured. +""" + +from __future__ import annotations + +import importlib.metadata as metadata + +from vllm.tool_parsers import ToolParserManager +from vllm.tool_parsers.structural_tag_registry import ( + _VLLM_STRUCTURAL_TAG_REGISTRY, + VLLM_BUILTIN_STRUCTURAL_TAG_MODELS, +) + +EXPECTED_VLLM_VERSION = "0.28.0" + + +def _check_version() -> list[str]: + installed = metadata.version("vllm") + if installed != EXPECTED_VLLM_VERSION: + return [ + f"vllm {installed!r} != expected {EXPECTED_VLLM_VERSION!r}; the Inkling " + "patches are cut against that exact base" + ] + return [] + + +def _check_structural_tag() -> list[str]: + errors: list[str] = [] + if "inkling" not in VLLM_BUILTIN_STRUCTURAL_TAG_MODELS: + errors.append( + "'inkling' missing from VLLM_BUILTIN_STRUCTURAL_TAG_MODELS -- the " + "structural-tag patch did not apply" + ) + if "inkling" not in _VLLM_STRUCTURAL_TAG_REGISTRY: + errors.append( + "no structural-tag builder registered for 'inkling' in " + "_VLLM_STRUCTURAL_TAG_REGISTRY" + ) + return errors + + +def _check_tool_parser() -> list[str]: + """The parser must ADVERTISE the tag, and must NOT hand-set the strict flag. + + `AbstractToolParser.__init_subclass__` forces `supports_required_and_named` + to False whenever `structural_tag_model` is set and + VLLM_ENFORCE_STRICT_TOOL_CALLING is on (the default). Setting it in the + subclass is therefore both redundant and silently overwritten; asserting the + derived value here is what keeps that from being reintroduced. + """ + import vllm.envs as envs + + errors: list[str] = [] + parser = ToolParserManager.get_tool_parser("inkling") + if getattr(parser, "structural_tag_model", None) != "inkling": + errors.append( + f"InklingToolParser.structural_tag_model = " + f"{getattr(parser, 'structural_tag_model', None)!r}, expected 'inkling'" + ) + expected = not envs.VLLM_ENFORCE_STRICT_TOOL_CALLING + actual = parser.supports_required_and_named + if actual is not expected: + errors.append( + f"InklingToolParser.supports_required_and_named = {actual!r}, expected " + f"{expected!r} (derived from VLLM_ENFORCE_STRICT_TOOL_CALLING)" + ) + return errors + + +def main() -> int: + errors = _check_version() + _check_structural_tag() + _check_tool_parser() + if errors: + print("Inkling runtime validation FAILED:") + for err in errors: + print(f" - {err}") + return 1 + print(f"Inkling runtime validation passed (vllm {EXPECTED_VLLM_VERSION}).") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/container/templates/args.Dockerfile b/container/templates/args.Dockerfile index 046f4d86e566..5b1d4680e8d5 100644 --- a/container/templates/args.Dockerfile +++ b/container/templates/args.Dockerfile @@ -98,8 +98,8 @@ ARG PLANNER_RUNTIME_IMAGE_TAG={{ context.dynamo.planner_runtime_image_tag }} {% if framework == "vllm" -%} ARG MAX_JOBS={{ context.vllm.max_jobs }} -ARG TRANSFORMERS_VERSION={{ context.vllm.transformers_version }} -ARG VLLM_OMNI_REF={{ context.vllm.vllm_omni_ref }} +ARG TRANSFORMERS_VERSION={{ context[framework][device_key].transformers_version }} +ARG VLLM_OMNI_REF={{ context[framework][device_key].vllm_omni_ref }} {% if device == "cuda" -%} # If left blank, then we will fallback to vLLM defaults diff --git a/container/templates/vllm_runtime.Dockerfile b/container/templates/vllm_runtime.Dockerfile index be1ca486ed44..1cc90dc282dd 100644 --- a/container/templates/vllm_runtime.Dockerfile +++ b/container/templates/vllm_runtime.Dockerfile @@ -262,6 +262,33 @@ RUN --mount=type=bind,source=./container/deps/vllm/protected_packages.txt,target export VLLM_OMNI_TARGET_DEVICE={{ device }}; \ bash /tmp/install_vllm_omni.sh +{% if device == "cuda" %} +# Inkling: vLLM ships an Inkling tool parser but registers no structural tag for +# it, so Dynamo finds no grammar to install for a tool request and silently +# degrades to tool_choice="auto" -- HTTP 200, no warning, forced tool choice not +# honoured. Layer the structural-tag patch here and assert its postconditions. +# +# The version test is load-bearing: these patches are cut against the exact +# v0.28.0 tree, and `patch --forward` would otherwise skip or fuzz a hunk on a +# different base and leave a half-patched image that still builds. +RUN --mount=type=bind,source=./container/deps/vllm/patches/v0.28.0/inkling,target=/tmp/inkling-vllm-patches,readonly \ + --mount=type=bind,source=./container/deps/vllm/validate_inkling_runtime.py,target=/tmp/validate_inkling_runtime.py,readonly \ + set -eux; \ + installed_vllm_version="$(python3 -c 'import importlib.metadata as md; print(md.version("vllm"))')"; \ + test "${installed_vllm_version}" = "0.28.0"; \ + apt-get update; \ + DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends patch; \ + site_parent="$(python3 -c 'import pathlib, vllm; print(pathlib.Path(vllm.__file__).resolve().parent.parent)')"; \ + for patch_file in /tmp/inkling-vllm-patches/*.patch; do \ + echo "Applying ${patch_file}"; \ + patch --batch --forward -p1 -d "${site_parent}" < "${patch_file}"; \ + done; \ + apt-get purge -y patch; \ + rm -rf /var/lib/apt/lists/*; \ + python3 /tmp/validate_inkling_runtime.py + +{% endif %} + {% if device == "xpu" %} # Remove conflicting standard triton package for XPU and reinstall triton-xpu # This must be done after vLLM-Omni installation to ensure no dependencies re-install triton diff --git a/lib/llm/src/kv_router/prefill_router/admission.rs b/lib/llm/src/kv_router/prefill_router/admission.rs index d1e39a9d9e61..458312fb6ed3 100644 --- a/lib/llm/src/kv_router/prefill_router/admission.rs +++ b/lib/llm/src/kv_router/prefill_router/admission.rs @@ -41,10 +41,10 @@ where }; if let Some(error) = first_output.err() { - return Err(PrefillError::PrefillError( - "Prefill router returned error in output".to_string(), - Some(Box::new(error)), - )); + // Include the worker's text. `to_pyerr` keeps only `Display`, so a + // `#[source]` is lost. See `PrefillError::PrefillError`. + let detail = format!("Prefill router returned error in output: {error}"); + return Err(PrefillError::PrefillError(detail, Some(Box::new(error)))); } if let Some(ref tracker) = tracker { @@ -72,10 +72,8 @@ where if !is_bootstrap { while let Some(next) = prefill_response.next().await { if let Some(error) = next.err() { - return Err(PrefillError::PrefillError( - "Prefill router returned error in output stream".to_string(), - Some(Box::new(error)), - )); + let detail = format!("Prefill router returned error in output stream: {error}"); + return Err(PrefillError::PrefillError(detail, Some(Box::new(error)))); } if let Some(output) = next.data.as_ref() && prompt_tokens_details.is_none() diff --git a/lib/llm/src/kv_router/prefill_router/mod.rs b/lib/llm/src/kv_router/prefill_router/mod.rs index b82467a8c3ef..c3c94596b6d3 100644 --- a/lib/llm/src/kv_router/prefill_router/mod.rs +++ b/lib/llm/src/kv_router/prefill_router/mod.rs @@ -79,6 +79,9 @@ pub enum PrefillError { #[error("Prefill router not yet activated")] NotActivated, + // Callers must include the worker's error text in this message. The + // frontend receives this error through `to_pyerr`, which keeps only + // `Display` and drops the source chain, so a `#[source]` is never seen. #[error("Prefill execution failed: {0}")] PrefillError( String, diff --git a/tests/frontend/test_prepost.py b/tests/frontend/test_prepost.py index c6fb1e8f75fc..af59ab8a2d0b 100644 --- a/tests/frontend/test_prepost.py +++ b/tests/frontend/test_prepost.py @@ -29,7 +29,7 @@ from vllm.sampling_params import SamplingParams from vllm.tool_parsers.hermes_tool_parser import Hermes2ProToolParser - from dynamo.frontend.prepost import StreamingPostProcessor + from dynamo.frontend.prepost import StreamingPostProcessor, _prepare_request else: # Fake some types so that `pre-commit` passes class CompletionOutput: @@ -2076,3 +2076,122 @@ def test_streaming_parallel_tool_calls_no_think( assert finish_reasons == [ "tool_calls" ], f"Expected finish_reason=['tool_calls']; got {finish_reasons}" + + +# --------------------------------------------------------------------------- +# Reasoning-parser adjust_request wiring +# --------------------------------------------------------------------------- +@pytest.fixture +def recording_reasoning_parser(): + """A reasoning-parser class plus the per-test list of instances it created. + + Returns ``(cls, instances)``. The list is owned by the fixture rather than + living on the class, so each test starts clean without depending on a shared + registry being cleared (see .ai/pytest-guidelines.md, "Hermetic Testing"). + """ + instances = [] + + class _RecordingReasoningParser: + """Stands in for a parser whose adjust_request mutates the request. + + Real example: a parser that splits thinking from the answer on text markers + needs skip_special_tokens=False so those markers survive detokenisation. + """ + + def __init__(self, tokenizer, chat_template_kwargs=None, model_config=None): + self.tokenizer = tokenizer + self.chat_template_kwargs = chat_template_kwargs + self.called = False + instances.append(self) + + def adjust_request(self, request): + self.called = True + request.skip_special_tokens = False + return request + + return _RecordingReasoningParser, instances + + +def _prep(tokenizer, *, reasoning_parser_class=None, chat_template_kwargs=None): + request = ChatCompletionRequest.model_construct( + messages=[{"role": "user", "content": "hi"}], + model="test", + tools=None, + tool_choice=None, + chat_template=None, + chat_template_kwargs=chat_template_kwargs, + add_generation_prompt=True, + continue_final_message=False, + documents=None, + reasoning_effort=None, + skip_special_tokens=True, + ) + return _prepare_request( + request, + tokenizer=tokenizer, + tool_parser_class=None, + reasoning_parser_class=reasoning_parser_class, + ) + + +class TestReasoningParserAdjustRequest: + """The reasoning parser's adjust_request must run during request preparation. + + Without it the reasoning channel only works when the *client* sends + skip_special_tokens=false, which ordinary OpenAI-compatible clients do not. + """ + + def test_adjust_request_is_called(self, tokenizer, recording_reasoning_parser): + parser_cls, instances = recording_reasoning_parser + req, _, _, _, _ = _prep(tokenizer, reasoning_parser_class=parser_cls) + assert len(instances) == 1 + assert instances[0].called + assert req.skip_special_tokens is False + + def test_not_called_when_thinking_disabled( + self, tokenizer, recording_reasoning_parser + ): + """Gating must match StreamingPostProcessor: no parser => no adjustment. + + If the sampling params were adjusted for a parser that then does not exist, + the model's control markers reach the client as visible text. + """ + parser_cls, instances = recording_reasoning_parser + req, _, _, _, _ = _prep( + tokenizer, + reasoning_parser_class=parser_cls, + chat_template_kwargs={"enable_thinking": False}, + ) + assert instances == [] + assert req.skip_special_tokens is True + + def test_enable_thinking_true_still_calls( + self, tokenizer, recording_reasoning_parser + ): + parser_cls, instances = recording_reasoning_parser + req, _, _, _, _ = _prep( + tokenizer, + reasoning_parser_class=parser_cls, + chat_template_kwargs={"enable_thinking": True}, + ) + assert instances[0].called + assert req.skip_special_tokens is False + + def test_no_parser_class_leaves_request_untouched(self, tokenizer): + req, _, _, _, _ = _prep(tokenizer, reasoning_parser_class=None) + assert req.skip_special_tokens is True + + def test_default_adjust_request_is_a_no_op( + self, tokenizer, recording_reasoning_parser + ): + """ReasoningParser.adjust_request returns the request unchanged by default, + so parsers that do not need the override are unaffected.""" + parser_cls, _ = recording_reasoning_parser + + class _PassThrough(parser_cls): + def adjust_request(self, request): + self.called = True + return request + + req, _, _, _, _ = _prep(tokenizer, reasoning_parser_class=_PassThrough) + assert req.skip_special_tokens is True diff --git a/tests/frontend/test_realtime_omni_bridge.py b/tests/frontend/test_realtime_omni_bridge.py index 51cc999c4943..1b67583f8b87 100644 --- a/tests/frontend/test_realtime_omni_bridge.py +++ b/tests/frontend/test_realtime_omni_bridge.py @@ -31,6 +31,21 @@ from tests.utils.managed_process import DynamoFrontendProcess, ManagedProcess from tests.utils.port_utils import ServicePorts +# realtime_omni_mock_worker.py imports vllm_omni and dynamo.vllm.omni at module +# scope, so if those are unavailable the worker dies at startup and this test +# fails as "Main server process exited with code 1" -- an opaque symptom for a +# plain missing/broken dependency. Probe the same imports here and skip instead. +# RuntimeError is included because diffusers re-raises transitive import +# failures as RuntimeError rather than ImportError. +try: + import vllm_omni.outputs.mm_outputs # noqa: F401 + + from dynamo.vllm.omni.realtime_handler import RealtimeOmniHandler # noqa: F401 + + _OMNI_AVAILABLE = True +except (ImportError, OSError, NotImplementedError, RuntimeError): + _OMNI_AVAILABLE = False + logger = logging.getLogger(__name__) # Shared with the worker module (realtime_omni_mock_worker.py imports these). @@ -44,6 +59,12 @@ pytest.mark.vllm, pytest.mark.multimodal, pytest.mark.gpu_0, + # skipif rather than a module-level skip, so the tests stay COLLECTED and are + # reported as skipped. A module-level skip can empty a marker selection, and + # pytest then exits 5 ("no tests ran"), which CI reports as a failure. + pytest.mark.skipif( + not _OMNI_AVAILABLE, reason="vLLM omni dependencies not available" + ), ] diff --git a/tests/serve/test_vllm_omni.py b/tests/serve/test_vllm_omni.py index 284f47a5277d..2c9c33251284 100644 --- a/tests/serve/test_vllm_omni.py +++ b/tests/serve/test_vllm_omni.py @@ -10,8 +10,16 @@ try: from dynamo.vllm.omni.args import OmniConfig # noqa: F401 -except (ImportError, OSError, NotImplementedError): - pytest.skip("vLLM omni dependencies not available", allow_module_level=True) + + _OMNI_AVAILABLE = True +except (ImportError, OSError, NotImplementedError, RuntimeError): + # RuntimeError matters: diffusers imports its pipelines lazily and re-raises + # ANY underlying failure as RuntimeError("Failed to import ... because of the + # following error"), so a broken transitive dependency never surfaces as + # ImportError here. Without it this module ERRORS instead of skipping -- + # which is what happened once vllm-omni became installable and its diffusers + # pin moved ahead of the base image's huggingface_hub. + _OMNI_AVAILABLE = False from tests.serve.common import ( WORKSPACE_DIR, @@ -29,6 +37,17 @@ logger = logging.getLogger(__name__) +# skipif, NOT `pytest.skip(allow_module_level=True)`. A module-level skip removes +# this file from COLLECTION entirely, so a marker selection whose only match lives +# here collects nothing and pytest exits 5 ("no tests ran") -- which CI reports as a +# job failure, not as a skip. The CI step `-m "(pre_merge and vllm and gpu_1) and not +# profiled_vram_gib"` is exactly that case: its sole match is the omni_audio +# parametrization below. Skipping at setup keeps the items collected and reported as +# skipped, which is what the runner expects. +pytestmark = pytest.mark.skipif( + not _OMNI_AVAILABLE, reason="vLLM omni dependencies not available" +) + vllm_dir = os.environ.get("VLLM_DIR") or os.path.join( WORKSPACE_DIR, "examples/backends/vllm" )