From ccbccb797a45bd8fd586ff7d87eabbf3fbf6f66d Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Thu, 30 Oct 2025 13:12:55 -0500 Subject: [PATCH 1/3] feat: add preserveReasoning flag to include reasoning in API history - Add optional preserveReasoning field to ModelInfo schema - Prepend reasoning in tags to assistant messages when enabled - Add comprehensive test coverage for reasoning preservation logic --- packages/types/src/model.ts | 1 + src/core/task/Task.ts | 9 +- .../__tests__/reasoning-preservation.test.ts | 328 ++++++++++++++++++ 3 files changed, 337 insertions(+), 1 deletion(-) create mode 100644 src/core/task/__tests__/reasoning-preservation.test.ts diff --git a/packages/types/src/model.ts b/packages/types/src/model.ts index 705f17039ec..993ac4e8b3d 100644 --- a/packages/types/src/model.ts +++ b/packages/types/src/model.ts @@ -68,6 +68,7 @@ export const modelInfoSchema = z.object({ requiredReasoningBudget: z.boolean().optional(), supportsReasoningEffort: z.boolean().optional(), requiredReasoningEffort: z.boolean().optional(), + preserveReasoning: z.boolean().optional(), supportedParameters: z.array(modelParametersSchema).optional(), inputPrice: z.number().optional(), outputPrice: z.number().optional(), diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 51799b742ec..24fbd0477af 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -2335,9 +2335,16 @@ export class Task extends EventEmitter implements TaskLike { }) } + // Check if we should preserve reasoning in the assistant message + let finalAssistantMessage = assistantMessage + if (reasoningMessage && this.api.getModel().info.preserveReasoning) { + // Prepend reasoning in XML tags to the assistant message so it's included in API history + finalAssistantMessage = `${reasoningMessage}\n${assistantMessage}` + } + await this.addToApiConversationHistory({ role: "assistant", - content: [{ type: "text", text: assistantMessage }], + content: [{ type: "text", text: finalAssistantMessage }], }) TelemetryService.instance.captureConversationMessage(this.taskId, "assistant") diff --git a/src/core/task/__tests__/reasoning-preservation.test.ts b/src/core/task/__tests__/reasoning-preservation.test.ts new file mode 100644 index 00000000000..52d8d99e734 --- /dev/null +++ b/src/core/task/__tests__/reasoning-preservation.test.ts @@ -0,0 +1,328 @@ +import { describe, it, expect, vi, beforeEach, beforeAll } from "vitest" +import type { ClineProvider } from "../../webview/ClineProvider" +import type { ProviderSettings, ModelInfo } from "@roo-code/types" + +// Mock vscode module before importing Task +vi.mock("vscode", () => ({ + workspace: { + createFileSystemWatcher: vi.fn(() => ({ + onDidCreate: vi.fn(), + onDidChange: vi.fn(), + onDidDelete: vi.fn(), + dispose: vi.fn(), + })), + getConfiguration: vi.fn(() => ({ + get: vi.fn(() => true), + })), + openTextDocument: vi.fn(), + applyEdit: vi.fn(), + }, + RelativePattern: vi.fn((base, pattern) => ({ base, pattern })), + window: { + createOutputChannel: vi.fn(() => ({ + appendLine: vi.fn(), + dispose: vi.fn(), + })), + createTextEditorDecorationType: vi.fn(() => ({ + dispose: vi.fn(), + })), + showTextDocument: vi.fn(), + activeTextEditor: undefined, + }, + Uri: { + file: vi.fn((path) => ({ fsPath: path })), + parse: vi.fn((str) => ({ toString: () => str })), + }, + Range: vi.fn(), + Position: vi.fn(), + WorkspaceEdit: vi.fn(() => ({ + replace: vi.fn(), + insert: vi.fn(), + delete: vi.fn(), + })), + ViewColumn: { + One: 1, + Two: 2, + Three: 3, + }, +})) + +// Mock other dependencies +vi.mock("../../services/mcp/McpServerManager", () => ({ + McpServerManager: { + getInstance: vi.fn().mockResolvedValue(null), + }, +})) + +vi.mock("../../integrations/terminal/TerminalRegistry", () => ({ + TerminalRegistry: { + releaseTerminalsForTask: vi.fn(), + }, +})) + +vi.mock("@roo-code/telemetry", () => ({ + TelemetryService: { + instance: { + captureTaskCreated: vi.fn(), + captureTaskRestarted: vi.fn(), + captureConversationMessage: vi.fn(), + captureLlmCompletion: vi.fn(), + captureConsecutiveMistakeError: vi.fn(), + }, + }, +})) + +describe("Task reasoning preservation", () => { + let mockProvider: Partial + let mockApiConfiguration: ProviderSettings + let Task: any + + beforeAll(async () => { + // Import Task after mocks are set up + const taskModule = await import("../Task") + Task = taskModule.Task + }) + + beforeEach(() => { + // Mock provider with necessary methods + mockProvider = { + postStateToWebview: vi.fn().mockResolvedValue(undefined), + getState: vi.fn().mockResolvedValue({ + mode: "code", + experiments: {}, + }), + context: { + globalStorageUri: { fsPath: "/test/storage" }, + extensionPath: "/test/extension", + } as any, + log: vi.fn(), + updateTaskHistory: vi.fn().mockResolvedValue(undefined), + postMessageToWebview: vi.fn().mockResolvedValue(undefined), + } + + mockApiConfiguration = { + apiProvider: "anthropic", + apiKey: "test-key", + } as ProviderSettings + }) + + it("should append reasoning to assistant message when preserveReasoning is true", async () => { + // Create a task instance + const task = new Task({ + provider: mockProvider as ClineProvider, + apiConfiguration: mockApiConfiguration, + task: "Test task", + startTask: false, + }) + + // Mock the API to return a model with preserveReasoning enabled + const mockModelInfo: ModelInfo = { + contextWindow: 16000, + supportsPromptCache: true, + preserveReasoning: true, + } + + task.api = { + getModel: vi.fn().mockReturnValue({ + id: "test-model", + info: mockModelInfo, + }), + } + + // Mock the API conversation history + task.apiConversationHistory = [] + + // Simulate adding an assistant message with reasoning + const assistantMessage = "Here is my response to your question." + const reasoningMessage = "Let me think about this step by step. First, I need to..." + + // Spy on addToApiConversationHistory + const addToApiHistorySpy = vi.spyOn(task as any, "addToApiConversationHistory") + + // Simulate what happens in the streaming loop when preserveReasoning is true + let finalAssistantMessage = assistantMessage + if (reasoningMessage && task.api.getModel().info.preserveReasoning) { + finalAssistantMessage = `${reasoningMessage}\n${assistantMessage}` + } + + await (task as any).addToApiConversationHistory({ + role: "assistant", + content: [{ type: "text", text: finalAssistantMessage }], + }) + + // Verify that reasoning was prepended in tags to the assistant message + expect(addToApiHistorySpy).toHaveBeenCalledWith({ + role: "assistant", + content: [ + { + type: "text", + text: "Let me think about this step by step. First, I need to...\nHere is my response to your question.", + }, + ], + }) + + // Verify the API conversation history contains the message with reasoning + expect(task.apiConversationHistory).toHaveLength(1) + expect(task.apiConversationHistory[0].content[0].text).toContain("") + expect(task.apiConversationHistory[0].content[0].text).toContain("") + expect(task.apiConversationHistory[0].content[0].text).toContain("Here is my response to your question.") + expect(task.apiConversationHistory[0].content[0].text).toContain( + "Let me think about this step by step. First, I need to...", + ) + }) + + it("should NOT append reasoning to assistant message when preserveReasoning is false", async () => { + // Create a task instance + const task = new Task({ + provider: mockProvider as ClineProvider, + apiConfiguration: mockApiConfiguration, + task: "Test task", + startTask: false, + }) + + // Mock the API to return a model with preserveReasoning disabled (or undefined) + const mockModelInfo: ModelInfo = { + contextWindow: 16000, + supportsPromptCache: true, + preserveReasoning: false, + } + + task.api = { + getModel: vi.fn().mockReturnValue({ + id: "test-model", + info: mockModelInfo, + }), + } + + // Mock the API conversation history + task.apiConversationHistory = [] + + // Simulate adding an assistant message with reasoning + const assistantMessage = "Here is my response to your question." + const reasoningMessage = "Let me think about this step by step. First, I need to..." + + // Spy on addToApiConversationHistory + const addToApiHistorySpy = vi.spyOn(task as any, "addToApiConversationHistory") + + // Simulate what happens in the streaming loop when preserveReasoning is false + let finalAssistantMessage = assistantMessage + if (reasoningMessage && task.api.getModel().info.preserveReasoning) { + finalAssistantMessage = `${reasoningMessage}\n${assistantMessage}` + } + + await (task as any).addToApiConversationHistory({ + role: "assistant", + content: [{ type: "text", text: finalAssistantMessage }], + }) + + // Verify that reasoning was NOT appended to the assistant message + expect(addToApiHistorySpy).toHaveBeenCalledWith({ + role: "assistant", + content: [{ type: "text", text: "Here is my response to your question." }], + }) + + // Verify the API conversation history does NOT contain reasoning + expect(task.apiConversationHistory).toHaveLength(1) + expect(task.apiConversationHistory[0].content[0].text).toBe("Here is my response to your question.") + expect(task.apiConversationHistory[0].content[0].text).not.toContain("") + }) + + it("should handle empty reasoning message gracefully when preserveReasoning is true", async () => { + // Create a task instance + const task = new Task({ + provider: mockProvider as ClineProvider, + apiConfiguration: mockApiConfiguration, + task: "Test task", + startTask: false, + }) + + // Mock the API to return a model with preserveReasoning enabled + const mockModelInfo: ModelInfo = { + contextWindow: 16000, + supportsPromptCache: true, + preserveReasoning: true, + } + + task.api = { + getModel: vi.fn().mockReturnValue({ + id: "test-model", + info: mockModelInfo, + }), + } + + // Mock the API conversation history + task.apiConversationHistory = [] + + const assistantMessage = "Here is my response." + const reasoningMessage = "" // Empty reasoning + + // Spy on addToApiConversationHistory + const addToApiHistorySpy = vi.spyOn(task as any, "addToApiConversationHistory") + + // Simulate what happens in the streaming loop + let finalAssistantMessage = assistantMessage + if (reasoningMessage && task.api.getModel().info.preserveReasoning) { + finalAssistantMessage = `${reasoningMessage}\n${assistantMessage}` + } + + await (task as any).addToApiConversationHistory({ + role: "assistant", + content: [{ type: "text", text: finalAssistantMessage }], + }) + + // Verify that no reasoning tags were added when reasoning is empty + expect(addToApiHistorySpy).toHaveBeenCalledWith({ + role: "assistant", + content: [{ type: "text", text: "Here is my response." }], + }) + + // Verify the message doesn't contain reasoning tags + expect(task.apiConversationHistory[0].content[0].text).toBe("Here is my response.") + expect(task.apiConversationHistory[0].content[0].text).not.toContain("") + }) + + it("should handle undefined preserveReasoning (defaults to false)", async () => { + // Create a task instance + const task = new Task({ + provider: mockProvider as ClineProvider, + apiConfiguration: mockApiConfiguration, + task: "Test task", + startTask: false, + }) + + // Mock the API to return a model without preserveReasoning field (undefined) + const mockModelInfo: ModelInfo = { + contextWindow: 16000, + supportsPromptCache: true, + // preserveReasoning is undefined + } + + task.api = { + getModel: vi.fn().mockReturnValue({ + id: "test-model", + info: mockModelInfo, + }), + } + + // Mock the API conversation history + task.apiConversationHistory = [] + + const assistantMessage = "Here is my response." + const reasoningMessage = "Some reasoning here." + + // Simulate what happens in the streaming loop + let finalAssistantMessage = assistantMessage + if (reasoningMessage && task.api.getModel().info.preserveReasoning) { + finalAssistantMessage = `${reasoningMessage}\n${assistantMessage}` + } + + await (task as any).addToApiConversationHistory({ + role: "assistant", + content: [{ type: "text", text: finalAssistantMessage }], + }) + + // Verify reasoning was NOT prepended (undefined defaults to false) + expect(task.apiConversationHistory[0].content[0].text).toBe("Here is my response.") + expect(task.apiConversationHistory[0].content[0].text).not.toContain("") + }) +}) From d4fc5afccb2121fb32661fea40e21248b891bbc2 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Thu, 30 Oct 2025 13:39:57 -0500 Subject: [PATCH 2/3] feat: add preserveReasoning property to MiniMax model configuration --- packages/types/src/providers/minimax.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/types/src/providers/minimax.ts b/packages/types/src/providers/minimax.ts index 825431b4824..47362e01bff 100644 --- a/packages/types/src/providers/minimax.ts +++ b/packages/types/src/providers/minimax.ts @@ -16,6 +16,7 @@ export const minimaxModels = { outputPrice: 1.2, cacheWritesPrice: 0, cacheReadsPrice: 0, + preserveReasoning: true, description: "MiniMax M2, a model born for Agents and code, featuring Top-tier Coding Capabilities, Powerful Agentic Performance, and Ultimate Cost-Effectiveness & Speed.", }, From 618d1070fc02235925569bf76b0b4517c6991891 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Mon, 3 Nov 2025 10:11:01 -0500 Subject: [PATCH 3/3] chore: change reasoning tag from thinking to think --- src/core/task/Task.ts | 2 +- .../__tests__/reasoning-preservation.test.ts | 22 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 24fbd0477af..0d13d740f65 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -2339,7 +2339,7 @@ export class Task extends EventEmitter implements TaskLike { let finalAssistantMessage = assistantMessage if (reasoningMessage && this.api.getModel().info.preserveReasoning) { // Prepend reasoning in XML tags to the assistant message so it's included in API history - finalAssistantMessage = `${reasoningMessage}\n${assistantMessage}` + finalAssistantMessage = `${reasoningMessage}\n${assistantMessage}` } await this.addToApiConversationHistory({ diff --git a/src/core/task/__tests__/reasoning-preservation.test.ts b/src/core/task/__tests__/reasoning-preservation.test.ts index 52d8d99e734..28d25c9a8e9 100644 --- a/src/core/task/__tests__/reasoning-preservation.test.ts +++ b/src/core/task/__tests__/reasoning-preservation.test.ts @@ -142,7 +142,7 @@ describe("Task reasoning preservation", () => { // Simulate what happens in the streaming loop when preserveReasoning is true let finalAssistantMessage = assistantMessage if (reasoningMessage && task.api.getModel().info.preserveReasoning) { - finalAssistantMessage = `${reasoningMessage}\n${assistantMessage}` + finalAssistantMessage = `${reasoningMessage}\n${assistantMessage}` } await (task as any).addToApiConversationHistory({ @@ -150,21 +150,21 @@ describe("Task reasoning preservation", () => { content: [{ type: "text", text: finalAssistantMessage }], }) - // Verify that reasoning was prepended in tags to the assistant message + // Verify that reasoning was prepended in tags to the assistant message expect(addToApiHistorySpy).toHaveBeenCalledWith({ role: "assistant", content: [ { type: "text", - text: "Let me think about this step by step. First, I need to...\nHere is my response to your question.", + text: "Let me think about this step by step. First, I need to...\nHere is my response to your question.", }, ], }) // Verify the API conversation history contains the message with reasoning expect(task.apiConversationHistory).toHaveLength(1) - expect(task.apiConversationHistory[0].content[0].text).toContain("") - expect(task.apiConversationHistory[0].content[0].text).toContain("") + expect(task.apiConversationHistory[0].content[0].text).toContain("") + expect(task.apiConversationHistory[0].content[0].text).toContain("") expect(task.apiConversationHistory[0].content[0].text).toContain("Here is my response to your question.") expect(task.apiConversationHistory[0].content[0].text).toContain( "Let me think about this step by step. First, I need to...", @@ -207,7 +207,7 @@ describe("Task reasoning preservation", () => { // Simulate what happens in the streaming loop when preserveReasoning is false let finalAssistantMessage = assistantMessage if (reasoningMessage && task.api.getModel().info.preserveReasoning) { - finalAssistantMessage = `${reasoningMessage}\n${assistantMessage}` + finalAssistantMessage = `${reasoningMessage}\n${assistantMessage}` } await (task as any).addToApiConversationHistory({ @@ -224,7 +224,7 @@ describe("Task reasoning preservation", () => { // Verify the API conversation history does NOT contain reasoning expect(task.apiConversationHistory).toHaveLength(1) expect(task.apiConversationHistory[0].content[0].text).toBe("Here is my response to your question.") - expect(task.apiConversationHistory[0].content[0].text).not.toContain("") + expect(task.apiConversationHistory[0].content[0].text).not.toContain("") }) it("should handle empty reasoning message gracefully when preserveReasoning is true", async () => { @@ -262,7 +262,7 @@ describe("Task reasoning preservation", () => { // Simulate what happens in the streaming loop let finalAssistantMessage = assistantMessage if (reasoningMessage && task.api.getModel().info.preserveReasoning) { - finalAssistantMessage = `${reasoningMessage}\n${assistantMessage}` + finalAssistantMessage = `${reasoningMessage}\n${assistantMessage}` } await (task as any).addToApiConversationHistory({ @@ -278,7 +278,7 @@ describe("Task reasoning preservation", () => { // Verify the message doesn't contain reasoning tags expect(task.apiConversationHistory[0].content[0].text).toBe("Here is my response.") - expect(task.apiConversationHistory[0].content[0].text).not.toContain("") + expect(task.apiConversationHistory[0].content[0].text).not.toContain("") }) it("should handle undefined preserveReasoning (defaults to false)", async () => { @@ -313,7 +313,7 @@ describe("Task reasoning preservation", () => { // Simulate what happens in the streaming loop let finalAssistantMessage = assistantMessage if (reasoningMessage && task.api.getModel().info.preserveReasoning) { - finalAssistantMessage = `${reasoningMessage}\n${assistantMessage}` + finalAssistantMessage = `${reasoningMessage}\n${assistantMessage}` } await (task as any).addToApiConversationHistory({ @@ -323,6 +323,6 @@ describe("Task reasoning preservation", () => { // Verify reasoning was NOT prepended (undefined defaults to false) expect(task.apiConversationHistory[0].content[0].text).toBe("Here is my response.") - expect(task.apiConversationHistory[0].content[0].text).not.toContain("") + expect(task.apiConversationHistory[0].content[0].text).not.toContain("") }) })