-
Notifications
You must be signed in to change notification settings - Fork 753
feat: DeepSeek V3.2 tool calling support #4822
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
feat: DeepSeek V3.2 tool calling support #4822
Conversation
Signed-off-by: Vladislav Nosivskoy <[email protected]>
Signed-off-by: Vladislav Nosivskoy <[email protected]>
Signed-off-by: Vladislav Nosivskoy <[email protected]>
|
👋 Hi vladnosiv! Thank you for contributing to ai-dynamo/dynamo. Just a reminder: The 🚀 |
WalkthroughSupport for DSML (DeepSeek-specific Markup Language) tool call parsing is introduced. A new DSML parser module is added with detection, positioning, and parsing functions. Configuration structures and integration into the tool calling system enable DeepSeek V3.2 format parsing. Test data and ignore rules are updated accordingly. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/parsers/src/tool_calling/dsml/parser.rs (1)
102-107: Consider caching compiled regex patterns.Regex compilation occurs on every function call (
extract_tool_calls,extract_invokes,parse_parameters). For high-throughput parsing scenarios, this could become a performance bottleneck.Since the config tokens are typically constant at runtime, consider caching the compiled regex patterns. One approach is to store pre-compiled
Regexinstances in theDsmlParserConfigstruct (possibly usingonce_cell::sync::Lazyor computing them lazily on first use).Also applies to: 131-136, 177-182
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/copyright-check.ps1(1 hunks).pre-commit-config.yaml(1 hunks)lib/llm/tests/data/deepseek-v3.2/test_output.txt(1 hunks)lib/parsers/src/tool_calling/config.rs(5 hunks)lib/parsers/src/tool_calling/dsml/mod.rs(1 hunks)lib/parsers/src/tool_calling/dsml/parser.rs(1 hunks)lib/parsers/src/tool_calling/mod.rs(2 hunks)lib/parsers/src/tool_calling/parsers.rs(7 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-10T22:32:12.978Z
Learnt from: zhongdaor-nv
Repo: ai-dynamo/dynamo PR: 2999
File: lib/parsers/src/tool_calling/harmony/harmony_parser.rs:250-256
Timestamp: 2025-09-10T22:32:12.978Z
Learning: In lib/parsers/src/tool_calling/harmony/harmony_parser.rs, the team prefers to maintain identical code patterns between parse_tool_calls_harmony and parse_tool_calls_harmony_complete functions, including message.content[0] indexing, to ensure consistency between streaming and complete parser implementations.
Applied to files:
lib/parsers/src/tool_calling/mod.rslib/parsers/src/tool_calling/dsml/mod.rslib/parsers/src/tool_calling/parsers.rslib/parsers/src/tool_calling/dsml/parser.rs
📚 Learning: 2025-09-08T21:18:43.478Z
Learnt from: nachiketb-nvidia
Repo: ai-dynamo/dynamo PR: 2936
File: lib/parsers/src/reasoning/granite_parser.rs:42-46
Timestamp: 2025-09-08T21:18:43.478Z
Learning: In GraniteReasoningParser in lib/parsers/src/reasoning/granite_parser.rs, the think_start_tokens and think_end_tokens are hardcoded in the constructor with fixed values, so unwrap() calls on these vectors are safe and won't panic.
Applied to files:
lib/parsers/src/tool_calling/config.rs
🧬 Code graph analysis (2)
lib/parsers/src/tool_calling/mod.rs (1)
lib/parsers/src/tool_calling/dsml/parser.rs (1)
try_tool_call_parse_dsml(55-92)
lib/parsers/src/tool_calling/dsml/parser.rs (2)
lib/parsers/src/tool_calling/parsers.rs (3)
extract_name_and_args(183-186)extract_name_and_args(1680-1683)extract_name_and_args(2597-2600)lib/parsers/src/tool_calling/config.rs (4)
default(31-40)default(60-69)default(90-99)default(150-154)
🪛 LanguageTool
lib/llm/tests/data/deepseek-v3.2/test_output.txt
[grammar] ~17-~17: Use a hyphen to join words.
Context: ...ibute should be set to "true" for string type parameters and "false" for other ty...
(QB_NEW_EN_HYPHEN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Build and Test - dynamo
- GitHub Check: clippy (.)
- GitHub Check: tests (launch/dynamo-run)
- GitHub Check: clippy (launch/dynamo-run)
- GitHub Check: tests (.)
- GitHub Check: clippy (lib/bindings/python)
- GitHub Check: tests (lib/runtime/examples)
- GitHub Check: tests (lib/bindings/python)
🔇 Additional comments (20)
.github/workflows/copyright-check.ps1 (1)
87-87: LGTM!The addition of the DeepSeek V3.2 test data path to the ignored paths list is appropriate and consistent with the existing
sample-modelsexclusion pattern..pre-commit-config.yaml (1)
66-67: LGTM!The exclusion pattern for trailing-whitespace on test data files is appropriately scoped to
.txtfiles in the DeepSeek V3.2 test directory.lib/parsers/src/tool_calling/config.rs (3)
72-100: LGTM!The
DsmlParserConfigstruct and itsDefaultimplementation are well-designed. The configuration fields map directly to the DSML format tokens, and the default values align with the DeepSeek V3.2 specification referenced in the parser implementation.
111-111: LGTM!The
Dsmlvariant integration intoParserConfigand the accessor method implementations correctly map the DSML function_calls tokens to the parser's generic interface.Also applies to: 124-124, 137-137
285-296: LGTM!The
deepseek_v3_2()constructor follows the established pattern of other model-specific constructors and includes helpful documentation of the DSML format.lib/parsers/src/tool_calling/dsml/parser.rs (5)
22-40: LGTM!The detection logic correctly handles both complete tokens and partial matches for streaming scenarios. The partial match loop is a thoughtful addition for handling incremental input.
42-51: LGTM!The end position calculation correctly accounts for the token length and provides a sensible fallback for incomplete input.
53-92: LGTM!The parsing entry point has clean control flow with appropriate early exits for edge cases. The separation of normal text extraction from tool call parsing is well-structured.
164-209: LGTM!The parameter parsing logic correctly distinguishes between string and non-string types using the
stringattribute. The fallback to string on JSON parse failure (lines 198-201) provides safe degradation for malformed input.
211-489: LGTM!Comprehensive test coverage including detection, positioning, single/multiple tool calls, mixed parameter types (strings, numbers, booleans, arrays, objects, null), and edge cases like empty strings. The tests validate the parser against realistic DSML payloads.
lib/parsers/src/tool_calling/mod.rs (1)
5-5: LGTM!The
dsmlmodule declaration and the re-export oftry_tool_call_parse_dsmlfollow the established patterns for other parser modules (harmony, json, xml, etc.), providing a clean public API.Also applies to: 18-18
lib/parsers/src/tool_calling/dsml/mod.rs (1)
1-9: LGTM!The module structure is clean and follows the established pattern of other parser modules (harmony, json, pythonic, xml). The public API exports align with the expected DSML parser interface.
lib/llm/tests/data/deepseek-v3.2/test_output.txt (1)
1-112: LGTM!Comprehensive test data that demonstrates the DSML format well, including:
- Single and multiple tool invocations
- The
string="true|false"attribute for type handling- Multi-turn conversation flow with function results
- Mixed language content (Chinese) for realistic testing
lib/parsers/src/tool_calling/parsers.rs (7)
5-7: LGTM!DSML imports follow the established pattern of other parser modules.
41-41: LGTM!Parser map entry follows the existing pattern and naming convention.
79-82: LGTM!The DSML parsing branch follows the exact pattern of other parser branches in the match statement.
131-131: LGTM!Detection branch follows the established pattern.
168-168: LGTM!End position detection follows the simpler pattern (like XML), appropriate for DSML's distinct end tokens.
204-204: LGTM!Test list correctly includes the new parser.
1583-1657: LGTM!Comprehensive tests covering:
- Single tool call parsing
- Multiple parallel tool calls
- The critical
string="true|false"attribute handling (line 1655 correctly validatestopnis parsed as number10, not string"10")The tests follow the established pattern and validate the key differentiating feature of the V3.2 format.
|
/ok to test 1040ada |
ayushag-nv
left a comment
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.
Looks good to me. Thanks @vladnosiv for the clean implementation. Just address "adding comments" for few function and the test_ouput.txt file related comments.
Signed-off-by: Vladislav Nosivskoy <[email protected]>
Signed-off-by: Vladislav Nosivskoy <[email protected]>
|
/ok to test abc486e |
Merged 238 commits from main branch to bring the feature branch up to date. Key conflicts resolved: - Removed lib/kvbm-kernels references (deleted in main) - Kept nova/nova-backend/kvbm workspace members from feature branch - Maintained v2 module API refactoring from feature branch - Updated Cargo.lock files to reflect new dependencies Major updates from main include: - LoRA support for vLLM (#4810) - Multimodal documentation (#4510) - Scaling adapter features (#4699, #4825) - Tool calling support (#4822, #4722) - NIXL connect improvements (#4433) Signed-off-by: Ryan Olson <[email protected]>
Signed-off-by: Vladislav Nosivskoy <[email protected]> Co-authored-by: Ayush Agarwal <[email protected]>
Signed-off-by: Vladislav Nosivskoy <[email protected]> Co-authored-by: Ayush Agarwal <[email protected]>
Overview:
Depends on #4797
Add DeepSeek V3.2 tool call parser support
Details:
string="true|false"attribute to distinguish string vs JSON types (numbers, booleans, arrays, objects, null)Where should the reviewer start?
lib/parsers/src/tool_calling/dsml/parser.rsmain implementationRelated Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Relates to #4796
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.