From fd49abcdb98d5a9214feb72caf827e890f5ba2d1 Mon Sep 17 00:00:00 2001 From: Florian Egger Date: Wed, 6 May 2026 16:30:16 +0200 Subject: [PATCH] Refactor error handling, client, and tests for Ollama integration --- .gitignore | 3 + __mocks__/obsidian.ts | 121 +- __mocks__/ollama-client.ts | 45 +- coverage/lcov-report/chat-view.ts.html | 716 +++++----- coverage/lcov-report/error-handler.ts.html | 354 ++--- coverage/lcov-report/index.html | 117 +- coverage/lcov-report/ollama-client.ts.html | 688 +++++---- coverage/lcov-report/tool-executor.ts.html | 383 +++-- coverage/lcov-report/types.ts.html | 524 +++---- coverage/lcov-report/utils.ts.html | 968 ++++--------- coverage/lcov-report/vault-indexer.ts.html | 1292 +++++++---------- coverage/lcov.info | 1478 +++++++++++--------- debug_heading.js | 46 + debug_heading_extraction.js | 34 + debug_scoring.js | 92 ++ debug_test.js | 30 + jest.setup.js | 70 + manifest.json | 16 + src/chat-view.ts | 13 + src/constants.ts | 12 + src/error-handler.ts | 139 +- main.ts => src/main.ts | 56 +- src/ollama-client.ts | 279 +++- src/tool-executor.ts | 139 +- src/types.ts | 186 ++- src/utils.ts | 206 ++- src/vault-indexer.ts | 278 +++- tests/chat-view.test.ts | 349 ++++- tests/error-handler.test.ts | 2 +- tests/ollama-client.test.ts | 15 +- tests/tool-executor.test.ts | 54 + tests/utils.test.ts | 4 - tests/vault-indexer.test.ts | 17 +- tsconfig.json | 2 +- tsconfig.test.json | 7 +- 35 files changed, 4970 insertions(+), 3765 deletions(-) create mode 100644 debug_heading.js create mode 100644 debug_heading_extraction.js create mode 100644 debug_scoring.js create mode 100644 debug_test.js create mode 100644 manifest.json create mode 100644 src/constants.ts rename main.ts => src/main.ts (70%) diff --git a/.gitignore b/.gitignore index 1b96262..7f1a909 100755 --- a/.gitignore +++ b/.gitignore @@ -33,3 +33,6 @@ npm-debug.log* # Test cache .jest-cache/ + +# review agents +agent_loop_gemma4/ diff --git a/__mocks__/obsidian.ts b/__mocks__/obsidian.ts index 551521e..5426045 100755 --- a/__mocks__/obsidian.ts +++ b/__mocks__/obsidian.ts @@ -1,50 +1,113 @@ // Enhanced Obsidian mock for testing + +// Mock Vault export class Vault { - getMarkdownFiles() { - return []; - } + getMarkdownFiles: () => any[]; + read: (file: any) => Promise; + create: (path: string, content: string) => Promise; - async read(file: any) { - return ''; - } - - async create(path: string, content: string) { - return null; + constructor() { + this.getMarkdownFiles = () => []; + this.read = async () => ''; + this.create = async () => null; } } +// Mock Workspace export class Workspace { - getLeaf() { - return { + getLeaf: () => any; + + constructor() { + this.getLeaf = () => ({ setViewState: jest.fn(), - }; + revealLeaf: jest.fn(), + }); } } +// Mock App export class App { - vault = new Vault(); - workspace = new Workspace(); + vault: Vault; + workspace: Workspace; + + constructor() { + this.vault = new Vault(); + this.workspace = new Workspace(); + } } +// Mock WorkspaceLeaf +export class WorkspaceLeaf { + app: App; + view: any; + setViewState: jest.Mock; + + constructor() { + this.app = new App(); + this.view = null; + this.setViewState = jest.fn(); + } +} + +// Mock ItemView - accepts a leaf and derives app from it export class ItemView { - contentEl: HTMLElement = document.createElement('div'); + contentEl: HTMLElement; + app: App; + + constructor(leaf?: WorkspaceLeaf) { + this.contentEl = document.createElement('div'); + if (leaf && leaf.app) { + this.app = leaf.app; + } else { + this.app = new App(); + } + } +} + +// Mock Notice - can be called with `new Notice(msg)` or as a function +export class Notice { + message: string; + + constructor(message: string) { + this.message = message; + // Also record via jest for testing + (Notice as any).lastMessage = message; + } +} +(Notice as any).lastMessage = ''; + +// Mock Setting +export class Setting { + containerEl: HTMLElement; + + constructor(containerEl: HTMLElement) { + this.containerEl = containerEl; + } + + setName(_name: string): this { + return this; + } + + setDesc(_desc: string): this { + return this; + } + + addText(_callback: (text: any) => void): this { + return this; + } +} + +// TFile type +export interface TFile { + basename: string; + path: string; +} + +// Plugin class (used by main.ts) +export class Plugin { app: App; constructor() { this.app = new App(); } } - -export class Notice { - static create(message: string) {} -} - -// Mock types for DOM elements -export type TFile = { - basename: string; -}; - -// Export additional types that might be used in tests -export const Plugin: any = jest.fn(); -export const WorkspaceLeaf: any = jest.fn(); -export const Setting: any = jest.fn(); diff --git a/__mocks__/ollama-client.ts b/__mocks__/ollama-client.ts index 3e6eb38..2870052 100644 --- a/__mocks__/ollama-client.ts +++ b/__mocks__/ollama-client.ts @@ -1,49 +1,44 @@ // Mock for ollama-client for testing -import { OllamaMessage, ToolCall } from '../src/types'; -import type { APIError } from '../src/error-handler'; +import type { OllamaMessage, OllamaTool } from '../src/types'; export class OllamaClient { private url: string; private model: string; // Mock fetch function for testing - private fetchFn: jest.Mock, [string, RequestInit?]>> = jest.fn(); + private fetchFn: typeof fetch; constructor(url: string, model: string, fetchFn?: typeof fetch) { this.url = url; this.model = model; - if (fetchFn) this.fetchFn = jest.fn(fetchFn); + this.fetchFn = fetchFn ?? (jest.fn() as typeof fetch); } - async *streamChatMessages( - prompt: string, - options: { abortSignal?: AbortSignal } = {} + async *streamChat( + _messages: OllamaMessage[], + _tools: OllamaTool[] = [] ): AsyncGenerator { // Mock implementation - simulate streaming response - const mockResponse = [ - { role: 'assistant', content: 'Part 1' }, - { role: 'assistant', content: 'Part 2' } + const mockMessages: OllamaMessage[] = [ + { role: 'assistant', content: 'Part 1', tool_calls: [] }, + { role: 'assistant', content: 'Part 2', tool_calls: [] }, ]; - for (const message of mockResponse) { + for (const message of mockMessages) { yield message; - await new Promise(resolve => setTimeout(resolve, 100)); // Simulate delay + await new Promise((resolve) => setTimeout(resolve, 10)); } } - async *streamToolMessages( - toolCall: ToolCall, - options: { abortSignal?: AbortSignal } = {} - ): AsyncGenerator { - // Mock implementation - simulate streaming response for tool - const mockResponse = [ - { role: 'assistant', content: `Tool ${toolCall.tool_name} Part 1` }, - { role: 'assistant', content: `Tool ${toolCall.tool_name} Part 2` } - ]; + async chat(_messages: OllamaMessage[], _tools: OllamaTool[] = []): Promise { + return { + role: 'assistant', + content: 'Mock response', + tool_calls: [], + }; + } - for (const message of mockResponse) { - yield message; - await new Promise(resolve => setTimeout(resolve, 100)); // Simulate delay - } + cancelStream(): void { + // No-op in mock } } diff --git a/coverage/lcov-report/chat-view.ts.html b/coverage/lcov-report/chat-view.ts.html index fb5ca7e..5b768f8 100755 --- a/coverage/lcov-report/chat-view.ts.html +++ b/coverage/lcov-report/chat-view.ts.html @@ -23,30 +23,30 @@
- 83.07% + 86.15% Statements - 162/195 + 168/195
- 72.3% + 76.92% Branches - 47/65 + 50/65
- 73.07% + 76.92% Functions - 19/26 + 20/26
- 86.18% + 88.95% Lines - 156/181 + 161/181
@@ -519,26 +519,26 @@   1x   -11x +31x       -11x -11x -11x -11x -11x -11x -11x -11x -11x +31x +31x +31x +31x +31x +31x +31x +31x +31x     -11x -11x -11x -11x -11x +31x +31x +31x +31x +31x       @@ -550,379 +550,379 @@       +5x +5x +5x +  +  +  +3x +3x +3x +3x 3x 3x 3x       -2x -2x -2x -2x -2x -2x -2x   -  -  -  -4x -2x -2x +15x +8x +8x           -11x -11x +27x +27x   -11x +27x     -11x +27x 4x   -11x +27x 4x     4x     -11x +27x +  +18x +  +18x +  +  +18x +18x +  +  +  +27x +  +  +254x +  +  +27x +128x +  +27x +243x +  +  +243x +104x +  +139x +  +  +139x +139x +  +  +  +  +27x +128x +3656x +24x +  +  +  +  +254x +27x +  +  +  +  +  +  +  +  +  +  +5x +  +  +5x +5x +  +  +  +  +  +  +5x +5x +  +  +  +  +  +  +  +  +5x +5x +5x +5x +5x +  +5x +  +  +  +  +5x +  +    8x +2x +  +  +  +  +8x +2x +  +  +  +  +8x +2x +  +  +    8x     -8x +  +  +  +  +  +  +  +  +  +138x +9x +9x +  +  +  +  +9x +  +  +  +146x +13x +9x +  +  +9x +  +13x +13x +  +  +  +  +12x +12x +  +12x +  +12x +12x +  +  +10x +10x +  +  +10x +10x +  +  +  +10x +  +  +  +10x +  +  +  +  +10x +  +  +  +120x +  +  +  +  +  +  +  +  +10x +  +  +  +  +  +  +  +  +  +  +  +  +  +  +  +  +  +10x +  +10x +10x +  +  +10x +  +  +  +  +  +  +10x +  +  +  +  +  +  +  +  +10x +  +10x +  +10x +10x +10x +10x +10x +10x +  +10x +10x +12x +12x +  +  +  +12x +12x +  +  +12x +1x +  +  +12x +  +  +  +10x +  +  +  8x       -11x     -11x +      -11x -11x   -11x +8x +  +1x +1x +  +  +  +  +1x +1x +  +  +1x +1x +1x +  +  +  +1x +  +  +  +  +1x +  +  +  +  +  +  +  +  +1x +1x +1x +  +  +1x +  +  +  +8x +7x +7x +7x 7x     -7x +  +  +8x 2x   -5x -  -  -5x -5x -  -  -  -  -11x -2x -2x -  -  -  -  -  -11x -11x -  -  -  -  -  -  -  -  -  -  -3x -  -  -3x -3x -  -  -  -  -  -  -3x -3x -  -  -  -  -  -  -  -  -3x -3x -3x -3x -3x -  -3x -  -  -  -  -3x -  -  -  -5x -1x -  -  -  -  -5x -1x -  -  -  -  -5x -1x -  -  -  -  -5x -  -  -  -  -  -  -  -  -  -  -  -6x -3x -3x -  -  -  -  -3x -  -  -  -6x -3x -3x -  -  -3x -  -3x -3x -  -  +8x     2x 2x   -2x -  -2x -2x -  -  -2x -2x -  -  -2x -2x -  -  -  -2x -  -  -  -2x -  -  -  -  -2x -  -  -  -  -  -  -  -  -  -  -  -  -2x -  -  -  -  -  -  -  -  -  -  -  -  -  -  -  -  -  -2x -  -2x -2x -  -  -2x -  -  -  -  -  -  -2x -  -  -  -  -  -  -  -  -2x -  -2x -  -2x -2x -2x -2x -2x -2x -  -2x -2x -2x -2x -  -  -  -2x -2x -  -  -2x -1x -  -  -2x -  -  -  -2x -  -  -  -2x -  -  -  -  -  -  -  -  -  -2x -  -1x -1x -  -  -  -  -1x -1x -  -  -1x -1x -1x -  -  -  -1x -  -  -  -  -1x -  -  -  -  -  -  -  -  -1x -1x -1x -  -  -1x -  -  -  -2x -1x -1x -1x -1x -  -  -  -  -2x -  -  -2x -  -  -  -  -  -2x -2x +12x +12x       @@ -1062,8 +1062,8 @@ export class ChatView extends ItemView { // Remove messages that are no longer in the array for (const el of Array.from(existingMessages)) { const id = el.getAttribute('data-msg-id'); - Iif (!id || !nonStreamingMessages.some((m) => m.id === id)) { - el.remove(); + if (!id || !nonStreamingMessages.some((m) => m.id === id)) { + el.remove(); } }   @@ -1176,7 +1176,7 @@ export class ChatView extends ItemView { try { // Guard against empty messages const userMessage = content.trim(); - Iif (!userMessage) return; + if (!userMessage) return;   // Search vault using user message as query const entries = await this.vaultIndexer.searchVault(userMessage, DEFAULT_VAULT_SEARCH_LIMIT); @@ -1200,8 +1200,8 @@ export class ChatView extends ItemView { const messages: OllamaMessage[] = [ systemMessage, ...this.messages.map( - (m) => - ({ + (m) => + ({ role: m.role, content: m.content, tool_calls: m.tool_calls, @@ -1344,14 +1344,14 @@ export class ChatView extends ItemView { }   // Limit conversation history to prevent memory issues - Iif (this.messages.length > MAX_MESSAGE_HISTORY) { - this.messages = this.messages.slice(-MAX_MESSAGE_HISTORY); + if (this.messages.length > MAX_MESSAGE_HISTORY) { + this.messages = this.messages.slice(-MAX_MESSAGE_HISTORY); } await this.render(); } catch (error) { // Use centralized error handler - ErrorHandler.handleError(error, 'ChatView.handleUserInput'); - this.cleanupStreamingResources(); + ErrorHandler.handleError(error, 'ChatView.handleUserInput'); + this.cleanupStreamingResources(); } finally { if (this.sendButton) { (this.sendButton as HTMLButtonElement).disabled = false; @@ -1366,7 +1366,7 @@ export class ChatView extends ItemView {
Code coverage generated by istanbul - at 2026-05-04T18:57:30.341Z + at 2026-05-06T13:14:32.634Z