Skip to content

fix(tool_parser): fix func call parsing for native tool-call token #1062

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.

Sign up for GitHub

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

Closed
ConnorLi96 wants to merge 2 commits into from
Closed

fix(tool_parser): fix func call parsing for native tool-call token #1062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aad18f3
fix(tool_parser): fix func call parsing for native tool-call token mo…
ConnorLi96 Apr 7, 2026
468f1df
fix(tool_parser): address review feedback on func call parsing
ConnorLi96 Apr 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 33 additions & 27 deletions crates/reasoning_parser/src/factory.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,35 +208,45 @@ impl ParserFactory {
let config = ParserConfig {
think_start_token: "<think>".to_string(),
think_end_token: "</think>".to_string(),
stream_reasoning: true,
max_buffer_size: DEFAULT_MAX_BUFFER_SIZE,
always_in_reasoning: false,
..Default::default()
};
Box::new(BaseReasoningParser::new(config).with_model_type("deepseek_v31".to_string()))
});

let kimi_tool_markers = vec!["<|tool_calls_section_begin|>".to_string()];

// Register Kimi-K2.5 parser (standard think tokens, always_in_reasoning=false)
registry.register_parser("kimi_k25", || {
let config = ParserConfig {
think_start_token: "<think>".to_string(),
think_end_token: "</think>".to_string(),
stream_reasoning: true,
max_buffer_size: DEFAULT_MAX_BUFFER_SIZE,
always_in_reasoning: false,
};
Box::new(BaseReasoningParser::new(config).with_model_type("kimi_k25".to_string()))
registry.register_parser("kimi_k25", {
let markers = kimi_tool_markers.clone();
move || {
let config = ParserConfig {
think_start_token: "<think>".to_string(),
think_end_token: "</think>".to_string(),
stream_reasoning: true,
max_buffer_size: DEFAULT_MAX_BUFFER_SIZE,
always_in_reasoning: false,
tool_section_start_markers: markers.clone(),
};
Box::new(BaseReasoningParser::new(config).with_model_type("kimi_k25".to_string()))
}
});
Comment on lines +219 to 232

@coderabbitai coderabbitai Bot Apr 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider using ..Default::default() for consistency.

The explicit fields stream_reasoning, max_buffer_size, and always_in_reasoning: false all match the default values. For consistency with the deepseek_v31 and passthrough parser registrations, consider simplifying:

♻️ Suggested simplification 
         registry.register_parser("kimi_k25", {
             let markers = kimi_tool_markers.clone();
             move || {
                 let config = ParserConfig {
                     think_start_token: "<think>".to_string(),
                     think_end_token: "</think>".to_string(),
-                    stream_reasoning: true,
-                    max_buffer_size: DEFAULT_MAX_BUFFER_SIZE,
-                    always_in_reasoning: false,
                     tool_section_start_markers: markers.clone(),
+                    ..Default::default()
                 };
                 Box::new(BaseReasoningParser::new(config).with_model_type("kimi_k25".to_string()))
             }
         });
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@crates/reasoning_parser/src/factory.rs` around lines 219 - 232, The
ParserConfig for the "kimi_k25" registration redundantly sets fields that match
defaults; update the closure passed to registry.register_parser("kimi_k25") to
construct ParserConfig using struct update syntax with ..Default::default() for
the defaulted fields so only the non-default tokens and
tool_section_start_markers are specified (i.e., build ParserConfig {
think_start_token: ..., think_end_token: ..., tool_section_start_markers:
markers.clone(), ..Default::default() }) and keep the
Box::new(BaseReasoningParser::new(config).with_model_type("kimi_k25".to_string()))
part unchanged.


// Register Kimi-K2-Thinking parser (standard think tokens, always_in_reasoning=true)
registry.register_parser("kimi_thinking", || {
let config = ParserConfig {
think_start_token: "<think>".to_string(),
think_end_token: "</think>".to_string(),
stream_reasoning: true,
max_buffer_size: DEFAULT_MAX_BUFFER_SIZE,
always_in_reasoning: true,
};
Box::new(BaseReasoningParser::new(config).with_model_type("kimi_thinking".to_string()))
registry.register_parser("kimi_thinking", {
let markers = kimi_tool_markers;
move || {
let config = ParserConfig {
think_start_token: "<think>".to_string(),
think_end_token: "</think>".to_string(),
stream_reasoning: true,
max_buffer_size: DEFAULT_MAX_BUFFER_SIZE,
always_in_reasoning: true,
tool_section_start_markers: markers.clone(),
};
Box::new(
BaseReasoningParser::new(config).with_model_type("kimi_thinking".to_string()),
)
}
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Register model patterns
Expand Down Expand Up @@ -293,9 +303,7 @@ impl ParserFactory {
let config = ParserConfig {
think_start_token: String::new(),
think_end_token: String::new(),
stream_reasoning: true,
max_buffer_size: DEFAULT_MAX_BUFFER_SIZE,
always_in_reasoning: false,
..Default::default()
};
Box::new(
BaseReasoningParser::new(config).with_model_type("passthrough".to_string()),
Expand All @@ -320,9 +328,7 @@ impl ParserFactory {
let config = ParserConfig {
think_start_token: String::new(),
think_end_token: String::new(),
stream_reasoning: true,
max_buffer_size: DEFAULT_MAX_BUFFER_SIZE,
always_in_reasoning: false,
..Default::default()
};
Box::new(BaseReasoningParser::new(config).with_model_type("passthrough".to_string()))
}
Expand Down
75 changes: 73 additions & 2 deletions crates/reasoning_parser/src/parsers/base.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,15 @@ impl BaseReasoningParser {
|| (self.config.think_end_token.starts_with(text)
&& self.config.think_end_token != text)
}

/// Find the earliest tool-section start marker in `text`.
fn find_tool_section_start(&self, text: &str) -> Option<usize> {
self.config
.tool_section_start_markers
.iter()
.filter_map(|marker| text.find(marker.as_str()))
.min()
}
}

impl ReasoningParser for BaseReasoningParser {
Expand All @@ -63,7 +72,11 @@ impl ReasoningParser for BaseReasoningParser {
.to_string();

if !processed_text.contains(&self.config.think_end_token) {
// Assume reasoning was truncated before end token
if let Some(tool_pos) = self.find_tool_section_start(&processed_text) {
let reasoning_text = processed_text[..tool_pos].trim().to_string();

@claude claude Bot Apr 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Nit: The hardcoded "<|tool_calls_section_begin|>" marker ties the reasoning parser to Kimi K2's specific token format. If another model family uses a different tool-call marker while also skipping </think>, this won't catch it. Consider extracting this as a configurable field on ParserConfig (or at least a constant) so it's easier to extend and easier to spot the coupling.

let normal_text = processed_text[tool_pos..].to_string();
return Ok(ParserResult::new(normal_text, reasoning_text));
}
return Ok(ParserResult::reasoning(processed_text));
}

Expand Down Expand Up @@ -133,7 +146,13 @@ impl ReasoningParser for BaseReasoningParser {

// Continue with reasoning content
if self.in_reasoning && self.config.stream_reasoning {
// Stream the content immediately
if let Some(tool_pos) = self.find_tool_section_start(&current_text) {
let reasoning_text = current_text[..tool_pos].trim().to_string();
let normal_text = current_text[tool_pos..].to_string();
self.buffer.clear();
Comment on lines +149 to +152

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Buffer partial tool-section markers across streaming chunks

The new reasoning bailout only checks for a full tool-section marker in current_text, but when no full marker is found this branch still streams current_text as reasoning and clears the buffer. If <|tool_calls_section_begin|> is split across chunk boundaries (a common streaming pattern), the first chunk drops the marker prefix, so later chunks never match the marker and the parser keeps treating tool-call tokens as reasoning. In that case, streaming tool calls are silently lost even though this code path is meant to recover when </think> is missing.

Useful? React with 👍 / 👎.

self.in_reasoning = false;
return Ok(ParserResult::new(normal_text, reasoning_text));
}
let reasoning_text = current_text;
self.buffer.clear();
Ok(ParserResult::reasoning(reasoning_text))
Expand Down Expand Up @@ -188,6 +207,7 @@ mod tests {
stream_reasoning,
max_buffer_size: DEFAULT_MAX_BUFFER_SIZE,
always_in_reasoning,
..Default::default()
};
BaseReasoningParser::new(config)
}
Expand Down Expand Up @@ -368,4 +388,55 @@ mod tests {
_ => panic!("Expected BufferOverflow error"),
}
}

fn create_parser_with_markers() -> BaseReasoningParser {
let config = ParserConfig {
think_start_token: "<think>".to_string(),
think_end_token: "</think>".to_string(),
tool_section_start_markers: vec!["<|tool_calls_section_begin|>".to_string()],
..Default::default()
};
BaseReasoningParser::new(config)
}

#[test]
fn test_tool_marker_stops_reasoning_non_streaming() {
let mut parser = create_parser_with_markers();
let input = "<think>thinking here<|tool_calls_section_begin|>tool call data";
let result = parser.detect_and_parse_reasoning(input).unwrap();
assert_eq!(result.reasoning_text, "thinking here");
assert_eq!(
result.normal_text,
"<|tool_calls_section_begin|>tool call data"
);
}

#[test]
fn test_tool_marker_stops_reasoning_streaming() {
let mut parser = create_parser_with_markers();
let r1 = parser
.parse_reasoning_streaming_incremental("<think>reasoning ")
.unwrap();
assert_eq!(r1.reasoning_text, "reasoning ");
assert!(parser.is_in_reasoning());

let r2 = parser
.parse_reasoning_streaming_incremental("more<|tool_calls_section_begin|>tool data")
.unwrap();
assert_eq!(r2.reasoning_text, "more");
assert_eq!(r2.normal_text, "<|tool_calls_section_begin|>tool data");
assert!(!parser.is_in_reasoning());
}

#[test]
fn test_no_markers_does_not_stop_reasoning() {
let mut parser = create_test_parser(false, true);
let input = "<think>thinking<|tool_calls_section_begin|>stuff";
let result = parser.detect_and_parse_reasoning(input).unwrap();
assert_eq!(
result.reasoning_text,
"thinking<|tool_calls_section_begin|>stuff"
);
assert_eq!(result.normal_text, "");
}
}
6 changes: 2 additions & 4 deletions crates/reasoning_parser/src/parsers/cohere_cmd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

use crate::{
parsers::BaseReasoningParser,
traits::{ParseError, ParserConfig, ParserResult, ReasoningParser, DEFAULT_MAX_BUFFER_SIZE},
traits::{ParseError, ParserConfig, ParserResult, ReasoningParser},
};

/// Cohere Command model reasoning parser.
Expand All @@ -22,9 +22,7 @@ impl CohereCmdParser {
let config = ParserConfig {
think_start_token: "<|START_THINKING|>".to_string(),
think_end_token: "<|END_THINKING|>".to_string(),
stream_reasoning: true,
max_buffer_size: DEFAULT_MAX_BUFFER_SIZE,
always_in_reasoning: false,
..Default::default()
};

Self {
Expand Down
5 changes: 2 additions & 3 deletions crates/reasoning_parser/src/parsers/deepseek_r1.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use crate::{
parsers::BaseReasoningParser,
traits::{ParseError, ParserConfig, ParserResult, ReasoningParser, DEFAULT_MAX_BUFFER_SIZE},
traits::{ParseError, ParserConfig, ParserResult, ReasoningParser},
};

/// DeepSeek-R1 reasoning parser.
Expand All @@ -21,9 +21,8 @@ impl DeepSeekR1Parser {
let config = ParserConfig {
think_start_token: "<think>".to_string(),
think_end_token: "</think>".to_string(),
stream_reasoning: true,
max_buffer_size: DEFAULT_MAX_BUFFER_SIZE,
always_in_reasoning: true,
..Default::default()
};

Self {
Expand Down
6 changes: 2 additions & 4 deletions crates/reasoning_parser/src/parsers/glm45.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

use crate::{
parsers::BaseReasoningParser,
traits::{ParseError, ParserConfig, ParserResult, ReasoningParser, DEFAULT_MAX_BUFFER_SIZE},
traits::{ParseError, ParserConfig, ParserResult, ReasoningParser},
};

/// GLM45 reasoning parser.
Expand All @@ -20,9 +20,7 @@ impl Glm45Parser {
let config = ParserConfig {
think_start_token: "<think>".to_string(),
think_end_token: "</think>".to_string(),
stream_reasoning: true,
max_buffer_size: DEFAULT_MAX_BUFFER_SIZE,
always_in_reasoning: false,
..Default::default()
};

Self {
Expand Down
6 changes: 2 additions & 4 deletions crates/reasoning_parser/src/parsers/kimi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

use crate::{
parsers::BaseReasoningParser,
traits::{ParseError, ParserConfig, ParserResult, ReasoningParser, DEFAULT_MAX_BUFFER_SIZE},
traits::{ParseError, ParserConfig, ParserResult, ReasoningParser},
};

/// Kimi reasoning parser.
Expand All @@ -20,9 +20,7 @@ impl KimiParser {
let config = ParserConfig {
think_start_token: "◁think▷".to_string(),
think_end_token: "◁/think▷".to_string(),
stream_reasoning: true,
max_buffer_size: DEFAULT_MAX_BUFFER_SIZE,
always_in_reasoning: false,
..Default::default()
};

Self {
Expand Down
5 changes: 2 additions & 3 deletions crates/reasoning_parser/src/parsers/minimax.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use crate::{
parsers::BaseReasoningParser,
traits::{ParseError, ParserConfig, ParserResult, ReasoningParser, DEFAULT_MAX_BUFFER_SIZE},
traits::{ParseError, ParserConfig, ParserResult, ReasoningParser},
};

/// MiniMax M2 reasoning parser.
Expand All @@ -21,9 +21,8 @@ impl MiniMaxParser {
let config = ParserConfig {
think_start_token: "<think>".to_string(),
think_end_token: "</think>".to_string(),
stream_reasoning: true,
max_buffer_size: DEFAULT_MAX_BUFFER_SIZE,
always_in_reasoning: true,
..Default::default()
};

Self {
Expand Down
6 changes: 2 additions & 4 deletions crates/reasoning_parser/src/parsers/nano_v3.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

use crate::{
parsers::BaseReasoningParser,
traits::{ParseError, ParserConfig, ParserResult, ReasoningParser, DEFAULT_MAX_BUFFER_SIZE},
traits::{ParseError, ParserConfig, ParserResult, ReasoningParser},
};

/// NanoV3 / Nemotron reasoning parser.
Expand All @@ -24,9 +24,7 @@ impl NanoV3Parser {
let config = ParserConfig {
think_start_token: "<think>".to_string(),
think_end_token: "</think>".to_string(),
stream_reasoning: true,
max_buffer_size: DEFAULT_MAX_BUFFER_SIZE,
always_in_reasoning: false,
..Default::default()
};

Self {
Expand Down
9 changes: 3 additions & 6 deletions crates/reasoning_parser/src/parsers/qwen3.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use crate::{
parsers::BaseReasoningParser,
traits::{ParseError, ParserConfig, ParserResult, ReasoningParser, DEFAULT_MAX_BUFFER_SIZE},
traits::{ParseError, ParserConfig, ParserResult, ReasoningParser},
};

/// Qwen3 reasoning parser.
Expand All @@ -21,9 +21,7 @@ impl Qwen3Parser {
let config = ParserConfig {
think_start_token: "<think>".to_string(),
think_end_token: "</think>".to_string(),
stream_reasoning: true,
max_buffer_size: DEFAULT_MAX_BUFFER_SIZE,
always_in_reasoning: false,
..Default::default()
};

Self {
Expand Down Expand Up @@ -84,9 +82,8 @@ impl QwenThinkingParser {
let config = ParserConfig {
think_start_token: "<think>".to_string(),
think_end_token: "</think>".to_string(),
stream_reasoning: true,
max_buffer_size: DEFAULT_MAX_BUFFER_SIZE,
always_in_reasoning: true,
..Default::default()
};

Self {
Expand Down
5 changes: 2 additions & 3 deletions crates/reasoning_parser/src/parsers/step3.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

use crate::{
parsers::BaseReasoningParser,
traits::{ParseError, ParserConfig, ParserResult, ReasoningParser, DEFAULT_MAX_BUFFER_SIZE},
traits::{ParseError, ParserConfig, ParserResult, ReasoningParser},
};

/// Step3 reasoning parser.
Expand All @@ -20,9 +20,8 @@ impl Step3Parser {
let config = ParserConfig {
think_start_token: "<think>".to_string(),
think_end_token: "</think>".to_string(),
stream_reasoning: true,
max_buffer_size: DEFAULT_MAX_BUFFER_SIZE,
always_in_reasoning: true,
..Default::default()
};

Self {
Expand Down
10 changes: 9 additions & 1 deletion crates/reasoning_parser/src/traits.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,13 @@ pub struct ParserConfig {
/// For models with a template thinking toggle, this should be `false` —
/// the runtime will call `mark_reasoning_started()` when appropriate.
pub always_in_reasoning: bool,

/// Tokens that mark the start of a tool-call section in the model's output.
///
/// When the model skips the reasoning end token and jumps straight to
/// tool calls, the reasoning parser uses these markers to stop consuming
/// content as reasoning and return it as normal text for the tool parser.
pub tool_section_start_markers: Vec<String>,
}

impl Default for ParserConfig {
Expand All @@ -136,7 +143,8 @@ impl Default for ParserConfig {
think_end_token: "</think>".to_string(),
stream_reasoning: true,
max_buffer_size: DEFAULT_MAX_BUFFER_SIZE,
always_in_reasoning: false, // Default to false (explicit reasoning)
always_in_reasoning: false,
tool_section_start_markers: Vec::new(),
}
}
}
Expand Down
Loading
Loading