diff --git a/CHANGELOG.md b/CHANGELOG.md index f197487..e77f645 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ ### Changes +- [Provider] Work a tool already did is no longer redone when the turn it was part of breaks partway + through. A model turn can run several tool calls before it fails — because the model asked for a + tool that does not exist, answered in plain text where a tool call was demanded, or ran the + conversation past the model's context limit. The whole turn was then retried from its starting + point, with no trace that the earlier calls had already gone through, so everything they had + created got created again, once per retry. The calls that completed now carry over into the retry + and into the conversation, and the model continues from them instead of repeating them. - Doc Collector: a `docs collect` run streamed with `--ws` now sends the spec index it generates as a `docs` frame — the file path and the full markdown of `docs/index.md` — so a listening UI can show the finished documentation the same way an exploration run streams its session report. diff --git a/src/ai/provider.ts b/src/ai/provider.ts index 0378000..0bad26d 100644 --- a/src/ai/provider.ts +++ b/src/ai/provider.ts @@ -426,10 +426,20 @@ export class Provider { const config = this.buildGenerateConfig({ tools: toolsWithCommentary, maxOutputTokens: 16384, toolChoice: 'auto', experimental_repairToolCall: repairToolCall }, { stopWhen: stopConditions, model }, options); let attemptMessages = messages; let invalidRequestFeedbackAdded = false; + const executedStepMessages: ModelMessage[] = []; try { const response = await this.withModelRequestSlot(() => withRetry(async () => { - const result = (await this.raceWithIdleTimeout((signal) => generateText({ messages: attemptMessages, ...config, abortSignal: signal }), config.timeout || 30000).catch((error) => { + const stepMessages: ModelMessage[] = []; + const onStepEnd = (step: any) => { + stepMessages.push(...(step.response?.messages || [])); + }; + const result = (await this.raceWithIdleTimeout((signal) => generateText({ messages: attemptMessages, ...config, abortSignal: signal, onStepEnd }), config.timeout || 30000).catch((error) => { + if (stepMessages.length > 0) { + tag('warning').log(`Keeping ${stepMessages.length} messages from tool steps that already ran before the failure`); + executedStepMessages.push(...stepMessages); + attemptMessages = [...attemptMessages, ...stepMessages]; + } if (!invalidRequestFeedbackAdded) { const amended = withInvalidRequestFeedback(attemptMessages, error); invalidRequestFeedbackAdded = amended !== attemptMessages; @@ -448,6 +458,8 @@ export class Provider { clearActivity(); + withExecutedSteps(response, executedStepMessages); + // Log tool usage summary if (response.toolCalls && response.toolCalls.length > 0) { responseLog(response.toolCalls); @@ -462,12 +474,13 @@ export class Provider { } catch (error: any) { clearActivity(); if (error?.message?.includes('Tool choice is required')) { - return { text: '', toolCalls: [], toolResults: [], response: { messages: [] }, usage: null }; + return { text: '', toolCalls: [], toolResults: [], responseMessages: executedStepMessages, usage: null }; } if (error?.name === 'AbortError') throw error; if (error instanceof ContextLengthError) throw error; if (Provider.isContextLengthError(error)) { - return this.recoverFromContextLength(error, messages, options, (m, o) => this.generateWithTools(m, model, tools, o)); + const recovered = await this.recoverFromContextLength(error, attemptMessages, options, (m, o) => this.generateWithTools(m, model, tools, o)); + return withExecutedSteps(recovered, executedStepMessages); } if (error.constructor?.name === 'AI_APICallError') { responseLog(error.message); @@ -703,6 +716,11 @@ function repairToolCall(options: ToolCallRepairOptions): any | null { return repairHarmonyChannel(options); } +function withExecutedSteps(result: any, executed: ModelMessage[]): any { + if (executed.length === 0) return result; + return Object.defineProperty(result, 'responseMessages', { value: [...executed, ...(result.responseMessages || [])], configurable: true, enumerable: true }); +} + function withInvalidRequestFeedback(messages: ModelMessage[], error: unknown): ModelMessage[] { if (!(error instanceof APICallError) || error.statusCode !== 400) return messages; tag('warning').log('Provider rejected the request as invalid — relaying its reason before the retry'); diff --git a/tests/unit/provider.test.ts b/tests/unit/provider.test.ts index c8627b3..441620f 100644 --- a/tests/unit/provider.test.ts +++ b/tests/unit/provider.test.ts @@ -348,6 +348,93 @@ describe('Provider', () => { expect(JSON.stringify(relayed.content)).toContain('attempted to call a tool which was not in request.tools'); }); + it('carries tool steps that already ran into the retry', async () => { + const prompts: any[] = []; + let calls = 0; + let created = 0; + const model = new MockLanguageModelV3({ + provider: 'test', + modelId: 'test', + doGenerate: async (params: any) => { + calls++; + prompts.push(params.prompt); + if (calls === 1) { + return { + text: undefined, + toolCalls: [{ toolCallId: 'c1', toolName: 'create', args: { name: 'suite' } }], + finishReason: 'tool-calls' as const, + usage: { inputTokens: 1, outputTokens: 1 }, + content: [{ type: 'tool-call' as const, toolCallId: 'c1', toolName: 'create', input: JSON.stringify({ name: 'suite' }) }], + }; + } + if (calls === 2) { + throw new APICallError({ url: 'http://test.local/v1/chat', statusCode: 400, message: 'Tool call validation failed: attempted to call a tool which was not in request.tools' }); + } + return { text: 'done', finishReason: 'stop', usage: { inputTokens: 1, outputTokens: 1 }, content: [{ type: 'text' as const, text: 'done' }] }; + }, + }); + aiConfig.retryDelay = 1; + const retryingProvider = new Provider(aiConfig); + const tools = { + create: tool({ + description: 'Create a record', + inputSchema: z.object({ name: z.string() }), + execute: async () => { + created++; + return { id: 1 }; + }, + }), + }; + + const response = await retryingProvider.generateWithTools([{ role: 'user', content: 'go' }], model, tools, { maxRetries: 3 }); + + expect(created).toBe(1); + expect(JSON.stringify(prompts[2])).toContain('c1'); + expect(response.responseMessages.filter((m: any) => m.role === 'tool')).toHaveLength(1); + expect(response.responseMessages.at(-1).content).toEqual([{ type: 'text', text: 'done' }]); + }); + + it('returns the tool steps that ran when the provider rejects every retry', async () => { + let calls = 0; + let created = 0; + const model = new MockLanguageModelV3({ + provider: 'test', + modelId: 'test', + doGenerate: async () => { + calls++; + if (calls === 1) { + return { + text: undefined, + toolCalls: [{ toolCallId: 'c1', toolName: 'create', args: { name: 'suite' } }], + finishReason: 'tool-calls' as const, + usage: { inputTokens: 1, outputTokens: 1 }, + content: [{ type: 'tool-call' as const, toolCallId: 'c1', toolName: 'create', input: JSON.stringify({ name: 'suite' }) }], + }; + } + throw new APICallError({ url: 'http://test.local/v1/chat', statusCode: 400, message: 'Tool choice is required, but model did not call a tool' }); + }, + }); + aiConfig.retryDelay = 1; + const retryingProvider = new Provider(aiConfig); + const tools = { + create: tool({ + description: 'Create a record', + inputSchema: z.object({ name: z.string() }), + execute: async () => { + created++; + return { id: 1 }; + }, + }), + }; + + const response = await retryingProvider.generateWithTools([{ role: 'user', content: 'go' }], model, tools, { maxRetries: 2 }); + + expect(created).toBe(1); + expect(response.text).toBe(''); + expect(response.responseMessages).toHaveLength(2); + expect(response.responseMessages[1].role).toBe('tool'); + }); + it('should honor configured retry attempts and delay', () => { aiConfig.retryAttempts = 7; aiConfig.retryDelay = 250; @@ -659,6 +746,49 @@ describe('Provider', () => { expect(response.text).toBe('recovered'); expect(calls).toBe(2); }); + + it('keeps tool steps that already ran when it reduces messages', async () => { + const prompts: any[] = []; + let calls = 0; + let created = 0; + const model = new MockLanguageModelV3({ + provider: 'test', + modelId: 'ctx-tools-model', + doGenerate: async (params: any) => { + calls++; + prompts.push(params.prompt); + if (calls === 1) { + return { + text: undefined, + toolCalls: [{ toolCallId: 'c1', toolName: 'create', args: { name: 'suite' } }], + finishReason: 'tool-calls' as const, + usage: { inputTokens: 1, outputTokens: 1 }, + content: [{ type: 'tool-call' as const, toolCallId: 'c1', toolName: 'create', input: JSON.stringify({ name: 'suite' }) }], + }; + } + if (calls === 2) throw new Error('context length exceeded'); + return { text: 'recovered', finishReason: 'stop' as const, usage: { inputTokens: 1, outputTokens: 1 }, content: [{ type: 'text' as const, text: 'recovered' }] }; + }, + }); + const tools = { + create: tool({ + description: 'Create a record', + inputSchema: z.object({ name: z.string() }), + execute: async () => { + created++; + return { id: 1 }; + }, + }), + }; + const messages: ModelMessage[] = [{ role: 'user', content: `${'x'.repeat(5000)}` }]; + + const response = await provider.generateWithTools(messages, model, tools, { maxRetries: 1 }); + + expect(created).toBe(1); + expect(response.text).toBe('recovered'); + expect(JSON.stringify(prompts[2])).toContain('c1'); + expect(response.responseMessages.filter((m: any) => m.role === 'tool')).toHaveLength(1); + }); }); describe('abort on idle timeout', () => {