refactor: remove legacy message transformation pipeline#11451
Open
roomote[bot] wants to merge 12 commits intomainfrom
Open
refactor: remove legacy message transformation pipeline#11451roomote[bot] wants to merge 12 commits intomainfrom
roomote[bot] wants to merge 12 commits intomainfrom
Conversation
roomote
bot
requested review from
cte,
daniel-lxs,
hannesrudolph,
jr and
mrubens
as code owners
dosubot
bot
added
the
size:XXL
Contributor
Author
Reviewed 0bd02ae. The error-wrapping removal is clean -- providers now throw raw AI SDK errors, and
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
Contributor
Author
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| this.apiConversationHistory.push({ | ||
| ...message, | ||
| ...(responseId ? { id: responseId } : {}), | ||
| ts: message.ts ?? Date.now(), |
Contributor
Author
There was a problem hiding this comment.
Nit: with both responseId spreads removed, const responseId = handler.getResponseId?.() (line 1072) is now unused, and the getResponseId entry in the handler type cast (line 1067) is also dead. The comment on line 1092 ("with responseId and timestamp") is stale too. Minor cleanup left behind by this refactoring.
Fix it with Roo Code or mention @roomote and request a fix.
- Remove buildCleanConversationHistory from Task.ts (effectively a no-op since all providers are AI SDK providers and preserveReasoning is always true) - Standardize all providers on sanitizeMessagesForProvider for message sanitization (xai, gemini, bedrock, openai-native, openai-codex) - Delete dead transform files with zero production callers: anthropic-filter.ts, r1-format.ts, openai-format.ts, mistral-format.ts - Remove convertToAiSdkMessages function and its unused imports from ~20 providers (imported by all, called by none) - Remove reasoning-preservation.test.ts (tested removed code) - Clean up stale comments referencing removed functions - Backward compatibility with old conversation histories is already handled by convertAnthropicToRooMessages at the persistence layer
…derOptions on save
…ng messages Remove the provider response ID (uid=502(hrudolph) gid=20(staff) groups=20(staff),101(access_bpf),12(everyone),61(localaccounts),79(_appserverusr),80(admin),81(_appserveradm),98(_lpadmin),333(piavpn),33(_appstore),100(_lpoperator),204(_developer),250(_analyticsusers),395(com.apple.access_ftp),398(com.apple.access_screensharing),399(com.apple.access_ssh),400(com.apple.access_remote_ae)) field from RooAssistantMessage type and stop propagating it through anthropicToRoo converter and Task.ts. Update corresponding tests to no longer assert on the removed field.
… for stripping test
Parse error.metadata.raw (OpenRouter/provider wrapper format) as the
primary error source. Remove silent fallback to generic statusText
('Bad Request'). When structured extraction fails, surface the raw
responseBody instead of swallowing it. Add 17 tests covering all
error format paths.
…null When NativeToolCallParser.parseToolCall() returns null in the 'tool_call' handler, push a fallback ToolUse block to assistantMessageContent instead of silently breaking. This ensures the didToolUse check at the end of the request sees a tool_use block, preventing the false '[ERROR] You did not use a tool' injection. The fallback block flows into presentAssistantMessage()'s existing unknown-tool handler which reports the actual error to the model. Add 4 tests covering the fallback behavior.
…main, needed by this PR)
hannesrudolph
force-pushed
the
feature/remove-legacy-message-transforms
branch
from
b04c931 to
21172d5
Compare
…oints to 1 - Move applyCacheBreakpoints() from Task.ts into each provider's createMessage() (anthropic, bedrock, openrouter, requesty, anthropic-vertex, roo) - Aligns with AI SDK best practice: cache control is a call-site concern - Remove stripCacheProviderOptions() from saveRooMessages() save path (no longer needed since cache markers don't contaminate persistent history) - Change maxBreakpoints default from 2 to 1 (3-point strategy: 1 system, 1 tool, 1 message)
…ssages - Remove handleAiSdkError() wrapping from all 24 provider files — typed AI SDK errors (APICallError, RetryError, TooManyRequestsError) now flow through directly - Update Task.ts to use APICallError.isInstance/RetryError.isInstance for typed error handling, extractAiSdkErrorMessage() for user display - Add findDeepestApiCallError() for recursive .cause/.lastError/.errors[] traversal - Fix processAiSdkStreamPart error case to use extractAiSdkErrorMessage instead of flattening to error.message (was losing responseBody with actual error details) - Fix extractMessageFromResponseBody to handle OpenRouter metadata.raw with nested Anthropic error format (rawObj.error.message) - Add checkIsAiSdkContextWindowError() for structured context window detection - Update backoffAndAnnounce() to read .statusCode from APICallError natively - Fix 25 provider error tests, add 8 new error extraction tests - Remove dead handleOpenAIError export from error-handler.ts - Add lastStreamError recovery to OpenRouter and native-ollama providers
dosubot
bot
added
size:XL
Comment on lines
+4589
to
4647
| const statusCode = APICallError.isInstance(error) | ||
| ? error.statusCode | ||
| : RetryError.isInstance(error) && APICallError.isInstance(error.lastError) | ||
| ? error.lastError.statusCode | ||
| : (error as any)?.status | ||
|
|
||
| // Prefer RetryInfo on 429 if present | ||
| if (statusCode === 429) { | ||
| // Try direct errorDetails (legacy Vertex), then try parsing from responseBody | ||
| let retryDelaySec: number | undefined | ||
| const errorDetails = (error as any)?.errorDetails | ||
| if (errorDetails) { | ||
| const retryInfo = errorDetails.find( | ||
| (d: any) => d["@type"] === "type.googleapis.com/google.rpc.RetryInfo", | ||
| ) | ||
| const match = retryInfo?.retryDelay?.match?.(/^(\d+)s$/) | ||
| if (match) { | ||
| retryDelaySec = Number(match[1]) + 1 | ||
| } | ||
| } | ||
| // Also try extracting from APICallError responseBody for Vertex errors | ||
| if (!retryDelaySec) { | ||
| const responseBody = APICallError.isInstance(error) | ||
| ? error.responseBody | ||
| : RetryError.isInstance(error) && APICallError.isInstance(error.lastError) | ||
| ? error.lastError.responseBody | ||
| : undefined | ||
| if (responseBody) { | ||
| try { | ||
| const parsed = JSON.parse(responseBody) | ||
| const retryInfo = parsed?.error?.details?.find( | ||
| (d: any) => d["@type"] === "type.googleapis.com/google.rpc.RetryInfo", | ||
| ) | ||
| const match = retryInfo?.retryDelay?.match?.(/^(\d+)s$/) | ||
| if (match) { | ||
| retryDelaySec = Number(match[1]) + 1 | ||
| } | ||
| } catch { | ||
| // responseBody not parseable, skip | ||
| } | ||
| } | ||
| } | ||
| if (retryDelaySec) { | ||
| exponentialDelay = retryDelaySec | ||
| } | ||
| } | ||
|
|
||
| const finalDelay = Math.max(exponentialDelay, rateLimitDelay) | ||
| if (finalDelay <= 0) { | ||
| return | ||
| } | ||
|
|
||
| // Build header text; fall back to error message if none provided | ||
| let headerText: string | ||
| if (statusCode) { | ||
| headerText = `${statusCode}\n${extractAiSdkErrorMessage(error)}` | ||
| } else { | ||
| headerText = extractAiSdkErrorMessage(error) | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Nit: This new block (lines 4589-4647) is indented one tab level deeper than the surrounding code in backoffAndAnnounce. It looks like the replacement was pasted at the wrong indent depth. The logic is correct but ~60 lines sitting at 4 tabs instead of 3 makes the method harder to scan.
Fix it with Roo Code or mention @roomote and request a fix.
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
2 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.
Summary
Removes the legacy message transformation pipeline that predates the AI SDK migration. The AI SDK now handles all provider-specific message format conversion natively.
What changed
Removed
buildCleanConversationHistoryfrom Task.tsThis method was effectively a no-op because:
preserveReasoningwas alwaystrue(all real providers returntruefromisAiSdkProvider())convertAnthropicToRooMessages()Standardized all providers on
sanitizeMessagesForProvider()5 providers had ad-hoc or missing message sanitization:
All now use
sanitizeMessagesForProvider()which allowlistsrole/content/providerOptionsand filtersRooReasoningMessageitems.Deleted dead transform files (zero production callers)
anthropic-filter.ts-filterNonAnthropicBlocks()r1-format.ts-convertToR1Format()openai-format.ts-convertToOpenAiMessages(),sanitizeGeminiMessages(),consolidateReasoningDetails()mistral-format.ts-convertToMistralMessages(),normalizeMistralToolCallId()Removed
convertToAiSdkMessages()from ai-sdk.tsImported by ~20 providers, called by zero. All providers use
sanitizeMessagesForProvider()instead.Removed unused imports
Cleaned up
convertToAiSdkMessagesimports from all 25 provider files.Stats
Testing