Fix: Handle malformed reasoning blocks from x-ai/grok models (#9239) #9241
Conversation
- Add sanitization for x-ai model reasoning blocks to prevent interference with tool parsing - Remove XML-like tags and diff markers from reasoning content - Add validation to skip corrupted tool calls from x-ai models - Clean reasoning artifacts from diff content before processing - Handle interleaved thinking blocks in content stream This fixes issue #9239 where x-ai/grok-code-fast-1 model was causing 'Edit Unsuccessful' errors due to malformed reasoning blocks interfering with the diff parsing logic.
dosubot
bot
added
size:L
Review complete. Found 2 issues that should be addressed:
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
hannesrudolph
added
the
Issue/PR - Triage
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| text: delta.reasoning, | ||
| text: reasoningText, | ||
| } | ||
| isProcessingReasoning = false |
There was a problem hiding this comment.
The flag isProcessingReasoning is reset immediately after yielding reasoning content. If a delta contains both reasoning and tool_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.
| // 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>")) { |
There was a problem hiding this comment.
In the diff item processing, only <think> tags are sanitized. For consistency with the other modules, you might also want to check and remove <reasoning> tags if they appear in the diff content.
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| isProcessingReasoning = true | ||
| yield { | ||
| type: "reasoning", | ||
| text: delta.reasoning, | ||
| text: reasoningText, | ||
| } | ||
| isProcessingReasoning = false |
There was a problem hiding this comment.
The isProcessingReasoning flag is ineffective because it's set to true and then immediately back to false after yielding. This happens before the check on line 187 that's supposed to skip tool calls during reasoning processing. The flag will never be true when checked because it's reset synchronously. Consider removing the flag resets (lines 162 and 183) and only resetting when reasoning processing completes, or restructure the logic to properly track reasoning state across async yields.
Fix it with Roo Code or mention @roomote and request a fix.
| // 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, "") |
There was a problem hiding this comment.
Inconsistent orphaned tag handling compared to ApplyDiffTool.ts. This code only checks for orphaned </think> tags but not </reasoning> tags. The ApplyDiffTool handles both (lines 65-66). For consistency and completeness, this should also check for and remove orphaned </reasoning> tags using the same pattern.
Fix it with Roo Code or mention @roomote and request a fix.
Description
This PR attempts to address Issue #9239 where the x-ai/grok-code-fast-1 model through OpenRouter causes frequent "Edit Unsuccessful" errors and displays unusual thinking blocks.
Problem
The x-ai/grok models were sending malformed reasoning blocks that were interfering with tool parsing, particularly affecting the diff application tools. This led to:
Solution
This PR adds robust handling for x-ai model-specific quirks:
1. Reasoning Block Sanitization (src/api/providers/roo.ts)
2. Diff Content Cleaning (src/core/tools/MultiApplyDiffTool.ts & ApplyDiffTool.ts)
Changes
Testing
✅ All existing tests pass
✅ Tested with affected provider tests
Related Issue
Fixes #9239
Feedback and guidance are welcome!
Important
Fixes handling of malformed reasoning blocks from x-ai models by sanitizing and cleaning reasoning artifacts in
roo.ts,ApplyDiffTool.ts, andMultiApplyDiffTool.ts.RooHandlerinroo.tsto remove XML-like tags and diff markers from x-ai models.ApplyDiffTool.tsandMultiApplyDiffTool.ts.roo.ts: Detects x-ai models, sanitizes reasoning text, and handles corrupted tool calls.ApplyDiffTool.ts: Cleans reasoning artifacts from diff content and handles corrupted XML structure.MultiApplyDiffTool.ts: Parses XML for diffs, cleans reasoning artifacts, and handles batch diff operations.This description was created by
for 6f99b2e. You can customize this summary. It will automatically update as commits are pushed.