-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Fix: Handle malformed reasoning blocks from x-ai/grok models (#9239) #9241
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -128,6 +128,12 @@ export class RooHandler extends BaseOpenAiCompatibleProvider<string> { | |
| let lastUsage: RooUsage | undefined = undefined | ||
| // Accumulate tool calls by index - similar to how reasoning accumulates | ||
| const toolCallAccumulator = new Map<number, { id: string; name: string; arguments: string }>() | ||
| // Track if we're currently processing reasoning to prevent interference with tool parsing | ||
| let isProcessingReasoning = false | ||
|
|
||
| // Check if this is an x-ai model that might have malformed reasoning blocks | ||
| const modelId = this.options.apiModelId || "" | ||
| const isXAIModel = modelId.includes("x-ai/") || modelId.includes("grok") | ||
|
|
||
| for await (const chunk of stream) { | ||
| const delta = chunk.choices[0]?.delta | ||
|
|
@@ -136,46 +142,128 @@ export class RooHandler extends BaseOpenAiCompatibleProvider<string> { | |
| if (delta) { | ||
| // Check for reasoning content (similar to OpenRouter) | ||
| if ("reasoning" in delta && delta.reasoning && typeof delta.reasoning === "string") { | ||
| // For x-ai models, sanitize reasoning text to prevent XML-like content from interfering | ||
| let reasoningText = delta.reasoning | ||
| if (isXAIModel) { | ||
| // Remove any XML-like tags that might interfere with tool parsing | ||
| reasoningText = reasoningText | ||
| .replace(/<\/?apply_diff[^>]*>/g, "") | ||
| .replace(/<\/?SEARCH[^>]*>/g, "") | ||
| .replace(/<\/?REPLACE[^>]*>/g, "") | ||
| .replace(/<<<<<<< SEARCH/g, "[SEARCH]") | ||
| .replace(/=======/g, "[SEPARATOR]") | ||
| .replace(/>>>>>>> REPLACE/g, "[REPLACE]") | ||
| } | ||
| isProcessingReasoning = true | ||
| yield { | ||
| type: "reasoning", | ||
| text: delta.reasoning, | ||
| text: reasoningText, | ||
| } | ||
| isProcessingReasoning = false | ||
|
Comment on lines
+157
to
+162
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Fix it with Roo Code or mention @roomote and request a fix. |
||
| } | ||
|
|
||
| // Also check for reasoning_content for backward compatibility | ||
| if ("reasoning_content" in delta && typeof delta.reasoning_content === "string") { | ||
| // Apply same sanitization for x-ai models | ||
| let reasoningText = delta.reasoning_content | ||
| if (isXAIModel) { | ||
| reasoningText = reasoningText | ||
| .replace(/<\/?apply_diff[^>]*>/g, "") | ||
| .replace(/<\/?SEARCH[^>]*>/g, "") | ||
| .replace(/<\/?REPLACE[^>]*>/g, "") | ||
| .replace(/<<<<<<< SEARCH/g, "[SEARCH]") | ||
| .replace(/=======/g, "[SEPARATOR]") | ||
| .replace(/>>>>>>> REPLACE/g, "[REPLACE]") | ||
| } | ||
| isProcessingReasoning = true | ||
| yield { | ||
| type: "reasoning", | ||
| text: delta.reasoning_content, | ||
| text: reasoningText, | ||
| } | ||
| isProcessingReasoning = false | ||
| } | ||
|
|
||
| // Check for tool calls in delta | ||
| if ("tool_calls" in delta && Array.isArray(delta.tool_calls)) { | ||
| // Check for tool calls in delta - but skip if we're processing reasoning to avoid interference | ||
| if (!isProcessingReasoning && "tool_calls" in delta && Array.isArray(delta.tool_calls)) { | ||
| for (const toolCall of delta.tool_calls) { | ||
| const index = toolCall.index | ||
| const existing = toolCallAccumulator.get(index) | ||
|
|
||
| if (existing) { | ||
| // Accumulate arguments for existing tool call | ||
| if (toolCall.function?.arguments) { | ||
| existing.arguments += toolCall.function.arguments | ||
| // For x-ai models, validate the arguments don't contain reasoning artifacts | ||
| let args = toolCall.function.arguments | ||
| if (isXAIModel && args) { | ||
| // Check if the arguments contain reasoning block artifacts | ||
| if ( | ||
| args.includes("<think>") || | ||
| args.includes("</think>") || | ||
| args.includes("<reasoning>") || | ||
| args.includes("</reasoning>") | ||
| ) { | ||
| // Skip this chunk as it's likely corrupted reasoning content | ||
| console.warn( | ||
| "[RooHandler] Skipping corrupted tool call arguments from x-ai model", | ||
| { | ||
| modelId, | ||
| corruptedContent: args.substring(0, 100), | ||
| }, | ||
| ) | ||
| continue | ||
| } | ||
| } | ||
| existing.arguments += args | ||
| } | ||
| } else { | ||
| // Start new tool call accumulation | ||
| const toolName = toolCall.function?.name || "" | ||
| const toolArgs = toolCall.function?.arguments || "" | ||
|
|
||
| // Validate tool name isn't corrupted by reasoning content | ||
| if (isXAIModel && (toolName.includes("think") || toolName.includes("reasoning"))) { | ||
| console.warn("[RooHandler] Skipping corrupted tool call from x-ai model", { | ||
| modelId, | ||
| corruptedName: toolName, | ||
| }) | ||
| continue | ||
| } | ||
|
|
||
| toolCallAccumulator.set(index, { | ||
| id: toolCall.id || "", | ||
| name: toolCall.function?.name || "", | ||
| arguments: toolCall.function?.arguments || "", | ||
| name: toolName, | ||
| arguments: toolArgs, | ||
| }) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (delta.content) { | ||
| yield { | ||
| type: "text", | ||
| text: delta.content, | ||
| // For x-ai models, check if content contains interleaved reasoning markers | ||
| let textContent = delta.content | ||
| if (isXAIModel) { | ||
| // Check for common reasoning block markers that shouldn't be in regular content | ||
| if (textContent.includes("<think>") || textContent.includes("</think>")) { | ||
| // Extract and handle the reasoning part separately | ||
| const thinkMatch = textContent.match(/<think>(.*?)<\/think>/s) | ||
| if (thinkMatch) { | ||
| // Emit the reasoning part | ||
| yield { | ||
| type: "reasoning", | ||
| text: thinkMatch[1], | ||
| } | ||
| // Remove the thinking block from the text | ||
| textContent = textContent.replace(/<think>.*?<\/think>/s, "") | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Only yield text if there's content after cleaning | ||
| if (textContent.trim()) { | ||
| yield { | ||
| type: "text", | ||
| text: textContent, | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -127,10 +127,28 @@ export async function applyDiffTool( | |
| if (argsXmlTag) { | ||
| // Parse file entries from XML (new way) | ||
| try { | ||
| // Check if this might be from an x-ai model and contains reasoning artifacts | ||
| const isXAIModel = cline.api.getModel().id.includes("x-ai/") || cline.api.getModel().id.includes("grok") | ||
| let cleanedXml = argsXmlTag | ||
|
|
||
| if (isXAIModel) { | ||
| // Clean up common reasoning artifacts that might have leaked into the XML | ||
| // Remove thinking tags that might interfere with parsing | ||
| cleanedXml = cleanedXml | ||
| .replace(/<think>.*?<\/think>/gs, "") | ||
| .replace(/<reasoning>.*?<\/reasoning>/gs, "") | ||
|
|
||
| // Check for corrupted XML structure due to reasoning blocks | ||
| if (cleanedXml.includes("</think>") && !cleanedXml.includes("<think>")) { | ||
| console.warn("[MultiApplyDiffTool] Detected orphaned reasoning closing tag in x-ai model response") | ||
| cleanedXml = cleanedXml.replace(/<\/think>/g, "") | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent orphaned tag handling compared to Fix it with Roo Code or mention @roomote and request a fix. |
||
| } | ||
| } | ||
|
|
||
| // IMPORTANT: We use parseXmlForDiff here instead of parseXml to prevent HTML entity decoding | ||
| // This ensures exact character matching when comparing parsed content against original file content | ||
| // Without this, special characters like & would be decoded to & causing diff mismatches | ||
| const parsed = parseXmlForDiff(argsXmlTag, ["file.diff.content"]) as ParsedXmlResult | ||
| const parsed = parseXmlForDiff(cleanedXml, ["file.diff.content"]) as ParsedXmlResult | ||
| const files = Array.isArray(parsed.file) ? parsed.file : [parsed.file].filter(Boolean) | ||
|
|
||
| for (const file of files) { | ||
|
|
@@ -158,6 +176,18 @@ export async function applyDiffTool( | |
| diffContent = typeof diff.content === "string" ? diff.content : "" | ||
| startLine = diff.start_line ? parseInt(diff.start_line) : undefined | ||
|
|
||
| // For x-ai models, validate diff content doesn't contain reasoning artifacts | ||
| if (isXAIModel && diffContent) { | ||
| // Check for reasoning block markers that shouldn't be in diff content | ||
| if (diffContent.includes("<think>") || diffContent.includes("</think>")) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the diff item processing, only |
||
| console.warn( | ||
| "[MultiApplyDiffTool] Cleaning reasoning artifacts from x-ai model diff content", | ||
| ) | ||
| // Remove thinking blocks but preserve the actual diff markers | ||
| diffContent = diffContent.replace(/<think>.*?<\/think>/gs, "") | ||
| } | ||
| } | ||
|
|
||
| // Only add to operations if we have valid content | ||
| if (diffContent) { | ||
| operationsMap[filePath].diff.push({ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The flag
isProcessingReasoningis reset immediately after yielding reasoning content. If a delta contains both reasoning andtool_calls, this may allow tool calls to be processed even when part of corrupted reasoning. Consider retaining the flag for the entire processing of the chunk.