Skip to content

Conversation

@roomote
Copy link
Contributor

@roomote roomote bot commented Aug 3, 2025

This PR fixes the issue where Qwen3 Coder model through OpenRouter sends thinking and tool call blocks as raw XML text instead of being properly parsed and displayed in their respective UI panes.

Problem

When using the qwen/qwen3-coder:free model through OpenRouter, the response contains <think>...</think> and <tool_call>...</tool_call> blocks as raw text in the content, which are displayed directly to the user instead of being parsed and shown in the appropriate UI sections.

Solution

  • Added XML parsing logic to the OpenRouter handler to detect and parse these XML blocks
  • Implemented a buffering mechanism to handle incomplete XML blocks across streaming chunks
  • Convert <think> blocks to reasoning chunks that display in the thinking pane
  • Convert <tool_call> blocks to a user-friendly format with [Tool Call]: prefix
  • Added comprehensive tests to ensure the parsing works correctly in various scenarios

Testing

  • Added 4 new test cases covering:
    • Basic <think> block parsing
    • Basic <tool_call> block parsing
    • Multiple and nested XML blocks
    • Incomplete XML blocks split across streaming chunks
  • All existing tests continue to pass

Fixes #6630


Important

Fixes XML parsing for <think> and <tool_call> blocks in OpenRouter responses, adding tests for various scenarios.

  • Behavior:
    • Parses <think> and <tool_call> XML blocks in OpenRouterHandler in openrouter.ts.
    • Converts <think> blocks to reasoning chunks and <tool_call> blocks to text with [Tool Call]: prefix.
    • Handles incomplete XML blocks across streaming chunks.
  • Testing:
    • Adds tests in openrouter.spec.ts for parsing <think> and <tool_call> blocks, nested/multiple XML blocks, and incomplete XML blocks.
    • Ensures all existing tests pass.

This description was created by Ellipsis for aa8cb2f. You can customize this summary. It will automatically update as commits are pushed.

- Add XML parsing for <think> and <tool_call> blocks in OpenRouter handler
- Handle incomplete XML blocks across streaming chunks
- Convert tool_call blocks to user-friendly format
- Add comprehensive tests for XML parsing functionality

Fixes #6630
@roomote roomote bot requested review from cte, jr and mrubens as code owners August 3, 2025 21:05
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Aug 3, 2025
import { addCacheBreakpoints as addGeminiCacheBreakpoints } from "../transform/caching/gemini"
import type { OpenRouterReasoningParams } from "../transform/reasoning"
import { getModelParams } from "../transform/model-params"
import { XmlMatcher } from "../../utils/xml-matcher"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove unused import 'XmlMatcher' if it's not needed to avoid confusion.

Suggested change
import { XmlMatcher } from "../../utils/xml-matcher"

This comment was generated because it violated a code review rule: irule_Vw7dJWzvznOJagxS.

yield { type: "text", text: delta.content }
buffer += delta.content

// Process complete XML blocks
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider refactoring the inline XML parsing logic (lines 158–216) into a shared utility (or use the imported XmlMatcher) for improved readability and maintainability.

This comment was generated because it violated a code review rule: irule_tTqpIuNs8DV0QFGj.

Copy link
Contributor Author

@roomote roomote bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote XML parsing code when XmlMatcher was right there. Peak efficiency.

import { addCacheBreakpoints as addGeminiCacheBreakpoints } from "../transform/caching/gemini"
import type { OpenRouterReasoningParams } from "../transform/reasoning"
import { getModelParams } from "../transform/model-params"
import { XmlMatcher } from "../../utils/xml-matcher"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I notice we're importing XmlMatcher but not using it. Since we've implemented custom XML parsing logic below, should we remove this unused import? Or perhaps we could consider using the existing XmlMatcher utility instead of the manual implementation?

processed = false

// Check for complete <think> blocks
const thinkMatch = buffer.match(/^(.*?)<think>([\s\S]*?)<\/think>(.*)$/s)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regex patterns assume well-formed XML. What happens if the model sends malformed XML like <think>content without closing tag or nested tags? The current implementation might not handle these edge cases gracefully. Should we add some validation or fallback behavior?


// Process complete XML blocks
let processed = true
while (processed) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For large responses with many XML blocks, this while loop with multiple regex matches could impact performance. Have we considered the performance implications? Perhaps we could optimize by combining the regex patterns or using a different parsing approach?

}

// Check for complete <tool_call> blocks
const toolMatch = buffer.match(/^(.*?)<tool_call>([\s\S]*?)<\/tool_call>(.*)$/s)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic for handling <think> and <tool_call> blocks is nearly identical. Could we refactor this into a helper function to reduce duplication? Something like:

Suggested change
const toolMatch = buffer.match(/^(.*?)<tool_call>([\s\S]*?)<\/tool_call>(.*)$/s)
// Helper function to process XML blocks
const processXmlBlock = (buffer: string, tagName: string, transform?: (content: string) => any) => {
const regex = new RegExp(`^(.*?)<${tagName}>([\s\S]*?)<\/${tagName}>(.*)$`, 's');
const match = buffer.match(regex);
if (match) {
const [, before, content, after] = match;
return { matched: true, before, content: transform ? transform(content) : content, after };
}
return { matched: false };
};

}

// Check if we have an incomplete tag at the end
const incompleteTag = buffer.match(/^(.*?)(<(?:think|tool_call)[^>]*(?:>[\s\S]*)?)?$/s)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This buffer management logic for incomplete tags is complex. Could we add some inline comments explaining the different scenarios? For example:

  • When we have an incomplete tag at the end
  • When we need to preserve partial tag content
  • When it's safe to yield all content

This would improve maintainability for future developers (including myself in 5 minutes).

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Aug 3, 2025
@vertigo235
Copy link

Isn't this also an issue when using ollama?

@daniel-lxs
Copy link
Member

This is not a proper fix, the model is outputting the tag <think> and hallucinating the tag <tool_call/>, closing as this is an issue with the model itself

@daniel-lxs daniel-lxs closed this Aug 4, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Aug 4, 2025
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Aug 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Qwen3 Coder diplays raw text for thinking and tool calls when using OpenRouter

5 participants