From 9bb96c212dda7071c2c62c2e874619a0f65a85e3 Mon Sep 17 00:00:00 2001 From: chenxi <16267732+chenxi-null@users.noreply.github.com> Date: Mon, 21 Apr 2025 09:04:47 +0800 Subject: [PATCH] fix: deepseek-reasoner does not support successive user or assistant messages in MCP scenario (#5112) * fix: deepseek-reasoner does not support successive user or assistant messages in MCP scenario. * fix: @ts-ignore --- .../providers/AiProvider/OpenAIProvider.ts | 11 +- .../src/services/ModelMessageService.ts | 45 +++--- .../__tests__/ModelMessageService.test.ts | 148 ++++++++++++++---- 3 files changed, 148 insertions(+), 56 deletions(-) diff --git a/src/renderer/src/providers/AiProvider/OpenAIProvider.ts b/src/renderer/src/providers/AiProvider/OpenAIProvider.ts index b4a15933..52ebe8bd 100644 --- a/src/renderer/src/providers/AiProvider/OpenAIProvider.ts +++ b/src/renderer/src/providers/AiProvider/OpenAIProvider.ts @@ -408,6 +408,9 @@ export default class OpenAIProvider extends BaseProvider { } as ChatCompletionMessageParam) toolResults.forEach((ts) => reqMessages.push(ts as ChatCompletionMessageParam)) + console.debug('[tool] reqMessages before processing', model.id, reqMessages) + reqMessages = processReqMessages(model, reqMessages) + console.debug('[tool] reqMessages', model.id, reqMessages) const newStream = await this.sdk.chat.completions // @ts-ignore key is not typed .create( @@ -506,9 +509,9 @@ export default class OpenAIProvider extends BaseProvider { await processToolUses(content, idx) } - // console.log('[before] reqMessages', reqMessages) + console.debug('[completions] reqMessages before processing', model.id, reqMessages) reqMessages = processReqMessages(model, reqMessages) - // console.log('[after] reqMessages', reqMessages) + console.debug('[completions] reqMessages', model.id, reqMessages) const stream = await this.sdk.chat.completions // @ts-ignore key is not typed .create( @@ -571,6 +574,7 @@ export default class OpenAIProvider extends BaseProvider { await this.checkIsCopilot() + console.debug('[translate] reqMessages', model.id, messages) // @ts-ignore key is not typed const response = await this.sdk.chat.completions.create({ model: model.id, @@ -646,6 +650,7 @@ export default class OpenAIProvider extends BaseProvider { await this.checkIsCopilot() + console.debug('[summaries] reqMessages', model.id, [systemMessage, userMessage]) // @ts-ignore key is not typed const response = await this.sdk.chat.completions.create({ model: model.id, @@ -680,6 +685,7 @@ export default class OpenAIProvider extends BaseProvider { role: 'user', content: messages.map((m) => m.content).join('\n') } + console.debug('[summaryForSearch] reqMessages', model.id, [systemMessage, userMessage]) // @ts-ignore key is not typed const response = await this.sdk.chat.completions.create( { @@ -771,6 +777,7 @@ export default class OpenAIProvider extends BaseProvider { try { await this.checkIsCopilot() + console.debug('[checkModel] body', model.id, body) const response = await this.sdk.chat.completions.create(body as ChatCompletionCreateParamsNonStreaming) return { diff --git a/src/renderer/src/services/ModelMessageService.ts b/src/renderer/src/services/ModelMessageService.ts index 9ce18859..0ef3001f 100644 --- a/src/renderer/src/services/ModelMessageService.ts +++ b/src/renderer/src/services/ModelMessageService.ts @@ -9,40 +9,33 @@ export function processReqMessages( return reqMessages } - return mergeSameRoleMessages(reqMessages) + return interleaveUserAndAssistantMessages(reqMessages) } function needStrictlyInterleaveUserAndAssistantMessages(model: Model) { return model.id === 'deepseek-reasoner' } -/** - * Merge successive messages with the same role - */ -function mergeSameRoleMessages(messages: ChatCompletionMessageParam[]): ChatCompletionMessageParam[] { - const split = '\n' - const processedMessages: ChatCompletionMessageParam[] = [] - let currentGroup: ChatCompletionMessageParam[] = [] - - for (const message of messages) { - if (currentGroup.length === 0 || currentGroup[0].role === message.role) { - currentGroup.push(message) - } else { - // merge the current group and add to processed messages - processedMessages.push({ - ...currentGroup[0], - content: currentGroup.map((m) => m.content).join(split) - }) - currentGroup = [message] - } +function interleaveUserAndAssistantMessages(messages: ChatCompletionMessageParam[]): ChatCompletionMessageParam[] { + if (!messages || messages.length === 0) { + return [] } - // process the last group - if (currentGroup.length > 0) { - processedMessages.push({ - ...currentGroup[0], - content: currentGroup.map((m) => m.content).join(split) - }) + const processedMessages: ChatCompletionMessageParam[] = [] + + for (let i = 0; i < messages.length; i++) { + const currentMessage = { ...messages[i] } + + if (i > 0 && currentMessage.role === messages[i - 1].role) { + // insert an empty message with the opposite role in between + const emptyMessageRole = currentMessage.role === 'user' ? 'assistant' : 'user' + processedMessages.push({ + role: emptyMessageRole, + content: '' + }) + } + + processedMessages.push(currentMessage) } return processedMessages diff --git a/src/renderer/src/services/__tests__/ModelMessageService.test.ts b/src/renderer/src/services/__tests__/ModelMessageService.test.ts index 9a2a0b38..0527e0c3 100644 --- a/src/renderer/src/services/__tests__/ModelMessageService.test.ts +++ b/src/renderer/src/services/__tests__/ModelMessageService.test.ts @@ -1,4 +1,4 @@ -import { Model } from '@renderer/types' +import type { Model } from '@renderer/types' import { ChatCompletionMessageParam } from 'openai/resources' import { describe, expect, it } from 'vitest' @@ -14,38 +14,47 @@ describe('ModelMessageService', () => { { role: 'assistant', content: 'Second answer' } ] - const createModel = (id: string): Model => ({ - id, - provider: 'test-provider', - name: id, - group: 'test-group' - }) - - it('should merge successive messages with same role for deepseek-reasoner model', () => { - const model = createModel('deepseek-reasoner') + it('should insert empty messages between consecutive same-role messages for deepseek-reasoner model', () => { + const model = { id: 'deepseek-reasoner' } as Model const result = processReqMessages(model, mockMessages) - expect(result.length).toBe(4) + expect(result.length).toBe(8) expect(result[0]).toEqual({ role: 'user', - content: 'First question\nAdditional context' + content: 'First question' }) expect(result[1]).toEqual({ role: 'assistant', - content: 'First answer\nAdditional information' + content: '' }) expect(result[2]).toEqual({ role: 'user', - content: 'Second question' + content: 'Additional context' }) expect(result[3]).toEqual({ + role: 'assistant', + content: 'First answer' + }) + expect(result[4]).toEqual({ + role: 'user', + content: '' + }) + expect(result[5]).toEqual({ + role: 'assistant', + content: 'Additional information' + }) + expect(result[6]).toEqual({ + role: 'user', + content: 'Second question' + }) + expect(result[7]).toEqual({ role: 'assistant', content: 'Second answer' }) }) - it('should not merge messages for other models', () => { - const model = createModel('gpt-4') + it('should not modify messages for other models', () => { + const model = { id: 'gpt-4' } as Model const result = processReqMessages(model, mockMessages) expect(result.length).toBe(mockMessages.length) @@ -53,7 +62,7 @@ describe('ModelMessageService', () => { }) it('should handle empty messages array', () => { - const model = createModel('deepseek-reasoner') + const model = { id: 'deepseek-reasoner' } as Model const result = processReqMessages(model, []) expect(result.length).toBe(0) @@ -61,16 +70,16 @@ describe('ModelMessageService', () => { }) it('should handle single message', () => { - const model = createModel('deepseek-reasoner') - const singleMessage = [{ role: 'user', content: 'Single message' }] - const result = processReqMessages(model, singleMessage as ChatCompletionMessageParam[]) + const model = { id: 'deepseek-reasoner' } as Model + const singleMessage: ChatCompletionMessageParam[] = [{ role: 'user', content: 'Single message' }] + const result = processReqMessages(model, singleMessage) expect(result.length).toBe(1) expect(result).toEqual(singleMessage) }) - it('should preserve other message properties when merging', () => { - const model = createModel('deepseek-reasoner') + it('should preserve other message properties when inserting empty messages', () => { + const model = { id: 'deepseek-reasoner' } as Model const messagesWithProps = [ { role: 'user', @@ -87,17 +96,26 @@ describe('ModelMessageService', () => { const result = processReqMessages(model, messagesWithProps) - expect(result.length).toBe(1) + expect(result.length).toBe(3) expect(result[0]).toEqual({ role: 'user', - content: 'First message\nSecond message', + content: 'First message', name: 'user1', function_call: { name: 'test', arguments: '{}' } }) + expect(result[1]).toEqual({ + role: 'assistant', + content: '' + }) + expect(result[2]).toEqual({ + role: 'user', + content: 'Second message', + name: 'user1' + }) }) it('should handle alternating roles correctly', () => { - const model = createModel('deepseek-reasoner') + const model = { id: 'deepseek-reasoner' } as Model const alternatingMessages = [ { role: 'user', content: 'Q1' }, { role: 'assistant', content: 'A1' }, @@ -112,7 +130,7 @@ describe('ModelMessageService', () => { }) it('should handle messages with empty content', () => { - const model = createModel('deepseek-reasoner') + const model = { id: 'deepseek-reasoner' } as Model const messagesWithEmpty = [ { role: 'user', content: 'Q1' }, { role: 'user', content: '' }, @@ -121,10 +139,84 @@ describe('ModelMessageService', () => { const result = processReqMessages(model, messagesWithEmpty) - expect(result.length).toBe(1) + expect(result.length).toBe(5) expect(result[0]).toEqual({ role: 'user', - content: 'Q1\n\nQ2' + content: 'Q1' + }) + expect(result[1]).toEqual({ + role: 'assistant', + content: '' + }) + expect(result[2]).toEqual({ + role: 'user', + content: '' + }) + expect(result[3]).toEqual({ + role: 'assistant', + content: '' + }) + expect(result[4]).toEqual({ + role: 'user', + content: 'Q2' + }) + }) + + it('should handle specific case with consecutive user messages', () => { + const model = { id: 'deepseek-reasoner' } as Model + const messages = [ + { role: 'assistant', content: 'Initial assistant message' }, + { role: 'user', content: 'First user message' }, + { role: 'user', content: 'Second user message' } + ] as ChatCompletionMessageParam[] + + const result = processReqMessages(model, messages) + + expect(result.length).toBe(4) + expect(result[0]).toEqual({ + role: 'assistant', + content: 'Initial assistant message' + }) + expect(result[1]).toEqual({ + role: 'user', + content: 'First user message' + }) + expect(result[2]).toEqual({ + role: 'assistant', + content: '' + }) + expect(result[3]).toEqual({ + role: 'user', + content: 'Second user message' + }) + }) + + it('should handle specific case with consecutive assistant messages', () => { + const model = { id: 'deepseek-reasoner' } as Model + const messages = [ + { role: 'user', content: 'Initial user message' }, + { role: 'assistant', content: 'First assistant message' }, + { role: 'assistant', content: 'Second assistant message' } + ] as ChatCompletionMessageParam[] + + const result = processReqMessages(model, messages) + + expect(result.length).toBe(4) + expect(result[0]).toEqual({ + role: 'user', + content: 'Initial user message' + }) + expect(result[1]).toEqual({ + role: 'assistant', + content: 'First assistant message' + }) + expect(result[2]).toEqual({ + role: 'user', + content: '' + }) + expect(result[3]).toEqual({ + role: 'assistant', + content: 'Second assistant message' }) }) })