mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-11 22:51:26 +00:00
fix: preserve reasoning.text type in consolidateReasoningDetails for Gemini models
Fixes two bugs causing assistant content[] to be stripped on Turn 2 for google/gemini-3.1-pro-preview via OpenRouter: 1. consolidateReasoningDetails() used a shared type variable across all entries in a group. When reasoning.text and reasoning.encrypted entries shared the same index, the text entry type was overwritten to "reasoning.encrypted". Now tracks types separately for text vs encrypted entries. 2. The corrupted block filter dropped any reasoning.encrypted entry without a data field, including mislabeled text entries that had a text field. Now only drops entries that have neither data nor text. Closes #11629
This commit is contained in:
Roo Code
parent
ae09ee5a64
commit
89308c0454
2 changed files with 201 additions and 29 deletions
|
|
@ -1125,6 +1125,85 @@ describe("consolidateReasoningDetails", () => {
|
|||
expect(result).toHaveLength(1)
|
||||
expect(result[0].summary).toBe("Summary part 1Summary part 2")
|
||||
})
|
||||
|
||||
// Regression tests for https://github.com/RooCodeInc/Roo-Code/issues/11629
|
||||
|
||||
it("should preserve reasoning.text type when grouped with reasoning.encrypted at same index", () => {
|
||||
// This is the exact scenario from the bug report: Gemini returns both
|
||||
// reasoning.text and reasoning.encrypted at index 0. The text entry
|
||||
// must keep its original type and not be overwritten by the encrypted type.
|
||||
const details: ReasoningDetail[] = [
|
||||
{
|
||||
type: "reasoning.text",
|
||||
text: "**Initiating the Analysis**...",
|
||||
id: "rs_text_1",
|
||||
format: "google-gemini-v1",
|
||||
index: 0,
|
||||
},
|
||||
{
|
||||
type: "reasoning.encrypted",
|
||||
data: "EtwECtkEAb4+9vu...",
|
||||
id: "rs_enc_1",
|
||||
format: "google-gemini-v1",
|
||||
index: 0,
|
||||
},
|
||||
]
|
||||
|
||||
const result = consolidateReasoningDetails(details)
|
||||
|
||||
// Should have two entries: one text, one encrypted
|
||||
expect(result).toHaveLength(2)
|
||||
|
||||
const textEntry = result.find((r) => r.text)
|
||||
const encryptedEntry = result.find((r) => r.data)
|
||||
|
||||
// Text entry must retain type "reasoning.text", NOT "reasoning.encrypted"
|
||||
expect(textEntry?.type).toBe("reasoning.text")
|
||||
expect(textEntry?.text).toBe("**Initiating the Analysis**...")
|
||||
|
||||
// Encrypted entry should keep its correct type
|
||||
expect(encryptedEntry?.type).toBe("reasoning.encrypted")
|
||||
expect(encryptedEntry?.data).toBe("EtwECtkEAb4+9vu...")
|
||||
})
|
||||
|
||||
it("should preserve mislabeled text entries (type=reasoning.encrypted with text but no data)", () => {
|
||||
// If a text entry was previously stored with the wrong type (reasoning.encrypted)
|
||||
// but has a text field and no data field, it should NOT be dropped as corrupted.
|
||||
const details: ReasoningDetail[] = [
|
||||
{
|
||||
type: "reasoning.encrypted",
|
||||
text: "Some reasoning text",
|
||||
// No data field - previously this was dropped as "corrupted"
|
||||
id: "rs_mislabeled",
|
||||
format: "google-gemini-v1",
|
||||
index: 0,
|
||||
},
|
||||
]
|
||||
|
||||
const result = consolidateReasoningDetails(details)
|
||||
|
||||
// Should preserve the entry and fix its type to reasoning.text
|
||||
expect(result).toHaveLength(1)
|
||||
expect(result[0].text).toBe("Some reasoning text")
|
||||
expect(result[0].type).toBe("reasoning.text")
|
||||
})
|
||||
|
||||
it("should still drop truly corrupted encrypted blocks (no data AND no text)", () => {
|
||||
const details: ReasoningDetail[] = [
|
||||
{
|
||||
type: "reasoning.encrypted",
|
||||
// No data, no text - truly corrupted
|
||||
id: "rs_corrupted",
|
||||
format: "google-gemini-v1",
|
||||
index: 0,
|
||||
},
|
||||
]
|
||||
|
||||
const result = consolidateReasoningDetails(details)
|
||||
|
||||
// Truly corrupted block should still be dropped
|
||||
expect(result).toHaveLength(0)
|
||||
})
|
||||
})
|
||||
|
||||
describe("sanitizeGeminiMessages", () => {
|
||||
|
|
@ -1302,4 +1381,73 @@ describe("sanitizeGeminiMessages", () => {
|
|||
|
||||
expect(result).toEqual(messages)
|
||||
})
|
||||
|
||||
// Regression test for https://github.com/RooCodeInc/Roo-Code/issues/11629
|
||||
it("should preserve tool_calls when reasoning_details contain mislabeled text entries", () => {
|
||||
// This reproduces the exact bug: reasoning_details has a text entry mislabeled
|
||||
// as "reasoning.encrypted" (no data field) and a real encrypted entry.
|
||||
// Previously, the mislabeled text entry was dropped as "corrupted", and if
|
||||
// the encrypted entry's ID didn't match the tool call, all tool_calls were dropped.
|
||||
const messages = [
|
||||
{
|
||||
role: "assistant",
|
||||
content: "",
|
||||
tool_calls: [
|
||||
{
|
||||
id: "tool_attempt_completion_BI9Id6PH8KtbhYBnuB4I",
|
||||
type: "function",
|
||||
function: {
|
||||
name: "attempt_completion",
|
||||
arguments: '{"result":"Test received successfully."}',
|
||||
},
|
||||
},
|
||||
],
|
||||
reasoning_details: [
|
||||
{
|
||||
// Mislabeled: type says encrypted but has text, no data
|
||||
type: "reasoning.encrypted",
|
||||
text: "**Acknowledging the Input**\nI see this as a test message...",
|
||||
id: "tool_attempt_completion_BI9Id6PH8KtbhYBnuB4I",
|
||||
format: "google-gemini-v1",
|
||||
index: 0,
|
||||
},
|
||||
{
|
||||
type: "reasoning.encrypted",
|
||||
data: "EtwECtkEAb4+9vuFAKED...",
|
||||
id: "tool_attempt_completion_BI9Id6PH8KtbhYBnuB4I",
|
||||
format: "google-gemini-v1",
|
||||
index: 0,
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
role: "tool",
|
||||
tool_call_id: "tool_attempt_completion_BI9Id6PH8KtbhYBnuB4I",
|
||||
content: "The user is satisfied with the result.",
|
||||
},
|
||||
] as any
|
||||
|
||||
const result = sanitizeGeminiMessages(messages, "google/gemini-3.1-pro-preview")
|
||||
|
||||
// Assistant message should preserve tool_calls
|
||||
expect(result).toHaveLength(2)
|
||||
const assistantMsg = result[0] as any
|
||||
expect(assistantMsg.tool_calls).toBeDefined()
|
||||
expect(assistantMsg.tool_calls).toHaveLength(1)
|
||||
expect(assistantMsg.tool_calls[0].id).toBe("tool_attempt_completion_BI9Id6PH8KtbhYBnuB4I")
|
||||
|
||||
// reasoning_details should have both entries (text with corrected type + encrypted)
|
||||
expect(assistantMsg.reasoning_details).toBeDefined()
|
||||
expect(assistantMsg.reasoning_details.length).toBeGreaterThanOrEqual(1)
|
||||
|
||||
// The text entry should have its type corrected to reasoning.text
|
||||
const textDetail = assistantMsg.reasoning_details.find((d: any) => d.text)
|
||||
if (textDetail) {
|
||||
expect(textDetail.type).toBe("reasoning.text")
|
||||
}
|
||||
|
||||
// Tool result message should be preserved
|
||||
expect(result[1].role).toBe("tool")
|
||||
expect((result[1] as any).tool_call_id).toBe("tool_attempt_completion_BI9Id6PH8KtbhYBnuB4I")
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -50,7 +50,11 @@ export function consolidateReasoningDetails(reasoningDetails: ReasoningDetail[])
|
|||
// Drop corrupted encrypted reasoning blocks that would otherwise trigger:
|
||||
// "Invalid input: expected string, received undefined" for reasoning_details.*.data
|
||||
// See: https://github.com/cline/cline/issues/8214
|
||||
if (detail.type === "reasoning.encrypted" && !detail.data) {
|
||||
// Only drop if it truly has no usable content (no `data` AND no `text`).
|
||||
// A mislabeled entry with `type: "reasoning.encrypted"` but a valid `text`
|
||||
// field should be preserved (it's a text entry with incorrect type).
|
||||
// See: https://github.com/RooCodeInc/Roo-Code/issues/11629
|
||||
if (detail.type === "reasoning.encrypted" && !detail.data && !detail.text) {
|
||||
continue
|
||||
}
|
||||
|
||||
|
|
@ -65,46 +69,66 @@ export function consolidateReasoningDetails(reasoningDetails: ReasoningDetail[])
|
|||
const consolidated: ReasoningDetail[] = []
|
||||
|
||||
for (const [index, details] of groupedByIndex.entries()) {
|
||||
// Concatenate all text parts
|
||||
// Concatenate all text parts.
|
||||
// Track types separately for text/summary entries vs encrypted entries,
|
||||
// because a group can contain both a reasoning.text and a reasoning.encrypted
|
||||
// entry at the same index. Using a single shared `type` variable would cause
|
||||
// the last entry's type to overwrite earlier ones, mislabeling text entries
|
||||
// as "reasoning.encrypted".
|
||||
// See: https://github.com/RooCodeInc/Roo-Code/issues/11629
|
||||
let concatenatedText = ""
|
||||
let concatenatedSummary = ""
|
||||
let signature: string | undefined
|
||||
let id: string | undefined
|
||||
let format = "unknown"
|
||||
let type = "reasoning.text"
|
||||
let textSignature: string | undefined
|
||||
let textId: string | undefined
|
||||
let textFormat = "unknown"
|
||||
let textType = "reasoning.text"
|
||||
|
||||
for (const detail of details) {
|
||||
if (detail.text) {
|
||||
concatenatedText += detail.text
|
||||
// Track type/metadata from text-bearing entries only
|
||||
if (detail.type) {
|
||||
textType = detail.type === "reasoning.encrypted" ? "reasoning.text" : detail.type
|
||||
}
|
||||
if (detail.signature) {
|
||||
textSignature = detail.signature
|
||||
}
|
||||
if (detail.id) {
|
||||
textId = detail.id
|
||||
}
|
||||
if (detail.format) {
|
||||
textFormat = detail.format
|
||||
}
|
||||
}
|
||||
if (detail.summary) {
|
||||
concatenatedSummary += detail.summary
|
||||
}
|
||||
// Keep the signature from the last item that has one
|
||||
if (detail.signature) {
|
||||
signature = detail.signature
|
||||
}
|
||||
// Keep the id from the last item that has one
|
||||
if (detail.id) {
|
||||
id = detail.id
|
||||
}
|
||||
// Keep format and type from any item (they should all be the same)
|
||||
if (detail.format) {
|
||||
format = detail.format
|
||||
}
|
||||
if (detail.type) {
|
||||
type = detail.type
|
||||
// Use text entry metadata for summaries too, but don't overwrite
|
||||
// if we already got it from a text entry
|
||||
if (!concatenatedText) {
|
||||
if (detail.type) {
|
||||
textType = detail.type
|
||||
}
|
||||
if (detail.signature) {
|
||||
textSignature = detail.signature
|
||||
}
|
||||
if (detail.id) {
|
||||
textId = detail.id
|
||||
}
|
||||
if (detail.format) {
|
||||
textFormat = detail.format
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Create consolidated entry for text
|
||||
if (concatenatedText) {
|
||||
const consolidatedEntry: ReasoningDetail = {
|
||||
type: type,
|
||||
type: textType,
|
||||
text: concatenatedText,
|
||||
signature: signature ?? undefined,
|
||||
id: id ?? undefined,
|
||||
format: format,
|
||||
signature: textSignature ?? undefined,
|
||||
id: textId ?? undefined,
|
||||
format: textFormat,
|
||||
index: index,
|
||||
}
|
||||
consolidated.push(consolidatedEntry)
|
||||
|
|
@ -113,11 +137,11 @@ export function consolidateReasoningDetails(reasoningDetails: ReasoningDetail[])
|
|||
// Create consolidated entry for summary (used by some providers)
|
||||
if (concatenatedSummary && !concatenatedText) {
|
||||
const consolidatedEntry: ReasoningDetail = {
|
||||
type: type,
|
||||
type: textType,
|
||||
summary: concatenatedSummary,
|
||||
signature: signature ?? undefined,
|
||||
id: id ?? undefined,
|
||||
format: format,
|
||||
signature: textSignature ?? undefined,
|
||||
id: textId ?? undefined,
|
||||
format: textFormat,
|
||||
index: index,
|
||||
}
|
||||
consolidated.push(consolidatedEntry)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue