mirror of
https://github.com/logancyang/obsidian-copilot.git
synced 2026-07-22 07:50:24 +00:00
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 <noreply@anthropic.com>
* 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 <noreply@anthropic.com>
* 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 <noreply@anthropic.com>
* 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 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
308c88332d
commit
54e341ca88
8 changed files with 70 additions and 66 deletions
16
package-lock.json
generated
16
package-lock.json
generated
|
|
@ -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": {
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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<string> {
|
||||
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<AgentRunContext> {
|
||||
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<number, { id?: string; name: string; args: string }> = new Map();
|
||||
const toolCallChunks: Map<number, ToolCallChunk> = 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<string, unknown> }> = [];
|
||||
for (const chunk of toolCallChunks.values()) {
|
||||
if (!chunk.name) continue;
|
||||
let args: Record<string, unknown> = {};
|
||||
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({
|
||||
|
|
|
|||
|
|
@ -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<number, ToolCallChunk>): 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 = {};
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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<any> {
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue