fix(deepseekv32): parse bare invoke blocks without function_calls wrapper - #23786
fix(deepseekv32): parse bare invoke blocks without function_calls wrapper#23786Kangyan-Zhou wants to merge 2 commits into
Conversation
…pper When tool_choice="required"/named triggers a structural_tag with at_least_one=True (PR sgl-project#21593), the grammar forces the model to emit a single match of the detector's structure_info. For DeepSeek-V3.2 that match is the inner <|DSML|invoke>...</|DSML|invoke> only, with no surrounding <|DSML|function_calls>...</|DSML|function_calls> wrapper. `has_tool_call` and `parse_streaming_increment` already accept that shape, but the non-streaming `detect_and_parse` required the wrapper and silently returned zero calls, surfacing in the nightly 8-GPU H200 job as `tool_call (3/9)` failures across all four V3.2 variants (`basic_format`/`required`/`specific` with empty asserts; `strict`/`multiturn`/`thinking` with "list index out of range" from indexing an empty `tool_calls`). `streaming` and `parallel` passed because they don't go through this path. Make `detect_and_parse` mirror the streaming path: scan from the earliest tool-call marker (`bot_token` or `<|DSML|invoke`), prefer wrapper-bounded content when a complete wrapper is present, otherwise parse invoke blocks directly from the remainder. Add three unit tests for the bare-invoke shapes (XML params, JSON params, multiple invokes) — all three fail without the fix and pass with it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request updates the DeepSeek V3.2 function call detector to support both wrapped and bare function call formats, which is necessary when tool selection is forced. It also adds comprehensive unit tests for these new scenarios. The review feedback identifies a potential data loss issue where text appearing after a complete function call wrapper is discarded; a code suggestion was provided to correctly partition the text into normal and scan segments regardless of the wrapper's position.
| # Use the earliest tool-call marker as the boundary for normal_text. | ||
| marker_indices = [i for i in (bot_idx, invoke_idx) if i != -1] | ||
| start_idx = min(marker_indices) | ||
| normal_text = text[:start_idx].strip() | ||
|
|
||
| # Prefer the wrapper-bounded content if a complete wrapper is present; | ||
| # otherwise scan everything from the first marker onward for invoke blocks. | ||
| function_calls_match = re.search(self.function_calls_regex, text, re.DOTALL) | ||
| scan_text = ( | ||
| function_calls_match.group(1) if function_calls_match else text[start_idx:] | ||
| ) |
There was a problem hiding this comment.
The current implementation of normal_text construction only captures text preceding the tool call markers. If a complete <|DSML|function_calls> wrapper is present, any text following the closing tag is discarded and lost from the output.
Additionally, the logic can be simplified by checking for the wrapper match first and using its boundaries to correctly partition the text into normal_text (everything outside the wrapper) and scan_text (everything inside).
| # Use the earliest tool-call marker as the boundary for normal_text. | |
| marker_indices = [i for i in (bot_idx, invoke_idx) if i != -1] | |
| start_idx = min(marker_indices) | |
| normal_text = text[:start_idx].strip() | |
| # Prefer the wrapper-bounded content if a complete wrapper is present; | |
| # otherwise scan everything from the first marker onward for invoke blocks. | |
| function_calls_match = re.search(self.function_calls_regex, text, re.DOTALL) | |
| scan_text = ( | |
| function_calls_match.group(1) if function_calls_match else text[start_idx:] | |
| ) | |
| # Prefer the wrapper-bounded content if a complete wrapper is present. | |
| # This allows capturing text both before and after the wrapper in normal_text. | |
| function_calls_match = re.search(self.function_calls_regex, text, re.DOTALL) | |
| if function_calls_match: | |
| normal_text = (text[:function_calls_match.start()] + text[function_calls_match.end():]).strip() | |
| scan_text = function_calls_match.group(1) | |
| else: | |
| # Use the earliest tool-call marker as the boundary for normal_text. | |
| start_idx = min(i for i in (bot_idx, invoke_idx) if i != -1) | |
| normal_text = text[:start_idx].strip() | |
| scan_text = text[start_idx:] |
…ing-text test - Drop two comments that restated the next line, shorten the top comment to keep only the load-bearing "why bare exists" context. - Add a test that puts assistant text after a bare invoke to lock in that the trailing text doesn't bleed into invoke content. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Covered by #34458 (5899674). You are credited as a co-author on that commit. If I misread what this PR does and part of it is still missing, please rebase on main and reopen — happy to take it. |
Summary
The nightly 8-GPU H200 job fails on
test_deepseek_v32_all_variantswithtool_call (3/9)across all four V3.2 variants (run #24754179764, job 72423730579). The pattern is:streaming,none, andparallelpass.Root cause
PR #21593 introduced
at_least_one=Truestructural_tag constraints fortool_choice="required"/named so the grammar forces the model to emit at least one match of the detector'sstructure_info. For DeepSeek-V3 that PR also updatedstructure_info()to embed the wrapper tokens (<|tool▁calls▁begin|>...<|tool▁calls▁end|>), but DeepSeek-V3.2'sstructure_info()was left pointing at the inner block only:So under
tool_choice="required"the constrained model emits a bare<|DSML|invoke>...</|DSML|invoke>with no surrounding<|DSML|function_calls>...</|DSML|function_calls>wrapper.DeepSeekV32Detector.has_tool_callandparse_streaming_incrementalready accept the bare shape (matches bothbot_tokenand<|DSML|invoke), butdetect_and_parse(the non-streaming path) required a complete wrapper match and silently returned zero calls. That maps cleanly to the test failures:basic_format/required/specific—assert msg.tool_calls and len(msg.tool_calls) > 0fails with no message becausemsg.tool_calls is None.strict/multiturn/thinking—tool_calls[0]raisesIndexError: list index out of range.streamingpasses because it goes through the streaming detector, which is already lenient.parallelpasses because it usestool_choice="auto"and the model emits the wrapper naturally.Change
Make
detect_and_parsemirror the streaming path: find the earliest tool-call marker (bot_tokenor<|DSML|invoke), use wrapper-bounded content when a complete wrapper is present, otherwise scan invoke blocks directly from the remainder.Test plan
mainand passes with the fix.pytest test/registered/unit/function_call/test_function_call_parser.py— 193/193 pass.pytest test/registered/unit/function_call/test_function_call_parser.py::TestDeepSeekV32Detector— 10/10 pass (7 pre-existing + 3 new).pre-commit run --filesclean (black/ruff/isort/codespell).test_deepseek_v32_all_variants(only available on the nightly runner; will verify once the next nightly runs against this branch, or by manually triggering after merge).🤖 Generated with Claude Code