Update tool parser and related documentation#11223
Update tool parser and related documentation#11223JustinTong0323 merged 5 commits intosgl-project:mainfrom
Conversation
- Added reasoning parser note for reasoning models in tool parser documentation. - Updated supported parsers table to include Qwen, DeepSeek, GLM, and Mistral models. - Refactored tool call parser mappings to use simplified names. - Implemented deprecation warnings for outdated tool call parsers in server arguments handling.
Summary of ChangesHello @JustinTong0323, 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 focuses on enhancing the tool parsing capabilities and user experience by updating relevant documentation, streamlining internal parser naming conventions, and introducing deprecation warnings for older parser names. These changes aim to improve clarity, maintainability, and compatibility with a broader range of language models. 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 updates the tool parser names for better consistency and adds support for new models like Qwen, DeepSeek, GLM, and Mistral. The changes include refactoring parser mappings, updating documentation, and adding deprecation warnings for backward compatibility. My review focuses on improving code clarity and fixing a minor issue in the documentation. Overall, the changes are well-structured and improve the usability of the tool parsing feature.
| "| `qwen` | Qwen series (e.g. `Qwen/Qwen3-Next-80B-A3B-Instruct`, `Qwen/Qwen3-VL-30B-A3B-Thinking`) except Qwen3-Coder| |\n", | ||
| "| `qwen3_coder` | Qwen3-Coder (e.g. `Qwen/Qwen3-Coder-30B-A3B-Instruct`) | |\n", | ||
| "| `deepseekv3` | DeepSeek-v3 (e.g., `deepseek-ai/DeepSeek-V3-0324`) | Recommend adding `--chat-template ./examples/chat_template/tool_chat_template_deepseekv3.jinja` to launch command. |\n", | ||
| "| `deepseekv31` | DeepSeek-V3.1 and DeepSeek-V3.2 (e.g. `deepseek-ai/DeepSeek-V3.1`, `deepseek-ai/DeepSeek-V3.2-Exp`) | Recommend adding `--chat-template ./examples/chat_template/tool_chat_template_deepseekv31.jinja` (Or ..deepseekv32.jinja for DeepSeek-V3.2) to launch command. |\n", |
There was a problem hiding this comment.
The path ..deepseekv32.jinja in the note is a bit ambiguous and could be a typo. For clarity and to avoid user confusion, it's better to provide the full relative path from the project root, similar to the other examples. Also, "Or" is typically not capitalized in parentheses.
| `deepseekv31` | DeepSeek-V3.1 and DeepSeek-V3.2 (e.g. `deepseek-ai/DeepSeek-V3.1`, `deepseek-ai/DeepSeek-V3.2-Exp`) | Recommend adding `--chat-template ./examples/chat_template/tool_chat_template_deepseekv31.jinja` (or `./examples/chat_template/tool_chat_template_deepseekv32.jinja` for DeepSeek-V3.2) to launch command. |\n
| if self.tool_call_parser in deprecated_tool_call_parsers: | ||
| logger.warning( | ||
| f"The tool_call_parser '{self.tool_call_parser}' is deprecated. Please use '{deprecated_tool_call_parsers[self.tool_call_parser]}' instead." | ||
| ) | ||
| self.tool_call_parser = deprecated_tool_call_parsers[self.tool_call_parser] |
There was a problem hiding this comment.
To improve readability and avoid multiple dictionary lookups, you can store the new parser name in a variable.
Additionally, the deprecated_tool_call_parsers dictionary is created on each call. Since this is a constant mapping, consider defining it as a module-level constant (e.g., _DEPRECATED_TOOL_CALL_PARSERS) outside this method to avoid repeated creation and improve performance.
| if self.tool_call_parser in deprecated_tool_call_parsers: | |
| logger.warning( | |
| f"The tool_call_parser '{self.tool_call_parser}' is deprecated. Please use '{deprecated_tool_call_parsers[self.tool_call_parser]}' instead." | |
| ) | |
| self.tool_call_parser = deprecated_tool_call_parsers[self.tool_call_parser] | |
| if self.tool_call_parser in deprecated_tool_call_parsers: | |
| new_parser = deprecated_tool_call_parsers[self.tool_call_parser] | |
| logger.warning( | |
| f"The tool_call_parser '{self.tool_call_parser}' is deprecated. Please use '{new_parser}' instead." | |
| ) | |
| self.tool_call_parser = new_parser |
| def _handle_deprecated_args(self): | ||
| pass | ||
| # handle deprecated tool call parsers | ||
| deprecated_tool_call_parsers = {"qwen25": "qwen", "glm45": "glm"} |
There was a problem hiding this comment.
I saw qwen25 still exists in the above map.
There was a problem hiding this comment.
Yes, here is just for the warning... Maybe remove in future
| "metadata": {}, | ||
| "source": [ | ||
| "## Currently supported parsers:\n", | ||
| "> For reasoning models, you can always enable reasoning parser together with tool call parser.\n", |
There was a problem hiding this comment.
Does this mean we have a single command argument for both parsers?
There was a problem hiding this comment.
Oh that's ambiguous, just remove this line.
| "| Parser | Supported Models | Notes |\n", | ||
| "|---|---|---|\n", | ||
| "| `llama3` | Llama 3.1 / 3.2 / 3.3 (e.g. `meta-llama/Llama-3.1-8B-Instruct`, `meta-llama/Llama-3.2-1B-Instruct`, `meta-llama/Llama-3.3-70B-Instruct`) | |\n", | ||
| "| `qwen` | Qwen series (e.g. `Qwen/Qwen3-Next-80B-A3B-Instruct`, `Qwen/Qwen3-VL-30B-A3B-Thinking`) except Qwen3-Coder| |\n", |
There was a problem hiding this comment.
Should we use alphabetical orders here to make it easier to maintain?
3 participants
Motivation
Modifications
Accuracy Tests
Benchmarking and Profiling
Checklist