mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-10-07 02:58:15 +00:00
fix: preserve tool_use blocks during condensation regardless of position
The Anthropic API requires every tool_use block to have a corresponding tool_result in the next user message. During conversation condensation, the bounded search (only last 3 messages) failed to find tool_use blocks that were further back in history, causing orphaned tool_result blocks. This fix replaces the bounded linear search with a performance-optimized indexed lookup: 1. Collect all tool_use IDs we need to find from kept messages 2. Early exit if no tool_results in kept messages 3. Build index in ONE pass through the condensed region 4. O(1) lookups instead of repeated O(n) searches Fixes: ROO-516
This commit is contained in:
parent
621d9500de
commit
e00e0c71e9
2 changed files with 49 additions and 35 deletions
|
|
@ -482,17 +482,17 @@ describe("getKeepMessagesWithToolBlocks", () => {
|
|||
expect(result.toolUseBlocksToPreserve).toContainEqual(toolUseBlock2)
|
||||
})
|
||||
|
||||
it("should not crash when tool_result references tool_use beyond search boundary", () => {
|
||||
it("should preserve tool_use even when it is far back in the conversation history", () => {
|
||||
const toolResultBlock = {
|
||||
type: "tool_result" as const,
|
||||
tool_use_id: "toolu_beyond_boundary",
|
||||
tool_use_id: "toolu_far_back",
|
||||
content: "result",
|
||||
}
|
||||
|
||||
// Tool_use is at ts:1, but with N_MESSAGES_TO_KEEP=3, we only search back 3 messages
|
||||
// from startIndex-1. StartIndex is 7 (messages.length=10, keepCount=3, startIndex=7).
|
||||
// So we search from index 6 down to index 4 (7-1 down to 7-3).
|
||||
// The tool_use at index 0 (ts:1) is beyond the search boundary.
|
||||
// Tool_use is at ts:1, far back in the conversation.
|
||||
// The search now covers the entire condensed region (from index 0 to startIndex-1),
|
||||
// so the tool_use WILL be found and preserved. This prevents the API error:
|
||||
// "tool_use ids were found without tool_result blocks immediately after"
|
||||
const messages: ApiMessage[] = [
|
||||
{
|
||||
role: "assistant",
|
||||
|
|
@ -500,7 +500,7 @@ describe("getKeepMessagesWithToolBlocks", () => {
|
|||
{ type: "text" as const, text: "Way back..." },
|
||||
{
|
||||
type: "tool_use" as const,
|
||||
id: "toolu_beyond_boundary",
|
||||
id: "toolu_far_back",
|
||||
name: "old_tool",
|
||||
input: {},
|
||||
},
|
||||
|
|
@ -522,7 +522,6 @@ describe("getKeepMessagesWithToolBlocks", () => {
|
|||
{ role: "user", content: "Message 10", ts: 10 },
|
||||
]
|
||||
|
||||
// Should not crash
|
||||
const result = getKeepMessagesWithToolBlocks(messages, 3)
|
||||
|
||||
// keepMessages should be the last 3 messages
|
||||
|
|
@ -531,8 +530,10 @@ describe("getKeepMessagesWithToolBlocks", () => {
|
|||
expect(result.keepMessages[1].ts).toBe(9)
|
||||
expect(result.keepMessages[2].ts).toBe(10)
|
||||
|
||||
// Should not preserve the tool_use since it's beyond the search boundary
|
||||
expect(result.toolUseBlocksToPreserve).toHaveLength(0)
|
||||
// Should NOW preserve the tool_use - this is the fix for the API error
|
||||
// where tool_result blocks were orphaned because their tool_use wasn't found
|
||||
expect(result.toolUseBlocksToPreserve).toHaveLength(1)
|
||||
expect(result.toolUseBlocksToPreserve[0].id).toBe("toolu_far_back")
|
||||
})
|
||||
|
||||
it("should not duplicate tool_use blocks when same tool_result ID appears multiple times", () => {
|
||||
|
|
|
|||
|
|
@ -105,41 +105,54 @@ export function getKeepMessagesWithToolBlocks(messages: ApiMessage[], keepCount:
|
|||
const reasoningBlocksToPreserve: Anthropic.Messages.ContentBlockParam[] = []
|
||||
const preservedToolUseIds = new Set<string>()
|
||||
|
||||
// Check ALL kept messages for tool_result blocks
|
||||
// First, collect all tool_use_ids we need to find from kept messages
|
||||
const toolUseIdsToFind = new Set<string>()
|
||||
for (const keepMsg of keepMessages) {
|
||||
if (!hasToolResultBlocks(keepMsg)) {
|
||||
continue
|
||||
}
|
||||
for (const toolResult of getToolResultBlocks(keepMsg)) {
|
||||
toolUseIdsToFind.add(toolResult.tool_use_id)
|
||||
}
|
||||
}
|
||||
|
||||
const toolResults = getToolResultBlocks(keepMsg)
|
||||
// Early exit if no tool_results in kept messages
|
||||
if (toolUseIdsToFind.size === 0) {
|
||||
return { keepMessages, toolUseBlocksToPreserve: [], reasoningBlocksToPreserve: [] }
|
||||
}
|
||||
|
||||
for (const toolResult of toolResults) {
|
||||
const toolUseId = toolResult.tool_use_id
|
||||
|
||||
// Skip if we've already found this tool_use
|
||||
if (preservedToolUseIds.has(toolUseId)) {
|
||||
continue
|
||||
// Build an index of tool_use ID -> message for the condensed region
|
||||
// This avoids repeated linear searches through the entire history
|
||||
// Only index assistant messages (which contain tool_use blocks)
|
||||
const toolUseIndex = new Map<string, ApiMessage>()
|
||||
for (let i = 0; i < startIndex; i++) {
|
||||
const msg = messages[i]
|
||||
if (msg.role !== "assistant") {
|
||||
continue
|
||||
}
|
||||
for (const toolUse of getToolUseBlocks(msg)) {
|
||||
// Only index IDs we're looking for
|
||||
if (toolUseIdsToFind.has(toolUse.id)) {
|
||||
toolUseIndex.set(toolUse.id, msg)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Search backwards through the condensed region (bounded)
|
||||
const searchStart = startIndex - 1
|
||||
const searchEnd = Math.max(0, startIndex - N_MESSAGES_TO_KEEP)
|
||||
const messagesToSearch = messages.slice(searchEnd, searchStart + 1)
|
||||
// Now look up each tool_use we need
|
||||
for (const toolUseId of toolUseIdsToFind) {
|
||||
if (preservedToolUseIds.has(toolUseId)) {
|
||||
continue
|
||||
}
|
||||
|
||||
// Find the message containing this tool_use
|
||||
const messageWithToolUse = findLast(messagesToSearch, (msg) => {
|
||||
return findToolUseBlockById(msg, toolUseId) !== undefined
|
||||
})
|
||||
const messageWithToolUse = toolUseIndex.get(toolUseId)
|
||||
if (messageWithToolUse) {
|
||||
const toolUse = findToolUseBlockById(messageWithToolUse, toolUseId)!
|
||||
toolUseBlocksToPreserve.push(toolUse)
|
||||
preservedToolUseIds.add(toolUseId)
|
||||
|
||||
if (messageWithToolUse) {
|
||||
const toolUse = findToolUseBlockById(messageWithToolUse, toolUseId)!
|
||||
toolUseBlocksToPreserve.push(toolUse)
|
||||
preservedToolUseIds.add(toolUseId)
|
||||
|
||||
// Also preserve reasoning blocks from that message
|
||||
const reasoning = getReasoningBlocks(messageWithToolUse)
|
||||
reasoningBlocksToPreserve.push(...reasoning)
|
||||
}
|
||||
// Also preserve reasoning blocks from that message
|
||||
const reasoning = getReasoningBlocks(messageWithToolUse)
|
||||
reasoningBlocksToPreserve.push(...reasoning)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue