Skip to content

Commit 3224d14

Browse files
committed
[Condense] Fix bug where bedrock requires a user message first
1 parent 116a50f commit 3224d14

File tree

2 files changed

+44
-27
lines changed

2 files changed

+44
-27
lines changed

src/core/condense/index.ts

Lines changed: 43 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ 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"
910

1011
export const N_MESSAGES_TO_KEEP = 3
1112

@@ -98,7 +99,30 @@ export async function summarizeConversation(
9899
)
99100

100101
const response: SummarizeResponse = { messages, cost: 0, summary: "" }
101-
const messagesToSummarize = getMessagesSinceLastSummary(messages.slice(0, -N_MESSAGES_TO_KEEP))
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)
102126

103127
if (messagesToSummarize.length <= 1) {
104128
const error =
@@ -126,32 +150,10 @@ export async function summarizeConversation(
126150
({ role, content }) => ({ role, content }),
127151
)
128152

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

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-
156+
// Note: this doesn't need to be a stream, consider using something like apiHandler.completePrompt
155157
const stream = handlerToUse.createMessage(promptToUse, requestMessages)
156158

157159
let summary = ""
@@ -205,13 +207,28 @@ export async function summarizeConversation(
205207
}
206208

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

211213
if (lastSummaryIndexReverse === -1) {
212214
return messages
213215
}
214216

215217
const lastSummaryIndex = messages.length - lastSummaryIndexReverse - 1
216-
return messages.slice(lastSummaryIndex)
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]
217234
}

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

0 commit comments

Comments
 (0)