diff --git a/web-app/src/containers/dialogs/LoadModelErrorDialog.tsx b/web-app/src/containers/dialogs/LoadModelErrorDialog.tsx index 2b7a1e7033..140aae03a1 100644 --- a/web-app/src/containers/dialogs/LoadModelErrorDialog.tsx +++ b/web-app/src/containers/dialogs/LoadModelErrorDialog.tsx @@ -52,7 +52,7 @@ export default function LoadModelErrorDialog() {
{t('common:error')} - Failed to load model + Something went wrong
diff --git a/web-app/src/hooks/useChat.ts b/web-app/src/hooks/useChat.ts index 11e12bec17..8b84b58814 100644 --- a/web-app/src/hooks/useChat.ts +++ b/web-app/src/hooks/useChat.ts @@ -247,8 +247,7 @@ export const useChat = () => { messages, currentAssistant?.instructions ) - - builder.addUserMessage(message) + if (troubleshooting) builder.addUserMessage(message) let isCompleted = false diff --git a/web-app/src/lib/__tests__/messages.test.ts b/web-app/src/lib/__tests__/messages.test.ts index dbf28fe21b..d3051f1a70 100644 --- a/web-app/src/lib/__tests__/messages.test.ts +++ b/web-app/src/lib/__tests__/messages.test.ts @@ -1,3 +1,4 @@ +import { describe, it, expect } from 'vitest' import { CompletionMessagesBuilder } from '../messages' import { ThreadMessage } from '@janhq/core' import { ChatCompletionMessageToolCall } from 'openai/resources' @@ -32,7 +33,7 @@ describe('CompletionMessagesBuilder', () => { it('should initialize with empty messages array when no system instruction', () => { const messages: ThreadMessage[] = [] const builder = new CompletionMessagesBuilder(messages) - + expect(builder.getMessages()).toEqual([]) }) @@ -40,7 +41,7 @@ describe('CompletionMessagesBuilder', () => { const messages: ThreadMessage[] = [] const systemInstruction = 'You are a helpful assistant.' const builder = new CompletionMessagesBuilder(messages, systemInstruction) - + const result = builder.getMessages() expect(result).toHaveLength(1) expect(result[0]).toEqual({ @@ -55,10 +56,10 @@ describe('CompletionMessagesBuilder', () => { createMockThreadMessage('assistant', 'Hi there', true), // has error createMockThreadMessage('user', 'How are you?', false), ] - + const builder = new CompletionMessagesBuilder(messages) const result = builder.getMessages() - + expect(result).toHaveLength(2) expect(result[0].content).toBe('Hello') expect(result[1].content).toBe('How are you?') @@ -66,26 +67,34 @@ describe('CompletionMessagesBuilder', () => { it('should normalize assistant message content', () => { const messages: ThreadMessage[] = [ - createMockThreadMessage('assistant', 'Let me think...Hello there!'), + createMockThreadMessage( + 'assistant', + 'Let me think...Hello there!' + ), ] - + const builder = new CompletionMessagesBuilder(messages) const result = builder.getMessages() - + expect(result).toHaveLength(1) expect(result[0].content).toBe('Hello there!') }) it('should preserve user message content without normalization', () => { const messages: ThreadMessage[] = [ - createMockThreadMessage('user', 'This should not be normalizedHello'), + createMockThreadMessage( + 'user', + 'This should not be normalizedHello' + ), ] - + const builder = new CompletionMessagesBuilder(messages) const result = builder.getMessages() - + expect(result).toHaveLength(1) - expect(result[0].content).toBe('This should not be normalizedHello') + expect(result[0].content).toBe( + 'This should not be normalizedHello' + ) }) it('should handle messages with empty content', () => { @@ -93,10 +102,10 @@ describe('CompletionMessagesBuilder', () => { ...createMockThreadMessage('user', ''), content: [{ type: 'text' as any, text: undefined }], } - + const builder = new CompletionMessagesBuilder([message]) const result = builder.getMessages() - + expect(result).toHaveLength(1) expect(result[0].content).toBe('.') }) @@ -104,12 +113,14 @@ describe('CompletionMessagesBuilder', () => { it('should handle messages with missing text value', () => { const message: ThreadMessage = { ...createMockThreadMessage('user', ''), - content: [{ type: 'text' as any, text: { value: '', annotations: [] } }], + content: [ + { type: 'text' as any, text: { value: '', annotations: [] } }, + ], } - + const builder = new CompletionMessagesBuilder([message]) const result = builder.getMessages() - + expect(result).toHaveLength(1) expect(result[0].content).toBe('.') }) @@ -118,9 +129,9 @@ describe('CompletionMessagesBuilder', () => { describe('addUserMessage', () => { it('should add user message to messages array', () => { const builder = new CompletionMessagesBuilder([]) - + builder.addUserMessage('Hello, how are you?') - + const result = builder.getMessages() expect(result).toHaveLength(1) expect(result[0]).toEqual({ @@ -129,23 +140,22 @@ describe('CompletionMessagesBuilder', () => { }) }) - it('should add multiple user messages', () => { + it('should not add consecutive user messages', () => { const builder = new CompletionMessagesBuilder([]) - + builder.addUserMessage('First message') builder.addUserMessage('Second message') - + const result = builder.getMessages() - expect(result).toHaveLength(2) - expect(result[0].content).toBe('First message') - expect(result[1].content).toBe('Second message') + expect(result).toHaveLength(1) + expect(result[0].content).toBe('Second message') }) it('should handle empty user message', () => { const builder = new CompletionMessagesBuilder([]) - + builder.addUserMessage('') - + const result = builder.getMessages() expect(result).toHaveLength(1) expect(result[0].content).toBe('') @@ -155,9 +165,9 @@ describe('CompletionMessagesBuilder', () => { describe('addAssistantMessage', () => { it('should add assistant message with normalized content', () => { const builder = new CompletionMessagesBuilder([]) - + builder.addAssistantMessage('Processing...Hello!') - + const result = builder.getMessages() expect(result).toHaveLength(1) expect(result[0]).toEqual({ @@ -170,9 +180,12 @@ describe('CompletionMessagesBuilder', () => { it('should add assistant message with refusal', () => { const builder = new CompletionMessagesBuilder([]) - - builder.addAssistantMessage('I cannot help with that', 'Content policy violation') - + + builder.addAssistantMessage( + 'I cannot help with that', + 'Content policy violation' + ) + const result = builder.getMessages() expect(result).toHaveLength(1) expect(result[0]).toEqual({ @@ -195,9 +208,13 @@ describe('CompletionMessagesBuilder', () => { }, }, ] - - builder.addAssistantMessage('Let me check the weather', undefined, toolCalls) - + + builder.addAssistantMessage( + 'Let me check the weather', + undefined, + toolCalls + ) + const result = builder.getMessages() expect(result).toHaveLength(1) expect(result[0]).toEqual({ @@ -220,13 +237,13 @@ describe('CompletionMessagesBuilder', () => { }, }, ] - + builder.addAssistantMessage( 'Searching...Here are the results', 'Cannot search sensitive content', toolCalls ) - + const result = builder.getMessages() expect(result).toHaveLength(1) expect(result[0]).toEqual({ @@ -241,9 +258,9 @@ describe('CompletionMessagesBuilder', () => { describe('addToolMessage', () => { it('should add tool message with correct format', () => { const builder = new CompletionMessagesBuilder([]) - + builder.addToolMessage('Weather data: 72°F', 'call_123') - + const result = builder.getMessages() expect(result).toHaveLength(1) expect(result[0]).toEqual({ @@ -255,10 +272,10 @@ describe('CompletionMessagesBuilder', () => { it('should add multiple tool messages', () => { const builder = new CompletionMessagesBuilder([]) - + builder.addToolMessage('First tool result', 'call_1') builder.addToolMessage('Second tool result', 'call_2') - + const result = builder.getMessages() expect(result).toHaveLength(2) expect(result[0].tool_call_id).toBe('call_1') @@ -267,9 +284,9 @@ describe('CompletionMessagesBuilder', () => { it('should handle empty tool content', () => { const builder = new CompletionMessagesBuilder([]) - + builder.addToolMessage('', 'call_123') - + const result = builder.getMessages() expect(result).toHaveLength(1) expect(result[0].content).toBe('') @@ -282,30 +299,32 @@ describe('CompletionMessagesBuilder', () => { const threadMessages: ThreadMessage[] = [ createMockThreadMessage('user', 'Hello'), ] - const builder = new CompletionMessagesBuilder(threadMessages, 'You are helpful') - + const builder = new CompletionMessagesBuilder( + threadMessages, + 'You are helpful' + ) + builder.addUserMessage('How are you?') builder.addAssistantMessage('I am well, thank you!') builder.addToolMessage('Tool response', 'call_123') - + const result = builder.getMessages() - expect(result).toHaveLength(5) + expect(result).toHaveLength(4) expect(result[0].role).toBe('system') expect(result[1].role).toBe('user') - expect(result[2].role).toBe('user') - expect(result[3].role).toBe('assistant') - expect(result[4].role).toBe('tool') + expect(result[2].role).toBe('assistant') + expect(result[3].role).toBe('tool') }) it('should return the same array reference (not immutable)', () => { const builder = new CompletionMessagesBuilder([]) - + builder.addUserMessage('Test message') const result1 = builder.getMessages() - + builder.addAssistantMessage('Response') const result2 = builder.getMessages() - + // Both should reference the same array and have 2 messages now expect(result1).toBe(result2) // Same reference expect(result1).toHaveLength(2) @@ -316,63 +335,75 @@ describe('CompletionMessagesBuilder', () => { describe('content normalization', () => { it('should remove thinking content from the beginning', () => { const builder = new CompletionMessagesBuilder([]) - - builder.addAssistantMessage('Let me analyze this...The answer is 42.') - + + builder.addAssistantMessage( + 'Let me analyze this...The answer is 42.' + ) + const result = builder.getMessages() expect(result[0].content).toBe('The answer is 42.') }) it('should handle nested thinking tags', () => { const builder = new CompletionMessagesBuilder([]) - - builder.addAssistantMessage('First thoughtNestedMore thinkingFinal answer') - + + builder.addAssistantMessage( + 'First thoughtNestedMore thinkingFinal answer' + ) + const result = builder.getMessages() expect(result[0].content).toBe('More thinkingFinal answer') }) it('should handle multiple thinking blocks', () => { const builder = new CompletionMessagesBuilder([]) - - builder.addAssistantMessage('FirstAnswerSecondMore content') - + + builder.addAssistantMessage( + 'FirstAnswerSecondMore content' + ) + const result = builder.getMessages() expect(result[0].content).toBe('AnswerSecondMore content') }) it('should handle content without thinking tags', () => { const builder = new CompletionMessagesBuilder([]) - + builder.addAssistantMessage('Just a normal response') - + const result = builder.getMessages() expect(result[0].content).toBe('Just a normal response') }) it('should handle empty content after removing thinking', () => { const builder = new CompletionMessagesBuilder([]) - + builder.addAssistantMessage('Only thinking content') - + const result = builder.getMessages() expect(result[0].content).toBe('') }) it('should handle unclosed thinking tags', () => { const builder = new CompletionMessagesBuilder([]) - - builder.addAssistantMessage('Unclosed thinking tag... Regular content') - + + builder.addAssistantMessage( + 'Unclosed thinking tag... Regular content' + ) + const result = builder.getMessages() - expect(result[0].content).toBe('Unclosed thinking tag... Regular content') + expect(result[0].content).toBe( + 'Unclosed thinking tag... Regular content' + ) }) it('should handle thinking tags with whitespace', () => { const builder = new CompletionMessagesBuilder([]) - - builder.addAssistantMessage(' \n Some thinking \n \n Clean answer') - + + builder.addAssistantMessage( + ' \n Some thinking \n \n Clean answer' + ) + const result = builder.getMessages() expect(result[0].content).toBe('Clean answer') }) @@ -382,11 +413,17 @@ describe('CompletionMessagesBuilder', () => { it('should handle complex conversation flow', () => { const threadMessages: ThreadMessage[] = [ createMockThreadMessage('user', 'What is the weather like?'), - createMockThreadMessage('assistant', 'I need to call weather APILet me check the weather for you.'), + createMockThreadMessage( + 'assistant', + 'I need to call weather APILet me check the weather for you.' + ), ] - - const builder = new CompletionMessagesBuilder(threadMessages, 'You are a weather assistant') - + + const builder = new CompletionMessagesBuilder( + threadMessages, + 'You are a weather assistant' + ) + // Add tool call and response const toolCalls: ChatCompletionMessageToolCall[] = [ { @@ -398,13 +435,22 @@ describe('CompletionMessagesBuilder', () => { }, }, ] - - builder.addAssistantMessage('Calling weather service...', undefined, toolCalls) - builder.addToolMessage('{"temperature": 72, "condition": "sunny"}', 'call_weather') - builder.addAssistantMessage('The weather is niceThe weather is 72°F and sunny!') - + + builder.addAssistantMessage( + 'Calling weather service...', + undefined, + toolCalls + ) + builder.addToolMessage( + '{"temperature": 72, "condition": "sunny"}', + 'call_weather' + ) + builder.addAssistantMessage( + 'The weather is niceThe weather is 72°F and sunny!' + ) + const result = builder.getMessages() - + expect(result).toHaveLength(6) expect(result[0].role).toBe('system') expect(result[1].role).toBe('user') @@ -419,9 +465,9 @@ describe('CompletionMessagesBuilder', () => { it('should handle empty thread messages with system instruction', () => { const builder = new CompletionMessagesBuilder([], 'System instruction') - + const result = builder.getMessages() - + expect(result).toHaveLength(1) expect(result[0]).toEqual({ role: 'system', @@ -429,4 +475,4 @@ describe('CompletionMessagesBuilder', () => { }) }) }) -}) \ No newline at end of file +}) diff --git a/web-app/src/lib/messages.ts b/web-app/src/lib/messages.ts index 56866b8dbd..05dc0164d3 100644 --- a/web-app/src/lib/messages.ts +++ b/web-app/src/lib/messages.ts @@ -26,7 +26,7 @@ export class CompletionMessagesBuilder { content: msg.role === 'assistant' ? this.normalizeContent(msg.content[0]?.text?.value || '.') - : (msg.content[0]?.text?.value || '.'), + : msg.content[0]?.text?.value || '.', }) as ChatCompletionMessageParam ) ) @@ -37,6 +37,10 @@ export class CompletionMessagesBuilder { * @param content - The content of the user message. */ addUserMessage(content: string) { + // Ensure no consecutive user messages + if (this.messages[this.messages.length - 1]?.role === 'user') { + this.messages.pop() + } this.messages.push({ role: 'user', content: content, diff --git a/web-app/src/routes/threads/$threadId.tsx b/web-app/src/routes/threads/$threadId.tsx index 63f2965172..697dc897f0 100644 --- a/web-app/src/routes/threads/$threadId.tsx +++ b/web-app/src/routes/threads/$threadId.tsx @@ -39,7 +39,7 @@ function ThreadDetail() { const lastScrollTopRef = useRef(0) const { currentThreadId, setCurrentThreadId } = useThreads() const { setCurrentAssistant, assistants } = useAssistant() - const { setMessages } = useMessages() + const { setMessages, deleteMessage } = useMessages() const { streamingContent } = useAppState() const { appMainViewBgColor, chatWidth } = useAppearance() const { sendMessage } = useChat() @@ -221,8 +221,23 @@ function ThreadDetail() { // used when there is a sent/added user message and no assistant message (error or manual deletion) const generateAIResponse = () => { const latestUserMessage = messages[messages.length - 1] - if (latestUserMessage?.content?.[0]?.text?.value) { + if ( + latestUserMessage?.content?.[0]?.text?.value && + latestUserMessage.role === 'user' + ) { sendMessage(latestUserMessage.content[0].text.value, false) + } else if (latestUserMessage?.metadata?.tool_calls) { + // Only regenerate assistant message is allowed + const threadMessages = [...messages] + let toSendMessage = threadMessages.pop() + while (toSendMessage && toSendMessage?.role !== 'user') { + deleteMessage(toSendMessage.thread_id, toSendMessage.id ?? '') + toSendMessage = threadMessages.pop() + } + if (toSendMessage) { + deleteMessage(toSendMessage.thread_id, toSendMessage.id ?? '') + sendMessage(toSendMessage.content?.[0]?.text?.value || '') + } } } @@ -232,7 +247,10 @@ function ThreadDetail() { const showScrollToBottomBtn = !isAtBottom && hasScrollbar const showGenerateAIResponseBtn = - messages[messages.length - 1]?.role === 'user' && !streamingContent + (messages[messages.length - 1]?.role === 'user' || + (messages[messages.length - 1]?.metadata && + 'tool_calls' in (messages[messages.length - 1].metadata ?? {}))) && + !streamingContent return (