feat(tool_parser): add DeepSeek V3.2 DSML tool call parser#1030
feat(tool_parser): add DeepSeek V3.2 DSML tool call parser#1030
Conversation
Signed-off-by: key4ng <rukeyang@gmail.com>
Signed-off-by: key4ng <rukeyang@gmail.com>
Signed-off-by: key4ng <rukeyang@gmail.com>
github-actions
bot
added
tests
📝 WalkthroughWalkthroughAdds a new DeepSeek V3.2 parser ( Changes
Sequence Diagram(s)(Skipped) Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cc2419261
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let has_partial_prefix = current_text.ends_with('<') | ||
| || current_text.ends_with("<｜") | ||
| || current_text.ends_with("</") | ||
| || current_text.ends_with("</｜"); | ||
|
|
||
| if !has_dsml && !has_partial_prefix { | ||
| let mut normal_text = std::mem::take(&mut self.buffer); |
There was a problem hiding this comment.
The streaming gate only recognizes four very short suffixes (<, <｜, </, </｜) as partial DSML prefixes, so a chunk that ends in a longer split marker like <｜DSML｜funct or <｜DSML｜inv falls into the !has_dsml && !has_partial_prefix branch and is flushed as normal text. Once that prefix is discarded, the next chunk cannot reconstruct the DSML block and the tool call is never parsed, which makes tool calling depend on lucky chunk boundaries.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
<｜DSML｜function_calls> is a single special token from the tokenizer — it arrives atomically, not split across chunks. Same pattern as V3/V3.1 parsers. SGLang checks for ｜DSML｜ substring as an extra safety net, but in practice this doesn't happen in the gRPC streaming path.
| if let Some(end) = match_end { | ||
| self.buffer = self.buffer[end..].to_string(); |
There was a problem hiding this comment.
When a chunk contains both assistant text and a DSML invoke block, the parser advances the buffer to self.buffer[end..] after matching the invoke, which drops everything before the match. Because parse_incremental always returns empty normal_text in this path, the pre-tool assistant text is lost instead of being streamed to the client.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pre-existing pattern across all parsers (V3, V3.1). In the gRPC streaming path, text content and tool call tokens arrive in separate chunks. SGLang has the same behavior — returns empty normal_text when in tool-call mode.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| self.buffer = self.buffer[end..].to_string(); | ||
| } | ||
| } | ||
| break; |
There was a problem hiding this comment.
🔴 Important: break should be continue when the invalid invoke is complete.
When an invalid tool name appears in a complete invoke block followed by valid invokes in the same buffer, this break stops the loop and the valid tools are never processed until the next chunk arrives. If this is the final chunk (e.g., the </｜DSML｜function_calls> closing tag was in the same chunk), the valid tools after the invalid one are lost entirely — they won't be in prev_tool_call_arr so get_unstreamed_tool_args won't recover them either.
| break; | |
| continue; |
The break for the !is_complete (partial) case below is still needed — you can't skip past an incomplete invoke. Consider splitting:
if is_complete {
if let Some(end) = match_end {
self.buffer = self.buffer[end..].to_string();
}
continue; // try next invoke in buffer
} else {
break; // wait for more data
}There was a problem hiding this comment.
Fixed in cdff142 — complete invalid invokes now continue to process remaining valid invokes. Incomplete invalid invokes call reset_current_tool_state and return early.
| tools: &[Tool], | ||
| ) -> ParserResult<StreamingParseResult> { | ||
| self.buffer.push_str(chunk); | ||
| let current_text = self.buffer.clone(); |
There was a problem hiding this comment.
🟡 Nit: Unnecessary buffer clone on every parse_incremental call. current_text is only used for contains() / ends_with() checks that could read &self.buffer directly. In the streaming hot path (called per-token), this allocation adds up.
| let current_text = self.buffer.clone(); | |
| let current_text = &self.buffer; |
(This also requires changing current_text.ends_with(...) etc. below to borrow, but since they already operate on &str it should be a no-op change.)
There was a problem hiding this comment.
Can't use a reference — self.buffer is mutated later in the method (on complete invoke: self.buffer = self.buffer[end..].to_string()). The borrow checker would reject it. Same pattern as V3/V3.1 parsers.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the DeepSeek32Parser to support the DeepSeek V3.2 DSML tool-calling format, providing both complete and incremental parsing. The parser is integrated into the ParserFactory with mappings for V3.2 and V3.2-Exp models, and integration tests are included. Feedback suggests improving the robustness of parameter parsing and using warning-level logging for invalid tool names.
| serde_json::from_str(value.trim()) | ||
| .unwrap_or_else(|_| Value::String(value.to_string())) |
There was a problem hiding this comment.
Using serde_json::from_str on a potentially malformed or partial string inside a loop can be inefficient and error-prone. Since the value is already trimmed, consider validating the JSON structure or using a more robust parsing approach for the XML parameter values.
There was a problem hiding this comment.
This parses string="false" parameter values — typically 1-3 per tool call. The fallback to Value::String on parse failure is the intended robust handling. Same approach as SGLang's _parse_parameters_from_xml.
|
|
||
| // Skip if tool name is not in provided tools list | ||
| if !func_name.is_empty() && !tool_indices.contains_key(func_name.as_str()) { | ||
| tracing::debug!("Invalid tool name '{}' - skipping", func_name); |
There was a problem hiding this comment.
Instead of using tracing::debug!, use tracing::warn! for invalid tool names. This aligns with the repository rule to log potential failures as warnings to aid in debugging, ensuring they are captured in production environments.
References
- Instead of silently ignoring potential failures (e.g., from serialization), log them as warnings to aid in debugging.
There was a problem hiding this comment.
All other parsers in the crate (deepseek.rs, deepseek31.rs, kimik2.rs, etc.) use tracing::debug\! for invalid tool names. Keeping consistent.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/tool_parser/src/parsers/deepseek32.rs`:
- Line 313: Remove the explicit drop(captures); statement in deepseek32.rs: the
local variable captures (which borrows buf_snapshot) will go out of scope
naturally, so delete the drop call and ensure there are no further references to
captures after its intended use (verify the surrounding code in the function
where captures and buf_snapshot are used).
- Around line 370-398: The argument diff logic in argument_diff can be
simplified: when is_complete is false and you have a prev_args (from
self.prev_tool_call_arr) and DSML parameters only ever accumulate, replace the
find_common_prefix-based branching inside the else-if that checks let Some(prev)
= &prev_args with a direct slice from sent_len into current_args (i.e., treat
the new content as current_args[sent_len..].to_string()); this removes the
prefix computation while preserving behavior for monotonic accumulation—keep the
existing handling for the is_complete branch and the None cases, and only change
the block that currently calls helpers::find_common_prefix and compares
prefix.len() to sent_len.
- Around line 316-324: The invalid-tool branch currently breaks leaving parser
state stale and preventing processing of remaining invokes; change it to follow
the pattern used in other parsers: when func_name is invalid and is_complete is
true, advance self.buffer via match_end (if Some(end)) and then reset parser
state (clear streamed_args_for_tool and set current_tool_name_sent = false) and
continue the loop instead of break; when func_name is invalid and is_complete is
false, reset the same state (clear streamed_args_for_tool and set
current_tool_name_sent = false) and return/exit early so partial invokes are
dropped and state is clean for the next chunk.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6a76df48-375a-4a91-898f-0a879f03acd3
📒 Files selected for processing (5)
crates/tool_parser/src/factory.rscrates/tool_parser/src/lib.rscrates/tool_parser/src/parsers/deepseek32.rscrates/tool_parser/src/parsers/mod.rscrates/tool_parser/tests/tool_parser_deepseek32.rs
| let prev_args = if tool_id < self.prev_tool_call_arr.len() { | ||
| self.prev_tool_call_arr[tool_id] | ||
| .get("arguments") | ||
| .and_then(|v| v.as_str()) | ||
| .map(|s| s.to_string()) | ||
| } else { | ||
| None | ||
| }; | ||
|
|
||
| let argument_diff = if is_complete { | ||
| if sent_len < current_args.len() { | ||
| Some(current_args[sent_len..].to_string()) | ||
| } else { | ||
| Some(String::new()) | ||
| } | ||
| } else if let Some(prev) = &prev_args { | ||
| if current_args == *prev { | ||
| None | ||
| } else { | ||
| let prefix = helpers::find_common_prefix(prev, ¤t_args); | ||
| if prefix.len() > sent_len { | ||
| Some(prefix[sent_len..].to_string()) | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
| } else { | ||
| None | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider simplifying argument diff logic.
The diff computation for partial invokes is more complex than necessary. The find_common_prefix approach works but the comparison against prev_args (which is also a JSON string) adds complexity. Since DSML parameters arrive as discrete tags that are parsed into a complete JSON object each time, a simpler approach might suffice:
For the non-complete case, you're comparing two JSON strings that may have different content (not just different lengths). The find_common_prefix handles the case where the JSON structure might change between chunks, but in practice DSML parameters accumulate monotonically.
💡 Optional: Simplify to match V3.1 pattern when applicable
If parameters only accumulate (never change), the diffing could be simplified to always use sent_len slicing:
- let argument_diff = if is_complete {
- if sent_len < current_args.len() {
- Some(current_args[sent_len..].to_string())
- } else {
- Some(String::new())
- }
- } else if let Some(prev) = &prev_args {
- if current_args == *prev {
- None
- } else {
- let prefix = helpers::find_common_prefix(prev, ¤t_args);
- if prefix.len() > sent_len {
- Some(prefix[sent_len..].to_string())
- } else {
- None
- }
- }
- } else {
- None
- };
+ // DSML parameters accumulate monotonically, so we can safely diff by length
+ let argument_diff = if current_args.len() > sent_len {
+ Some(current_args[sent_len..].to_string())
+ } else {
+ None
+ };However, the current approach is safer if parameter values can change mid-stream (e.g., due to partial tag content being updated). Keep as-is if that scenario is possible.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/tool_parser/src/parsers/deepseek32.rs` around lines 370 - 398, The
argument diff logic in argument_diff can be simplified: when is_complete is
false and you have a prev_args (from self.prev_tool_call_arr) and DSML
parameters only ever accumulate, replace the find_common_prefix-based branching
inside the else-if that checks let Some(prev) = &prev_args with a direct slice
from sent_len into current_args (i.e., treat the new content as
current_args[sent_len..].to_string()); this removes the prefix computation while
preserving behavior for monotonic accumulation—keep the existing handling for
the is_complete branch and the None cases, and only change the block that
currently calls helpers::find_common_prefix and compares prefix.len() to
sent_len.
There was a problem hiding this comment.
Keeping the find_common_prefix approach — with partial parameter parsing, the JSON can change structure between chunks (e.g., a partial non-string value parsed as string then corrected to the real type). The common-prefix algorithm handles this safely. Matches SGLang's diffing logic.
There was a problem hiding this comment.
@key4ng, that makes complete sense — the partial non-string value scenario (e.g., a numeric parsed as a string mid-stream, then corrected once the full tag arrives) is a real edge case that the sent_len-only slice would mishandle by potentially re-emitting already-sent characters. The find_common_prefix approach correctly identifies only the genuinely new suffix. Thanks for the clarification!
✏️ Learnings added
Learnt from: key4ng
Repo: lightseekorg/smg PR: 1030
File: crates/tool_parser/src/parsers/deepseek32.rs:363-391
Timestamp: 2026-04-03T01:54:42.272Z
Learning: In `crates/tool_parser/src/parsers/deepseek32.rs`, the `find_common_prefix`-based argument diffing in `parse_incremental` is intentional and should not be simplified to a `sent_len`-only slice. With DSML partial parameter parsing, a non-string parameter value can first be emitted as a `Value::String` (partial, fallback) and then corrected to its true JSON type once the closing tag arrives, changing the JSON structure between chunks. The common-prefix algorithm handles this safely and matches SGLang's diffing logic.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: key4ng
Repo: lightseekorg/smg PR: 1006
File: crates/tool_parser/src/parsers/deepseek31.rs:186-188
Timestamp: 2026-04-01T04:15:45.938Z
Learning: For the DeepSeek parser implementations in crates/tool_parser/src/parsers/deepseek*.rs (including deepseek31.rs and deepseek.rs), do not flag the streaming multi-tool-call buffering logic as incorrect. The use of a greedy partial_tool_call_regex with tool_call_end_pattern.find(), the lazy .*? behavior in the end-pattern, and advancing current_tool_id while draining the first complete JSON block from the buffer (and slicing the remaining text at mat.end()) is intentional to support sequential tool calls across chunks. The logic should only be considered correct if it drains exactly one completed tool-call block when is_complete_json triggers, advances current_tool_id, resets current_tool_name_sent, and leaves the remainder of the buffer for subsequent tool calls (matching the established V3 DeepSeekParser behavior).
Learnt from: key4ng
Repo: lightseekorg/smg PR: 1006
File: crates/tool_parser/src/parsers/deepseek31.rs:162-181
Timestamp: 2026-04-01T04:14:42.282Z
Learning: In the DeepSeek parsers (`crates/tool_parser/src/parsers/deepseek.rs` and `crates/tool_parser/src/parsers/deepseek31.rs`), do not treat `parse_incremental` failing to split a plain-text prefix before the first tool marker within the same chunk as a bug. This is intentional for the gRPC streaming path: tokens are delivered individually such that normal text and tool-call markers never coexist in the same chunk. Only flag parsing issues if they break the realistic multi-chunk behavior covered by `test_deepseek31_streaming_text_before_tools`.
Learnt from: vschandramourya
Repo: lightseekorg/smg PR: 953
File: model_gateway/src/routers/grpc/regular/processor.rs:765-780
Timestamp: 2026-03-27T23:46:40.172Z
Learning: In repo lightseekorg/smg, file model_gateway/src/routers/grpc/regular/processor.rs: The finish_reason parsing in `process_non_streaming_completion_response` is intentionally NOT extracted to `utils::parse_finish_reason`. The completions endpoint uses OpenAI-style `Option<String>` from `openai_protocol::completion::CompletionChoice`, while `utils::parse_finish_reason` maps to the typed generate-endpoint format. These are different API contracts; do not flag the inline finish_reason parsing block in the completions processor as duplicate code or request extraction into the shared utility.
Learnt from: slin1237
Repo: lightseekorg/smg PR: 489
File: mesh/src/crdt.rs:296-299
Timestamp: 2026-02-21T02:36:31.543Z
Learning: Repo lightseekorg/smg — For clippy/lint-only PRs (e.g., PR `#489`), avoid requesting stylistic doc comments when an item is already annotated with #[expect(...)] (e.g., #[expect(dead_code)] on SyncCRDTMap::contains_key in mesh/src/crdt.rs); such style changes are considered out of scope.
Learnt from: vschandramourya
Repo: lightseekorg/smg PR: 915
File: model_gateway/src/routers/grpc/client.rs:387-423
Timestamp: 2026-03-26T17:06:14.307Z
Learning: In repo lightseekorg/smg, in `model_gateway/src/routers/grpc/client.rs` and the corresponding backend builders (`crates/grpc_client/src/sglang_scheduler.rs`, `vllm_engine.rs`, `trtllm_service.rs`): The per-backend divergence in handling `CompletionRequest.max_tokens == None` is intentional. SGLang and vLLM pass `None` through to their proto builders, while TRT-LLM falls back to `16`. This matches the pre-existing per-backend pattern used in the chat/messages request builders. Do not flag this divergence as a bug or request normalization at the `build_completion_request` dispatcher layer in `client.rs`.
Learnt from: XinyueZhang369
Repo: lightseekorg/smg PR: 399
File: protocols/src/interactions.rs:505-509
Timestamp: 2026-02-19T03:08:50.192Z
Learning: In code reviews for Rust projects using the validator crate (v0.20.0), ensure that custom validation functions for numeric primitive types (e.g., f32, i32, u32, i16, etc.) accept the value by value, not by reference. Example: fn validate(value: f32) { ... }. The validator derive macro has a hardcoded list of numeric types that are passed by value, while all other types are passed by reference. Apply this guideline whenever validating numeric fields to align with the derive macro behavior.
Learnt from: slin1237
Repo: lightseekorg/smg PR: 489
File: model_gateway/src/core/token_bucket.rs:58-63
Timestamp: 2026-02-21T02:30:51.443Z
Learning: For lint-only/Clippy enforcement PRs in this repository, avoid introducing behavioral changes (e.g., new input validation or logic changes). Treat such PRs as non-functional changes and plan a separate follow-up issue/PR for hardening or behavior changes. This applies broadly to Rust files across the repo; during review, focus on lint/style corrections and clearly note any intentional exceptions.
Learnt from: slin1237
Repo: lightseekorg/smg PR: 489
File: protocols/src/responses.rs:928-931
Timestamp: 2026-02-21T02:36:00.882Z
Learning: In Rust code across the repository, use the marker INVARIANT: to document assumptions in safe code. Reserve SAFETY: for explaining why unsafe blocks are sound. This improves clarity of invariants and safety reasoning. Example reference: protocols/src/responses.rs near validate_tool_choice_with_tools().
Learnt from: slin1237
Repo: lightseekorg/smg PR: 489
File: mesh/src/sync.rs:83-83
Timestamp: 2026-02-21T02:37:01.416Z
Learning: General Rust formatting rule: format! with implicit captures only supports simple identifiers, not full expressions like {state.model_id}. For cases where you want to interpolate a field or expression, bind the value first and interpolate the binding, e.g., let model_id = &state.model_id; and then use format!("policy:{}", model_id). In the specific file mesh/src/sync.rs, prefer format!("policy:{}", state.model_id) or bind to a local variable if you need named interpolation, to keep clarity and avoid unintended captures.
Learnt from: zhaowenzi
Repo: lightseekorg/smg PR: 807
File: model_gateway/src/middleware.rs:61-81
Timestamp: 2026-03-18T21:32:00.041Z
Learning: In Rust code using the http crate, HeaderMap::get() is effectively case-insensitive because HeaderName normalizes keys to lowercase on insertion and lookup. Do not require or perform explicit .to_lowercase() before HeaderMap::get() calls. Mark as not a concern for case-sensitivity in lookups; only consider normalization when inserting or comparing via HeaderName, not in lookups.
Learnt from: key4ng
Repo: lightseekorg/smg PR: 867
File: tui/src/app.rs:798-813
Timestamp: 2026-03-22T20:13:55.778Z
Learning: In this repo (lightseekorg/smg), treat the workspace `Cargo.toml`’s `package.rust-version` (MSRV) as the source of truth (e.g., `rust-version = "1.85"`). When reviewing Rust changes, do not flag usage of Rust language/library features that were stabilized on or before the MSRV (e.g., `Option::is_none_or`, stabilized in 1.82, is compatible with an MSRV of 1.85). Always verify the MSRV from the workspace `Cargo.toml` rather than relying on issue templates.
…r stripping Signed-off-by: key4ng <rukeyang@gmail.com>
…ation Signed-off-by: key4ng <rukeyang@gmail.com>
…reaking Signed-off-by: key4ng <rukeyang@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c7c1c2f6c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self.prev_tool_call_arr[tool_id] = serde_json::json!({ | ||
| "name": func_name, | ||
| "arguments": current_args, | ||
| }); |
There was a problem hiding this comment.
get_unstreamed_tool_args relies on helpers::get_unstreamed_args, which serializes the stored arguments field and compares it to raw streamed bytes. Here arguments is stored as a JSON string (current_args) rather than a JSON value, so recovery compares escaped text (e.g. "{...}") against raw {...} and can fail to flush missing bytes; in truncation cases it can emit escaped JSON-string arguments instead of an object. This breaks the end-of-stream argument recovery path used by the gateway.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pre-existing pattern — same as V3/V3.1 parsers. In gRPC streaming, text and tool tokens arrive in separate chunks.
| if let Some(end) = match_end { | ||
| self.buffer = self.buffer[end..].to_string(); | ||
| } | ||
| } | ||
| break; |
There was a problem hiding this comment.
When an invalid tool name is encountered in a complete <｜DSML｜invoke ...></｜DSML｜invoke>, the code trims that invoke from self.buffer and then breaks. If the same chunk already contains a subsequent valid invoke, it remains buffered and is only parsed if another delta arrives; if streaming ends on this chunk, the valid call is dropped. After advancing the buffer past the invalid invoke, this path should continue the loop instead of exiting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cdff142 — complete invalid invokes now continue to process remaining valid invokes in the buffer.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/tool_parser/src/parsers/deepseek32.rs`:
- Around line 336-381: The bug is that prev_tool_call_arr stores "arguments" as
an object but the code expects a string, so prev_args becomes None and the first
partial chunk is dropped; update the logic in the block computing argument_diff
(around parse_parameters_from_dsml, streamed_args_for_tool, prev_tool_call_arr
and helpers::find_common_prefix) to treat a missing/non-string previous
arguments as an empty string (or initialize "arguments" as an empty string when
setting prev_tool_call_arr) and then compute the diff from sent_len (i.e., if
prev_args is None treat prev = "" and emit current_args[sent_len..] when
!is_complete or when appropriate), ensuring the first partial arguments are
returned instead of None.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 574ade0e-dcfd-4409-a2d4-abea50e43965
📒 Files selected for processing (1)
crates/tool_parser/src/parsers/deepseek32.rs
| self.prev_tool_call_arr[tool_id] = serde_json::json!({ | ||
| "name": func_name, | ||
| "arguments": {}, | ||
| }); | ||
| } | ||
|
|
||
| // Parse current arguments (partial or complete) | ||
| let current_args = self.parse_parameters_from_dsml(&invoke_content, !is_complete); | ||
| let tool_id = self.current_tool_id as usize; | ||
|
|
||
| // Compute diff against what we've already sent | ||
| let sent_len = self | ||
| .streamed_args_for_tool | ||
| .get(tool_id) | ||
| .map(|s| s.len()) | ||
| .unwrap_or(0); | ||
|
|
||
| let prev_args = if tool_id < self.prev_tool_call_arr.len() { | ||
| self.prev_tool_call_arr[tool_id] | ||
| .get("arguments") | ||
| .and_then(|v| v.as_str()) | ||
| .map(|s| s.to_string()) | ||
| } else { | ||
| None | ||
| }; | ||
|
|
||
| let argument_diff = if is_complete { | ||
| if sent_len < current_args.len() { | ||
| Some(current_args[sent_len..].to_string()) | ||
| } else { | ||
| Some(String::new()) | ||
| } | ||
| } else if let Some(prev) = &prev_args { | ||
| if current_args == *prev { | ||
| None | ||
| } else { | ||
| let prefix = helpers::find_common_prefix(prev, ¤t_args); | ||
| if prefix.len() > sent_len { | ||
| Some(prefix[sent_len..].to_string()) | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
| } else { | ||
| None | ||
| }; |
There was a problem hiding this comment.
Bug: First partial arguments are not emitted during streaming.
When the tool name is first sent (lines 336-339), arguments is set to {} (a JSON object). However, lines 353-360 attempt to retrieve it via .as_str(), which returns None for objects. When is_complete=false and prev_args=None (lines 379-381), argument_diff becomes None, causing the first partial arguments to be silently dropped.
Trace:
- First chunk arrives with partial invoke
- Tool name emitted,
prev_tool_call_arr[0]set to{"arguments": {}} current_argsparsed with actual contentprev_args=None(object{}fails.as_str())argument_diff=None→ first partial arguments not streamed- Only subsequent chunks stream arguments correctly
🛠️ Proposed fix: Emit from sent_len when prev_args is None
} else if let Some(prev) = &prev_args {
if current_args == *prev {
None
} else {
let prefix = helpers::find_common_prefix(prev, ¤t_args);
if prefix.len() > sent_len {
Some(prefix[sent_len..].to_string())
} else {
None
}
}
+ } else if sent_len < current_args.len() {
+ // First partial chunk - emit everything from sent_len
+ Some(current_args[sent_len..].to_string())
} else {
None
};Additionally, consider initializing arguments as an empty string to maintain type consistency:
self.prev_tool_call_arr[tool_id] = serde_json::json!({
"name": func_name,
- "arguments": {},
+ "arguments": "",
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/tool_parser/src/parsers/deepseek32.rs` around lines 336 - 381, The bug
is that prev_tool_call_arr stores "arguments" as an object but the code expects
a string, so prev_args becomes None and the first partial chunk is dropped;
update the logic in the block computing argument_diff (around
parse_parameters_from_dsml, streamed_args_for_tool, prev_tool_call_arr and
helpers::find_common_prefix) to treat a missing/non-string previous arguments as
an empty string (or initialize "arguments" as an empty string when setting
prev_tool_call_arr) and then compute the diff from sent_len (i.e., if prev_args
is None treat prev = "" and emit current_args[sent_len..] when !is_complete or
when appropriate), ensuring the first partial arguments are returned instead of
None.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cdff14236c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| &mut self.streamed_args_for_tool, | ||
| &self.prev_tool_call_arr, | ||
| ); | ||
| return Ok(StreamingParseResult::default()); |
There was a problem hiding this comment.
If a chunk contains a valid complete invoke followed by an invalid incomplete invoke, this branch returns StreamingParseResult::default() and drops all_calls already accumulated earlier in the same loop iteration. In that case, previously parsed valid tool calls are lost permanently (and subsequent indices can drift), even though they were complete and should have been emitted.
Useful? React with 👍 / 👎.
| for fragment in fragments.iter().rev() { | ||
| result = result.trim_end_matches(|c: char| fragment.contains(c)).to_string(); | ||
| } |
There was a problem hiding this comment.
strip_dsml_trailing uses trim_end_matches with a character predicate (fragment.contains(c)), which removes any trailing characters that appear in fragment text (for example, any ending e/r/t/a/m/p from "parameter") even when no DSML suffix is present. During partial argument parsing this can truncate legitimate argument bytes (e.g. "value" -> "valu"), so if generation stops mid-invoke the recovered tool arguments are incorrect.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| if tools.is_empty() { | ||
| return Ok((text.to_string(), vec![])); |
There was a problem hiding this comment.
🟡 Nit: When tools.is_empty() but DSML markers were detected, this returns text.to_string() — which includes raw DSML markup (<｜DSML｜function_calls>, etc.) in the "normal text" output.
This triggers when:
- The model outputs an empty
function_callsblock (no invokes inside) - The model's output is truncated mid-DSML so
tool_call_complete_regexdoesn't match (missing closing tag), yet the opening marker is present - The
function_callsblock contains content that doesn't matchinvoke_complete_regex
In all cases, normal_text (line 214) already holds the clean text before the DSML marker, so returning it preserves correctness.
| return Ok((text.to_string(), vec![])); | |
| return Ok((normal_text, vec![])); |
| self.prev_tool_call_arr[tool_id] = serde_json::json!({ | ||
| "name": func_name, | ||
| "arguments": {}, | ||
| }); |
There was a problem hiding this comment.
🟣 Pre-existing: "arguments": {} initializes the value as a JSON object, but the reader at line 365 uses .as_str() which returns None for non-string JSON values. This means the first partial argument diff after name emission is silently skipped (the else { None } branch at line 390 fires).
No data is lost — the skipped diff is recovered on the next chunk via find_common_prefix, and complete invokes use the is_complete branch which doesn't depend on prev_args. But it does cause a one-chunk streaming delay for partial invokes when tool name and first args arrive in the same chunk.
This is the same pattern used across all existing parsers (deepseek, deepseek31, etc.) so not introduced by this PR. Initializing with "arguments": "" (empty string) would make as_str() return Some("") and eliminate the delay, but that's a broader change across all parsers.
Signed-off-by: key4ng <rukeyang@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/tool_parser/src/parsers/deepseek32.rs (1)
346-349:⚠️ Potential issue | 🟠 MajorEmit the first partial argument delta.
Line 348 seeds
argumentswith a non-string value, but Lines 363-367 only recover previous arguments via.as_str(). On the first incomplete invoke,prev_argsis therefore missing, so Lines 389-390 returnNoneand the stream emits the tool name without any initial parameter bytes.🛠️ Possible fix
- let prev_args = if tool_id < self.prev_tool_call_arr.len() { - self.prev_tool_call_arr[tool_id] - .get("arguments") - .and_then(|v| v.as_str()) - .map(|s| s.to_string()) - } else { - None - }; + let prev_args = if tool_id < self.prev_tool_call_arr.len() { + self.prev_tool_call_arr[tool_id] + .get("arguments") + .and_then(|v| v.as_str()) + .unwrap_or_default() + .to_string() + } else { + String::new() + }; let argument_diff = if is_complete { if sent_len < current_args.len() { Some(current_args[sent_len..].to_string()) } else { Some(String::new()) } - } else if let Some(prev) = &prev_args { - if current_args == *prev { + } else if prev_args.is_empty() { + if sent_len < current_args.len() { + Some(current_args[sent_len..].to_string()) + } else { + None + } + } else if current_args == prev_args { None } else { - let prefix = helpers::find_common_prefix(prev, ¤t_args); + let prefix = helpers::find_common_prefix(&prev_args, ¤t_args); if prefix.len() > sent_len { Some(prefix[sent_len..].to_string()) } else { None } } - } else { - None };Also applies to: 363-390
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tool_parser/src/parsers/deepseek32.rs` around lines 346 - 349, The code seeds prev_tool_call_arr[tool_id] with "arguments": {} (in deepseek32 parser) but later logic expects a string via .as_str(), so on the first partial invoke prev_args is missing and no initial parameter bytes are emitted; fix by initializing the seeded value to an empty string (e.g., set "arguments" to "" instead of {}) or alternatively update the prev_args recovery (where .as_str() is used) to handle non-string JSON by calling .as_str().unwrap_or_else(|| json_value.to_string().as_str()) or using to_string()/unwrap_or_default() so the first partial argument delta is emitted; modify the code around prev_tool_call_arr, tool_id and the prev_args extraction to ensure arguments are a string when consumed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/tool_parser/src/parsers/deepseek32.rs`:
- Around line 244-270: The partial-marker detection in the has_partial_prefix
check is too narrow (only '<', '<｜', '</', '</｜') causing longer truncated
tokens (e.g. '<｜DSML' or '<｜DSML｜fun') to be treated as normal_text; update the
computation of has_partial_prefix in the parsing function (the variables
current_text, has_dsml, has_partial_prefix and the early-return branch that
yields StreamingParseResult::default()) so it detects any trailing incomplete
tag: find the last '<' (or "</") in current_text and treat it as a partial
prefix if there is no matching closing '>' after it (or use a regex like
r"</?[^>]*$" to detect an unterminated tag); this will ensure such longer
partial DSML fragments are buffered instead of flushed as normal_text.
---
Duplicate comments:
In `@crates/tool_parser/src/parsers/deepseek32.rs`:
- Around line 346-349: The code seeds prev_tool_call_arr[tool_id] with
"arguments": {} (in deepseek32 parser) but later logic expects a string via
.as_str(), so on the first partial invoke prev_args is missing and no initial
parameter bytes are emitted; fix by initializing the seeded value to an empty
string (e.g., set "arguments" to "" instead of {}) or alternatively update the
prev_args recovery (where .as_str() is used) to handle non-string JSON by
calling .as_str().unwrap_or_else(|| json_value.to_string().as_str()) or using
to_string()/unwrap_or_default() so the first partial argument delta is emitted;
modify the code around prev_tool_call_arr, tool_id and the prev_args extraction
to ensure arguments are a string when consumed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 09577cc6-67fa-45f1-936b-c32a8acefbd2
📒 Files selected for processing (1)
crates/tool_parser/src/parsers/deepseek32.rs
| // Check for DSML markers or partial DSML prefixes | ||
| let has_dsml = | ||
| self.has_tool_markers(¤t_text) || current_text.contains("<｜DSML｜invoke"); | ||
| let has_partial_prefix = current_text.ends_with('<') | ||
| || current_text.ends_with("<｜") | ||
| || current_text.ends_with("</") | ||
| || current_text.ends_with("</｜"); | ||
|
|
||
| if !has_dsml && !has_partial_prefix { | ||
| let mut normal_text = std::mem::take(&mut self.buffer); | ||
| for end_token in [ | ||
| "</｜DSML｜function_calls>", | ||
| "</｜DSML｜invoke>", | ||
| "</｜DSML｜parameter>", | ||
| "<｜end▁of▁sentence｜>", | ||
| ] { | ||
| normal_text = normal_text.replace(end_token, ""); | ||
| } | ||
| return Ok(StreamingParseResult { | ||
| normal_text, | ||
| calls: vec![], | ||
| }); | ||
| } | ||
|
|
||
| // If we have partial prefix but no actual DSML content, buffer and wait | ||
| if !has_dsml && has_partial_prefix { | ||
| return Ok(StreamingParseResult::default()); |
There was a problem hiding this comment.
Broaden partial-marker buffering beyond four hard-coded suffixes.
Lines 247-250 only preserve <, <｜, </, and </｜. If a chunk ends with a longer partial tag such as <｜DSML or <｜DSML｜fun, Lines 252-266 flush it as normal_text, and the next chunk can no longer reconstruct a valid DSML marker. That drops the upcoming tool call on those chunk boundaries.
🛠️ Possible fix
+ const DSML_MARKERS: &[&str] = &[
+ "<｜DSML｜function_calls>",
+ "</｜DSML｜function_calls>",
+ "<｜DSML｜invoke",
+ "</｜DSML｜invoke>",
+ "<｜DSML｜parameter",
+ "</｜DSML｜parameter>",
+ ];
let has_dsml =
self.has_tool_markers(¤t_text) || current_text.contains("<｜DSML｜invoke");
- let has_partial_prefix = current_text.ends_with('<')
- || current_text.ends_with("<｜")
- || current_text.ends_with("</")
- || current_text.ends_with("</｜");
+ let has_partial_prefix = DSML_MARKERS.iter().any(|marker| {
+ marker
+ .char_indices()
+ .skip(1)
+ .any(|(idx, _)| current_text.ends_with(&marker[..idx]))
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/tool_parser/src/parsers/deepseek32.rs` around lines 244 - 270, The
partial-marker detection in the has_partial_prefix check is too narrow (only
'<', '<｜', '</', '</｜') causing longer truncated tokens (e.g. '<｜DSML' or
'<｜DSML｜fun') to be treated as normal_text; update the computation of
has_partial_prefix in the parsing function (the variables current_text,
has_dsml, has_partial_prefix and the early-return branch that yields
StreamingParseResult::default()) so it detects any trailing incomplete tag: find
the last '<' (or "</") in current_text and treat it as a partial prefix if there
is no matching closing '>' after it (or use a regex like r"</?[^>]*$" to detect
an unterminated tag); this will ensure such longer partial DSML fragments are
buffered instead of flushed as normal_text.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fa9333830
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if !func_name.is_empty() && !tool_indices.contains_key(func_name.as_str()) { | ||
| tracing::debug!("Invalid tool name '{}' - skipping", func_name); |
There was a problem hiding this comment.
Because func_name is trimmed before validation, an invoke like name=" " becomes an empty string and bypasses the invalid-tool guard (if !func_name.is_empty() && ...). In that case the parser can emit argument chunks without ever emitting a tool name, then increments current_tool_id, which shifts indices for subsequent valid calls in the same stream. This produces malformed streaming tool-call events from a single malformed invoke instead of cleanly skipping it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
crates/tool_parser/src/parsers/deepseek32.rs (2)
348-351:⚠️ Potential issue | 🟠 MajorStill dropping the first partial argument delta.
prev_tool_call_arr[tool_id]["arguments"]is initialized as{}, but Lines 365-369 only read strings. On the first incomplete invoke,prev_argsbecomesNone, so the!is_completepath returnsNoneand no parameters are streamed until a later chunk. Initialize"arguments"as""or treat a non-string previous value as empty.🛠️ Minimal fix
self.prev_tool_call_arr[tool_id] = serde_json::json!({ "name": func_name, - "arguments": {}, + "arguments": "", }); ... - } else { - None + } else if sent_len < current_args.len() { + Some(current_args[sent_len..].to_string()) + } else { + None };Also applies to: 365-392
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tool_parser/src/parsers/deepseek32.rs` around lines 348 - 351, The current initialization of self.prev_tool_call_arr[tool_id]["arguments"] as an object causes the first partial argument delta to be dropped because prev_args (read elsewhere in deepseek32.rs around the prev_args / is_complete logic) expects a string; change the initialization in the prev_tool_call_arr entry for func_name to set "arguments" to an empty string "" (or alternatively update the code that reads prev_args to treat non-string values as empty string) so that the first incomplete chunk is appended/streamed correctly; update any logic that merges incoming argument chunks (the code paths around prev_args and is_complete) to handle and coerce non-string previous values to "" before concatenation.
246-272:⚠️ Potential issue | 🟠 MajorLonger truncated DSML prefixes are still flushed as normal text.
Lines 249-252 only preserve
<,<｜,</, and</｜. A chunk ending with<｜DSMLor<｜DSML｜invstill reaches Line 254 and gets emitted asnormal_text, so the next chunk can never reconstruct the tag. Buffer any trailing unterminated<...fragment instead of hard-coding four suffixes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tool_parser/src/parsers/deepseek32.rs` around lines 246 - 272, The code currently only treats four very short suffixes as partial DSML prefixes (has_partial_prefix) so longer truncated tags like "<｜DSML" get flushed as normal_text; change has_partial_prefix to detect any trailing unterminated '<' fragment by finding the last '<' in current_text and checking if there is no corresponding '>' after it (i.e., an open tag that runs to the end of the chunk), and if so treat that suffix as a partial prefix; when producing normal_text (in the branch that strips end tokens and returns StreamingParseResult), remove that trailing unterminated fragment from normal_text and put it back into self.buffer so the fragment is preserved for the next chunk; update uses of has_partial_prefix, current_text, buffer, and StreamingParseResult accordingly instead of hard-coding the four suffixes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/tool_parser/src/parsers/deepseek32.rs`:
- Around line 51-67: strip_dsml_trailing currently trims by character set and
can remove legitimate argument characters; change it to only remove an actual
trailing substring that is a prefix of the full DSML closing tag. For each
fragment group (DSML_PARAM_END_FRAGMENTS and DSML_INVOKE_END_FRAGMENTS) build
the full closing string by concatenating the fragments (e.g.
"</｜DSML｜parameter>"), then for the input string find the longest k>0 such that
result.ends_with(&full[..k]) and remove exactly that suffix (no per-character
trimming). Update strip_dsml_trailing to perform this suffix-prefix check and
removal using the concatenated full tag rather than fragment.contains(c); keep
references to DSML_PARAM_END_FRAGMENTS and DSML_INVOKE_END_FRAGMENTS and the
function name strip_dsml_trailing for locating the change.
---
Duplicate comments:
In `@crates/tool_parser/src/parsers/deepseek32.rs`:
- Around line 348-351: The current initialization of
self.prev_tool_call_arr[tool_id]["arguments"] as an object causes the first
partial argument delta to be dropped because prev_args (read elsewhere in
deepseek32.rs around the prev_args / is_complete logic) expects a string; change
the initialization in the prev_tool_call_arr entry for func_name to set
"arguments" to an empty string "" (or alternatively update the code that reads
prev_args to treat non-string values as empty string) so that the first
incomplete chunk is appended/streamed correctly; update any logic that merges
incoming argument chunks (the code paths around prev_args and is_complete) to
handle and coerce non-string previous values to "" before concatenation.
- Around line 246-272: The code currently only treats four very short suffixes
as partial DSML prefixes (has_partial_prefix) so longer truncated tags like
"<｜DSML" get flushed as normal_text; change has_partial_prefix to detect any
trailing unterminated '<' fragment by finding the last '<' in current_text and
checking if there is no corresponding '>' after it (i.e., an open tag that runs
to the end of the chunk), and if so treat that suffix as a partial prefix; when
producing normal_text (in the branch that strips end tokens and returns
StreamingParseResult), remove that trailing unterminated fragment from
normal_text and put it back into self.buffer so the fragment is preserved for
the next chunk; update uses of has_partial_prefix, current_text, buffer, and
StreamingParseResult accordingly instead of hard-coding the four suffixes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 08b90a6f-e740-4a73-b664-9be71109e9f4
📒 Files selected for processing (1)
crates/tool_parser/src/parsers/deepseek32.rs
| /// DSML token fragments for stripping partial closing tags during streaming. | ||
| /// Applied in reverse order using character-level right-trimming, following | ||
| /// SGLang's exact fragment definitions. | ||
| const DSML_PARAM_END_FRAGMENTS: &[&str] = &["</", "｜DSML｜", "parameter"]; | ||
| const DSML_INVOKE_END_FRAGMENTS: &[&str] = &["</", "｜DSML｜", "inv", "oke"]; | ||
|
|
||
| /// Strip trailing DSML fragment characters from a string. | ||
| /// Iterates fragments in reverse, stripping any trailing characters | ||
| /// that appear in each fragment (mimics Python's `str.rstrip`). | ||
| fn strip_dsml_trailing(s: &str, fragments: &[&str]) -> String { | ||
| let mut result = s.to_string(); | ||
| for fragment in fragments.iter().rev() { | ||
| result = result | ||
| .trim_end_matches(|c: char| fragment.contains(c)) | ||
| .to_string(); | ||
| } | ||
| result |
There was a problem hiding this comment.
strip_dsml_trailing removes real argument characters.
Line 64 trims by character set, not by suffix. That means a partial value ending in bar is reduced to b, and a direct-JSON body ending with a legitimate e/o/k/r can lose that byte even when no DSML fragment is present. Because this truncated snapshot feeds later diffing, partial tool arguments can be streamed incorrectly. Strip only the longest trailing suffix that is actually a prefix of </｜DSML｜parameter> or </｜DSML｜invoke>.
🛠️ One safe shape
-const DSML_PARAM_END_FRAGMENTS: &[&str] = &["</", "｜DSML｜", "parameter"];
-const DSML_INVOKE_END_FRAGMENTS: &[&str] = &["</", "｜DSML｜", "inv", "oke"];
+const DSML_PARAMETER_END_TAG: &str = "</｜DSML｜parameter>";
+const DSML_INVOKE_END_TAG: &str = "</｜DSML｜invoke>";
-fn strip_dsml_trailing(s: &str, fragments: &[&str]) -> String {
- let mut result = s.to_string();
- for fragment in fragments.iter().rev() {
- result = result
- .trim_end_matches(|c: char| fragment.contains(c))
- .to_string();
- }
- result
+fn strip_dsml_trailing(s: &str, closing_tag: &str) -> String {
+ for (idx, _) in s.char_indices() {
+ if closing_tag.starts_with(&s[idx..]) {
+ return s[..idx].to_string();
+ }
+ }
+ s.to_string()
}
...
- return strip_dsml_trailing(trimmed, DSML_INVOKE_END_FRAGMENTS);
+ return strip_dsml_trailing(trimmed, DSML_INVOKE_END_TAG);
...
- let cleaned = strip_dsml_trailing(remaining, DSML_PARAM_END_FRAGMENTS);
+ let cleaned = strip_dsml_trailing(remaining, DSML_PARAMETER_END_TAG);Also applies to: 126-127, 163-164
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/tool_parser/src/parsers/deepseek32.rs` around lines 51 - 67,
strip_dsml_trailing currently trims by character set and can remove legitimate
argument characters; change it to only remove an actual trailing substring that
is a prefix of the full DSML closing tag. For each fragment group
(DSML_PARAM_END_FRAGMENTS and DSML_INVOKE_END_FRAGMENTS) build the full closing
string by concatenating the fragments (e.g. "</｜DSML｜parameter>"), then for the
input string find the longest k>0 such that result.ends_with(&full[..k]) and
remove exactly that suffix (no per-character trimming). Update
strip_dsml_trailing to perform this suffix-prefix check and removal using the
concatenated full tag rather than fragment.contains(c); keep references to
DSML_PARAM_END_FRAGMENTS and DSML_INVOKE_END_FRAGMENTS and the function name
strip_dsml_trailing for locating the change.
1 participant
Description
Problem
DeepSeek V3.2 introduces a new XML-like DSML format for tool calls, replacing the special-token approach used in V3/V3.1. The gateway has no parser for this format, so V3.2 models cannot use tool calling through the gRPC streaming path.
Solution
Add a new
DeepSeek32Parserthat handles the DSML format with incremental streaming support, following the SGLangDeepSeekV32Detectorpattern.Changes
crates/tool_parser/src/parsers/deepseek32.rs— handles DSML format with regex-based parsing<｜DSML｜parameter>) and direct JSON fallback inside invoke blocksstring="true"→ string,string="false"→ parsed JSONdeepseek32parser with model mappingsdeepseek-v3.2*/deepseek-ai/DeepSeek-V3.2*→deepseek32(DSML format)deepseek-v3.2-exp*/deepseek-ai/DeepSeek-V3.2-Exp*→deepseek31(V3.2-Exp uses V3.1 format)Test Plan
Checklist
cargo +nightly fmtpassescargo clippy --all-targets --all-features -- -D warningspassesSummary by CodeRabbit
New Features
Tests