[router][tool parser] Modify tool parser to return both normal text and tool calls (non-stream)#10995
[router][tool parser] Modify tool parser to return both normal text and tool calls (non-stream)#10995
Conversation
1. Updated `ToolParser` trait signature - Changed parse_complete from returning `ToolParserResult<Vec<ToolCall>>` to `ToolParserResult<(String, Vec<ToolCall>)>` 2. Updated all parser implementations with efficient normal text extraction: - JsonParser: Returns empty normal text for all cases (as requested) - MistralParser: Implemented `extract_json_array_with_pos()` method to eliminate redundant find() calls - LlamaParser: Extracts text before `<|python_tag|>` markers - QwenParser: Uses `captures_iter()` with match positions for both tool calls and normal text - DeepSeekParser: Uses `find_iter()` with match positions - KimiK2Parser: Uses `captures_iter()` with match positions - PythonicParser: Empty normal text (by design) - Step3Parser: Uses regex extractors with match positions - GptOssParser: Uses `captures_iter()` with match positions 3. Fixed router implementation - Use normal_text as processed_text 4. Fixed all test cases - Updated 166+ test cases across multiple files to handle the new (String, Vec<ToolCall>) tuple format 5. Fix tool_parser_benchmark.rs
- Use stack based approach to extract json from a mixed text - Properly handle reset state when we hit malformed JSON - Add unit tests
- Add normal text extraction in `extract_tool_calls` - Add unit tests
- When tool call parsing fails, the original text should be preserved as normal text
Summary of ChangesHello @CatherineSue, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the robustness and completeness of the tool parsing system by ensuring that all input text is accounted for. Previously, if tool calls were not found or parsing encountered an error, any non-tool text might have been implicitly discarded. The updated system now explicitly separates and returns both successfully parsed tool calls and any remaining 'normal' text, preventing data loss and providing a more reliable foundation for downstream processing, especially in scenarios involving mixed model outputs or partial parsing failures. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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 is a significant and valuable refactoring of the tool parsing logic. The core change to have parse_complete return both normal text and tool calls is well-implemented across the majority of the parsers. The addition of fallback tests is also a great improvement for robustness.
I've identified a few areas for improvement:
- A high-severity bug in
llama_parser.rswhere it incorrectly recalculatesnormal_textinstead of using the value from the underlyingjson_parser. - A medium-severity issue in several parsers where text between tool calls is lost.
- A medium-severity issue in the new fallback tests where assertions are incorrect.
- A medium-severity issue in
mistral_parser.rswhere text after the tool call block is lost.
Addressing these points will make the new parsing logic even more robust and correct. Overall, this is a solid contribution.
- When split among separators we should preserve the separator - Update tests
- When all cases fail, it should return original text - Add test case
CatherineSue
changed the title
…nd tool calls (non-stream) (sgl-project#10995)
3 participants
Motivation
Tool parsers should reserve any content before and after tool calls. Especially, when tool call parsing fails, the original text should be preserved as normal text.
The current behavior only returns a tool-calls list. When it is empty due to parsing failures, the user won't get any content. But completion tokens are still counted.
Modifications
reasoning_parser/traits.rsModify the tool parser system to return both normal text and tool calls
ToolParsertrait signature - Changed parse_complete from returningToolParserResult<Vec<ToolCall>>toToolParserResult<(String, Vec<ToolCall>)>extract_json_from_textto return both normal text and json contentextract_json_array_with_pos()method to extract normal text before the tool calls. (After is ignored)<|python_tag|>markerscaptures_iter()with match positions for both tool calls and normal textfind_iter()with match positionscaptures_iter()with match positionsProper error handling for parsing failures
ParsingFailederror in MistralParserparse_json_arraythrows aToolParserError::ParsingFailederror but it is not captured later.Accuracy Tests
JsonParser
This PR refactored
extract_json_from_textto use a stack based approach to extract a valid JSON object from a mixed test.Here is a detailed comparison:
JSON Parser Implementation Comparison
Detailed Edge Case Analysis
Benchmarking and Profiling
Checklist