feat: Qwen3 coder tool parser #4415
Conversation
2ez4bz
force-pushed
the
dev-qwen3-coder-parser
branch
from
0e0ab3d to
a4923fe
Compare
| let function_regex = FUNCTION_REGEX.get_or_init(|| { | ||
| // Match <function=name>content</function> or partial <function=name>content | ||
| // (?s) makes . match newlines | ||
| Regex::new(r"(?s)<function=([^>]+)>(.*?)(?:</function>|$)").unwrap() |
There was a problem hiding this comment.
Per https://github.com/sgl-project/sglang/blob/e389f91decdad61653edc57c765ef6041506e4a2/python/sglang/srt/function_call/qwen3_coder_detector.py#L52, this matches both properly terminated function blocks like:
<function=foo>...</function>as well as blocks that are missing a trailing </function>:
<function=foo>...Ditto for the parameter_regex below.
| } | ||
|
|
||
| /// Simple HTML unescape for common entities. | ||
| fn html_unescape(s: &str) -> String { |
There was a problem hiding this comment.
Tries to mimic https://github.com/sgl-project/sglang/blob/main/python/sglang/srt/function_call/qwen3_coder_detector.py#L21
Open to better ideas 🙏
2ez4bz
force-pushed
the
dev-qwen3-coder-parser
branch
from
a4923fe to
6127f58
Compare
|
/ok to test |
2ez4bz
force-pushed
the
dev-qwen3-coder-parser
branch
from
6127f58 to
7e53904
Compare
WalkthroughAdds a new Qwen3Coder tool-calling parser (detection, end-position, and parsing) integrated into the tool-calling framework and a dev-test dependency update ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
2ez4bz
force-pushed
the
dev-qwen3-coder-parser
branch
from
7e53904 to
7d5927e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
lib/parsers/src/tool_calling/qwen3_coder/parser.rs (2)
171-198: Consider broadening whitespace trimming for consistency.The function has good defensive parsing with multiple fallbacks. One minor observation: line 197 uses
.trim_matches('\n')to strip only newlines from strings, while other parsers in the codebase might use.trim()for all whitespace. This might be intentional for preserving spaces, but worth considering if it aligns with expected behavior.// Default to string, stripping newlines from start and end. - serde_json::Value::String(unescaped.trim_matches('\n').to_string()) + serde_json::Value::String(unescaped.trim().to_string())
201-208: Consider a dedicated HTML entity library for completeness.The simple replacement approach works for common entities and matches the SGLang reference (per your past comments). However, if the Qwen3 model outputs other entities like
,', numeric entities ({), or Unicode (🚀), they won't be decoded. Consider using a library likehtml-escapeorquick-xml's unescape utilities if broader entity support becomes needed.For now, this is fine given it matches the reference implementation and handles the expected cases.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
lib/parsers/Cargo.toml(1 hunks)lib/parsers/src/tool_calling/config.rs(2 hunks)lib/parsers/src/tool_calling/mod.rs(2 hunks)lib/parsers/src/tool_calling/parsers.rs(9 hunks)lib/parsers/src/tool_calling/qwen3_coder/mod.rs(1 hunks)lib/parsers/src/tool_calling/qwen3_coder/parser.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/parsers.rslib/parsers/src/tool_calling/qwen3_coder/parser.rslib/parsers/src/tool_calling/qwen3_coder/mod.rs
🧬 Code graph analysis (3)
lib/parsers/src/tool_calling/mod.rs (2)
lib/parsers/src/tool_calling/config.rs (1)
qwen3_coder(182-188)lib/parsers/src/tool_calling/qwen3_coder/parser.rs (1)
try_tool_call_parse_qwen3_coder(51-63)
lib/parsers/src/tool_calling/parsers.rs (2)
lib/parsers/src/tool_calling/config.rs (1)
qwen3_coder(182-188)lib/parsers/src/tool_calling/qwen3_coder/parser.rs (3)
detect_tool_call_start_qwen3_coder(17-34)find_tool_call_end_position_qwen3_coder(38-46)try_tool_call_parse_qwen3_coder(51-63)
lib/parsers/src/tool_calling/qwen3_coder/mod.rs (1)
lib/parsers/src/tool_calling/qwen3_coder/parser.rs (3)
detect_tool_call_start_qwen3_coder(17-34)find_tool_call_end_position_qwen3_coder(38-46)try_tool_call_parse_qwen3_coder(51-63)
⏰ 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). (14)
- GitHub Check: operator (amd64)
- GitHub Check: operator (arm64)
- GitHub Check: sglang (amd64)
- GitHub Check: sglang (arm64)
- GitHub Check: vllm (arm64)
- GitHub Check: trtllm (arm64)
- GitHub Check: trtllm (amd64)
- GitHub Check: vllm (amd64)
- GitHub Check: Build and Test - dynamo
- GitHub Check: tests (lib/bindings/python)
- GitHub Check: tests (launch/dynamo-run)
- GitHub Check: clippy (.)
- GitHub Check: tests (lib/runtime/examples)
- GitHub Check: tests (.)
🔇 Additional comments (14)
lib/parsers/Cargo.toml (1)
42-43: LGTM! Appropriate test dependency addition.The rstest dev-dependency enables clean parameterized testing (as seen in the qwen3_coder parser tests). Since it's test-only, there's no runtime impact.
lib/parsers/src/tool_calling/config.rs (2)
18-19: LGTM! Clear enum variant addition.The Qwen3Coder variant is well-documented with the format example, making it easy for users to understand the expected structure.
182-188: LGTM! Consistent config constructor pattern.The unused JsonParserConfig follows the same pattern as the
pythonic()constructor, maintaining consistency across parser configs.lib/parsers/src/tool_calling/mod.rs (1)
9-9: LGTM! Standard module integration.The module declaration and public re-export follow the established pattern used by other parsers in the codebase.
Also applies to: 24-24
lib/parsers/src/tool_calling/qwen3_coder/mod.rs (1)
1-10: LGTM! Clean module structure.The module correctly exposes the three main parser functions (detect, find_end, parse) following the same pattern as other parser modules.
lib/parsers/src/tool_calling/parsers.rs (4)
16-19: LGTM! Complete parser integration.The Qwen3Coder parser is correctly integrated at all dispatch points:
- Imports added
- Registered in parser map
- Handlers added to try_tool_call_parse, detect_tool_call_start, and find_tool_call_end_position
The integration follows the exact pattern used by other parsers.
Also applies to: 38-38, 67-70, 121-121, 157-157
1676-1711: LGTM! Good parallel tool call coverage.The test verifies that multiple Qwen3Coder tool calls are correctly parsed and validated using the existing test helper infrastructure.
2437-2449: LGTM! Detection tests cover key scenarios.The tests verify both complete token detection and partial token handling for streaming use cases.
2452-2767: LGTM! Comprehensive test suite.The Qwen3Coder tests cover:
- Simple and multiple parameters
- Normal text handling
- Parallel tool calls
- JSON/numeric parameter values
- HTML entity unescaping
- Missing closing tags
- Edge cases (no tool calls, compact format, mixed types, arrays)
This provides strong confidence in the parser implementation.
lib/parsers/src/tool_calling/qwen3_coder/parser.rs (5)
17-34: LGTM! Robust streaming detection.The partial token matching logic correctly handles streaming scenarios where the start token arrives in chunks.
38-46: LGTM! Simple and correct end position logic.Returns the position after the end token or the chunk length if not found, which is appropriate for streaming contexts.
66-107: LGTM! Careful extraction with graceful handling.The function correctly:
- Separates normal text from tool call blocks
- Handles multiple tool calls
- Gracefully handles missing end tokens by treating remaining text as normal content
111-167: LGTM! Well-structured parsing with efficient regex compilation.The use of
OnceLockfor regex compilation is good for performance. The regex patterns intentionally handle missing closing tags (per your past review comments and SGLang reference), which provides robustness during streaming.
210-465: LGTM! Excellent test coverage with rstest parameterization.The tests comprehensively cover:
- Detection and end position logic
- Value parsing with multiple types
- HTML entity unescaping
- Simple and complex tool calls
- Missing closing tags (intentional tolerance)
- Error handling
The use of rstest for parameterized tests (lines 237-261) makes the test cases clean and maintainable.
rmccorm4
added
the
frontend
ayushag-nv
left a comment
ayushag-nv
left a comment
There was a problem hiding this comment.
The folder structure is not right. We are not putting model specific parser at top level folder. try to put them into one of the categories and then can have model specific file if required.
| Typescript, | ||
| Xml, | ||
| /// Qwen3Coder format: `<tool_call><function=name><parameter=key>value</parameter></function></tool_call>` | ||
| Qwen3Coder, |
There was a problem hiding this comment.
There should not be new type added if its under Xml
There was a problem hiding this comment.
Removed in favor of Xml.
| /// Format: <tool_call><function=name>... | ||
| pub fn detect_tool_call_start_qwen3_coder(chunk: &str) -> bool { | ||
| // Check for complete or partial start token. | ||
| let start_token = "<tool_call>"; |
There was a problem hiding this comment.
Can we think more on how to generalize structure for xml based parsers. Don't like harded start, end tokens in the code.
There was a problem hiding this comment.
As discussed on slack: will do this in a follow-up. Left some TODO comments alluding to this.
| /// Find the end position of a Qwen3Coder tool call. | ||
| /// Returns the position after </tool_call> or the length of the chunk if not found. | ||
| pub fn find_tool_call_end_position_qwen3_coder(chunk: &str) -> usize { | ||
| let end_token = "</tool_call>"; |
There was a problem hiding this comment.
As discussed on slack: will do this in a follow-up. Left some TODO comments alluding to this.
Note: the current implementation is hardcoded for Qwen3 coder.
2ez4bz
force-pushed
the
dev-qwen3-coder-parser
branch
from
7d5927e to
9c39848
Compare
Overview:
This PR adds a tool call parser for Qwen3 coder using SGLang's implementation as a reference.
Details:
This PR tries to follow the pattern for existing parser implementations to add a new one for Qwen3 coder.
Where should the reviewer start?
Probably the unit tests serve as a good starting point.
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
New Features
Tests
Chores