From f89b036d17a730b063c850181a2cfd54dad45c27 Mon Sep 17 00:00:00 2001 From: Andrew Pareles Date: Fri, 7 Mar 2025 19:37:39 -0800 Subject: [PATCH] fix empty message order --- .../contrib/void/browser/chatThreadService.ts | 43 ++-- .../react/src/sidebar-tsx/SidebarChat.tsx | 4 +- .../contrib/void/common/llmMessageTypes.ts | 10 +- .../llmMessage/preprocessLLMMessages.ts | 189 +++++++++++------- .../llmMessage/sendLLMMessage.impl.ts | 4 +- 5 files changed, 150 insertions(+), 100 deletions(-) diff --git a/src/vs/workbench/contrib/void/browser/chatThreadService.ts b/src/vs/workbench/contrib/void/browser/chatThreadService.ts index df577509..f95fd790 100644 --- a/src/vs/workbench/contrib/void/browser/chatThreadService.ts +++ b/src/vs/workbench/contrib/void/browser/chatThreadService.ts @@ -33,19 +33,24 @@ const findLastIndex = (arr: T[], condition: (t: T) => boolean): number => { -const toLLMChatMessage = (c: ChatMessage): LLMChatMessage | null => { - if (c.role === 'user') { - return { role: c.role, content: c.content || '(empty message)' } - } - else if (c.role === 'assistant') - return { role: c.role, content: c.content || '(empty message)' } - else if (c.role === 'tool') - return { role: c.role, id: c.id, name: c.name, params: c.paramsStr, content: c.content || '(empty output)' } - else if (c.role === 'tool_request') - return null - else { - throw new Error(`Role ${(c as any).role} not recognized.`) +const toLLMChatMessages = (chatMessages: ChatMessage[]): LLMChatMessage[] => { + const llmChatMessages: LLMChatMessage[] = [] + for (const c of chatMessages) { + if (c.role === 'user') { + llmChatMessages.push({ role: c.role, content: c.content }) + } + else if (c.role === 'assistant') + llmChatMessages.push({ role: c.role, content: c.content }) + else if (c.role === 'tool') + llmChatMessages.push({ role: c.role, id: c.id, name: c.name, params: c.paramsStr, content: c.content }) + else if (c.role === 'tool_request') { + // pass + } + else { + throw new Error(`Role ${(c as any).role} not recognized.`) + } } + return llmChatMessages } @@ -92,7 +97,7 @@ export type ToolRequestApproval = { export type ChatMessage = | { role: 'user'; - content: string | null; // content displayed to the LLM on future calls - allowed to be '', will be replaced with (empty) + content: string; // content displayed to the LLM on future calls - allowed to be '', will be replaced with (empty) displayContent: string | null; // content displayed to user - allowed to be '', will be ignored selections: StagingSelectionItem[] | null; // the user's selection state: { @@ -101,8 +106,8 @@ export type ChatMessage = } } | { role: 'assistant'; - content: string | null; // content received from LLM - allowed to be '', will be replaced with (empty) - reasoning: string | null; // reasoning from the LLM, used for step-by-step thinking + content: string; // content received from LLM - allowed to be '', will be replaced with (empty) + reasoning: string; // reasoning from the LLM, used for step-by-step thinking } | ToolMessage | ToolRequestApproval @@ -307,9 +312,9 @@ class ChatThreadService extends Disposable implements IChatThreadService { // ---------- streaming ---------- - private _finishStreamingTextMessage = (threadId: string, options: { content: string, reasoning?: string }, error?: { message: string, fullError: Error | null }) => { + private _finishStreamingTextMessage = (threadId: string, options: { content: string, reasoning: string }, error?: { message: string, fullError: Error | null }) => { // add assistant's message to chat history, and clear selection - this._addMessageToThread(threadId, { role: 'assistant', content: options.content, reasoning: options.reasoning || null }) + this._addMessageToThread(threadId, { role: 'assistant', content: options.content, reasoning: options.reasoning }) this._setStreamState(threadId, { messageSoFar: undefined, reasoningSoFar: undefined, streamingToken: undefined, error }) } @@ -401,7 +406,7 @@ class ChatThreadService extends Disposable implements IChatThreadService { const awaitable = new Promise((res, rej) => { res_ = res }) // replace last userMessage with userMessageFullContent (which contains all the files too) - const messages_ = this.getCurrentThread().messages.map(m => (toLLMChatMessage(m))).filter(m => !!m) + const messages_ = toLLMChatMessages(this.getCurrentThread().messages) const lastUserMsgIdx = findLastIndex(messages_, m => m.role === 'user') if (lastUserMsgIdx === -1) throw new Error(`Void: No user message found.`) // should never be -1 @@ -430,7 +435,7 @@ class ChatThreadService extends Disposable implements IChatThreadService { this._finishStreamingTextMessage(threadId, { content: fullText, reasoning: fullReasoning }) } else { - this._addMessageToThread(threadId, { role: 'assistant', content: fullText, reasoning: fullReasoning || null }) + this._addMessageToThread(threadId, { role: 'assistant', content: fullText, reasoning: fullReasoning }) this._setStreamState(threadId, { messageSoFar: undefined, reasoningSoFar: undefined }) // clear streaming message // deal with the tool diff --git a/src/vs/workbench/contrib/void/browser/react/src/sidebar-tsx/SidebarChat.tsx b/src/vs/workbench/contrib/void/browser/react/src/sidebar-tsx/SidebarChat.tsx index 82170820..3fded5d4 100644 --- a/src/vs/workbench/contrib/void/browser/react/src/sidebar-tsx/SidebarChat.tsx +++ b/src/vs/workbench/contrib/void/browser/react/src/sidebar-tsx/SidebarChat.tsx @@ -1436,8 +1436,8 @@ export const SidebarChat = () => { : null diff --git a/src/vs/workbench/contrib/void/common/llmMessageTypes.ts b/src/vs/workbench/contrib/void/common/llmMessageTypes.ts index 6b21718d..5bac84cc 100644 --- a/src/vs/workbench/contrib/void/common/llmMessageTypes.ts +++ b/src/vs/workbench/contrib/void/common/llmMessageTypes.ts @@ -3,7 +3,6 @@ * Licensed under the Apache License, Version 2.0. See LICENSE.txt for more information. *--------------------------------------------------------------------------------------*/ -import type { ChatMessage } from '../browser/chatThreadService.js' import type { InternalToolInfo, ToolName } from '../browser/toolsService.js' import { FeatureName, OptionsOfModelSelection, ProviderName, SettingsOfProvider } from './voidSettingsTypes.js' @@ -29,7 +28,10 @@ export const getErrorMessage: (error: unknown) => string = (error) => { export type LLMChatMessage = { - role: 'system' | 'user'; + role: 'system'; + content: string; +} | { + role: 'user'; content: string; } | { role: 'assistant', @@ -51,8 +53,8 @@ export type ToolCallType = { export type OnText = (p: { fullText: string; fullReasoning: string }) => void -export type OnFinalMessage = (p: { fullText: string, toolCalls?: ToolCallType[], fullReasoning?: string }) => void // id is tool_use_id -export type OnError = (p: { message: string, fullError: Error | null }) => void +export type OnFinalMessage = (p: { fullText: string; fullReasoning: string; toolCalls?: ToolCallType[]; }) => void // id is tool_use_id +export type OnError = (p: { message: string; fullError: Error | null }) => void export type AbortRef = { current: (() => void) | null } diff --git a/src/vs/workbench/contrib/void/electron-main/llmMessage/preprocessLLMMessages.ts b/src/vs/workbench/contrib/void/electron-main/llmMessage/preprocessLLMMessages.ts index eb912711..007899cb 100644 --- a/src/vs/workbench/contrib/void/electron-main/llmMessage/preprocessLLMMessages.ts +++ b/src/vs/workbench/contrib/void/electron-main/llmMessage/preprocessLLMMessages.ts @@ -32,6 +32,9 @@ type InternalLLMChatMessage = { } +const EMPTY_MESSAGE = '(empty message)' +const EMPTY_TOOL_CONTENT = '(empty content)' + const prepareMessages_normalize = ({ messages: messages_ }: { messages: LLMChatMessage[] }) => { const messages = deepClone(messages_) const newMessages: LLMChatMessage[] = [] @@ -149,27 +152,28 @@ openai on prompting - https://platform.openai.com/docs/guides/reasoning#advice-o openai on developer system message - https://cdn.openai.com/spec/model-spec-2024-05-08.html#follow-the-chain-of-command */ +type PrepareMessagesToolsOpenAI = ( + Exclude | { + role: 'assistant', + content: string | { type: 'text'; text: string }[]; + tool_calls?: { + type: 'function'; + id: string; + function: { + name: string; + arguments: string; + } + }[] + } | { + role: 'tool', + id: string; // old val + tool_call_id: string; // new val + content: string; + } +)[] const prepareMessages_tools_openai = ({ messages }: { messages: InternalLLMChatMessage[], }) => { - const newMessages: ( - Exclude | { - role: 'assistant', - content: string | object[]; - tool_calls?: { - type: 'function'; - id: string; - function: { - name: string; - arguments: string; - } - }[] - } | { - role: 'tool', - id: string; // old val - tool_call_id: string; // new val - content: string; - } - )[] = []; + const newMessages: PrepareMessagesToolsOpenAI = []; for (let i = 0; i < messages.length; i += 1) { const currMsg = messages[i] @@ -196,7 +200,7 @@ const prepareMessages_tools_openai = ({ messages }: { messages: InternalLLMChatM newMessages.push({ role: 'tool', id: currMsg.id, - content: currMsg.content, + content: currMsg.content || EMPTY_TOOL_CONTENT, tool_call_id: currMsg.id, }) } @@ -226,33 +230,43 @@ anthropic RESPONSE (role=user): }] */ -const prepareMessages_tools_anthropic = ({ messages }: { messages: InternalLLMChatMessage[], }) => { - const newMessages: ( - Exclude | { - role: 'assistant', - content: string | ( - | { - type: 'text'; - text: string; - } - | { - type: 'tool_use'; - name: string; - input: Record; - id: string; - })[] - } | { - role: 'user', - content: string | ({ +type PrepareMessagesToolsAnthropic = ( + Exclude | { + role: 'assistant', + content: string | ( + | { type: 'text'; text: string; - } | { - type: 'tool_result'; - tool_use_id: string; - content: string; + } + | { + type: 'tool_use'; + name: string; + input: Record; + id: string; })[] - } - )[] = messages; + } | { + role: 'user', + content: string | ({ + type: 'text'; + text: string; + } | { + type: 'tool_result'; + tool_use_id: string; + content: string; + })[] + } +)[] +/* +Converts: + +assistant: ...content +tool: (id, name, params) +-> +assistant: ...content, call(name, id, params) +user: ...content, result(id, content) +*/ +const prepareMessages_tools_anthropic = ({ messages }: { messages: InternalLLMChatMessage[], }) => { + const newMessages: PrepareMessagesToolsAnthropic = messages; for (let i = 0; i < newMessages.length; i += 1) { @@ -282,8 +296,9 @@ const prepareMessages_tools_anthropic = ({ messages }: { messages: InternalLLMCh +type PrepareMessagesTools = PrepareMessagesToolsAnthropic | PrepareMessagesToolsOpenAI -const prepareMessages_tools = ({ messages, supportsTools }: { messages: InternalLLMChatMessage[], supportsTools: false | 'anthropic-style' | 'openai-style' }) => { +const prepareMessages_tools = ({ messages, supportsTools }: { messages: InternalLLMChatMessage[], supportsTools: false | 'anthropic-style' | 'openai-style' }): { messages: PrepareMessagesTools } => { if (!supportsTools) { return { messages: messages } } @@ -302,36 +317,27 @@ const prepareMessages_tools = ({ messages, supportsTools }: { messages: Internal -/* -Gemini has this, but they're openai-compat so we don't need to implement this -gemini request: -{ "role": "assistant", - "content": null, - "function_call": { - "name": "get_weather", - "arguments": { - "latitude": 48.8566, - "longitude": 2.3522 +// do this at end +const prepareMessages_noEmptyMessage = ({ messages }: { messages: PrepareMessagesTools }): { messages: PrepareMessagesTools } => { + for (const currMsg of messages) { + + if (currMsg.role === 'assistant' || currMsg.role === 'user') { + if (typeof currMsg.content === 'string') { + currMsg.content = currMsg.content || EMPTY_MESSAGE + } + else { + for (const c of currMsg.content) { + if (c.type === 'text') c.text = c.text || EMPTY_MESSAGE + else if (c.type === 'tool_use') { } + else if (c.type === 'tool_result') { } + } + } } + } + return { messages } } -gemini response: -{ "role": "assistant", - "function_response": { - "name": "get_weather", - "response": { - "temperature": "15°C", - "condition": "Cloudy" - } - } -} -*/ - - - - - // --- CHAT --- @@ -349,8 +355,8 @@ export const prepareMessages = ({ }) => { const { messages: messages1 } = prepareMessages_normalize({ messages }) const { messages: messages2, separateSystemMessageStr } = prepareMessages_systemMessage({ messages: messages1, aiInstructions, supportsSystemMessage }) - const { messages: messages4 } = prepareMessages_tools({ messages: messages2, supportsTools }) - + const { messages: messages3 } = prepareMessages_tools({ messages: messages2, supportsTools }) + const { messages: messages4 } = prepareMessages_noEmptyMessage({ messages: messages3 }) return { messages: messages4 as any, separateSystemMessageStr @@ -386,3 +392,40 @@ ${messages.prefix}` const ret = { prefix, suffix, stopTokens, maxTokens: 300 } as const return ret } + + + + + + + +/* +Gemini has this, but they're openai-compat so we don't need to implement this +gemini request: +{ "role": "assistant", + "content": null, + "function_call": { + "name": "get_weather", + "arguments": { + "latitude": 48.8566, + "longitude": 2.3522 + } + } +} + +gemini response: +{ "role": "assistant", + "function_response": { + "name": "get_weather", + "response": { + "temperature": "15°C", + "condition": "Cloudy" + } + } +} +*/ + + + + + diff --git a/src/vs/workbench/contrib/void/electron-main/llmMessage/sendLLMMessage.impl.ts b/src/vs/workbench/contrib/void/electron-main/llmMessage/sendLLMMessage.impl.ts index d93ff32a..925d5be3 100644 --- a/src/vs/workbench/contrib/void/electron-main/llmMessage/sendLLMMessage.impl.ts +++ b/src/vs/workbench/contrib/void/electron-main/llmMessage/sendLLMMessage.impl.ts @@ -140,7 +140,7 @@ const _sendOpenAICompatibleFIM = ({ messages: messages_, onFinalMessage, onError }) .then(async response => { const fullText = response.choices[0]?.text - onFinalMessage({ fullText, }); + onFinalMessage({ fullText, fullReasoning: '' }); }) .catch(error => { if (error instanceof OpenAI.APIError && error.status === 401) { onError({ message: invalidApiKeyMessage(providerName), fullError: error }); } @@ -458,7 +458,7 @@ const sendOllamaFIM = ({ messages: messages_, onFinalMessage, onError, settingsO const newText = chunk.response fullText += newText } - onFinalMessage({ fullText }) + onFinalMessage({ fullText, fullReasoning: '' }) }) // when error/fail .catch((error) => {