-
Notifications
You must be signed in to change notification settings - Fork 18
fix(tool_calling): recover DeepSeek DSML tool calls into canonical format #151
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6c38511
a1a6846
06bface
277fb2e
3fa54ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -206,23 +206,194 @@ static TOOL_CALL_TAG_RE: LazyLock<Regex> = | |
| /// `Borrowed` no-op unless a *piped* tag is actually present, so well-formed | ||
| /// output — which the base parser already handles — and P-Format pipe args are | ||
| /// untouched. | ||
| static DSML_CALLS_BLOCK_RE: LazyLock<Regex> = LazyLock::new(|| { | ||
| Regex::new( | ||
| r"(?is)<[||]{1,2}\s*DSML\s*[||]{1,2}\s*calls?\s*>(.*?)(?:</[||]{1,2}\s*DSML\s*[||]{1,2}\s*calls?\s*>|<[||]{1,2}\s*DSML\s*[||]{1,2}\s*/calls?\s*>|</tool_call>|$)", | ||
| ) | ||
| .unwrap() | ||
| }); | ||
|
|
||
| static DSML_INVOKE_OPEN_RE: LazyLock<Regex> = LazyLock::new(|| { | ||
| Regex::new( | ||
| r#"(?is)<(?:[||]{1,2}\s*DSML\s*[||]{1,2}\s*)?invoke\s+name\s*=\s*"([^"]+)"[^>]*>"#, | ||
| ) | ||
| .unwrap() | ||
| }); | ||
|
|
||
| static DSML_INVOKE_CLOSE_RE: LazyLock<Regex> = LazyLock::new(|| { | ||
| Regex::new(r"(?is)</(?:[||]{1,2}\s*DSML\s*[||]{1,2}\s*)?invoke\s*>|</tool_call>").unwrap() | ||
| }); | ||
|
|
||
| static DSML_PARAMETER_OPEN_RE: LazyLock<Regex> = LazyLock::new(|| { | ||
| Regex::new( | ||
| r#"(?is)<(?:[||]{1,2}\s*DSML\s*[||]{1,2}\s*)?parameter(?:\s+name\s*=\s*"([^"]*)")?[^>]*>"#, | ||
| ) | ||
| .unwrap() | ||
| }); | ||
|
|
||
| static DSML_PARAMETER_CLOSE_RE: LazyLock<Regex> = LazyLock::new(|| { | ||
| Regex::new(r"(?is)</(?:[||]{1,2}\s*DSML\s*[||]{1,2}\s*)?parameter\s*>").unwrap() | ||
| }); | ||
|
|
||
| /// Parse arguments from a DSML `<invoke>` block body. | ||
| /// | ||
| /// Handles: | ||
| /// 1. `<parameter name="...">value</parameter>` tags (single `arguments` envelope or multiple named params). | ||
| /// 2. Direct JSON object bodies (when the model omits `<parameter>` tags or emits only a closing `</parameter>` tag). | ||
| /// 3. Empty bodies (`{}`). | ||
| fn parse_dsml_invoke_arguments(body: &str) -> serde_json::Value { | ||
| let param_matches: Vec<_> = DSML_PARAMETER_OPEN_RE.captures_iter(body).collect(); | ||
| let mut named_params = Vec::new(); | ||
| for (i, cap) in param_matches.iter().enumerate() { | ||
| let name = cap.get(1).map(|m| m.as_str().trim()).unwrap_or(""); | ||
| let start = cap.get(0).unwrap().end(); | ||
| let end = if i + 1 < param_matches.len() { | ||
| param_matches[i + 1].get(0).unwrap().start() | ||
| } else { | ||
| body.len() | ||
| }; | ||
| let mut raw_val = &body[start..end]; | ||
| if let Some(close_m) = DSML_PARAMETER_CLOSE_RE.find(raw_val) { | ||
| raw_val = &raw_val[..close_m.start()]; | ||
| } | ||
| let val = raw_val.trim(); | ||
| if !name.is_empty() { | ||
| named_params.push((name, val)); | ||
| } | ||
| } | ||
|
|
||
| if !named_params.is_empty() { | ||
| if named_params.len() == 1 && TOOL_ARG_KEYS.contains(&named_params[0].0) { | ||
| let val_str = named_params[0].1; | ||
| if let Some((json_val, _)) = extract_first_json_value_with_end(val_str) | ||
| && json_val.is_object() | ||
| { | ||
| return json_val; | ||
| } | ||
| if let Ok(json_val) = serde_json::from_str::<serde_json::Value>(val_str) { | ||
| if json_val.is_object() { | ||
| return json_val; | ||
| } | ||
| return serde_json::json!({ named_params[0].0: json_val }); | ||
| } | ||
| return serde_json::json!({ named_params[0].0: val_str }); | ||
| } | ||
|
|
||
| let mut map = serde_json::Map::new(); | ||
| for (pname, pval_str) in named_params { | ||
| map.insert(pname.to_string(), parameter_scalar_value(pval_str)); | ||
| } | ||
| return serde_json::Value::Object(map); | ||
| } | ||
|
|
||
| // Bare JSON object directly inside invoke body (with or without unclosed parameter tags) | ||
| if let Some((json_val, _)) = extract_first_json_value_with_end(body) | ||
| && json_val.is_object() | ||
| { | ||
| return json_val; | ||
| } | ||
|
|
||
| let trimmed = body.trim(); | ||
| if trimmed.is_empty() { | ||
| return serde_json::Value::Object(serde_json::Map::new()); | ||
| } | ||
|
|
||
| serde_json::json!({ "input": trimmed }) | ||
| } | ||
|
|
||
| /// Normalize DeepSeek DSML tool calls (`<||DSML|| calls><||DSML|| invoke name="...">...</||DSML|| calls>`) | ||
| /// into canonical `<tool_call>` tags with JSON payloads. | ||
| /// | ||
| /// DeepSeek models (such as DeepSeek-V3, DeepSeek-V4, DeepSeek-Flash) emit DSML syntax when asked | ||
| /// to invoke tools in text/P-Format mode. Without this normalization, OpenHuman treats DSML tags | ||
| /// as narrative text and fails to execute the tool calls. | ||
| fn normalize_dsml_tool_calls(s: &str) -> Cow<'_, str> { | ||
| if !s.contains("DSML") && !s.contains("dsml") { | ||
| return Cow::Borrowed(s); | ||
|
Comment on lines
+311
to
+312
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Use the case-insensitive matcher for the fast path. The regular expression accepts mixed-case markers, but this check accepts only Remove this check, or perform an ASCII case-insensitive search. 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| if !DSML_CALLS_BLOCK_RE.is_match(s) { | ||
| return Cow::Borrowed(s); | ||
| } | ||
|
|
||
| let mut out = String::with_capacity(s.len()); | ||
| let mut cursor = 0; | ||
|
|
||
| for mat in DSML_CALLS_BLOCK_RE.find_iter(s) { | ||
| let block_start = mat.start(); | ||
| let block_end = mat.end(); | ||
| out.push_str(&s[cursor..block_start]); | ||
|
|
||
| let block_match = DSML_CALLS_BLOCK_RE.captures(&s[block_start..block_end]); | ||
| let inner = block_match | ||
| .and_then(|c| c.get(1)) | ||
| .map(|m| m.as_str()) | ||
| .unwrap_or(""); | ||
|
|
||
| let invoke_matches: Vec<_> = DSML_INVOKE_OPEN_RE.captures_iter(inner).collect(); | ||
| let mut recovered_any = false; | ||
| for (i, inv_cap) in invoke_matches.iter().enumerate() { | ||
| let name = inv_cap.get(1).map(|m| m.as_str().trim()).unwrap_or(""); | ||
| if name.is_empty() { | ||
| continue; | ||
| } | ||
| let body_start = inv_cap.get(0).unwrap().end(); | ||
| let body_end = if i + 1 < invoke_matches.len() { | ||
| invoke_matches[i + 1].get(0).unwrap().start() | ||
| } else { | ||
| inner.len() | ||
| }; | ||
| let mut raw_body = &inner[body_start..body_end]; | ||
| if let Some(close_m) = DSML_INVOKE_CLOSE_RE.find(raw_body) { | ||
| raw_body = &raw_body[..close_m.start()]; | ||
| } | ||
|
|
||
| let arguments = parse_dsml_invoke_arguments(raw_body); | ||
|
|
||
| let payload = serde_json::json!({ | ||
| "name": name, | ||
| "arguments": arguments, | ||
| }); | ||
| out.push_str("<tool_call>\n"); | ||
| out.push_str(&payload.to_string()); | ||
| out.push_str("\n</tool_call>"); | ||
| recovered_any = true; | ||
| } | ||
|
|
||
| if !recovered_any { | ||
| out.push_str(&s[block_start..block_end]); | ||
|
Comment on lines
+363
to
+364
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '205,340p' crates/tinyagents-harness/src/tool_calling/parse.rs
sed -n '500,660p' crates/tinyagents-harness/src/tool_calling/parse_test.rsRepository: tinyhumansai/tinyagents Length of output: 11380 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- normalization implementation ---'
sed -n '270,345p' crates/tinyagents-harness/src/tool_calling/parse.rs
printf '%s\n' '--- normalization-related tests ---'
rg -n -C 5 'normalize_dsml|dsml_.*(narrative|missing|partial|malformed|closing)|DSML.*calls' crates/tinyagents-harness/src/tool_calling/parse_test.rs crates/tinyagents-harness/src/tool_calling/parse.rsRepository: tinyhumansai/tinyagents Length of output: 18397 Preserve unmatched content in partial DSML blocks. When Preserve unmatched ranges in 🤖 Prompt for AI Agents |
||
| } else { | ||
| tinyagents_tracing::debug!( | ||
| "[agent_parse] normalized DeepSeek DSML tool calls into canonical <tool_call> tags" | ||
| ); | ||
| } | ||
|
|
||
| cursor = block_end; | ||
| } | ||
|
|
||
| out.push_str(&s[cursor..]); | ||
| Cow::Owned(out) | ||
| } | ||
|
|
||
| fn normalize_garbled_tool_call_tags(s: &str) -> Cow<'_, str> { | ||
| let s = normalize_dsml_tool_calls(s); | ||
| let s_ref = s.as_ref(); | ||
| // Garbling always leaks a `|` into a tag; no `|` anywhere → nothing to do. | ||
| if !s.contains('|') { | ||
| return Cow::Borrowed(s); | ||
| if !s_ref.contains('|') { | ||
| return s; | ||
| } | ||
| let tags: Vec<(usize, usize)> = TOOL_CALL_TAG_RE | ||
| .find_iter(s) | ||
| .find_iter(s_ref) | ||
| .map(|m| (m.start(), m.end())) | ||
| .collect(); | ||
| // Need at least one open/close pair, and at least one tag must actually be | ||
| // garbled (contain a pipe) — otherwise the base parser handles it verbatim, | ||
| // and P-Format `name[a|b]` args (pipes in the BODY, not the tags) are left | ||
| // alone. | ||
| if tags.len() < 2 || !tags.iter().any(|&(a, b)| s[a..b].contains('|')) { | ||
| return Cow::Borrowed(s); | ||
| if tags.len() < 2 || !tags.iter().any(|&(a, b)| s_ref[a..b].contains('|')) { | ||
| return s; | ||
| } | ||
| let mut out = String::with_capacity(s.len()); | ||
| let mut out = String::with_capacity(s_ref.len()); | ||
| let mut cursor = 0usize; | ||
| // `as_chunks::<2>()` rather than `chunks_exact(2)`: the chunk size is a | ||
| // constant, so this hands back fixed-size arrays and the two destructurings | ||
|
|
@@ -231,13 +402,13 @@ fn normalize_garbled_tool_call_tags(s: &str) -> Cow<'_, str> { | |
| for &[open, close] in tags.as_chunks::<2>().0 { | ||
| let (open_start, open_end) = open; | ||
| let (close_start, close_end) = close; | ||
| out.push_str(&s[cursor..open_start]); // text before the open tag, verbatim | ||
| out.push_str(&s_ref[cursor..open_start]); // text before the open tag, verbatim | ||
| out.push_str("<tool_call>"); | ||
| // Strip the `call:` prefix, then try to recover a Kimi-family | ||
| // `NAME{…}` argument-sentinel body into canonical JSON (#5119). When | ||
| // the body is already canonical JSON / P-Format the recovery is a no-op | ||
| // and the stripped body flows through unchanged. | ||
| let stripped = strip_call_prefix(&s[open_end..close_start]); | ||
| let stripped = strip_call_prefix(&s_ref[open_end..close_start]); | ||
| match recover_sentinel_tool_call_body(stripped) { | ||
| Some(recovered) => { | ||
| // Recovered a Kimi-family `NAME{…}` sentinel body into canonical | ||
|
|
@@ -269,7 +440,7 @@ fn normalize_garbled_tool_call_tags(s: &str) -> Cow<'_, str> { | |
| cursor = close_end; | ||
| } | ||
| // Trailing text, plus any final unpaired tag, verbatim. | ||
| out.push_str(&s[cursor..]); | ||
| out.push_str(&s_ref[cursor..]); | ||
| Cow::Owned(out) | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: tinyhumansai/tinyagents
Length of output: 5222
🏁 Script executed:
Repository: tinyhumansai/tinyagents
Length of output: 50382
🏁 Script executed:
Repository: tinyhumansai/tinyagents
Length of output: 50382
🏁 Script executed:
Repository: tinyhumansai/tinyagents
Length of output: 22384
Map an empty
argumentsenvelope to{}.When the parameter value is empty or whitespace-only,
DSML_PARAMETER_REtrims it to an emptyval_str. This branch then emits{"arguments":{"arguments":""}}in the canonical tool-call payload instead of{}. Add the empty check before JSON parsing.Proposed fix
if named_params.len() == 1 && TOOL_ARG_KEYS.contains(&named_params[0].0) { let val_str = named_params[0].1; + if val_str.is_empty() { + return serde_json::json!({}); + } if let Some((json_val, _)) = extract_first_json_value_with_end(val_str) {📝 Committable suggestion
🤖 Prompt for AI Agents