feat(tool call): Enhance Llama32Detector for improved JSON parsing in non-stream

[CatherineSue]

Motivation

Resolves: #6589 (comment)

Modifications

• Introduced error handling to handle invalid JSON part with mixed text
• Added a new UT for Llama32Detector

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! Gemini here, providing a summary of this pull request by @CatherineSue.

This PR aims to enhance the Llama32Detector's ability to parse tool calls, specifically addressing issues where the model's output might contain malformed JSON or a mix of text and JSON, particularly in non-streaming scenarios. The core change involves replacing the previous simple split-and-parse logic with a more robust iterative parsing approach using json.JSONDecoder().raw_decode. This new method allows the detector to find and parse multiple valid JSON objects even if they are interspersed with or preceded by invalid text, and correctly handle any trailing text after the last valid JSON.

Additionally, a new dedicated test class has been added to thoroughly test the Llama32Detector under various conditions, including those with invalid JSON parts and mixed text, ensuring the improved parsing logic functions as expected.

Highlights

• Improved JSON Parsing: The Llama32Detector's detect_and_parse method has been refactored to use json.JSONDecoder().raw_decode in a loop. This allows it to parse multiple tool calls and gracefully handle invalid JSON segments or mixed text within the output, skipping malformed parts and continuing to search for valid JSON objects.
• Robust Error Handling: The new parsing logic includes error handling for json.JSONDecodeError, enabling the detector to recover from parsing failures for individual JSON parts and attempt to find the next valid tool call object within the text.
• Handling Trailing Text: The updated parser now correctly identifies and includes any text that appears after the last successfully parsed JSON object in the normal_text output.
• New Unit Tests: A dedicated test class TestLlama32Detector has been added with several test cases covering various scenarios, including single/multiple tool calls, mixed valid/invalid JSON, trailing text, and the presence of the <|python_tag|>.

Changelog

• python/sglang/srt/function_call/llama32_detector.py
  • Modified detect_and_parse to use text.split("<|python_tag|>", maxsplit=1) for the initial split.
  • Replaced the simple split-by-separator and json.loads loop with an iterative parsing approach using json.JSONDecoder().raw_decode.
  • Added error handling (try...except json.JSONDecodeError) to skip invalid JSON parts and search for the next potential JSON object start ({"name":).
  • Introduced safe_idx to track the end of the last successfully parsed JSON object.
  • Ensured trailing text after the last valid JSON object is appended to the normal_text result.
• test/srt/test_function_call_parser.py
  • Added a new test class TestLlama32Detector.
  • Added setUp method to initialize tools and the detector.
  • Added test cases for test_single_json, test_multiple_json_with_separator, test_multiple_json_with_separator_customized, test_json_with_trailing_text, test_invalid_then_valid_json, test_plain_text_only, and test_with_python_tag_prefix to cover various parsing scenarios for the Llama32Detector.

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.

---

JSON, a data tree,
Sometimes tangled, wild, and free.
Parse with care, you see,
Or errors you will decree.

Footnotes

1. 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. ↩

[CatherineSue]

Test:
warnings are expected

[gemini-code-assist]

Code Review

This pull request introduces valuable enhancements to the Llama32Detector for parsing JSON tool calls, especially in handling malformed or mixed content by using raw_decode and an error recovery mechanism. The added unit tests are helpful for verifying these changes.

I've identified a high-severity issue concerning potential loss of non-JSON text situated between valid tool calls or before a parse error, and a medium-severity issue related to the handling of the tool call separator. Addressing these will significantly improve the robustness and correctness of the parser.

Summary of Findings

• Potential Data Loss: Intermediate non-JSON text between tool calls or before a recovered parsing error might be lost. The current logic for constructing normal_text primarily relies on trailing_text, which captures only the final unparsed segment after the last successful JSON parse. (Severity: high)
• Separator Handling Rigidity: The parser advances its index by a fixed len(self.tool_call_separator) after each successful JSON parse. This assumes the separator is always present and identical, which could lead to misparsing if the actual output varies (e.g., missing separator, different separator, variable whitespace). (Severity: medium)
• Test Case Clarity (Low Severity - Not Commented): The test test_multiple_json_with_separator_customized passes due to the error recovery mechanism rather than flexible separator handling. Its name might be slightly misleading. This is a minor point and not a functional bug in the main code.

Merge Readiness

The pull request makes good progress in improving JSON parsing for Llama32Detector. However, due to the identified high-severity issue regarding potential data loss of intermediate text, I recommend addressing this before merging. The medium-severity issue concerning separator handling should also be considered for improved robustness. Once these points are addressed, the PR should be in a much stronger position. As an AI, I am not authorized to approve pull requests; please ensure further review and approval from team members.

[gemini-code-assist]

There's a potential for data loss of non-JSON text segments that appear between valid JSON tool calls or before a JSON parsing error that's subsequently recovered from.

Currently, safe_idx is updated only after a successful JSON parse and the assumed separator. If a parsing error occurs, idx might jump to the next potential JSON start (next_obj_start), but the text between the previous safe_idx and the point of error, or between the point of error and next_obj_start, might not be fully incorporated into the normal_text.

The trailing_text (calculated as action_text[safe_idx:].strip()) captures text after the last position safe_idx was updated (i.e., after the last successfully parsed JSON and its separator). This means intermediate unparseable segments could be lost.

For example, with action_text = '{"name":"call1"}; intermediate text {"name":"call2"}; final text':

1. call1 is parsed. safe_idx is updated to point after call1; (i.e., start of intermediate text).
2. An attempt to parse from intermediate text fails. idx jumps to the start of call2.
3. call2 is parsed. safe_idx is updated to point after call2; (i.e., start of final text).
4. Loop ends or next parse fails.
5. trailing_text becomes final text.
   In this scenario, intermediate text is lost.

Could we adjust the logic to accumulate all non-JSON segments into normal_text? This would likely involve collecting parts of action_text that are skipped or determined to be unparseable into a list and then joining them.

A conceptual approach:

1. Initialize collected_normal_text_parts = [normal_text_prefix] (where normal_text_prefix is from the <|python_tag|> split).
2. In the parsing loop, when raw_decode fails:
   a. The text from the previous safe_idx to the current idx (where the error occurred) is an interstitial segment. Add action_text[safe_idx:idx] to collected_normal_text_parts if idx > safe_idx.
   b. If next_obj_start is found, the segment action_text[idx:next_obj_start] is the unparseable JSON. Add this to collected_normal_text_parts.
   c. Update idx = next_obj_start and, crucially, safe_idx = idx so the next segment starts correctly.
   d. If next_obj_start is not found, the remainder action_text[idx:] is unparseable. Add it to collected_normal_text_parts and break.
3. After the loop, if it exited normally, any remaining text from safe_idx to len(action_text) should be added.
4. Join collected_normal_text_parts to form the final normal_text.

This would ensure all parts of action_text are accounted for, either as a parsed tool call or as part of the normal text output.

        collected_normal_text_parts = [normal_text]  # `normal_text` here is from the initial split
        decoder = json.JSONDecoder()
        current_idx = 0
        # safe_idx tracks the start of the current segment being processed.
        # It's updated to mark the beginning of text that will either be parsed as JSON
        # or appended to collected_normal_text_parts.
        safe_idx = 0 
        all_actions = []
        action_text_len = len(action_text)

        while current_idx < action_text_len:
            try:
                # Append text between the last safe_idx and current parsing position to normal_text_parts.
                # This captures text that was not part of a successful JSON parse in the previous step.
                if current_idx > safe_idx:
                    collected_normal_text_parts.append(action_text[safe_idx:current_idx])
                
                obj, end_offset = decoder.raw_decode(action_text[current_idx:])
                all_actions.append(obj)
                current_idx += end_offset
                safe_idx = current_idx # Mark the end of the successfully parsed JSON object.

                # Attempt to skip the tool_call_separator. The original code advanced idx by len(separator).
                # This assumes the separator is present. A more robust handling might be needed here (see other comment),
                # but for this suggestion, we'll stick to the PR's implicit consumption.
                # If the separator is considered part of the scannable action_text:
                if current_idx < action_text_len and action_text.startswith(self.tool_call_separator, current_idx):
                    current_idx += len(self.tool_call_separator)
                # If separator is not strictly required or can vary, this part needs more thought.
                # For now, this mimics the PR's advancement past an assumed separator.
                safe_idx = current_idx # Update safe_idx to be after the JSON and its assumed separator.

            except json.JSONDecodeError as e:
                logger.warning(
                    f"Failed to parse JSON part starting at index {current_idx}: {action_text[current_idx:]}, JSON parse error: {str(e)}"
                )
                # Append text from last safe_idx up to where the error occurred.
                if current_idx > safe_idx:
                    collected_normal_text_parts.append(action_text[safe_idx:current_idx])

                next_obj_start = action_text.find('{"name":', current_idx + 1)
                if next_obj_start != -1:
                    # The text from current_idx (where error occurred) to next_obj_start is the unparseable segment.
                    collected_normal_text_parts.append(action_text[current_idx:next_obj_start])
                    current_idx = next_obj_start
                    safe_idx = current_idx # Next attempt starts here; this becomes the new safe point.
                else:
                    # No more '{"name":' found. The rest of the string from current_idx is normal text.
                    collected_normal_text_parts.append(action_text[current_idx:])
                    safe_idx = action_text_len # All remaining text accounted for.
                    break # Exit loop
                continue

        # After the loop, if there's any remaining text from the last safe_idx to the end.
        if safe_idx < action_text_len:
            collected_normal_text_parts.append(action_text[safe_idx:])

        calls = self.parse_base_json(all_actions, tools) if all_actions else []
        final_normal_text = "".join(collected_normal_text_parts)
        return StreamingParseResult(
            normal_text=final_normal_text.strip(), calls=calls
        )

[CatherineSue]

action_text = '{"name":"call1"}; intermediate text {"name":"call2"}; final text'
This too is an extreme case, usually model doesn't give this kind of response. We can deal with it when such case happens. Otherwise the logic becomes too complicated.

Even with the logic you suggested, the collected_normal_text_parts may overlook if the intermediate text involves some malformed JSON. For instance, action_text = '{"name":"call1"}; {"malperformed, {"name":"call2"}; final text'