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
2026-05-21 16:47:08 +02:00
parent f6edc321e3
commit 624df32c37
5 changed files with 155 additions and 76 deletions
@@ -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 () => {
@@ -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;
@@ -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();
+          }
        )
    );

@@ -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 {
@@ -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;