From f74286f3c8e77d1021e5755ab38f232d62175676 Mon Sep 17 00:00:00 2001 From: Andrew Beal Date: Sat, 4 Jul 2026 15:39:12 +0100 Subject: [PATCH] Fix toolCallStarted to emit when function name first becomes available Ensures toolCallStarted fires once when the accumulated function name first becomes non-empty, handling providers that stream function names across multiple deltas. Also adds Obsidian DOM helper mocks for tests. --- .../ChatCompletions/ChatCompletionsAIClass.ts | 15 ++- AIClasses/Gemini/Gemini.ts | 8 ++ __tests__/AIClasses/Gemini.test.ts | 35 +++++++ __tests__/AIClasses/Local.test.ts | 22 +++++ __tests__/AIClasses/OpenAI.test.ts | 18 ++++ __tests__/setup.ts | 91 ++++++++++++++++++- 6 files changed, 184 insertions(+), 5 deletions(-) diff --git a/AIClasses/ChatCompletions/ChatCompletionsAIClass.ts b/AIClasses/ChatCompletions/ChatCompletionsAIClass.ts index 28275f3..529d315 100644 --- a/AIClasses/ChatCompletions/ChatCompletionsAIClass.ts +++ b/AIClasses/ChatCompletions/ChatCompletionsAIClass.ts @@ -28,6 +28,8 @@ export abstract class ChatCompletionsAIClass extends BaseAIClass { // Accumulation state for streaming tool calls private accumulatedToolCalls: Map = new Map(); + // Indices for which toolCallStarted has already been emitted this stream + private startedToolCalls: Set = new Set(); public async* streamRequest(conversation: Conversation): AsyncGenerator { const messages = await this.buildMessages(conversation); @@ -62,6 +64,7 @@ export abstract class ChatCompletionsAIClass extends BaseAIClass { private async buildMessages(conversation: Conversation): Promise { this.accumulatedToolCalls.clear(); + this.startedToolCalls.clear(); // Refresh file cache only if conversation has attachments if (conversation.hasAttachments()) { @@ -115,10 +118,6 @@ export abstract class ChatCompletionsAIClass extends BaseAIClass { name: tc.function?.name || "", args: tc.function?.arguments || "" }); - - if (tc.function?.name) { - toolCallStarted = tc.function.name; - } } else { // Accumulate arguments for existing tool call const existing = this.accumulatedToolCalls.get(index)!; @@ -126,6 +125,14 @@ export abstract class ChatCompletionsAIClass extends BaseAIClass { if (tc.function?.name) existing.name += tc.function.name; if (tc.function?.arguments) existing.args += tc.function.arguments; } + + // Some providers stream the function name fragmented across multiple deltas rather than + // whole in the first one, so fire "started" the moment the accumulated name first becomes non-empty. + const accumulated = this.accumulatedToolCalls.get(index)!; + if (!this.startedToolCalls.has(index) && accumulated.name) { + this.startedToolCalls.add(index); + toolCallStarted = accumulated.name; + } } } diff --git a/AIClasses/Gemini/Gemini.ts b/AIClasses/Gemini/Gemini.ts index aa7df5e..f873abf 100644 --- a/AIClasses/Gemini/Gemini.ts +++ b/AIClasses/Gemini/Gemini.ts @@ -151,6 +151,7 @@ export class Gemini extends BaseAIClass { let text = ""; let toolCall: AIToolCall | undefined = undefined; + let toolCallStarted: string | undefined = undefined; const candidate = data.candidates?.[0]; if (candidate) { @@ -163,6 +164,12 @@ export class Gemini extends BaseAIClass { const parts = candidate.content?.parts || []; for (const part of parts) { if (part.functionCall) { + // Signal tool call start the first time we see its name + // Gemini delivers the whole call - name, args, and finishReason - in a single chunk, so this must NOT short-circuit the finalize logic below + if (!this.accumulatedFunctionName && part.functionCall.name) { + toolCallStarted = part.functionCall.name; + } + // Accumulate function name if (part.functionCall.name) { this.accumulatedFunctionName = part.functionCall.name; @@ -204,6 +211,7 @@ export class Gemini extends BaseAIClass { content: text, isComplete: isComplete, toolCall: toolCall, + toolCallStarted: toolCallStarted, shouldContinue: shouldContinue, }; } catch (error) { diff --git a/__tests__/AIClasses/Gemini.test.ts b/__tests__/AIClasses/Gemini.test.ts index ba424ea..b505672 100644 --- a/__tests__/AIClasses/Gemini.test.ts +++ b/__tests__/AIClasses/Gemini.test.ts @@ -160,6 +160,41 @@ describe('Gemini', () => { expect((gemini as any).accumulatedFunctionArgs).toEqual({ query: 'test' }); }); + it('should emit toolCallStarted on the first chunk revealing the function name, and not again for subsequent chunks', () => { + const startChunk = JSON.stringify({ + candidates: [{ + content: { + parts: [{ + functionCall: { + name: 'search_vault_files', + args: { query: 'test' } + } + }] + } + }] + }); + + const startResult = (gemini as any).parseStreamChunk(startChunk); + expect(startResult.toolCallStarted).toBe('search_vault_files'); + expect(startResult.isComplete).toBe(false); + + const continuationChunk = JSON.stringify({ + candidates: [{ + content: { + parts: [{ + functionCall: { + name: 'search_vault_files', + args: { limit: 10 } + } + }] + } + }] + }); + + const continuationResult = (gemini as any).parseStreamChunk(continuationChunk); + expect(continuationResult.toolCallStarted).toBeUndefined(); + }); + it('should capture thoughtSignature when present in part', () => { const signature = 'base64EncodedSignature=='; const chunk = JSON.stringify({ diff --git a/__tests__/AIClasses/Local.test.ts b/__tests__/AIClasses/Local.test.ts index 302319c..d56c8c4 100644 --- a/__tests__/AIClasses/Local.test.ts +++ b/__tests__/AIClasses/Local.test.ts @@ -424,6 +424,28 @@ describe('Local', () => { expect(result.isComplete).toBe(false); }); + it('should emit toolCallStarted when a tool call delta first reveals a function name', () => { + const chunk = JSON.stringify({ + choices: [{ + delta: { + tool_calls: [{ + index: 0, + id: 'call_123', + type: 'function', + function: { + name: 'search_vault_files', + arguments: '{"query":' + } + }] + }, + finish_reason: null + }] + }); + + const result = (local as any).parseStreamChunk(chunk); + expect(result.toolCallStarted).toBe('search_vault_files'); + }); + it('should extract simple text content via the shared extractContents', async () => { const contents = [ new ConversationContent({ role: Role.User, content: 'Hello', displayContent: 'Hello' }) diff --git a/__tests__/AIClasses/OpenAI.test.ts b/__tests__/AIClasses/OpenAI.test.ts index c4f5ffb..2bf45e1 100644 --- a/__tests__/AIClasses/OpenAI.test.ts +++ b/__tests__/AIClasses/OpenAI.test.ts @@ -130,6 +130,24 @@ describe('OpenAI', () => { expect(result.isComplete).toBe(false); }); + it('should emit toolCallStarted on output_item.added for a function_call item', () => { + const chunk = JSON.stringify({ + type: 'response.output_item.added', + output_index: 0, + item: { + id: 'item_123', + type: 'function_call', + name: 'search_vault_files', + call_id: 'call_123' + } + }); + + const result = (openai as any).parseStreamChunk(chunk); + + expect(result.toolCallStarted).toBe('search_vault_files'); + expect(result.isComplete).toBe(false); + }); + it('should handle complete function call in output_item.done event', () => { // Responses API provides the complete function call in response.output_item.done event const chunk = JSON.stringify({ diff --git a/__tests__/setup.ts b/__tests__/setup.ts index dd41cf9..5d45d56 100644 --- a/__tests__/setup.ts +++ b/__tests__/setup.ts @@ -26,13 +26,102 @@ if (typeof (global as any).activeDocument === 'undefined') { // Note: Component class is now defined in __mocks__/obsidian.ts // The vitest alias in vitest.config.ts handles mocking 'obsidian' imports -// Add Obsidian's .empty() method to HTMLElement prototype for testing +// Add Obsidian's DOM helpers (empty(), createDiv(), createEl(), createSpan(), createFragment()) +// for testing. Obsidian patches these onto HTMLElement.prototype at runtime, and also +// exposes createDiv/createEl/createSpan/createFragment as bare globals. if (typeof HTMLElement !== 'undefined') { HTMLElement.prototype.empty = function() { while (this.firstChild) { this.removeChild(this.firstChild); } }; + + type DomElementInfo = { + cls?: string | string[]; + text?: string | DocumentFragment; + attr?: Record; + title?: string; + parent?: Node; + value?: string; + type?: string; + prepend?: boolean; + placeholder?: string; + href?: string; + }; + + function applyElementInfo(el: HTMLElement, options?: string | DomElementInfo): void { + if (!options) return; + const info: DomElementInfo = typeof options === 'string' ? { cls: options } : options; + + if (info.cls) { + const classes = Array.isArray(info.cls) ? info.cls : [info.cls]; + el.classList.add(...classes.filter(Boolean)); + } + if (info.text !== undefined) { + if (info.text instanceof DocumentFragment) { + el.appendChild(info.text); + } else { + el.textContent = info.text; + } + } + if (info.title !== undefined) el.setAttribute('title', info.title); + if (info.value !== undefined) el.setAttribute('value', info.value); + if (info.type !== undefined) el.setAttribute('type', info.type); + if (info.href !== undefined) el.setAttribute('href', info.href); + if (info.placeholder !== undefined) el.setAttribute('placeholder', info.placeholder); + if (info.attr) { + for (const [key, value] of Object.entries(info.attr)) { + if (value === null) continue; + el.setAttribute(key, String(value)); + } + } + + const parent = info.parent ?? el.parentNode; + if (parent) { + if (info.prepend) { + parent.insertBefore(el, parent.firstChild); + } else if (parent !== el.parentNode) { + parent.appendChild(el); + } + } + } + + function createElImpl( + this: HTMLElement | void, + tag: K, + options?: string | DomElementInfo, + callback?: (el: HTMLElementTagNameMap[K]) => void + ): HTMLElementTagNameMap[K] { + const el = document.createElement(tag); + applyElementInfo(el, options); + if (this instanceof HTMLElement && el.parentNode !== this) { + this.appendChild(el); + } + if (callback) callback(el); + return el; + } + + HTMLElement.prototype.createEl = function(tag: string, options?: string | DomElementInfo, callback?: (el: HTMLElement) => void) { + return createElImpl.call(this, tag as keyof HTMLElementTagNameMap, options, callback as any); + }; + HTMLElement.prototype.createDiv = function(options?: string | DomElementInfo, callback?: (el: HTMLDivElement) => void) { + return this.createEl('div', options, callback as any); + }; + HTMLElement.prototype.createSpan = function(options?: string | DomElementInfo, callback?: (el: HTMLSpanElement) => void) { + return this.createEl('span', options, callback as any); + }; + + (global as any).createEl = (tag: string, options?: string | DomElementInfo, callback?: (el: HTMLElement) => void) => + createElImpl.call(undefined, tag as keyof HTMLElementTagNameMap, options, callback as any); + (global as any).createDiv = (options?: string | DomElementInfo, callback?: (el: HTMLDivElement) => void) => + createElImpl.call(undefined, 'div', options, callback as any); + (global as any).createSpan = (options?: string | DomElementInfo, callback?: (el: HTMLSpanElement) => void) => + createElImpl.call(undefined, 'span', options, callback as any); + (global as any).createFragment = (callback?: (el: DocumentFragment) => void) => { + const fragment = document.createDocumentFragment(); + if (callback) callback(fragment); + return fragment; + }; } // Clean up after each test