From 9765fe438398d405d5bb40e1f68fd65d5d560830 Mon Sep 17 00:00:00 2001 From: Yifan Jiang <19356972+yifjiang@users.noreply.github.com> Date: Sat, 1 Aug 2026 23:03:33 -0700 Subject: [PATCH 01/22] fix(frontend): call the reasoning parser's adjust_request during preprocessing vLLM reasoning parsers expose adjust_request() so they can declare the sampling params they need. Parsers that split thinking from the answer on text markers require `skip_special_tokens=False`, otherwise the markers are stripped before the parser ever sees them and the split silently does not happen. `_prepare_request` calls the TOOL parser's adjust_request but there is no equivalent for the reasoning parser -- it does not even receive `reasoning_parser_class`, since the parser is constructed later, in StreamingPostProcessor, long after the sampling request is finalised. Net effect: `reasoning_content` comes back empty unless the CLIENT sends `skip_special_tokens: false`. Ordinary OpenAI-compatible clients do not, so on such models the reasoning channel silently does nothing for normal traffic. The tool path is additionally gated on `tool_choice != "none"`, so a plain tools-free chat request gets no adjust_request call from either parser. Mirror the tool-parser path: construct the reasoning parser during request preparation and let it adjust the request. The gating matters and is deliberately identical to StreamingPostProcessor's own parser construction (`reasoning_parser_class and not thinking_disabled`, where `thinking_disabled = chat_template_kwargs.get("enable_thinking") is False`). If the sampling params were adjusted for a parser that then ends up None, the model's control markers would reach the client as visible text. The call is therefore placed after chat_template_kwargs is resolved. This is a no-op for parsers that do not need it: the base ReasoningParser.adjust_request is `return request`. Two files are required. prepost.py alone is inert -- vllm_processor.py must pass reasoning_parser_class into preprocess_chat_request, otherwise the new parameter defaults to None and the gate never fires. Adds unit coverage for the wiring and, importantly, for the gate: the parser must NOT be constructed or consulted when enable_thinking is False. Signed-off-by: Yifan Jiang <19356972+yifjiang@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) --- components/src/dynamo/frontend/prepost.py | 23 ++++ .../src/dynamo/frontend/vllm_processor.py | 1 + tests/frontend/test_prepost.py | 104 +++++++++++++++++- 3 files changed, 127 insertions(+), 1 deletion(-) diff --git a/components/src/dynamo/frontend/prepost.py b/components/src/dynamo/frontend/prepost.py index 512450a987be..e3c866fe2482 100644 --- a/components/src/dynamo/frontend/prepost.py +++ b/components/src/dynamo/frontend/prepost.py @@ -419,6 +419,7 @@ def _prepare_request( *, tokenizer: TokenizerLike, tool_parser_class: type[ToolParser] | None, + reasoning_parser_class: type[ReasoningParser] | 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, @@ -495,6 +496,26 @@ 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_class is not None and ( + chat_template_kwargs.get("enable_thinking") is not False + ): + request_for_sampling = reasoning_parser_class( + tokenizer, + chat_template_kwargs=chat_template_kwargs, + ).adjust_request(request_for_sampling) + # Mistral warns that tokenize=False is unsafe for chat templates. is_mistral_tokenizer = ( tokenizer.__class__.__name__ == "MistralTokenizer" @@ -536,6 +557,7 @@ async def preprocess_chat_request( tokenizer: TokenizerLike, renderer: _Renderer, tool_parser_class: type[ToolParser] | None, + reasoning_parser_class: type[ReasoningParser] | 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, @@ -567,6 +589,7 @@ async def preprocess_chat_request( request, tokenizer=tokenizer, tool_parser_class=tool_parser_class, + reasoning_parser_class=reasoning_parser_class, 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, diff --git a/components/src/dynamo/frontend/vllm_processor.py b/components/src/dynamo/frontend/vllm_processor.py index cb5ded3fe439..ac4c31bfd9a4 100644 --- a/components/src/dynamo/frontend/vllm_processor.py +++ b/components/src/dynamo/frontend/vllm_processor.py @@ -542,6 +542,7 @@ 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, 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, diff --git a/tests/frontend/test_prepost.py b/tests/frontend/test_prepost.py index c6fb1e8f75fc..c8046036e22c 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,105 @@ 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 +# --------------------------------------------------------------------------- +class _RecordingReasoningParser: + """Stand-in for a reasoning 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. + """ + + instances: list["_RecordingReasoningParser"] = [] + + def __init__(self, tokenizer, chat_template_kwargs=None): + self.tokenizer = tokenizer + self.chat_template_kwargs = chat_template_kwargs + self.called = False + type(self).instances.append(self) + + def adjust_request(self, request): + self.called = True + request.skip_special_tokens = False + return request + + +def _prep(tokenizer, *, reasoning_parser_class=None, chat_template_kwargs=None): + _RecordingReasoningParser.instances.clear() + 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): + req, _, _, _, _ = _prep( + tokenizer, reasoning_parser_class=_RecordingReasoningParser + ) + assert len(_RecordingReasoningParser.instances) == 1 + assert _RecordingReasoningParser.instances[0].called + assert req.skip_special_tokens is False + + def test_not_called_when_thinking_disabled(self, tokenizer): + """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. + """ + req, _, _, _, _ = _prep( + tokenizer, + reasoning_parser_class=_RecordingReasoningParser, + chat_template_kwargs={"enable_thinking": False}, + ) + assert _RecordingReasoningParser.instances == [] + assert req.skip_special_tokens is True + + def test_enable_thinking_true_still_calls(self, tokenizer): + req, _, _, _, _ = _prep( + tokenizer, + reasoning_parser_class=_RecordingReasoningParser, + chat_template_kwargs={"enable_thinking": True}, + ) + assert _RecordingReasoningParser.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): + """ReasoningParser.adjust_request returns the request unchanged by default, + so parsers that do not need the override are unaffected.""" + + class _PassThrough(_RecordingReasoningParser): + def adjust_request(self, request): + self.called = True + return request + + req, _, _, _, _ = _prep(tokenizer, reasoning_parser_class=_PassThrough) + assert req.skip_special_tokens is True From 943288939e9928e8cee844dfc391dd22b7482b61 Mon Sep 17 00:00:00 2001 From: Yifan Jiang <19356972+yifjiang@users.noreply.github.com> Date: Tue, 25 Aug 2026 17:46:58 -0700 Subject: [PATCH 02/22] fix(frontend): forward the reasoning parser's adjusted guidance Addresses review feedback on #12541: adjust_request() can change more than skip_special_tokens. A reasoning parser that rewrites the caller's constraint -- converting response_format into the structural tag its model actually speaks -- had that rewrite dropped. parser_guided_decoding was gated on `tool_parser is not None`, and build_tool_call_guided_decoding returns early when the request has no tools, so the rewrite never reached guided_decoding. The request carried the structural tag while the engine was handed the pre-adjustment JSON schema captured before _prepare_request ran. Recompute the parser guidance when either parser is active, and let a rewrite that only the reasoning parser can have made (no tool parser in the request, so it is attributable) take precedence over the caller's pre-adjustment copy. Scoped deliberately: when a tool parser is also active the change is not attributable to either parser, and parser-vs-assistant precedence is already settled there -- the caller's constraint wins, pinned by test_assistant_guidance_takes_precedence_over_auto_tool_guidance and the response-format-precedence row of TOOL_GUIDANCE_PARITY_CASES. Revisiting that policy is a separate change. Also single-source the reasoning-parser gate as _reasoning_parser_enabled(), a TypeGuard now shared by _prepare_request, preprocess_chat_request and StreamingPostProcessor. The three must agree: 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. Tests: four cases in TestReasoningParserGuidanceForwarding covering the rewrite with no tool parser, a pass-through parser, no parser, and the thinking-disabled gate. The first fails without this change with the reported symptom ({"json": {"type": "object"}} instead of the structural tag). The reasoning-parser stubs move to a per-test fixture so no mutable registry is shared at module scope, per .ai/pytest-guidelines.md (CodeRabbit RUF012, Devin). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01LqtTc1QsXejXiEdpmLZ4Wz --- components/src/dynamo/frontend/prepost.py | 70 ++++++++++-- .../tests/test_vllm_processor_unit.py | 105 ++++++++++++++++++ tests/frontend/test_prepost.py | 75 ++++++++----- 3 files changed, 209 insertions(+), 41 deletions(-) diff --git a/components/src/dynamo/frontend/prepost.py b/components/src/dynamo/frontend/prepost.py index e3c866fe2482..222801a99da0 100644 --- a/components/src/dynamo/frontend/prepost.py +++ b/components/src/dynamo/frontend/prepost.py @@ -9,7 +9,7 @@ from concurrent.futures import ThreadPoolExecutor from copy import deepcopy from dataclasses import dataclass -from typing import Any, Protocol, cast +from typing import Any, Protocol, TypeGuard, cast from vllm.entrypoints.chat_utils import make_tool_call_id from vllm.entrypoints.openai.chat_completion.protocol import ( @@ -414,6 +414,28 @@ 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, *, @@ -508,9 +530,7 @@ def _prepare_request( # # ReasoningParser.adjust_request is `return request` by default, so this is a no-op # for parsers that do not need it. - if reasoning_parser_class is not None and ( - chat_template_kwargs.get("enable_thinking") is not False - ): + if _reasoning_parser_enabled(reasoning_parser_class, chat_template_kwargs): request_for_sampling = reasoning_parser_class( tokenizer, chat_template_kwargs=chat_template_kwargs, @@ -600,9 +620,18 @@ async def preprocess_chat_request( 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 ) @@ -631,11 +660,30 @@ 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 + # A rewrite that only the reasoning parser can have made. With no tool parser + # there is no other adjust_request in this request, so the change is + # attributable, and tool_guided_decoding cannot carry it out either: + # build_tool_call_guided_decoding returns early when the request has no tools. + # Without this the request ends up holding the parser's structural tag while + # the engine is handed the pre-adjustment copy captured before + # _prepare_request ran. + # + # Scoped to that case on purpose. When a tool parser is also active the change + # is not attributable to either parser, and parser-vs-assistant precedence is + # already settled there -- the caller's explicit constraint wins, pinned by + # test_assistant_guidance_takes_precedence_over_auto_tool_guidance and the + # response-format-precedence row of TOOL_GUIDANCE_PARITY_CASES. Revisiting that + # policy is a separate change. + reasoning_only_guided_decoding = ( + parser_guided_decoding if tool_parser is None else None ) + guided_decoding: dict[str, Any] | None + if reasoning_only_guided_decoding is not None and not is_forced_tool_choice: + guided_decoding = reasoning_only_guided_decoding + 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 @@ -704,14 +752,12 @@ 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, ) - 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 ) 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..50043ddbc986 100644 --- a/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py +++ b/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py @@ -1708,6 +1708,111 @@ 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): + del tokenizer, chat_template_kwargs + + 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 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): + return await prepost_module.preprocess_chat_request( + request if request is not None else self._request(), + tokenizer=tokenizer, + renderer=self._renderer(), + tool_parser_class=None, + **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_no_reasoning_parser_keeps_the_caller_constraint(self, tokenizer): + result = await self._preprocess(tokenizer, reasoning_parser_class=None) + + 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 + + class TestToolCallGuidedDecoding: def _request(self, tokenizer, **overrides): raw_request = {**TOOL_REQUEST, "tool_choice": "auto", **overrides} diff --git a/tests/frontend/test_prepost.py b/tests/frontend/test_prepost.py index c8046036e22c..c618cbb7ca68 100644 --- a/tests/frontend/test_prepost.py +++ b/tests/frontend/test_prepost.py @@ -2081,29 +2081,38 @@ def test_streaming_parallel_tool_calls_no_think( # --------------------------------------------------------------------------- # Reasoning-parser adjust_request wiring # --------------------------------------------------------------------------- -class _RecordingReasoningParser: - """Stand-in for a reasoning parser whose adjust_request mutates the request. +@pytest.fixture +def recording_reasoning_parser(): + """A reasoning-parser class plus the per-test list of instances it created. - Real example: a parser that splits thinking from the answer on text markers needs - skip_special_tokens=False so those markers survive detokenisation. + 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. + """ - instances: list["_RecordingReasoningParser"] = [] + def __init__(self, tokenizer, chat_template_kwargs=None): + self.tokenizer = tokenizer + self.chat_template_kwargs = chat_template_kwargs + self.called = False + instances.append(self) - def __init__(self, tokenizer, chat_template_kwargs=None): - self.tokenizer = tokenizer - self.chat_template_kwargs = chat_template_kwargs - self.called = False - type(self).instances.append(self) + def adjust_request(self, request): + self.called = True + request.skip_special_tokens = False + return request - 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): - _RecordingReasoningParser.instances.clear() request = ChatCompletionRequest.model_construct( messages=[{"role": "user", "content": "hi"}], model="test", @@ -2132,46 +2141,54 @@ class TestReasoningParserAdjustRequest: skip_special_tokens=false, which ordinary OpenAI-compatible clients do not. """ - def test_adjust_request_is_called(self, tokenizer): - req, _, _, _, _ = _prep( - tokenizer, reasoning_parser_class=_RecordingReasoningParser - ) - assert len(_RecordingReasoningParser.instances) == 1 - assert _RecordingReasoningParser.instances[0].called + 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): + 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=_RecordingReasoningParser, + reasoning_parser_class=parser_cls, chat_template_kwargs={"enable_thinking": False}, ) - assert _RecordingReasoningParser.instances == [] + assert instances == [] assert req.skip_special_tokens is True - def test_enable_thinking_true_still_calls(self, tokenizer): + def test_enable_thinking_true_still_calls( + self, tokenizer, recording_reasoning_parser + ): + parser_cls, instances = recording_reasoning_parser req, _, _, _, _ = _prep( tokenizer, - reasoning_parser_class=_RecordingReasoningParser, + reasoning_parser_class=parser_cls, chat_template_kwargs={"enable_thinking": True}, ) - assert _RecordingReasoningParser.instances[0].called + 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): + 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(_RecordingReasoningParser): + class _PassThrough(parser_cls): def adjust_request(self, request): self.called = True return request From 2b896715fb0391dd80a6ce9b322a1b52355f5ef8 Mon Sep 17 00:00:00 2001 From: Yifan Jiang <19356972+yifjiang@users.noreply.github.com> Date: Wed, 26 Aug 2026 11:35:03 -0700 Subject: [PATCH 03/22] fix(frontend): attribute the guidance rewrite instead of inferring it Removes the tradeoff in the previous commit rather than settling it. Both parsers rewrite the same request inside _prepare_request -- tool first, reasoning second -- and the code diffed the guidance across both calls at once. That single diff cannot say which parser moved it, so attribution was inferred from `tool_parser is None`. The inference is what forced a choice: with both parsers active a reasoning rewrite was unattributable, so it lost to the caller's copy, which is the bug this PR exists to fix. Take the reading between the two calls and attribution stops being a guess: client -> after_tool_parser changed by the tool parser after_tool_parser -> final changed by the reasoning parser Each rewrite then gets its own precedence, which is what was wanted all along. A reasoning rewrite wins whether or not a tool parser is active, closing the gap. A tool rewrite still loses to the caller's explicit constraint, so the pinned policy is untouched. 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 forwarding what the request holds, which is the invariant the report was about. _prepare_request takes an optional write-only guidance_snapshots dict rather than a sixth return value: the return is unpacked at 22 call sites across the two test files and widening it would have churned all of them. The snapshot deliberately uses request_for_sampling.structured_outputs, the same accessor as adjusted_structured_guidance, since it is only ever compared against that value; extract_structured_outputs() is a different view and would have made the diff spurious. Deletes reasoning_only_guided_decoding and its scoping rationale. Validated against the previous commit, same container, prepost.py swapped underneath: test_vllm_processor_unit.py before 11 failed / 117 passed after 10 failed / 118 passed regressions: none; the only delta is the new gap test tests/frontend/test_prepost.py 15 passed both The gap test fails on the previous commit with the exact symptom from the report -- {'json': {'type': 'object'}} instead of the structural tag -- and the three pinned cases pass under both. Signed-off-by: Yifan Jiang <19356972+yifjiang@users.noreply.github.com> --- components/src/dynamo/frontend/prepost.py | 64 ++++++++++++++----- .../tests/test_vllm_processor_unit.py | 39 ++++++++++- 2 files changed, 85 insertions(+), 18 deletions(-) diff --git a/components/src/dynamo/frontend/prepost.py b/components/src/dynamo/frontend/prepost.py index 222801a99da0..ffbe2960691a 100644 --- a/components/src/dynamo/frontend/prepost.py +++ b/components/src/dynamo/frontend/prepost.py @@ -447,6 +447,7 @@ def _prepare_request( 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. @@ -480,6 +481,22 @@ def _prepare_request( tool_parser = tool_parser_class(tokenizer, request_for_sampling.tools) request_for_sampling = tool_parser.adjust_request(request_for_sampling) + # Both parsers rewrite the same request in place, tool first and reasoning + # below, so a single before/after diff cannot say which one moved the + # guidance. Reading it here splits that into two attributable diffs and is the + # only reason the caller can apply one precedence rule to a reasoning rewrite + # and a different one to a tool rewrite. Optional and write-only, so the + # existing call sites are unaffected. + if guidance_snapshots is not None: + guidance_snapshots["after_tool_parser"] = deepcopy( + # Same accessor as adjusted_structured_guidance below, deliberately: + # this value is only ever compared against that one, and + # extract_structured_outputs() is a different view. + _guided_decoding_from_structured_outputs( + request_for_sampling.structured_outputs + ) + ) + # Strip tools from the template when tool_choice=none so the model doesn't # see them and generate raw XML tool calls in its response. tool_dicts = ( @@ -599,6 +616,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, @@ -615,6 +633,7 @@ async def preprocess_chat_request( 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( @@ -660,26 +679,37 @@ async def preprocess_chat_request( "tool_choice forces a tool call and cannot be combined with an " "explicit guided_* or structured_outputs constraint." ) - # A rewrite that only the reasoning parser can have made. With no tool parser - # there is no other adjust_request in this request, so the change is - # attributable, and tool_guided_decoding cannot carry it out either: - # build_tool_call_guided_decoding returns early when the request has no tools. - # Without this the request ends up holding the parser's structural tag while - # the engine is handed the pre-adjustment copy captured before - # _prepare_request ran. + # 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: # - # Scoped to that case on purpose. When a tool parser is also active the change - # is not attributable to either parser, and parser-vs-assistant precedence is - # already settled there -- the caller's explicit constraint wins, pinned by - # test_assistant_guidance_takes_precedence_over_auto_tool_guidance and the - # response-format-precedence row of TOOL_GUIDANCE_PARITY_CASES. Revisiting that - # policy is a separate change. - reasoning_only_guided_decoding = ( - parser_guided_decoding if tool_parser is None else None + # 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. + 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_only_guided_decoding is not None and not is_forced_tool_choice: - guided_decoding = reasoning_only_guided_decoding + 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: 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 50043ddbc986..aeb6d5b1af4e 100644 --- a/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py +++ b/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py @@ -1760,11 +1760,11 @@ def _renderer(): ) 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(), - tool_parser_class=None, **kwargs, ) @@ -1812,6 +1812,43 @@ async def test_thinking_disabled_does_not_recompute(self, tokenizer): 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"}} + class TestToolCallGuidedDecoding: def _request(self, tokenizer, **overrides): From 9692a98974a745adb5eb35ad7ef43741b96c7386 Mon Sep 17 00:00:00 2001 From: Yifan Jiang <19356972+yifjiang@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:03:19 -0700 Subject: [PATCH 04/22] fix(frontend): thread model_config to the reasoning parser; gate the snapshot Addresses three review threads on #12541. [P1] model_config. ReasoningParser.__init__ reads model_config out of **kwargs, and BaseCohereCommandReasoningParser.adjust_request returns the request untouched when it is absent (`if model_architecture is None: return request`). Constructing without it made this PR's whole adjust_request() call a no-op for the only shipped parser that overrides it. Verified against real vLLM: same parser, same request, model_config absent -> response_format kept and structured_outputs None; present -> structural tag produced and response_format cleared. The config is already reachable as self.input_processor.model_config (InputProcessor sets it from vllm_config), so no constructor change is needed. Threaded to all three construction sites -- _prepare_request, StreamingPostProcessor, and _build_reasoning_parser_metadata. The third does not read it, but constructing one of them differently is exactly the drift that hid this. NOT added to parser_kwargs: that dict goes on the wire to the worker and ModelConfig is not serializable. [P2] Snapshot cost. The guidance snapshot moved inside the _reasoning_parser_enabled() gate, since its only reader sits behind the same test and short-circuits. The deepcopy is gone: adjust_request() replaces request.structured_outputs with a fresh object rather than mutating it, so a plain reference still denotes the pre-adjustment value -- and aliasing lets the caller's `!=` short-circuit on identity instead of walking the tree. Measured on a 1.49 MB schema: 56.081 ms -> 0.0001 ms for the copy, plus 0.7155 ms -> 0.0001 ms on the comparison it feeds. Tool-only and no-parser requests now pay neither. [P2] Boundary test. test_generator_inner_forwards_reasoning_parser_and_model_config drives the real _generator_inner with a spy and asserts both wiring arguments arrive. Deleting either line fails only this test; before it, deleting reasoning_parser_class left all 143 green. Traded out the overlapping test_no_reasoning_parser_keeps_the_caller_constraint per the review. Also adds _ModelConfigGatedReasoningParser, a fake that mirrors Cohere's actual contract (no model_config => no rewrite). The existing fakes ignored model_config entirely, which is why a no-op passed them. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01LqtTc1QsXejXiEdpmLZ4Wz --- components/src/dynamo/frontend/prepost.py | 56 +++++--- .../tests/test_vllm_processor_unit.py | 129 ++++++++++++++++-- .../src/dynamo/frontend/vllm_processor.py | 12 ++ tests/frontend/test_prepost.py | 2 +- 4 files changed, 171 insertions(+), 28 deletions(-) diff --git a/components/src/dynamo/frontend/prepost.py b/components/src/dynamo/frontend/prepost.py index ffbe2960691a..3e57c6c01ceb 100644 --- a/components/src/dynamo/frontend/prepost.py +++ b/components/src/dynamo/frontend/prepost.py @@ -9,7 +9,7 @@ from concurrent.futures import ThreadPoolExecutor from copy import deepcopy from dataclasses import dataclass -from typing import Any, Protocol, TypeGuard, cast +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 +34,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.""" @@ -442,6 +445,7 @@ def _prepare_request( 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, @@ -481,22 +485,6 @@ def _prepare_request( tool_parser = tool_parser_class(tokenizer, request_for_sampling.tools) request_for_sampling = tool_parser.adjust_request(request_for_sampling) - # Both parsers rewrite the same request in place, tool first and reasoning - # below, so a single before/after diff cannot say which one moved the - # guidance. Reading it here splits that into two attributable diffs and is the - # only reason the caller can apply one precedence rule to a reasoning rewrite - # and a different one to a tool rewrite. Optional and write-only, so the - # existing call sites are unaffected. - if guidance_snapshots is not None: - guidance_snapshots["after_tool_parser"] = deepcopy( - # Same accessor as adjusted_structured_guidance below, deliberately: - # this value is only ever compared against that one, and - # extract_structured_outputs() is a different view. - _guided_decoding_from_structured_outputs( - request_for_sampling.structured_outputs - ) - ) - # Strip tools from the template when tool_choice=none so the model doesn't # see them and generate raw XML tool calls in its response. tool_dicts = ( @@ -548,9 +536,35 @@ def _prepare_request( # 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. @@ -595,6 +609,7 @@ async def preprocess_chat_request( 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, @@ -628,6 +643,7 @@ async def preprocess_chat_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, @@ -700,6 +716,10 @@ async def preprocess_chat_request( # 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 ) @@ -762,6 +782,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, @@ -786,6 +807,7 @@ def __init__( reasoning_parser_class( tokenizer, chat_template_kwargs=chat_template_kwargs, + model_config=model_config, ) if _reasoning_parser_enabled(reasoning_parser_class, chat_template_kwargs) and response_reasoning_ended is not True 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 aeb6d5b1af4e..e9f3dceddb4a 100644 --- a/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py +++ b/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py @@ -936,7 +936,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 +961,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 @@ -1191,6 +1191,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( @@ -1230,7 +1233,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 +1299,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 +1385,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 @@ -1715,8 +1783,9 @@ class _FakeStructuralTagReasoningParser: structural tag its model actually speaks. """ - def __init__(self, tokenizer, chat_template_kwargs=None): + 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 @@ -1731,6 +1800,22 @@ 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. @@ -1790,12 +1875,6 @@ async def test_passthrough_parser_keeps_the_caller_constraint(self, tokenizer): assert result.guided_decoding == {"json": {"type": "object"}} - @pytest.mark.asyncio - async def test_no_reasoning_parser_keeps_the_caller_constraint(self, tokenizer): - result = await self._preprocess(tokenizer, reasoning_parser_class=None) - - 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. @@ -1849,6 +1928,36 @@ async def test_tool_parser_rewrite_still_loses_to_the_caller(self, tokenizer): 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 TestToolCallGuidedDecoding: def _request(self, tokenizer, **overrides): diff --git a/components/src/dynamo/frontend/vllm_processor.py b/components/src/dynamo/frontend/vllm_processor.py index ac4c31bfd9a4..13ec592706ee 100644 --- a/components/src/dynamo/frontend/vllm_processor.py +++ b/components/src/dynamo/frontend/vllm_processor.py @@ -193,6 +193,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 +204,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 @@ -543,6 +552,7 @@ async def _generator_inner( 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, @@ -645,6 +655,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 @@ -746,6 +757,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 ), diff --git a/tests/frontend/test_prepost.py b/tests/frontend/test_prepost.py index c618cbb7ca68..af59ab8a2d0b 100644 --- a/tests/frontend/test_prepost.py +++ b/tests/frontend/test_prepost.py @@ -2098,7 +2098,7 @@ class _RecordingReasoningParser: needs skip_special_tokens=False so those markers survive detokenisation. """ - def __init__(self, tokenizer, chat_template_kwargs=None): + def __init__(self, tokenizer, chat_template_kwargs=None, model_config=None): self.tokenizer = tokenizer self.chat_template_kwargs = chat_template_kwargs self.called = False From 3f9e09c5ff8c84dd31f5e2fd96ca975bc3992bc7 Mon Sep 17 00:00:00 2001 From: Sungsoo Ha Date: Thu, 27 Aug 2026 21:35:24 -0700 Subject: [PATCH 05/22] fix(frontend): report reasoning token usage on the vllm chat-processor path Squashed from ai-dynamo/dynamo#13947 for this release branch. dynamo #12181 added reasoning-token usage only to the Rust OpenAIPreprocessor. `--dyn-chat-processor vllm` selects the Python path, which bypasses it, and the worker's _build_completion_usage() emits no `completion_tokens_details` key at all -- so `reasoning_tokens` was ABSENT from usage (NVBug 6678449b). The count is accumulated where the reasoning parser CLASSIFIES its output (StreamingPostProcessor.reasoning_token_total), not where the response is projected. Deriving it from the emitted choices loses it whenever a projection drops or defers the reasoning: include_reasoning=false suppresses the delta, and the non-streaming tool path buffers every chunk and releases on a zero-token terminal delta. Measured on Inkling-NVFP4 / GB300 with vLLM 0.28.0, reported vs an independent tokenization of the returned reasoning_content, 3 runs each: non-streaming tool call before 0.03/0.03/0.02 after 1.00/1.00/1.00 plain reasoning request before 1.04/1.03/1.02 after 1.00/1.03/1.04 Before, the tool case reported a constant 7 whatever the real count was. Signed-off-by: Sungsoo Ha --- components/src/dynamo/frontend/prepost.py | 30 +++ .../tests/test_vllm_processor_unit.py | 186 ++++++++++++++++++ .../src/dynamo/frontend/vllm_processor.py | 51 ++++- 3 files changed, 266 insertions(+), 1 deletion(-) diff --git a/components/src/dynamo/frontend/prepost.py b/components/src/dynamo/frontend/prepost.py index 9e5116f54141..83fb3d69a142 100644 --- a/components/src/dynamo/frontend/prepost.py +++ b/components/src/dynamo/frontend/prepost.py @@ -853,6 +853,19 @@ def __init__( self._reasoning_parser_streaming_started = False self._tool_parser_streaming_started = False + # Reasoning tokens attributed to THIS choice, accumulated where the + # reasoning parser CLASSIFIES output -- deliberately not where the + # response is projected. Two projections would otherwise lose the count + # entirely (NVBug 6678449b): + # * `_suppress_reasoning_output` (include_reasoning=false) drops the + # parsed reasoning from the emitted delta, so a response with hidden + # reasoning would report zero; + # * the non-streaming tool path buffers every generated chunk and + # emits once on the terminal delta, so per-chunk observation sees + # only a zero-token terminal frame. + # Counting at classification is invariant to both. + self.reasoning_token_total = 0 + self._control_markers = tuple( t for t in getattr(tokenizer, "all_special_tokens", ()) if t ) @@ -1265,6 +1278,15 @@ def _process_non_streaming_tool_output(self, output: Any) -> dict[str, Any] | No current_text, request=self.request_for_sampling, ) + # This path buffers every generated chunk and parses once here, so + # there is no per-chunk classification to count. The reasoning text + # IS known now, so count it exactly -- and do it before the + # include_reasoning projection on the next line, which would + # otherwise hide it. + if saved_reasoning: + self.reasoning_token_total += len( + self.tokenizer.encode(saved_reasoning, add_special_tokens=False) + ) if not self.request_for_sampling.include_reasoning: saved_reasoning = None @@ -1457,6 +1479,14 @@ def process_output(self, output: Any) -> dict[str, Any] | None: ) self._tool_parser_streaming_started = False + # Account BEFORE the projection below. `delta_message` is the parser's + # classification; `reasoning` is what survives include_reasoning. Reading + # the latter is what reported zero for hidden reasoning. + # Chunk-granular, matching the Rust estimator: a chunk carrying both + # reasoning and visible content counts entirely as reasoning. + if delta_message is not None and delta_message.reasoning: + self.reasoning_token_total += len(delta_token_ids) + choice = None if delta_message is None: if self.in_progress_tool_calls: 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 556b4b687205..3ac7c5bfca7d 100644 --- a/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py +++ b/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py @@ -2940,3 +2940,189 @@ 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): + self.tokenizer = tokenizer + self.chat_template_kwargs = chat_template_kwargs or {} + + 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, "" + + +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_chunks_only(self, tokenizer): + post = self._post(tokenizer) + post.process_output(self._out("thinking", [1, 2, 3])) + post.process_output(self._out("more", [4, 5])) + post.process_output(self._out("answer", [6, 7, 8, 9])) + assert post.reasoning_token_total == 5 + + # -- 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_content_only_counts_nothing(self, tokenizer): + post = self._post(tokenizer) + post.process_output(self._out("hi", [1, 2, 3, 4])) + assert post.reasoning_token_total == 0 + + # -- 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 diff --git a/components/src/dynamo/frontend/vllm_processor.py b/components/src/dynamo/frontend/vllm_processor.py index 13ec592706ee..44c70e1a1bcd 100644 --- a/components/src/dynamo/frontend/vllm_processor.py +++ b/components/src/dynamo/frontend/vllm_processor.py @@ -64,6 +64,51 @@ } +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( + getattr(post, "reasoning_token_total", 0) + 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 {}) + if not details.get("reasoning_tokens"): + 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 @@ -851,6 +896,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", [])) @@ -975,7 +1024,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 = { From ba7fa4c002c9341f4df58e53c96bb2ad9d206bfa Mon Sep 17 00:00:00 2001 From: Sungsoo Ha Date: Thu, 27 Aug 2026 21:39:07 -0700 Subject: [PATCH 06/22] build(container): add Inkling vLLM 0.28.0 runtime support Bumps the vLLM runtime base to v0.28.0-ubuntu2404 and layers the Inkling structural-tag patch onto it, following the Nemotron-3 Ultra pattern (#10234). Why 0.28.0: Inkling's tool parser and the structural-tag registry the patch extends only exist from that release. The tag is a multi-arch manifest list (linux/arm64 + linux/amd64), so GB300 aarch64 is covered. xpu and cpu stay at 0.27.1 -- untested at 0.28.0, and Inkling does not use them. Why the patch: 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, and a forced tool choice is simply not honoured. Upstream as vllm-project/vllm#54120; the .patch here is cut against the exact v0.28.0 tree and applies with `patch --forward -p1` clean. validate_inkling_runtime.py asserts the POSTCONDITIONS rather than trusting patch's exit code: 'inkling' registered in both the builtin set and the builder registry, structural_tag_model == "inkling", and supports_required_and_named equal to the value AbstractToolParser.__init_subclass__ derives from VLLM_ENFORCE_STRICT_TOOL_CALLING (setting it by hand is redundant and silently overwritten). Verified against the real patched runtime, and verified to FAIL when the tag is unregistered or the vLLM version differs. The build-time version test is load-bearing: `patch --forward` would otherwise skip or fuzz a hunk on a different base and leave a half-patched image that still builds successfully. NOTE: the image alone is not sufficient. The worker must also run with --dyn-enable-structural-tag AND --structured-outputs-config '{"enable_in_reasoning": true}'; without the second, vLLM never fills the grammar bitmask for a reasoning model and the grammar is inert. That belongs in the recipe, not here. Signed-off-by: Sungsoo Ha --- container/context.yaml | 6 +- .../inkling/0001-inkling-structural-tag.patch | 168 ++++++++++++++++++ .../deps/vllm/validate_inkling_runtime.py | 92 ++++++++++ container/templates/vllm_runtime.Dockerfile | 27 +++ 4 files changed, 292 insertions(+), 1 deletion(-) create mode 100644 container/deps/vllm/patches/v0.28.0/inkling/0001-inkling-structural-tag.patch create mode 100644 container/deps/vllm/validate_inkling_runtime.py diff --git a/container/context.yaml b/container/context.yaml index 1d88711f2d46..4228b2f4d61d 100644 --- a/container/context.yaml +++ b/container/context.yaml @@ -71,7 +71,11 @@ 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 needs 0.28.0: its tool parser and structural-tag registry (which the + # Inkling structural-tag patch extends) only exist from that release. 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 # 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 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..88126a7821d8 --- /dev/null +++ b/container/deps/vllm/patches/v0.28.0/inkling/0001-inkling-structural-tag.patch @@ -0,0 +1,168 @@ +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..75cc7f63268 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,102 @@ 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. ++_INKLING_MESSAGE_MODEL = "<|message_model|>" ++_INKLING_CONTENT_TEXT = "<|content_text|>" ++_INKLING_TOOL_JSON = "<|content_invoke_tool_json|>" ++_INKLING_END_MESSAGE = "<|end_message|>" ++_INKLING_TOOL_OPEN = _INKLING_MESSAGE_MODEL + _INKLING_TOOL_JSON ++_INKLING_TEXT_OPEN = _INKLING_MESSAGE_MODEL + _INKLING_CONTENT_TEXT ++ ++ ++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_END_MESSAGE, ++ ) ++ ++ ++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_END_MESSAGE]), ++ end=_INKLING_END_MESSAGE, ++ ) ++ ++ ++@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..50d6de19be54 --- /dev/null +++ b/container/deps/vllm/validate_inkling_runtime.py @@ -0,0 +1,92 @@ +#!/usr/bin/env python3 +# 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/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 From 5e37195a1dadfa7b01cb0449d4b0480c9bfa18cc Mon Sep 17 00:00:00 2001 From: Sungsoo Ha Date: Thu, 27 Aug 2026 21:40:29 -0700 Subject: [PATCH 07/22] style(container): drop the shebang from validate_inkling_runtime.py The pre-commit shebang hook requires an executable bit for a file that declares one. This validator is invoked as `python3 ` from the Dockerfile, exactly like its sibling validate_torch_compile_smoke.py, which carries no shebang and mode 644. Match the sibling rather than making the file executable. Signed-off-by: Sungsoo Ha --- container/deps/vllm/validate_inkling_runtime.py | 1 - 1 file changed, 1 deletion(-) diff --git a/container/deps/vllm/validate_inkling_runtime.py b/container/deps/vllm/validate_inkling_runtime.py index 50d6de19be54..f03f6219dc72 100644 --- a/container/deps/vllm/validate_inkling_runtime.py +++ b/container/deps/vllm/validate_inkling_runtime.py @@ -1,4 +1,3 @@ -#!/usr/bin/env python3 # 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. From bb9d2234a05ae1d07b56cb16e800a31e9340088f Mon Sep 17 00:00:00 2001 From: Sungsoo Ha Date: Thu, 27 Aug 2026 21:43:08 -0700 Subject: [PATCH 08/22] test(frontend): accept model_config in the accounting test's reasoning parser #12541 threads model_config into the reasoning-parser constructor, so the fake parser in the reasoning-accounting tests needs to accept it. Test-only; the production path already passes it. Caught by running the suite on the merged tree -- the cherry-pick auto-merged without conflict, which is not the same as being correct. Signed-off-by: Sungsoo Ha --- .../src/dynamo/frontend/tests/test_vllm_processor_unit.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 3ac7c5bfca7d..d1ceb425962d 100644 --- a/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py +++ b/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py @@ -2947,9 +2947,10 @@ class _AccountingReasoningParser: engine_based_streaming = False - def __init__(self, tokenizer, *, chat_template_kwargs=None): + 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 From 00245376f2c0088f5d441d15b84bc6e1905632b2 Mon Sep 17 00:00:00 2001 From: Sungsoo Ha Date: Thu, 27 Aug 2026 22:00:32 -0700 Subject: [PATCH 09/22] docs(container): state the real reason the vLLM bump to 0.28.0 is required The previous note claimed Inkling's tool parser and structural-tag registry only exist from 0.28.0. That is wrong -- both files, and every symbol the Inkling structural-tag patch touches, are present in 0.27.1, and the patch applies there cleanly. The bump is required for a different reason. The Inkling NVIDIA path moved substantially after 0.27.1, and the MTP support this recipe runs on 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 land in the same range (vllm#49876, #50528, #51391), which is the behaviour the frontend commits on this branch interact with. Separately, every Inkling number we have -- p0, IFEval/GPQA-D, and the moontrace throughput ladder -- was measured on 0.28.0, so shipping 0.27.1 would hand QA an engine combination that has never been validated for this model. Signed-off-by: Sungsoo Ha --- container/context.yaml | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/container/context.yaml b/container/context.yaml index 4228b2f4d61d..93096c9e3811 100644 --- a/container/context.yaml +++ b/container/context.yaml @@ -71,9 +71,18 @@ vllm: base_image: nvcr.io/nvidia/cuda-dl-base runtime_image: vllm/vllm-openai base_image_tag: 25.11-cuda13.0-devel-ubuntu24.04 - # Inkling needs 0.28.0: its tool parser and structural-tag registry (which the - # Inkling structural-tag patch extends) only exist from that release. Multi-arch - # manifest list (linux/arm64 + linux/amd64), so GB300 aarch64 is covered. + # 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 # Baseline is vllm-openai's TRUE base, nvidia/cuda:13.0.2-base-ubuntu24.04 From 087d0005c4d999372ec8ecb067f164c2dace1c03 Mon Sep 17 00:00:00 2001 From: Sungsoo Ha Date: Thu, 27 Aug 2026 22:16:56 -0700 Subject: [PATCH 10/22] fix(container): anchor the Inkling structural tag at the content-kind marker Refreshes the vendored patch from vllm-project/vllm#54120, which now begins the tool/text tags at `<|content_invoke_tool_json|>` / `<|content_text|>` instead of 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. The tags are `tags_with_separator` with `at_least_one=true`, so they anchor rather than trigger -- the old grammar therefore required a SECOND role marker. Compiling the tag with xgrammar shows the inversion, for forced/required/auto alike: suffix-only with duplicate marker before rejected accepted after accepted rejected It appeared to work only because the parser absorbed the duplicate, while manufacturing exactly the unconsumed-control-marker residue #12510 strips. Validated on Inkling-NVFP4 / GB300, vLLM 0.28.0, MTP 8, 2-replica AGG, applied over the shipped image via ConfigMap: forced tool choice honoured 5/5 against a prompt matching the other tool; required = 1 call; parallel_tool_calls true/false = 2/1; no control-marker residue in content -- all in both streaming and non-streaming. Full p0 blocking count unchanged at 2 (the unrelated reasoning_budget_zero vLLM limitation). Note the probe suite could NOT have caught this: control_marker_residue passed before the fix too, because the parser was absorbing the duplicate. The grammar-level acceptance test is what exposed it. Reported by dynamo-review-agent on #13954. Signed-off-by: Sungsoo Ha --- .../inkling/0001-inkling-structural-tag.patch | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) 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 index 88126a7821d8..216723d4e69b 100644 --- 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 @@ -22,7 +22,7 @@ index ab760015af7..517854500bc 100644 + # 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..75cc7f63268 100644 +index 1f9a651ee46..7a0688bb447 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( @@ -63,7 +63,7 @@ index 1f9a651ee46..75cc7f63268 100644 ] -@@ -577,6 +583,102 @@ def _k3_tools_channel(tools: list[FunctionToolParam]) -> TagFormat: +@@ -577,6 +583,111 @@ def _k3_tools_channel(tools: list[FunctionToolParam]) -> TagFormat: ) @@ -78,12 +78,21 @@ index 1f9a651ee46..75cc7f63268 100644 +# 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. -+_INKLING_MESSAGE_MODEL = "<|message_model|>" ++# ++# 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_TOOL_OPEN = _INKLING_MESSAGE_MODEL + _INKLING_TOOL_JSON -+_INKLING_TEXT_OPEN = _INKLING_MESSAGE_MODEL + _INKLING_CONTENT_TEXT ++_INKLING_TOOL_OPEN = _INKLING_TOOL_JSON ++_INKLING_TEXT_OPEN = _INKLING_CONTENT_TEXT + + +def _inkling_call_schema(tool: FunctionToolParam) -> dict[str, Any]: From 420f0d99525dd8748c7a767d3bec74bb18990fb7 Mon Sep 17 00:00:00 2001 From: Sungsoo Ha Date: Thu, 27 Aug 2026 23:06:12 -0700 Subject: [PATCH 11/22] fix(container): move vllm-omni to 0.28.0rc1 alongside the vLLM bump `vllm-runtime / Test cuda13.0` failed on both amd64 and arm64: RuntimeWarning: vLLM and vLLM-Omni appear to have mismatched major/minor versions: --> vLLM-Omni version 0.27.0rc1 --> vLLM version 0.28.0 vllm-omni asserts matching major/minor against the installed vLLM at import, so `vllm_omni_ref` is coupled to the runtime image tag and has to move with it. vllm-omni 0.28.0rc1 is published on PyPI (py3-none-any, not yanked). Adds a comment recording the coupling so the next bump does not repeat it. Signed-off-by: Sungsoo Ha --- container/context.yaml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/container/context.yaml b/container/context.yaml index 93096c9e3811..008c6fd46388 100644 --- a/container/context.yaml +++ b/container/context.yaml @@ -105,7 +105,11 @@ vllm: base_image_tag: 22.04 runtime_image_tag: v0.27.1 # baseline_sbom: not yet captured for cpu — runtime build runs without subtraction - vllm_omni_ref: "v0.27.0rc1" + # Moves WITH the vLLM runtime pin above: vllm-omni asserts matching major/minor + # against the installed vLLM at import, so leaving this at 0.27.0rc1 while vLLM + # is 0.28.0 fails `vllm-runtime / Test` on both arches with + # "vLLM and vLLM-Omni appear to have mismatched major/minor versions". + vllm_omni_ref: "v0.28.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. From 48a72a1c2b6bafcc393137ce44875e6a13ccc635 Mon Sep 17 00:00:00 2001 From: Sungsoo Ha Date: Thu, 27 Aug 2026 23:20:32 -0700 Subject: [PATCH 12/22] fix(container): make vllm_omni_ref per-device so xpu/cpu keep 0.27.0rc1 `vllm_omni_ref` was global while only the cuda13.0 stanza moved to vLLM 0.28.0, so the xpu and cpu images -- still on vLLM 0.27.1 -- would have installed vLLM-Omni 0.28.0rc1 and hit the very major/minor mismatch the previous commit pinned away from. Confirmed by rendering: before this change all three devices emitted `ARG VLLM_OMNI_REF=v0.28.0rc1` while xpu/cpu kept `RUNTIME_IMAGE_TAG=v0.27.1`, and install_vllm_omni.sh runs on every device. Move the ref into each device stanza next to the runtime_image_tag it must track, and read it in args.Dockerfile from the selected device context: cuda13.0 vLLM v0.28.0-ubuntu2404 omni v0.28.0rc1 xpu vLLM v0.27.1 omni v0.27.0rc1 cpu vLLM v0.27.1 omni v0.27.0rc1 Chosen over bumping xpu/cpu to vLLM 0.28.0 (their 0.28.0 base images do exist) because those targets are untested here and Inkling does not use them; this keeps their behaviour byte-identical to the base branch. The ARG sits inside `{% if framework == "vllm" %}`, so sglang/dynamo/trtllm never evaluate it -- verified by rendering each. (trtllm/cuda13.0 is an invalid combination on the base branch too; trtllm renders on cuda13.1, which is what CI builds.) Reported by dynamo-review-agent on #13954. Signed-off-by: Sungsoo Ha --- container/context.yaml | 13 ++++++++----- container/templates/args.Dockerfile | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/container/context.yaml b/container/context.yaml index 008c6fd46388..a4efa1e39ba9 100644 --- a/container/context.yaml +++ b/container/context.yaml @@ -85,6 +85,12 @@ vllm: # 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.0rc1" # 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 @@ -98,18 +104,15 @@ 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 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 # baseline_sbom: not yet captured for cpu — runtime build runs without subtraction - # Moves WITH the vLLM runtime pin above: vllm-omni asserts matching major/minor - # against the installed vLLM at import, so leaving this at 0.27.0rc1 while vLLM - # is 0.28.0 fails `vllm-runtime / Test` on both arches with - # "vLLM and vLLM-Omni appear to have mismatched major/minor versions". - vllm_omni_ref: "v0.28.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. diff --git a/container/templates/args.Dockerfile b/container/templates/args.Dockerfile index 046f4d86e566..886082b25649 100644 --- a/container/templates/args.Dockerfile +++ b/container/templates/args.Dockerfile @@ -99,7 +99,7 @@ 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 VLLM_OMNI_REF={{ context[framework][device_key].vllm_omni_ref }} {% if device == "cuda" -%} # If left blank, then we will fallback to vLLM defaults From 7d1d51de0e78a137b37ad2ce0df6d14d398b81d6 Mon Sep 17 00:00:00 2001 From: Sungsoo Ha Date: Thu, 27 Aug 2026 23:29:28 -0700 Subject: [PATCH 13/22] fix(container): pin transformers per-device; sync the #13947 review fixes Two problems, both found while checking the branch against what was actually validated rather than waiting for another CI round. 1. transformers_version was global at 5.14.1, but the VALIDATED Inkling image runs transformers 5.15.1. Shipping the pin would hand QA a transformers we never measured -- which also undercuts the argument for carrying the existing p0/accuracy/throughput numbers forward to the release container. The pin's own removal condition is met on cuda: it exists because vLLM 0.27.1 lacks the Transformers 5.15 heterogeneous Gemma 4 support, and vllm#49797 landed that and IS in 0.28.0 (verified: in v0.28.0, not in v0.27.1). xpu/cpu are still on vLLM 0.27.1 and still need 5.14.1, so -- exactly as with vllm_omni_ref -- the value has to be per-device, not global: cuda13.0 vLLM v0.28.0-ubuntu2404 omni v0.28.0rc1 transformers 5.15.1 xpu vLLM v0.27.1 omni v0.27.0rc1 transformers 5.14.1 cpu vLLM v0.27.1 omni v0.27.0rc1 transformers 5.14.1 TRANSFORMERS_VERSION is a hard `transformers==${...}` install plus a build-time equality assertion, so an empty value is not an option; each stanza carries a concrete version. 2. The frontend files had drifted from #13947, which picked up two CodeRabbit fixes after the squash landed here: a negative backend `reasoning_tokens` survived the falsiness check (-1 is truthy) and `total` used a defensive getattr on an attribute its own constructor guarantees. Forward-ported both, with the parametrised regression test; the annotator block is now identical between the two PRs. Rendered all three vllm devices plus sglang / dynamo / trtllm(13.1) to confirm the per-device lookup does not disturb other frameworks. Accounting suite 13/13. Signed-off-by: Sungsoo Ha --- .../frontend/tests/test_vllm_processor_unit.py | 13 +++++++++++++ components/src/dynamo/frontend/vllm_processor.py | 9 ++++++--- container/context.yaml | 14 ++++++++++---- container/templates/args.Dockerfile | 2 +- 4 files changed, 30 insertions(+), 8 deletions(-) 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 d1ceb425962d..7df05ed1729b 100644 --- a/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py +++ b/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py @@ -3127,3 +3127,16 @@ def test_annotate_replaces_a_zero_backend_value(self, tokenizer): 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/vllm_processor.py b/components/src/dynamo/frontend/vllm_processor.py index 44c70e1a1bcd..b5525189fadf 100644 --- a/components/src/dynamo/frontend/vllm_processor.py +++ b/components/src/dynamo/frontend/vllm_processor.py @@ -95,15 +95,18 @@ def __init__(self, post_processors: dict[int, StreamingPostProcessor]) -> None: @property def total(self) -> int: return sum( - getattr(post, "reasoning_token_total", 0) - for post in self._post_processors.values() + 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 {}) - if not details.get("reasoning_tokens"): + # 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 diff --git a/container/context.yaml b/container/context.yaml index a4efa1e39ba9..9f9b18a776a6 100644 --- a/container/context.yaml +++ b/container/context.yaml @@ -91,6 +91,12 @@ vllm: # 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.0rc1" + # PER-DEVICE for the same reason as vllm_omni_ref. The 5.14.1 pin below + # exists because vLLM 0.27.1 lacks the Transformers 5.15 heterogeneous + # Gemma 4 config support; vllm#49797 landed that and IS in 0.28.0, so this + # stanza takes 5.15.1 -- which is also what the validated Inkling image + # runs, so the shipped container matches what was measured. + transformers_version: "5.15.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 @@ -105,6 +111,8 @@ vllm: 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 @@ -112,11 +120,9 @@ vllm: 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 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/templates/args.Dockerfile b/container/templates/args.Dockerfile index 886082b25649..5b1d4680e8d5 100644 --- a/container/templates/args.Dockerfile +++ b/container/templates/args.Dockerfile @@ -98,7 +98,7 @@ 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 TRANSFORMERS_VERSION={{ context[framework][device_key].transformers_version }} ARG VLLM_OMNI_REF={{ context[framework][device_key].vllm_omni_ref }} {% if device == "cuda" -%} From af5399926fa90d039015b5e3f0673a31a0cae986 Mon Sep 17 00:00:00 2001 From: Sungsoo Ha Date: Fri, 28 Aug 2026 00:43:56 -0700 Subject: [PATCH 14/22] fix(container): parser-counter reasoning accounting; refresh the Inkling patch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Syncs both of our unmerged fixes to their reviewed state and re-validates the container on real hardware. frontend (ai-dynamo/dynamo#13947, P1 review): reasoning tokens are now counted by the pinned vLLM parser's own `count_reasoning_tokens` over a per-choice ledger of the ORIGINAL generated ids, appended before any branch in process_output. The per-chunk classifier and the terminal `tokenizer.encode(saved_reasoning)` retokenisation are both removed -- the latter drifted because detokenisation is not injective (generated id 77150 decodes to '１０' and re-encodes to two tokens). vLLM patch (vllm-project/vllm#54120): regenerated against the v0.28.0 tree, now also accepting `<|content_model_end_sampling|>`. The Inkling parser treats it as a co-equal terminator -- `is_reasoning_end` returns True on it and `count_reasoning_tokens` closes a block on either -- so permitting only `<|end_message|>` masked a valid natural stop. Applies clean with the Dockerfile's own `patch --batch --forward -p1`. Validated on Inkling-NVFP4 / GB300, vLLM 0.28.0, MTP 8, on BOTH topologies with the full patch set overlaid on the shipped image. p0: 13/15 with 2 blocking on each, and the two blockers are the same unpatchable vLLM limitation (reasoning_budget_zero, NVBug 6678449a): 2-replica AGG structural_tag_grammar pass · reasoning ratio 1.00 · kv_routing pass 1P1D disagg structural_tag_grammar pass · reasoning ratio 1.00 p0 was extended to cover these changes, because it structurally could not before: the tag can be wrong while every wire-level probe passes (the parser absorbs the malformed output -- control_marker_residue passed before BOTH tag defects), and reasoning_tokens can be non-zero but the wrong order (the pre-fix code reported a constant 7 against 324 real tokens). Signed-off-by: Sungsoo Ha --- components/src/dynamo/frontend/prepost.py | 47 +++---- .../tests/test_vllm_processor_unit.py | 131 ++++++++++++++++-- .../inkling/0001-inkling-structural-tag.patch | 18 ++- 3 files changed, 153 insertions(+), 43 deletions(-) diff --git a/components/src/dynamo/frontend/prepost.py b/components/src/dynamo/frontend/prepost.py index 83fb3d69a142..65e650e9beaa 100644 --- a/components/src/dynamo/frontend/prepost.py +++ b/components/src/dynamo/frontend/prepost.py @@ -853,18 +853,13 @@ def __init__( self._reasoning_parser_streaming_started = False self._tool_parser_streaming_started = False - # Reasoning tokens attributed to THIS choice, accumulated where the - # reasoning parser CLASSIFIES output -- deliberately not where the - # response is projected. Two projections would otherwise lose the count - # entirely (NVBug 6678449b): - # * `_suppress_reasoning_output` (include_reasoning=false) drops the - # parsed reasoning from the emitted delta, so a response with hidden - # reasoning would report zero; - # * the non-streaming tool path buffers every generated chunk and - # emits once on the terminal delta, so per-chunk observation sees - # only a zero-token terminal frame. - # Counting at classification is invariant to both. - self.reasoning_token_total = 0 + # 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 @@ -908,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]]: @@ -1278,15 +1280,6 @@ def _process_non_streaming_tool_output(self, output: Any) -> dict[str, Any] | No current_text, request=self.request_for_sampling, ) - # This path buffers every generated chunk and parses once here, so - # there is no per-chunk classification to count. The reasoning text - # IS known now, so count it exactly -- and do it before the - # include_reasoning projection on the next line, which would - # otherwise hide it. - if saved_reasoning: - self.reasoning_token_total += len( - self.tokenizer.encode(saved_reasoning, add_special_tokens=False) - ) if not self.request_for_sampling.include_reasoning: saved_reasoning = None @@ -1312,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(): @@ -1479,14 +1476,6 @@ def process_output(self, output: Any) -> dict[str, Any] | None: ) self._tool_parser_streaming_started = False - # Account BEFORE the projection below. `delta_message` is the parser's - # classification; `reasoning` is what survives include_reasoning. Reading - # the latter is what reported zero for hidden reasoning. - # Chunk-granular, matching the Rust estimator: a chunk carrying both - # reasoning and visible content counts entirely as reasoning. - if delta_message is not None and delta_message.reasoning: - self.reasoning_token_total += len(delta_token_ids) - choice = None if delta_message is None: if self.in_progress_tool_calls: 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 7df05ed1729b..c9d8db48ebac 100644 --- a/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py +++ b/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py @@ -2980,6 +2980,21 @@ def extract_reasoning(self, text, request=None): 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. @@ -3043,12 +3058,21 @@ def _annotator(*posts): # -- streaming classification ------------------------------------------ - def test_streaming_counts_reasoning_chunks_only(self, tokenizer): - post = self._post(tokenizer) - post.process_output(self._out("thinking", [1, 2, 3])) - post.process_output(self._out("more", [4, 5])) - post.process_output(self._out("answer", [6, 7, 8, 9])) - assert post.reasoning_token_total == 5 + 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 ------------ @@ -3093,11 +3117,100 @@ def test_non_streaming_buffering_counts_even_when_suppressed(self, tokenizer): post.process_output(self._out("answer", [3], finish_reason="stop")) assert post.reasoning_token_total > 0 - def test_content_only_counts_nothing(self, tokenizer): - post = self._post(tokenizer) - post.process_output(self._out("hi", [1, 2, 3, 4])) + 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 '１０', 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): 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 index 216723d4e69b..af4b76b58ef6 100644 --- 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 @@ -22,7 +22,7 @@ index ab760015af7..517854500bc 100644 + # 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..7a0688bb447 100644 +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( @@ -63,7 +63,7 @@ index 1f9a651ee46..7a0688bb447 100644 ] -@@ -577,6 +583,111 @@ def _k3_tools_channel(tools: list[FunctionToolParam]) -> TagFormat: +@@ -577,6 +583,119 @@ def _k3_tools_channel(tools: list[FunctionToolParam]) -> TagFormat: ) @@ -91,8 +91,16 @@ index 1f9a651ee46..7a0688bb447 100644 +_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]: @@ -116,7 +124,7 @@ index 1f9a651ee46..7a0688bb447 100644 + return TagFormat( + begin=_INKLING_TOOL_OPEN, + content=JSONSchemaFormat(json_schema=_inkling_call_schema(tool)), -+ end=_INKLING_END_MESSAGE, ++ end=_INKLING_ENDS, + ) + + @@ -132,8 +140,8 @@ index 1f9a651ee46..7a0688bb447 100644 +def _inkling_text_block() -> TagFormat: + return TagFormat( + begin=_INKLING_TEXT_OPEN, -+ content=AnyTextFormat(excludes=[_INKLING_END_MESSAGE]), -+ end=_INKLING_END_MESSAGE, ++ content=AnyTextFormat(excludes=_INKLING_ENDS), ++ end=_INKLING_ENDS, + ) + + From a4a7aac97063878d997ed1411ce0ba43e8a2a8b4 Mon Sep 17 00:00:00 2001 From: Sungsoo Ha Date: Fri, 28 Aug 2026 01:03:15 -0700 Subject: [PATCH 15/22] fix(container): keep transformers <5.15 -- vLLM-Omni is the binding constraint The multi-arch build failed at the vLLM-Omni install on both arches: Because vllm-omni>=0.28.0rc1 depends on transformers>=5.10.1,<5.15 and transformers==5.15.1, we can conclude that vllm-omni>=0.28.0rc1 cannot be used. vllm-omni 0.28.0rc1 declares a hard upper bound of <5.15, so pinning 5.15.1 makes the image unbuildable. Reverted to 5.14.1, which satisfies omni 0.28.0rc1 on cuda and omni 0.27.0rc1 on xpu/cpu. The earlier bump was justified with bad evidence: "the validated image runs transformers 5.15.1". That image is the thin pip-layer dev build, which does NOT install vllm-omni at all -- so it could never have exercised this combination. A running image only demonstrates the combination it actually contains. vllm#49797 (Gemma 4 heterogeneous config) being in 0.28.0 makes 5.15 PERMISSIBLE for vLLM, which needs only >=5.5.3 -- but not required. Omni is what binds here. Revisit when an omni release relaxes the bound. Adds check_pin_compatibility.py, which resolves the pins in each device stanza against their PyPI metadata and fails on a conflict. It reproduces this exact error in under a second instead of ~15 minutes into a multi-arch build, and it is verified in both directions: the fixed context passes, and reintroducing 5.15.1 reproduces the failure. Note it only compares pins against EACH OTHER, so a conflict with an unpinned transitive dependency still needs a real build. Signed-off-by: Sungsoo Ha --- container/context.yaml | 15 +- .../deps/vllm/check_pin_compatibility.py | 131 ++++++++++++++++++ 2 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 container/deps/vllm/check_pin_compatibility.py diff --git a/container/context.yaml b/container/context.yaml index 9f9b18a776a6..4e9960aeac96 100644 --- a/container/context.yaml +++ b/container/context.yaml @@ -91,12 +91,15 @@ vllm: # 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.0rc1" - # PER-DEVICE for the same reason as vllm_omni_ref. The 5.14.1 pin below - # exists because vLLM 0.27.1 lacks the Transformers 5.15 heterogeneous - # Gemma 4 config support; vllm#49797 landed that and IS in 0.28.0, so this - # stanza takes 5.15.1 -- which is also what the validated Inkling image - # runs, so the shipped container matches what was measured. - transformers_version: "5.15.1" + # MUST stay <5.15 while vLLM-Omni is pinned to 0.28.0rc1: 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 diff --git a/container/deps/vllm/check_pin_compatibility.py b/container/deps/vllm/check_pin_compatibility.py new file mode 100644 index 000000000000..0c708ba7fa3d --- /dev/null +++ b/container/deps/vllm/check_pin_compatibility.py @@ -0,0 +1,131 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Resolve container/context.yaml's vLLM pins against PyPI BEFORE building. + +`context.yaml` pins several packages independently, but they constrain each +other, and the resolver only says so ~15 minutes into a multi-arch build: + + Because vllm-omni>=0.28.0rc1 depends on transformers>=5.10.1,<5.15 and + transformers==5.15.1, we can conclude that vllm-omni>=0.28.0rc1 cannot be used. + +That pairing is easy to create by accident: each pin is defensible on its own +(omni must track vLLM's major/minor; transformers wants to track the vLLM +release), and nothing checks them together. Checking a pin against a *running +image* is not a substitute either -- the thin dev image carries transformers +5.15.1 and does not install omni at all, so it "proves" a combination that +cannot actually resolve. + + python3 container/deps/vllm/check_pin_compatibility.py [context.yaml] + +Exit 0 if every device stanza resolves, 1 otherwise. Requires network. +""" + +from __future__ import annotations + +import json +import re +import sys +import urllib.request +from pathlib import Path + +import yaml + +# (pinned package, the key holding its version) -- extend as pins are added. +PINNED = {"vllm-omni": "vllm_omni_ref", "transformers": "transformers_version"} + + +def _requires(package: str, version: str) -> list[str]: + url = f"https://pypi.org/pypi/{package}/{version}/json" + with urllib.request.urlopen(url, timeout=30) as resp: + return json.load(resp)["info"].get("requires_dist") or [] + + +def _parse(spec: str) -> list[tuple[str, tuple[int, ...]]]: + """`transformers<5.15,>=5.10.1` -> [('<', (5,15)), ('>=', (5,10,1))].""" + out = [] + for clause in spec.split(";")[0].split(","): + clause = clause.strip() + for op in (">=", "<=", "==", "!=", "<", ">", "~="): + _, sep, rest = clause.partition(op) + if sep: + # Take the leading numeric release only. A plain rstrip of digits + # would turn "5.15" into "5" and make every bound nonsense. + m = re.match(r"(\d+(?:\.\d+)*)", rest.strip()) + if m: + out.append((op, tuple(int(n) for n in m.group(1).split(".")))) + break + return out + + +def _satisfies(version: str, spec: str) -> bool: + base = version.lstrip("v").split("rc")[0] + try: + have = tuple(int(n) for n in base.split(".")) + except ValueError: + return True # unparseable -- do not fail the build on a guess + for op, want in _parse(spec): + n = min(len(have), len(want)) + h, w = have[:n], want[:n] + if op == ">=" and not h >= w: + return False + if op == "<" and not h < w: + return False + if op == "<=" and not h <= w: + return False + if op == ">" and not h > w: + return False + return True + + +def main(path: str) -> int: + ctx = yaml.safe_load(Path(path).read_text())["vllm"] + devices = [ + k for k, v in ctx.items() if isinstance(v, dict) and "runtime_image_tag" in v + ] + failures = [] + + for dev in devices: + stanza = { + **{k: v for k, v in ctx.items() if not isinstance(v, dict)}, + **ctx[dev], + } + pinned = { + pkg: str(stanza[key]).lstrip("v") + for pkg, key in PINNED.items() + if stanza.get(key) + } + for pkg, ver in pinned.items(): + try: + reqs = _requires(pkg, ver) + except Exception as exc: # noqa: BLE001 - reported, not masked + print(f" {dev}: could not read {pkg}=={ver} from PyPI: {exc}") + continue + for req in reqs: + name = ( + req.split()[0] + .split("[")[0] + .split(">")[0] + .split("<")[0] + .split("=")[0] + ) + if name in pinned and name != pkg: + if not _satisfies(pinned[name], req): + failures.append( + f"{dev}: {pkg}=={ver} requires `{req.split(';')[0].strip()}`, " + f"but {name} is pinned to {pinned[name]}" + ) + print(f" {dev}: {pinned}") + + if failures: + print("\nPIN CONFLICT -- this would fail mid-build:") + for f in failures: + print(f" - {f}") + return 1 + print("\nAll device stanzas resolve.") + return 0 + + +if __name__ == "__main__": + raise SystemExit( + main(sys.argv[1] if len(sys.argv) > 1 else "container/context.yaml") + ) From 818ec7650202d20551e1bea8bf35c5674af882b1 Mon Sep 17 00:00:00 2001 From: Sungsoo Ha Date: Fri, 28 Aug 2026 01:30:52 -0700 Subject: [PATCH 16/22] Revert "add check_pin_compatibility.py" Dropping the preflight script. The review correctly found it fails OPEN -- a PyPI fetch error was printed and ignored, so an unreachable index would exit 0 and falsely validate. A gate that passes when it cannot check is worse than no gate. That is a symptom rather than the reason to remove it. The script only compared pinned packages against EACH OTHER, so it never covered transitive dependencies; the container build already catches this class and everything metadata cannot see. It was added reacting to my own mistake, and it costs reviewers a second source of truth about dependency resolution that can drift from what uv does. What prevents recurrence is the comment kept in container/context.yaml: it records that vLLM-Omni, not vLLM, is what binds transformers below 5.15, and that the thin dev image carrying 5.15.1 does not install omni at all -- so it was never evidence that the combination resolves. That sits at the point of edit, which is where the mistake was made. Signed-off-by: Sungsoo Ha --- .../deps/vllm/check_pin_compatibility.py | 131 ------------------ 1 file changed, 131 deletions(-) delete mode 100644 container/deps/vllm/check_pin_compatibility.py diff --git a/container/deps/vllm/check_pin_compatibility.py b/container/deps/vllm/check_pin_compatibility.py deleted file mode 100644 index 0c708ba7fa3d..000000000000 --- a/container/deps/vllm/check_pin_compatibility.py +++ /dev/null @@ -1,131 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 -"""Resolve container/context.yaml's vLLM pins against PyPI BEFORE building. - -`context.yaml` pins several packages independently, but they constrain each -other, and the resolver only says so ~15 minutes into a multi-arch build: - - Because vllm-omni>=0.28.0rc1 depends on transformers>=5.10.1,<5.15 and - transformers==5.15.1, we can conclude that vllm-omni>=0.28.0rc1 cannot be used. - -That pairing is easy to create by accident: each pin is defensible on its own -(omni must track vLLM's major/minor; transformers wants to track the vLLM -release), and nothing checks them together. Checking a pin against a *running -image* is not a substitute either -- the thin dev image carries transformers -5.15.1 and does not install omni at all, so it "proves" a combination that -cannot actually resolve. - - python3 container/deps/vllm/check_pin_compatibility.py [context.yaml] - -Exit 0 if every device stanza resolves, 1 otherwise. Requires network. -""" - -from __future__ import annotations - -import json -import re -import sys -import urllib.request -from pathlib import Path - -import yaml - -# (pinned package, the key holding its version) -- extend as pins are added. -PINNED = {"vllm-omni": "vllm_omni_ref", "transformers": "transformers_version"} - - -def _requires(package: str, version: str) -> list[str]: - url = f"https://pypi.org/pypi/{package}/{version}/json" - with urllib.request.urlopen(url, timeout=30) as resp: - return json.load(resp)["info"].get("requires_dist") or [] - - -def _parse(spec: str) -> list[tuple[str, tuple[int, ...]]]: - """`transformers<5.15,>=5.10.1` -> [('<', (5,15)), ('>=', (5,10,1))].""" - out = [] - for clause in spec.split(";")[0].split(","): - clause = clause.strip() - for op in (">=", "<=", "==", "!=", "<", ">", "~="): - _, sep, rest = clause.partition(op) - if sep: - # Take the leading numeric release only. A plain rstrip of digits - # would turn "5.15" into "5" and make every bound nonsense. - m = re.match(r"(\d+(?:\.\d+)*)", rest.strip()) - if m: - out.append((op, tuple(int(n) for n in m.group(1).split(".")))) - break - return out - - -def _satisfies(version: str, spec: str) -> bool: - base = version.lstrip("v").split("rc")[0] - try: - have = tuple(int(n) for n in base.split(".")) - except ValueError: - return True # unparseable -- do not fail the build on a guess - for op, want in _parse(spec): - n = min(len(have), len(want)) - h, w = have[:n], want[:n] - if op == ">=" and not h >= w: - return False - if op == "<" and not h < w: - return False - if op == "<=" and not h <= w: - return False - if op == ">" and not h > w: - return False - return True - - -def main(path: str) -> int: - ctx = yaml.safe_load(Path(path).read_text())["vllm"] - devices = [ - k for k, v in ctx.items() if isinstance(v, dict) and "runtime_image_tag" in v - ] - failures = [] - - for dev in devices: - stanza = { - **{k: v for k, v in ctx.items() if not isinstance(v, dict)}, - **ctx[dev], - } - pinned = { - pkg: str(stanza[key]).lstrip("v") - for pkg, key in PINNED.items() - if stanza.get(key) - } - for pkg, ver in pinned.items(): - try: - reqs = _requires(pkg, ver) - except Exception as exc: # noqa: BLE001 - reported, not masked - print(f" {dev}: could not read {pkg}=={ver} from PyPI: {exc}") - continue - for req in reqs: - name = ( - req.split()[0] - .split("[")[0] - .split(">")[0] - .split("<")[0] - .split("=")[0] - ) - if name in pinned and name != pkg: - if not _satisfies(pinned[name], req): - failures.append( - f"{dev}: {pkg}=={ver} requires `{req.split(';')[0].strip()}`, " - f"but {name} is pinned to {pinned[name]}" - ) - print(f" {dev}: {pinned}") - - if failures: - print("\nPIN CONFLICT -- this would fail mid-build:") - for f in failures: - print(f" - {f}") - return 1 - print("\nAll device stanzas resolve.") - return 0 - - -if __name__ == "__main__": - raise SystemExit( - main(sys.argv[1] if len(sys.argv) > 1 else "container/context.yaml") - ) From 51c315dee5e87120a6dfeaebc8afbfce9fa26efc Mon Sep 17 00:00:00 2001 From: Sungsoo Ha Date: Fri, 28 Aug 2026 10:43:31 -0700 Subject: [PATCH 17/22] test: accept vLLM 0.28.0's session_id field; skip omni tests when diffusers breaks Two independent CI failures, neither in the shipped code paths. 1. test_engine_core_struct_contract. vLLM 0.28.0 appends `session_id` to EngineCoreRequest, after `abort_immediately`. The observed tuple differs from the nearest allowed variant by that one field. Added session_id variants (plain and vllm-omni), mirroring how the test already handles abort_immediately and model_intermediate_buffer. No production change: the test docstring claims vllm_processor.py reads EngineCoreRequest positionally, but it does not -- it uses attribute access (vllm_preproc.mm_features) and builds EngineCoreOutput by keyword (EngineCoreOutput(**output_kwargs)). A trailing append is therefore compatible, which is what the model_intermediate_buffer comment already says. 2. test_vllm_omni.py errored 4x and test_realtime_omni_bridge failed with "Main server process exited with code 1", both from: RuntimeError: Failed to import diffusers.pipelines.pipeline_utils ... cannot import name 'get_cached_repo_tree' from 'huggingface_hub' The omni guard caught (ImportError, OSError, NotImplementedError). diffusers imports its pipelines lazily and re-raises ANY underlying failure as RuntimeError, so a broken transitive dependency never reaches that tuple and the module ERRORS instead of skipping. Added RuntimeError. test_realtime_omni_bridge had no omni guard at all; its worker (realtime_omni_mock_worker.py) imports vllm_omni and dynamo.vllm.omni at module scope, so the same failure killed the worker and surfaced as an opaque server-exit. It now probes those imports and skips. These tests could not run at all before vllm-omni became installable on this branch, so they were silently skipping rather than passing. Making omni installable is what exposed the guard gap. Measured, not inferred: the base image ships huggingface_hub 1.28.0, which DOES export get_cached_repo_tree (verified in the wheel), and the build never installs, moves or downgrades hub. So this is a diffusers-0.40.0-vs-hub-1.28.0 incompatibility, not a pin conflict introduced here -- worth reporting upstream separately. Skipping restores the behaviour the suite already intended. Signed-off-by: Sungsoo Ha --- .../src/dynamo/vllm/tests/test_vllm_renderer_api.py | 8 ++++++++ tests/frontend/test_realtime_omni_bridge.py | 13 +++++++++++++ tests/serve/test_vllm_omni.py | 8 +++++++- 3 files changed, 28 insertions(+), 1 deletion(-) 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..3694296752a3 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 diff --git a/tests/frontend/test_realtime_omni_bridge.py b/tests/frontend/test_realtime_omni_bridge.py index 51cc999c4943..344cb3eb20a2 100644 --- a/tests/frontend/test_realtime_omni_bridge.py +++ b/tests/frontend/test_realtime_omni_bridge.py @@ -31,6 +31,19 @@ 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 +except (ImportError, OSError, NotImplementedError, RuntimeError): + pytest.skip("vLLM omni dependencies not available", allow_module_level=True) + logger = logging.getLogger(__name__) # Shared with the worker module (realtime_omni_mock_worker.py imports these). diff --git a/tests/serve/test_vllm_omni.py b/tests/serve/test_vllm_omni.py index 284f47a5277d..fe2c5e3f2059 100644 --- a/tests/serve/test_vllm_omni.py +++ b/tests/serve/test_vllm_omni.py @@ -10,7 +10,13 @@ try: from dynamo.vllm.omni.args import OmniConfig # noqa: F401 -except (ImportError, OSError, NotImplementedError): +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. pytest.skip("vLLM omni dependencies not available", allow_module_level=True) from tests.serve.common import ( From 92f3966ba9d76e063189cf9f3f31b04f941746aa Mon Sep 17 00:00:00 2001 From: Sungsoo Ha Date: Fri, 28 Aug 2026 11:10:35 -0700 Subject: [PATCH 18/22] test: accept vLLM 0.28.0's two new EngineCoreOutput fields The request-side assertion now passes; this is the output-side one behind it. vLLM 0.28.0 appends `mm_cache_miss_hashes` and `new_sampling_mask` after num_nans_in_logits -- after vLLM's own fields, before anything vllm-omni adds. No production change. vllm_processor.py builds EngineCoreOutput BY KEYWORD from four required kwargs and feature-detects optional fields through __struct_fields__, so appended fields take their msgspec defaults. That is not just an argument from the code: this exact path served ~7000 requests on vLLM 0.28.0 during validation (p0 plus a full 3541-request moontrace replay on both 2-replica AGG and 1P1D) with no error, so the defaults are demonstrably acceptable for detokenization. Rather than enumerate another four hand-written tuples, the variants are now generated as a product of (base | cached-token) x (with | without the 0.28.0 extras) x (with | without the omni extras) -- 8 variants from the same three building blocks the test already defined. Verified the CI-observed tuple is accepted and all four previously-valid shapes still are. Signed-off-by: Sungsoo Ha --- .../vllm/tests/test_vllm_renderer_api.py | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) 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 3694296752a3..852e4588821d 100755 --- a/components/src/dynamo/vllm/tests/test_vllm_renderer_api.py +++ b/components/src/dynamo/vllm/tests/test_vllm_renderer_api.py @@ -459,11 +459,22 @@ def test_engine_core_struct_contract(self): 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, ( From 1491de135eebb601c6ee439c5d7d461d51a92332 Mon Sep 17 00:00:00 2001 From: Sungsoo Ha Date: Fri, 28 Aug 2026 11:11:56 -0700 Subject: [PATCH 19/22] fix(test): drop variables orphaned by the generated output variants The previous commit replaced the hand-written omni output tuples with a generated product, leaving omni_output_fields and omni_cached_token_output_fields assigned but unused -- ruff F841, which I pushed without reading the pre-commit result. They are referenced nowhere else; the generated variants cover the same shapes. Signed-off-by: Sungsoo Ha --- components/src/dynamo/vllm/tests/test_vllm_renderer_api.py | 4 ---- 1 file changed, 4 deletions(-) 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 852e4588821d..357d17cd9811 100755 --- a/components/src/dynamo/vllm/tests/test_vllm_renderer_api.py +++ b/components/src/dynamo/vllm/tests/test_vllm_renderer_api.py @@ -455,10 +455,6 @@ 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 - ) # 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 From e19fdf858c38ef2fade428a08df9d44e669ddc7a Mon Sep 17 00:00:00 2001 From: Sungsoo Ha Date: Fri, 28 Aug 2026 13:55:33 -0700 Subject: [PATCH 20/22] fix(test): skip omni tests at setup, not at collection `pytest.skip(..., allow_module_level=True)` removes the module from COLLECTION, not just from running. The CI step `-m "(pre_merge and vllm and gpu_1) and not profiled_vram_gib"` has exactly one match in the repo -- `test_omni_serve_deployment[omni_audio]` in tests/serve/test_vllm_omni.py -- so once that module stopped being collected, the step selected nothing and pytest exited 5 ("no tests ran"), which the runner reports as a job failure. The test was already marked `pytest.mark.skip`, so before this it was collected and reported as skipped and the step exited 0. Guard the probe import into a flag and skip via `pytest.mark.skipif` instead: same intent, but the items stay collected and are reported as skipped. Verified in the runtime container with the omni import blocked, running that exact marker expression against tests/serve/test_vllm_omni.py: module-level skip -> "1 skipped", exit 5; skipif -> "1 skipped, 5 deselected", exit 0. tests/frontend/test_realtime_omni_bridge.py carried the same pattern and gets the same treatment, so it cannot empty a selection later. Signed-off-by: Sungsoo Ha --- tests/frontend/test_realtime_omni_bridge.py | 10 +++++++++- tests/serve/test_vllm_omni.py | 15 ++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/tests/frontend/test_realtime_omni_bridge.py b/tests/frontend/test_realtime_omni_bridge.py index 344cb3eb20a2..1b67583f8b87 100644 --- a/tests/frontend/test_realtime_omni_bridge.py +++ b/tests/frontend/test_realtime_omni_bridge.py @@ -41,8 +41,10 @@ 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): - pytest.skip("vLLM omni dependencies not available", allow_module_level=True) + _OMNI_AVAILABLE = False logger = logging.getLogger(__name__) @@ -57,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 fe2c5e3f2059..2c9c33251284 100644 --- a/tests/serve/test_vllm_omni.py +++ b/tests/serve/test_vllm_omni.py @@ -10,6 +10,8 @@ try: from dynamo.vllm.omni.args import OmniConfig # noqa: F401 + + _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 @@ -17,7 +19,7 @@ # 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. - pytest.skip("vLLM omni dependencies not available", allow_module_level=True) + _OMNI_AVAILABLE = False from tests.serve.common import ( WORKSPACE_DIR, @@ -35,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" ) From 766bf8924e93152f2287162697080b69ad65115c Mon Sep 17 00:00:00 2001 From: Sungsoo Ha Date: Fri, 28 Aug 2026 13:00:55 -0700 Subject: [PATCH 21/22] fix(frontend): stop reporting client errors as 500 on the vLLM chat-processor path Two request-rejection paths in the Python chat processor (`--dyn-chat-processor vllm`) surfaced as HTTP 500 with no usable explanation. The Rust OpenAIPreprocessor handles both correctly; the Python path does not go through it. 1. A worker-side rejection -- an unsupported sampling parameter, an unparseable grammar -- crosses the Rust boundary as a plain exception whose text is `BackendInvalidArgument: {"message":...,"code":400}`. The generic `except Exception` turned that into an internal error, discarding both the status and the reason. `backend_invalid_argument_to_http_error()` recovers them, mirroring `extract_backend_error_if_present` in lib/llm/src/http/service/openai.rs. 2. `logprobs`/`top_logprobs` are not implemented on this path (see the existing TODO), but the code warned and continued anyway, emitting a response with an empty logprobs struct that the HTTP layer cannot deserialize ("invalid length 0, expected struct ChatChoiceLogprobs with 2 elements"). It now rejects up front. The rejection is 400 rather than the 501 used for other unimplemented fields (`previous_response_id`, `background`, `prompt`): those are raised inside the Rust HTTP service, whereas a status asserted by a Python engine is triaged by SanitizedError::for_backend_status, which forwards a 4xx verbatim but replaces the body of any 5xx. A 501 raised here reaches the client as an opaque 500 with the reason lost -- observed, not assumed. Measured on Inkling-NVFP4 / GB300 (2-replica AGG, vLLM 0.28.0): min_p, malformed json_schema, logprobs and top_logprobs all go 500 -> 400 carrying the backend's own message; the other seven probe cases are unchanged. test_generator_preserves_zero_top_logprobs pinned the warn-and-continue behaviour, so it is repointed at the rejection and renamed. It still guards the subtlety it always did: `logprobs=True` with `top_logprobs=0` projects to 0, so a truthiness guard would let exactly that request through and re-create the 500 -- the guard tests `is not None`. Signed-off-by: Sungsoo Ha (cherry picked from commit 894ee680f295bd31cc2fa6ec03e7bad83afe4108) --- .../frontend/tests/test_frontend_utils.py | 71 +++++++++++++++++++ .../tests/test_vllm_processor_unit.py | 61 +++++++++++++--- components/src/dynamo/frontend/utils.py | 51 +++++++++++++ .../src/dynamo/frontend/vllm_processor.py | 35 ++++++++- 4 files changed, 207 insertions(+), 11 deletions(-) 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 c9d8db48ebac..50020c7a7540 100644 --- a/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py +++ b/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py @@ -32,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 @@ -1151,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() @@ -1185,7 +1184,7 @@ 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( @@ -1206,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( { @@ -1217,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 @@ -1492,6 +1494,47 @@ 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 + ): + # The mapping is narrow on purpose: anything without the discriminator + # keeps the existing internal-error behaviour. + 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()) + + assert chunks[-1]["error"]["type"] == "internal_error" + @pytest.mark.asyncio async def test_routed_engine_gets_extra_args_metadata(self, vllm_processor_module): routed_engine = _FakeRoutedEngine() diff --git a/components/src/dynamo/frontend/utils.py b/components/src/dynamo/frontend/utils.py index 88e1d24e2242..46a7c58cfa7e 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,55 @@ def make_internal_error(request_id: str, detail: str | None = None) -> dict[str, } +_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 b5525189fadf..f69e16564cd1 100644 --- a/components/src/dynamo/frontend/vllm_processor.py +++ b/components/src/dynamo/frontend/vllm_processor.py @@ -39,11 +39,13 @@ 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 ( + backend_invalid_argument_to_http_error, extract_mm_urls, handle_engine_error, make_internal_error, @@ -667,9 +669,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"): @@ -1053,6 +1072,18 @@ 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)) finally: From 36201cfd1b753ce1c78040e82081f363124350e0 Mon Sep 17 00:00:00 2001 From: Sungsoo Ha Date: Mon, 31 Aug 2026 10:35:46 -0700 Subject: [PATCH 22/22] fix(kv-router): preserve the prefill worker's error status through disagg On a disaggregated deployment a request the prefill worker had correctly rejected with 400 reached the client as a 500. The worker's error was attached to `PrefillError::PrefillError` as a `#[source]` only, and every path out of the prefill router reaches the frontend through `to_pyerr`, which formats with `Display` and drops the error chain (`lib/bindings/python/rust/lib.rs`). The frontend therefore saw the bare "Prefill router returned error in output" and could not tell an invalid argument from an engine fault. Fold the worker's text into the message at the two sites that wrap a real error, so the `BackendInvalidArgument: {...}` discriminator survives the Display-only boundary. The HTTP layer and the Python chat processors already key on it, so no other change is needed for the status to reach the client. `match_error_chain` is unaffected -- it matches error TYPES through the chain, never this string -- and the sourceless "no output (stream ended)" case is left alone. Also stop yielding untagged error dicts from the vLLM chat processor. The binding only treats a dict as an `Annotated` envelope when it carries `_dynamo_annotated` (`depythonize_annotated`); a bare `{"error": {...}}` is parsed as a completion chunk, fails with "missing field `id`", and the client gets an opaque 500 in place of the message -- so the reason never arrives even when the frontend has one. `as_error_envelope()` tags it; the shared helpers keep their shape, so the SGLang processor and the existing tests are untouched. Measured on Inkling-NVFP4 / GB300 1P1D disagg, where min_p and two malformed json_schema payloads returned 500 3/3 while logprobs (rejected frontend-side, so path-independent) correctly returned 400. Signed-off-by: Sungsoo Ha --- components/src/dynamo/frontend/utils.py | 18 +++++++++++++++++ .../src/dynamo/frontend/vllm_processor.py | 13 ++++++++---- .../src/kv_router/prefill_router/admission.rs | 20 +++++++++++-------- lib/llm/src/kv_router/prefill_router/mod.rs | 5 +++++ 4 files changed, 44 insertions(+), 12 deletions(-) diff --git a/components/src/dynamo/frontend/utils.py b/components/src/dynamo/frontend/utils.py index 46a7c58cfa7e..8dcb86ddb16e 100644 --- a/components/src/dynamo/frontend/utils.py +++ b/components/src/dynamo/frontend/utils.py @@ -183,6 +183,24 @@ 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]: + """Wrap an OpenAI-style error dict as an annotated error frame. + + A bare ``{"error": {...}}`` yielded from the async generator is NOT a valid + response object: the binding only treats a dict as an ``Annotated`` envelope + when it carries ``_dynamo_annotated`` (``depythonize_annotated`` in + ``lib/bindings/python/rust/engine.rs``). Without the tag it is parsed as a + completion chunk, fails with ``missing field `id```, and the client gets an + opaque 500 in place of the message -- so the error text never arrives. + + Tagging it keeps every field optional and lets the HTTP layer's existing + error handling read the message off ``comment``; when that message carries a + serialized backend status, the layer recovers that status too. + """ + 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: " diff --git a/components/src/dynamo/frontend/vllm_processor.py b/components/src/dynamo/frontend/vllm_processor.py index f69e16564cd1..78acc9a5eaa5 100644 --- a/components/src/dynamo/frontend/vllm_processor.py +++ b/components/src/dynamo/frontend/vllm_processor.py @@ -45,6 +45,7 @@ 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, @@ -947,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 @@ -1085,7 +1090,7 @@ async def _generate_and_stream( ) 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/lib/llm/src/kv_router/prefill_router/admission.rs b/lib/llm/src/kv_router/prefill_router/admission.rs index d1e39a9d9e61..0afaed9aa232 100644 --- a/lib/llm/src/kv_router/prefill_router/admission.rs +++ b/lib/llm/src/kv_router/prefill_router/admission.rs @@ -41,10 +41,15 @@ where }; if let Some(error) = first_output.err() { - return Err(PrefillError::PrefillError( - "Prefill router returned error in output".to_string(), - Some(Box::new(error)), - )); + // Fold the worker's own error text into the message. `to_pyerr` + // formats with `Display` only, so a `#[source]` never reaches the + // frontend: without this it saw the bare wrapper text and could not + // tell an invalid argument from an engine fault, so a request the + // worker had correctly rejected with 400 reached the client as a + // 500. The rendered text keeps the `BackendInvalidArgument: {...}` + // discriminator the HTTP layer and chat processors already key on. + 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 +77,9 @@ 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..c5a8c10b7ddb 100644 --- a/lib/llm/src/kv_router/prefill_router/mod.rs +++ b/lib/llm/src/kv_router/prefill_router/mod.rs @@ -79,6 +79,11 @@ pub enum PrefillError { #[error("Prefill router not yet activated")] NotActivated, + // Callers that wrap a worker error MUST fold its text into this message -- + // see `consume_prefill_stream`. Everything downstream of a disaggregated + // prefill reaches the frontend through `to_pyerr`, which formats with + // `Display` only (`lib/bindings/python/rust/lib.rs`) and drops the error + // chain, so a `#[source]` alone is invisible past that boundary. #[error("Prefill execution failed: {0}")] PrefillError( String,