From 9fd7c9f7dbd390fdfd11efb62d44085d19afbf46 Mon Sep 17 00:00:00 2001 From: Keiven Chang Date: Tue, 28 Apr 2026 10:41:38 -0700 Subject: [PATCH] feat(v4): cherry-pick #8670 onto release/deepseekv4 Squashed cherry-pick of: - chore: DeepSeek V4 hardening follow-up (Ayush Agarwal) - fix(sglang): pass parallel_tool_calls to FunctionCallParser path Original PR: #8670 Signed-off-by: Keiven Chang --- lib/llm/src/preprocessor.rs | 28 +- lib/llm/src/preprocessor/prompt.rs | 20 + .../src/preprocessor/prompt/deepseek_v4.rs | 979 +++++++++--------- .../chat_completion_stream_special_chars.json | 2 +- lib/parsers/src/reasoning/mod.rs | 29 +- lib/parsers/src/tool_calling/config.rs | 79 +- lib/parsers/src/tool_calling/dsml/parser.rs | 449 +++++--- lib/parsers/src/tool_calling/parsers.rs | 2 - 8 files changed, 923 insertions(+), 665 deletions(-) diff --git a/lib/llm/src/preprocessor.rs b/lib/llm/src/preprocessor.rs index 851c143f9e24..8eaca1378088 100644 --- a/lib/llm/src/preprocessor.rs +++ b/lib/llm/src/preprocessor.rs @@ -1260,13 +1260,15 @@ impl OpenAIPreprocessor { } Some("deepseek_r1") | Some("deepseek_v4") | Some("deepseek-v4") | Some("deepseekv4") => { - if let Some(args) = chat_template_args { - if let Some(thinking) = args.get("thinking") { - return thinking == &serde_json::Value::Bool(false); - } - if let Some(mode) = args.get("thinking_mode").and_then(|v| v.as_str()) { - return mode == "chat"; - } + if let Some(enabled) = + crate::preprocessor::prompt::thinking_bool_from_args(chat_template_args) + { + return !enabled; + } + if let Some(args) = chat_template_args + && let Some(mode) = args.get("thinking_mode").and_then(|v| v.as_str()) + { + return mode == "chat"; } false } @@ -1875,6 +1877,18 @@ mod tests { true, "deepseekv4 (joined alias) + thinking_mode=chat → disabled", ), + ( + Some("deepseek_v4"), + Some(&enable_thinking_false), + true, + "deepseek_v4 + enable_thinking=false → disabled (vLLM alias)", + ), + ( + Some("deepseek_v4"), + Some(&enable_thinking_true), + false, + "deepseek_v4 + enable_thinking=true → enabled (vLLM alias)", + ), ]; for (parser, args, expected, desc) in cases { diff --git a/lib/llm/src/preprocessor/prompt.rs b/lib/llm/src/preprocessor/prompt.rs index 8b1f5fde437e..b101db169382 100644 --- a/lib/llm/src/preprocessor/prompt.rs +++ b/lib/llm/src/preprocessor/prompt.rs @@ -31,6 +31,26 @@ mod template; pub use template::{ChatTemplate, ContextMixins}; +/// Shared helper: extract a boolean thinking toggle from `chat_template_args`. +/// +/// Reads the two equivalent keys (`thinking`, `enable_thinking` — vLLM's +/// canonical kwarg) in order and returns the first bool value found, or `None` +/// if neither key is present (or neither carries a bool). Used by the V4 +/// formatter's `resolve_thinking_mode` and by the reasoning-parser gate in +/// `OpenAIPreprocessor::is_reasoning_disabled_by_request` so both paths agree +/// on the signal interpretation. +pub(crate) fn thinking_bool_from_args( + args: Option<&HashMap>, +) -> Option { + let args = args?; + for key in ["thinking", "enable_thinking"] { + if let Some(v) = args.get(key).and_then(|x| x.as_bool()) { + return Some(v); + } + } + None +} + #[derive(Debug)] pub enum TokenInput { Single(Vec), diff --git a/lib/llm/src/preprocessor/prompt/deepseek_v4.rs b/lib/llm/src/preprocessor/prompt/deepseek_v4.rs index 37c534ed3ef6..199150245002 100644 --- a/lib/llm/src/preprocessor/prompt/deepseek_v4.rs +++ b/lib/llm/src/preprocessor/prompt/deepseek_v4.rs @@ -30,35 +30,37 @@ pub mod tokens { pub const TASK_READ_URL: &str = "<|read_url|>"; } -/// DSML outer block name for tool-call groups: `<|DSML|tool_calls>...`. const TOOL_CALLS_BLOCK_NAME: &str = "tool_calls"; -/// Wire-format tags that wrap a tool result inside a user content block. -const TOOL_RESULT_OPEN: &str = ""; -const TOOL_RESULT_CLOSE: &str = ""; - -/// Preamble line that introduces a response-format schema in both the -/// system and developer roles. The `{}` placeholder is filled by the -/// caller with the JSON-serialized schema. -const RESPONSE_FORMAT_PREAMBLE: &str = - "## Response Format:\n\nYou MUST strictly adhere to the following schema to reply:\n{}"; - -/// Roles whose messages are always kept by `drop_thinking_messages`. All other -/// roles before the last user/developer message are dropped (assistant turns -/// keep their non-`reasoning_content` fields; everything else is removed). -const KEEP_ROLES: &[&str] = &[ - "user", - "system", - "tool", - "latest_reminder", - "direct_search_results", -]; - -/// Placeholder the Python reference inserts when it can't render an -/// unsupported content-block or tool-result item type. Rendered into the -/// assistant-visible prompt so a human can tell something was skipped; -/// dynamo additionally emits a `tracing::warn!` so ops see it too. -const UNSUPPORTED_PLACEHOLDER_FMT: &str = "[Unsupported {}]"; +const RESPONSE_FORMAT_TEMPLATE: &str = + "## Response Format:\n\nYou MUST strictly adhere to the following schema to reply:\n{schema}"; + +const TOOLS_TEMPLATE: &str = r#"## Tools + +You have access to a set of tools to help answer the user's question. You can invoke tools by writing a "<{dsml_token}tool_calls>" block like the following: + +<{dsml_token}tool_calls> +<{dsml_token}invoke name="$TOOL_NAME"> +<{dsml_token}parameter name="$PARAMETER_NAME" string="true|false">$PARAMETER_VALUE +... + +<{dsml_token}invoke name="$TOOL_NAME2"> +... + + + +String parameters should be specified as is and set `string="true"`. For all other types (numbers, booleans, arrays, objects), pass the value in JSON format and set `string="false"`. + +If thinking_mode is enabled (triggered by {thinking_start_token}), you MUST output your complete reasoning inside {thinking_start_token}...{thinking_end_token} BEFORE any tool calls or final response. + +Otherwise, output directly after {thinking_end_token} with tool calls or final response. + +### Available Tool Schemas + +{tool_schemas} + +You MUST strictly follow the above defined tool name and parameter schemas to invoke tool calls. +"#; const REASONING_EFFORT_MAX: &str = "Reasoning Effort: Absolute maximum with no shortcuts permitted.\nYou MUST be very thorough in your thinking and comprehensively decompose the problem to resolve the root cause, rigorously stress-testing your logic against all potential paths, edge cases, and adversarial scenarios.\nExplicitly write out your entire deliberation process, documenting every intermediate step, considered alternative, and rejected hypothesis to ensure absolutely no assumption is left unchecked.\n\n"; @@ -70,9 +72,6 @@ pub enum ThinkingMode { } impl ThinkingMode { - /// Tiny branch-free mapping to a static string. `#[inline]` because this - /// is called from inside the per-message render hot path. - #[inline] pub fn as_str(&self) -> &'static str { match self { ThinkingMode::Chat => "chat", @@ -91,11 +90,7 @@ pub enum ReasoningEffort { /// Serialize a JSON value to match Python's `json.dumps(ensure_ascii=False)` spacing. /// Python's default separators are `(', ', ': ')`; we use a custom `Formatter` /// so escape sequences inside strings can't confuse state tracking. -/// -/// Returns `Result` so serialization / UTF-8 errors propagate to the caller -/// rather than silently collapsing to `"{}"` (the old error-swallow behavior -/// dropped the entire request payload with no signal up the stack). -fn to_json(value: &JsonValue) -> Result { +fn to_json(value: &JsonValue) -> String { use serde::Serialize; use serde_json::ser::Formatter; use std::io; @@ -132,21 +127,16 @@ fn to_json(value: &JsonValue) -> Result { } } - // Size the buffer from a compact pre-serialization. Python-style spacing - // adds exactly one byte per structural separator (`,` → `, `, `:` → `: `), - // which bounds the final length by ~1.125× the compact length. This keeps - // small payloads cheap and avoids the 5–9 reallocations the prior fixed - // 64-byte hint forced on KB-sized tool schemas and response formats. - let compact_len = serde_json::to_string(value) - .context("to_json: compact pre-serialization failed")? - .len(); - let capacity = compact_len.saturating_add(compact_len / 8).max(256); - let mut buf = Vec::with_capacity(capacity); + let mut buf = Vec::with_capacity(64); let mut ser = serde_json::Serializer::with_formatter(&mut buf, PythonFormatter); - value - .serialize(&mut ser) - .context("to_json: Python-formatter serialization failed")?; - String::from_utf8(buf).context("to_json: serialized output is not valid UTF-8") + if let Err(e) = value.serialize(&mut ser) { + tracing::warn!(error = %e, "to_json: serialize failed; falling back to empty object"); + return "{}".to_string(); + } + String::from_utf8(buf).unwrap_or_else(|e| { + tracing::warn!(error = %e, "to_json: serialized output not valid UTF-8; falling back to empty object"); + "{}".to_string() + }) } /// Extract function definitions from OpenAI-format tool list. @@ -158,51 +148,17 @@ fn tools_from_openai_format(tools: &[JsonValue]) -> Vec { } /// Render tool schemas into the system prompt format. -/// -/// Previously built via four sequential `String::replace` passes over a -/// `{placeholder}`-based template, which allocated a fresh copy of the whole -/// (schema-inlined, potentially kB-scale) string on each substitution. A -/// single `format!` with named arguments collapses that to one allocation -/// sized from the final length. -fn render_tools(tools: &[JsonValue]) -> Result { +fn render_tools(tools: &[JsonValue]) -> String { let tools_json: Vec = tools_from_openai_format(tools) .iter() .map(to_json) - .collect::>()?; - let schemas = tools_json.join("\n"); - - Ok(format!( - r#"## Tools - -You have access to a set of tools to help answer the user's question. You can invoke tools by writing a "<{dsml}tool_calls>" block like the following: - -<{dsml}tool_calls> -<{dsml}invoke name="$TOOL_NAME"> -<{dsml}parameter name="$PARAMETER_NAME" string="true|false">$PARAMETER_VALUE -... - -<{dsml}invoke name="$TOOL_NAME2"> -... - - - -String parameters should be specified as is and set `string="true"`. For all other types (numbers, booleans, arrays, objects), pass the value in JSON format and set `string="false"`. - -If thinking_mode is enabled (triggered by {think_open}), you MUST output your complete reasoning inside {think_open}...{think_close} BEFORE any tool calls or final response. - -Otherwise, output directly after {think_close} with tool calls or final response. - -### Available Tool Schemas - -{schemas} + .collect(); -You MUST strictly follow the above defined tool name and parameter schemas to invoke tool calls. -"#, - dsml = tokens::DSML_TOKEN, - think_open = tokens::THINKING_START, - think_close = tokens::THINKING_END, - schemas = schemas, - )) + TOOLS_TEMPLATE + .replace("{tool_schemas}", &tools_json.join("\n")) + .replace("{dsml_token}", tokens::DSML_TOKEN) + .replace("{thinking_start_token}", tokens::THINKING_START) + .replace("{thinking_end_token}", tokens::THINKING_END) } /// Find the index of the last user/developer message. @@ -217,18 +173,16 @@ fn find_last_user_index(messages: &[JsonValue]) -> Option { .rev() .find(|(_, msg)| { msg.get("role") - .and_then(JsonValue::as_str) - .is_some_and(|r| matches!(r, "user" | "developer")) + .and_then(|r| r.as_str()) + .map(|r| r == "user" || r == "developer") + .unwrap_or(false) }) .map(|(idx, _)| idx) } /// Extract visible text from OpenAI-style message content. -/// -/// Returns `Result` because the `_ => to_json(content)` fallback can now fail -/// (to_json itself returns `Result`). Callers already sit in `Result` context. -fn extract_visible_text(content: &JsonValue) -> Result { - Ok(match content { +fn extract_visible_text(content: &JsonValue) -> String { + match content { JsonValue::String(text) => text.clone(), JsonValue::Array(items) => items .iter() @@ -236,11 +190,11 @@ fn extract_visible_text(content: &JsonValue) -> Result { if let Some(text) = item.as_str() { return Some(text.to_string()); } - let item_type = item.get("type").and_then(JsonValue::as_str); + let item_type = item.get("type").and_then(|v| v.as_str()); if item_type == Some("text") { return item .get("text") - .and_then(JsonValue::as_str) + .and_then(|v| v.as_str()) .map(|text| text.to_string()); } tracing::warn!( @@ -250,12 +204,12 @@ fn extract_visible_text(content: &JsonValue) -> Result { None }) .collect::(), - _ => to_json(content)?, - }) + _ => to_json(content), + } } /// Normalize message `content` fields for text-only DeepSeek V4 rendering. -fn normalize_message_contents(messages: &mut [JsonValue]) -> Result<()> { +fn normalize_message_contents(messages: &mut [JsonValue]) { for msg in messages { let Some(content) = msg.get("content") else { continue; @@ -264,19 +218,18 @@ fn normalize_message_contents(messages: &mut [JsonValue]) -> Result<()> { if !content.is_string() && !content.is_array() { continue; } - let normalized = extract_visible_text(content)?; + let normalized = extract_visible_text(content); if let Some(obj) = msg.as_object_mut() { obj.insert("content".to_string(), JsonValue::String(normalized)); } } - Ok(()) } /// Encode tool call arguments into DSML parameter format. fn encode_arguments_to_dsml(tool_call: &JsonValue) -> Result { let arguments_str = tool_call .get("arguments") - .and_then(JsonValue::as_str) + .and_then(|a| a.as_str()) .context("Missing or invalid 'arguments' field")?; // Python falls back to `{"arguments": raw_string}` on parse failure. @@ -291,12 +244,11 @@ fn encode_arguments_to_dsml(tool_call: &JsonValue) -> Result { let mut params = Vec::new(); for (key, value) in arguments_obj { - // Dispatch on the concrete variant so we don't do the `is_string` / `as_str` - // / `unwrap` dance (unwrap is technically safe after is_string but fragile - // against future refactors). - let (is_string, value_str) = match value { - JsonValue::String(s) => (true, s.clone()), - _ => (false, to_json(value)?), + let is_string = value.is_string(); + let value_str = if is_string { + value.as_str().unwrap().to_string() + } else { + to_json(value) }; params.push(format!( "<{}parameter name=\"{}\" string=\"{}\">{}", @@ -312,11 +264,6 @@ fn encode_arguments_to_dsml(tool_call: &JsonValue) -> Result { } /// Lookup the task token for a quick-instruction task. -/// -/// Called once per assistant-turn in `render_message` and once per transition -/// token lookup; `#[inline]` because the static-str match is smaller than the -/// call overhead. -#[inline] fn task_token(task: &str) -> Option<&'static str> { match task { "action" => Some(tokens::TASK_ACTION), @@ -329,177 +276,6 @@ fn task_token(task: &str) -> Option<&'static str> { } } -/// Append the "Response Format" schema block to `prompt` if the message has one. -fn append_response_format(msg: &JsonValue, prompt: &mut String) -> Result<()> { - if let Some(response_format) = msg.get("response_format") { - prompt.push_str("\n\n"); - prompt.push_str(&RESPONSE_FORMAT_PREAMBLE.replace("{}", &to_json(response_format)?)); - } - Ok(()) -} - -/// Append the tools section to `prompt` if the message has a `tools` array. -fn append_tools_section(msg: &JsonValue, prompt: &mut String) -> Result<()> { - if let Some(tools) = msg.get("tools").and_then(JsonValue::as_array) { - prompt.push_str("\n\n"); - prompt.push_str(&render_tools(tools)?); - } - Ok(()) -} - -/// Render the `system` role: raw content, then optional tools + response-format. -fn render_system_role(msg: &JsonValue, prompt: &mut String) -> Result<()> { - let content = msg.get("content").and_then(JsonValue::as_str).unwrap_or(""); - prompt.push_str(content); - append_tools_section(msg, prompt)?; - append_response_format(msg, prompt)?; - Ok(()) -} - -/// Render the `developer` role: wraps content in USER_START, then optional -/// tools + response-format. Developer content is required (non-empty). -fn render_developer_role(msg: &JsonValue, prompt: &mut String) -> Result<()> { - let content = msg - .get("content") - .and_then(JsonValue::as_str) - .filter(|s| !s.is_empty()) - .context("Developer role requires content")?; - - prompt.push_str(tokens::USER_START); - prompt.push_str(content); - append_tools_section(msg, prompt)?; - append_response_format(msg, prompt)?; - Ok(()) -} - -/// Render the `user` role: either content-blocks (with tool_result wrapping) -/// or a plain string `content` field. -fn render_user_role(msg: &JsonValue, prompt: &mut String) -> Result<()> { - prompt.push_str(tokens::USER_START); - if let Some(blocks) = msg.get("content_blocks").and_then(JsonValue::as_array) { - let mut parts: Vec = Vec::with_capacity(blocks.len()); - for block in blocks { - let block_type = block.get("type").and_then(JsonValue::as_str).unwrap_or(""); - match block_type { - "text" => { - let text = block.get("text").and_then(JsonValue::as_str).unwrap_or(""); - parts.push(text.to_string()); - } - "tool_result" => { - let rendered = render_tool_result_content( - block.get("content").unwrap_or(&JsonValue::Null), - )?; - parts.push(format!( - "{}{}{}", - TOOL_RESULT_OPEN, rendered, TOOL_RESULT_CLOSE - )); - } - other => { - tracing::warn!( - block_type = other, - "DeepSeek V4 formatter emitted placeholder for unsupported user content block type", - ); - parts.push(UNSUPPORTED_PLACEHOLDER_FMT.replace("{}", other)); - } - } - } - prompt.push_str(&parts.join("\n\n")); - } else { - let content = msg.get("content").and_then(JsonValue::as_str).unwrap_or(""); - prompt.push_str(content); - } - Ok(()) -} - -/// Render the `latest_reminder` role: LATEST_REMINDER token + content. -fn render_latest_reminder_role(msg: &JsonValue, prompt: &mut String) { - let content = msg.get("content").and_then(JsonValue::as_str).unwrap_or(""); - prompt.push_str(tokens::LATEST_REMINDER); - prompt.push_str(content); -} - -/// Render the `assistant` role: optional thinking prefix, content, optional -/// `tool_calls` DSML block, optional EOS. -/// -/// Needs `index` and `messages` to peek at the previous turn's `task` field -/// (suppresses thinking prefix when the prior message was a task message). -fn render_assistant_role( - msg: &JsonValue, - index: usize, - messages: &[JsonValue], - thinking_mode: ThinkingMode, - drop_thinking: bool, - last_user_idx: Option, - prompt: &mut String, -) -> Result<()> { - let content = msg.get("content").and_then(JsonValue::as_str).unwrap_or(""); - let reasoning = msg - .get("reasoning_content") - .and_then(JsonValue::as_str) - .unwrap_or(""); - let wo_eos = msg - .get("wo_eos") - .and_then(JsonValue::as_bool) - .unwrap_or(false); - - let prev_has_task = index > 0 - && messages[index - 1] - .get("task") - .map(|v| !v.is_null()) - .unwrap_or(false); - - if thinking_mode == ThinkingMode::Thinking && !prev_has_task { - let render_thinking = !drop_thinking || last_user_idx.is_none_or(|u| index > u); - if render_thinking { - prompt.push_str(reasoning); - prompt.push_str(tokens::THINKING_END); - } - } - - prompt.push_str(content); - - if let Some(tool_calls) = msg.get("tool_calls").and_then(JsonValue::as_array) - && !tool_calls.is_empty() - { - prompt.push_str("\n\n"); - prompt.push_str(&format!( - "<{}{}>\n", - tokens::DSML_TOKEN, - TOOL_CALLS_BLOCK_NAME - )); - - let mut invocations = Vec::with_capacity(tool_calls.len()); - for tc in tool_calls { - // Accept both OpenAI-format (nested `function`) and internal - // `{name, arguments}` shape, matching Python's `tool_calls_from_openai_format`. - let fn_obj = tc.get("function").unwrap_or(tc); - let name = fn_obj - .get("name") - .and_then(JsonValue::as_str) - .context("Missing tool call name")?; - let arguments = encode_arguments_to_dsml(fn_obj)?; - invocations.push(format!( - "<{}invoke name=\"{}\">\n{}\n", - tokens::DSML_TOKEN, - name, - arguments, - tokens::DSML_TOKEN - )); - } - prompt.push_str(&invocations.join("\n")); - prompt.push_str(&format!( - "\n", - tokens::DSML_TOKEN, - TOOL_CALLS_BLOCK_NAME - )); - } - - if !wo_eos { - prompt.push_str(tokens::EOS); - } - Ok(()) -} - /// Render a single message at the given index. fn render_message( index: usize, @@ -513,7 +289,7 @@ fn render_message( let role = msg .get("role") - .and_then(JsonValue::as_str) + .and_then(|r| r.as_str()) .context("Missing 'role' field")?; let mut prompt = String::new(); @@ -527,35 +303,165 @@ fn render_message( } match role { - "system" => render_system_role(msg, &mut prompt)?, - "developer" => render_developer_role(msg, &mut prompt)?, - "user" => render_user_role(msg, &mut prompt)?, - "latest_reminder" => render_latest_reminder_role(msg, &mut prompt), - "tool" => anyhow::bail!( - "deepseek_v4 merges tool messages into user; preprocess with merge_tool_messages()" - ), - "assistant" => render_assistant_role( - msg, - index, - messages, - thinking_mode, - drop_thinking, - last_user_idx, - &mut prompt, - )?, - other => anyhow::bail!("Unknown role: {other}"), + "system" => { + let content = msg.get("content").and_then(|c| c.as_str()).unwrap_or(""); + prompt.push_str(content); + if let Some(tools) = msg.get("tools").and_then(|t| t.as_array()) { + prompt.push_str("\n\n"); + prompt.push_str(&render_tools(tools)); + } + if let Some(response_format) = msg.get("response_format") { + prompt.push_str("\n\n"); + prompt.push_str( + &RESPONSE_FORMAT_TEMPLATE.replace("{schema}", &to_json(response_format)), + ); + } + } + + "developer" => { + let content = msg + .get("content") + .and_then(|c| c.as_str()) + .filter(|s| !s.is_empty()) + .context("Developer role requires content")?; + + let mut content_developer = String::from(tokens::USER_START); + content_developer.push_str(content); + + if let Some(tools) = msg.get("tools").and_then(|t| t.as_array()) { + content_developer.push_str("\n\n"); + content_developer.push_str(&render_tools(tools)); + } + if let Some(response_format) = msg.get("response_format") { + content_developer.push_str("\n\n"); + content_developer.push_str( + &RESPONSE_FORMAT_TEMPLATE.replace("{schema}", &to_json(response_format)), + ); + } + prompt.push_str(&content_developer); + } + + "user" => { + prompt.push_str(tokens::USER_START); + if let Some(blocks) = msg.get("content_blocks").and_then(|b| b.as_array()) { + let mut parts: Vec = Vec::with_capacity(blocks.len()); + for block in blocks { + let block_type = block.get("type").and_then(|v| v.as_str()).unwrap_or(""); + match block_type { + "text" => { + let text = block.get("text").and_then(|v| v.as_str()).unwrap_or(""); + parts.push(text.to_string()); + } + "tool_result" => { + let rendered = render_tool_result_content( + block.get("content").unwrap_or(&JsonValue::Null), + ); + parts.push(format!("{}", rendered)); + } + other => { + parts.push(format!("[Unsupported {}]", other)); + } + } + } + prompt.push_str(&parts.join("\n\n")); + } else { + let content = msg.get("content").and_then(|c| c.as_str()).unwrap_or(""); + prompt.push_str(content); + } + } + + "latest_reminder" => { + let content = msg.get("content").and_then(|c| c.as_str()).unwrap_or(""); + prompt.push_str(tokens::LATEST_REMINDER); + prompt.push_str(content); + } + + "tool" => { + anyhow::bail!( + "deepseek_v4 merges tool messages into user; preprocess with merge_tool_messages()" + ); + } + + "assistant" => { + let content = msg.get("content").and_then(|c| c.as_str()).unwrap_or(""); + let reasoning = msg + .get("reasoning_content") + .and_then(|c| c.as_str()) + .unwrap_or(""); + let wo_eos = msg.get("wo_eos").and_then(|v| v.as_bool()).unwrap_or(false); + + let prev_has_task = index > 0 + && messages[index - 1] + .get("task") + .map(|v| !v.is_null()) + .unwrap_or(false); + + let mut thinking_part = String::new(); + if thinking_mode == ThinkingMode::Thinking && !prev_has_task { + let render_thinking = !drop_thinking || last_user_idx.is_none_or(|u| index > u); + if render_thinking { + thinking_part.push_str(reasoning); + thinking_part.push_str(tokens::THINKING_END); + } + } + + prompt.push_str(&thinking_part); + prompt.push_str(content); + + if let Some(tool_calls) = msg.get("tool_calls").and_then(|t| t.as_array()) + && !tool_calls.is_empty() + { + prompt.push_str("\n\n"); + prompt.push_str(&format!( + "<{}{}>\n", + tokens::DSML_TOKEN, + TOOL_CALLS_BLOCK_NAME + )); + + let mut invocations = Vec::with_capacity(tool_calls.len()); + for tc in tool_calls { + // Accept both OpenAI-format (nested `function`) and internal + // `{name, arguments}` shape, matching Python's `tool_calls_from_openai_format`. + let fn_obj = tc.get("function").unwrap_or(tc); + let name = fn_obj + .get("name") + .and_then(|n| n.as_str()) + .context("Missing tool call name")?; + let arguments = encode_arguments_to_dsml(fn_obj)?; + invocations.push(format!( + "<{}invoke name=\"{}\">\n{}\n", + tokens::DSML_TOKEN, + name, + arguments, + tokens::DSML_TOKEN + )); + } + prompt.push_str(&invocations.join("\n")); + prompt.push_str(&format!( + "\n", + tokens::DSML_TOKEN, + TOOL_CALLS_BLOCK_NAME + )); + } + + if !wo_eos { + prompt.push_str(tokens::EOS); + } + } + + other => anyhow::bail!("Unknown role: {}", other), } // Early return if the next message is not assistant/latest_reminder — no transition appended. if index + 1 < messages.len() { - let next_role = messages[index + 1].get("role").and_then(JsonValue::as_str); + let next_role = messages[index + 1].get("role").and_then(|r| r.as_str()); if !matches!(next_role, Some("assistant") | Some("latest_reminder")) { return Ok(prompt); } } // Transition tokens based on task field and role. - let task = msg.get("task").and_then(JsonValue::as_str); + let task = msg.get("task").and_then(|v| v.as_str()); if let Some(task) = task { let sp = task_token(task).with_context(|| format!("Invalid task: '{}'", task))?; if task != "action" { @@ -584,49 +490,39 @@ fn render_message( } /// Render a tool_result `content` payload (string or content-block list). -fn render_tool_result_content(content: &JsonValue) -> Result { - Ok(match content { +fn render_tool_result_content(content: &JsonValue) -> String { + match content { JsonValue::String(s) => s.clone(), JsonValue::Array(items) => { let mut parts: Vec = Vec::with_capacity(items.len()); for item in items { - let item_type = item.get("type").and_then(JsonValue::as_str).unwrap_or(""); + let item_type = item.get("type").and_then(|v| v.as_str()).unwrap_or(""); if item_type == "text" { parts.push( item.get("text") - .and_then(JsonValue::as_str) + .and_then(|v| v.as_str()) .unwrap_or("") .to_string(), ); } else { - tracing::warn!( - item_type, - "DeepSeek V4 formatter emitted placeholder for unsupported tool_result content item type", - ); - parts.push(UNSUPPORTED_PLACEHOLDER_FMT.replace("{}", item_type)); + parts.push(format!("[Unsupported {}]", item_type)); } } parts.join("\n\n") } JsonValue::Null => String::new(), - _ => to_json(content)?, - }) + _ => to_json(content), + } } /// Merge `tool` role messages into preceding user `content_blocks` and collapse /// consecutive user turns, matching Python's `merge_tool_messages`. -/// -/// Iterates the input by reference. Each message is cloned at most once — and -/// only when the control flow actually moves it into `merged` (the "other -/// role" pass-through branch). The `tool` and `user` branches extract the few -/// fields they need and build fresh JSON objects, so cloning the whole input -/// message up front (as the original implementation did) was pure overhead -/// on long chat histories. pub fn merge_tool_messages(messages: &[JsonValue]) -> Vec { let mut merged: Vec = Vec::with_capacity(messages.len()); for msg in messages { - let role = msg.get("role").and_then(JsonValue::as_str).unwrap_or(""); + let msg = msg.clone(); + let role = msg.get("role").and_then(|r| r.as_str()).unwrap_or(""); if role == "tool" { let tool_block = serde_json::json!({ @@ -638,21 +534,17 @@ pub fn merge_tool_messages(messages: &[JsonValue]) -> Vec { let can_merge = merged .last() .map(|m| { - m.get("role").and_then(JsonValue::as_str) == Some("user") + m.get("role").and_then(|r| r.as_str()) == Some("user") && m.get("content_blocks").is_some() }) .unwrap_or(false); if can_merge { - // `can_merge` already checked `content_blocks.is_some()`; if - // the subsequent `as_array_mut()` ever fails (data invariant - // violation) we match the original behavior and silently - // drop rather than falling through to push a new user msg. - if let Some(last) = merged.last_mut() - && let Some(blocks) = last - .as_object_mut() - .and_then(|o| o.get_mut("content_blocks")) - .and_then(JsonValue::as_array_mut) + let last = merged.last_mut().unwrap(); + if let Some(blocks) = last + .as_object_mut() + .and_then(|o| o.get_mut("content_blocks")) + .and_then(|v| v.as_array_mut()) { blocks.push(tool_block); } @@ -665,7 +557,7 @@ pub fn merge_tool_messages(messages: &[JsonValue]) -> Vec { } else if role == "user" { let text = msg .get("content") - .and_then(JsonValue::as_str) + .and_then(|c| c.as_str()) .unwrap_or("") .to_string(); let text_block = serde_json::json!({ "type": "text", "text": text }); @@ -673,18 +565,18 @@ pub fn merge_tool_messages(messages: &[JsonValue]) -> Vec { let can_merge = merged .last() .map(|m| { - m.get("role").and_then(JsonValue::as_str) == Some("user") + m.get("role").and_then(|r| r.as_str()) == Some("user") && m.get("content_blocks").is_some() && m.get("task").map(|v| v.is_null()).unwrap_or(true) }) .unwrap_or(false); if can_merge { - if let Some(last) = merged.last_mut() - && let Some(blocks) = last - .as_object_mut() - .and_then(|o| o.get_mut("content_blocks")) - .and_then(JsonValue::as_array_mut) + let last = merged.last_mut().unwrap(); + if let Some(blocks) = last + .as_object_mut() + .and_then(|o| o.get_mut("content_blocks")) + .and_then(|v| v.as_array_mut()) { blocks.push(text_block); } @@ -705,8 +597,7 @@ pub fn merge_tool_messages(messages: &[JsonValue]) -> Vec { merged.push(new_msg); } } else { - // Pass-through: clone only when we're actually moving the message. - merged.push(msg.clone()); + merged.push(msg); } } @@ -720,18 +611,18 @@ pub fn sort_tool_results_by_call_order(mut messages: Vec) -> Vec = HashMap::new(); for msg in &mut messages { - let role = msg.get("role").and_then(JsonValue::as_str).unwrap_or(""); + let role = msg.get("role").and_then(|r| r.as_str()).unwrap_or(""); if role == "assistant" { - if let Some(tcs) = msg.get("tool_calls").and_then(JsonValue::as_array) { + if let Some(tcs) = msg.get("tool_calls").and_then(|t| t.as_array()) { last_order.clear(); for (idx, tc) in tcs.iter().enumerate() { let id = tc .get("id") - .and_then(JsonValue::as_str) + .and_then(|v| v.as_str()) .or_else(|| { tc.get("function") .and_then(|f| f.get("id")) - .and_then(JsonValue::as_str) + .and_then(|v| v.as_str()) }) .unwrap_or(""); if !id.is_empty() { @@ -743,7 +634,7 @@ pub fn sort_tool_results_by_call_order(mut messages: Vec) -> Vec) -> Vec = blocks .iter() .enumerate() - .filter(|(_, b)| b.get("type").and_then(JsonValue::as_str) == Some("tool_result")) + .filter(|(_, b)| b.get("type").and_then(|v| v.as_str()) == Some("tool_result")) .map(|(i, _)| i) .collect(); @@ -760,10 +651,7 @@ pub fn sort_tool_results_by_call_order(mut messages: Vec) -> Vec = tool_positions.iter().map(|&i| blocks[i].clone()).collect(); tool_blocks.sort_by_key(|b| { - let id = b - .get("tool_use_id") - .and_then(JsonValue::as_str) - .unwrap_or(""); + let id = b.get("tool_use_id").and_then(|v| v.as_str()).unwrap_or(""); *last_order.get(id).unwrap_or(&0) }); for (sorted_idx, &pos) in tool_positions.iter().enumerate() { @@ -777,23 +665,20 @@ pub fn sort_tool_results_by_call_order(mut messages: Vec) -> Vec, - last_user_idx: Option, -) -> (Vec, Option) { +fn drop_thinking_messages(messages: Vec) -> Vec { + let last_user_idx = find_last_user_index(&messages); let mut out = Vec::with_capacity(messages.len()); - let mut new_last_user_idx: Option = None; + const KEEP: &[&str] = &[ + "user", + "system", + "tool", + "latest_reminder", + "direct_search_results", + ]; + for (idx, mut msg) in messages.into_iter().enumerate() { - let role = msg.get("role").and_then(JsonValue::as_str).unwrap_or(""); - if KEEP_ROLES.contains(&role) || last_user_idx.is_none_or(|u| idx >= u) { - if last_user_idx == Some(idx) { - new_last_user_idx = Some(out.len()); - } + let role = msg.get("role").and_then(|r| r.as_str()).unwrap_or(""); + if KEEP.contains(&role) || last_user_idx.is_none_or(|u| idx >= u) { out.push(msg); } else if role == "assistant" { if let Some(obj) = msg.as_object_mut() { @@ -803,7 +688,7 @@ fn drop_thinking_messages( } // developer and other roles before last_user_idx are dropped. } - (out, new_last_user_idx) + out } /// Encode messages to prompt string with default options. @@ -852,17 +737,11 @@ pub fn encode_messages_with_options( }); let effective_drop_thinking = drop_thinking && !has_tools; - // Locate the last user/developer message once. `drop_thinking_messages` - // both consumes this (to know what to keep) and returns the new index - // (to avoid a second full scan over its output list). - let mut last_user_idx = find_last_user_index(&full); - if thinking_mode == ThinkingMode::Thinking && effective_drop_thinking { - let (dropped, new_last_user) = drop_thinking_messages(full, last_user_idx); - full = dropped; - last_user_idx = new_last_user; + full = drop_thinking_messages(full); } + let last_user_idx = find_last_user_index(&full); for idx in 0..full.len() { let part = render_message( idx, @@ -899,21 +778,54 @@ impl DeepSeekV4Formatter { Self::new(ThinkingMode::Chat) } + fn resolve_reasoning_effort( + args: Option<&std::collections::HashMap>, + ) -> Option { + let args = args?; + let v = args.get("reasoning_effort")?; + match v.as_str() { + Some("max") => Some(ReasoningEffort::Max), + Some("high") => Some(ReasoningEffort::High), + _ => { + tracing::warn!( + value = ?v, + "chat_template_args.reasoning_effort must be a string of \"max\" or \"high\"; ignoring and using default (none)" + ); + None + } + } + } + + fn resolve_drop_thinking( + args: Option<&std::collections::HashMap>, + ) -> bool { + let Some(args) = args else { return true }; + let Some(v) = args.get("drop_thinking") else { + return true; + }; + if let Some(b) = v.as_bool() { + return b; + } + tracing::warn!( + value = ?v, + "chat_template_args.drop_thinking must be a bool; ignoring and using default (true)" + ); + true + } + fn resolve_thinking_mode( &self, args: Option<&std::collections::HashMap>, ) -> ThinkingMode { - if let Some(args) = args - && let Some(thinking) = args.get("thinking").and_then(JsonValue::as_bool) - { - return if thinking { + if let Some(enabled) = super::thinking_bool_from_args(args) { + return if enabled { ThinkingMode::Thinking } else { ThinkingMode::Chat }; } if let Some(args) = args - && let Some(mode) = args.get("thinking_mode").and_then(JsonValue::as_str) + && let Some(mode) = args.get("thinking_mode").and_then(|v| v.as_str()) { match mode { "chat" => return ThinkingMode::Chat, @@ -931,7 +843,10 @@ impl super::OAIPromptFormatter for DeepSeekV4Formatter { } fn render(&self, req: &dyn super::OAIChatLikeRequest) -> Result { - let thinking_mode = self.resolve_thinking_mode(req.chat_template_args()); + let args = req.chat_template_args(); + let thinking_mode = self.resolve_thinking_mode(args); + let reasoning_effort = Self::resolve_reasoning_effort(args); + let drop_thinking = Self::resolve_drop_thinking(args); let messages_value = req.messages(); let messages_json = @@ -942,7 +857,7 @@ impl super::OAIPromptFormatter for DeepSeekV4Formatter { .context("Messages is not an array")? .clone(); - normalize_message_contents(&mut messages_array)?; + normalize_message_contents(&mut messages_array); let tools_json = req .tools() @@ -959,7 +874,7 @@ impl super::OAIPromptFormatter for DeepSeekV4Formatter { if tools_json.is_some() || response_format_json.is_some() { let system_idx = messages_array .iter() - .position(|msg| msg.get("role").and_then(JsonValue::as_str) == Some("system")); + .position(|msg| msg.get("role").and_then(|r| r.as_str()) == Some("system")); if let Some(idx) = system_idx { if let Some(msg) = messages_array.get_mut(idx) @@ -989,7 +904,13 @@ impl super::OAIPromptFormatter for DeepSeekV4Formatter { } } - encode_messages(&messages_array, thinking_mode, true) + encode_messages_with_options( + &messages_array, + thinking_mode, + true, + drop_thinking, + reasoning_effort, + ) } } @@ -1124,148 +1045,194 @@ mod tests { #[test] fn test_to_json_preserves_spacing_past_escaped_backslash() { let v = json!({"path": "\\", "count": 5}); - let got = to_json(&v).expect("to_json must succeed on well-formed input"); + let got = to_json(&v); assert_eq!( got, r#"{"path": "\\", "count": 5}"#, "to_json must match Python's json.dumps formatting past an escaped backslash" ); } - /// Larger-than-64-byte inputs (typical tool schemas / response formats) - /// must round-trip unchanged — pins that the capacity hint doesn't - /// truncate and that Python spacing holds across deep nesting. #[test] - fn test_to_json_handles_large_payload() { - // Build a ~5 KB tool-like schema by nesting an array of items. - let items: Vec = (0..200) - .map(|i| json!({"name": format!("field_{i}"), "type": "string", "i": i})) - .collect(); - let v = json!({ - "type": "object", - "properties": {"items": {"type": "array", "items": items}}, - "required": ["items"], - }); - - let got = to_json(&v).expect("to_json must succeed on well-formed input"); - // Baseline: default serde_json::to_string round-trips. - let parsed: serde_json::Value = serde_json::from_str(&got).expect("round-trip parse"); - assert_eq!( - parsed, v, - "to_json output must round-trip back to the input" + fn test_resolve_drop_thinking_warns_on_malformed_value() { + use std::collections::HashMap; + // String "false" where a bool is expected → fall back to default (true) and warn. + let mut args = HashMap::new(); + args.insert( + "drop_thinking".to_string(), + serde_json::Value::String("false".to_string()), ); - // Python spacing assertions: no bare `",` or `":` sequences outside of - // string literals. The payload contains no commas or colons inside - // string values, so a byte scan is sufficient. - assert!( - !got.contains("\",\""), - "expected ', ' between keys — raw '\",\"' should not appear", + assert!(DeepSeekV4Formatter::resolve_drop_thinking(Some(&args))); + // Malformed reasoning_effort falls back to None. + let mut args2 = HashMap::new(); + args2.insert( + "reasoning_effort".to_string(), + serde_json::Value::String("HIGH".to_string()), ); - assert!( - !got.contains("\":\""), - "expected ': ' between key and value — raw '\":\"' should not appear", + assert_eq!( + DeepSeekV4Formatter::resolve_reasoning_effort(Some(&args2)), + None ); - // Sanity: large payload exercised (> 5KB). - assert!( - got.len() > 5_000, - "test payload is too small: {}", - got.len() + } + + #[test] + fn test_resolve_thinking_mode_honors_enable_thinking() { + use std::collections::HashMap; + let f = DeepSeekV4Formatter::new_thinking(); + let mut args = HashMap::new(); + args.insert( + "enable_thinking".to_string(), + serde_json::Value::Bool(false), ); + assert_eq!(f.resolve_thinking_mode(Some(&args)), ThinkingMode::Chat); + args.insert("enable_thinking".to_string(), serde_json::Value::Bool(true)); + assert_eq!(f.resolve_thinking_mode(Some(&args)), ThinkingMode::Thinking); + } + + struct MockRequest { + messages: JsonValue, + chat_template_args: Option>, + } + + impl MockRequest { + fn new(messages: JsonValue) -> Self { + Self { + messages, + chat_template_args: None, + } + } + + fn with_chat_template_args( + mut self, + args: std::collections::HashMap, + ) -> Self { + self.chat_template_args = Some(args); + self + } + } + + impl super::super::OAIChatLikeRequest for MockRequest { + fn model(&self) -> String { + "deepseek-v4".to_string() + } + + fn messages(&self) -> minijinja::value::Value { + minijinja::value::Value::from_serialize(&self.messages) + } + + fn should_add_generation_prompt(&self) -> bool { + true + } + + fn chat_template_args( + &self, + ) -> Option<&std::collections::HashMap> { + self.chat_template_args.as_ref() + } } - /// `drop_thinking_messages` now takes the pre-drop last-user index and - /// returns the post-drop index instead of forcing the caller to rescan - /// the output list. This test pins that the returned index is - /// equivalent to a full `find_last_user_index` rescan, for the shape - /// that actually shifts indices (non-KEEP role dropped before the - /// final user/developer). #[test] - fn test_drop_thinking_messages_returns_post_drop_last_user_idx() { - // A `developer` message before the last user triggers an index - // shift: it's not in KEEP and is at idx < last_user_idx, so it - // gets dropped and every surviving message's index decreases by 1. - let messages = vec![ - json!({"role": "developer", "content": "dev1"}), - json!({"role": "assistant", "reasoning_content": "r", "content": "a1"}), - json!({"role": "user", "content": "u1"}), - json!({"role": "developer", "content": "dev2"}), - ]; - let pre_drop_idx = find_last_user_index(&messages); - // The last user-like message is the trailing developer at idx 3. - assert_eq!(pre_drop_idx, Some(3)); - - let (dropped, new_idx) = drop_thinking_messages(messages, pre_drop_idx); - - // developer at idx 0: dropped (not KEEP, before last_user_idx). - // assistant at idx 1: kept with reasoning stripped. - // user at idx 2: kept. - // developer at idx 3: kept (is last_user_idx). - assert_eq!(dropped.len(), 3, "expected 3 messages after drop"); + fn test_render_wires_reasoning_effort_max_from_chat_template_args() { + use super::super::OAIPromptFormatter; + use std::collections::HashMap; + + let mut args = HashMap::new(); + args.insert("reasoning_effort".to_string(), json!("max")); + + let req = MockRequest::new(json!([ + {"role": "system", "content": "sys"}, + {"role": "user", "content": "hi"} + ])) + .with_chat_template_args(args); + + let formatter = DeepSeekV4Formatter::new_thinking(); + let out = formatter.render(&req).unwrap(); + + assert!(out.starts_with(tokens::BOS)); + let after_bos = &out[tokens::BOS.len()..]; assert!( - dropped[0].get("reasoning_content").is_none(), - "assistant reasoning_content must be stripped", - ); - assert_eq!( - dropped[0].get("role").and_then(JsonValue::as_str), - Some("assistant"), - ); - assert_eq!( - dropped[1].get("role").and_then(JsonValue::as_str), - Some("user") + after_bos.starts_with("Reasoning Effort:"), + "REASONING_EFFORT_MAX preamble should appear at start (after BOS), got:\n{}", + out ); - assert_eq!( - dropped[2].get("role").and_then(JsonValue::as_str), - Some("developer"), + } + + #[test] + fn test_render_drop_thinking_override_from_chat_template_args() { + use super::super::OAIPromptFormatter; + use std::collections::HashMap; + + let messages = json!([ + {"role": "user", "content": "first"}, + {"role": "assistant", "reasoning_content": "PRIOR", "content": "reply"}, + {"role": "user", "content": "again"} + ]); + + // Default (drop_thinking=true): prior reasoning stripped. + let req_default = MockRequest::new(messages.clone()); + let formatter = DeepSeekV4Formatter::new_thinking(); + let out_default = formatter.render(&req_default).unwrap(); + assert!( + !out_default.contains("PRIOR"), + "default drop_thinking=true should strip prior reasoning, got:\n{}", + out_default ); - // The core invariant: the returned post-drop index is identical to - // what a full `find_last_user_index` rescan would produce. This - // is what allows `encode_messages_with_options` to skip the - // previously-redundant second scan. - let rescan_idx = find_last_user_index(&dropped); - assert_eq!( - new_idx, rescan_idx, - "drop_thinking_messages must return the same index find_last_user_index would compute on its output", + // drop_thinking=false override: prior reasoning survives. + let mut args = HashMap::new(); + args.insert("drop_thinking".to_string(), json!(false)); + let req_keep = MockRequest::new(messages).with_chat_template_args(args); + let out_keep = formatter.render(&req_keep).unwrap(); + assert!( + out_keep.contains("PRIOR"), + "drop_thinking=false override should preserve prior reasoning, got:\n{}", + out_keep ); - assert_eq!(new_idx, Some(2), "developer shifted from idx 3 to idx 2"); } - /// Sanity check for the no-shift case: when nothing before the last - /// user/developer is droppable, the index returned by - /// `drop_thinking_messages` must equal the input index. + // N4: developer-role interactions with drop_thinking. + // find_last_user_index returns the index of user OR developer messages; the + // drop_thinking reasoning cutoff and the thinking-seed insertion treat + // user and developer identically. + #[test] - fn test_drop_thinking_messages_preserves_idx_when_nothing_dropped() { - let messages = vec![ - json!({"role": "system", "content": "s"}), - json!({"role": "user", "content": "u1"}), - json!({"role": "assistant", "reasoning_content": "r", "content": "a"}), - json!({"role": "user", "content": "u2"}), - ]; - let pre_drop_idx = find_last_user_index(&messages); - assert_eq!(pre_drop_idx, Some(3)); - - let (dropped, new_idx) = drop_thinking_messages(messages, pre_drop_idx); - assert_eq!(dropped.len(), 4, "nothing should be dropped here"); - // No shift: last user still at the same index. - assert_eq!(new_idx, Some(3)); - assert_eq!(new_idx, find_last_user_index(&dropped)); + fn test_developer_only_conversation_renders_developer_content() { + let messages = json!([ + {"role": "system", "content": "sys"}, + {"role": "developer", "content": "x"}, + {"role": "assistant", "reasoning_content": "R", "content": "ok"} + ]); + let out = + encode_messages(messages.as_array().unwrap(), ThinkingMode::Thinking, true).unwrap(); + assert!( + out.contains("x"), + "developer content should appear in output, got:\n{}", + out + ); } - /// Edge case: no user/developer anywhere in the history. Both the - /// pre-drop and post-drop indices must be `None` so the render loop - /// treats every message as "after last user" (Python `-1` sentinel). #[test] - fn test_drop_thinking_messages_no_user_in_history() { - let messages = vec![ - json!({"role": "system", "content": "s"}), - json!({"role": "assistant", "reasoning_content": "r", "content": "a"}), - ]; - let pre_drop_idx = find_last_user_index(&messages); - assert_eq!(pre_drop_idx, None); - - let (dropped, new_idx) = drop_thinking_messages(messages, pre_drop_idx); - // With `last_user_idx == None`, the `idx >= u` guard is vacuously - // true for every index, so the assistant is kept (with reasoning). - assert_eq!(dropped.len(), 2); - assert_eq!(new_idx, None); + fn test_developer_as_last_user_index_controls_reasoning_cutoff() { + // Indices: 0=user, 1=assistant(FIRST), 2=developer(y), 3=assistant(SECOND). + // find_last_user_index = 2 (developer). With drop_thinking=true: + // - assistant idx=1 < 2 → reasoning_content stripped. + // - assistant idx=3 >= 2 → reasoning_content preserved. + let messages = json!([ + {"role": "user", "content": "a"}, + {"role": "assistant", "reasoning_content": "FIRST", "content": "r1"}, + {"role": "developer", "content": "y"}, + {"role": "assistant", "reasoning_content": "SECOND", "content": "r2"} + ]); + let out = + encode_messages(messages.as_array().unwrap(), ThinkingMode::Thinking, true).unwrap(); + assert!( + !out.contains("FIRST"), + "reasoning before last user/developer (idx 1 < 2) should be stripped, got:\n{}", + out + ); + assert!( + out.contains("SECOND"), + "reasoning at/after last user/developer (idx 3 > 2) should survive, got:\n{}", + out + ); } } diff --git a/lib/llm/tests/data/vllm/deepseek-v4/chat_completion_stream_special_chars.json b/lib/llm/tests/data/vllm/deepseek-v4/chat_completion_stream_special_chars.json index 2d5375f10014..38ab2a150cd8 100644 --- a/lib/llm/tests/data/vllm/deepseek-v4/chat_completion_stream_special_chars.json +++ b/lib/llm/tests/data/vllm/deepseek-v4/chat_completion_stream_special_chars.json @@ -1,6 +1,6 @@ { "request_id": "deepseek-v4-special-chars-test", - "expected_output": {"normal_content": "", "reasoning_content": "The user wants me to save a multiline note with special characters, quotes, unicode, and emoji. I'll call save_note.", "tool_calls": [{"id": "call_1", "type": "function", "function": {"name": "save_note", "arguments": "{\"note\": \"He said \\\"hello\\\".\\n\\t'world' `backtick` — 中文测试 — 🚀✨ & .\"}"}}]}, + "expected_output": {"normal_content": "", "reasoning_content": "The user wants me to save a multiline note with special characters, quotes, unicode, and emoji. I'll call save_note.", "tool_calls": [{"id": "call_1", "type": "function", "function": {"name": "save_note", "arguments": "{\"note\": \"He said \\\"hello\\\".\\n\\t'world' `backtick` — 中文测试 — 🚀✨ & .\"}"}}], "finish_reason": "tool_calls"}, "input_stream": [ {"data":{"id":"chatcmpl-deepseek-v4-special","choices":[{"index":0,"delta":{"content":"The user wants me to save a multiline note with special characters, quotes, unicode, and emoji. I'll call save_note.","role":"assistant","reasoning_content":"The user wants me to save a multiline note with special characters, quotes, unicode, and emoji. I'll call save_note."}}]}}, {"data":{"id":"chatcmpl-deepseek-v4-special","choices":[{"index":0,"delta":{"content":"<|DSML|tool_calls>\n","role":"assistant"}}]}}, diff --git a/lib/parsers/src/reasoning/mod.rs b/lib/parsers/src/reasoning/mod.rs index 5f259bc80f74..e14dd3117955 100644 --- a/lib/parsers/src/reasoning/mod.rs +++ b/lib/parsers/src/reasoning/mod.rs @@ -294,7 +294,6 @@ mod tests { assert!(parsers.contains(&parser)); } } - /// `CASE.10` — reasoning-only (V4 ``/``). #[test] fn test_deepseek_v4_detect_and_parse() { @@ -305,7 +304,6 @@ mod tests { assert_eq!(result.normal_text, "answer"); } } - /// `CASE.3` / `CASE.10` — no reasoning tags ⇒ no `reasoning_content`. #[test] fn test_deepseek_v4_no_forced_reasoning_without_tags() { @@ -314,7 +312,6 @@ mod tests { assert_eq!(result.reasoning_text, ""); assert_eq!(result.normal_text, "answer only"); } - /// `CASE.8` — streaming reasoning parse (chunked). #[test] fn test_deepseek_v4_streaming() { @@ -535,4 +532,30 @@ mod tests { assert_eq!(all_reasoning, "reasoning done."); assert_eq!(all_content, "Hello world"); } + + // P2-1: V4 production regime where the prompt ends in , so the stream + // begins INSIDE a reasoning block (no opening sentinel). The caller + // initializes the parser via set_in_reasoning(true); bytes before + // must route to reasoning_content, bytes after to normal content. + #[test] + fn test_deepseek_v4_streaming_with_set_in_reasoning() { + let mut parser = ReasoningParserType::get_reasoning_parser_from_name("deepseek_v4"); + parser.set_in_reasoning(true); + + // Token-by-token stream, starting with raw reasoning (no prefix), + // in the middle, then normal content. + let tokens = &[ + "Wei", "gh", "ing ", "options", ".", "", "Bei", "jing", " is", " sunny.", + ]; + + let mut all_reasoning = String::new(); + let mut all_content = String::new(); + for token in tokens { + let r = parser.parse_reasoning_streaming_incremental(token, &[]); + all_reasoning.push_str(&r.reasoning_text); + all_content.push_str(&r.normal_text); + } + assert_eq!(all_reasoning, "Weighing options."); + assert_eq!(all_content, "Beijing is sunny."); + } } diff --git a/lib/parsers/src/tool_calling/config.rs b/lib/parsers/src/tool_calling/config.rs index 35ab92208f7e..282c7ce2b41d 100644 --- a/lib/parsers/src/tool_calling/config.rs +++ b/lib/parsers/src/tool_calling/config.rs @@ -80,10 +80,12 @@ impl Default for XmlParserConfig { /// Configuration for DSML-style tool call parser (DeepSeek V3.2+) #[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] pub struct DsmlParserConfig { - /// Start token for function_calls block (e.g., "<|DSML|function_calls>") - pub function_calls_start: String, - /// End token for function_calls block (e.g., "") - pub function_calls_end: String, + /// Start token for the DSML block (e.g., "<|DSML|function_calls>" or "<|DSML|tool_calls>") + #[serde(alias = "function_calls_start")] + pub block_start: String, + /// End token for the DSML block (e.g., "" or "") + #[serde(alias = "function_calls_end")] + pub block_end: String, /// Start prefix for invoke (e.g., "<|DSML|invoke name=") pub invoke_start_prefix: String, /// End token for invoke (e.g., "") @@ -97,8 +99,8 @@ pub struct DsmlParserConfig { impl Default for DsmlParserConfig { fn default() -> Self { Self { - function_calls_start: "<|DSML|function_calls>".to_string(), - function_calls_end: "".to_string(), + block_start: "<|DSML|function_calls>".to_string(), + block_end: "".to_string(), invoke_start_prefix: "<|DSML|invoke name=".to_string(), invoke_end: "".to_string(), parameter_prefix: "<|DSML|parameter name=".to_string(), @@ -213,7 +215,7 @@ impl ParserConfig { ParserConfig::Xml(config) => vec![config.tool_call_start_token.clone()], ParserConfig::Pythonic => vec![], ParserConfig::Typescript => vec![], - ParserConfig::Dsml(config) => vec![config.function_calls_start.clone()], + ParserConfig::Dsml(config) => vec![config.block_start.clone()], ParserConfig::Glm47(config) => vec![config.tool_call_start.clone()], ParserConfig::KimiK2(config) => config.section_start_variants.clone(), } @@ -228,7 +230,7 @@ impl ParserConfig { ParserConfig::Xml(config) => vec![config.tool_call_end_token.clone()], ParserConfig::Pythonic => vec![], ParserConfig::Typescript => vec![], - ParserConfig::Dsml(config) => vec![config.function_calls_end.clone()], + ParserConfig::Dsml(config) => vec![config.block_end.clone()], ParserConfig::Glm47(config) => vec![config.tool_call_end.clone()], ParserConfig::KimiK2(config) => config.section_end_variants.clone(), } @@ -379,6 +381,16 @@ impl ToolCallConfig { } } + fn deepseek_dsml(block_name: &str) -> Self { + Self { + parser_config: ParserConfig::Dsml(DsmlParserConfig { + block_start: format!("<|DSML|{}>", block_name), + block_end: format!("", block_name), + ..Default::default() + }), + } + } + pub fn deepseek_v3_2() -> Self { // DeepSeek V3.2 format (DSML): // <|DSML|function_calls> @@ -386,9 +398,7 @@ impl ToolCallConfig { // <|DSML|parameter name="param_name" string="true|false">value // // - Self { - parser_config: ParserConfig::Dsml(DsmlParserConfig::default()), - } + Self::deepseek_dsml("function_calls") } pub fn deepseek_v4() -> Self { @@ -398,13 +408,7 @@ impl ToolCallConfig { // <|DSML|parameter name="param_name" string="true|false">value // // - Self { - parser_config: ParserConfig::Dsml(DsmlParserConfig { - function_calls_start: "<|DSML|tool_calls>".to_string(), - function_calls_end: "".to_string(), - ..Default::default() - }), - } + Self::deepseek_dsml("tool_calls") } pub fn minimax_m2() -> Self { @@ -447,3 +451,42 @@ impl ToolCallConfig { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn dsml_config_deserializes_legacy_function_calls_aliases() { + let legacy = serde_json::json!({ + "function_calls_start": "<|DSML|function_calls>", + "function_calls_end": "", + "invoke_start_prefix": "<|DSML|invoke name=", + "invoke_end": "", + "parameter_prefix": "<|DSML|parameter name=", + "parameter_end": "", + }); + let cfg: DsmlParserConfig = serde_json::from_value(legacy).unwrap(); + assert_eq!(cfg.block_start, "<|DSML|function_calls>"); + assert_eq!(cfg.block_end, ""); + assert_eq!(cfg.invoke_start_prefix, "<|DSML|invoke name="); + } + + #[test] + fn deepseek_dsml_factory_produces_expected_block_tokens() { + let v3_2 = ToolCallConfig::deepseek_v3_2(); + let v4 = ToolCallConfig::deepseek_v4(); + let v3_2_cfg = match v3_2.parser_config { + ParserConfig::Dsml(c) => c, + _ => panic!("expected Dsml variant for v3_2"), + }; + let v4_cfg = match v4.parser_config { + ParserConfig::Dsml(c) => c, + _ => panic!("expected Dsml variant for v4"), + }; + assert_eq!(v3_2_cfg.block_start, "<|DSML|function_calls>"); + assert_eq!(v3_2_cfg.block_end, ""); + assert_eq!(v4_cfg.block_start, "<|DSML|tool_calls>"); + assert_eq!(v4_cfg.block_end, ""); + } +} diff --git a/lib/parsers/src/tool_calling/dsml/parser.rs b/lib/parsers/src/tool_calling/dsml/parser.rs index cec8e75f67db..66107a0549dc 100644 --- a/lib/parsers/src/tool_calling/dsml/parser.rs +++ b/lib/parsers/src/tool_calling/dsml/parser.rs @@ -1,89 +1,22 @@ // SPDX-FileCopyrightText: Copyright (c) 2024-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -// Reference implementation: -// https://huggingface.co/deepseek-ai/DeepSeek-V3.2/tree/main/encoding/encoding_dsv32.py +// Reference implementations: +// V3.2: https://huggingface.co/deepseek-ai/DeepSeek-V3.2/tree/main/encoding/encoding_dsv32.py +// V4: https://huggingface.co/deepseek-ai/DeepSeek-V4-Pro/tree/main/encoding/encoding_dsv4.py +// +// V4 reuses this same DSML engine; only the outer block name changes from +// `function_calls` to `tool_calls` (configured via DsmlParserConfig). use regex::Regex; -use std::collections::HashMap; -use std::sync::{Arc, OnceLock, RwLock}; use uuid::Uuid; use super::super::config::DsmlParserConfig; use super::super::response::{CalledFunction, ToolCallResponse, ToolCallType}; -/// Compiled regex trio for a given `DsmlParserConfig`. Compiled once and -/// reused across every subsequent parse/stream chunk. -struct DsmlRegexes { - block: Regex, - invoke: Regex, - parameter: Regex, -} - -/// Cache key = the six config strings that drive the three regex patterns. -/// V3.2 and V4 are the only variants in use, so the cache has at most two -/// entries for the lifetime of the process. -type DsmlRegexKey = (String, String, String, String, String, String); - -fn regex_cache() -> &'static RwLock>> { - static CACHE: OnceLock>>> = OnceLock::new(); - CACHE.get_or_init(|| RwLock::new(HashMap::new())) -} - -/// Return the compiled regex trio for `config`, compiling on first use. -/// -/// Each parse call previously recompiled three regexes from `format!`'d -/// patterns that embed the config strings — expensive on streaming hot paths. -/// The cache is keyed on the raw config strings (not the escaped patterns) -/// so distinct configs that happen to escape identically still get distinct -/// entries. -fn get_dsml_regexes(config: &DsmlParserConfig) -> anyhow::Result> { - let key: DsmlRegexKey = ( - config.function_calls_start.clone(), - config.function_calls_end.clone(), - config.invoke_start_prefix.clone(), - config.invoke_end.clone(), - config.parameter_prefix.clone(), - config.parameter_end.clone(), - ); - // Fast path: shared read lock, common after the first parse of each config. - if let Some(regexes) = regex_cache() - .read() - .expect("DSML regex cache read lock poisoned") - .get(&key) - { - return Ok(Arc::clone(regexes)); - } - // Slow path: compile and install. Use `entry` so a concurrent compiler of - // the same key only inserts once (we still compile speculatively, then - // drop the duplicate — cheap on a map of <= 2 keys). - let block = Regex::new(&format!( - r"(?s){}\s*(.*?)\s*{}", - regex::escape(&config.function_calls_start), - regex::escape(&config.function_calls_end), - ))?; - let invoke = Regex::new(&format!( - r#"(?s){}\"([^"]+)\"\s*>(.*?){}"#, - regex::escape(&config.invoke_start_prefix), - regex::escape(&config.invoke_end), - ))?; - let parameter = Regex::new(&format!( - r#"(?s){}\"([^"]+)\"\s+string=\"(true|false)\"\s*>(.*?){}"#, - regex::escape(&config.parameter_prefix), - regex::escape(&config.parameter_end), - ))?; - let regexes = Arc::new(DsmlRegexes { - block, - invoke, - parameter, - }); - let mut cache = regex_cache() - .write() - .expect("DSML regex cache write lock poisoned"); - Ok(Arc::clone(cache.entry(key).or_insert(regexes))) -} - -/// DeepSeek V3.2 uses DSML (DeepSeek Markup Language) format for tool calls: +/// DeepSeek V3.2 / V4 use DSML (DeepSeek Markup Language) format for tool calls. +/// V3.2 wraps calls in `<|DSML|function_calls>`; V4 wraps them in +/// `<|DSML|tool_calls>`. The inner invoke / parameter grammar is identical: /// /// <|DSML|function_calls> /// <|DSML|invoke name="function_name"> @@ -93,7 +26,7 @@ fn get_dsml_regexes(config: &DsmlParserConfig) -> anyhow::Result /// Check if a chunk contains the start of a DSML tool call pub fn detect_tool_call_start_dsml(chunk: &str, config: &DsmlParserConfig) -> bool { - let start_token = &config.function_calls_start; + let start_token = &config.block_start; // Check for complete start token if chunk.contains(start_token.as_str()) { @@ -114,7 +47,7 @@ pub fn detect_tool_call_start_dsml(chunk: &str, config: &DsmlParserConfig) -> bo /// Find the end position of a DSML tool call block pub fn find_tool_call_end_position_dsml(chunk: &str, config: &DsmlParserConfig) -> usize { - let end_token = &config.function_calls_end; + let end_token = &config.block_end; if let Some(pos) = chunk.find(end_token.as_str()) { pos + end_token.len() @@ -123,6 +56,21 @@ pub fn find_tool_call_end_position_dsml(chunk: &str, config: &DsmlParserConfig) } } +/// Build the regex that matches a complete DSML tool_calls / function_calls block. +/// Shared by `extract_tool_calls_with_regex` and `try_tool_call_parse_dsml` so +/// the two stay in lockstep on how a block is recognised. +fn build_block_regex(config: &DsmlParserConfig) -> anyhow::Result { + // Matches: <|DSML|function_calls> ... + // Pattern: (?s) = dot matches newlines + // \s*(.*?)\s* = capture content between start/end tags (non-greedy) + let block_pattern = format!( + r"(?s){}\s*(.*?)\s*{}", + regex::escape(&config.block_start), + regex::escape(&config.block_end) + ); + Ok(Regex::new(&block_pattern)?) +} + /// Parse DSML formatted tool calls from a message /// Returns (parsed_tool_calls, normal_text_content) pub fn try_tool_call_parse_dsml( @@ -137,49 +85,59 @@ pub fn try_tool_call_parse_dsml( } // Check if tool call block exists - if !trimmed.contains(&config.function_calls_start) { + let start_idx = trimmed.find(&config.block_start); + if start_idx.is_none() { return Ok((vec![], Some(trimmed.to_string()))); } - // Extract normal text before tool calls - let normal_text = if let Some(start_idx) = trimmed.find(&config.function_calls_start) { - let text = trimmed[..start_idx].trim(); - if text.is_empty() { - String::new() - } else { - text.to_string() - } - } else { - String::new() - }; - // Extract tool calls blocks - let tool_calls = extract_tool_calls(trimmed, config)?; + let block_regex = build_block_regex(config)?; + let tool_calls = extract_tool_calls_with_regex(trimmed, &block_regex, config)?; if tool_calls.is_empty() { - // No valid tool calls found - return Ok((vec![], Some(trimmed.to_string()))); + // A block-start was detected but no valid invokes parsed. Do NOT leak + // the DSML markup back to the client; return only the pre-block text + // and emit a diagnostic with a prefix of the failed block. + // + // Note: an unterminated block-start here means `block_regex` finds no + // match at all, so any valid block *after* the unterminated one is + // lost. This matches the pre-existing conservative P1-3 contract. + if let Some(idx) = start_idx { + let failed = &trimmed[idx..]; + let prefix: String = failed.chars().take(120).collect(); + tracing::warn!( + "DSML tool_calls block parsed no invokes; suppressing markup. prefix={:?}", + prefix + ); + } + let pre_block_text = start_idx + .map(|idx| trimmed[..idx].to_string()) + .unwrap_or_default(); + return Ok((vec![], Some(pre_block_text))); } + // Preserve inter-block and trailing text: strip every complete block span + // from the trimmed input rather than slicing up to the first start token. + // Without this we silently lose text between and after multiple blocks. + let normal_text = block_regex.replace_all(trimmed, "").to_string(); + Ok((tool_calls, Some(normal_text))) } -/// Extract all tool calls from the DSML formatted text -fn extract_tool_calls( +/// Extract all tool calls matched by `block_regex` from the DSML formatted text. +fn extract_tool_calls_with_regex( text: &str, + block_regex: &Regex, config: &DsmlParserConfig, ) -> anyhow::Result> { let mut tool_calls = Vec::new(); - let regexes = get_dsml_regexes(config)?; - // Find all function_calls blocks — the block regex captures the content - // between start/end tags (non-greedy, dot-matches-newline). - for block_match in regexes.block.captures_iter(text) { + for block_match in block_regex.captures_iter(text) { if let Some(block_content) = block_match.get(1) { let block = block_content.as_str(); // Extract individual invokes from this block - let invokes = extract_invokes(block, ®exes)?; + let invokes = extract_invokes(block, config)?; tool_calls.extend(invokes); } } @@ -188,24 +146,40 @@ fn extract_tool_calls( } /// Extract individual invoke blocks from function_calls content -fn extract_invokes(block: &str, regexes: &DsmlRegexes) -> anyhow::Result> { +fn extract_invokes( + block: &str, + config: &DsmlParserConfig, +) -> anyhow::Result> { let mut invokes = Vec::new(); - // Matches: <|DSML|invoke name="function_name">..content.. - for invoke_match in regexes.invoke.captures_iter(block) { + // Regex to match: <|DSML|invoke name="function_name">..content.. + // Note: invoke_start_prefix is "<|DSML|invoke name=" (no quotes, we add them in pattern) + let invoke_pattern = format!( + r#"(?s){}\"([^"]+)\"\s*>(.*?){}"#, + regex::escape(&config.invoke_start_prefix), + regex::escape(&config.invoke_end) + ); + let invoke_regex = Regex::new(&invoke_pattern)?; + + for invoke_match in invoke_regex.captures_iter(block) { if let (Some(name_match), Some(content_match)) = (invoke_match.get(1), invoke_match.get(2)) { let function_name = name_match.as_str().trim().to_string(); let invoke_content = content_match.as_str(); // Parse parameters from invoke content - let parameters = parse_parameters(invoke_content, regexes)?; + let parameters = parse_parameters(invoke_content, config)?; // Create tool call response let arguments_json = serde_json::to_string(¶meters)?; + // OpenAI-style id: "call_" + 24 lowercase hex chars. + // Take the simple (32-hex, no hyphens) form of a v4 UUID and truncate. + let uuid_simple = Uuid::new_v4().simple().to_string(); + let id = format!("call_{}", &uuid_simple[..24]); + invokes.push(ToolCallResponse { - id: format!("call-{}", Uuid::new_v4()), + id, tp: ToolCallType::Function, function: CalledFunction { name: function_name, @@ -221,29 +195,40 @@ fn extract_invokes(block: &str, regexes: &DsmlRegexes) -> anyhow::Result anyhow::Result> { let mut parameters = serde_json::Map::new(); - // Matches: <|DSML|parameter name="param_name" string="true|false">value - for param_match in regexes.parameter.captures_iter(content) { - if let (Some(name_match), Some(string_match), Some(value_match)) = - (param_match.get(1), param_match.get(2), param_match.get(3)) - { + // Build pattern with proper escaping + // Match: <|DSML|parameter name="param_name" string="true|false">value + // Note: parameter_prefix is "<|DSML|parameter name=" (no quotes, we add them in pattern) + let prefix_escaped = regex::escape(&config.parameter_prefix); + let end_escaped = regex::escape(&config.parameter_end); + + // The `string="true|false"` attribute is optional: some model outputs omit it. + // When absent we best-effort parse the value (JSON → String fallback). + let param_pattern = format!( + r#"(?s){}\"([^"]+)\"(?:\s+string=\"(true|false)\")?\s*>(.*?){}"#, + prefix_escaped, end_escaped + ); + + let param_regex = Regex::new(¶m_pattern)?; + + for param_match in param_regex.captures_iter(content) { + if let (Some(name_match), Some(value_match)) = (param_match.get(1), param_match.get(3)) { let param_name = name_match.as_str().trim(); - let is_string = string_match.as_str() == "true"; let param_value = value_match.as_str().trim(); - // Parse value based on string attribute - let value = if is_string { - // String type - use as-is + // Parse value based on string attribute (if present). + // `string="true"` forces the String branch; every other case + // (`string="false"` or attribute omitted) tries JSON first and + // falls back to String. + let string_attr = param_match.get(2).map(|m| m.as_str()); + let value = if string_attr == Some("true") { serde_json::Value::String(param_value.to_string()) } else { - // Non-string type - parse as JSON - serde_json::from_str(param_value).unwrap_or_else(|_| { - // Fallback to string if JSON parsing fails - serde_json::Value::String(param_value.to_string()) - }) + serde_json::from_str(param_value) + .unwrap_or_else(|_| serde_json::Value::String(param_value.to_string())) }; parameters.insert(param_name.to_string(), value); @@ -268,8 +253,8 @@ mod tests { fn get_v4_test_config() -> DsmlParserConfig { DsmlParserConfig { - function_calls_start: "<|DSML|tool_calls>".to_string(), - function_calls_end: "".to_string(), + block_start: "<|DSML|tool_calls>".to_string(), + block_end: "".to_string(), ..Default::default() } } @@ -464,7 +449,10 @@ mod tests { let config = get_v4_test_config(); let (calls, normal) = try_tool_call_parse_dsml(input, &config).unwrap(); assert_eq!(calls.len(), 2); - assert_eq!(normal, Some("Let's check this.".to_string())); + // Tolerant match: preamble must carry the prose; whitespace is + // implementation-defined. + let normal = normal.unwrap(); + assert_eq!(normal.trim(), "Let's check this."); let (name1, args1) = extract_name_and_args(calls[0].clone()); assert_eq!(name1, "get_favorite_tourist_spot"); @@ -506,7 +494,31 @@ mod tests { let config = get_test_config(); let (calls, normal) = try_tool_call_parse_dsml(input, &config).unwrap(); assert_eq!(calls.len(), 1); - assert_eq!(normal, Some("Here's the result:".to_string())); + // Tolerant whitespace match. + let normal = normal.unwrap(); + assert_eq!(normal.trim(), "Here's the result:"); + } + + #[test] + fn test_parse_preserves_whitespace_before_dsml_block() { + // vLLM preserves whitespace verbatim before the DSML block; the parser + // must as well so clients see identical prompts across servers. + let input = "Let me check the forecast.\n\n<|DSML|tool_calls> +<|DSML|invoke name=\"get_weather\"> +<|DSML|parameter name=\"city\" string=\"true\">SF + +"; + + let config = get_v4_test_config(); + let (calls, normal) = try_tool_call_parse_dsml(input, &config).unwrap(); + assert_eq!(calls.len(), 1); + let normal = normal.unwrap(); + assert!( + normal.ends_with("\n\n"), + "Expected trailing \\n\\n preserved, got {:?}", + normal + ); + assert_eq!(normal, "Let me check the forecast.\n\n"); } #[test] @@ -651,6 +663,181 @@ mod tests { } #[test] + fn test_empty_invokes_does_not_leak_dsml_markup() { + // Valid block-start + mangled content (invoke tag but no closing/params) + // followed by block-end. extract_tool_calls returns empty; we must not + // leak DSML markup into normal_content. + let input = "Let me check. <|DSML|tool_calls>\n<|DSML|invoke name=\"broken\">\n"; + + let config = get_v4_test_config(); + let (calls, normal) = try_tool_call_parse_dsml(input, &config).unwrap(); + + assert!( + calls.is_empty(), + "Expected no tool calls, got {}", + calls.len() + ); + let normal = normal.unwrap(); + assert!( + normal.contains("Let me check."), + "Expected preamble in normal_content, got {:?}", + normal + ); + assert!( + !normal.contains("<|DSML|"), + "normal_content leaked DSML markup: {:?}", + normal + ); + } + + #[test] + fn test_parse_parameter_missing_string_attribute() { + // Model emits a parameter without the `string="..."` attribute. + // The parser should best-effort parse: JSON first, then fall back to string. + let input = r#"<|DSML|function_calls> +<|DSML|invoke name="greet"> +<|DSML|parameter name="name">Alice + +"#; + + let config = get_test_config(); + let (calls, _) = try_tool_call_parse_dsml(input, &config).unwrap(); + assert_eq!(calls.len(), 1); + + let (name, args) = extract_name_and_args(calls[0].clone()); + assert_eq!(name, "greet"); + assert_eq!(args["name"], "Alice"); + } + + #[test] + fn test_parse_string_false_with_bare_word_value() { + // `string="false"` with a non-JSON bare word should still appear + // in the arguments (as the string fallback). + let input = r#"<|DSML|function_calls> +<|DSML|invoke name="run"> +<|DSML|parameter name="mode" string="false">quickly + +"#; + + let config = get_test_config(); + let (calls, _) = try_tool_call_parse_dsml(input, &config).unwrap(); + assert_eq!(calls.len(), 1); + + let (_, args) = extract_name_and_args(calls[0].clone()); + assert_eq!(args["mode"], "quickly"); + } + + #[test] + fn test_tool_call_id_format_openai_style() { + let input = r#"<|DSML|function_calls> +<|DSML|invoke name="get_weather"> +<|DSML|parameter name="location" string="true">San Francisco + +"#; + + let config = get_test_config(); + let (calls, _) = try_tool_call_parse_dsml(input, &config).unwrap(); + assert_eq!(calls.len(), 1); + + // Shape-only assertion: OpenAI-style `call_` prefix + at least 20 + // lowercase alphanumeric characters. We intentionally do NOT pin the + // exact length / alphabet so the id generator can evolve without + // churning this test. + let id = &calls[0].id; + assert!( + id.starts_with("call_"), + "id should start with call_: {}", + id + ); + let suffix = &id["call_".len()..]; + assert!( + suffix.len() >= 20, + "suffix must be at least 20 chars: {}", + suffix + ); + assert!( + suffix + .bytes() + .all(|b| b.is_ascii_lowercase() || b.is_ascii_digit()), + "suffix must match [a-z0-9]+: {}", + suffix + ); + } + + #[test] + fn test_multi_block_preserves_inter_and_trailing_text() { + // Two complete DSML blocks with text before, between, and after. + // Both blocks must be parsed AND the inter-block / trailing text must + // survive in normal_content. + let input = "pre <|DSML|tool_calls>\n<|DSML|invoke name=\"a\">\n\n middle <|DSML|tool_calls>\n<|DSML|invoke name=\"b\">\n\n tail"; + + let config = get_v4_test_config(); + let (calls, normal) = try_tool_call_parse_dsml(input, &config).unwrap(); + assert_eq!(calls.len(), 2, "expected both blocks parsed"); + assert_eq!(calls[0].function.name, "a"); + assert_eq!(calls[1].function.name, "b"); + + let normal = normal.unwrap(); + assert!( + normal.contains(" middle "), + "inter-block text lost: {:?}", + normal + ); + assert!(normal.contains(" tail"), "trailing text lost: {:?}", normal); + assert!( + !normal.contains("<|DSML|"), + "normal_content leaked DSML markup: {:?}", + normal + ); + } + + #[test] + fn test_unterminated_block_followed_by_valid_block() { + // An unterminated DSML start appears before a complete block. The + // non-greedy block regex spans from the FIRST block_start to the + // sole block_end, swallowing the nested second block_start as part + // of the block content. + // + // Within that captured span the non-greedy invoke regex pairs the + // FIRST `` with the FIRST `` — so + // one tool call is recovered under the name "broken". This is the + // observed contract today; the test locks it in so any future + // behavior change is explicit rather than silent. + let input = "pre <|DSML|tool_calls>\n<|DSML|invoke name=\"broken\">\n mid <|DSML|tool_calls>\n<|DSML|invoke name=\"ok\">\n\n tail"; + + let config = get_v4_test_config(); + let (calls, normal) = try_tool_call_parse_dsml(input, &config).unwrap(); + + assert_eq!(calls.len(), 1, "exactly one invoke recovered"); + assert_eq!( + calls[0].function.name, "broken", + "outer invoke name is matched first (non-greedy)" + ); + + let normal = normal.unwrap(); + assert!( + normal.starts_with("pre"), + "pre-block text must survive: {:?}", + normal + ); + assert!( + normal.contains(" tail"), + "trailing text must survive: {:?}", + normal + ); + assert!( + !normal.contains("<|DSML|tool_calls>"), + "normal_content leaked block_start: {:?}", + normal + ); + assert!( + !normal.contains(""), + "normal_content leaked block_end: {:?}", + normal + ); + } + + #[test] // CASE.7, CASE.14 fn test_parse_null_parameter() { let input = r#"<|DSML|function_calls> <|DSML|invoke name="test"> @@ -683,7 +870,12 @@ mod tests { /// parser gained end-token recovery; see /// `kimi_k2_parser.rs::test_parse_malformed_no_section_end` for the /// post-fix recovery pattern. - #[test] + /// + /// Note: post-hardening, the parser no longer leaks raw DSML markup + /// into `normal_text` when block-start appears but no invokes parse — + /// it returns the pre-block text only (empty here, since the input + /// starts with the block-start fence). The call is still dropped. + #[test] // CASE.5, CASE.23 — V4 variant fn test_parse_deepseek_v4_missing_end_token() { // Start fence + complete invoke, but no . let input = "<|DSML|tool_calls>\n\ @@ -702,8 +894,9 @@ mod tests { ); assert_eq!( normal_text.as_deref(), - Some(input), - "Unrecovered payload should fall through to normal_text verbatim." + Some(""), + "Pre-block text is empty here; raw DSML markup must not leak \ + into normal_text (post-hardening behavior)." ); } diff --git a/lib/parsers/src/tool_calling/parsers.rs b/lib/parsers/src/tool_calling/parsers.rs index c3b0c9f34ab1..13da760491ac 100644 --- a/lib/parsers/src/tool_calling/parsers.rs +++ b/lib/parsers/src/tool_calling/parsers.rs @@ -1707,7 +1707,6 @@ Remember, San Francisco weather can be quite unpredictable, particularly with it assert_eq!(args["topn"], 10); // Should be number, not string assert_eq!(args["source"], "web"); } - /// `CASE.1` — single-call happy path (V4). #[tokio::test] async fn test_deepseek_v4_single_tool_call() { @@ -1731,7 +1730,6 @@ Remember, San Francisco weather can be quite unpredictable, particularly with it assert_eq!(args["timezone"], "Asia/Shanghai"); } /// Alias registration: verifies `deepseek-v4` and `deepseekv4` route to the same parser as `deepseek_v4`. Not a CASE.*; covers registry plumbing. - #[tokio::test] async fn test_deepseek_v4_compatibility_aliases() { let input = r#"<|DSML|tool_calls>