Skip to content

Commit 6cd810e

Browse files
canrobins13canrobins13
committed
Revert previous commits
1 parent 4bbeb5f commit 6cd810e

File tree

3 files changed

+31
-101
lines changed

3 files changed

+31
-101
lines changed

‎src/core/condense/__tests__/index.test.ts‎

Lines changed: 4 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import { describe, expect, it, jest, beforeEach } from "@jest/globals"
55
import { TelemetryService } from "@roo-code/telemetry"
66

77
import { ApiHandler } from "../../../api"
8-
import { AwsBedrockHandler } from "../../../api/providers"
98
import { ApiMessage } from "../../task-persistence/apiMessages"
109
import { maybeRemoveImageBlocks } from "../../../api/transform/image-cleaning"
1110
import { summarizeConversation, getMessagesSinceLastSummary, N_MESSAGES_TO_KEEP } from "../index"
@@ -26,20 +25,14 @@ const taskId = "test-task-id"
2625
const DEFAULT_PREV_CONTEXT_TOKENS = 1000
2726

2827
describe("getMessagesSinceLastSummary", () => {
29-
let mockApiHandler: ApiHandler
30-
31-
beforeEach(() => {
32-
mockApiHandler = {} as unknown as ApiHandler
33-
})
34-
3528
it("should return all messages when there is no summary", () => {
3629
const messages: ApiMessage[] = [
3730
{ role: "user", content: "Hello", ts: 1 },
3831
{ role: "assistant", content: "Hi there", ts: 2 },
3932
{ role: "user", content: "How are you?", ts: 3 },
4033
]
4134

42-
const result = getMessagesSinceLastSummary(messages, mockApiHandler)
35+
const result = getMessagesSinceLastSummary(messages)
4336
expect(result).toEqual(messages)
4437
})
4538

@@ -52,7 +45,7 @@ describe("getMessagesSinceLastSummary", () => {
5245
{ role: "assistant", content: "I'm good", ts: 5 },
5346
]
5447

55-
const result = getMessagesSinceLastSummary(messages, mockApiHandler)
48+
const result = getMessagesSinceLastSummary(messages)
5649
expect(result).toEqual([
5750
{ role: "assistant", content: "Summary of conversation", ts: 3, isSummary: true },
5851
{ role: "user", content: "How are you?", ts: 4 },
@@ -69,63 +62,17 @@ describe("getMessagesSinceLastSummary", () => {
6962
{ role: "user", content: "What's new?", ts: 5 },
7063
]
7164

72-
const result = getMessagesSinceLastSummary(messages, mockApiHandler)
65+
const result = getMessagesSinceLastSummary(messages)
7366
expect(result).toEqual([
7467
{ role: "assistant", content: "Second summary", ts: 4, isSummary: true },
7568
{ role: "user", content: "What's new?", ts: 5 },
7669
])
7770
})
7871

7972
it("should handle empty messages array", () => {
80-
const result = getMessagesSinceLastSummary([], mockApiHandler)
73+
const result = getMessagesSinceLastSummary([])
8174
expect(result).toEqual([])
8275
})
83-
84-
it("should prepend user message when using AwsBedrockHandler with summary as first message", () => {
85-
const mockAwsBedrockHandler = new AwsBedrockHandler({
86-
apiModelId: "anthropic.claude-3-5-sonnet-20241022-v2:0",
87-
awsAccessKey: "test-key",
88-
awsSecretKey: "test-secret",
89-
awsRegion: "us-east-1",
90-
})
91-
92-
const messages: ApiMessage[] = [
93-
{ role: "user", content: "Hello", ts: 1 },
94-
{ role: "assistant", content: "Hi there", ts: 2 },
95-
{ role: "assistant", content: "Summary of conversation", ts: 1000, isSummary: true },
96-
{ role: "user", content: "How are you?", ts: 1001 },
97-
{ role: "assistant", content: "I'm good", ts: 1002 },
98-
]
99-
100-
const result = getMessagesSinceLastSummary(messages, mockAwsBedrockHandler)
101-
102-
// Should prepend user message before the summary
103-
expect(result).toEqual([
104-
{ role: "user", content: "Please continue from the following summary:", ts: 999 },
105-
{ role: "assistant", content: "Summary of conversation", ts: 1000, isSummary: true },
106-
{ role: "user", content: "How are you?", ts: 1001 },
107-
{ role: "assistant", content: "I'm good", ts: 1002 },
108-
])
109-
})
110-
111-
it("should not prepend user message when using non-AwsBedrockHandler", () => {
112-
const messages: ApiMessage[] = [
113-
{ role: "user", content: "Hello", ts: 1 },
114-
{ role: "assistant", content: "Hi there", ts: 2 },
115-
{ role: "assistant", content: "Summary of conversation", ts: 1000, isSummary: true },
116-
{ role: "user", content: "How are you?", ts: 1001 },
117-
{ role: "assistant", content: "I'm good", ts: 1002 },
118-
]
119-
120-
const result = getMessagesSinceLastSummary(messages, mockApiHandler)
121-
122-
// Should not prepend user message for non-AwsBedrockHandler
123-
expect(result).toEqual([
124-
{ role: "assistant", content: "Summary of conversation", ts: 1000, isSummary: true },
125-
{ role: "user", content: "How are you?", ts: 1001 },
126-
{ role: "assistant", content: "I'm good", ts: 1002 },
127-
])
128-
})
12976
})
13077

13178
describe("summarizeConversation", () => {

‎src/core/condense/index.ts‎

Lines changed: 26 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { t } from "../../i18n"
66
import { ApiHandler } from "../../api"
77
import { ApiMessage } from "../task-persistence/apiMessages"
88
import { maybeRemoveImageBlocks } from "../../api/transform/image-cleaning"
9-
import { AwsBedrockHandler } from "../../api/providers"
109

1110
export const N_MESSAGES_TO_KEEP = 3
1211

@@ -99,30 +98,7 @@ export async function summarizeConversation(
9998
)
10099

101100
const response: SummarizeResponse = { messages, cost: 0, summary: "" }
102-
103-
// Use condensing API handler if provided, otherwise use main API handler
104-
let handlerToUse = condensingApiHandler || apiHandler
105-
106-
// Check if the chosen handler supports the required functionality
107-
if (!handlerToUse || typeof handlerToUse.createMessage !== "function") {
108-
console.warn(
109-
"Chosen API handler for condensing does not support message creation or is invalid, falling back to main apiHandler.",
110-
)
111-
112-
handlerToUse = apiHandler // Fallback to the main, presumably valid, apiHandler
113-
114-
// Ensure the main apiHandler itself is valid before this point or add another check.
115-
if (!handlerToUse || typeof handlerToUse.createMessage !== "function") {
116-
// This case should ideally not happen if main apiHandler is always valid.
117-
// Consider throwing an error or returning a specific error response.
118-
console.error("Main API handler is also invalid for condensing. Cannot proceed.")
119-
// Return an appropriate error structure for SummarizeResponse
120-
const error = t("common:errors.condense_handler_invalid")
121-
return { ...response, error }
122-
}
123-
}
124-
125-
const messagesToSummarize = getMessagesSinceLastSummary(messages.slice(0, -N_MESSAGES_TO_KEEP), handlerToUse)
101+
const messagesToSummarize = getMessagesSinceLastSummary(messages.slice(0, -N_MESSAGES_TO_KEEP))
126102

127103
if (messagesToSummarize.length <= 1) {
128104
const error =
@@ -150,10 +126,32 @@ export async function summarizeConversation(
150126
({ role, content }) => ({ role, content }),
151127
)
152128

129+
// Note: this doesn't need to be a stream, consider using something like apiHandler.completePrompt
153130
// Use custom prompt if provided and non-empty, otherwise use the default SUMMARY_PROMPT
154131
const promptToUse = customCondensingPrompt?.trim() ? customCondensingPrompt.trim() : SUMMARY_PROMPT
155132

156-
// Note: this doesn't need to be a stream, consider using something like apiHandler.completePrompt
133+
// Use condensing API handler if provided, otherwise use main API handler
134+
let handlerToUse = condensingApiHandler || apiHandler
135+
136+
// Check if the chosen handler supports the required functionality
137+
if (!handlerToUse || typeof handlerToUse.createMessage !== "function") {
138+
console.warn(
139+
"Chosen API handler for condensing does not support message creation or is invalid, falling back to main apiHandler.",
140+
)
141+
142+
handlerToUse = apiHandler // Fallback to the main, presumably valid, apiHandler
143+
144+
// Ensure the main apiHandler itself is valid before this point or add another check.
145+
if (!handlerToUse || typeof handlerToUse.createMessage !== "function") {
146+
// This case should ideally not happen if main apiHandler is always valid.
147+
// Consider throwing an error or returning a specific error response.
148+
console.error("Main API handler is also invalid for condensing. Cannot proceed.")
149+
// Return an appropriate error structure for SummarizeResponse
150+
const error = t("common:errors.condense_handler_invalid")
151+
return { ...response, error }
152+
}
153+
}
154+
157155
const stream = handlerToUse.createMessage(promptToUse, requestMessages)
158156

159157
let summary = ""
@@ -207,28 +205,13 @@ export async function summarizeConversation(
207205
}
208206

209207
/* Returns the list of all messages since the last summary message, including the summary. Returns all messages if there is no summary. */
210-
export function getMessagesSinceLastSummary(messages: ApiMessage[], apiHandler: ApiHandler): ApiMessage[] {
208+
export function getMessagesSinceLastSummary(messages: ApiMessage[]): ApiMessage[] {
211209
let lastSummaryIndexReverse = [...messages].reverse().findIndex((message) => message.isSummary)
212210

213211
if (lastSummaryIndexReverse === -1) {
214212
return messages
215213
}
216214

217215
const lastSummaryIndex = messages.length - lastSummaryIndexReverse - 1
218-
const messagesSinceSummary = messages.slice(lastSummaryIndex)
219-
return maybePrependUserMessage(messagesSinceSummary, apiHandler)
220-
}
221-
222-
function maybePrependUserMessage(messages: ApiMessage[], apiHandler: ApiHandler): ApiMessage[] {
223-
if (messages.length === 0 || !messages[0].isSummary || !(apiHandler instanceof AwsBedrockHandler)) {
224-
return messages
225-
}
226-
// Bedrock requires the first message to be a user message.
227-
// See https://github.com/RooCodeInc/Roo-Code/issues/4147
228-
const userMessage: ApiMessage = {
229-
role: "user",
230-
content: "Please continue from the following summary:",
231-
ts: messages[0]?.ts ? messages[0].ts - 1 : Date.now(),
232-
}
233-
return [userMessage, ...messages]
216+
return messages.slice(lastSummaryIndex)
234217
}

‎src/core/task/Task.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1650,7 +1650,7 @@ export class Task extends EventEmitter<ClineEvents> {
16501650
}
16511651
}
16521652

1653-
const messagesSinceLastSummary = getMessagesSinceLastSummary(this.apiConversationHistory, this.api)
1653+
const messagesSinceLastSummary = getMessagesSinceLastSummary(this.apiConversationHistory)
16541654
const cleanConversationHistory = maybeRemoveImageBlocks(messagesSinceLastSummary, this.api).map(
16551655
({ role, content }) => ({ role, content }),
16561656
)

0 commit comments

Comments
 (0)