From 624df32c3756bbb747c711e0b4d430f64957ed0e Mon Sep 17 00:00:00 2001 From: Florian Egger Date: Thu, 21 May 2026 16:47:08 +0200 Subject: [PATCH] Add model change callback and improve chat UI robustness - Persist selected model to settings when changed via dropdown - Reorder chat controls: model selector before mode selector, new chat before delete - Add null guards for modelSelectorEl during async populate - Fix streaming state and empty response handling in tool nudge loop - Expand vault operation keyword detection - Enable text selection in chat messages - Include original error message in chat error display --- main.js | 94 ++++++++++++++++++++++++++------------ src/chat-view.ts | 99 ++++++++++++++++++++++++++++------------- src/main.ts | 9 +++- styles.css | 6 +++ tests/chat-view.test.ts | 23 ++++------ 5 files changed, 155 insertions(+), 76 deletions(-) diff --git a/main.js b/main.js index 672044b..f874773 100644 --- a/main.js +++ b/main.js @@ -10907,7 +10907,7 @@ var ErrorHandler = class { // src/chat-view.ts var ChatView = class extends import_obsidian5.ItemView { - constructor(leaf, settings, vectorStore, structuredMemoryManager, telemetryManager, chatHistoryManager) { + constructor(leaf, settings, vectorStore, structuredMemoryManager, telemetryManager, chatHistoryManager, onModelChange) { super(leaf); // State this.messages = []; @@ -10973,6 +10973,7 @@ var ChatView = class extends import_obsidian5.ItemView { this.structuredMemoryManager = structuredMemoryManager; this.telemetryManager = telemetryManager; this.chatHistoryManager = chatHistoryManager; + this.onModelChange = onModelChange; this.workflowEngine = new WorkflowEngine( this.app.vault, this.app, @@ -11011,6 +11012,7 @@ var ChatView = class extends import_obsidian5.ItemView { newSettings ); this.agentOllamaClient = (newSettings.agentModel ?? newSettings.model) === (newSettings.chatModel ?? newSettings.model) ? this.ollamaClient : this.createOllamaClient(newSettings.agentModel ?? newSettings.model, newSettings); + void this.populateModelDropdown(); this.workflowEngine = new WorkflowEngine( this.app.vault, this.app, @@ -11076,6 +11078,7 @@ var ChatView = class extends import_obsidian5.ItemView { this.chatContainer = null; this.showLogsButton = null; this.logsContainer = null; + this.modelSelectorEl = null; this.historySelectEl = null; this.historyDeleteButton = null; return Promise.resolve(); @@ -11136,6 +11139,21 @@ var ChatView = class extends import_obsidian5.ItemView { container.appendChild(this.lastMessageEl); } } + if (!this.modelSelectorEl) { + this.modelSelectorEl = newChatContainer.createEl("select", { + cls: "ollama-model-selector" + }); + this.modelSelectorEl.addEventListener("change", () => { + const selectedModel = this.modelSelectorEl.value; + this.ollamaClient.setModel(selectedModel); + if (this.agentOllamaClient !== this.ollamaClient) { + this.agentOllamaClient.setModel(selectedModel); + } + this.onModelChange?.(selectedModel); + }); + } + this.populateModelDropdown(); + newChatContainer.appendChild(this.modelSelectorEl); if (!this.modeSelectorEl) { this.modeSelectorEl = newChatContainer.createEl("select", { cls: "ollama-mode-selector" @@ -11155,20 +11173,6 @@ var ChatView = class extends import_obsidian5.ItemView { } else { newChatContainer.appendChild(this.modeSelectorEl); } - if (!this.modelSelectorEl) { - this.modelSelectorEl = newChatContainer.createEl("select", { - cls: "ollama-model-selector" - }); - this.modelSelectorEl.addEventListener("change", () => { - const selectedModel = this.modelSelectorEl.value; - this.ollamaClient.setModel(selectedModel); - if (this.agentOllamaClient !== this.ollamaClient) { - this.agentOllamaClient.setModel(selectedModel); - } - }); - } - this.populateModelDropdown(); - newChatContainer.appendChild(this.modelSelectorEl); if (!this.historySelectEl) { this.historySelectEl = newChatContainer.createEl("select", { cls: "ollama-history-selector" @@ -11189,6 +11193,14 @@ var ChatView = class extends import_obsidian5.ItemView { } this.populateHistoryDropdown(); newChatContainer.appendChild(this.historySelectEl); + if (!this.newChatButton) { + this.newChatButton = newChatContainer.createEl("button", { + cls: "ollama-new-chat-button", + text: "New Chat" + }); + } else { + newChatContainer.appendChild(this.newChatButton); + } if (!this.historyDeleteButton) { this.historyDeleteButton = newChatContainer.createEl("button", { cls: "ollama-history-delete-button", @@ -11206,14 +11218,6 @@ var ChatView = class extends import_obsidian5.ItemView { }); } newChatContainer.appendChild(this.historyDeleteButton); - if (!this.newChatButton) { - this.newChatButton = newChatContainer.createEl("button", { - cls: "ollama-new-chat-button", - text: "New Chat" - }); - } else { - newChatContainer.appendChild(this.newChatButton); - } if (!this.showLogsButton) { this.showLogsButton = newChatContainer.createEl("button", { cls: "ollama-show-logs-button", @@ -11432,6 +11436,7 @@ var ChatView = class extends import_obsidian5.ItemView { const previousValue = this.modelSelectorEl.value; this.modelSelectorEl.innerHTML = ""; const models = await this.ollamaClient.listModels(); + if (!this.modelSelectorEl) return; const currentModel = this.ollamaClient.getModel(); if (models.length === 0) { const option = this.modelSelectorEl.createEl("option", { @@ -11442,6 +11447,7 @@ var ChatView = class extends import_obsidian5.ItemView { return; } for (const model of models) { + if (!this.modelSelectorEl) return; const displayName = model.name; const option = this.modelSelectorEl.createEl("option", { text: displayName, @@ -11451,6 +11457,7 @@ var ChatView = class extends import_obsidian5.ItemView { option.setAttribute("selected", "selected"); } } + if (!this.modelSelectorEl) return; if (previousValue && models.some((m) => m.name === previousValue)) { this.modelSelectorEl.value = previousValue; } else if (models.some((m) => m.name === currentModel)) { @@ -12143,7 +12150,7 @@ ${actualMessage}` : actualMessage; fullResponse = ""; this.updateMessageById(assistantMessageId, { content: "", - isStreaming: true, + isStreaming: false, isThinking: false }); let attempts = 0; @@ -12157,7 +12164,7 @@ ${actualMessage}` : actualMessage; ); const nudgeMessages = [ ...currentMessages, - { role: "assistant", content: currentResponse }, + ...currentResponse.trim() ? [{ role: "assistant", content: currentResponse }] : [], { role: "user", content: attempts === 1 ? "You indicated you would take action but did not emit any tool_calls. Emit the required tool_calls now. Do not output explanatory text." : "You still have not emitted any tool_calls. Remember: when you need vault information, you MUST call tools immediately. Emit the tool_calls now. No text." @@ -12209,7 +12216,7 @@ ${actualMessage}` : actualMessage; } if (toolCalls.length === 0) { this.updateMessageById(assistantMessageId, { - content: fullResponse, + content: fullResponse || "(No response)", isStreaming: false, isThinking: false }); @@ -12252,8 +12259,9 @@ ${actualMessage}` : actualMessage; this.syncMessagesToSession(); } catch (error) { ErrorHandler.handleError(error, "ChatView.handleUserInput"); + const errorMsg = error instanceof Error ? error.message : String(error); this.updateMessageById(assistantMessageId, { - content: "An error occurred while processing your request.", + content: `An error occurred: ${errorMsg}`, isStreaming: false, isThinking: false }); @@ -12315,7 +12323,28 @@ ${actualMessage}` : actualMessage; "tell me about", "vault", "notes", - "files" + "files", + "continue", + "go ahead", + "start", + "do it", + "execute", + "run", + "proceed", + "next", + "now", + "yes", + "ok", + "okay", + "sure", + "please", + "step", + "merge", + "clean", + "fix", + "update", + "implement", + "apply" ]; return operationPhrases.some((phrase) => lower.includes(phrase)); } @@ -13762,7 +13791,14 @@ var OllamaPlugin = class extends import_obsidian7.Plugin { this.vaultVectorStore, this.structuredMemoryManager, this.telemetryManager, - this.chatHistoryManager + this.chatHistoryManager, + (model) => { + this.settings.chatModel = model; + this.settings.agentModel = model; + this.settings.model = model; + void this.saveSettings(); + this.notifyChatViews(); + } ) ); this.addRibbonIcon("bot", "Open Ollama Chat", async () => { diff --git a/src/chat-view.ts b/src/chat-view.ts index 3285f2e..5555120 100755 --- a/src/chat-view.ts +++ b/src/chat-view.ts @@ -57,7 +57,8 @@ export class ChatView extends ItemView { vectorStore?: VaultVectorStore, structuredMemoryManager?: StructuredMemoryManager, telemetryManager?: TelemetryManager, - chatHistoryManager?: ChatHistoryManager + chatHistoryManager?: ChatHistoryManager, + onModelChange?: (model: string) => void ) { super(leaf); this.messages = []; @@ -96,6 +97,7 @@ export class ChatView extends ItemView { this.structuredMemoryManager = structuredMemoryManager; this.telemetryManager = telemetryManager; this.chatHistoryManager = chatHistoryManager; + this.onModelChange = onModelChange; this.workflowEngine = new WorkflowEngine( this.app.vault, this.app, @@ -129,6 +131,8 @@ export class ChatView extends ItemView { (newSettings.agentModel ?? newSettings.model) === (newSettings.chatModel ?? newSettings.model) ? this.ollamaClient : this.createOllamaClient(newSettings.agentModel ?? newSettings.model, newSettings); + // Refresh model dropdown so it reflects the newly saved model + void this.populateModelDropdown(); this.workflowEngine = new WorkflowEngine( this.app.vault, this.app, @@ -202,6 +206,7 @@ export class ChatView extends ItemView { this.chatContainer = null; this.showLogsButton = null; this.logsContainer = null; + this.modelSelectorEl = null; this.historySelectEl = null; this.historyDeleteButton = null; return Promise.resolve(); @@ -280,6 +285,23 @@ export class ChatView extends ItemView { } } + // Setup model selector + if (!this.modelSelectorEl) { + this.modelSelectorEl = newChatContainer.createEl('select', { + cls: 'ollama-model-selector', + }); + this.modelSelectorEl.addEventListener('change', () => { + const selectedModel = this.modelSelectorEl!.value; + this.ollamaClient.setModel(selectedModel); + if (this.agentOllamaClient !== this.ollamaClient) { + this.agentOllamaClient.setModel(selectedModel); + } + this.onModelChange?.(selectedModel); + }); + } + this.populateModelDropdown(); + newChatContainer.appendChild(this.modelSelectorEl); + // Setup mode selector if (!this.modeSelectorEl) { this.modeSelectorEl = newChatContainer.createEl('select', { @@ -301,22 +323,6 @@ export class ChatView extends ItemView { newChatContainer.appendChild(this.modeSelectorEl); } - // Setup model selector - if (!this.modelSelectorEl) { - this.modelSelectorEl = newChatContainer.createEl('select', { - cls: 'ollama-model-selector', - }); - this.modelSelectorEl.addEventListener('change', () => { - const selectedModel = this.modelSelectorEl!.value; - this.ollamaClient.setModel(selectedModel); - if (this.agentOllamaClient !== this.ollamaClient) { - this.agentOllamaClient.setModel(selectedModel); - } - }); - } - this.populateModelDropdown(); - newChatContainer.appendChild(this.modelSelectorEl); - // Setup chat history selector if (!this.historySelectEl) { this.historySelectEl = newChatContainer.createEl('select', { @@ -339,6 +345,16 @@ export class ChatView extends ItemView { this.populateHistoryDropdown(); newChatContainer.appendChild(this.historySelectEl); + // Setup new chat button + if (!this.newChatButton) { + this.newChatButton = newChatContainer.createEl('button', { + cls: 'ollama-new-chat-button', + text: 'New Chat', + }); + } else { + newChatContainer.appendChild(this.newChatButton); + } + // Setup delete history button if (!this.historyDeleteButton) { this.historyDeleteButton = newChatContainer.createEl('button', { @@ -358,16 +374,6 @@ export class ChatView extends ItemView { } newChatContainer.appendChild(this.historyDeleteButton); - // Setup new chat button - if (!this.newChatButton) { - this.newChatButton = newChatContainer.createEl('button', { - cls: 'ollama-new-chat-button', - text: 'New Chat', - }); - } else { - newChatContainer.appendChild(this.newChatButton); - } - // Setup show-logs toggle button if (!this.showLogsButton) { this.showLogsButton = newChatContainer.createEl('button', { @@ -641,6 +647,7 @@ export class ChatView extends ItemView { this.modelSelectorEl.innerHTML = ''; const models = await this.ollamaClient.listModels(); + if (!this.modelSelectorEl) return; // Guard: view may have closed while fetching const currentModel = this.ollamaClient.getModel(); if (models.length === 0) { @@ -654,6 +661,7 @@ export class ChatView extends ItemView { } for (const model of models) { + if (!this.modelSelectorEl) return; const displayName = model.name; const option = this.modelSelectorEl.createEl('option', { text: displayName, @@ -664,6 +672,7 @@ export class ChatView extends ItemView { } } + if (!this.modelSelectorEl) return; // If the previously selected value is still valid, keep it if (previousValue && models.some((m) => m.name === previousValue)) { this.modelSelectorEl.value = previousValue; @@ -1468,11 +1477,11 @@ export class ChatView extends ItemView { if (modelMentionedActions || userWantsVaultOps) { shouldFallbackToReadTools = true; - // Suppress the model's "Let me..." text — replace with actual tool execution + // Suppress the model's "Let me..." text — clear it from the DOM immediately fullResponse = ''; this.updateMessageById(assistantMessageId, { content: '', - isStreaming: true, + isStreaming: false, isThinking: false, }); @@ -1488,9 +1497,12 @@ export class ChatView extends ItemView { attempts === 1 ? 'Thinking…' : `Retrying (${attempts}/${maxAttempts})…` ); + // Build nudge messages — skip empty assistant content to avoid API issues const nudgeMessages: OllamaMessage[] = [ ...currentMessages, - { role: 'assistant', content: currentResponse }, + ...(currentResponse.trim() + ? [{ role: 'assistant' as const, content: currentResponse }] + : []), { role: 'user', content: @@ -1553,7 +1565,7 @@ export class ChatView extends ItemView { // Update assistant message — only if no tool calls were processed if (toolCalls.length === 0) { this.updateMessageById(assistantMessageId, { - content: fullResponse, + content: fullResponse || '(No response)', isStreaming: false, isThinking: false, }); @@ -1607,8 +1619,9 @@ export class ChatView extends ItemView { this.syncMessagesToSession(); } catch (error) { ErrorHandler.handleError(error, 'ChatView.handleUserInput'); + const errorMsg = error instanceof Error ? error.message : String(error); this.updateMessageById(assistantMessageId, { - content: 'An error occurred while processing your request.', + content: `An error occurred: ${errorMsg}`, isStreaming: false, isThinking: false, }); @@ -1676,6 +1689,27 @@ export class ChatView extends ItemView { 'vault', 'notes', 'files', + 'continue', + 'go ahead', + 'start', + 'do it', + 'execute', + 'run', + 'proceed', + 'next', + 'now', + 'yes', + 'ok', + 'okay', + 'sure', + 'please', + 'step', + 'merge', + 'clean', + 'fix', + 'update', + 'implement', + 'apply', ]; return operationPhrases.some((phrase) => lower.includes(phrase)); } @@ -1772,6 +1806,7 @@ export class ChatView extends ItemView { private telemetryManager?: TelemetryManager; private chatHistoryManager?: ChatHistoryManager; private vectorStore?: VaultVectorStore; + private onModelChange?: (model: string) => void; private modeSelectorEl: HTMLSelectElement | null = null; private modelSelectorEl: HTMLSelectElement | null = null; diff --git a/src/main.ts b/src/main.ts index e0feb4f..c4d429d 100755 --- a/src/main.ts +++ b/src/main.ts @@ -43,7 +43,14 @@ export default class OllamaPlugin extends Plugin { this.vaultVectorStore, this.structuredMemoryManager, this.telemetryManager, - this.chatHistoryManager + this.chatHistoryManager, + (model: string) => { + this.settings.chatModel = model; + this.settings.agentModel = model; + this.settings.model = model; + void this.saveSettings(); + this.notifyChatViews(); + } ) ); diff --git a/styles.css b/styles.css index f64a62a..8f1cbd4 100644 --- a/styles.css +++ b/styles.css @@ -36,6 +36,8 @@ overflow-y: auto; flex: 1 1 auto; min-height: 0; + user-select: text; + -webkit-user-select: text; } .ollama-input-container { @@ -56,6 +58,8 @@ border-radius: var(--ollama-radius); border: 1px solid var(--ollama-border); max-width: 90%; + user-select: text; + -webkit-user-select: text; } .ollama-message-user { @@ -99,6 +103,8 @@ line-height: var(--line-height-normal); white-space: pre-wrap; word-break: break-word; + user-select: text; + -webkit-user-select: text; } .ollama-new-chat-container { diff --git a/tests/chat-view.test.ts b/tests/chat-view.test.ts index adcde8a..73efa38 100755 --- a/tests/chat-view.test.ts +++ b/tests/chat-view.test.ts @@ -121,7 +121,8 @@ describe('ChatView', () => { undefined, undefined, undefined, - new ChatHistoryManager() + new ChatHistoryManager(), + undefined ); // Obsidian's contentEl has a createEl helper that standard DOM lacks // Unlike standard DOM, Obsidian elements can create nested elements with createEl @@ -423,13 +424,11 @@ describe('ChatView', () => { })() ); - const handleToolSpy = jest - .spyOn(view['toolExecutor'], 'handleToolCall') - .mockResolvedValue({ - success: true, - message: 'Found vault context', - data: [{ path: 'Prompts.md', title: 'Prompts' }], - }); + const handleToolSpy = jest.spyOn(view['toolExecutor'], 'handleToolCall').mockResolvedValue({ + success: true, + message: 'Found vault context', + data: [{ path: 'Prompts.md', title: 'Prompts' }], + }); jest.spyOn(view['ollamaClient'], 'chat').mockResolvedValue({ role: 'assistant', content: 'I found vault context and can now suggest the next organization step.', @@ -441,14 +440,10 @@ describe('ChatView', () => { expect(handleToolSpy).toHaveBeenCalled(); expect( - handleToolSpy.mock.calls.some( - ([toolCall]) => toolCall.function.name === 'get_vault_stats' - ) + handleToolSpy.mock.calls.some(([toolCall]) => toolCall.function.name === 'get_vault_stats') ).toBe(true); expect( - handleToolSpy.mock.calls.some( - ([toolCall]) => toolCall.function.name === 'list_vault_tags' - ) + handleToolSpy.mock.calls.some(([toolCall]) => toolCall.function.name === 'list_vault_tags') ).toBe(true); const messages = (view as any).messages;