Skip to content

Commit 3134ae1

Browse files
authored
Fix chat name generation by filtering user messages (#1555)
Fixes OPS-2963. ## Additional Notes - We were sending the whole chat history (including user, assistant and reasoning) messages which made the LLMs misbehave in several ways like schema errors, hallucinations and outputting part of reasoning messages as chat name - Now we only send user messages to the LLM to generate a name and it works. I've tested with Vertex (Claude and Gemini), GPT (o1, 4o, 5, 5-mini, 5-nano) ### Before and After the change with Vertex provider: - LLM responded with a sentence from reasoning messages. <img width="1080" height="471" alt="Screenshot 2025-11-03 at 6 14 23 PM" src="https://github.com/user-attachments/assets/da89cf8c-87cb-43f5-a329-0657403bd3f1" /> <img width="1080" height="471" alt="Screenshot 2025-11-03 at 6 15 05 PM" src="https://github.com/user-attachments/assets/45b8b9d2-7ad1-4fc1-8ac2-b38f510e9c4c" /> ### With GPT-5 Nano (Before and after the change): <img width="1080" height="471" alt="Screenshot 2025-11-03 at 6 17 11 PM" src="https://github.com/user-attachments/assets/5ac76e82-7796-4be3-ac24-b2957414fb27" /> <img width="1080" height="471" alt="Screenshot 2025-11-03 at 6 17 55 PM" src="https://github.com/user-attachments/assets/029f316b-b11e-4b65-8b2a-f4e6083380ff" />
1 parent 1a845e3 commit 3134ae1

File tree

2 files changed

+170
-7
lines changed

2 files changed

+170
-7
lines changed

packages/server/api/src/app/ai/chat/ai-mcp-chat.controller.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {
2020
UpdateChatModelRequest,
2121
UpdateChatModelResponse,
2222
} from '@openops/shared';
23-
import { ModelMessage } from 'ai';
23+
import { ModelMessage, UserModelMessage } from 'ai';
2424
import { FastifyReply } from 'fastify';
2525
import { StatusCodes } from 'http-status-codes';
2626
import {
@@ -243,7 +243,19 @@ export const aiMCPChatController: FastifyPluginAsyncTypebox = async (app) => {
243243
return await reply.code(200).send({ chatName: DEFAULT_CHAT_NAME });
244244
}
245245

246-
const rawChatName = await generateChatName(chatHistory, projectId);
246+
const userMessages = chatHistory.filter(
247+
(msg): msg is UserModelMessage =>
248+
msg &&
249+
typeof msg === 'object' &&
250+
'role' in msg &&
251+
msg.role === 'user',
252+
);
253+
254+
if (userMessages.length === 0) {
255+
return await reply.code(200).send({ chatName: DEFAULT_CHAT_NAME });
256+
}
257+
258+
const rawChatName = await generateChatName(userMessages, projectId);
247259
const chatName = rawChatName.trim() || DEFAULT_CHAT_NAME;
248260

249261
await updateChatName(chatId, userId, projectId, chatName);

packages/server/api/test/unit/ai/ai-mcp-chat.controller.test.ts

Lines changed: 156 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -376,9 +376,19 @@ describe('AI MCP Chat Controller - Tool Service Interactions', () => {
376376
expect(mockReply.send).toHaveBeenCalledWith({
377377
chatName: 'AWS Cost Optimization',
378378
});
379+
expect(generateChatName).toHaveBeenCalledWith(
380+
[{ role: 'user', content: 'How do I optimize AWS costs?' }],
381+
'test-project-id',
382+
);
383+
expect(updateChatName).toHaveBeenCalledWith(
384+
'test-chat-id',
385+
'test-user-id',
386+
'test-project-id',
387+
'AWS Cost Optimization',
388+
);
379389
});
380390

381-
it('should return "New Chat" for empty messages', async () => {
391+
it('should return "New Chat" for empty chatHistory', async () => {
382392
(getConversation as jest.Mock).mockResolvedValue({ chatHistory: [] });
383393

384394
await postHandler(
@@ -387,10 +397,102 @@ describe('AI MCP Chat Controller - Tool Service Interactions', () => {
387397
);
388398

389399
expect(mockReply.code).toHaveBeenCalledWith(200);
390-
expect(mockReply.send).toHaveBeenCalledWith({ chatName: 'New Chat' });
400+
expect(mockReply.send).toHaveBeenCalledWith({
401+
chatName: 'New Chat',
402+
});
403+
expect(generateChatName).not.toHaveBeenCalled();
404+
expect(updateChatName).not.toHaveBeenCalled();
405+
});
406+
407+
it('should return "New Chat" when chatHistory has no user messages', async () => {
408+
(getConversation as jest.Mock).mockResolvedValue({
409+
chatHistory: [
410+
{ role: 'assistant', content: 'Some response' },
411+
{ role: 'system', content: 'System message' },
412+
{
413+
role: 'tool',
414+
content: [{ type: 'tool-result', toolCallId: '123' }],
415+
},
416+
],
417+
});
418+
419+
await postHandler(
420+
{ ...mockRequest, body: { chatId: 'test-chat-id' } } as FastifyRequest,
421+
mockReply as unknown as FastifyReply,
422+
);
423+
424+
expect(mockReply.code).toHaveBeenCalledWith(200);
425+
expect(mockReply.send).toHaveBeenCalledWith({
426+
chatName: 'New Chat',
427+
});
428+
expect(generateChatName).not.toHaveBeenCalled();
429+
expect(updateChatName).not.toHaveBeenCalled();
430+
});
431+
432+
it('should filter out invalid message objects without role property', async () => {
433+
(getConversation as jest.Mock).mockResolvedValue({
434+
chatHistory: [
435+
{ role: 'user', content: 'Valid message' },
436+
{ content: 'Invalid message object' },
437+
{ type: 'reasoning', text: 'Some reasoning' },
438+
{ role: 'user', content: 'Another valid message' },
439+
{ role: 'assistant', content: 'Response' },
440+
],
441+
});
442+
(generateChatName as jest.Mock).mockResolvedValue('Filtered Name');
443+
444+
await postHandler(
445+
{ ...mockRequest, body: { chatId: 'test-chat-id' } } as FastifyRequest,
446+
mockReply as unknown as FastifyReply,
447+
);
448+
449+
expect(mockReply.code).toHaveBeenCalledWith(200);
450+
expect(mockReply.send).toHaveBeenCalledWith({
451+
chatName: 'Filtered Name',
452+
});
453+
expect(generateChatName).toHaveBeenCalledWith(
454+
[
455+
{ role: 'user', content: 'Valid message' },
456+
{ role: 'user', content: 'Another valid message' },
457+
],
458+
'test-project-id',
459+
);
460+
});
461+
462+
it('should call generateChatName with all user messages (filtered)', async () => {
463+
const mockChatHistory = [
464+
{ role: 'user', content: 'First question' },
465+
{ role: 'assistant', content: 'First answer' },
466+
{ role: 'user', content: 'Second question' },
467+
{ role: 'assistant', content: 'Second answer' },
468+
{ role: 'user', content: 'Third question' },
469+
];
470+
471+
(getConversation as jest.Mock).mockResolvedValue({
472+
chatHistory: mockChatHistory,
473+
});
474+
(generateChatName as jest.Mock).mockResolvedValue('Multi-Question Chat');
475+
476+
await postHandler(
477+
{ ...mockRequest, body: { chatId: 'test-chat-id' } } as FastifyRequest,
478+
mockReply as unknown as FastifyReply,
479+
);
480+
481+
expect(generateChatName).toHaveBeenCalledWith(
482+
[
483+
{ role: 'user', content: 'First question' },
484+
{ role: 'user', content: 'Second question' },
485+
{ role: 'user', content: 'Third question' },
486+
],
487+
'test-project-id',
488+
);
489+
expect(mockReply.code).toHaveBeenCalledWith(200);
490+
expect(mockReply.send).toHaveBeenCalledWith({
491+
chatName: 'Multi-Question Chat',
492+
});
391493
});
392494

393-
it('should return "New Chat" if LLM returns empty', async () => {
495+
it('should return "New Chat" if LLM returns empty/whitespace', async () => {
394496
(getConversation as jest.Mock).mockResolvedValue({
395497
chatHistory: [
396498
{ role: 'user', content: 'Hello?' },
@@ -405,7 +507,32 @@ describe('AI MCP Chat Controller - Tool Service Interactions', () => {
405507
);
406508

407509
expect(mockReply.code).toHaveBeenCalledWith(200);
408-
expect(mockReply.send).toHaveBeenCalledWith({ chatName: 'New Chat' });
510+
expect(mockReply.send).toHaveBeenCalledWith({
511+
chatName: 'New Chat',
512+
});
513+
expect(updateChatName).toHaveBeenCalledWith(
514+
'test-chat-id',
515+
'test-user-id',
516+
'test-project-id',
517+
'New Chat',
518+
);
519+
});
520+
521+
it('should return "New Chat" if LLM returns empty string', async () => {
522+
(getConversation as jest.Mock).mockResolvedValue({
523+
chatHistory: [{ role: 'user', content: 'Test question' }],
524+
});
525+
(generateChatName as jest.Mock).mockResolvedValue('');
526+
527+
await postHandler(
528+
{ ...mockRequest, body: { chatId: 'test-chat-id' } } as FastifyRequest,
529+
mockReply as unknown as FastifyReply,
530+
);
531+
532+
expect(mockReply.code).toHaveBeenCalledWith(200);
533+
expect(mockReply.send).toHaveBeenCalledWith({
534+
chatName: 'New Chat',
535+
});
409536
});
410537

411538
it('should persist chatName in chat context', async () => {
@@ -418,17 +545,22 @@ describe('AI MCP Chat Controller - Tool Service Interactions', () => {
418545
(generateChatName as jest.Mock).mockResolvedValue(
419546
'OpenOps Platform Overview',
420547
);
421-
const postHandler = handlers['/chat-name'];
548+
422549
await postHandler(
423550
{ ...mockRequest, body: { chatId: 'test-chat-id' } } as FastifyRequest,
424551
mockReply as unknown as FastifyReply,
425552
);
553+
426554
expect(updateChatName).toHaveBeenCalledWith(
427555
'test-chat-id',
428556
'test-user-id',
429557
'test-project-id',
430558
'OpenOps Platform Overview',
431559
);
560+
expect(mockReply.code).toHaveBeenCalledWith(200);
561+
expect(mockReply.send).toHaveBeenCalledWith({
562+
chatName: 'OpenOps Platform Overview',
563+
});
432564
});
433565

434566
it('should handle errors gracefully', async () => {
@@ -444,6 +576,25 @@ describe('AI MCP Chat Controller - Tool Service Interactions', () => {
444576
expect.objectContaining({ message: 'Internal server error' }),
445577
);
446578
});
579+
580+
it('should handle generateChatName errors gracefully', async () => {
581+
(getConversation as jest.Mock).mockResolvedValue({
582+
chatHistory: [{ role: 'user', content: 'Test question' }],
583+
});
584+
(generateChatName as jest.Mock).mockRejectedValue(
585+
new Error('LLM API error'),
586+
);
587+
588+
await postHandler(
589+
{ ...mockRequest, body: { chatId: 'test-chat-id' } } as FastifyRequest,
590+
mockReply as unknown as FastifyReply,
591+
);
592+
593+
expect(mockReply.code).toHaveBeenCalledWith(500);
594+
expect(mockReply.send).toHaveBeenCalledWith(
595+
expect.objectContaining({ message: 'Internal server error' }),
596+
);
597+
});
447598
});
448599

449600
describe('GET /all-chats (list all chats)', () => {

0 commit comments

Comments
 (0)