From 7049a167fb2157da540edeaa68e9b1f1b2c791d6 Mon Sep 17 00:00:00 2001 From: Aniruddh Krovvidi <224641992+ankrovv@users.noreply.github.com> Date: Fri, 10 Jul 2026 19:20:33 -0700 Subject: [PATCH 1/4] fix(responses): accept reasoning effort none Signed-off-by: Aniruddh Krovvidi <224641992+ankrovv@users.noreply.github.com> --- crates/protocols/src/responses.rs | 13 +++++ crates/protocols/tests/responses.rs | 10 ++++ .../src/routers/grpc/harmony/builder.rs | 53 +++++++++++++------ .../grpc/regular/responses/conversions.rs | 20 +++++++ .../src/routers/openai/responses/route.rs | 20 ++++++- model_gateway/tests/api/api_endpoints_test.rs | 31 +++++++++++ 6 files changed, 130 insertions(+), 17 deletions(-) diff --git a/crates/protocols/src/responses.rs b/crates/protocols/src/responses.rs index 55463a3b7..b27fe367b 100644 --- a/crates/protocols/src/responses.rs +++ b/crates/protocols/src/responses.rs @@ -1531,12 +1531,25 @@ fn default_reasoning_effort() -> Option { #[derive(Debug, Clone, Deserialize, Serialize, schemars::JsonSchema)] #[serde(rename_all = "snake_case")] pub enum ReasoningEffort { + None, Minimal, Low, Medium, High, } +impl ReasoningEffort { + pub const fn as_str(&self) -> &'static str { + match self { + Self::None => "none", + Self::Minimal => "minimal", + Self::Low => "low", + Self::Medium => "medium", + Self::High => "high", + } + } +} + #[derive(Debug, Clone, Deserialize, Serialize, schemars::JsonSchema)] #[serde(rename_all = "snake_case")] pub enum ReasoningSummary { diff --git a/crates/protocols/tests/responses.rs b/crates/protocols/tests/responses.rs index c8cc6075c..5d4bf40a0 100644 --- a/crates/protocols/tests/responses.rs +++ b/crates/protocols/tests/responses.rs @@ -10,6 +10,16 @@ use openai_protocol::{ use serde_json::json; use validator::Validate; +#[test] +fn reasoning_effort_none_round_trips() { + let payload = json!({"effort": "none"}); + let reasoning: ResponseReasoningParam = + serde_json::from_value(payload.clone()).expect("reasoning effort none should deserialize"); + + assert!(matches!(reasoning.effort, Some(ReasoningEffort::None))); + assert_eq!(serde_json::to_value(reasoning).expect("serialize"), payload); +} + #[test] fn summary_text_content_round_trips_spec_shape() { // Spec: `summary: array of SummaryTextContent { text, type: "summary_text" }`. diff --git a/model_gateway/src/routers/grpc/harmony/builder.rs b/model_gateway/src/routers/grpc/harmony/builder.rs index f10efeba3..27c1f929e 100644 --- a/model_gateway/src/routers/grpc/harmony/builder.rs +++ b/model_gateway/src/routers/grpc/harmony/builder.rs @@ -415,28 +415,29 @@ impl HarmonyBuilder { self.build_system_message(reasoning_effort, has_tools) } - /// Build system message from ResponsesRequest + /// Convert Responses reasoning effort to Harmony's supported values. /// /// # Arguments /// * `request` - The ResponsesRequest - /// * `with_custom_tools` - Whether custom tools (beyond built-ins) are present - fn build_system_message_from_responses( - &self, + /// + /// Returns an error for `none`, which Harmony cannot represent. + fn reasoning_effort_from_responses( request: &ResponsesRequest, - with_custom_tools: bool, - ) -> HarmonyMessage { - let reasoning_effort = request + ) -> Result, String> { + request .reasoning .as_ref() .and_then(|r| r.effort.as_ref()) .map(|effort| match effort { - ResponsesReasoningEffort::High => ReasoningEffort::High, - ResponsesReasoningEffort::Medium => ReasoningEffort::Medium, - ResponsesReasoningEffort::Low => ReasoningEffort::Low, - ResponsesReasoningEffort::Minimal => ReasoningEffort::Low, - }); - - self.build_system_message(reasoning_effort, with_custom_tools) + ResponsesReasoningEffort::None => { + Err("reasoning.effort 'none' is not supported by Harmony models".to_string()) + } + ResponsesReasoningEffort::High => Ok(ReasoningEffort::High), + ResponsesReasoningEffort::Medium => Ok(ReasoningEffort::Medium), + ResponsesReasoningEffort::Low => Ok(ReasoningEffort::Low), + ResponsesReasoningEffort::Minimal => Ok(ReasoningEffort::Low), + }) + .transpose() } /// Build developer message with common logic @@ -535,6 +536,7 @@ impl HarmonyBuilder { request: &ResponsesRequest, ) -> Result, String> { let mut all_messages = Vec::new(); + let reasoning_effort = Self::reasoning_effort_from_responses(request)?; // Handle new vs continuing conversation if request.previous_response_id.is_none() { @@ -549,7 +551,7 @@ impl HarmonyBuilder { let with_custom_tools = has_custom_tools(&tool_types); // Add system message - let sys_msg = self.build_system_message_from_responses(request, with_custom_tools); + let sys_msg = self.build_system_message(reasoning_effort, with_custom_tools); all_messages.push(sys_msg); // Add developer message if we have custom tools or instructions @@ -1169,6 +1171,27 @@ mod tests { use super::*; + #[test] + fn responses_reasoning_effort_none_is_rejected_for_all_harmony_requests() { + for previous_response_id in [None, Some("resp_previous".to_string())] { + let request = ResponsesRequest { + input: ResponseInput::Text("Answer briefly".to_string()), + previous_response_id, + reasoning: Some(openai_protocol::responses::ResponseReasoningParam { + effort: Some(openai_protocol::responses::ReasoningEffort::None), + summary: None, + }), + ..Default::default() + }; + + let result = HarmonyBuilder::new().construct_input_messages_with_harmony(&request); + assert_eq!( + result.err().as_deref(), + Some("reasoning.effort 'none' is not supported by Harmony models") + ); + } + } + /// Invariant: `image_generation` must never be advertised as a /// gpt-oss native builtin tool. If a future change re-adds it, /// gpt-oss's behavior becomes undefined (hallucinated tool call diff --git a/model_gateway/src/routers/grpc/regular/responses/conversions.rs b/model_gateway/src/routers/grpc/regular/responses/conversions.rs index 08c92def4..4b8dd0ba5 100644 --- a/model_gateway/src/routers/grpc/regular/responses/conversions.rs +++ b/model_gateway/src/routers/grpc/regular/responses/conversions.rs @@ -240,6 +240,11 @@ pub(crate) fn responses_to_chat(req: &ResponsesRequest) -> Result Date: Fri, 3 Jul 2026 09:09:09 -0700 Subject: [PATCH 2/4] fix(grpc/chat): honor reasoning_effort "none"/"minimal" (disable thinking) reasoning_effort is forwarded into the chat-template kwargs, but templates that gate thinking on a boolean toggle and treat reasoning_effort only as a level (GLM-4.x/5, Qwen3; DeepSeek-V3.1, Kimi-K2.5) kept thinking on for reasoning_effort="none", and the reasoning parser still mis-attributed the reply to reasoning_content with empty content on the default client path. - Add `thinking_from_reasoning_effort` (none/minimal => off, per OpenAI) and `resolve_user_thinking` (explicit chat_template_kwargs toggle still wins). - Use it at every OpenAI-chat reasoning gate: non-streaming (`should_mark_reasoning_started` in processor.rs) and streaming (thinking_override in streaming.rs), plus `require_reasoning` in chat request building. (The /v1/messages path already derives thinking from ThinkingConfig.) - Inject the disable default into the template kwargs under BOTH toggle key names (`enable_thinking` and `thinking`) so GLM/Qwen and DeepSeek/Kimi templates are covered; explicit kwargs still override. Addresses review: downstream parser state (non-streaming + streaming) and the `thinking` key name for DeepSeek/Kimi templates. Verified on GLM-5.2-NVFP4 over smg gRPC (non-streaming): reasoning_effort="none" -> reasoning_content empty, answer in content; "high"/baseline unchanged; explicit enable_thinking=true overrides. Unit test added for the mapping. Note: "minimal" is treated as off (binary templates have no minimal tier), an approximation of OpenAI's "lowest effort". Signed-off-by: qywu --- .../src/routers/grpc/regular/processor.rs | 3 +- .../regular/stages/chat/request_building.rs | 3 +- .../src/routers/grpc/regular/streaming.rs | 3 +- .../src/routers/grpc/utils/chat_utils.rs | 14 +++++ model_gateway/src/routers/grpc/utils/mod.rs | 4 +- .../src/routers/grpc/utils/parsers.rs | 63 +++++++++++++++++++ 6 files changed, 86 insertions(+), 4 deletions(-) diff --git a/model_gateway/src/routers/grpc/regular/processor.rs b/model_gateway/src/routers/grpc/regular/processor.rs index 9cf5b3bb3..63e24fae7 100644 --- a/model_gateway/src/routers/grpc/regular/processor.rs +++ b/model_gateway/src/routers/grpc/regular/processor.rs @@ -107,8 +107,9 @@ impl ResponseProcessor { // If the template injected `` in the prefill (thinking toggle // is supported and effectively ON), start in reasoning mode. if utils::should_mark_reasoning_started( - utils::extract_thinking_from_kwargs( + utils::resolve_user_thinking( original_request.chat_template_kwargs.as_ref(), + original_request.reasoning_effort.as_deref(), tokenizer.as_ref(), ), tokenizer.as_ref(), diff --git a/model_gateway/src/routers/grpc/regular/stages/chat/request_building.rs b/model_gateway/src/routers/grpc/regular/stages/chat/request_building.rs index e854fde6d..cec1315f7 100644 --- a/model_gateway/src/routers/grpc/regular/stages/chat/request_building.rs +++ b/model_gateway/src/routers/grpc/regular/stages/chat/request_building.rs @@ -99,8 +99,9 @@ impl PipelineStage for ChatRequestBuildingStage { let require_reasoning = ctx.tokenizer_arc().is_some_and(|tokenizer| { utils::should_mark_reasoning_started( - utils::extract_thinking_from_kwargs( + utils::resolve_user_thinking( chat_request.chat_template_kwargs.as_ref(), + chat_request.reasoning_effort.as_deref(), tokenizer.as_ref(), ), tokenizer.as_ref(), diff --git a/model_gateway/src/routers/grpc/regular/streaming.rs b/model_gateway/src/routers/grpc/regular/streaming.rs index c3e736b8d..9d40ed014 100644 --- a/model_gateway/src/routers/grpc/regular/streaming.rs +++ b/model_gateway/src/routers/grpc/regular/streaming.rs @@ -281,8 +281,9 @@ impl StreamingProcessor { // the template injected `` in the prefill — parsers should start // in reasoning mode. let thinking_override = utils::should_mark_reasoning_started( - utils::extract_thinking_from_kwargs( + utils::resolve_user_thinking( original_request.chat_template_kwargs.as_ref(), + original_request.reasoning_effort.as_deref(), tokenizer.as_ref(), ), tokenizer.as_ref(), diff --git a/model_gateway/src/routers/grpc/utils/chat_utils.rs b/model_gateway/src/routers/grpc/utils/chat_utils.rs index b0a0dfd1a..5c5d30982 100644 --- a/model_gateway/src/routers/grpc/utils/chat_utils.rs +++ b/model_gateway/src/routers/grpc/utils/chat_utils.rs @@ -388,6 +388,20 @@ pub fn process_chat_messages( "reasoning_effort".to_string(), Value::String(reasoning_effort.clone()), ); + + // OpenAI semantics: reasoning_effort "none"/"minimal" means "do not + // produce reasoning". Many chat templates gate thinking on a boolean + // toggle and treat `reasoning_effort` only as a *level* (so "none" + // would otherwise still think). Translate the disable case to the + // thinking toggle = false. Templates use different key names + // (`enable_thinking` for GLM-4.x/5, Qwen3; `thinking` for + // DeepSeek-V3.1, Kimi-K2.5), so set both; the unused one is ignored. + // These are defaults only: an explicit chat_template_kwargs value + // (applied below) still wins. + if matches!(reasoning_effort.as_str(), "none" | "minimal") { + combined_template_kwargs.insert("enable_thinking".to_string(), Value::Bool(false)); + combined_template_kwargs.insert("thinking".to_string(), Value::Bool(false)); + } } // Add any additional template kwargs from request diff --git a/model_gateway/src/routers/grpc/utils/mod.rs b/model_gateway/src/routers/grpc/utils/mod.rs index 026de0265..cf25537ca 100644 --- a/model_gateway/src/routers/grpc/utils/mod.rs +++ b/model_gateway/src/routers/grpc/utils/mod.rs @@ -25,4 +25,6 @@ pub(crate) use parsers::{ }; // `pub` (not `pub(crate)`) so the Go bindings can reuse the gateway's reasoning // detection instead of duplicating it. -pub use parsers::{extract_thinking_from_kwargs, should_mark_reasoning_started}; +pub use parsers::{ + extract_thinking_from_kwargs, resolve_user_thinking, should_mark_reasoning_started, +}; diff --git a/model_gateway/src/routers/grpc/utils/parsers.rs b/model_gateway/src/routers/grpc/utils/parsers.rs index 1fde53b84..fd373fcf6 100644 --- a/model_gateway/src/routers/grpc/utils/parsers.rs +++ b/model_gateway/src/routers/grpc/utils/parsers.rs @@ -45,6 +45,40 @@ pub fn extract_thinking_from_kwargs( .and_then(|v| v.as_bool()) } +/// Map an OpenAI `reasoning_effort` value to a thinking preference. +/// +/// OpenAI semantics: `"none"`/`"minimal"` mean "do not produce reasoning", so +/// they map to thinking OFF (`Some(false)`). Level values (`"low"`/`"medium"`/ +/// `"high"`) don't toggle thinking on their own, so they return `None` (defer to +/// the template default / explicit thinking kwarg). +pub(crate) fn thinking_from_reasoning_effort(reasoning_effort: Option<&str>) -> Option { + match reasoning_effort { + Some("none") | Some("minimal") => Some(false), + _ => None, + } +} + +/// Precedence for the effective thinking preference: an explicit template +/// toggle (already extracted from kwargs) always wins; otherwise fall back to +/// the OpenAI `reasoning_effort` mapping. +fn resolve_thinking_pref(explicit: Option, reasoning_effort: Option<&str>) -> Option { + explicit.or_else(|| thinking_from_reasoning_effort(reasoning_effort)) +} + +/// Resolve the user's effective thinking preference, honoring an explicit +/// template thinking kwarg first (it always wins), then falling back to the +/// OpenAI `reasoning_effort` mapping. +pub fn resolve_user_thinking( + kwargs: Option<&std::collections::HashMap>, + reasoning_effort: Option<&str>, + tokenizer: &dyn Tokenizer, +) -> Option { + resolve_thinking_pref( + extract_thinking_from_kwargs(kwargs, tokenizer), + reasoning_effort, + ) +} + /// Check if a reasoning parser is available for the given model pub(crate) fn check_reasoning_parser_availability( reasoning_parser_factory: &ReasoningParserFactory, @@ -156,6 +190,35 @@ pub(crate) fn create_tool_parser( mod tests { use super::*; + #[test] + fn resolve_thinking_pref_explicit_kwarg_wins() { + // An explicit template toggle always wins over the reasoning_effort mapping. + assert_eq!(resolve_thinking_pref(Some(true), Some("none")), Some(true)); + assert_eq!( + resolve_thinking_pref(Some(false), Some("high")), + Some(false) + ); + // No explicit toggle -> fall back to the reasoning_effort mapping. + assert_eq!(resolve_thinking_pref(None, Some("none")), Some(false)); + assert_eq!(resolve_thinking_pref(None, Some("minimal")), Some(false)); + assert_eq!(resolve_thinking_pref(None, Some("high")), None); + assert_eq!(resolve_thinking_pref(None, None), None); + } + + #[test] + fn thinking_from_reasoning_effort_maps_disable_values() { + // "none"/"minimal" mean do-not-reason -> thinking OFF + assert_eq!(thinking_from_reasoning_effort(Some("none")), Some(false)); + assert_eq!(thinking_from_reasoning_effort(Some("minimal")), Some(false)); + // level values do not toggle thinking on their own + assert_eq!(thinking_from_reasoning_effort(Some("low")), None); + assert_eq!(thinking_from_reasoning_effort(Some("medium")), None); + assert_eq!(thinking_from_reasoning_effort(Some("high")), None); + // unspecified / unknown -> defer + assert_eq!(thinking_from_reasoning_effort(None), None); + assert_eq!(thinking_from_reasoning_effort(Some("bogus")), None); + } + #[test] fn create_reasoning_parser_returns_independent_instances() { let factory = ReasoningParserFactory::new(); From 956098849ec5dc252fef41ff764f9a9737268b54 Mon Sep 17 00:00:00 2001 From: Simo Lin <25425177+slin1237@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:59:32 -0700 Subject: [PATCH 3/4] refactor(grpc/chat): unify thinking resolution by layer Follow-up to #1876. reasoning_effort -> thinking was resolved entirely in the gateway via a blind dual-key write (enable_thinking + thinking), duplicated in message_utils. Split it by layer with one owner per concern: - openai-protocol: thinking_from_reasoning_effort() -- the OpenAI-protocol interpretation (none/minimal -> off); model-agnostic, documented, tested. - llm-tokenizer: ChatTemplateParams.thinking; ChatTemplateState::apply sets the template's own toggle key via thinking_key_name() (the single correct key, not both). An explicit template_kwargs entry still wins. - gateway: chat_utils and message_utils drop the dual-key writes and pass the resolved preference through ChatTemplateParams; parsers.rs delegates the reasoning_effort mapping to the protocol fn (removes the duplicated rule). - smg-golang: migrated to resolve_user_thinking so the Go path honors reasoning_effort; extract_thinking_from_kwargs demoted to pub(crate). Behavior is unchanged for target models; the dual-key guess and the duplicated none/minimal rule are gone. Unit tests added at the protocol and tokenizer layers, where the logic now lives. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com> --- bindings/golang/src/utils.rs | 8 ++- crates/protocols/src/chat.rs | 38 ++++++++++ crates/tokenizer/src/chat_template.rs | 71 +++++++++++++++++++ .../src/routers/grpc/utils/chat_utils.rs | 25 +++---- .../src/routers/grpc/utils/message_utils.rs | 34 +++------ model_gateway/src/routers/grpc/utils/mod.rs | 4 +- .../src/routers/grpc/utils/parsers.rs | 33 ++------- 7 files changed, 139 insertions(+), 74 deletions(-) diff --git a/bindings/golang/src/utils.rs b/bindings/golang/src/utils.rs index 7e299a828..3fdf623f3 100644 --- a/bindings/golang/src/utils.rs +++ b/bindings/golang/src/utils.rs @@ -2,7 +2,7 @@ use llm_tokenizer::traits::Tokenizer; use openai_protocol::chat::ChatCompletionRequest; -use smg::routers::grpc::utils::{extract_thinking_from_kwargs, should_mark_reasoning_started}; +use smg::routers::grpc::utils::{resolve_user_thinking, should_mark_reasoning_started}; use uuid::Uuid; /// Helper function to generate tool call ID (matches router implementation) @@ -32,7 +32,11 @@ pub(crate) fn chat_requires_reasoning( tokenizer: &dyn Tokenizer, ) -> bool { should_mark_reasoning_started( - extract_thinking_from_kwargs(request.chat_template_kwargs.as_ref(), tokenizer), + resolve_user_thinking( + request.chat_template_kwargs.as_ref(), + request.reasoning_effort.as_deref(), + tokenizer, + ), tokenizer, ) } diff --git a/crates/protocols/src/chat.rs b/crates/protocols/src/chat.rs index 9f98a7c5c..7d9b6148e 100644 --- a/crates/protocols/src/chat.rs +++ b/crates/protocols/src/chat.rs @@ -320,6 +320,25 @@ pub struct ChatCompletionRequest { pub other: Map, } +/// Map an OpenAI `reasoning_effort` to a thinking on/off preference. +/// +/// This is the protocol-level interpretation of "does the caller want +/// reasoning?" — independent of any model/template. `reasoning_effort` is a +/// *level* (`"low"`/`"medium"`/`"high"`) plus the vendor-extension `"none"`. +/// +/// Both `"none"` and `"minimal"` map to thinking OFF (`Some(false)`). +/// `"minimal"` is treated as an off-signal deliberately: templates that expose +/// only a boolean thinking toggle (GLM/Qwen3) cannot do "a little" reasoning, +/// so the lowest OpenAI level is the closest available "do not reason". +/// Level values return `None` — no opinion, defer to the template default or an +/// explicit thinking kwarg. +pub fn thinking_from_reasoning_effort(reasoning_effort: Option<&str>) -> Option { + match reasoning_effort { + Some("none") | Some("minimal") => Some(false), + _ => None, + } +} + // ============================================================================ // Validation Functions // ============================================================================ @@ -750,3 +769,22 @@ pub struct ChatStreamChoice { #[serde(skip_serializing_if = "Option::is_none")] pub matched_stop: Option, } + +#[cfg(test)] +mod tests { + use super::thinking_from_reasoning_effort; + + #[test] + fn thinking_from_reasoning_effort_maps_disable_values() { + // "none"/"minimal" mean do-not-reason -> thinking OFF. + assert_eq!(thinking_from_reasoning_effort(Some("none")), Some(false)); + assert_eq!(thinking_from_reasoning_effort(Some("minimal")), Some(false)); + // Level values do not toggle thinking on their own. + assert_eq!(thinking_from_reasoning_effort(Some("low")), None); + assert_eq!(thinking_from_reasoning_effort(Some("medium")), None); + assert_eq!(thinking_from_reasoning_effort(Some("high")), None); + // Unspecified / unknown -> defer. + assert_eq!(thinking_from_reasoning_effort(None), None); + assert_eq!(thinking_from_reasoning_effort(Some("bogus")), None); + } +} diff --git a/crates/tokenizer/src/chat_template.rs b/crates/tokenizer/src/chat_template.rs index fb25f6925..1a3fd0125 100644 --- a/crates/tokenizer/src/chat_template.rs +++ b/crates/tokenizer/src/chat_template.rs @@ -48,6 +48,16 @@ pub enum ThinkingKeyName { Thinking, } +impl ThinkingKeyName { + /// The template kwarg name this toggle uses. + pub fn as_kwarg(self) -> &'static str { + match self { + ThinkingKeyName::EnableThinking => "enable_thinking", + ThinkingKeyName::Thinking => "thinking", + } + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub enum ThinkingToggle { /// Template has no thinking toggle. The model either always reasons @@ -466,6 +476,11 @@ pub struct ChatTemplateParams<'a> { /// Special tokens to inject into the template context. /// Many templates reference `{{ bos_token }}`, `{{ eos_token }}`, etc. pub special_tokens: Option<&'a crate::traits::SpecialTokens>, + /// Resolved thinking preference. When `Some`, `apply` sets the template's + /// own thinking-toggle key (`enable_thinking`/`thinking`, per detection) to + /// this value as a default. An explicit `template_kwargs` entry for that + /// key still wins. + pub thinking: Option, } /// JSON separator pair passed through HuggingFace's `tojson` filter. @@ -1064,6 +1079,23 @@ impl ChatTemplateState { https://huggingface.co/docs/transformers/main/en/chat_templating", ) })?; + + // Apply the resolved thinking preference under the template's own toggle + // key (`enable_thinking` vs `thinking`, per detection). Injected as a + // default: an explicit `template_kwargs` entry for that key still wins. + if let (Some(thinking), Some(key)) = (params.thinking, self.thinking_key_name) { + let mut kwargs = params.template_kwargs.cloned().unwrap_or_default(); + kwargs + .entry(key.as_kwarg().to_string()) + .or_insert(serde_json::Value::Bool(thinking)); + let params = ChatTemplateParams { + template_kwargs: Some(&kwargs), + thinking: None, + ..params + }; + return render_chat_template(env, messages, params); + } + render_chat_template(env, messages, params) } @@ -1190,4 +1222,43 @@ mod tests { assert_eq!(result, "hello"); } + + #[test] + fn thinking_param_sets_template_key_and_explicit_wins() { + use std::collections::HashMap; + + // Template echoes the enable_thinking value so we can observe what was set. + let state = ChatTemplateState::new(Some("{{ enable_thinking }}".to_string())).unwrap(); + assert_eq!( + state.thinking_key_name(), + Some(ThinkingKeyName::EnableThinking) + ); + + // thinking = Some(false) injects enable_thinking=false under the model's key. + let out = state + .apply( + &[], + ChatTemplateParams { + thinking: Some(false), + ..Default::default() + }, + ) + .unwrap(); + assert_eq!(out, "false"); + + // An explicit template_kwargs entry overrides the injected default. + let mut kwargs: HashMap = HashMap::new(); + kwargs.insert("enable_thinking".to_string(), serde_json::Value::Bool(true)); + let out = state + .apply( + &[], + ChatTemplateParams { + thinking: Some(false), + template_kwargs: Some(&kwargs), + ..Default::default() + }, + ) + .unwrap(); + assert_eq!(out, "true"); + } } diff --git a/model_gateway/src/routers/grpc/utils/chat_utils.rs b/model_gateway/src/routers/grpc/utils/chat_utils.rs index 5c5d30982..1bfdd3512 100644 --- a/model_gateway/src/routers/grpc/utils/chat_utils.rs +++ b/model_gateway/src/routers/grpc/utils/chat_utils.rs @@ -382,26 +382,15 @@ pub fn process_chat_messages( let kwargs_capacity = 1 + request.chat_template_kwargs.as_ref().map_or(0, |k| k.len()); let mut combined_template_kwargs = HashMap::with_capacity(kwargs_capacity); - // Add reasoning_effort if present (like Python does) + // Add reasoning_effort if present (like Python does): some templates read + // it as a *level*. The thinking on/off projection is applied separately + // via `ChatTemplateParams.thinking` below, so the tokenizer sets the + // model's own toggle key. if let Some(reasoning_effort) = &request.reasoning_effort { combined_template_kwargs.insert( "reasoning_effort".to_string(), Value::String(reasoning_effort.clone()), ); - - // OpenAI semantics: reasoning_effort "none"/"minimal" means "do not - // produce reasoning". Many chat templates gate thinking on a boolean - // toggle and treat `reasoning_effort` only as a *level* (so "none" - // would otherwise still think). Translate the disable case to the - // thinking toggle = false. Templates use different key names - // (`enable_thinking` for GLM-4.x/5, Qwen3; `thinking` for - // DeepSeek-V3.1, Kimi-K2.5), so set both; the unused one is ignored. - // These are defaults only: an explicit chat_template_kwargs value - // (applied below) still wins. - if matches!(reasoning_effort.as_str(), "none" | "minimal") { - combined_template_kwargs.insert("enable_thinking".to_string(), Value::Bool(false)); - combined_template_kwargs.insert("thinking".to_string(), Value::Bool(false)); - } } // Add any additional template kwargs from request @@ -421,6 +410,12 @@ pub fn process_chat_messages( add_generation_prompt: true, tools: tools_json.as_deref(), template_kwargs: final_template_kwargs, + // Project OpenAI `reasoning_effort` (none/minimal) onto the model's + // thinking toggle; the tokenizer applies it under the correct key. + // An explicit chat_template_kwargs toggle still wins (in apply). + thinking: openai_protocol::chat::thinking_from_reasoning_effort( + request.reasoning_effort.as_deref(), + ), ..Default::default() }; diff --git a/model_gateway/src/routers/grpc/utils/message_utils.rs b/model_gateway/src/routers/grpc/utils/message_utils.rs index 2367e6fbd..18b424767 100644 --- a/model_gateway/src/routers/grpc/utils/message_utils.rs +++ b/model_gateway/src/routers/grpc/utils/message_utils.rs @@ -5,8 +5,6 @@ //! instead of `ChatCompletionRequest` / `ChatMessage`. #![allow(dead_code)] // wired in follow-up PR (pipeline factory) -use std::collections::HashMap; - use llm_tokenizer::{ chat_template::{ChatTemplateContentFormat, ChatTemplateParams}, traits::Tokenizer, @@ -74,35 +72,21 @@ pub fn process_messages( .transpose() .map_err(|e| format!("Failed to serialize tools: {e}"))?; - // Step 5: Build template kwargs from ThinkingConfig - let mut combined_template_kwargs = HashMap::new(); - - // Pass both `enable_thinking` (Qwen3) and `thinking` (Kimi-K2.5) since - // different model templates use different kwarg names for the same concept. - // Adaptive mode is treated as "thinking on"; the model decides whether to actually emit it. - match &request.thinking { - Some(ThinkingConfig::Enabled { .. } | ThinkingConfig::Adaptive { .. }) => { - combined_template_kwargs.insert("enable_thinking".to_string(), json!(true)); - combined_template_kwargs.insert("thinking".to_string(), json!(true)); - } - Some(ThinkingConfig::Disabled) => { - combined_template_kwargs.insert("enable_thinking".to_string(), json!(false)); - combined_template_kwargs.insert("thinking".to_string(), json!(false)); - } - None => {} // Let template use its default behavior - } - - let final_template_kwargs = if combined_template_kwargs.is_empty() { - None - } else { - Some(&combined_template_kwargs) + // Step 5: Project the Anthropic ThinkingConfig onto a thinking on/off + // preference. Adaptive is treated as "thinking on"; the model decides + // whether to actually emit it. The tokenizer applies this under the model's + // own toggle key (`enable_thinking`/`thinking`) in `apply`. + let thinking = match &request.thinking { + Some(ThinkingConfig::Enabled { .. } | ThinkingConfig::Adaptive { .. }) => Some(true), + Some(ThinkingConfig::Disabled) => Some(false), + None => None, // Let template use its default behavior }; // Step 6: Apply chat template let params = ChatTemplateParams { add_generation_prompt: true, tools: tools_json.as_deref(), - template_kwargs: final_template_kwargs, + thinking, ..Default::default() }; diff --git a/model_gateway/src/routers/grpc/utils/mod.rs b/model_gateway/src/routers/grpc/utils/mod.rs index cf25537ca..68ec9d50f 100644 --- a/model_gateway/src/routers/grpc/utils/mod.rs +++ b/model_gateway/src/routers/grpc/utils/mod.rs @@ -25,6 +25,4 @@ pub(crate) use parsers::{ }; // `pub` (not `pub(crate)`) so the Go bindings can reuse the gateway's reasoning // detection instead of duplicating it. -pub use parsers::{ - extract_thinking_from_kwargs, resolve_user_thinking, should_mark_reasoning_started, -}; +pub use parsers::{resolve_user_thinking, should_mark_reasoning_started}; diff --git a/model_gateway/src/routers/grpc/utils/parsers.rs b/model_gateway/src/routers/grpc/utils/parsers.rs index fd373fcf6..a8ca3bc19 100644 --- a/model_gateway/src/routers/grpc/utils/parsers.rs +++ b/model_gateway/src/routers/grpc/utils/parsers.rs @@ -4,6 +4,7 @@ use llm_tokenizer::{ chat_template::{ThinkingKeyName, ThinkingToggle}, traits::Tokenizer, }; +use openai_protocol::chat::thinking_from_reasoning_effort; use reasoning_parser::{ParserFactory as ReasoningParserFactory, ReasoningParser}; use serde_json::Value; use tool_parser::{ @@ -32,7 +33,7 @@ pub fn should_mark_reasoning_started( /// Only checks the key that the template actually uses (e.g. `enable_thinking` /// for Qwen3, `thinking` for Kimi-K2.5). This prevents mismatches where the /// user passes the wrong key name and the template ignores it. -pub fn extract_thinking_from_kwargs( +pub(crate) fn extract_thinking_from_kwargs( kwargs: Option<&std::collections::HashMap>, tokenizer: &dyn Tokenizer, ) -> Option { @@ -45,22 +46,10 @@ pub fn extract_thinking_from_kwargs( .and_then(|v| v.as_bool()) } -/// Map an OpenAI `reasoning_effort` value to a thinking preference. -/// -/// OpenAI semantics: `"none"`/`"minimal"` mean "do not produce reasoning", so -/// they map to thinking OFF (`Some(false)`). Level values (`"low"`/`"medium"`/ -/// `"high"`) don't toggle thinking on their own, so they return `None` (defer to -/// the template default / explicit thinking kwarg). -pub(crate) fn thinking_from_reasoning_effort(reasoning_effort: Option<&str>) -> Option { - match reasoning_effort { - Some("none") | Some("minimal") => Some(false), - _ => None, - } -} - /// Precedence for the effective thinking preference: an explicit template /// toggle (already extracted from kwargs) always wins; otherwise fall back to -/// the OpenAI `reasoning_effort` mapping. +/// the protocol-level OpenAI `reasoning_effort` mapping +/// ([`thinking_from_reasoning_effort`]). fn resolve_thinking_pref(explicit: Option, reasoning_effort: Option<&str>) -> Option { explicit.or_else(|| thinking_from_reasoning_effort(reasoning_effort)) } @@ -205,20 +194,6 @@ mod tests { assert_eq!(resolve_thinking_pref(None, None), None); } - #[test] - fn thinking_from_reasoning_effort_maps_disable_values() { - // "none"/"minimal" mean do-not-reason -> thinking OFF - assert_eq!(thinking_from_reasoning_effort(Some("none")), Some(false)); - assert_eq!(thinking_from_reasoning_effort(Some("minimal")), Some(false)); - // level values do not toggle thinking on their own - assert_eq!(thinking_from_reasoning_effort(Some("low")), None); - assert_eq!(thinking_from_reasoning_effort(Some("medium")), None); - assert_eq!(thinking_from_reasoning_effort(Some("high")), None); - // unspecified / unknown -> defer - assert_eq!(thinking_from_reasoning_effort(None), None); - assert_eq!(thinking_from_reasoning_effort(Some("bogus")), None); - } - #[test] fn create_reasoning_parser_returns_independent_instances() { let factory = ReasoningParserFactory::new(); From 94dfe2e98da4c12ac9607482f45f7102dcb28d9e Mon Sep 17 00:00:00 2001 From: Simo Lin <25425177+slin1237@users.noreply.github.com> Date: Fri, 10 Jul 2026 20:10:31 -0700 Subject: [PATCH 4/4] fix(tokenizer): apply resolved thinking for DeepSeek V3.2/V4 native encoders Preserve explicit template-kwargs precedence while falling back to the resolved thinking setting for native DeepSeek encoders. Avoid cloning template kwargs when the toggle is already explicit. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com> --- crates/tokenizer/src/chat_template.rs | 29 ++++++----- crates/tokenizer/src/huggingface.rs | 71 +++++++++++++++++++++++++-- 2 files changed, 85 insertions(+), 15 deletions(-) diff --git a/crates/tokenizer/src/chat_template.rs b/crates/tokenizer/src/chat_template.rs index 1a3fd0125..241e625ca 100644 --- a/crates/tokenizer/src/chat_template.rs +++ b/crates/tokenizer/src/chat_template.rs @@ -1081,19 +1081,24 @@ impl ChatTemplateState { })?; // Apply the resolved thinking preference under the template's own toggle - // key (`enable_thinking` vs `thinking`, per detection). Injected as a - // default: an explicit `template_kwargs` entry for that key still wins. + // key (`enable_thinking` vs `thinking`, per detection). Skip entirely + // (no clone) when the caller already set that key explicitly — the + // explicit value wins. if let (Some(thinking), Some(key)) = (params.thinking, self.thinking_key_name) { - let mut kwargs = params.template_kwargs.cloned().unwrap_or_default(); - kwargs - .entry(key.as_kwarg().to_string()) - .or_insert(serde_json::Value::Bool(thinking)); - let params = ChatTemplateParams { - template_kwargs: Some(&kwargs), - thinking: None, - ..params - }; - return render_chat_template(env, messages, params); + let kwarg_key = key.as_kwarg(); + if params + .template_kwargs + .is_none_or(|k| !k.contains_key(kwarg_key)) + { + let mut kwargs = params.template_kwargs.cloned().unwrap_or_default(); + kwargs.insert(kwarg_key.to_string(), serde_json::Value::Bool(thinking)); + let params = ChatTemplateParams { + template_kwargs: Some(&kwargs), + thinking: None, + ..params + }; + return render_chat_template(env, messages, params); + } } render_chat_template(env, messages, params) diff --git a/crates/tokenizer/src/huggingface.rs b/crates/tokenizer/src/huggingface.rs index e13ca1b00..df3aaf182 100644 --- a/crates/tokenizer/src/huggingface.rs +++ b/crates/tokenizer/src/huggingface.rs @@ -486,14 +486,18 @@ fn detect_renderer_from_config(dir: &std::path::Path) -> Renderer { // --------------------------------------------------------------------------- // DeepSeek V3.2 / V4 dispatch shims // --------------------------------------------------------------------------- -/// Derive the V3.2 / V4 thinking mode from `template_kwargs`. Only the -/// `thinking` key is honored, matching sglang's DeepSeek serving path and -/// the `ThinkingKeyName::Thinking` contract reported by this tokenizer. +/// Derive the V3.2 / V4 thinking mode. These native encoders bypass +/// `ChatTemplateState::apply`, so this is where the resolved thinking preference +/// is consumed. An explicit `template_kwargs["thinking"]` wins; otherwise fall +/// back to `params.thinking` (resolved from `reasoning_effort` / Anthropic +/// `ThinkingConfig`) — same precedence as the Jinja path. Default off, matching +/// the `ThinkingKeyName::Thinking` / `DefaultOff` contract reported here. fn derive_thinking_mode(params: &ChatTemplateParams) -> deepseek_v32::ThinkingMode { let enabled = params .template_kwargs .and_then(|k| k.get("thinking")) .and_then(serde_json::Value::as_bool) + .or(params.thinking) .unwrap_or(false); if enabled { deepseek_v32::ThinkingMode::Thinking @@ -578,3 +582,64 @@ fn apply_deepseek_v4( deepseek_v4::encode_messages(msgs, thinking_mode, &encode_params) .map_err(|e| Error::msg(format!("DeepSeek V4 encode failed: {e}"))) } + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use super::derive_thinking_mode; + use crate::{chat_template::ChatTemplateParams, encoders::deepseek_v32::ThinkingMode}; + + fn thinking_kwargs(value: bool) -> HashMap { + HashMap::from([("thinking".to_string(), serde_json::Value::Bool(value))]) + } + + // Regression: DeepSeek V3.2/V4 bypass ChatTemplateState::apply, so the + // resolved `params.thinking` (from reasoning_effort / Anthropic ThinkingConfig) + // must be honored here — with an explicit `template_kwargs["thinking"]` still + // winning. Same precedence as the Jinja path. + #[test] + fn derive_thinking_mode_honors_params_thinking_and_explicit_override() { + // No signal at all -> Chat (default off). + assert!(matches!( + derive_thinking_mode(&ChatTemplateParams::default()), + ThinkingMode::Chat + )); + + // params.thinking is the fallback when there is no explicit kwarg. + assert!(matches!( + derive_thinking_mode(&ChatTemplateParams { + thinking: Some(true), + ..Default::default() + }), + ThinkingMode::Thinking + )); + assert!(matches!( + derive_thinking_mode(&ChatTemplateParams { + thinking: Some(false), + ..Default::default() + }), + ThinkingMode::Chat + )); + + // An explicit template_kwargs["thinking"] wins over params.thinking. + let on = thinking_kwargs(true); + assert!(matches!( + derive_thinking_mode(&ChatTemplateParams { + thinking: Some(false), + template_kwargs: Some(&on), + ..Default::default() + }), + ThinkingMode::Thinking + )); + let off = thinking_kwargs(false); + assert!(matches!( + derive_thinking_mode(&ChatTemplateParams { + thinking: Some(true), + template_kwargs: Some(&off), + ..Default::default() + }), + ThinkingMode::Chat + )); + } +}