mirror of
https://github.com/lobehub/lobehub.git
synced 2026-03-27 13:29:15 +07:00
🐛 fix(store): delete message before regeneration (#11760)
🐛 fix(store): delete message before regeneration to fix LOBE-2533 When "delete and regenerate" was called, regeneration happened first which switched to a new branch. This caused the original message to no longer appear in displayMessages, so deleteMessage couldn't find the message and failed silently. Changes: - Reorder operations: delete first, then regenerate - Get parent user message ID before deletion (needed for regeneration) - Add early return if message has no parentId - Add test case to verify correct operation order Closes: LOBE-2533 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -462,6 +462,122 @@ describe('Generation Actions', () => {
|
||||
// Should complete operation
|
||||
expect(mockCompleteOperation).toHaveBeenCalledWith('test-op-id');
|
||||
});
|
||||
|
||||
it('should delete message BEFORE regeneration to prevent message not found issue (LOBE-2533)', async () => {
|
||||
// This test verifies the fix for LOBE-2533:
|
||||
// When "delete and regenerate" is called, if regeneration happens first,
|
||||
// it switches to a new branch, causing the original message to no longer
|
||||
// appear in displayMessages. Then deleteMessage cannot find the message
|
||||
// and fails silently.
|
||||
//
|
||||
// The fix: delete first, then regenerate.
|
||||
|
||||
const callOrder: string[] = [];
|
||||
|
||||
// Re-setup mock to track call order
|
||||
const { useChatStore } = await import('@/store/chat');
|
||||
vi.mocked(useChatStore.getState).mockReturnValue({
|
||||
messagesMap: {},
|
||||
operations: {},
|
||||
messageLoadingIds: [],
|
||||
cancelOperations: mockCancelOperations,
|
||||
cancelOperation: mockCancelOperation,
|
||||
deleteMessage: vi.fn().mockImplementation(() => {
|
||||
callOrder.push('deleteMessage');
|
||||
return Promise.resolve();
|
||||
}),
|
||||
switchMessageBranch: vi.fn().mockImplementation(() => {
|
||||
callOrder.push('switchMessageBranch');
|
||||
return Promise.resolve();
|
||||
}),
|
||||
startOperation: mockStartOperation,
|
||||
completeOperation: mockCompleteOperation,
|
||||
failOperation: mockFailOperation,
|
||||
internal_execAgentRuntime: vi.fn().mockImplementation(() => {
|
||||
callOrder.push('internal_execAgentRuntime');
|
||||
return Promise.resolve();
|
||||
}),
|
||||
} as any);
|
||||
|
||||
const context: ConversationContext = {
|
||||
agentId: 'session-1',
|
||||
topicId: 'topic-1',
|
||||
threadId: null,
|
||||
groupId: 'group-1',
|
||||
};
|
||||
|
||||
const store = createStore({ context });
|
||||
|
||||
// Set displayMessages and dbMessages
|
||||
act(() => {
|
||||
store.setState({
|
||||
displayMessages: [
|
||||
{ id: 'msg-1', role: 'user', content: 'Hello' },
|
||||
{ id: 'msg-2', role: 'assistant', content: 'Hi there', parentId: 'msg-1' },
|
||||
],
|
||||
dbMessages: [
|
||||
{ id: 'msg-1', role: 'user', content: 'Hello' },
|
||||
{ id: 'msg-2', role: 'assistant', content: 'Hi there', parentId: 'msg-1' },
|
||||
],
|
||||
} as any);
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await store.getState().delAndRegenerateMessage('msg-2');
|
||||
});
|
||||
|
||||
// CRITICAL: deleteMessage must be called BEFORE switchMessageBranch and internal_execAgentRuntime
|
||||
// If regeneration (which calls switchMessageBranch) happens first, the message
|
||||
// won't be found in displayMessages and deletion will fail silently.
|
||||
expect(callOrder[0]).toBe('deleteMessage');
|
||||
expect(callOrder).toContain('switchMessageBranch');
|
||||
expect(callOrder).toContain('internal_execAgentRuntime');
|
||||
|
||||
// Verify deleteMessage is called before any regeneration-related calls
|
||||
const deleteIndex = callOrder.indexOf('deleteMessage');
|
||||
const switchIndex = callOrder.indexOf('switchMessageBranch');
|
||||
const execIndex = callOrder.indexOf('internal_execAgentRuntime');
|
||||
|
||||
expect(deleteIndex).toBeLessThan(switchIndex);
|
||||
expect(deleteIndex).toBeLessThan(execIndex);
|
||||
});
|
||||
|
||||
it('should not proceed if assistant message has no parentId', async () => {
|
||||
const { useChatStore } = await import('@/store/chat');
|
||||
vi.mocked(useChatStore.getState).mockReturnValue({
|
||||
messagesMap: {},
|
||||
operations: {},
|
||||
messageLoadingIds: [],
|
||||
startOperation: mockStartOperation,
|
||||
completeOperation: mockCompleteOperation,
|
||||
deleteMessage: mockDeleteMessage,
|
||||
} as any);
|
||||
|
||||
const context: ConversationContext = {
|
||||
agentId: 'session-1',
|
||||
topicId: null,
|
||||
threadId: null,
|
||||
};
|
||||
|
||||
const store = createStore({ context });
|
||||
|
||||
// Set displayMessages with assistant message that has no parentId
|
||||
act(() => {
|
||||
store.setState({
|
||||
displayMessages: [
|
||||
{ id: 'msg-1', role: 'assistant', content: 'Hi there' }, // no parentId
|
||||
],
|
||||
} as any);
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await store.getState().delAndRegenerateMessage('msg-1');
|
||||
});
|
||||
|
||||
// Should not proceed - no operation created, no delete called
|
||||
expect(mockStartOperation).not.toHaveBeenCalled();
|
||||
expect(mockDeleteMessage).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('delAndResendThreadMessage', () => {
|
||||
|
||||
@@ -206,18 +206,29 @@ export const generationSlice: StateCreator<
|
||||
},
|
||||
|
||||
delAndRegenerateMessage: async (messageId: string) => {
|
||||
const { context } = get();
|
||||
const { context, displayMessages } = get();
|
||||
const chatStore = useChatStore.getState();
|
||||
|
||||
// Find the assistant message and get parent user message ID before deletion
|
||||
// This is needed because after deletion, we can't find the parent anymore
|
||||
const currentMessage = displayMessages.find((c) => c.id === messageId);
|
||||
if (!currentMessage) return;
|
||||
|
||||
const userId = currentMessage.parentId;
|
||||
if (!userId) return;
|
||||
|
||||
// Create operation to track context (use 'regenerate' type since this is a regenerate action)
|
||||
const { operationId } = chatStore.startOperation({
|
||||
context: { ...context, messageId },
|
||||
type: 'regenerate',
|
||||
});
|
||||
|
||||
// Regenerate first, then delete
|
||||
await get().regenerateAssistantMessage(messageId);
|
||||
// IMPORTANT: Delete first, then regenerate (LOBE-2533)
|
||||
// If we regenerate first, it switches to a new branch, causing the original
|
||||
// message to no longer appear in displayMessages. Then deleteMessage cannot
|
||||
// find the message and fails silently.
|
||||
await chatStore.deleteMessage(messageId, { operationId });
|
||||
await get().regenerateUserMessage(userId);
|
||||
chatStore.completeOperation(operationId);
|
||||
},
|
||||
|
||||
|
||||
Reference in New Issue
Block a user