From 54e341ca888bad54fb86e44c8c1865ba6038ba0b Mon Sep 17 00:00:00 2001 From: Logan Yang Date: Tue, 3 Feb 2026 22:18:39 -0800 Subject: [PATCH] Update ollama (#2147) * Update ollama chat model to fix thinking * fix: consolidate tool call parsing and add actionable error logging - Remove duplicate tool call parsing in AutonomousAgentChainRunner by using shared buildToolCallsFromChunks() function - Add sanitizeToolArgs() to remove empty objects from tool args (fixes Ollama returning timeRange: {} which fails Zod validation) - Add actionable logging for tool call failures with full args context - Clean up unused parameters in AutonomousAgentChainRunner - Simplify ToolManager.callTool to throw instead of swallowing errors Co-Authored-By: Claude Opus 4.5 * fix: add repeatPenalty for Ollama to reduce repetitive outputs Local models via Ollama are prone to repetitive hallucination loops. Adding repeatPenalty=1.1 applies a slight penalty to repeated tokens, which helps reduce this pattern. Co-Authored-By: Claude Opus 4.5 * fix: increase Ollama context window to 128k Default numCtx of 8192 causes prompt truncation with larger contexts. Setting numCtx=131072 (128k) to support models with large context. Ollama automatically caps at model's actual maximum if exceeded. Co-Authored-By: Claude Opus 4.5 * fix: handle Ollama empty object tool args at schema level - Make timeRange inner fields (startTime, endTime) optional in schema - Update validateTimeRange to handle empty or partial objects from LLMs - Apply validation in all search tools (local, lexical, semantic) - Remove sanitizeToolArgs function that was too aggressive - Fixes tool calls that legitimately accept {} as valid input Co-Authored-By: Claude Opus 4.5 --------- Co-authored-by: Claude Opus 4.5 --- package-lock.json | 16 ++++----- package.json | 2 +- .../chainRunner/AutonomousAgentChainRunner.ts | 35 +++++++------------ .../chainRunner/utils/nativeToolCalling.ts | 3 +- .../chainRunner/utils/toolExecution.ts | 13 +++++-- src/LLMProviders/chatModelManager.ts | 11 ++++-- src/tools/SearchTools.ts | 24 +++++++------ src/tools/toolManager.ts | 32 +++++++---------- 8 files changed, 70 insertions(+), 66 deletions(-) diff --git a/package-lock.json b/package-lock.json index 8a5ed6c4..341a392c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -85,7 +85,7 @@ "turndown": "^7.2.2" }, "devDependencies": { - "@langchain/ollama": "^1.0.0", + "@langchain/ollama": "^1.2.2", "@tailwindcss/container-queries": "^0.1.1", "@testing-library/jest-dom": "^5.16.5", "@testing-library/react": "^14.0.0", @@ -4684,13 +4684,13 @@ } }, "node_modules/@langchain/ollama": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/@langchain/ollama/-/ollama-1.0.0.tgz", - "integrity": "sha512-zqn6i7haMjvZW4FQWo0GrF4wYL5mLurdL0qoe+moYWYSCGaay4K7e/4dqM5C/MR16/HPFDzFbBRMkni2PDRBgA==", + "version": "1.2.2", + "resolved": "https://registry.npmjs.org/@langchain/ollama/-/ollama-1.2.2.tgz", + "integrity": "sha512-UmbDSToADT15O7kMBm+0MVs/Zie8SHVFMKt4BDSyunXZOgDdqIPvc4pEqy50m1/gq6kIaVuR2hRAy5kcF0V8kg==", "dev": true, "license": "MIT", "dependencies": { - "ollama": "^0.5.12", + "ollama": "^0.6.3", "uuid": "^10.0.0" }, "engines": { @@ -19971,9 +19971,9 @@ } }, "node_modules/ollama": { - "version": "0.5.18", - "resolved": "https://registry.npmjs.org/ollama/-/ollama-0.5.18.tgz", - "integrity": "sha512-lTFqTf9bo7Cd3hpF6CviBe/DEhewjoZYd9N/uCe7O20qYTvGqrNOFOBDj3lbZgFWHUgDv5EeyusYxsZSLS8nvg==", + "version": "0.6.3", + "resolved": "https://registry.npmjs.org/ollama/-/ollama-0.6.3.tgz", + "integrity": "sha512-KEWEhIqE5wtfzEIZbDCLH51VFZ6Z3ZSa6sIOg/E/tBV8S51flyqBOXi+bRxlOYKDf8i327zG9eSTb8IJxvm3Zg==", "dev": true, "license": "MIT", "dependencies": { diff --git a/package.json b/package.json index 26d3fb65..453196e0 100644 --- a/package.json +++ b/package.json @@ -35,7 +35,7 @@ }, "license": "AGPL-3.0", "devDependencies": { - "@langchain/ollama": "^1.0.0", + "@langchain/ollama": "^1.2.2", "@tailwindcss/container-queries": "^0.1.1", "@testing-library/jest-dom": "^5.16.5", "@testing-library/react": "^14.0.0", diff --git a/src/LLMProviders/chainRunner/AutonomousAgentChainRunner.ts b/src/LLMProviders/chainRunner/AutonomousAgentChainRunner.ts index 5b9396c4..017192c6 100644 --- a/src/LLMProviders/chainRunner/AutonomousAgentChainRunner.ts +++ b/src/LLMProviders/chainRunner/AutonomousAgentChainRunner.ts @@ -22,7 +22,12 @@ import { logToolCall, logToolResult, } from "./utils/toolExecution"; -import { createToolResultMessage, generateToolCallId } from "./utils/nativeToolCalling"; +import { + createToolResultMessage, + generateToolCallId, + buildToolCallsFromChunks, + ToolCallChunk, +} from "./utils/nativeToolCalling"; import { ensureCiCOrderingWithQuestion } from "./utils/cicPromptUtils"; import { LayerToMessagesConverter } from "@/context/LayerToMessagesConverter"; @@ -273,7 +278,7 @@ export class AutonomousAgentChainRunner extends CopilotPlusChainRunner { */ public static async generateSystemPrompt( availableTools: StructuredTool[], - adapter: ModelAdapter, + _adapter?: ModelAdapter, // Unused, kept for backwards compatibility with tests userMemoryManager?: UserMemoryManager ): Promise { const basePrompt = await getSystemPromptWithMemory(userMemoryManager); @@ -500,13 +505,13 @@ export class AutonomousAgentChainRunner extends CopilotPlusChainRunner { * * @param userMessage - The initiating user message from the UI. * @param chatModel - The active chat model instance. - * @param updateLoadingMessage - Optional callback to show loading status. + * @param _updateLoadingMessage - Unused, kept for potential future use. * @returns Context required for the ReAct agent loop. */ private async prepareAgentConversation( userMessage: ChatMessage, chatModel: any, - updateLoadingMessage?: (message: string) => void + _updateLoadingMessage?: (message: string) => void // Unused, kept for potential future use ): Promise { const messages: BaseMessage[] = []; const availableTools = this.getAvailableTools(); @@ -856,7 +861,7 @@ export class AutonomousAgentChainRunner extends CopilotPlusChainRunner { _updateCurrentAiMessage: (message: string) => void ): Promise<{ content: string; aiMessage: AIMessage; streamingResult: StreamingResult }> { let fullContent = ""; - const toolCallChunks: Map = new Map(); + const toolCallChunks: Map = new Map(); // Helper to handle content updates - don't detect final response here, // let runReActLoop decide based on whether there are tool calls @@ -908,24 +913,8 @@ export class AutonomousAgentChainRunner extends CopilotPlusChainRunner { } } - // Build tool calls from accumulated chunks - const toolCalls: Array<{ id: string; name: string; args: Record }> = []; - for (const chunk of toolCallChunks.values()) { - if (!chunk.name) continue; - let args: Record = {}; - if (chunk.args) { - try { - args = JSON.parse(chunk.args); - } catch { - logWarn(`Failed to parse tool args: ${chunk.args}`); - } - } - toolCalls.push({ - id: chunk.id || generateToolCallId(), - name: chunk.name, - args, - }); - } + // Build tool calls from accumulated chunks (with sanitization for empty objects) + const toolCalls = buildToolCallsFromChunks(toolCallChunks); // Build AIMessage const aiMessage = new AIMessage({ diff --git a/src/LLMProviders/chainRunner/utils/nativeToolCalling.ts b/src/LLMProviders/chainRunner/utils/nativeToolCalling.ts index a720db13..a6cf80dd 100644 --- a/src/LLMProviders/chainRunner/utils/nativeToolCalling.ts +++ b/src/LLMProviders/chainRunner/utils/nativeToolCalling.ts @@ -7,6 +7,7 @@ import { AIMessage, ToolMessage } from "@langchain/core/messages"; import { ToolCall as LangChainToolCall } from "@langchain/core/messages/tool"; +import { logError } from "@/logger"; /** * Standardized tool call structure extracted from AIMessage @@ -108,7 +109,7 @@ export function buildToolCallsFromChunks(chunks: Map): Na try { args = JSON.parse(chunk.args); } catch { - // If args can't be parsed, use empty object + logError(`[ToolCall] Failed to parse args for tool "${chunk.name}": ${chunk.args}`); args = {}; } } diff --git a/src/LLMProviders/chainRunner/utils/toolExecution.ts b/src/LLMProviders/chainRunner/utils/toolExecution.ts index a58b8698..739367eb 100644 --- a/src/LLMProviders/chainRunner/utils/toolExecution.ts +++ b/src/LLMProviders/chainRunner/utils/toolExecution.ts @@ -124,10 +124,19 @@ export async function executeSequentialToolCall( success: true, }; } catch (error) { - logError(`Error executing tool ${toolCall.name}:`, error); + // Log actionable error with args for debugging schema mismatches + const errorMsg = err2String(error); + const isSchemaError = errorMsg.includes("schema"); + if (isSchemaError) { + logError( + `[ToolCall] Schema validation failed for "${toolCall.name}". Args: ${JSON.stringify(toolCall.args, null, 2)}` + ); + } else { + logError(`[ToolCall] Error executing "${toolCall.name}": ${errorMsg}`); + } return { toolName: toolCall.name, - result: `Error: ${err2String(error)}`, + result: `Error: ${errorMsg}`, success: false, }; } diff --git a/src/LLMProviders/chatModelManager.ts b/src/LLMProviders/chatModelManager.ts index f7bcc84b..45fff2f6 100644 --- a/src/LLMProviders/chatModelManager.ts +++ b/src/LLMProviders/chatModelManager.ts @@ -284,9 +284,16 @@ export default class ChatModelManager { model: modelName, // MUST NOT use /v1 in the baseUrl for ollama baseUrl: customModel.baseUrl || "http://localhost:11434", - headers: new Headers({ + headers: { Authorization: `Bearer ${await getDecryptedKey(customModel.apiKey || "default-key")}`, - }), + }, + // Enable thinking for models with REASONING capability (e.g., qwen3, deepseek-r1) + // Thinking content goes to additional_kwargs.reasoning_content + think: customModel.capabilities?.includes(ModelCapability.REASONING) ?? false, + // Reduce repetition in local models (1.1 = slight penalty, helps with hallucination loops) + repeatPenalty: 1.1, + // Request large context window - Ollama caps at model's actual max anyway + numCtx: 131072, }, [ChatModelProviders.LM_STUDIO]: { modelName: modelName, diff --git a/src/tools/SearchTools.ts b/src/tools/SearchTools.ts index c42b0a30..863a2ac8 100644 --- a/src/tools/SearchTools.ts +++ b/src/tools/SearchTools.ts @@ -70,8 +70,8 @@ const localSearchSchema = z.object({ ), timeRange: z .object({ - startTime: z.number().describe("Start time as epoch milliseconds"), - endTime: z.number().describe("End time as epoch milliseconds"), + startTime: z.number().optional().describe("Start time as epoch milliseconds"), + endTime: z.number().optional().describe("End time as epoch milliseconds"), }) .optional() .describe("Optional time range filter. Use epoch milliseconds from getTimeRangeMs result."), @@ -222,7 +222,8 @@ const lexicalSearchTool = createLangChainTool({ name: "lexicalSearch", description: "Search for notes using lexical/keyword-based search", schema: localSearchSchema, - func: async ({ timeRange, query, salientTerms }) => { + func: async ({ timeRange: rawTimeRange, query, salientTerms }) => { + const timeRange = validateTimeRange(rawTimeRange); return await performLexicalSearch({ timeRange, query, salientTerms }); }, }); @@ -232,7 +233,8 @@ const semanticSearchTool = createLangChainTool({ name: "semanticSearch", description: "Search for notes using semantic/meaning-based search with embeddings", schema: localSearchSchema, - func: async ({ timeRange, query, salientTerms }) => { + func: async ({ timeRange: rawTimeRange, query, salientTerms }) => { + const timeRange = validateTimeRange(rawTimeRange); const settings = getSettings(); const returnAll = timeRange !== undefined; @@ -305,19 +307,21 @@ const semanticSearchTool = createLangChainTool({ /** * Validate and sanitize time range to prevent LLM hallucinations. - * Returns undefined if the time range is invalid or nonsensical. + * Returns undefined if the time range is invalid, incomplete, or nonsensical. + * Handles cases where LLMs return empty objects {} or partial objects. */ function validateTimeRange(timeRange?: { - startTime: number; - endTime: number; + startTime?: number; + endTime?: number; }): { startTime: number; endTime: number } | undefined { if (!timeRange) return undefined; const { startTime, endTime } = timeRange; - // Check for invalid values (0, negative, or non-numbers) + // Check for missing, invalid values (0, negative, or non-numbers) + // This handles LLM returning {} or {startTime: undefined, endTime: undefined} if (!startTime || !endTime || startTime <= 0 || endTime <= 0) { - logInfo("localSearch: Ignoring invalid time range (zero or negative values)"); + logInfo("localSearch: Ignoring invalid time range (missing, zero, or negative values)"); return undefined; } @@ -327,7 +331,7 @@ function validateTimeRange(timeRange?: { return undefined; } - return timeRange; + return { startTime, endTime }; } // Smart wrapper that uses RetrieverFactory for unified retriever selection diff --git a/src/tools/toolManager.ts b/src/tools/toolManager.ts index 8149a878..73d66c5f 100644 --- a/src/tools/toolManager.ts +++ b/src/tools/toolManager.ts @@ -1,4 +1,4 @@ -import { Notice } from "obsidian"; +import { logWarn } from "@/logger"; export const getToolDescription = (tool: string): string => { switch (tool) { @@ -16,28 +16,22 @@ export const getToolDescription = (tool: string): string => { }; export class ToolManager { + /** + * Call a tool with the given arguments. + * Throws on error so caller can handle with proper context (args, tool name). + */ static async callTool(tool: any, args: any): Promise { - try { - if (!tool) { - throw new Error("Tool is undefined"); - } + if (!tool) { + throw new Error("Tool is undefined"); + } - const result = await tool.call(args); + const result = await tool.call(args); - if (result === undefined || result === null) { - console.warn(`Tool ${tool.name} returned null/undefined result`); - return null; - } - - return result; - } catch (error) { - console.error(`Error calling tool:`, error); - if (error instanceof Error) { - new Notice(error.message); - } else { - new Notice("An error occurred while executing the tool. Check console for details."); - } + if (result === undefined || result === null) { + logWarn(`[ToolCall] Tool "${tool.name}" returned null/undefined`); return null; } + + return result; } }