diff --git a/package-lock.json b/package-lock.json index d78c332..377acc0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "SnapMind", - "version": "0.5.3", + "version": "0.5.4", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "SnapMind", - "version": "0.5.3", + "version": "0.5.4", "license": "Apache-2.0", "dependencies": { "@heroui/react": "^2.8.9", diff --git a/package.json b/package.json index 161cc97..cef29a8 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "SnapMind", "private": true, - "version": "0.5.3", + "version": "0.5.4", "description": "A quick prompt launcher.", "author": "Louis Liu", "license": "Apache-2.0", diff --git a/src/components/ErrorMessage.tsx b/src/components/ErrorMessage.tsx new file mode 100644 index 0000000..afc5ed0 --- /dev/null +++ b/src/components/ErrorMessage.tsx @@ -0,0 +1,43 @@ +import { Accordion, AccordionItem } from '@heroui/react'; + +import Icon from './Icon'; +import { Message } from '@/types/chat'; +import { getTextContent } from '@/services/providers/core/messageUtils'; + +interface ErrorMessageProps { + message: Message; +} + +export default function ErrorMessage({ message }: ErrorMessageProps) { + const headline = getTextContent(message.content); + const detail = message.detail; + + return ( +
+
+ {detail ? ( + + {headline}} + indicator={} + classNames={{ + content: 'pt-0 pb-2', + }} + > + 
+                {detail}
+
+ + + ) : ( +
+ + {headline} +
+ )} +
+
+ ); +} diff --git a/src/components/__tests__/ErrorMessage.test.tsx b/src/components/__tests__/ErrorMessage.test.tsx new file mode 100644 index 0000000..c858fe0 --- /dev/null +++ b/src/components/__tests__/ErrorMessage.test.tsx @@ -0,0 +1,46 @@ +import { describe, it, expect } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import ErrorMessage from '../ErrorMessage'; +import { Message } from '@/types/chat'; + +describe('ErrorMessage', () => { + it('renders headline for role:error message', () => { + const message: Message = { + role: 'error', + content: 'Failed to get response.', + detail: 'API error: 401 Invalid API key', + }; + render(); + expect(screen.getByText('Failed to get response.')).toBeInTheDocument(); + }); + + it('keeps detail hidden until expanded', async () => { + const user = userEvent.setup(); + const message: Message = { + role: 'error', + content: 'Failed to get response.', + detail: 'API error: 401 Invalid API key', + }; + render(); + + expect(screen.queryByText(/API error: 401 Invalid API key/)).not.toBeInTheDocument(); + + const trigger = screen.getByRole('button', { name: /Failed to get response/i }); + await user.click(trigger); + + expect(screen.getByText(/API error: 401 Invalid API key/)).toBeInTheDocument(); + }); + + it('renders headline only when detail is missing', () => { + const message: Message = { + role: 'error', + content: 'Failed to get response.', + }; + render(); + expect(screen.getByText('Failed to get response.')).toBeInTheDocument(); + // No accordion button rendered when there's no detail to reveal. + expect(screen.queryByRole('button')).not.toBeInTheDocument(); + }); +}); diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index 19c9f91..1d0dc33 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -127,7 +127,9 @@ "sendMessage": "Send a message...", "thinking": "Thinking", "thought": "Thought", - "sources": "Sources" + "sources": "Sources", + "errorHeadline": "Failed to get response.", + "errorHint": "Check your API key, model, and network connection, then try again." }, "common": { "brand": "SnapMind", diff --git a/src/i18n/locales/zh-hans.json b/src/i18n/locales/zh-hans.json index 74cb9ec..36830fb 100644 --- a/src/i18n/locales/zh-hans.json +++ b/src/i18n/locales/zh-hans.json @@ -127,7 +127,9 @@ "sendMessage": "发送消息...", "thinking": "思考过程", "thought": "思考过程", - "sources": "来源" + "sources": "来源", + "errorHeadline": "获取响应失败。", + "errorHint": "请检查 API 密钥、模型和网络连接后重试。" }, "common": { "brand": "SnapMind", diff --git a/src/i18n/locales/zh-hant.json b/src/i18n/locales/zh-hant.json index 95da50d..cf6832b 100644 --- a/src/i18n/locales/zh-hant.json +++ b/src/i18n/locales/zh-hant.json @@ -127,7 +127,9 @@ "sendMessage": "傳送訊息...", "thinking": "思考過程", "thought": "思考過程", - "sources": "來源" + "sources": "來源", + "errorHeadline": "獲取回應失敗。", + "errorHint": "請檢查 API 金鑰、模型和網路連線後重試。" }, "common": { "brand": "SnapMind", diff --git a/src/pages/ChatMessage/ChatMessage.tsx b/src/pages/ChatMessage/ChatMessage.tsx index ee690f5..4790510 100644 --- a/src/pages/ChatMessage/ChatMessage.tsx +++ b/src/pages/ChatMessage/ChatMessage.tsx @@ -6,6 +6,7 @@ import rehypeHighlight from 'rehype-highlight'; import { Message, ImageContentPart } from '@/types/chat'; import ThinkingMessage from '@/components/ThinkingMessage'; +import ErrorMessage from '@/components/ErrorMessage'; import MessageWebSources from '@/components/MessageWebSources'; import { useChatMessage } from '@/hooks/useChatMessage'; @@ -35,6 +36,10 @@ export default function ChatMessage({ message }: ChatMessageProps) { const isUser = message.role === 'user'; const { thinking, main, isThinking } = useChatMessage(message.content, isUser); + if (message.role === 'error') { + return ; + } + // Define role-based style classes const bubbleBaseClasses = 'p-[10px_14px] text-base leading-normal break-words transition-colors'; diff --git a/src/pages/ChatPopup/ChatPopup.tsx b/src/pages/ChatPopup/ChatPopup.tsx index b2523d1..101abe6 100644 --- a/src/pages/ChatPopup/ChatPopup.tsx +++ b/src/pages/ChatPopup/ChatPopup.tsx @@ -150,16 +150,34 @@ export default function ChatPopup({ initialMessage }: ChatPopupProps) { // Keep unfinished message, add a new message for abort setMessages((msgs) => [...msgs, { role: 'system', content: 'Response is aborted.' }]); } else { - setMessages((msgs) => [ - ...msgs.slice(0, -1), // Remove the streaming message - { role: 'assistant', content: 'Error: Unable to get response.' }, - ]); + const errorMessage = error instanceof Error ? error.message : String(error ?? ''); + const hint = t( + 'chat.errorHint', + 'Check your API key, model, and network connection, then try again.' + ); + const detail = errorMessage ? `${errorMessage}\n\n${hint}` : hint; + setMessages((msgs) => { + // Only remove the last message if it's the empty streaming placeholder. + // (If AIService constructor threw, the placeholder was never appended and + // slice(0, -1) would delete the user's message.) + const last = msgs[msgs.length - 1]; + const isPlaceholder = last?.role === 'assistant' && last?.content === ''; + const base = isPlaceholder ? msgs.slice(0, -1) : msgs; + return [ + ...base, + { + role: 'error', + content: t('chat.errorHeadline', 'Failed to get response.'), + detail, + }, + ]; + }); } } finally { setLoading(false); } }, - [settings] + [settings, t] ); useEffect(() => { diff --git a/src/services/AIService.ts b/src/services/AIService.ts index f11d0f7..f0e353d 100644 --- a/src/services/AIService.ts +++ b/src/services/AIService.ts @@ -116,9 +116,12 @@ export class AIService { ...(options?.onWebSources ? { onWebSources: options.onWebSources } : {}), }; + // Error messages are display-only and should never be replayed to the LLM. + const cleanMessages = messages.filter((m) => m.role !== 'error'); + return { role: 'assistant', - content: await activeProvider.sendMessage(messages, providerOptions, onToken), + content: await activeProvider.sendMessage(cleanMessages, providerOptions, onToken), }; } catch (err) { loggerService.error(`[renderer] ${this.providerSetting.id} error:`, err); diff --git a/src/services/__tests__/AIService.test.ts b/src/services/__tests__/AIService.test.ts new file mode 100644 index 0000000..c659bcc --- /dev/null +++ b/src/services/__tests__/AIService.test.ts @@ -0,0 +1,92 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +import { AIService } from '../AIService'; +import type { Message } from '@/types/chat'; + +const sendMessageSpy = vi.fn(); + +vi.mock('../providers/ProviderFactory', () => ({ + default: { + createProvider: vi.fn(() => ({ + sendMessage: sendMessageSpy, + getModels: vi.fn(), + })), + }, +})); + +function makeSettings() { + return { + chat: { + defaultProvider: 'openai', + defaultModel: 'gpt-4', + temperature: 0.7, + max_tokens: 2048, + top_p: 0.95, + streamingEnabled: true, + reasoningEnabled: false, + webSearchEnabled: false, + }, + providers: [ + { + id: 'openai', + name: 'OpenAI', + apiKey: 'test-key', + host: 'https://api.openai.com/v1', + models: [{ id: 'gpt-4' }], + }, + ], + } as any; +} + +describe('AIService.sendMessageToAI', () => { + beforeEach(() => { + sendMessageSpy.mockReset(); + sendMessageSpy.mockResolvedValue('ok'); + }); + + it('filters out role:"error" messages before sending to provider', async () => { + const service = new AIService(makeSettings()); + const messages: Message[] = [ + { role: 'user', content: 'hi' }, + { role: 'assistant', content: 'hello' }, + { role: 'error', content: 'Failed to get response.', detail: 'API error: 401' }, + { role: 'user', content: 'try again' }, + ]; + + await service.sendMessageToAI(messages, () => {}); + + expect(sendMessageSpy).toHaveBeenCalledTimes(1); + const sent = sendMessageSpy.mock.calls[0][0] as Message[]; + expect(sent).toHaveLength(3); + expect(sent.every((m) => m.role !== 'error')).toBe(true); + expect(sent.map((m) => m.content)).toEqual(['hi', 'hello', 'try again']); + }); + + it('preserves role:"system" messages when sending', async () => { + const service = new AIService(makeSettings()); + const messages: Message[] = [ + { role: 'user', content: 'do it' }, + { role: 'system', content: 'Response is aborted.' }, + { role: 'user', content: 'retry' }, + ]; + + await service.sendMessageToAI(messages, () => {}); + + const sent = sendMessageSpy.mock.calls[0][0] as Message[]; + expect(sent).toEqual(messages); + }); + + it('preserves user and assistant messages in order', async () => { + const service = new AIService(makeSettings()); + const messages: Message[] = [ + { role: 'user', content: 'a' }, + { role: 'assistant', content: 'b' }, + { role: 'user', content: 'c' }, + ]; + + await service.sendMessageToAI(messages, () => {}); + + const sent = sendMessageSpy.mock.calls[0][0] as Message[]; + expect(sent).toEqual(messages); + }); +}); diff --git a/src/services/providers/__tests__/OllamaProvider.test.ts b/src/services/providers/__tests__/OllamaProvider.test.ts index 9326ea6..179d32c 100644 --- a/src/services/providers/__tests__/OllamaProvider.test.ts +++ b/src/services/providers/__tests__/OllamaProvider.test.ts @@ -231,7 +231,7 @@ describe('OllamaProvider', () => { expect(body.think).toBe('medium'); }); - it('should omit think when reasoning is disabled', async () => { + it('should set think: false when reasoning is disabled so thinking models stop thinking', async () => { setupFetchMock( mockFetchResponse({ message: { content: 'Response' }, @@ -240,13 +240,31 @@ describe('OllamaProvider', () => { ); await provider.sendMessage(messages, { - model: 'llama2', + model: 'deepseek-r1', + stream: false, + reasoning: false, + }); + + const body = JSON.parse((global.fetch as any).mock.calls[0][1].body); + expect(body.think).toBe(false); + }); + + it('should use think: low for gpt-oss when reasoning is disabled (gpt-oss ignores think: false)', async () => { + setupFetchMock( + mockFetchResponse({ + message: { content: 'Response' }, + done: true, + }) + ); + + await provider.sendMessage(messages, { + model: 'gpt-oss:120b-cloud', stream: false, reasoning: false, }); const body = JSON.parse((global.fetch as any).mock.calls[0][1].body); - expect(body.think).toBeUndefined(); + expect(body.think).toBe('low'); }); it('should stream thinking then content into think tags', async () => { diff --git a/src/services/providers/adapters/ollamaRequestBuilder.ts b/src/services/providers/adapters/ollamaRequestBuilder.ts index 6429364..ad074f1 100644 --- a/src/services/providers/adapters/ollamaRequestBuilder.ts +++ b/src/services/providers/adapters/ollamaRequestBuilder.ts @@ -14,15 +14,19 @@ import { toOllamaMessage } from '../core/messageUtils'; const OLLAMA_DEFAULT_ORIGIN = 'http://localhost:11434'; -/** Ollama `/api/chat` `think` field — GPT-OSS ignores booleans; use a level. */ +/** Ollama `/api/chat` `think` field. + * Reasoning models (deepseek-r1, qwen3, …) think by default on Ollama unless + * `think` is explicitly set to `false`, so we always send the user's choice. + * GPT-OSS is a reasoning model at its core — it ignores `think: false` and keeps + * thinking; the best we can do when the user disables thinking is drop to the + * lowest reasoning effort. */ function ollamaThinkFromReasoning( reasoning: boolean | undefined, model: string | undefined -): boolean | 'low' | 'medium' | 'high' | undefined { - if (!reasoning) return undefined; - const id = (model || '').toLowerCase(); - if (id.includes('gpt-oss')) return 'medium'; - return true; +): boolean | 'low' | 'medium' | 'high' { + const isGptOss = (model || '').toLowerCase().includes('gpt-oss'); + if (!reasoning) return isGptOss ? 'low' : false; + return isGptOss ? 'medium' : true; } export const ollamaRequestBuilder: RequestBuilder = { @@ -59,8 +63,7 @@ export const ollamaRequestBuilder: RequestBuilder = { messages: messages.map((m) => ({ role: m.role, ...toOllamaMessage(m.content) })), stream: options?.stream !== false, }; - const think = ollamaThinkFromReasoning(options?.reasoning, options?.model); - if (think !== undefined) body.think = think; + body.think = ollamaThinkFromReasoning(options?.reasoning, options?.model); if (Object.keys(runtimeOptions).length > 0) body.options = runtimeOptions; return body; }, diff --git a/src/types/chat.d.ts b/src/types/chat.d.ts index 9e2e223..7d1ee9c 100644 --- a/src/types/chat.d.ts +++ b/src/types/chat.d.ts @@ -17,7 +17,8 @@ export type ImageContentPart = { export type ContentPart = TextContentPart | ImageContentPart; export type Message = { - role: 'user' | 'assistant' | 'system'; + role: 'user' | 'assistant' | 'system' | 'error'; content: string | ContentPart[]; sources?: ChatSource[]; + detail?: string; };