fix(tool call): Fix tool_index in PythonicDetector and issues with mixed output in non-streaming #6678

CatherineSue · 2025-05-27T22:21:32Z

Motivation

PythonicDetector doesn't support tool_index in the streaming mode
When there are mixed outputs (text containing both normal_text and function_call), the detect_and_parse doesn't extract the function call as it simply does regex.match

This PR fixes the above issues.

Modifications

Add tool_index in the ToolCallItem generated in PythonicDetector
Fix issues when there are mixed outputs in detect_and_parse

Checklist

Format your code according to the Code Formatting with Pre-Commit.
Add unit tests as outlined in the Running Unit Tests.
Update documentation / docstrings / example tutorials as needed, according to Writing Documentation.
Provide throughput / latency benchmark results and accuracy evaluation results as needed, according to Benchmark and Profiling and Accuracy Results.
For reviewers: If you haven't made any contributions to this PR and are only assisting with merging the main branch, please remove yourself as a co-author when merging the PR.
Please feel free to join our Slack channel at https://slack.sglang.ai to discuss your PR.

gemini-code-assist

Hello @CatherineSue, I'm Gemini Code Assist¹! 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!

Summary of Changes

Hello, I'm Gemini, your friendly code assistant. I've reviewed this pull request by CatherineSue which focuses on improving the tool call detection logic, specifically for the Pythonic format detector. The primary goals are to correctly handle the tool_index when parsing Pythonic tool calls and to fix an issue where the detector failed to find tool calls if they were embedded within other text (mixed output) in non-streaming scenarios. The changes involve modifying the PythonicDetector to use regex search instead of match, allowing it to find tool calls anywhere in the input string, and adjusting how the tool_index is determined for Pythonic calls. Additionally, a helper method in the base detector class was made private.

Highlights

Pythonic Detector Improvements: The PythonicDetector's detect_and_parse method is updated to correctly extract tool calls even when they are surrounded by other text. This is achieved by switching from regex.match (which only matches from the start of the string) to regex.search (which finds the pattern anywhere).
Tool Index Calculation: For Pythonic tool calls, the tool_index in the resulting ToolCallItem is now set to the index of the call within the parsed list of calls, rather than attempting to map the function name to an index from the provided tools list. This aligns with how Pythonic tool calls are typically structured as a list of function calls.
Undefined Function Handling: A check is added in the PythonicDetector to log a warning if the model attempts to call a function name that is not present in the list of provided tools.
Internal Method Visibility: A helper method ends_with_partial_token in the base detector class is renamed to _ends_with_partial_token to indicate it's intended for internal use.

Changelog

Click here to see the changelog

python/sglang/srt/function_call/base_format_detector.py
- Renamed ends_with_partial_token method to _ends_with_partial_token (line 75).
- Updated calls to the renamed method _ends_with_partial_token (line 111).
python/sglang/srt/function_call/pythonic_detector.py
- Changed regex.match to regex.search in has_tool_call to find pattern anywhere (line 36).
- Refactored detect_and_parse to use regex.search to find the tool call part within the text (line 42).
- Added logic to extract and preserve text before and after the found tool call part (lines 44-52).
- Modified ast.parse to parse only the extracted tool call text (line 55).
- Added a check to ensure the called function name exists in the provided tools (lines 73-78).
- Updated ToolCallItem creation to use the call's index within the list for tool_index (line 84).
- Ensured the combined normal text (before and after the tool call) is returned (line 90).
python/sglang/srt/function_call/qwen25_detector.py
- Updated call to the renamed method _ends_with_partial_token (line 89).

Using Gemini Code Assist

The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and 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 to provide feedback.

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.

A list of calls,
In Python's guise,
Parsed from text,
Before your eyes.
No longer matched, but searched.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request aims to fix two main issues: the tool_index in PythonicDetector for streaming mode and the handling of mixed text/tool_call outputs in detect_and_parse for non-streaming scenarios. The changes in PythonicDetector are significant and generally move in the right direction, especially the use of regex.search for more flexible tool call detection and the updated logic for tool_index.

I've identified a critical issue regarding a potential AttributeError if the regex search doesn't find a match, and a couple of medium-severity suggestions related to error handling and test coverage. Addressing the critical issue is essential before merging.

Summary of Findings

Potential AttributeError in PythonicDetector: A critical issue was found in python/sglang/srt/function_call/pythonic_detector.py where match.start() and match.end() could be called on a None object if self.tool_call_regex.search(text) doesn't find a match. This needs to be addressed to prevent runtime errors.
Normal Text Handling in Error Cases: In PythonicDetector.detect_and_parse, when a tool call is matched by regex but subsequent AST parsing or other processing fails, the method currently returns the original input string as normal_text. It's suggested to consider returning the already-parsed normal_text (text surrounding the attempted tool call) to better reflect the parsing progress.
Test Coverage for detect_and_parse: The PR description mentions fixing issues with mixed outputs in detect_and_parse. While new tests are added, dedicated unit tests for PythonicDetector.detect_and_parse specifically covering various mixed-content scenarios (e.g., text before tool call, text after, text both before and after, text only, tool call only) would be beneficial to directly validate this aspect of the fix.
Method Renaming for Clarity: The renaming of ends_with_partial_token to _ends_with_partial_token in base_format_detector.py is a good practice, aligning with PEP 8 conventions for internal helper methods. This was a low severity finding and not commented on directly due to review settings.
Tool Index Logic: The change in PythonicDetector to set ToolCallItem.tool_index to call_index (the index of the call within the response's list of calls) is a clear and intentional change, aligning with the PR's goal. This was a low severity finding and not commented on directly due to review settings.

Merge Readiness

This pull request makes important improvements to tool call parsing. However, there is a critical issue identified (potential AttributeError) that must be resolved before merging. Additionally, there are a couple of medium-severity suggestions regarding error handling and test coverage that would improve the robustness and maintainability of the code. I recommend addressing these points, especially the critical one. As an AI, I am not authorized to approve pull requests; please ensure further review and approval by team members once the changes are made.

gemini-code-assist · 2025-05-27T22:23:27Z

python/sglang/srt/function_call/pythonic_detector.py

-            # Not a pythonic tool call format
-            return StreamingParseResult(normal_text=text, calls=[])
+
+        match = self.tool_call_regex.search(text)


The match object, resulting from self.tool_call_regex.search(text), could be None if no tool call pattern is found in the input text. However, the subsequent lines (44-45) attempt to call match.start() and match.end() without first checking if match is None. This will lead to an AttributeError if no match is found.

Could we add a check for match is None immediately after this line and handle the no-match case (e.g., by returning StreamingParseResult(normal_text=text, calls=[])) before attempting to use match?

Fixed: added a check for match is None

python/sglang/srt/function_call/pythonic_detector.py

CatherineSue · 2025-05-27T22:32:51Z

Test for mixed output:
Before:

After:

Test for tool_index:
Before:
This is correct simply because the two functions are different names and it has the same index in the tools list.

After:

…detector.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

…xed output in non-streaming (sgl-project#6678)

…d-on-v0.4.10.post2 (#10) * Cherry-pick conflicts: features-based-on-v0.4.6.post5 → features-based-on-v0.4.7.post1 (#4) * [Docker] Use local source file instead of git clone sglang (#4) * [OAI] Support non-normalized logprobs in OpenAI server (#5961) * feat: Improve Mistral and Qwen25 function call parsing (#6597) * [Bugfix]: Fix call for function_call_parser.multi_format_detector in adapter.py (#6650) * fix(tool call): Fix tool_index in PythonicDetector and issues with mixed output in non-streaming (#6678) * bugfix(OAI): Fix image_data processing for jinja chat templates (#6877) * compressed-tensors check_accelerate error need install accelerate * Fix OOM in Llama4 with large images * Fix lint * ✅ Resolved: Cherry-pick --------- Co-authored-by: Chao Yang <chao.c.yang@oracle.com> Co-authored-by: Simo Lin <linsimo.mark@gmail.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * ✅ Resolved: Cherry-pick from features-based-on-v0.4.7.post1 to features-based-on-v0.4.10.post2 * Delete openai_api folder --------- Co-authored-by: Chao Yang <chao.c.yang@oracle.com> Co-authored-by: Simo Lin <linsimo.mark@gmail.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

CatherineSue added 2 commits May 27, 2025 15:17

Support tool_index in PythonicDetector

bf96854

Fix PythonicDetector with combined normal texts and calls output

8ed8d74

CatherineSue requested review from merrymercy, Ying1123, hnyls2002, zhyncs, ispobock and ByronHsu as code owners May 27, 2025 22:21

gemini-code-assist bot reviewed May 27, 2025

View reviewed changes

gemini-code-assist bot suggested changes May 27, 2025

View reviewed changes

zhyncs assigned JustinTong0323 May 27, 2025

CatherineSue and others added 3 commits May 27, 2025 15:36

Update gemini's suggestion: python/sglang/srt/function_call/pythonic_…

9a626d4

…detector.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Update gemini's suggestion: python/sglang/srt/function_call/pythonic_…

7550fdd

…detector.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Fix match is None when no tool call detected

392b94c

zhyncs approved these changes May 27, 2025

View reviewed changes

zhyncs merged commit bdb962d into main May 27, 2025
28 of 39 checks passed

zhyncs deleted the chang/pythonic-tool-index branch May 27, 2025 23:18

CatherineSue mentioned this pull request May 28, 2025

[Feature] Tool Call Roadmap #6589

Closed

7 tasks

Layssy pushed a commit to Layssy/sglang-iaas that referenced this pull request Jun 9, 2025

fix(tool call): Fix tool_index in PythonicDetector and issues with mi…

1dd51a0

…xed output in non-streaming (sgl-project#6678)

xwu-intel pushed a commit to xwu-intel/sglang that referenced this pull request Jun 17, 2025

fix(tool call): Fix tool_index in PythonicDetector and issues with mi…

854a8d7

…xed output in non-streaming (sgl-project#6678)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(tool call): Fix tool_index in PythonicDetector and issues with mixed output in non-streaming #6678

fix(tool call): Fix tool_index in PythonicDetector and issues with mixed output in non-streaming #6678

Uh oh!

CatherineSue commented May 27, 2025 •

edited

Loading

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot May 27, 2025

Uh oh!

CatherineSue May 27, 2025

Uh oh!

Uh oh!

Uh oh!

CatherineSue commented May 27, 2025

Uh oh!

Uh oh!

Uh oh!

fix(tool call): Fix tool_index in PythonicDetector and issues with mixed output in non-streaming #6678

fix(tool call): Fix tool_index in PythonicDetector and issues with mixed output in non-streaming #6678

Uh oh!

Conversation

CatherineSue commented May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Checklist

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Changelog

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Summary of Findings

Merge Readiness

Uh oh!

gemini-code-assist bot May 27, 2025

Choose a reason for hiding this comment

Uh oh!

CatherineSue May 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

CatherineSue commented May 27, 2025

Uh oh!

Uh oh!

Uh oh!

CatherineSue commented May 27, 2025 •

edited

Loading