chat : accept packed XML tool calls in the Qwen3-Coder parser - #231
Open
danielhanchen wants to merge 2 commits into
Open
danielhanchen wants to merge 2 commits into
danielhanchen wants to merge 2 commits into
Conversation
MiMo-V2.6-Distill-Qwen-9B uses the Qwen3-Coder XML tool-call format but emits it with no newlines between tags: <tool_call><function=f><parameter=k>v</parameter></function></tool_call> The parser hardcoded "\n</parameter>\n", ">\n", "</function>\n" and "<tool_call>\n", so the first argument of a packed call consumed every later tag, the grammar never accepted, and generation ran to max_tokens with tool_calls empty. Make the newlines around the tags optional. Argument values are scanned for "\n</parameter>" before "</parameter>", so output from models that emit the newlines parses exactly as before. Add parse tests for the packed, half-packed, and packed parallel forms.
|
You have reached your Codex usage limits for security reviews. Please try again later. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
A string value now ends at </parameter> only when the next tag follows, so a literal </parameter> inside the value is kept, as it was before the packed-format change. The closing tag is confirmed with a lookahead so a partial stream does not close the value early. The grammar accepts the original newline form or the packed form, so constrained sampling for newline-emitting models is unchanged. Shorten comments and add a test for a literal </parameter> in a value.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
MiMo-V2.6-Distill-Qwen-9B uses the Qwen3-Coder XML tool-call format and is routed to
common_chat_params_init_qwen3_coder, but it emits the tags packed, with no newlines:The parser hardcoded
"\n</parameter>\n",">\n","</function>\n"and"<tool_call>\n". On packed output, the first string argument scanned forward for"\n</parameter>\n"and consumed every later tag, so the grammar never accepted. The server returned the call as rawcontentwithtool_calls: null, and generation ran untilmax_tokens. This matches the reports in the XiaomiMiMo/MiMo-V2.6-Distill-Qwen-9B discussions (#6, #7).Change
common/chat.cpp, Qwen3-Coder tool-call parser only:<tool_call>,<function=...>and<parameter=...>is now optional (a single\n, so any further leading whitespace stays part of the value)."\n</parameter>"or"</parameter>", scanned longest first. Output from models that emit the newlines parses exactly as before, and the trailing\nstays out of the value.</parameter>and</function>is absorbed withp.space()rather than a required\n.<tool_call>for Qwen3-Coder uses the same relaxed opener.The generated grammar accepts both the newline and packed forms, so constrained sampling does not force either one.
Tests
tests/test-chat.cpp, three new parse cases next to the existing Qwen3-Coder ones:</parameter>)test-chatpasses on this branch. With only the parser change reverted, the new cases fail.End to end on llama-server with the same change on upstream master, MiMo-V2.6-Distill-Qwen-9B BF16 with sampling: 4/4 tool calls parsed into
tool_calls, 4/4 follow-up turns stopped cleanly, and no runaway generations. Without the change, every call came back as raw content and ran to the token cap.