mirror of
https://github.com/n8n-io/n8n.git
synced 2026-07-28 03:24:59 +02:00
fix: Extend orphan message cleanup to handle trailing orphans from memory trimming (#34385)
Co-authored-by: Prathamesh Hukkeri <prathamesh04@users.noreply.github.com>
This commit is contained in:
parent
7d231684d1
commit
96264a063b
|
|
@ -155,39 +155,70 @@ export function buildToolContext(steps: ToolCallData[]): string {
|
|||
}
|
||||
|
||||
/**
|
||||
* Removes orphaned ToolMessages and AIMessages with tool_calls from the start of chat history.
|
||||
* Removes orphaned ToolMessages and AIMessages with tool_calls from the start and end of chat history.
|
||||
* This happens when memory trimming cuts messages mid-turn, leaving incomplete tool call sequences.
|
||||
*
|
||||
* @param chatHistory - Array of messages to clean up
|
||||
* @returns Cleaned array with orphaned messages removed from the start
|
||||
* @returns Cleaned array with orphaned messages removed from both ends
|
||||
*/
|
||||
function cleanupOrphanedMessages(chatHistory: BaseMessage[]): BaseMessage[] {
|
||||
export function cleanupOrphanedMessages(chatHistory: BaseMessage[]): BaseMessage[] {
|
||||
const result = [...chatHistory];
|
||||
|
||||
// Clean up orphaned messages from the start
|
||||
let changed = true;
|
||||
while (changed && chatHistory.length > 0) {
|
||||
while (changed && result.length > 0) {
|
||||
changed = false;
|
||||
|
||||
// Remove orphaned ToolMessages at the start
|
||||
while (chatHistory.length > 0 && chatHistory[0] instanceof ToolMessage) {
|
||||
chatHistory.shift();
|
||||
while (result.length > 0 && result[0] instanceof ToolMessage) {
|
||||
result.shift();
|
||||
changed = true;
|
||||
}
|
||||
|
||||
// Remove AIMessages with tool_calls if they don't have following ToolMessages
|
||||
if (chatHistory.length > 0) {
|
||||
const firstMessage = chatHistory[0];
|
||||
if (result.length > 0) {
|
||||
const firstMessage = result[0];
|
||||
const hasOrphanedAIMessage =
|
||||
firstMessage instanceof AIMessage &&
|
||||
(firstMessage.tool_calls?.length ?? 0) > 0 &&
|
||||
!(chatHistory[1] instanceof ToolMessage);
|
||||
!(result[1] instanceof ToolMessage);
|
||||
|
||||
if (hasOrphanedAIMessage) {
|
||||
chatHistory.shift();
|
||||
result.shift();
|
||||
changed = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return chatHistory;
|
||||
// Clean up orphaned messages from the end
|
||||
changed = true;
|
||||
while (changed && result.length > 0) {
|
||||
changed = false;
|
||||
const lastIdx = result.length - 1;
|
||||
const lastMessage = result[lastIdx];
|
||||
|
||||
if (lastMessage instanceof ToolMessage) {
|
||||
// Scan backwards past all consecutive trailing ToolMessages
|
||||
let precedingIdx = lastIdx - 1;
|
||||
while (precedingIdx >= 0 && result[precedingIdx] instanceof ToolMessage) {
|
||||
precedingIdx--;
|
||||
}
|
||||
// The ToolMessage group is orphaned if not preceded by an AIMessage with tool_calls
|
||||
const precedingMessage = precedingIdx >= 0 ? result[precedingIdx] : undefined;
|
||||
const isPaired =
|
||||
precedingMessage instanceof AIMessage && (precedingMessage.tool_calls?.length ?? 0) > 0;
|
||||
if (!isPaired) {
|
||||
result.pop();
|
||||
changed = true;
|
||||
}
|
||||
} else if (lastMessage instanceof AIMessage && (lastMessage.tool_calls?.length ?? 0) > 0) {
|
||||
// An AIMessage with tool_calls at the end is orphaned (no ToolMessage follows)
|
||||
result.pop();
|
||||
changed = true;
|
||||
}
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ import {
|
|||
buildToolContext,
|
||||
extractToolCallId,
|
||||
buildMessagesFromSteps,
|
||||
cleanupOrphanedMessages,
|
||||
} from '../memoryManagement';
|
||||
import type { ToolCallData } from '../types';
|
||||
|
||||
|
|
@ -217,6 +218,180 @@ describe('memoryManagement', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('cleanupOrphanedMessages', () => {
|
||||
it('should return empty array when all messages are orphans', () => {
|
||||
const chatHistory = [
|
||||
new ToolMessage({ content: 'Result', tool_call_id: 'id-1', name: 'tool' }),
|
||||
new AIMessage({
|
||||
content: 'Call',
|
||||
tool_calls: [{ id: 'call-1', name: 'tool', args: {}, type: 'tool_call' as const }],
|
||||
}),
|
||||
];
|
||||
|
||||
const result = cleanupOrphanedMessages(chatHistory);
|
||||
|
||||
expect(result).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should remove orphaned AIMessage with tool_calls at end (missing trailing ToolMessage)', () => {
|
||||
const chatHistory = [
|
||||
new HumanMessage('Question'),
|
||||
new AIMessage('Answer'),
|
||||
new AIMessage({
|
||||
content: 'Calling tool',
|
||||
tool_calls: [{ id: 'call-1', name: 'tool', args: {}, type: 'tool_call' as const }],
|
||||
}),
|
||||
];
|
||||
|
||||
const result = cleanupOrphanedMessages(chatHistory);
|
||||
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result[0]).toBeInstanceOf(HumanMessage);
|
||||
expect(result[1]).toBeInstanceOf(AIMessage);
|
||||
});
|
||||
|
||||
it('should remove orphaned ToolMessages at end (no preceding AIMessage with tool_calls)', () => {
|
||||
const chatHistory = [
|
||||
new HumanMessage('Question'),
|
||||
new AIMessage('Answer'),
|
||||
new ToolMessage({ content: 'Result', tool_call_id: 'id-1', name: 'tool' }),
|
||||
];
|
||||
|
||||
const result = cleanupOrphanedMessages(chatHistory);
|
||||
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result[0]).toBeInstanceOf(HumanMessage);
|
||||
expect(result[1]).toBeInstanceOf(AIMessage);
|
||||
});
|
||||
|
||||
it('should preserve valid tool call pair at end', () => {
|
||||
const chatHistory = [
|
||||
new HumanMessage('Question'),
|
||||
new AIMessage('Answer'),
|
||||
new AIMessage({
|
||||
content: 'Calling tool',
|
||||
tool_calls: [{ id: 'call-1', name: 'tool', args: {}, type: 'tool_call' as const }],
|
||||
}),
|
||||
new ToolMessage({ content: 'Result', tool_call_id: 'call-1', name: 'tool' }),
|
||||
];
|
||||
|
||||
const result = cleanupOrphanedMessages(chatHistory);
|
||||
|
||||
expect(result).toHaveLength(4);
|
||||
});
|
||||
|
||||
it('should handle trimming that splits mid-turn at end', () => {
|
||||
// Simulates slice that kept AIMessage(tool_calls) but lost its ToolMessage
|
||||
const chatHistory = [
|
||||
new HumanMessage('Hello'),
|
||||
new AIMessage('Response'),
|
||||
new HumanMessage('Use tool'),
|
||||
new AIMessage({
|
||||
content: 'Calling tool',
|
||||
tool_calls: [{ id: 'call-1', name: 'tool', args: {}, type: 'tool_call' as const }],
|
||||
}),
|
||||
];
|
||||
|
||||
const result = cleanupOrphanedMessages(chatHistory);
|
||||
|
||||
expect(result).toHaveLength(3);
|
||||
expect(result[0]).toBeInstanceOf(HumanMessage);
|
||||
expect(result[0].content).toBe('Hello');
|
||||
expect(result[1]).toBeInstanceOf(AIMessage);
|
||||
expect(result[2]).toBeInstanceOf(HumanMessage);
|
||||
expect(result[2].content).toBe('Use tool');
|
||||
});
|
||||
|
||||
it('should handle trimming that splits mid-turn at start', () => {
|
||||
// Simulates slice that lost AIMessage but kept ToolMessage
|
||||
const chatHistory = [
|
||||
new ToolMessage({ content: 'Result', tool_call_id: 'id-1', name: 'tool' }),
|
||||
new HumanMessage('Next question'),
|
||||
new AIMessage('Answer'),
|
||||
];
|
||||
|
||||
const result = cleanupOrphanedMessages(chatHistory);
|
||||
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result[0]).toBeInstanceOf(HumanMessage);
|
||||
expect(result[1]).toBeInstanceOf(AIMessage);
|
||||
});
|
||||
|
||||
it('should preserve complete tool call sequences', () => {
|
||||
const chatHistory = [
|
||||
new HumanMessage('Hello'),
|
||||
new AIMessage({
|
||||
content: 'Calling tool',
|
||||
tool_calls: [{ id: 'call-1', name: 'tool', args: {}, type: 'tool_call' as const }],
|
||||
}),
|
||||
new ToolMessage({ content: 'Result', tool_call_id: 'call-1', name: 'tool' }),
|
||||
new AIMessage('Done'),
|
||||
];
|
||||
|
||||
const result = cleanupOrphanedMessages(chatHistory);
|
||||
|
||||
expect(result).toHaveLength(4);
|
||||
expect(result[0]).toBeInstanceOf(HumanMessage);
|
||||
expect(result[1]).toBeInstanceOf(AIMessage);
|
||||
expect(result[2]).toBeInstanceOf(ToolMessage);
|
||||
expect(result[3]).toBeInstanceOf(AIMessage);
|
||||
});
|
||||
|
||||
it('should not mutate the original array', () => {
|
||||
const chatHistory = [
|
||||
new ToolMessage({ content: 'Result', tool_call_id: 'id-1', name: 'tool' }),
|
||||
new HumanMessage('Question'),
|
||||
];
|
||||
|
||||
const originalLength = chatHistory.length;
|
||||
cleanupOrphanedMessages(chatHistory);
|
||||
|
||||
expect(chatHistory).toHaveLength(originalLength);
|
||||
});
|
||||
|
||||
it('should preserve parallel tool call results at end', () => {
|
||||
// Parallel tool calls: one AIMessage with multiple tool_calls, followed by multiple ToolMessages
|
||||
const aiMessage = new AIMessage({
|
||||
content: 'Calling tools',
|
||||
tool_calls: [
|
||||
{ id: 'call-1', name: 'tool1', args: {}, type: 'tool_call' as const },
|
||||
{ id: 'call-2', name: 'tool2', args: {}, type: 'tool_call' as const },
|
||||
],
|
||||
});
|
||||
|
||||
const chatHistory = [
|
||||
new HumanMessage('Question'),
|
||||
aiMessage,
|
||||
new ToolMessage({ content: 'Result 1', tool_call_id: 'call-1', name: 'tool1' }),
|
||||
new ToolMessage({ content: 'Result 2', tool_call_id: 'call-2', name: 'tool2' }),
|
||||
];
|
||||
|
||||
const result = cleanupOrphanedMessages(chatHistory);
|
||||
|
||||
expect(result).toHaveLength(4);
|
||||
expect(result[0]).toBeInstanceOf(HumanMessage);
|
||||
expect(result[1]).toBe(aiMessage);
|
||||
expect(result[2]).toBeInstanceOf(ToolMessage);
|
||||
expect(result[3]).toBeInstanceOf(ToolMessage);
|
||||
});
|
||||
|
||||
it('should remove orphaned parallel tool results when AIMessage is missing', () => {
|
||||
// Multiple ToolMessages at end but no preceding AIMessage with tool_calls
|
||||
const chatHistory = [
|
||||
new HumanMessage('Question'),
|
||||
new AIMessage('Answer'),
|
||||
new ToolMessage({ content: 'Result 1', tool_call_id: 'call-1', name: 'tool1' }),
|
||||
new ToolMessage({ content: 'Result 2', tool_call_id: 'call-2', name: 'tool2' }),
|
||||
];
|
||||
|
||||
const result = cleanupOrphanedMessages(chatHistory);
|
||||
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result[0]).toBeInstanceOf(HumanMessage);
|
||||
expect(result[1]).toBeInstanceOf(AIMessage);
|
||||
});
|
||||
});
|
||||
|
||||
describe('saveToMemory', () => {
|
||||
it('should save conversation to memory', async () => {
|
||||
const input = 'What is 2+2?';
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user