diff --git a/Cargo.lock b/Cargo.lock index 96481da3a56d..bd84037340f0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1563,7 +1563,7 @@ version = "3.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "faf9468729b8cbcea668e36183cb69d317348c2e08e994829fb56ebfdfbaac34" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.61.2", ] [[package]] @@ -2711,9 +2711,9 @@ dependencies = [ [[package]] name = "dynamo-parsers" -version = "2.1.2" +version = "3.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2577dd23a85f6108a628580e2f7f0ec7c149c21d2fe280210e4be7752e6dee03" +checksum = "e1d76028cd9b3d99e733427f6c9fc0e176ddd5269437f4a71492c0ec8838bb5e" dependencies = [ "anyhow", "num-traits", @@ -2729,9 +2729,9 @@ dependencies = [ [[package]] name = "dynamo-parsers-v2" -version = "0.1.10" +version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "94ed5c413c09fd40cfb214a1e8cdcc8c7f536186f3bc5e433accfe24721b927c" +checksum = "cf3d2b9c65028a66f05b2db424d5f472b13662e7c7382c36685b884d6c8e2329" dependencies = [ "anyhow", "dynamo-parsers", @@ -6951,8 +6951,8 @@ version = "0.13.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "be769465445e8c1474e9c5dac2018218498557af32d9ed057325ec9a41ae81bf" dependencies = [ - "heck 0.4.1", - "itertools 0.10.5", + "heck 0.5.0", + "itertools 0.14.0", "log", "multimap", "once_cell", @@ -6971,8 +6971,8 @@ version = "0.14.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "343d3bd7056eda839b03204e68deff7d1b13aba7af2b2fd16890697274262ee7" dependencies = [ - "heck 0.4.1", - "itertools 0.10.5", + "heck 0.5.0", + "itertools 0.14.0", "log", "multimap", "petgraph 0.8.3", @@ -6993,7 +6993,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8a56d757972c98b346a9b766e3f02746cde6dd1cd1d1d563472929fdd74bec4d" dependencies = [ "anyhow", - "itertools 0.10.5", + "itertools 0.14.0", "proc-macro2", "quote", "syn 2.0.117", @@ -7006,7 +7006,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "27c6023962132f4b30eb4c172c91ce92d933da334c59c23cddee82358ddafb0b" dependencies = [ "anyhow", - "itertools 0.10.5", + "itertools 0.14.0", "proc-macro2", "quote", "syn 2.0.117", @@ -10745,7 +10745,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.61.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 93d9510876d4..b61eb4fbea1b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -53,11 +53,11 @@ dynamo-memory = { path = "lib/memory", version = "1.3.0" } dynamo-mocker = { path = "lib/mocker", version = "1.3.0" } dynamo-kv-router = { path = "lib/kv-router", version = "1.3.0", features = ["metrics", "runtime-protocols"] } dynamo-protocols = { version = "=2.0.1" } -dynamo-parsers = { version = "2.1.2" } +dynamo-parsers = { version = "3.0.0" } # Published on crates.io. To see all available versions: # web: https://crates.io/crates/dynamo-parsers-v2/versions # sparse index: curl https://index.crates.io/dy/na/dynamo-parsers-v2 (one JSON line per release; read the "vers" field) -dynamo-parsers-v2 = { version = "0.1.10" } +dynamo-parsers-v2 = { version = "0.1.11" } dynamo-backend-common = { path = "lib/backend-common", version = "1.3.0" } fastokens = { version = "0.2.0" } diff --git a/components/src/dynamo/frontend/sglang_prepost.py b/components/src/dynamo/frontend/sglang_prepost.py index 436f4fddee1d..91b5a6f41b15 100644 --- a/components/src/dynamo/frontend/sglang_prepost.py +++ b/components/src/dynamo/frontend/sglang_prepost.py @@ -93,9 +93,29 @@ def detect_force_reasoning_from_template(chat_template: str | None) -> bool: # Reasoning parsers that default to "thinking on" unless the client # explicitly opts out via chat_template_kwargs. Mirrors sglang's # serving_chat._get_reasoning_from_request table. -_THINKING_BY_DEFAULT = {"qwen3", "glm45", "nemotron_3", "interns1", "kimi_k2"} +_THINKING_BY_DEFAULT = { + "qwen3", + "glm45", + "nemotron_3", + "interns1", + "kimi_k2", +} _THINKING_OPT_IN = {"deepseek-v3", "deepseek-v4", "gemma4"} +_SGLANG_PARSER_NAME_ALIASES = { + # Dynamo's Rust parser registry accepts these MiniMax-M3 aliases. SGLang + # builds that include MiniMax-M3 expose the parser as "minimax-m3". + "minimax_m3": "minimax-m3", + "minimax_m3_nom": "minimax-m3", + "minimax-m3-nom": "minimax-m3", +} + + +def _normalize_sglang_parser_name(parser_name: str | None) -> str | None: + if not parser_name: + return parser_name + return _SGLANG_PARSER_NAME_ALIASES.get(parser_name, parser_name) + def resolve_request_force_reasoning( request: dict[str, Any], @@ -110,10 +130,14 @@ def resolve_request_force_reasoning( * opt-out families (``glm45``/``qwen3``/``kimi_k2``/...): on by default, ``chat_template_kwargs.enable_thinking=False`` (or ``thinking=False`` for ``kimi_k2``) disables it. + * MiniMax-M3 defaults to adaptive, but SGLang still enables the + reasoning parser unless ``chat_template_kwargs.thinking_mode`` is + explicitly ``"disabled"``. * opt-in families (``deepseek-v3``/``gemma4``): off by default, enabled by ``chat_template_kwargs.{thinking,enable_thinking}=True``. * anything else: follow the statically-detected template default. """ + reasoning_parser_name = _normalize_sglang_parser_name(reasoning_parser_name) if not reasoning_parser_name: return False @@ -121,6 +145,9 @@ def resolve_request_force_reasoning( request.get("chat_template_kwargs") or request.get("chat_template_args") or {} ) + if reasoning_parser_name == "minimax-m3": + return kwargs.get("thinking_mode") != "disabled" + if reasoning_parser_name in _THINKING_BY_DEFAULT: flag_key = ( "thinking" if reasoning_parser_name == "kimi_k2" else "enable_thinking" @@ -258,6 +285,7 @@ def create_parsers( if tool_choice == "required" or _is_named_tool_choice(tool_choice): tool_call_parser = JsonArrayParser() elif tool_call_parser_name: + tool_call_parser_name = _normalize_sglang_parser_name(tool_call_parser_name) tool_call_parser = FunctionCallParser( tools=sglang_tools, tool_call_parser=tool_call_parser_name, @@ -268,6 +296,7 @@ def create_parsers( tool_choice ) if reasoning_parser_name and not guided_decoding_active: + reasoning_parser_name = _normalize_sglang_parser_name(reasoning_parser_name) reasoning_parser = ReasoningParser( model_type=reasoning_parser_name, stream_reasoning=True, @@ -374,6 +403,9 @@ def setdefault_reasoning(enabled: bool) -> None: # Different SGLang model families consult different template toggles. chat_template_kwargs.setdefault("thinking", enabled) chat_template_kwargs.setdefault("enable_thinking", enabled) + chat_template_kwargs.setdefault( + "thinking_mode", "enabled" if enabled else "disabled" + ) thinking = request.get("thinking") if isinstance(thinking, bool): @@ -535,6 +567,7 @@ def build_tool_call_guided_decoding( ), ) elif tool_call_parser_name: + tool_call_parser_name = _normalize_sglang_parser_name(tool_call_parser_name) parser = FunctionCallParser( tools=sglang_tools, tool_call_parser=tool_call_parser_name, @@ -841,18 +874,22 @@ def __init__( history_tool_calls_count: int = 0, sglang_tools: list[SglangTool] | None = None, tool_call_parser_name: str | None = None, + eos_token_ids: list[int] | None = None, ) -> None: self.tokenizer = tokenizer self.tool_call_parser = tool_call_parser self.reasoning_parser = reasoning_parser self.history_tool_calls_count = history_tool_calls_count self._sglang_tools = sglang_tools or [] - self._tool_call_parser_name = tool_call_parser_name + self._tool_call_parser_name = _normalize_sglang_parser_name( + tool_call_parser_name + ) self._fast_plain_text = tool_call_parser is None and reasoning_parser is None # Preserve special tokens when a tool call parser is active so # delimiter tokens (e.g. <|tool_call|>) remain visible to the parser. self._skip_special_tokens = tool_call_parser is None self._is_json_array_parser = isinstance(tool_call_parser, JsonArrayParser) + self._eos_token_ids = set(eos_token_ids or []) self._all_token_ids: list[int] = [] # Tool call accumulation. SGLang's streaming parser returns @@ -869,6 +906,13 @@ def __init__( # Full text accumulator for robust finish-time re-parse. self._tool_text_parts: list[str] = [] + def _strip_trailing_eos_token_ids(self, token_ids: list[int]) -> list[int]: + if not self._eos_token_ids: + return token_ids + while token_ids and token_ids[-1] in self._eos_token_ids: + token_ids.pop() + return token_ids + def _tool_call_id(self, name: str, index: int) -> str: return _tool_call_id_for_parser( self._tool_call_parser_name, @@ -927,6 +971,8 @@ def process_output(self, engine_response: dict[str, Any]) -> dict[str, Any] | No raw_ids = engine_response.get("token_ids") token_ids = raw_ids if isinstance(raw_ids, list) else list(raw_ids or []) finish_reason = engine_response.get("finish_reason") + if finish_reason: + token_ids = self._strip_trailing_eos_token_ids(list(token_ids)) delta_text = self._incremental_decode(token_ids) if token_ids else "" diff --git a/components/src/dynamo/frontend/sglang_processor.py b/components/src/dynamo/frontend/sglang_processor.py index 8e228ec6fe3c..b3e3f00c255c 100644 --- a/components/src/dynamo/frontend/sglang_processor.py +++ b/components/src/dynamo/frontend/sglang_processor.py @@ -57,6 +57,28 @@ def _cached_tokens_from_usage(usage: dict[str, Any] | None) -> int | None: return cached_tokens if isinstance(cached_tokens, int) else None +def _normalize_eos_token_ids(value: Any) -> list[int]: + if isinstance(value, int) and not isinstance(value, bool): + return [value] + if isinstance(value, (list, tuple, set)): + token_ids: list[int] = [] + seen: set[int] = set() + for token_id in value: + if isinstance(token_id, int) and not isinstance(token_id, bool): + if token_id not in seen: + token_ids.append(token_id) + seen.add(token_id) + return token_ids + return [] + + +def _tokenizer_eos_token_ids(tokenizer: Any) -> list[int]: + eos_token_ids = _normalize_eos_token_ids(getattr(tokenizer, "eos_token_ids", None)) + if eos_token_ids: + return eos_token_ids + return _normalize_eos_token_ids(getattr(tokenizer, "eos_token_id", None)) + + def _load_tokenizer(source_path: str, trust_remote_code: bool): """Load the SGLang tokenizer, falling back to an on-disk chat template (e.g. chat_template.json) when the tokenizer defines none.""" @@ -156,7 +178,7 @@ def _init_worker( def _preprocess_worker( request: dict[str, Any], model_name: str, - eos_token_id: int | None, + eos_token_ids: list[int] | None, ) -> SglangPreprocessWorkerResult: """Preprocess a request in a worker process and return a picklable result.""" pre = preprocess_chat_request( @@ -176,7 +198,7 @@ def _preprocess_worker( request, pre.prompt_token_ids, model_name, - eos_token_id, + eos_token_ids, pre.guided_decoding, pre.tool_call_parser, ) @@ -198,7 +220,7 @@ def _build_dynamo_preproc( request: dict[str, Any], prompt_token_ids: list[int], model_name: str, - eos_token_id: int | None, + eos_token_ids: int | list[int] | None, guided_decoding: dict[str, Any] | None = None, tool_call_parser: ToolCallParserType | None = None, ) -> dict[str, Any]: @@ -261,7 +283,7 @@ def _build_dynamo_preproc( "skip_special_tokens": tool_call_parser is None, "return_tokens_as_token_ids": request.get("return_tokens_as_token_ids"), }, - "eos_token_ids": [eos_token_id] if eos_token_id is not None else [], + "eos_token_ids": _normalize_eos_token_ids(eos_token_ids), "annotations": [], "routing": request.get("routing"), } @@ -288,7 +310,7 @@ def __init__( routed_engine: RoutedEngine, tool_call_parser_name: str | None, reasoning_parser_name: str | None, - eos_token_id: int | None, + eos_token_ids: list[int] | None, debug_perf: bool = False, preprocess_pool: ProcessPoolExecutor | None = None, preprocess_workers: int = 0, @@ -313,7 +335,7 @@ def __init__( self.tool_call_parser_name = tool_call_parser_name self.reasoning_parser_name = reasoning_parser_name self.exclude_tools_when_tool_choice_none = True - self.eos_token_id = eos_token_id + self.eos_token_ids = _normalize_eos_token_ids(eos_token_ids) self.debug_perf = debug_perf self.stream_interval = stream_interval self.preprocess_pool = preprocess_pool @@ -393,7 +415,7 @@ async def _generator_inner( request, tokens, request["model"], - self.eos_token_id, + self.eos_token_ids, pre.guided_decoding, pre.tool_call_parser, ) @@ -412,6 +434,7 @@ async def _generator_inner( ), sglang_tools=convert_tools(request.get("tools")), tool_call_parser_name=self.tool_call_parser_name, + eos_token_ids=self.eos_token_ids, ) async for item in self._generate_and_stream( @@ -434,7 +457,7 @@ async def _generator_inner_pool( _preprocess_worker, request, request["model"], - self.eos_token_id, + self.eos_token_ids, ) preproc_result: SglangPreprocessWorkerResult = ( await asyncio.wrap_future(future) @@ -468,6 +491,7 @@ async def _generator_inner_pool( ), sglang_tools=convert_tools(request.get("tools")), tool_call_parser_name=self.tool_call_parser_name, + eos_token_ids=self.eos_token_ids, ) async for item in self._generate_and_stream( @@ -552,6 +576,7 @@ async def _generate_and_stream( # First chunk flushes immediately (si=1) to minimize TTFT. flush_threshold = 1 if first_chunk else stream_interval if finish_reason or len(pending_token_ids) >= flush_threshold: + usage_for_metrics = pending_usage mapped_response = { "token_ids": pending_token_ids, "finish_reason": finish_reason, @@ -578,6 +603,7 @@ async def _generate_and_stream( } if pending_usage: dynamo_out["usage"] = pending_usage + pending_usage = None response_nvext: dict[str, Any] = {} if stop_reason is not None and nvext_extra_field_requested( request, "stop_reason" @@ -597,7 +623,7 @@ async def _generate_and_stream( "output_tokens": cumulative_output_tokens, "chunk_tokens": len(pending_token_ids), } - cached_tokens = _cached_tokens_from_usage(pending_usage) + cached_tokens = _cached_tokens_from_usage(usage_for_metrics) if cached_tokens is not None: metrics["cached_tokens"] = cached_tokens envelope["event"] = "llm_metrics" @@ -677,7 +703,7 @@ async def chat_engine_factory( logger.info("Loading SGLang tokenizer from %s", local_dir) tokenizer = _load_tokenizer(local_dir, self.trust_remote_code) - eos_token_id = getattr(tokenizer, "eos_token_id", None) + eos_token_ids = _tokenizer_eos_token_ids(tokenizer) # Static reasoning-template scan (mirrors sglang's template_manager). # Shared with worker-pool processes via initargs so they compute the @@ -748,7 +774,7 @@ async def chat_engine_factory( routed_engine, tool_call_parser_name, reasoning_parser_name, - eos_token_id, + eos_token_ids, debug_perf=self.debug_perf, preprocess_pool=preprocess_pool, preprocess_workers=preprocess_workers, diff --git a/components/src/dynamo/frontend/tests/test_sglang_processor_metrics_unit.py b/components/src/dynamo/frontend/tests/test_sglang_processor_metrics_unit.py index 15df82cf5acc..40a26451c7c2 100644 --- a/components/src/dynamo/frontend/tests/test_sglang_processor_metrics_unit.py +++ b/components/src/dynamo/frontend/tests/test_sglang_processor_metrics_unit.py @@ -189,7 +189,7 @@ def test_stream_emits_llm_metrics_annotation(module_stubs): ), tool_call_parser_name=None, reasoning_parser_name=None, - eos_token_id=None, + eos_token_ids=None, ) async def collect(): diff --git a/components/src/dynamo/frontend/tests/test_sglang_processor_unit.py b/components/src/dynamo/frontend/tests/test_sglang_processor_unit.py index 48177de10d8f..86f5f8fdcc5b 100644 --- a/components/src/dynamo/frontend/tests/test_sglang_processor_unit.py +++ b/components/src/dynamo/frontend/tests/test_sglang_processor_unit.py @@ -28,11 +28,13 @@ _flatten_message_content, _normalize_assistant_tool_call_arguments, _normalize_prompt_token_ids, + _normalize_sglang_parser_name, _parse_json_array_buffer, build_tool_call_guided_decoding, convert_tools, create_parsers, preprocess_chat_request, + resolve_request_force_reasoning, ) from dynamo.frontend.sglang_processor import ( SglangPreprocessWorkerResult, @@ -40,7 +42,9 @@ _build_dynamo_preproc, _init_worker, _map_finish_reason, + _normalize_eos_token_ids, _runtime_config_parser_name, + _tokenizer_eos_token_ids, ) from dynamo.frontend.utils import ( PreprocessError, @@ -80,7 +84,7 @@ def test_defaults(self): {"model": "test", "messages": []}, prompt_token_ids=[1, 2, 3], model_name="test", - eos_token_id=2, + eos_token_ids=2, ) sampling = result["sampling_options"] assert sampling["n"] == 1 @@ -99,7 +103,7 @@ def test_top_k_zero_maps_to_negative_one(self): {"model": "test", "top_k": 0}, prompt_token_ids=[1], model_name="test", - eos_token_id=None, + eos_token_ids=None, ) assert result["sampling_options"]["top_k"] == -1 @@ -109,7 +113,7 @@ def test_top_k_positive_preserved(self): {"model": "test", "top_k": 50}, prompt_token_ids=[1], model_name="test", - eos_token_id=None, + eos_token_ids=None, ) assert result["sampling_options"]["top_k"] == 50 @@ -143,7 +147,7 @@ def test_guided_decoding_passthrough(self): {"model": "test"}, prompt_token_ids=[1, 2, 3], model_name="test", - eos_token_id=None, + eos_token_ids=None, guided_decoding={"json": {"type": "object"}}, ) assert result["sampling_options"]["guided_decoding"] == { @@ -205,11 +209,27 @@ def test_eos_token_id_present(self): result = _build_dynamo_preproc({"model": "test"}, [1], "test", 151643) assert result["eos_token_ids"] == [151643] + def test_eos_token_ids_list_deduped(self): + """All configured EOS token IDs are forwarded.""" + result = _build_dynamo_preproc({"model": "test"}, [1], "test", [2, 3, 2]) + assert result["eos_token_ids"] == [2, 3] + def test_eos_token_id_none(self): """None eos_token_id becomes empty list.""" result = _build_dynamo_preproc({"model": "test"}, [1], "test", None) assert result["eos_token_ids"] == [] + def test_tokenizer_eos_token_ids_prefers_full_list(self): + tokenizer = types.SimpleNamespace(eos_token_ids=[2, 3, 2], eos_token_id=2) + assert _tokenizer_eos_token_ids(tokenizer) == [2, 3] + + def test_tokenizer_eos_token_ids_falls_back_to_single_id(self): + tokenizer = types.SimpleNamespace(eos_token_id=2) + assert _tokenizer_eos_token_ids(tokenizer) == [2] + + def test_normalize_eos_token_ids_ignores_non_ints_and_bools(self): + assert _normalize_eos_token_ids([2, True, "3", 4, 2]) == [2, 4] + def test_logprobs_true_with_top_logprobs(self): """logprobs=True with top_logprobs=5 yields 5.""" result = _build_dynamo_preproc( @@ -537,6 +557,164 @@ def test_reasoning_active_when_tool_choice_auto(self): assert tcp is not None assert rp is not None + def test_minimax_m3_dynamo_aliases_are_normalized_for_sglang(self, monkeypatch): + """Dynamo parser aliases should not leak into SGLang parser lookup.""" + + # Test double: capture the parser name Dynamo passes to SGLang without + # depending on SGLang's real parser implementation. + class FakeFunctionCallParser: + def __init__(self, *, tools, tool_call_parser): + self.tools = tools + self.tool_call_parser = tool_call_parser + + class FakeReasoningParser: + def __init__(self, *, model_type, stream_reasoning, force_reasoning): + self.model_type = model_type + self.stream_reasoning = stream_reasoning + self.force_reasoning = force_reasoning + + monkeypatch.setattr( + sglang_prepost_module, + "FunctionCallParser", + FakeFunctionCallParser, + ) + monkeypatch.setattr( + sglang_prepost_module, + "ReasoningParser", + FakeReasoningParser, + ) + + tcp, rp = create_parsers( + { + "tools": [ + { + "type": "function", + "function": { + "name": "get_weather", + "parameters": {"type": "object", "properties": {}}, + }, + } + ], + "tool_choice": "auto", + }, + tool_call_parser_name="minimax-m3-nom", + reasoning_parser_name="minimax_m3", + ) + + assert tcp.tool_call_parser == "minimax-m3" + assert rp.model_type == "minimax-m3" + + +def test_normalize_sglang_parser_name_accepts_minimax_m3_aliases(): + assert _normalize_sglang_parser_name("minimax-m3") == "minimax-m3" + assert _normalize_sglang_parser_name("minimax_m3") == "minimax-m3" + assert _normalize_sglang_parser_name("minimax_m3_nom") == "minimax-m3" + assert _normalize_sglang_parser_name("minimax-m3-nom") == "minimax-m3" + assert _normalize_sglang_parser_name("kimi_k2") == "kimi_k2" + + +def test_minimax_m3_force_reasoning_uses_thinking_mode(): + assert ( + resolve_request_force_reasoning( + {"chat_template_kwargs": {}}, + "minimax_m3", + template_default=False, + ) + is True + ) + assert ( + resolve_request_force_reasoning( + {"chat_template_kwargs": {"thinking_mode": "disabled"}}, + "minimax-m3", + template_default=True, + ) + is False + ) + + +class _CapturingReasoningParser: + def __init__(self, *, model_type, stream_reasoning, force_reasoning): + self.model_type = model_type + self.stream_reasoning = stream_reasoning + self.force_reasoning = force_reasoning + + +@pytest.mark.parametrize( + "request_update", + [ + {"thinking": False}, + {"thinking": {"type": "disabled"}}, + {"reasoning_effort": "none"}, + ], +) +def test_minimax_m3_openai_disabled_thinking_sets_thinking_mode( + request_update, monkeypatch +): + monkeypatch.setattr( + sglang_prepost_module, + "ReasoningParser", + _CapturingReasoningParser, + ) + + request = { + "model": MODEL, + "messages": [{"role": "user", "content": "Hello"}], + } + request.update(request_update) + + class CapturingTokenizer: + chat_template = "template" + + def apply_chat_template(self, messages, **kwargs): + return [1, 2, 3] + + result = preprocess_chat_request( + request, + tokenizer=CapturingTokenizer(), + tool_call_parser_name=None, + reasoning_parser_name="minimax_m3", + ) + + assert result.request["chat_template_kwargs"]["thinking"] is False + assert result.request["chat_template_kwargs"]["enable_thinking"] is False + assert result.request["chat_template_kwargs"]["thinking_mode"] == "disabled" + assert result.force_reasoning is False + assert result.reasoning_parser.model_type == "minimax-m3" + assert result.reasoning_parser.force_reasoning is False + + +def test_minimax_m3_reasoning_effort_none_keeps_explicit_thinking_mode(monkeypatch): + monkeypatch.setattr( + sglang_prepost_module, + "ReasoningParser", + _CapturingReasoningParser, + ) + + request = { + "model": MODEL, + "messages": [{"role": "user", "content": "Hello"}], + "chat_template_kwargs": {"thinking_mode": "enabled"}, + "reasoning_effort": "none", + } + + class CapturingTokenizer: + chat_template = "template" + + def apply_chat_template(self, messages, **kwargs): + return [1, 2, 3] + + result = preprocess_chat_request( + request, + tokenizer=CapturingTokenizer(), + tool_call_parser_name=None, + reasoning_parser_name="minimax-m3", + ) + + assert result.request["chat_template_kwargs"]["thinking_mode"] == "enabled" + assert result.force_reasoning is True + assert result.reasoning_parser.model_type == "minimax-m3" + assert result.reasoning_parser.force_reasoning is True + class TestBuildToolCallGuidedDecoding: # FRONTEND.3 — guided-decoding setup for tool_choice def test_none_when_no_tools(self): @@ -570,6 +748,44 @@ def test_none_when_tool_choice_none(self): is None ) + def test_auto_tool_guidance_normalizes_minimax_m3_alias(self, monkeypatch): + tools = convert_tools( + [ + { + "type": "function", + "function": { + "name": "get_weather", + "parameters": {"type": "object", "properties": {}}, + }, + } + ] + ) + seen = {} + + # Test double: capture the parser name used for guided decoding setup. + class FakeFunctionCallParser: + def __init__(self, *, tools, tool_call_parser): + seen["tool_call_parser"] = tool_call_parser + + def get_structure_constraint(self, tool_choice, **kwargs): + assert tool_choice == "auto" + return "structural_tag", {"type": "object"} + + monkeypatch.setattr( + sglang_prepost_module, + "FunctionCallParser", + FakeFunctionCallParser, + ) + + guided = build_tool_call_guided_decoding( + {"tool_choice": "auto"}, + tool_call_parser_name="minimax_m3_nom", + sglang_tools=tools, + ) + + assert seen["tool_call_parser"] == "minimax-m3" + assert guided == {"structural_tag": {"type": "object"}} + def test_required_tool_choice_builds_json_schema_guidance(self): tools = convert_tools( [ @@ -1570,8 +1786,10 @@ def apply_chat_template(self, messages, **kwargs): assert result.prompt_token_ids == [1, 2, 3] assert captured["kwargs"]["thinking"] is True assert captured["kwargs"]["enable_thinking"] is True + assert captured["kwargs"]["thinking_mode"] == "enabled" assert result.request["chat_template_kwargs"]["thinking"] is True assert result.request["chat_template_kwargs"]["enable_thinking"] is True + assert result.request["chat_template_kwargs"]["thinking_mode"] == "enabled" assert "chat_template_kwargs" not in request @pytest.mark.parametrize( @@ -1608,8 +1826,10 @@ def apply_chat_template(self, messages, **kwargs): assert captured["kwargs"]["thinking"] is False assert captured["kwargs"]["enable_thinking"] is False + assert captured["kwargs"]["thinking_mode"] == "disabled" assert result.request["chat_template_kwargs"]["thinking"] is False assert result.request["chat_template_kwargs"]["enable_thinking"] is False + assert result.request["chat_template_kwargs"]["thinking_mode"] == "disabled" assert result.force_reasoning is False def test_reasoning_effort_none_keeps_explicit_template_flags(self): @@ -1942,7 +2162,7 @@ async def collect(): ), tool_call_parser_name=None, reasoning_parser_name=None, - eos_token_id=None, + eos_token_ids=None, ) post = SglangStreamingPostProcessor( tokenizer=tokenizer, tool_call_parser=None, reasoning_parser=None @@ -1971,7 +2191,7 @@ def _run_stream(self, tokenizer, items): routed_engine=FakeRoutedEngine(items=items), tool_call_parser_name=None, reasoning_parser_name=None, - eos_token_id=None, + eos_token_ids=None, ) post = SglangStreamingPostProcessor( tokenizer=tokenizer, tool_call_parser=None, reasoning_parser=None @@ -2037,6 +2257,17 @@ def test_lookback_trimming(self, tokenizer): # Should be trimmed, not 200 tokens assert len(post._all_token_ids) < 200 + def test_strips_all_configured_trailing_eos_token_ids(self, tokenizer): + """Any configured EOS id is stripped from the final chunk before decode.""" + post = SglangStreamingPostProcessor( + tokenizer=tokenizer, + tool_call_parser=None, + reasoning_parser=None, + eos_token_ids=[2, 3], + ) + + assert post._strip_trailing_eos_token_ids([10, 3, 2]) == [10] + # --------------------------------------------------------------------------- # SglangStreamingPostProcessor: fast plain text path diff --git a/lib/bindings/kvbm/Cargo.lock b/lib/bindings/kvbm/Cargo.lock index 445784c650c5..bf75e62b27d6 100644 --- a/lib/bindings/kvbm/Cargo.lock +++ b/lib/bindings/kvbm/Cargo.lock @@ -1705,9 +1705,9 @@ dependencies = [ [[package]] name = "dynamo-parsers" -version = "2.1.2" +version = "3.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2577dd23a85f6108a628580e2f7f0ec7c149c21d2fe280210e4be7752e6dee03" +checksum = "e1d76028cd9b3d99e733427f6c9fc0e176ddd5269437f4a71492c0ec8838bb5e" dependencies = [ "anyhow", "num-traits", @@ -1723,9 +1723,9 @@ dependencies = [ [[package]] name = "dynamo-parsers-v2" -version = "0.1.10" +version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "94ed5c413c09fd40cfb214a1e8cdcc8c7f536186f3bc5e433accfe24721b927c" +checksum = "cf3d2b9c65028a66f05b2db424d5f472b13662e7c7382c36685b884d6c8e2329" dependencies = [ "anyhow", "dynamo-parsers", diff --git a/lib/bindings/python/Cargo.lock b/lib/bindings/python/Cargo.lock index e985baafa527..47ac7bcdb0ee 100644 --- a/lib/bindings/python/Cargo.lock +++ b/lib/bindings/python/Cargo.lock @@ -2318,9 +2318,9 @@ dependencies = [ [[package]] name = "dynamo-parsers" -version = "2.1.2" +version = "3.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2577dd23a85f6108a628580e2f7f0ec7c149c21d2fe280210e4be7752e6dee03" +checksum = "e1d76028cd9b3d99e733427f6c9fc0e176ddd5269437f4a71492c0ec8838bb5e" dependencies = [ "anyhow", "num-traits", @@ -2336,9 +2336,9 @@ dependencies = [ [[package]] name = "dynamo-parsers-v2" -version = "0.1.10" +version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "94ed5c413c09fd40cfb214a1e8cdcc8c7f536186f3bc5e433accfe24721b927c" +checksum = "cf3d2b9c65028a66f05b2db424d5f472b13662e7c7382c36685b884d6c8e2329" dependencies = [ "anyhow", "dynamo-parsers", diff --git a/lib/bindings/python/Cargo.toml b/lib/bindings/python/Cargo.toml index 1be82aa6b875..59cdbf43fb6e 100644 --- a/lib/bindings/python/Cargo.toml +++ b/lib/bindings/python/Cargo.toml @@ -49,7 +49,7 @@ mm-routing = ["dynamo-llm/mm-routing"] aiconfigurator-core = { git = "https://github.com/ai-dynamo/aiconfigurator.git", rev = "ffbab15797d3a3f14c382e937c0ab0c1c5cbc530", package = "aiconfigurator-core", optional = true } dynamo-runtime = { path = "../../runtime" } dynamo-mocker = { path = "../../mocker" } -dynamo-parsers = { version = "2.1.2" } +dynamo-parsers = { version = "3.0.0" } dynamo-backend-common = { path = "../../backend-common" } anyhow = { version = "1" } diff --git a/lib/llm/src/http/service/openai.rs b/lib/llm/src/http/service/openai.rs index 4d34846eb7e4..5ef50bab6379 100644 --- a/lib/llm/src/http/service/openai.rs +++ b/lib/llm/src/http/service/openai.rs @@ -3730,7 +3730,9 @@ mod tests { assert_eq!(error_response.0, StatusCode::BAD_REQUEST); assert_eq!( error_response.1.message, - format!("{VALIDATION_PREFIX}`thinking.type` must be `enabled` or `disabled`") + format!( + "{VALIDATION_PREFIX}`thinking.type` must be `enabled`, `disabled`, or `adaptive`" + ) ); } } diff --git a/lib/llm/src/model_card.rs b/lib/llm/src/model_card.rs index 5229fa792984..a69e6a9ce71a 100644 --- a/lib/llm/src/model_card.rs +++ b/lib/llm/src/model_card.rs @@ -50,6 +50,60 @@ fn extract_hf_special_tokens(hf: &HfTokenizer) -> Vec { out } +/// Resolve the static architectural context limit from local HF metadata. +/// +/// `config.json` is authoritative when it declares `max_position_embeddings`; +/// multimodal configs may instead nest it under `text_config`; tokenizer +/// `model_max_length` is only a last resort. Missing fields fall through to the +/// next source, but malformed present fields return an error so bad model +/// metadata cannot silently resize request validation and planner limits. +/// +/// This was added for the MiniMax-M3-VL : its multimodal +/// Hugging Face config keeps the language model context under +/// `config.json.text_config.max_position_embeddings` +fn architectural_max_context_length_from_repo(local_path: &Path) -> anyhow::Result> { + let tokenizer_context_length = || { + crate::file_json_field( + &local_path.join("tokenizer_config.json"), + "model_max_length", + ) + .ok() + }; + + let config_path = local_path.join("config.json"); + let config_json = match std::fs::read_to_string(&config_path) { + Ok(config_json) => Some(config_json), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => None, + Err(err) => { + return Err(err).with_context(|| format!("Failed to read {}", config_path.display())); + } + }; + let Some(config_json) = config_json else { + return Ok(tokenizer_context_length().filter(|context_length| *context_length > 0)); + }; + let config: serde_json::Value = serde_json::from_str(&config_json) + .with_context(|| format!("Failed to parse JSON from file: {}", config_path.display()))?; + + let context_length = match config.get("max_position_embeddings") { + Some(value) => Some( + serde_json::from_value(value.clone()) + .context("Failed to deserialize max_position_embeddings")?, + ), + None => match config + .get("text_config") + .and_then(|text_config| text_config.get("max_position_embeddings")) + { + Some(value) => Some( + serde_json::from_value(value.clone()) + .context("Failed to deserialize text_config.max_position_embeddings")?, + ), + None => tokenizer_context_length(), + }, + }; + + Ok(context_length.filter(|context_length| *context_length > 0)) +} + #[derive(Serialize, Deserialize, Clone, Debug)] #[serde(rename_all = "snake_case")] pub enum ModelInfoType { @@ -1431,18 +1485,12 @@ impl ModelDeploymentCard { ) -> anyhow::Result { let local_path = local_path.as_ref(); - // This is usually the right choice + // Prefer the model config's architectural context length. Some + // multimodal HF configs, including MiniMax-M3-VL, store the language + // model config under `text_config`; tokenizer `model_max_length` can + // be a much larger sentinel/default and must be a last resort. let architectural_max_context_length = - crate::file_json_field(&local_path.join("config.json"), "max_position_embeddings") - // But sometimes this is - .or_else(|_| { - crate::file_json_field( - &local_path.join("tokenizer_config.json"), - "model_max_length", - ) - }) - .ok() - .filter(|context_length| *context_length > 0); + architectural_max_context_length_from_repo(local_path)?; let is_mistral_model = is_exclusively_mistral_model(local_path); @@ -2555,6 +2603,87 @@ mod tests { mod ownership_tests { use super::*; + #[test] + fn architectural_context_prefers_config_then_text_config_then_tokenizer() -> anyhow::Result<()> + { + let dir = tempfile::tempdir()?; + + std::fs::write( + dir.path().join("config.json"), + r#"{"max_position_embeddings": 4096}"#, + )?; + std::fs::write( + dir.path().join("tokenizer_config.json"), + r#"{"model_max_length": 131072}"#, + )?; + assert_eq!( + architectural_max_context_length_from_repo(dir.path())?, + Some(4096) + ); + + std::fs::write( + dir.path().join("config.json"), + r#"{"text_config": {"max_position_embeddings": 8192}}"#, + )?; + assert_eq!( + architectural_max_context_length_from_repo(dir.path())?, + Some(8192) + ); + + std::fs::write(dir.path().join("config.json"), r#"{"model_type": "mock"}"#)?; + assert_eq!( + architectural_max_context_length_from_repo(dir.path())?, + Some(131072) + ); + + std::fs::write(dir.path().join("config.json"), r#"{"text_config": {}}"#)?; + assert_eq!( + architectural_max_context_length_from_repo(dir.path())?, + Some(131072) + ); + + std::fs::remove_file(dir.path().join("config.json"))?; + assert_eq!( + architectural_max_context_length_from_repo(dir.path())?, + Some(131072) + ); + + Ok(()) + } + + #[test] + fn architectural_context_errors_on_malformed_present_fields() -> anyhow::Result<()> { + let dir = tempfile::tempdir()?; + std::fs::write( + dir.path().join("tokenizer_config.json"), + r#"{"model_max_length": 131072}"#, + )?; + + std::fs::write( + dir.path().join("config.json"), + r#"{"max_position_embeddings": "not-a-number"}"#, + )?; + let err = architectural_max_context_length_from_repo(dir.path()).unwrap_err(); + assert!( + err.to_string() + .contains("Failed to deserialize max_position_embeddings"), + "{err:?}" + ); + + std::fs::write( + dir.path().join("config.json"), + r#"{"text_config": {"max_position_embeddings": "not-a-number"}}"#, + )?; + let err = architectural_max_context_length_from_repo(dir.path()).unwrap_err(); + assert!( + err.to_string() + .contains("Failed to deserialize text_config.max_position_embeddings"), + "{err:?}" + ); + + Ok(()) + } + #[test] fn effective_context_prefers_runtime_then_architecture_then_unknown() { let mut card = ModelDeploymentCard::with_name_only("model"); diff --git a/lib/llm/src/preprocessor.rs b/lib/llm/src/preprocessor.rs index a238b0bcbf43..45068f49599c 100644 --- a/lib/llm/src/preprocessor.rs +++ b/lib/llm/src/preprocessor.rs @@ -662,13 +662,13 @@ impl OpenAIPreprocessor { }; TEMPLATE_SECONDS.observe(template_start.elapsed().as_secs_f64()); - // Check if the chat template injected a reasoning start token at the end - // of the prompt (e.g., Qwen3.5 appends `\n` when enable_thinking - // is not explicitly false). If so, the model's completion starts - // mid-reasoning and the parser should begin in reasoning mode. - let prompt_injected_reasoning = formatted_prompt - .as_ref() - .is_some_and(|p| p.trim_end().ends_with("")); + // Generic reasoning parsers start from ``; MiniMax M3 starts + // from ``. If the chat template injected that opener at the + // end of the prompt, the model completion starts mid-reasoning. + let prompt_injected_reasoning = Self::prompt_injected_reasoning_start( + self.runtime_config.reasoning_parser.as_deref(), + formatted_prompt.as_deref(), + ); let tokenize_start = Instant::now(); let (token_ids, annotations) = { @@ -2461,12 +2461,26 @@ impl OpenAIPreprocessor { // - harmony / gpt_oss: `<|channel|>analysis<|message|>...<|end|>`. // - kimi_k2: `<|tool_calls_section_begin|>` / `<|tool_calls_section_end|>`. // - kimi_k25: `` (special token id 163607). + // - minimax_m3: `]<]minimax[>[` tool-call namespace tokens and + // `` reasoning markers. matches!( tool_call_parser, - Some("gemma4") | Some("gemma-4") | Some("harmony") | Some("kimi_k2") + Some("gemma4") + | Some("gemma-4") + | Some("harmony") + | Some("kimi_k2") + | Some("minimax_m3") + | Some("minimax-m3") + | Some("minimax_m3_nom") + | Some("minimax-m3-nom") ) || matches!( reasoning_parser, - Some("gemma4") | Some("gemma-4") | Some("gpt_oss") | Some("kimi_k25") + Some("gemma4") + | Some("gemma-4") + | Some("gpt_oss") + | Some("kimi_k25") + | Some("minimax_m3") + | Some("minimax-m3") ) } @@ -2515,10 +2529,31 @@ impl OpenAIPreprocessor { fn skips_guided_json_when_prompt_injected(reasoning_parser: Option<&str>) -> bool { matches!( reasoning_parser, - Some("deepseek_v4" | "deepseek-v4" | "deepseekv4" | "glm45") + Some( + "deepseek_v4" + | "deepseek-v4" + | "deepseekv4" + | "glm45" + | "minimax_m3" + | "minimax-m3" + ) ) } + fn prompt_injected_reasoning_start( + reasoning_parser: Option<&str>, + formatted_prompt: Option<&str>, + ) -> bool { + let Some(prompt) = formatted_prompt.map(str::trim_end) else { + return false; + }; + + match reasoning_parser { + Some("minimax_m3") | Some("minimax-m3") => prompt.ends_with(""), + _ => prompt.ends_with(""), + } + } + /// Check if reasoning parsing should be disabled based on per-request parameters. /// For kimi_k25: disabled when chat_template_args contains "thinking": false. /// For Nemotron force-reasoning aliases: disabled when chat_template_args @@ -2531,6 +2566,9 @@ impl OpenAIPreprocessor { /// defined and enable_thinking` (truthy), so when callers explicitly set the /// flag false the model emits no `<|channel>` markers and the parser would /// only ever fall through. + /// For MiniMax M3: disabled when chat_template_args contains + /// "thinking_mode": "disabled", matching SGLang's MiniMax M3 request + /// convention. fn is_reasoning_disabled_by_request( reasoning_parser: Option<&str>, chat_template_args: Option<&std::collections::HashMap>, @@ -2579,6 +2617,14 @@ impl OpenAIPreprocessor { } false } + Some("minimax_m3") | Some("minimax-m3") => { + if let Some(args) = chat_template_args + && let Some(mode) = args.get("thinking_mode").and_then(|v| v.as_str()) + { + return mode == "disabled"; + } + false + } _ => false, } } @@ -3339,6 +3385,18 @@ mod tests { "kimi_k2 tool-call only → required \ (`<|tool_calls_section_begin|>` / `<|tool_calls_section_end|>` are special)", ), + ( + Some("minimax_m3"), + Some("minimax_m3"), + true, + "minimax_m3 paired → required", + ), + ( + Some("minimax-m3-nom"), + Some("minimax-m3"), + true, + "MiniMax M3 SGLang aliases → required", + ), (None, None, false, "no parsers → not required"), ]; for (tool, reasoning, expected, desc) in cases { @@ -3370,6 +3428,57 @@ mod tests { assert!(!f(Some(true), None, None)); } + #[test] + fn test_prompt_injected_reasoning_start_by_parser() { + let cases = [ + ( + Some("minimax_m3"), + Some("...\n"), + true, + "MiniMax M3 starts from ", + ), + ( + Some("minimax-m3"), + Some("...\n"), + false, + "MiniMax M3 prefilled end marker means not in reasoning", + ), + ( + Some("minimax_m3"), + Some("...\n"), + false, + "MiniMax M3 must not use generic ", + ), + ( + Some("qwen3"), + Some("...\n"), + true, + "Qwen-style templated behavior remains", + ), + ( + Some("deepseek_v4"), + Some("...\n"), + true, + "existing parser behavior remains", + ), + ( + None, + Some("...\n"), + true, + "legacy no-parser detection", + ), + (Some("minimax_m3"), None, false, "no prompt"), + ]; + + for (parser, prompt, expected, desc) in cases { + assert_eq!( + OpenAIPreprocessor::prompt_injected_reasoning_start(parser, prompt), + expected, + "FAILED: {desc}", + ); + } + } + #[test] fn test_backend_extra_args_preserves_nvext_and_sampling_extensions() { let request: NvCreateChatCompletionRequest = serde_json::from_value(serde_json::json!({ @@ -3521,6 +3630,14 @@ mod tests { ); m }; + let thinking_mode_disabled = { + let mut m = std::collections::HashMap::new(); + m.insert( + "thinking_mode".to_string(), + serde_json::Value::String("disabled".to_string()), + ); + m + }; let empty_args = std::collections::HashMap::new(); // (parser, args, expected_disabled, description) @@ -3721,6 +3838,30 @@ mod tests { true, "gemma-4 (hyphen alias) + enable_thinking=false → disabled", ), + ( + Some("minimax_m3"), + Some(&thinking_mode_disabled), + true, + "minimax_m3 + thinking_mode=disabled → disabled", + ), + ( + Some("minimax-m3"), + Some(&thinking_mode_disabled), + true, + "minimax-m3 + thinking_mode=disabled → disabled", + ), + ( + Some("minimax_m3"), + Some(&thinking_mode_thinking), + false, + "minimax_m3 + thinking_mode=thinking → enabled", + ), + ( + Some("minimax_m3"), + None, + false, + "minimax_m3 + no args → enabled", + ), ]; for (parser, args, expected, desc) in cases { diff --git a/lib/llm/src/protocols/openai/chat_completions.rs b/lib/llm/src/protocols/openai/chat_completions.rs index 43654dca952d..e07af11084a5 100644 --- a/lib/llm/src/protocols/openai/chat_completions.rs +++ b/lib/llm/src/protocols/openai/chat_completions.rs @@ -107,7 +107,7 @@ pub struct NvCreateChatCompletionRequest { pub chat_template_args: Option>, /// OpenAI-style thinking control from client request payloads. - /// Normalized into `chat_template_args.thinking` before preprocessing. + /// Normalized into `chat_template_args` before preprocessing. #[serde(default, skip_serializing_if = "Option::is_none")] pub thinking: Option, @@ -131,10 +131,10 @@ impl NvCreateChatCompletionRequest { /// Normalize OpenAI-style DS-V4 reasoning controls into the template kwargs /// consumed by the SGLang/DeepSeek-V4 prompt formatter. pub fn normalize_reasoning_template_args(&mut self) -> anyhow::Result<()> { - let thinking_enabled = self + let thinking_mode = self .thinking .as_ref() - .map(openai_thinking_enabled) + .map(openai_thinking_mode) .transpose()? .flatten(); let reasoning_effort = self @@ -143,17 +143,37 @@ impl NvCreateChatCompletionRequest { .as_ref() .and_then(|effort| serde_json::to_value(effort).ok()); - if thinking_enabled.is_none() && reasoning_effort.is_none() { + if thinking_mode.is_none() && reasoning_effort.is_none() { return Ok(()); } let args = self.chat_template_args.get_or_insert_with(HashMap::new); - if let Some(enabled) = thinking_enabled { - args.entry("thinking".to_string()) - .or_insert(serde_json::Value::Bool(enabled)); + if let Some(mode) = thinking_mode { + match mode { + OpenAiThinkingMode::Enabled => { + args.insert("thinking".to_string(), serde_json::Value::Bool(true)); + args.insert( + "thinking_mode".to_string(), + serde_json::Value::String("enabled".to_string()), + ); + } + OpenAiThinkingMode::Disabled => { + args.insert("thinking".to_string(), serde_json::Value::Bool(false)); + args.insert( + "thinking_mode".to_string(), + serde_json::Value::String("disabled".to_string()), + ); + } + OpenAiThinkingMode::Adaptive => { + args.insert( + "thinking_mode".to_string(), + serde_json::Value::String("adaptive".to_string()), + ); + } + } } if let Some(effort) = reasoning_effort { - args.entry("reasoning_effort".to_string()).or_insert(effort); + args.insert("reasoning_effort".to_string(), effort); } // The raw `thinking` payload has been folded into `chat_template_args`; @@ -164,23 +184,34 @@ impl NvCreateChatCompletionRequest { } } -fn openai_thinking_enabled(value: &serde_json::Value) -> anyhow::Result> { +enum OpenAiThinkingMode { + Enabled, + Disabled, + Adaptive, +} + +fn openai_thinking_mode(value: &serde_json::Value) -> anyhow::Result> { if let Some(enabled) = value.as_bool() { - return Ok(Some(enabled)); + return Ok(Some(if enabled { + OpenAiThinkingMode::Enabled + } else { + OpenAiThinkingMode::Disabled + })); } let Some(thinking_object) = value.as_object() else { anyhow::bail!( - "`thinking` must be a boolean or an object with `type` set to `enabled` or `disabled`" + "`thinking` must be a boolean or an object with `type` set to `enabled`, `disabled`, or `adaptive`" ); }; let Some(thinking_type) = thinking_object.get("type").and_then(|v| v.as_str()) else { - anyhow::bail!("`thinking.type` must be `enabled` or `disabled`"); + anyhow::bail!("`thinking.type` must be `enabled`, `disabled`, or `adaptive`"); }; match thinking_type { - "enabled" => Ok(Some(true)), - "disabled" => Ok(Some(false)), - _ => anyhow::bail!("`thinking.type` must be `enabled` or `disabled`"), + "enabled" => Ok(Some(OpenAiThinkingMode::Enabled)), + "disabled" => Ok(Some(OpenAiThinkingMode::Disabled)), + "adaptive" => Ok(Some(OpenAiThinkingMode::Adaptive)), + _ => anyhow::bail!("`thinking.type` must be `enabled`, `disabled`, or `adaptive`"), } } @@ -896,6 +927,87 @@ mod tests { ); } + #[test] + fn test_validate_messages_rejects_bad_tool_call_arguments() { + for arguments in ["{invalid json}", "[]", "null", "\"not an object\""] { + let request_json = json!({ + "model": "test-model", + "messages": [ + {"role": "user", "content": "weather?"}, + { + "role": "assistant", + "tool_calls": [{ + "id": "call_1", + "type": "function", + "function": { + "name": "get_weather", + "arguments": arguments + } + }] + }, + {"role": "tool", "tool_call_id": "call_1", "content": "sunny"} + ], + "tools": [{ + "type": "function", + "function": { + "name": "get_weather", + "parameters": {"type": "object", "properties": {}} + } + }] + }); + + let request: NvCreateChatCompletionRequest = + serde_json::from_value(request_json).expect("Failed to deserialize request"); + let err = ValidateRequest::validate(&request) + .expect_err("bad tool_call arguments should fail validation"); + let err = err.to_string(); + assert!( + err.contains("`messages[1].tool_calls[0].function.arguments`"), + "unexpected error for {arguments:?}: {err}" + ); + assert!( + err.contains("valid JSON object string"), + "unexpected error for {arguments:?}: {err}" + ); + } + } + + #[test] + fn test_validate_messages_accepts_empty_tool_call_arguments() { + for arguments in ["", " \n\t ", "{}"] { + let request_json = json!({ + "model": "test-model", + "messages": [ + {"role": "user", "content": "weather?"}, + { + "role": "assistant", + "tool_calls": [{ + "id": "call_1", + "type": "function", + "function": { + "name": "get_weather", + "arguments": arguments + } + }] + }, + {"role": "tool", "tool_call_id": "call_1", "content": "sunny"} + ], + "tools": [{ + "type": "function", + "function": { + "name": "get_weather", + "parameters": {"type": "object", "properties": {}} + } + }] + }); + + let request: NvCreateChatCompletionRequest = + serde_json::from_value(request_json).expect("Failed to deserialize request"); + ValidateRequest::validate(&request) + .unwrap_or_else(|err| panic!("empty tool_call arguments should validate: {err}")); + } + } + #[test] fn test_validate_tools_valid_names() { fn make_tool(name: &str) -> ChatCompletionTool { @@ -960,9 +1072,91 @@ mod tests { .as_ref() .expect("chat_template_args should be populated"); assert_eq!(args.get("thinking"), Some(&json!(true))); + assert_eq!(args.get("thinking_mode"), Some(&json!("enabled"))); assert_eq!(args.get("reasoning_effort"), Some(&json!("max"))); } + #[test] + fn test_openai_thinking_adaptive_normalizes_to_template_mode() { + let json_str = json!({ + "model": "MiniMaxAI/MiniMax-M3", + "messages": [ + {"role": "user", "content": "Hello"} + ], + "thinking": {"type": "adaptive"} + }); + + let mut request: NvCreateChatCompletionRequest = + serde_json::from_value(json_str).expect("Failed to deserialize request"); + request + .normalize_reasoning_template_args() + .expect("adaptive thinking payload should normalize"); + + let args = request + .chat_template_args + .as_ref() + .expect("chat_template_args should be populated"); + assert_eq!(args.get("thinking_mode"), Some(&json!("adaptive"))); + assert_eq!(args.get("thinking"), None); + assert!(request.thinking.is_none()); + } + + #[test] + fn test_openai_thinking_disabled_normalizes_to_template_mode() { + let json_str = json!({ + "model": "MiniMaxAI/MiniMax-M3", + "messages": [ + {"role": "user", "content": "Hello"} + ], + "thinking": {"type": "disabled"} + }); + + let mut request: NvCreateChatCompletionRequest = + serde_json::from_value(json_str).expect("Failed to deserialize request"); + request + .normalize_reasoning_template_args() + .expect("disabled thinking payload should normalize"); + + let args = request + .chat_template_args + .as_ref() + .expect("chat_template_args should be populated"); + assert_eq!(args.get("thinking"), Some(&json!(false))); + assert_eq!(args.get("thinking_mode"), Some(&json!("disabled"))); + } + + #[test] + fn test_openai_thinking_top_level_overrides_stale_template_args() { + let json_str = json!({ + "model": "MiniMaxAI/MiniMax-M3", + "messages": [ + {"role": "user", "content": "Hello"} + ], + "chat_template_args": { + "thinking": true, + "thinking_mode": "thinking", + "reasoning_effort": "high" + }, + "reasoning_effort": "none", + "thinking": {"type": "disabled"} + }); + + let mut request: NvCreateChatCompletionRequest = + serde_json::from_value(json_str).expect("Failed to deserialize request"); + request + .normalize_reasoning_template_args() + .expect("top-level thinking payload should normalize"); + + let args = request + .chat_template_args + .as_ref() + .expect("chat_template_args should be populated"); + assert_eq!(args.get("thinking"), Some(&json!(false))); + assert_eq!(args.get("thinking_mode"), Some(&json!("disabled"))); + assert_eq!(args.get("reasoning_effort"), Some(&json!("none"))); + assert!(request.thinking.is_none()); + } + #[test] fn test_invalid_openai_thinking_payload_is_rejected() { for invalid_thinking in [ diff --git a/lib/llm/src/protocols/openai/validate.rs b/lib/llm/src/protocols/openai/validate.rs index 075c4cbcb547..abaa42f1eac5 100644 --- a/lib/llm/src/protocols/openai/validate.rs +++ b/lib/llm/src/protocols/openai/validate.rs @@ -443,6 +443,35 @@ pub fn validate_messages( if messages.is_empty() { anyhow::bail!("Messages array cannot be empty"); } + // Prior assistant tool-call messages in the request must carry arguments + // as a JSON object string; reject bad non-empty shapes before chat-template rendering. + // This was caught in MiniMax-M3 multi-turn tool-call tests + for (message_index, message) in messages.iter().enumerate() { + if let dynamo_protocols::types::ChatCompletionRequestMessage::Assistant(assistant) = message + && let Some(tool_calls) = &assistant.tool_calls + { + for (tool_call_index, tool_call) in tool_calls.iter().enumerate() { + validate_json_object_string( + &tool_call.function.arguments, + format!( + "`messages[{message_index}].tool_calls[{tool_call_index}].function.arguments`" + ), + )?; + } + } + } + Ok(()) +} + +fn validate_json_object_string(value: &str, field: String) -> Result<(), anyhow::Error> { + if value.trim().is_empty() { + return Ok(()); + } + let parsed: serde_json::Value = serde_json::from_str(value) + .map_err(|error| anyhow::anyhow!("{field} must be a valid JSON object string: {error}"))?; + if !parsed.is_object() { + anyhow::bail!("{field} must be a valid JSON object string"); + } Ok(()) } diff --git a/lib/llm/tests/postprocessor_parsing_stream.rs b/lib/llm/tests/postprocessor_parsing_stream.rs index 6fd41905771b..d0e23dfe2a66 100644 --- a/lib/llm/tests/postprocessor_parsing_stream.rs +++ b/lib/llm/tests/postprocessor_parsing_stream.rs @@ -1545,6 +1545,43 @@ async fn tool_choice_deepseek_v4_required_prompt_injected_bare_json_recovers() { ); } +/// MiniMax M3 + required + `prompt_injected_reasoning=true` + bare JSON. +/// +/// MiniMax M3 chat templates seed `` rather than ``. When +/// guided decoding emits the constrained tool-call JSON from token 0, that JSON +/// must bypass reasoning parsing so the immediate jail can extract tool_calls. +#[tokio::test] +async fn tool_choice_minimax_m3_required_prompt_injected_bare_json_recovers() { + let bare_json = r#"[{"name":"get_weather","parameters":{"location":"San Francisco"}}]"#; + let preprocessor = build_preprocessor(Some("minimax_m3"), Some("minimax_m3")); + let request = streaming_tool_request(ChatCompletionToolChoiceOption::Required); + let input_stream = stream::iter( + vec![mock_content_chunk(bare_json), mock_final_chunk()] + .into_iter() + .map(Annotated::from_data), + ); + let output_stream = preprocessor + .postprocessor_parsing_stream(input_stream, &request, true, false) + .expect("postprocessor_parsing_stream should build"); + let DrainOutput { + reasoning, + content, + tool_calls, + finish_reasons, + } = drain_stream(output_stream).await; + + let case = "MiniMax M3 required + prompt_injected=true + bare JSON"; + assert!( + reasoning.is_empty(), + "{case}: guided JSON must not be classified as reasoning_content, got: {reasoning:?}" + ); + assert_clean_tool_call(case, &content, &tool_calls, "San Francisco"); + assert!( + finish_reasons.contains(&FinishReason::ToolCalls), + "{case}: expected ToolCalls finish_reason, got: {finish_reasons:?}" + ); +} + /// Exercises the experimental parsers-v2 gate end-to-end. `tool_choice=Auto` + a v2 /// family (`qwen3_coder`) is the only combination the gate routes to /// `tool_parser_v2::apply_stream`; `required`/`named` (above) always stay on the v1 @@ -1667,6 +1704,42 @@ async fn tool_choice_deepseek_v4_named_prompt_injected_bare_params_recovers() { ); } +/// MiniMax M3 + named tool_choice + `prompt_injected_reasoning=true` + bare +/// parameters object. Exercises the named SingleObject immediate-jail path. +#[tokio::test] +async fn tool_choice_minimax_m3_named_prompt_injected_bare_params_recovers() { + let bare_params = r#"{"location":"San Francisco"}"#; + let preprocessor = build_preprocessor(Some("minimax-m3"), Some("minimax-m3")); + let request = streaming_tool_request(ChatCompletionToolChoiceOption::Named( + "get_weather".to_string().into(), + )); + let input_stream = stream::iter( + vec![mock_content_chunk(bare_params), mock_final_chunk()] + .into_iter() + .map(Annotated::from_data), + ); + let output_stream = preprocessor + .postprocessor_parsing_stream(input_stream, &request, true, false) + .expect("postprocessor_parsing_stream should build"); + let DrainOutput { + reasoning, + content, + tool_calls, + finish_reasons, + } = drain_stream(output_stream).await; + + let case = "MiniMax M3 named + prompt_injected=true + bare params"; + assert!( + reasoning.is_empty(), + "{case}: guided JSON must not be classified as reasoning_content, got: {reasoning:?}" + ); + assert_clean_tool_call(case, &content, &tool_calls, "San Francisco"); + assert!( + finish_reasons.contains(&FinishReason::ToolCalls), + "{case}: expected ToolCalls finish_reason, got: {finish_reasons:?}" + ); +} + /// GLM + required + `prompt_injected_reasoning=true` + bare JSON. /// /// Mirrors the DeepSeek V4 guided-decoding failure shape for the `glm45`