fix: Better filtering of invalid messages (#417)

* fix: Better filtering of invalid messages

* cr

* cr
This commit is contained in:
Brace Sproul 2025-07-15 16:14:33 -07:00 • committed by GitHub
parent e6a8bf9f69
commit 2b1f301848
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
18 changed files with 605 additions and 110 deletions

View file

@ -6,13 +6,13 @@ import {
MAX_INTERNAL_TOKENS, MAX_INTERNAL_TOKENS,
} from "../utils/tokens.js"; } from "../utils/tokens.js";
describe("calculateConversationHistoryTokenCount", () => { describe("calculateConversationHistoryTokenCount", async () => {
it("should return 0 for empty messages array", () => { it("should return 0 for empty messages array", async () => {
const result = calculateConversationHistoryTokenCount([]); const result = calculateConversationHistoryTokenCount([]);
expect(result).toBe(0); expect(result).toBe(0);
}); });
it("should calculate token count for human messages", () => { it("should calculate token count for human messages", async () => {
const messages = [ const messages = [
new HumanMessage({ new HumanMessage({
content: "This is a test message with exactly 10 words in it.", content: "This is a test message with exactly 10 words in it.",
@ -25,7 +25,7 @@ describe("calculateConversationHistoryTokenCount", () => {
expect(result).toBe(13); expect(result).toBe(13);
}); });
it("should calculate token count for AI messages with usage metadata", () => { it("should calculate token count for AI messages with usage metadata", async () => {
const messages = [ const messages = [
new AIMessage({ new AIMessage({
content: "AI response", content: "AI response",
@ -41,7 +41,7 @@ describe("calculateConversationHistoryTokenCount", () => {
expect(result).toBe(20); expect(result).toBe(20);
}); });
it("should calculate token count for AI messages without usage metadata", () => { it("should calculate token count for AI messages without usage metadata", async () => {
const messages = [ const messages = [
new AIMessage({ new AIMessage({
content: "This is an AI response with no usage metadata.", content: "This is an AI response with no usage metadata.",
@ -54,7 +54,7 @@ describe("calculateConversationHistoryTokenCount", () => {
expect(result).toBe(12); expect(result).toBe(12);
}); });
it("should calculate token count for AI messages with tool calls", () => { it("should calculate token count for AI messages with tool calls", async () => {
const messages = [ const messages = [
new AIMessage({ new AIMessage({
content: "Using a tool", content: "Using a tool",
@ -75,7 +75,7 @@ describe("calculateConversationHistoryTokenCount", () => {
expect(result).toBeGreaterThan(0); expect(result).toBeGreaterThan(0);
}); });
it("should calculate token count for tool messages", () => { it("should calculate token count for tool messages", async () => {
const messages = [ const messages = [
new ToolMessage({ new ToolMessage({
content: "Result of tool execution with some data.", content: "Result of tool execution with some data.",
@ -89,7 +89,7 @@ describe("calculateConversationHistoryTokenCount", () => {
expect(result).toBe(10); expect(result).toBe(10);
}); });
it("should exclude hidden messages when option is provided", () => { it("should exclude hidden messages when option is provided", async () => {
const messages = [ const messages = [
new HumanMessage({ new HumanMessage({
content: "Visible message", content: "Visible message",
@ -110,7 +110,7 @@ describe("calculateConversationHistoryTokenCount", () => {
expect(resultWithOption).toBe(4); // "Visible message" is ~4 tokens expect(resultWithOption).toBe(4); // "Visible message" is ~4 tokens
}); });
it("should exclude messages from the end when option is provided", () => { it("should exclude messages from the end when option is provided", async () => {
const messages = [ const messages = [
new HumanMessage({ content: "First message" }), new HumanMessage({ content: "First message" }),
new HumanMessage({ content: "Second message" }), new HumanMessage({ content: "Second message" }),
@ -127,22 +127,107 @@ describe("calculateConversationHistoryTokenCount", () => {
// First two messages should be ~7 tokens // First two messages should be ~7 tokens
expect(resultWithOption).toBe(7); expect(resultWithOption).toBe(7);
}); });
it("should not separate AI messages with tool calls from their tool messages when excluding from end", async () => {
const aiMessageWithToolCalls = new AIMessage({
content: "I'll help you with that",
tool_calls: [
{
name: "test_tool",
args: { param: "value" },
id: "call_123",
},
],
});
const toolMessage = new ToolMessage({
content: "Tool result",
tool_call_id: "call_123",
});
const messages = [
new HumanMessage({ content: "First message" }),
aiMessageWithToolCalls,
toolMessage,
new HumanMessage({ content: "Last message" }),
];
// Try to exclude 2 messages from the end, which would normally cut between AI and tool message
const result = calculateConversationHistoryTokenCount(messages, {
excludeCountFromEnd: 2,
});
// Should only count the first human message since we can't separate AI/tool pair
const expectedResult = calculateConversationHistoryTokenCount([
new HumanMessage({ content: "First message" }),
]);
expect(result).toBe(expectedResult);
});
it("should preserve multiple tool messages following an AI message", async () => {
const aiMessageWithToolCalls = new AIMessage({
content: "I'll use multiple tools",
tool_calls: [
{
name: "tool1",
args: { param: "value1" },
id: "call_1",
},
{
name: "tool2",
args: { param: "value2" },
id: "call_2",
},
],
});
const toolMessage1 = new ToolMessage({
content: "Tool 1 result",
tool_call_id: "call_1",
});
const toolMessage2 = new ToolMessage({
content: "Tool 2 result",
tool_call_id: "call_2",
});
const messages = [
new HumanMessage({ content: "First message" }),
aiMessageWithToolCalls,
toolMessage1,
toolMessage2,
new HumanMessage({ content: "Last message" }),
];
// Try to exclude 3 messages from the end, which would cut in the middle of tool messages
const result = calculateConversationHistoryTokenCount(messages, {
excludeCountFromEnd: 3,
});
// Should only count the first human message
const expectedResult = calculateConversationHistoryTokenCount([
new HumanMessage({ content: "First message" }),
]);
expect(result).toBe(expectedResult);
});
}); });
describe("getMessagesSinceLastSummary", () => { describe("getMessagesSinceLastSummary", async () => {
it("should return all messages when there is no summary message", () => { it("should return all messages when there is no summary message", async () => {
const messages = [ const messages = [
new HumanMessage({ content: "Message 1" }), new HumanMessage({ content: "Message 1" }),
new AIMessage({ content: "Message 2" }), new AIMessage({ content: "Message 2" }),
new HumanMessage({ content: "Message 3" }), new HumanMessage({ content: "Message 3" }),
]; ];
const result = getMessagesSinceLastSummary(messages); const result = await getMessagesSinceLastSummary(messages);
expect(result).toHaveLength(3); expect(result).toHaveLength(3);
expect(result).toEqual(messages); expect(result).toEqual(messages);
}); });
it("should return messages after the last summary message", () => { it("should return messages after the last summary message", async () => {
const summaryMessage = new AIMessage({ const summaryMessage = new AIMessage({
content: "Summary of conversation", content: "Summary of conversation",
additional_kwargs: { summary_message: true }, additional_kwargs: { summary_message: true },
@ -155,13 +240,13 @@ describe("getMessagesSinceLastSummary", () => {
new AIMessage({ content: "Message 4" }), new AIMessage({ content: "Message 4" }),
]; ];
const result = getMessagesSinceLastSummary(messages); const result = await getMessagesSinceLastSummary(messages);
expect(result).toHaveLength(2); expect(result).toHaveLength(2);
expect(result[0].content).toBe("Message 3"); expect(result[0].content).toBe("Message 3");
expect(result[1].content).toBe("Message 4"); expect(result[1].content).toBe("Message 4");
}); });
it("should exclude hidden messages when option is provided", () => { it("should exclude hidden messages when option is provided", async () => {
const summaryMessage = new AIMessage({ const summaryMessage = new AIMessage({
content: "Summary of conversation", content: "Summary of conversation",
additional_kwargs: { summary_message: true }, additional_kwargs: { summary_message: true },
@ -177,7 +262,7 @@ describe("getMessagesSinceLastSummary", () => {
new AIMessage({ content: "Another visible message" }), new AIMessage({ content: "Another visible message" }),
]; ];
const result = getMessagesSinceLastSummary(messages, { const result = await getMessagesSinceLastSummary(messages, {
excludeHiddenMessages: true, excludeHiddenMessages: true,
}); });
@ -186,7 +271,7 @@ describe("getMessagesSinceLastSummary", () => {
expect(result[1].content).toBe("Another visible message"); expect(result[1].content).toBe("Another visible message");
}); });
it("should exclude messages from the end when option is provided", () => { it("should exclude messages from the end when option is provided", async () => {
const summaryMessage = new AIMessage({ const summaryMessage = new AIMessage({
content: "Summary of conversation", content: "Summary of conversation",
additional_kwargs: { summary_message: true }, additional_kwargs: { summary_message: true },
@ -199,7 +284,7 @@ describe("getMessagesSinceLastSummary", () => {
new HumanMessage({ content: "Message 3" }), new HumanMessage({ content: "Message 3" }),
]; ];
const result = getMessagesSinceLastSummary(messages, { const result = await getMessagesSinceLastSummary(messages, {
excludeCountFromEnd: 1, excludeCountFromEnd: 1,
}); });
@ -208,7 +293,7 @@ describe("getMessagesSinceLastSummary", () => {
expect(result[1].content).toBe("Message 2"); expect(result[1].content).toBe("Message 2");
}); });
it("should handle both excludeHiddenMessages and excludeCountFromEnd options", () => { it("should handle both excludeHiddenMessages and excludeCountFromEnd options", async () => {
const summaryMessage = new AIMessage({ const summaryMessage = new AIMessage({
content: "Summary of conversation", content: "Summary of conversation",
additional_kwargs: { summary_message: true }, additional_kwargs: { summary_message: true },
@ -225,7 +310,7 @@ describe("getMessagesSinceLastSummary", () => {
new HumanMessage({ content: "Message 4" }), new HumanMessage({ content: "Message 4" }),
]; ];
const result = getMessagesSinceLastSummary(messages, { const result = await getMessagesSinceLastSummary(messages, {
excludeHiddenMessages: true, excludeHiddenMessages: true,
excludeCountFromEnd: 1, excludeCountFromEnd: 1,
}); });
@ -235,7 +320,304 @@ describe("getMessagesSinceLastSummary", () => {
expect(result[1].content).toBe("Message 3"); expect(result[1].content).toBe("Message 3");
}); });
it("should return empty array if all messages are before the summary", () => { it("should not separate AI messages with tool calls from their tool messages when excluding from end", async () => {
const summaryMessage = new AIMessage({
content: "Summary of conversation",
additional_kwargs: { summary_message: true },
});
const aiMessageWithToolCalls = new AIMessage({
content: "I'll help you with that",
tool_calls: [
{
name: "test_tool",
args: { param: "value" },
id: "call_123",
},
],
});
const toolMessage = new ToolMessage({
content: "Tool result",
tool_call_id: "call_123",
});
const messages = [
summaryMessage,
new HumanMessage({ content: "First message" }),
aiMessageWithToolCalls,
toolMessage,
new HumanMessage({ content: "Last message" }),
];
// Try to exclude 2 messages from the end, which would normally cut between AI and tool message
const result = await getMessagesSinceLastSummary(messages, {
excludeCountFromEnd: 2,
});
// Should only include the first human message since we can't separate AI/tool pair
expect(result).toHaveLength(1);
expect(result[0].content).toBe("First message");
});
it("should preserve multiple tool messages following an AI message in getMessagesSinceLastSummary", async () => {
const summaryMessage = new AIMessage({
content: "Summary of conversation",
additional_kwargs: { summary_message: true },
});
const aiMessageWithToolCalls = new AIMessage({
content: "I'll use multiple tools",
tool_calls: [
{
name: "tool1",
args: { param: "value1" },
id: "call_1",
},
{
name: "tool2",
args: { param: "value2" },
id: "call_2",
},
],
});
const toolMessage1 = new ToolMessage({
content: "Tool 1 result",
tool_call_id: "call_1",
});
const toolMessage2 = new ToolMessage({
content: "Tool 2 result",
tool_call_id: "call_2",
});
const messages = [
summaryMessage,
new HumanMessage({ content: "First message" }),
aiMessageWithToolCalls,
toolMessage1,
toolMessage2,
new HumanMessage({ content: "Last message" }),
];
// Try to exclude 3 messages from the end, which would cut in the middle of tool messages
const result = await getMessagesSinceLastSummary(messages, {
excludeCountFromEnd: 3,
});
// Should only include the first human message
expect(result).toHaveLength(1);
expect(result[0].content).toBe("First message");
});
it("should exclude entire AI/tool group when cut point would separate them", async () => {
const summaryMessage = new AIMessage({
content: "Summary of conversation",
additional_kwargs: { summary_message: true },
});
const aiMessageWithToolCalls = new AIMessage({
content: "I'll use a tool",
tool_calls: [
{
name: "test_tool",
args: { param: "value" },
id: "call_123",
},
],
});
const toolMessage = new ToolMessage({
content: "Tool result",
tool_call_id: "call_123",
});
const messages = [
summaryMessage,
new HumanMessage({ content: "First message" }),
aiMessageWithToolCalls,
toolMessage,
];
// Try to exclude 1 message from the end (just the tool message)
const result = await getMessagesSinceLastSummary(messages, {
excludeCountFromEnd: 1,
});
// Should exclude the entire AI/tool group to maintain integrity
expect(result).toHaveLength(1);
expect(result[0].content).toBe("First message");
});
it("should preserve AI message with multiple tool calls and their corresponding tool messages", async () => {
const summaryMessage = new AIMessage({
content: "Summary of conversation",
additional_kwargs: { summary_message: true },
});
const aiMessageWithMultipleToolCalls = new AIMessage({
content: "I'll use multiple tools to help you",
tool_calls: [
{
name: "search_tool",
args: { query: "example query" },
id: "call_search_123",
},
{
name: "calculator_tool",
args: { expression: "2 + 2" },
id: "call_calc_456",
},
{
name: "file_tool",
args: { filename: "test.txt" },
id: "call_file_789",
},
],
});
const searchToolMessage = new ToolMessage({
content: "Search results found",
tool_call_id: "call_search_123",
});
const calculatorToolMessage = new ToolMessage({
content: "Result: 4",
tool_call_id: "call_calc_456",
});
const fileToolMessage = new ToolMessage({
content: "File contents: Hello world",
tool_call_id: "call_file_789",
});
const messages = [
summaryMessage,
new HumanMessage({ content: "First message" }),
aiMessageWithMultipleToolCalls,
searchToolMessage,
calculatorToolMessage,
fileToolMessage,
new HumanMessage({ content: "After all tools" }),
new HumanMessage({ content: "Last message" }),
];
// Try to exclude 4 messages from the end, which would cut in the middle of the tool messages
const result = await getMessagesSinceLastSummary(messages, {
excludeCountFromEnd: 4,
});
// Should include the first human message and the complete AI/tool group since we can't separate them
expect(result).toHaveLength(5);
expect(result[0].content).toBe("First message");
expect(result[1].content).toBe("I'll use multiple tools to help you");
expect(result[2].content).toBe("Search results found");
expect(result[3].content).toBe("Result: 4");
expect(result[4].content).toBe("File contents: Hello world");
});
it("should include complete AI/tool group when exclusion doesn't break the group", async () => {
const summaryMessage = new AIMessage({
content: "Summary of conversation",
additional_kwargs: { summary_message: true },
});
const aiMessageWithMultipleToolCalls = new AIMessage({
content: "I'll use two tools",
tool_calls: [
{
name: "tool1",
args: { param: "value1" },
id: "call_1",
},
{
name: "tool2",
args: { param: "value2" },
id: "call_2",
},
],
});
const tool1Message = new ToolMessage({
content: "Tool 1 result",
tool_call_id: "call_1",
});
const tool2Message = new ToolMessage({
content: "Tool 2 result",
tool_call_id: "call_2",
});
const messages = [
summaryMessage,
new HumanMessage({ content: "First message" }),
aiMessageWithMultipleToolCalls,
tool1Message,
tool2Message,
new HumanMessage({ content: "After tools" }),
new HumanMessage({ content: "Second to last" }),
new HumanMessage({ content: "Last message" }),
];
// Try to exclude 2 messages from the end (just the last two human messages)
const result = await getMessagesSinceLastSummary(messages, {
excludeCountFromEnd: 2,
});
// Should include the first human message, the AI message, both tool messages, and the "After tools" message
expect(result).toHaveLength(5);
expect(result[0].content).toBe("First message");
expect(result[1].content).toBe("I'll use two tools");
expect(result[2].content).toBe("Tool 1 result");
expect(result[3].content).toBe("Tool 2 result");
expect(result[4].content).toBe("After tools");
});
it("should handle case where AI/tool group can be included entirely", async () => {
const summaryMessage = new AIMessage({
content: "Summary of conversation",
additional_kwargs: { summary_message: true },
});
const aiMessageWithToolCalls = new AIMessage({
content: "I'll use a tool",
tool_calls: [
{
name: "test_tool",
args: { param: "value" },
id: "call_123",
},
],
});
const toolMessage = new ToolMessage({
content: "Tool result",
tool_call_id: "call_123",
});
const messages = [
summaryMessage,
new HumanMessage({ content: "First message" }),
aiMessageWithToolCalls,
toolMessage,
new HumanMessage({ content: "After tool message" }),
new HumanMessage({ content: "Last message" }),
];
// Try to exclude 2 messages from the end (the last two human messages)
const result = await getMessagesSinceLastSummary(messages, {
excludeCountFromEnd: 2,
});
// Should include the first human message and the complete AI/tool group
expect(result).toHaveLength(3);
expect(result[0].content).toBe("First message");
expect(result[1].content).toBe("I'll use a tool");
expect(result[2].content).toBe("Tool result");
});
it("should return empty array if all messages are before the summary", async () => {
const messages = [ const messages = [
new HumanMessage({ content: "Message 1" }), new HumanMessage({ content: "Message 1" }),
new AIMessage({ content: "Message 2" }), new AIMessage({ content: "Message 2" }),
@ -245,13 +627,13 @@ describe("getMessagesSinceLastSummary", () => {
}), }),
]; ];
const result = getMessagesSinceLastSummary(messages); const result = await getMessagesSinceLastSummary(messages);
expect(result).toHaveLength(0); expect(result).toHaveLength(0);
}); });
}); });
describe("MAX_INTERNAL_TOKENS constant", () => { describe("MAX_INTERNAL_TOKENS constant", async () => {
it("should be defined as 60,000", () => { it("should be defined as 60,000", async () => {
expect(MAX_INTERNAL_TOKENS).toBe(60_000); expect(MAX_INTERNAL_TOKENS).toBe(60_000);
}); });
}); });

View file

@ -17,12 +17,12 @@ import {
import { SYSTEM_PROMPT } from "./prompt.js"; import { SYSTEM_PROMPT } from "./prompt.js";
import { getRepoAbsolutePath } from "@open-swe/shared/git"; import { getRepoAbsolutePath } from "@open-swe/shared/git";
import { getMissingMessages } from "../../../../utils/github/issue-messages.js"; import { getMissingMessages } from "../../../../utils/github/issue-messages.js";
import { filterHiddenMessages } from "../../../../utils/message/filter-hidden.js";
import { getPlansFromIssue } from "../../../../utils/github/issue-task.js"; import { getPlansFromIssue } from "../../../../utils/github/issue-task.js";
import { createSearchTool } from "../../../../tools/search.js"; import { createSearchTool } from "../../../../tools/search.js";
import { formatCustomRulesPrompt } from "../../../../utils/custom-rules.js"; import { formatCustomRulesPrompt } from "../../../../utils/custom-rules.js";
import { createPlannerNotesTool } from "../../../../tools/planner-notes.js"; import { createPlannerNotesTool } from "../../../../tools/planner-notes.js";
import { getMcpTools } from "../../../../utils/mcp-client.js"; import { getMcpTools } from "../../../../utils/mcp-client.js";
import { filterMessagesWithoutContent } from "../../../../utils/message/content.js";
const logger = createLogger(LogLevel.INFO, "GeneratePlanningMessageNode"); const logger = createLogger(LogLevel.INFO, "GeneratePlanningMessageNode");
@ -73,6 +73,15 @@ export async function generateAction(
getMissingMessages(state, config), getMissingMessages(state, config),
getPlansFromIssue(state, config), getPlansFromIssue(state, config),
]); ]);
const inputMessages = filterMessagesWithoutContent([
...state.messages,
...missingMessages,
]);
if (!inputMessages.length) {
throw new Error("No messages to process.");
}
const response = await modelWithTools const response = await modelWithTools
.withConfig({ tags: ["nostream"] }) .withConfig({ tags: ["nostream"] })
.invoke([ .invoke([
@ -83,8 +92,7 @@ export async function generateAction(
taskPlan: latestTaskPlan ?? state.taskPlan, taskPlan: latestTaskPlan ?? state.taskPlan,
}), }),
}, },
...filterHiddenMessages(state.messages), ...inputMessages,
...missingMessages,
]); ]);
logger.info("Generated planning message", { logger.info("Generated planning message", {

View file

@ -13,12 +13,12 @@ import {
isFollowupRequest, isFollowupRequest,
} from "../../utils/followup.js"; } from "../../utils/followup.js";
import { stopSandbox } from "../../../../utils/sandbox.js"; import { stopSandbox } from "../../../../utils/sandbox.js";
import { filterHiddenMessages } from "../../../../utils/message/filter-hidden.js";
import { z } from "zod"; import { z } from "zod";
import { formatCustomRulesPrompt } from "../../../../utils/custom-rules.js"; import { formatCustomRulesPrompt } from "../../../../utils/custom-rules.js";
import { getPlannerNotes } from "../../utils/get-notes.js"; import { getPlannerNotes } from "../../utils/get-notes.js";
import { PLANNER_NOTES_PROMPT, SYSTEM_PROMPT } from "./prompt.js"; import { PLANNER_NOTES_PROMPT, SYSTEM_PROMPT } from "./prompt.js";
import { DO_NOT_RENDER_ID_PREFIX } from "@open-swe/shared/constants"; import { DO_NOT_RENDER_ID_PREFIX } from "@open-swe/shared/constants";
import { filterMessagesWithoutContent } from "../../../../utils/message/content.js";
function formatSystemPrompt(state: PlannerGraphState): string { function formatSystemPrompt(state: PlannerGraphState): string {
// It's a followup if there's more than one human message. // It's a followup if there's more than one human message.
@ -68,6 +68,14 @@ export async function generatePlan(
}); });
} }
const inputMessages = filterMessagesWithoutContent([
...state.messages,
...(optionalToolMessage ? [optionalToolMessage] : []),
]);
if (!inputMessages.length) {
throw new Error("No messages to process.");
}
const response = await modelWithTools const response = await modelWithTools
.withConfig({ tags: ["nostream"] }) .withConfig({ tags: ["nostream"] })
.invoke([ .invoke([
@ -75,8 +83,7 @@ export async function generatePlan(
role: "system", role: "system",
content: formatSystemPrompt(state), content: formatSystemPrompt(state),
}, },
...filterHiddenMessages(state.messages), ...inputMessages,
...(optionalToolMessage ? [optionalToolMessage] : []),
]); ]);
const toolCall = response.tool_calls?.[0]; const toolCall = response.tool_calls?.[0];

View file

@ -58,7 +58,7 @@ function createAcceptedPlanMessage(input: {
const acceptedPlanMessage = new AIMessage({ const acceptedPlanMessage = new AIMessage({
id: `${DO_NOT_RENDER_ID_PREFIX}${uuidv4()}`, id: `${DO_NOT_RENDER_ID_PREFIX}${uuidv4()}`,
content: "", content: "Accepted plan",
additional_kwargs: { additional_kwargs: {
hidden: true, hidden: true,
customNodeEvents: [acceptedPlanEvent], customNodeEvents: [acceptedPlanEvent],

View file

@ -116,6 +116,10 @@ export async function takeActions(
result = toolResult.result; result = toolResult.result;
toolCallStatus = toolResult.status; toolCallStatus = toolResult.status;
} }
if (!result) {
result = toolCallStatus;
}
} catch (e) { } catch (e) {
toolCallStatus = "error"; toolCallStatus = "error";
if ( if (

View file

@ -34,6 +34,7 @@ import {
formatCodeReviewPrompt, formatCodeReviewPrompt,
getCodeReviewFields, getCodeReviewFields,
} from "../../../../utils/review.js"; } from "../../../../utils/review.js";
import { filterMessagesWithoutContent } from "../../../../utils/message/content.js";
const logger = createLogger(LogLevel.INFO, "GenerateMessageNode"); const logger = createLogger(LogLevel.INFO, "GenerateMessageNode");
@ -117,6 +118,14 @@ export async function generateAction(
getPlansFromIssue(state, config), getPlansFromIssue(state, config),
]); ]);
const inputMessages = filterMessagesWithoutContent([
...state.internalMessages,
...missingMessages,
]);
if (!inputMessages.length) {
throw new Error("No messages to process.");
}
const response = await modelWithTools.invoke([ const response = await modelWithTools.invoke([
{ {
role: "system", role: "system",
@ -125,8 +134,7 @@ export async function generateAction(
taskPlan: latestTaskPlan ?? state.taskPlan, taskPlan: latestTaskPlan ?? state.taskPlan,
}), }),
}, },
...state.internalMessages, ...inputMessages,
...missingMessages,
]); ]);
const hasToolCalls = !!response.tool_calls?.length; const hasToolCalls = !!response.tool_calls?.length;

View file

@ -29,6 +29,7 @@ import {
} from "@open-swe/shared/open-swe/tools"; } from "@open-swe/shared/open-swe/tools";
import { import {
calculateConversationHistoryTokenCount, calculateConversationHistoryTokenCount,
getMessagesSinceLastSummary,
MAX_INTERNAL_TOKENS, MAX_INTERNAL_TOKENS,
} from "../../../utils/tokens.js"; } from "../../../utils/tokens.js";
import { z } from "zod"; import { z } from "zod";
@ -117,8 +118,14 @@ Once you've determined the status of the current task, call either the \`mark_ta
const newMessages = [response, toolMessage]; const newMessages = [response, toolMessage];
const newMessageList = [...state.internalMessages, ...newMessages];
const wouldBeConversationHistoryToSummarize =
await getMessagesSinceLastSummary(newMessageList, {
excludeHiddenMessages: true,
excludeCountFromEnd: 20,
});
const totalInternalTokenCount = calculateConversationHistoryTokenCount( const totalInternalTokenCount = calculateConversationHistoryTokenCount(
state.internalMessages, wouldBeConversationHistoryToSummarize,
{ {
// Retain the last 20 messages from state // Retain the last 20 messages from state
excludeHiddenMessages: true, excludeHiddenMessages: true,
@ -135,12 +142,15 @@ Once you've determined the status of the current task, call either the \`mark_ta
internalMessages: newMessages, internalMessages: newMessages,
}; };
// Check if we have any messages to summarize, and if we're at or above the max token limit.
if (totalInternalTokenCount >= MAX_INTERNAL_TOKENS) { if (totalInternalTokenCount >= MAX_INTERNAL_TOKENS) {
logger.info( logger.info(
"Internal messages list is at or above the max token limit. Routing to summarize history step.", "Internal messages list is at or above the max token limit. Routing to summarize history step.",
{ {
totalInternalTokenCount, totalInternalTokenCount,
maxInternalTokenCount: MAX_INTERNAL_TOKENS, maxInternalTokenCount: MAX_INTERNAL_TOKENS,
wouldBeConversationHistoryToSummarizeLength:
wouldBeConversationHistoryToSummarize.length,
}, },
); );
return new Command({ return new Command({

View file

@ -18,8 +18,6 @@ import { getMessageContentString } from "@open-swe/shared/messages";
import { getMessageString } from "../../../utils/message/content.js"; import { getMessageString } from "../../../utils/message/content.js";
import { getActivePlanItems } from "@open-swe/shared/open-swe/tasks"; import { getActivePlanItems } from "@open-swe/shared/open-swe/tasks";
import { createConversationHistorySummaryToolFields } from "@open-swe/shared/open-swe/tools"; import { createConversationHistorySummaryToolFields } from "@open-swe/shared/open-swe/tools";
import { z } from "zod";
import { DO_NOT_RENDER_ID_PREFIX } from "@open-swe/shared/constants";
import { getUserRequest } from "../../../utils/user-request.js"; import { getUserRequest } from "../../../utils/user-request.js";
import { getMessagesSinceLastSummary } from "../../../utils/tokens.js"; import { getMessagesSinceLastSummary } from "../../../utils/tokens.js";
@ -91,45 +89,9 @@ const formatPrompt = (inputs: {
); );
}; };
/** function createSummaryMessages(summary: string): BaseMessage[] {
* Create an AI & tool message pair for the generated task summary. const dummySummarizeHistoryToolName =
* This is not included in the internal message state, but is exposed to createConversationHistorySummaryToolFields().name;
* users so they can see the summary of the actions that were taken.
*/
function createUserFacingConversationSummaryMessages(
summary: string,
): BaseMessage[] {
const conversationSummaryTool = createConversationHistorySummaryToolFields();
const conversationSummaryToolCallArgs: z.infer<
typeof conversationSummaryTool.schema
> = {
conversation_history_summary: summary,
};
const conversationSummaryToolCallId = uuidv4();
const conversationSummaryPublicMessages = [
new AIMessage({
id: uuidv4(),
content: "",
tool_calls: [
{
id: conversationSummaryToolCallId,
name: conversationSummaryTool.name,
args: conversationSummaryToolCallArgs,
},
],
}),
new ToolMessage({
id: `${DO_NOT_RENDER_ID_PREFIX}${uuidv4()}`,
tool_call_id: conversationSummaryToolCallId,
content: "",
}),
];
return conversationSummaryPublicMessages;
}
function createInternalSummaryMessages(summary: string): BaseMessage[] {
const dummySummarizeHistoryToolName = "summarize_conversation_history";
const dummySummarizeHistoryToolCallId = uuidv4(); const dummySummarizeHistoryToolCallId = uuidv4();
return [ return [
new AIMessage({ new AIMessage({
@ -169,7 +131,7 @@ export async function summarizeHistory(
const userRequest = getUserRequest(state.messages); const userRequest = getUserRequest(state.messages);
const plan = getActivePlanItems(state.taskPlan); const plan = getActivePlanItems(state.taskPlan);
const conversationHistoryToSummarize = getMessagesSinceLastSummary( const conversationHistoryToSummarize = await getMessagesSinceLastSummary(
state.internalMessages, state.internalMessages,
{ {
excludeHiddenMessages: true, excludeHiddenMessages: true,
@ -193,14 +155,13 @@ export async function summarizeHistory(
]); ]);
const summaryString = getMessageContentString(response.content); const summaryString = getMessageContentString(response.content);
const taskSummaryMessages = const summaryMessages = createSummaryMessages(summaryString);
createUserFacingConversationSummaryMessages(summaryString);
const newInternalMessages = [ const newInternalMessages = [
...conversationHistoryToSummarize.map( ...conversationHistoryToSummarize.map(
(m) => new RemoveMessage({ id: m.id ?? "" }), (m) => new RemoveMessage({ id: m.id ?? "" }),
), ),
...createInternalSummaryMessages(summaryString), ...summaryMessages,
]; ];
logger.info( logger.info(
@ -208,7 +169,7 @@ export async function summarizeHistory(
); );
return { return {
messages: taskSummaryMessages, messages: summaryMessages,
internalMessages: newInternalMessages, internalMessages: newInternalMessages,
}; };
} }

View file

@ -106,6 +106,10 @@ export async function takeAction(
result = toolResult.result; result = toolResult.result;
toolCallStatus = toolResult.status; toolCallStatus = toolResult.status;
} }
if (!result) {
result = toolCallStatus;
}
} catch (e) { } catch (e) {
toolCallStatus = "error"; toolCallStatus = "error";
if ( if (

View file

@ -37,7 +37,7 @@ function createReviewStartedMessage() {
new ToolMessage({ new ToolMessage({
id: uuidv4(), id: uuidv4(),
tool_call_id: toolCallId, tool_call_id: toolCallId,
content: "", content: "Review started",
additional_kwargs: { additional_kwargs: {
hidden: true, hidden: true,
}, },

View file

@ -96,6 +96,9 @@ export async function takeReviewerActions(
status: "success" | "error"; status: "success" | "error";
}; };
result = toolResult.result; result = toolResult.result;
if (!result) {
result = toolResult.status;
}
toolCallStatus = toolResult.status; toolCallStatus = toolResult.status;
} catch (e) { } catch (e) {
toolCallStatus = "error"; toolCallStatus = "error";

View file

@ -71,7 +71,7 @@ export async function initializeSandbox(
const createEventsMessage = () => [ const createEventsMessage = () => [
new AIMessage({ new AIMessage({
id: `${DO_NOT_RENDER_ID_PREFIX}${uuidv4()}`, id: `${DO_NOT_RENDER_ID_PREFIX}${uuidv4()}`,
content: "", content: "Initialize sandbox",
additional_kwargs: { additional_kwargs: {
hidden: true, hidden: true,
customNodeEvents: events, customNodeEvents: events,

View file

@ -34,7 +34,7 @@ export function createShellTool(
} }
return { return {
result: response.result, result: response.result ?? `exit code: ${response.exitCode}`,
status: "success", status: "success",
}; };
} catch (e) { } catch (e) {

View file

@ -59,3 +59,20 @@ export function getMessageString(message: BaseMessage): string {
return getUnknownMessageString(message); return getUnknownMessageString(message);
} }
export function filterMessagesWithoutContent(
messages: BaseMessage[],
filterHidden = true,
): BaseMessage[] {
return messages.filter((m) => {
if (filterHidden && m.additional_kwargs?.hidden) {
return false;
}
const messageContentStr = getMessageContentString(m.content);
if (!isAIMessage(m)) {
return !!messageContentStr;
}
const toolCallsCount = m.tool_calls?.length || 0;
return !!messageContentStr || toolCallsCount > 0;
});
}

View file

@ -5,6 +5,7 @@ import {
isToolMessage, isToolMessage,
} from "@langchain/core/messages"; } from "@langchain/core/messages";
import { getMessageContentString } from "@open-swe/shared/messages"; import { getMessageContentString } from "@open-swe/shared/messages";
import { traceable } from "langsmith/traceable";
// After 60k tokens, summarize the conversation history. // After 60k tokens, summarize the conversation history.
export const MAX_INTERNAL_TOKENS = 60_000; export const MAX_INTERNAL_TOKENS = 60_000;
@ -17,39 +18,121 @@ export function calculateConversationHistoryTokenCount(
}, },
) { ) {
let totalChars = 0; let totalChars = 0;
const messagesToCount = options?.excludeCountFromEnd let messagesToCount = messages;
? messages.slice(0, -options.excludeCountFromEnd)
: messages; if (options?.excludeCountFromEnd && options.excludeCountFromEnd > 0) {
messagesToCount = getMessagesExcludingFromEnd(
messages,
options.excludeCountFromEnd,
);
}
messagesToCount.forEach((m) => { messagesToCount.forEach((m) => {
if (options?.excludeHiddenMessages && m.additional_kwargs?.hidden) { if (options?.excludeHiddenMessages && m.additional_kwargs?.hidden) {
return; return;
} }
if (isHumanMessage(m) || isToolMessage(m)) { if (isHumanMessage(m) || isToolMessage(m)) {
const contentString = getMessageContentString(m.content); const contentString = getMessageContentString(m.content);
totalChars += contentString.length; // Divide each char by 4 as it's roughly one token per 4 characters.
totalChars += contentString.length / 4;
} }
if (isAIMessage(m)) { if (isAIMessage(m)) {
const usageMetadata = m.usage_metadata; const usageMetadata = m.usage_metadata;
if (usageMetadata) { if (usageMetadata) {
// multiply by 4 here since we divide by 4 to estimate tokens. totalChars += usageMetadata.output_tokens;
totalChars += usageMetadata.total_tokens * 4;
} else { } else {
const contentString = getMessageContentString(m.content); const contentString = getMessageContentString(m.content);
totalChars += contentString.length; totalChars += contentString.length / 4;
m.tool_calls?.forEach((tc) => { m.tool_calls?.forEach((tc) => {
totalChars += tc.name.length; const nameAndArgs = tc.name + JSON.stringify(tc.args);
totalChars += JSON.stringify(tc.args).length; totalChars += nameAndArgs.length / 4;
}); });
} }
} }
}); });
// Estimate 1 token for every 4 characters. return totalChars;
return Math.ceil(totalChars / 4);
} }
export function getMessagesSinceLastSummary( /**
* Helper function to exclude messages from the end while preserving AI/tool message pairs
*/
function getMessagesExcludingFromEnd(
messages: BaseMessage[],
excludeCount: number,
): BaseMessage[] {
if (excludeCount <= 0 || excludeCount >= messages.length) {
return excludeCount >= messages.length ? [] : messages;
}
let endIndex = messages.length - excludeCount;
// Check if we're breaking up an AI message with tool calls and its corresponding tool messages
// We need to look backwards from the cut point to ensure we don't separate AI/tool pairs
while (endIndex > 0 && endIndex < messages.length) {
const messageAtCutPoint = messages[endIndex - 1];
// If the message before the cut point is an AI message with tool calls,
// we need to check if there are corresponding tool messages after it
if (
isAIMessage(messageAtCutPoint) &&
(messageAtCutPoint as any).tool_calls &&
(messageAtCutPoint as any).tool_calls.length > 0
) {
// Count how many tool messages follow this AI message
let toolMessageCount = 0;
for (
let i = endIndex;
i < messages.length && isToolMessage(messages[i]);
i++
) {
toolMessageCount++;
}
// If there are tool messages that would be cut off, move the cut point back
// to include the AI message and all its tool messages, or exclude them entirely
if (toolMessageCount > 0) {
// Move cut point back to exclude the AI message entirely (safer approach)
endIndex--;
continue;
}
}
// If the message at the cut point is a tool message, check if it belongs to an AI message
if (isToolMessage(messages[endIndex])) {
// Look backwards to find the corresponding AI message
let aiMessageIndex = endIndex - 1;
while (aiMessageIndex >= 0 && isToolMessage(messages[aiMessageIndex])) {
aiMessageIndex--;
}
// If we found an AI message with tool calls, include all related messages
if (
aiMessageIndex >= 0 &&
isAIMessage(messages[aiMessageIndex]) &&
(messages[aiMessageIndex] as any).tool_calls &&
(messages[aiMessageIndex] as any).tool_calls.length > 0
) {
// Move cut point to include the entire AI/tool group
let toolGroupEnd = endIndex;
while (
toolGroupEnd < messages.length &&
isToolMessage(messages[toolGroupEnd])
) {
toolGroupEnd++;
}
endIndex = toolGroupEnd;
break;
}
}
break;
}
return messages.slice(0, endIndex);
}
export function getMessagesSinceLastSummaryFunc(
messages: BaseMessage[], messages: BaseMessage[],
options?: { options?: {
excludeHiddenMessages?: boolean; excludeHiddenMessages?: boolean;
@ -76,14 +159,18 @@ export function getMessagesSinceLastSummary(
// Apply excludeCountFromEnd option if provided // Apply excludeCountFromEnd option if provided
if (options?.excludeCountFromEnd && options.excludeCountFromEnd > 0) { if (options?.excludeCountFromEnd && options.excludeCountFromEnd > 0) {
messagesAfterLastSummary = messagesAfterLastSummary.slice( messagesAfterLastSummary = getMessagesExcludingFromEnd(
0, messagesAfterLastSummary,
Math.max( options.excludeCountFromEnd,
0,
messagesAfterLastSummary.length - options.excludeCountFromEnd,
),
); );
} }
return messagesAfterLastSummary; return messagesAfterLastSummary;
} }
export const getMessagesSinceLastSummary = traceable(
getMessagesSinceLastSummaryFunc,
{
name: "get-messages-since-last-summary",
},
);

View file

@ -45,9 +45,9 @@ export function TaskSummary({
const getStatusText = () => { const getStatusText = () => {
switch (status) { switch (status) {
case "loading": case "loading":
return "Preparing task summary..."; return "Preparing action reflection...";
case "generating": case "generating":
return "Generating summary..."; return "Generating action reflection...";
case "done": case "done":
if (completed === false) { if (completed === false) {
return "Task not completed"; return "Task not completed";
@ -72,7 +72,7 @@ export function TaskSummary({
</span> </span>
</div> </div>
{summaryText && status === "done" && ( {summaryText && (
<div <div
className={cn( className={cn(
"border-t p-2", "border-t p-2",
@ -91,7 +91,7 @@ export function TaskSummary({
)} )}
> >
<FileText className="h-3 w-3" /> <FileText className="h-3 w-3" />
{showSummary ? "Hide summary" : "Show summary"} {showSummary ? "Hide reflection" : "Show reflection"}
</button> </button>
{showSummary && ( {showSummary && (
<BasicMarkdownText <BasicMarkdownText

View file

@ -50,6 +50,7 @@ import { z } from "zod";
import { isAIMessageSDK, isToolMessageSDK } from "@/lib/langchain-messages"; import { isAIMessageSDK, isToolMessageSDK } from "@/lib/langchain-messages";
import { useStream } from "@langchain/langgraph-sdk/react"; import { useStream } from "@langchain/langgraph-sdk/react";
import { ConversationHistorySummary } from "@/components/gen-ui/conversation-summary"; import { ConversationHistorySummary } from "@/components/gen-ui/conversation-summary";
import { getMessageContentString } from "@open-swe/shared/messages";
// Used only for Zod type inference. // Used only for Zod type inference.
const dummyRepo = { owner: "dummy", repo: "dummy" }; const dummyRepo = { owner: "dummy", repo: "dummy" };
@ -359,13 +360,16 @@ export function AssistantMessage({
// Check if this is a conversation history summary message // Check if this is a conversation history summary message
if (conversationHistorySummaryToolCall && aiToolCalls.length === 1) { if (conversationHistorySummaryToolCall && aiToolCalls.length === 1) {
const args = const correspondingToolResult = toolResults.find(
conversationHistorySummaryToolCall.args as ConversationHistorySummaryToolArgs; (tr) => tr && tr.tool_call_id === conversationHistorySummaryToolCall.id,
);
return ( return (
<div className="flex flex-col gap-4"> <div className="flex flex-col gap-4">
<ConversationHistorySummary <ConversationHistorySummary
summary={args.conversation_history_summary} summary={getMessageContentString(
correspondingToolResult?.content ?? "",
)}
/> />
</div> </div>
); );

View file

@ -410,11 +410,11 @@ export function createWriteTechnicalNotesToolFields() {
export function createConversationHistorySummaryToolFields() { export function createConversationHistorySummaryToolFields() {
const conversationHistorySummarySchema = z.object({ const conversationHistorySummarySchema = z.object({
conversation_history_summary: z.string(), reasoning: z.string(),
}); });
return { return {
name: "conversation_history_summary", name: "summarize_conversation_history",
description: description:
"<not used as an actual tool call. only used as shared types between the client and agent>", "<not used as an actual tool call. only used as shared types between the client and agent>",
schema: conversationHistorySummarySchema, schema: conversationHistorySummarySchema,