mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-06 08:18:39 +00:00
fix: preserve GPT-5 conversation continuity after subtask completion
- Modified completeSubtask to check if the model is GPT-5 before setting skipPrevResponseIdOnce - GPT-5 models now maintain previous_response_id chain after subtask completion - Non-GPT-5 models continue to skip previous_response_id as before - Added comprehensive tests for both GPT-5 and non-GPT-5 behavior Fixes #8402
This commit is contained in:
parent
702b269a1b
commit
f9a8ccd9f8
4 changed files with 139 additions and 3 deletions
1
.review/pr-8274
Submodule
1
.review/pr-8274
Submodule
|
|
@ -0,0 +1 @@
|
|||
Subproject commit e46929b8d8add0cd3c412d69f8ac882c405a4ba9
|
||||
|
|
@ -1666,9 +1666,20 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
content: [{ type: "text", text: `[new_task completed] Result: ${lastMessage}` }],
|
||||
})
|
||||
|
||||
// Set skipPrevResponseIdOnce to ensure the next API call sends the full conversation
|
||||
// including the subtask result, not just from before the subtask was created
|
||||
this.skipPrevResponseIdOnce = true
|
||||
// For GPT-5 models, we should NOT skip the previous_response_id after subtask completion
|
||||
// because GPT-5 relies on response continuity to maintain context. When we skip it,
|
||||
// GPT-5 only receives the latest message without the subtask result context.
|
||||
// For other models, we skip to ensure the full conversation is sent.
|
||||
const modelId = this.api.getModel().id
|
||||
const isGpt5Model = modelId && modelId.startsWith("gpt-5")
|
||||
|
||||
if (!isGpt5Model) {
|
||||
// Only skip previous_response_id for non-GPT-5 models
|
||||
// This ensures the next API call sends the full conversation
|
||||
// including the subtask result, not just from before the subtask was created
|
||||
this.skipPrevResponseIdOnce = true
|
||||
}
|
||||
// For GPT-5 models, we maintain continuity by keeping the previous_response_id chain intact
|
||||
} catch (error) {
|
||||
this.providerRef
|
||||
.deref()
|
||||
|
|
|
|||
|
|
@ -1776,4 +1776,127 @@ describe("Cline", () => {
|
|||
consoleErrorSpy.mockRestore()
|
||||
})
|
||||
})
|
||||
|
||||
describe("GPT-5 Subtask Completion", () => {
|
||||
it("should NOT skip previous_response_id for GPT-5 models after subtask completion", async () => {
|
||||
// Create a mock GPT-5 API configuration
|
||||
const gpt5ApiConfig = {
|
||||
apiProvider: "openai-native" as const,
|
||||
apiModelId: "gpt-5-codex",
|
||||
openAiNativeApiKey: "test-key",
|
||||
}
|
||||
|
||||
// Create parent task with GPT-5 model
|
||||
const parentTask = new Task({
|
||||
provider: mockProvider,
|
||||
apiConfiguration: gpt5ApiConfig,
|
||||
task: "parent task",
|
||||
startTask: false,
|
||||
})
|
||||
|
||||
// Mock the API model to return GPT-5
|
||||
vi.spyOn(parentTask.api, "getModel").mockReturnValue({
|
||||
id: "gpt-5-codex",
|
||||
info: {
|
||||
contextWindow: 128000,
|
||||
maxTokens: 4096,
|
||||
inputPrice: 0.25,
|
||||
outputPrice: 0.75,
|
||||
} as any,
|
||||
})
|
||||
|
||||
// Spy on say and addToApiConversationHistory
|
||||
const saySpy = vi.spyOn(parentTask, "say").mockResolvedValue(undefined)
|
||||
const addToApiSpy = vi.spyOn(parentTask as any, "addToApiConversationHistory").mockResolvedValue(undefined)
|
||||
|
||||
// Call completeSubtask
|
||||
await parentTask.completeSubtask("Subtask completed successfully")
|
||||
|
||||
// Verify the subtask result was added
|
||||
expect(saySpy).toHaveBeenCalledWith("subtask_result", "Subtask completed successfully")
|
||||
expect(addToApiSpy).toHaveBeenCalledWith({
|
||||
role: "user",
|
||||
content: [{ type: "text", text: "[new_task completed] Result: Subtask completed successfully" }],
|
||||
})
|
||||
|
||||
// Verify skipPrevResponseIdOnce is NOT set for GPT-5 models
|
||||
expect((parentTask as any).skipPrevResponseIdOnce).toBe(false)
|
||||
})
|
||||
|
||||
it("should skip previous_response_id for non-GPT-5 models after subtask completion", async () => {
|
||||
// Create a mock non-GPT-5 API configuration
|
||||
const claudeApiConfig = {
|
||||
apiProvider: "anthropic" as const,
|
||||
apiModelId: "claude-3-5-sonnet-20241022",
|
||||
apiKey: "test-key",
|
||||
}
|
||||
|
||||
// Create parent task with Claude model
|
||||
const parentTask = new Task({
|
||||
provider: mockProvider,
|
||||
apiConfiguration: claudeApiConfig,
|
||||
task: "parent task",
|
||||
startTask: false,
|
||||
})
|
||||
|
||||
// Mock the API model to return Claude
|
||||
vi.spyOn(parentTask.api, "getModel").mockReturnValue({
|
||||
id: "claude-3-5-sonnet-20241022",
|
||||
info: {
|
||||
contextWindow: 200000,
|
||||
maxTokens: 4096,
|
||||
inputPrice: 0.25,
|
||||
outputPrice: 0.75,
|
||||
} as any,
|
||||
})
|
||||
|
||||
// Spy on say and addToApiConversationHistory
|
||||
const saySpy = vi.spyOn(parentTask, "say").mockResolvedValue(undefined)
|
||||
const addToApiSpy = vi.spyOn(parentTask as any, "addToApiConversationHistory").mockResolvedValue(undefined)
|
||||
|
||||
// Call completeSubtask
|
||||
await parentTask.completeSubtask("Subtask completed successfully")
|
||||
|
||||
// Verify the subtask result was added
|
||||
expect(saySpy).toHaveBeenCalledWith("subtask_result", "Subtask completed successfully")
|
||||
expect(addToApiSpy).toHaveBeenCalledWith({
|
||||
role: "user",
|
||||
content: [{ type: "text", text: "[new_task completed] Result: Subtask completed successfully" }],
|
||||
})
|
||||
|
||||
// Verify skipPrevResponseIdOnce IS set for non-GPT-5 models
|
||||
expect((parentTask as any).skipPrevResponseIdOnce).toBe(true)
|
||||
})
|
||||
|
||||
it("should handle edge case where model ID is undefined", async () => {
|
||||
// Create task with minimal configuration
|
||||
const parentTask = new Task({
|
||||
provider: mockProvider,
|
||||
apiConfiguration: mockApiConfig,
|
||||
task: "parent task",
|
||||
startTask: false,
|
||||
})
|
||||
|
||||
// Mock the API model to return undefined ID
|
||||
vi.spyOn(parentTask.api, "getModel").mockReturnValue({
|
||||
id: undefined as any,
|
||||
info: {
|
||||
contextWindow: 200000,
|
||||
maxTokens: 4096,
|
||||
inputPrice: 0.25,
|
||||
outputPrice: 0.75,
|
||||
} as any,
|
||||
})
|
||||
|
||||
// Spy on say and addToApiConversationHistory
|
||||
const saySpy = vi.spyOn(parentTask, "say").mockResolvedValue(undefined)
|
||||
const addToApiSpy = vi.spyOn(parentTask as any, "addToApiConversationHistory").mockResolvedValue(undefined)
|
||||
|
||||
// Call completeSubtask
|
||||
await parentTask.completeSubtask("Subtask completed")
|
||||
|
||||
// When model ID is undefined, should default to skipping (non-GPT-5 behavior)
|
||||
expect((parentTask as any).skipPrevResponseIdOnce).toBe(true)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
1
tmp/pr-8287-Roo-Code
Submodule
1
tmp/pr-8287-Roo-Code
Submodule
|
|
@ -0,0 +1 @@
|
|||
Subproject commit 88a473b017af37091c85ce3056e444e856f80d6e
|
||||
Loading…
Add table
Reference in a new issue