Add escaping of whitespaces in qwen3coder tool output #3691
Conversation
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
atobiszei
force-pushed
the
atobisze_check_qwen3coder_fix
branch
from
0b9b92d to
cca1f42
Compare
atobiszei
force-pushed
the
atobisze_check_qwen3coder_fix
branch
from
6b13486 to
8b7f8f6
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds whitespace escaping functionality to the Qwen3Coder tool output parser and improves handling of improperly formatted tool calls that begin with <function=...> tags instead of the expected <tool_call> tags.
- Introduces an
escapeStringfunction to properly escape newlines in tool call arguments - Adds support for parsing tool calls that start with
<function=...>directly (without<tool_call>) - Updates test cases to reflect the new escaped output format and additional parsing scenarios
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| qwen3coder_tool_parser.cpp | Implements whitespace escaping function and enhanced parsing logic for malformed tool calls |
| qwen3coder_tool_parser.hpp | Adds <function= tag to special parsing start tags set |
| qwen3coder_output_parser_test.cpp | Updates tests for escaped output and adds test for improper tag handling |
| spelling-whitelist.txt | Removes specific line reference for test file spelling issues |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
atobiszei
commented
Oct 14, 2025
atobiszei
commented
Oct 16, 2025
dtrawins
approved these changes
Oct 16, 2025
mzegla
reviewed
Oct 16, 2025
| this->lastProcessedPosition = pos + Qwen3CoderToolParser::TOOL_START_TAG.length(); | ||
| this->currentState = State::InsideToolCall; | ||
| this->toolCallPositions.begin.push(pos); | ||
| // normally we expect <tool_call> tag but we observerd that sometimes model generates <function=...> directly |
There was a problem hiding this comment.
Suggested change
| // normally we expect <tool_call> tag but we observerd that sometimes model generates <function=...> directly | |
| // normally we expect <tool_call> tag but we observed that sometimes model generates <function=...> directly |
| this->currentState = State::InsideToolCall; | ||
| this->toolCallPositions.begin.push(posTool); | ||
| } else { | ||
| // found <function=...> first, we will assume <tool_call> is missing and we will add it |
There was a problem hiding this comment.
"we will add it" - do you inject "<tool_call>" somewhere?
There was a problem hiding this comment.
misleading comment - i will remove it.
Comment on lines
+182
to
+187
| } else if (posTool < posFunc) { | ||
| // found <tool_call> first | ||
| this->lastProcessedPosition = posTool + Qwen3CoderToolParser::TOOL_START_TAG.length(); | ||
| this->currentState = State::InsideToolCall; | ||
| this->toolCallPositions.begin.push(posTool); | ||
| } else { |
There was a problem hiding this comment.
Does it cover well cases when posTool == std::string::npos or posFunc == std::string::npos?
There was a problem hiding this comment.
then posTool != posFunc so we hit either else if or else
mzegla
approved these changes
Oct 17, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Without escaping several multi turn scenarios in BFCL had problem parsing model response. While this escaping is shallow (does not differentiate at which level of JSON/array we are) it improves results, and we don't have for now examples of this being to eager with escaping.
Additionally:
-> treat improper begining of tool (
<function=...>) as an additional start of tool section. Several issues in BFCL were due to improper model output (lack of<tool_call>), but there were <function=...>Ticket:CVS-174650