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 1/4] 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 2/4] 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 3/4] 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 4/4] 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