diff --git a/jest.config.js b/jest.config.js index 3186135..34667c3 100644 --- a/jest.config.js +++ b/jest.config.js @@ -55,11 +55,12 @@ export default { // because its denominator grew: vertex, mock and global rose, orchestration and its tools fell. // That is a finer instrument reading the same suite, which is why these two moved down. coverageThreshold: { - 'src/orchestration/': {statements: 96.96, branches: 92.89, functions: 97.67, lines: 97.1}, - 'src/orchestration/tools/': {statements: 92.77, branches: 92.3, functions: 92.3, lines: 94.28}, + 'src/orchestration/': {statements: 97.02, branches: 93.42, functions: 97.72, lines: 97.14}, + 'src/orchestration/tools/': {statements: 93.02, branches: 92.59, functions: 92.59, lines: 94.36}, 'src/infrastructure/stores/': {statements: 100, branches: 100, functions: 100, lines: 100}, 'src/providers/vertex/': {statements: 94.23, branches: 84.7, functions: 100, lines: 97.72}, 'src/providers/mock/': {statements: 100, branches: 81.25, functions: 100, lines: 100}, - global: {statements: 91.3, branches: 83.55, functions: 89.79, lines: 91.92}, + // Measured over the files no path floor above claims, which is what Jest holds `global` to. + global: {statements: 95.05, branches: 90.7, functions: 93.75, lines: 95.74}, }, }; diff --git a/specs/hal-engine-architecture/spec.md b/specs/hal-engine-architecture/spec.md index a672f48..17c268c 100644 --- a/specs/hal-engine-architecture/spec.md +++ b/specs/hal-engine-architecture/spec.md @@ -387,14 +387,22 @@ Here is what the message handler does when a tool call comes through: ### Whether a round continues -- A round continues only when the model stopped for `tool_use`, named at least one tool, and a `ToolRegistry` is configured ([validated by: continues when the model requested a tool and a registry can run it](../../src/orchestration/orchestratorHelpers.test.ts#L11)). -- With no registry configured the loop stops, whatever the model asked for, because nothing could execute the call ([validated by: stops when no registry is configured, whatever the model asked for](../../src/orchestration/orchestratorHelpers.test.ts#L15)). -- A `tool_use` stop naming no tool stops the loop, rather than re-querying with nothing to add ([validated by: stops when the model named no tool](../../src/orchestration/orchestratorHelpers.test.ts#L19)). -- Any other stop reason ends the loop even when tool calls are pending ([validated by: stops when the model finished for a reason other than tool use](../../src/orchestration/orchestratorHelpers.test.ts#L23)). +- A round continues only when the model stopped for `tool_use`, named at least one tool, and a `ToolRegistry` is configured ([validated by: continues when the model requested a tool and a registry can run it](../../src/orchestration/orchestratorHelpers.test.ts#L18)). +- With no registry configured the loop stops, whatever the model asked for, because nothing could execute the call ([validated by: stops when no registry is configured, whatever the model asked for](../../src/orchestration/orchestratorHelpers.test.ts#L22)). +- A `tool_use` stop naming no tool stops the loop, rather than re-querying with nothing to add ([validated by: stops when the model named no tool](../../src/orchestration/orchestratorHelpers.test.ts#L26)). +- Any other stop reason ends the loop even when tool calls are pending ([validated by: stops when the model finished for a reason other than tool use](../../src/orchestration/orchestratorHelpers.test.ts#L30)). +- The gate is the whole truth table of its three conditions: of the eight combinations of stop reason, pending calls and registry, only `tool_use` with a pending call and a registry continues ([validated by: continues for tool_use with a pending call and a registry alone, and stops for the other seven rows](../../src/orchestration/orchestratorHelpers.test.ts#L61)). +- Seen from outside, a scripted `tool_use` turn with no registry ends after one round: one provider call, no executor, and the next scripted round never streams ([validated by: ends a scripted tool_use turn after one round with no registry: one provider call, the tool_use and stop chunks alone](../../src/toolLoop.test.ts#L172)). +- A round that stops with `end_turn` despite carrying a tool call ends the turn with zero executions ([validated by: ends after one round with zero executions when the stop reason is end_turn despite a tool call: the second scripted round never streams](../../src/orchestration/chatOrchestrator.test.ts#L884)). +- A stream that ends without a `stop` chunk leaves the stop reason empty, so the turn ends after that round with zero executions and no second provider call ([validated by: ends after one round with zero executions when the stream ends without a stop chunk: the second scripted round never streams](../../src/orchestration/chatOrchestrator.test.ts#L888)). +- The helpers the round is built from are pure. `collectText` joins the text chunks of a stream and ignores every other type ([validated by: joins the text chunks of a stream in order and ignores every other chunk type](../../src/orchestration/orchestratorHelpers.test.ts#L87)). +- `collectToolCalls` appends a `tool_use` chunk's call to the pending list and ignores the rest ([validated by: appends the call of a tool_use chunk to the pending list and leaves it alone for text and stop chunks](../../src/orchestration/orchestratorHelpers.test.ts#L95)). +- `extractStopReason` reads a stop chunk and answers `null` otherwise ([validated by: reads the reason off a stop chunk and answers null for a text chunk](../../src/orchestration/orchestratorHelpers.test.ts#L108)). +- `extractUsage` reads a stop chunk's usage and answers `undefined` for a stop without one or any other chunk ([validated by: reads the usage off a stop chunk that reports one, and answers undefined for a stop without usage and for a text chunk](../../src/orchestration/orchestratorHelpers.test.ts#L118)). ### Across rounds -- A tool call's `signature`, an opaque token its provider needs back, is copied onto its `tool_use` block for the next round, and a call without one gets no `signature` key ([validated by: puts sig-1 on the signed call's tool_use block and no signature key on the unsigned one](../../src/orchestration/orchestratorHelpers.test.ts#L29)). +- A tool call's `signature`, an opaque token its provider needs back, is copied onto its `tool_use` block for the next round, and a call without one gets no `signature` key ([validated by: puts sig-1 on the signed call's tool_use block and no signature key on the unsigned one](../../src/orchestration/orchestratorHelpers.test.ts#L36)). - A call to a tool name the model invented is answered rather than raised, with a result naming it and the registered tools, and the turn goes on ([validated by: answers a call to a tool name the model invented, and the turn goes on to its answer](../../src/orchestration/chatOrchestrator.test.ts#L757)). - So is a call whose executor throws: it gets a result saying the call failed, its message reaches only the log, as [tool-executor-throw](../hal-engine-tool-executor-throw/spec.md) specifies, and the turn goes on ([validated by: answers a call whose executor threw, and the turn goes on to its answer with afterModelResponse and no onError](../../src/orchestration/chatOrchestrator.test.ts#L770)). - A tool signals a failed call by returning `isError: true` beside its `result`, and the engine marks the failures it substitutes the same way, so the provider is told the call failed instead of leaving the model to infer it from prose; [the tool responses spec](../hal-engine-tool-responses/spec.md) gives each provider's encoding ([validated by: carries isError true from an executor's {result: 'lookup failed', isError: true} onto the tool_result the provider's next call gets](../../src/orchestration/toolResultError.test.ts#L62)). @@ -405,6 +413,14 @@ Here is what the message handler does when a tool call comes through: - The usage `afterModelResponse` receives is `totalUsage`: the sum of what every provider call in the turn reported, so a turn that ran tools counts every round rather than the last one alone ([validated by: sums the usage of every round, not only the last](../../src/orchestration/chatOrchestrator.test.ts#L616)). - A round that reports no usage drops out of the sum rather than voiding it: the rounds that did report are still counted, and a turn where nothing reported reports nothing ([validated by: counts every reporting round even when a round between them reports none](../../src/orchestration/chatOrchestrator.test.ts#L638)). +### A round with more than one call + +- A round carrying two calls runs both executors, and the next round's request carries one assistant message with two `tool_use` items and one user message with two `tool_result` items whose ids match, in the order the model asked ([validated by: runs both executors of a two-call round and sends the next round one assistant message with two tool_use items and one user message with two tool_result items whose ids match, in script order](../../src/toolLoop.test.ts#L63)). +- When one of two executors suppresses the answer, exactly one `suppress_output` chunk follows the round's `tool_result` chunk ([validated by: yields exactly one suppress_output chunk, after the tool_result chunk, when one of two executors suppresses](../../src/toolLoop.test.ts#L91)). +- Two `entry_upsert` messages from one round take sequential indices from the session's length at call time, and both entries are readable from the session afterwards ([validated by: gives two upserted entries sequential indices from the session length at call time, and both are readable from the session afterwards](../../src/toolLoop.test.ts#L113)). +- A round whose executors return no client messages yields no `tool_result` chunk at all ([validated by: yields no tool_result chunk for a round whose executors return no client messages](../../src/toolLoop.test.ts#L148)). +- The pending calls are a fresh list each round, so a call from one round never reaches the next round's request ([validated by: carries no pending call from one round into the next: three rounds leave two assistant messages with one tool_use item each, and each executor ran once](../../src/toolLoop.test.ts#L159)). + The frontend shows a spinner on the last tool entry while the stream is still processing (`isProcessing` is `true`). The `stream_end` message clears the processing state, which hides the spinner. This works correctly even when tool suppression prevents assistant entries from reaching the frontend. See [tool-responses.md](../hal-engine-tool-responses/spec.md) for details on client messages and suppression. ## Thinking Tag Parsing diff --git a/src/orchestration/chatOrchestrator.test.ts b/src/orchestration/chatOrchestrator.test.ts index 11a10e4..13faac7 100644 --- a/src/orchestration/chatOrchestrator.test.ts +++ b/src/orchestration/chatOrchestrator.test.ts @@ -862,3 +862,30 @@ async function abortedMidStream(end: () => void): Promise<{outcome: string; call return {outcome: await outcomeOf(chunksFrom(stream)), callOrder}; } + +// A scripted first round, then a second round that must never stream; the registry holds the tool TOOL_CALL names and records every run. +const gatedTurn = async (firstRound: MessageChunk[]) => { + const ran: string[] = []; + const provider = scriptedProvider([firstRound, [{type: 'text', text: 'never'}, STOP_DONE]]); + const registry = new ToolRegistry(); + registry.register({name: 'lookup', description: 'looks things up', inputSchema: {type: 'object'}}, async () => { + ran.push('lookup'); + return {result: 'found'}; + }); + + const chunks = await chunksFrom( + createChatOrchestrator(provider, promptBuilder, registry).processMessageStream(createSession('ask')) + ); + + return {ran, types: chunks.map(c => c.type)}; +}; + +describe('ChatOrchestrator tool loop gating, observed from outside', () => { + it('ends after one round with zero executions when the stop reason is end_turn despite a tool call: the second scripted round never streams', async () => { + expect(await gatedTurn([TOOL_CALL, STOP_DONE])).toEqual({ran: [], types: ['tool_use', 'stop']}); + }); + + it('ends after one round with zero executions when the stream ends without a stop chunk: the second scripted round never streams', async () => { + expect(await gatedTurn([TOOL_CALL])).toEqual({ran: [], types: ['tool_use']}); + }); +}); diff --git a/src/orchestration/orchestratorHelpers.test.ts b/src/orchestration/orchestratorHelpers.test.ts index 8a129d8..fe7f4dc 100644 --- a/src/orchestration/orchestratorHelpers.test.ts +++ b/src/orchestration/orchestratorHelpers.test.ts @@ -1,6 +1,13 @@ -import {buildToolCallMessages, shouldContinueToolLoop} from './orchestratorHelpers.js'; +import { + buildToolCallMessages, + collectText, + collectToolCalls, + extractStopReason, + extractUsage, + shouldContinueToolLoop, +} from './orchestratorHelpers.js'; import type {ToolRegistry} from './tools/registry.js'; -import type {ToolCall} from '../types/ai.js'; +import type {MessageChunk, ToolCall} from '../types/ai.js'; // All three conditions matter; dropping the registry check spins the tool loop. @@ -41,3 +48,80 @@ describe('buildToolCallMessages', () => { }); }); }); + +const STOP_FOR_TOOL: MessageChunk = {type: 'stop', stopReason: 'tool_use'}; +const STOP_DONE: MessageChunk = {type: 'stop', stopReason: 'end_turn'}; +const TEXT: MessageChunk = {type: 'text', text: 'word '}; + +const streamOf = async function* (chunks: MessageChunk[]): AsyncGenerator { + yield* chunks; +}; + +describe('shouldContinueToolLoop truth table', () => { + it('continues for tool_use with a pending call and a registry alone, and stops for the other seven rows', () => { + const rows = [ + ['tool_use', A_TOOL, REGISTRY], + ['tool_use', A_TOOL, undefined], + ['tool_use', [], REGISTRY], + ['tool_use', [], undefined], + ['end_turn', A_TOOL, REGISTRY], + ['end_turn', A_TOOL, undefined], + ['end_turn', [], REGISTRY], + ['end_turn', [], undefined], + ] as const; + + expect(rows.map(([reason, calls, registry]) => shouldContinueToolLoop(reason, [...calls], registry))).toEqual([ + true, + false, + false, + false, + false, + false, + false, + false, + ]); + }); +}); + +describe('collectText', () => { + it('joins the text chunks of a stream in order and ignores every other chunk type', async () => { + const tool: MessageChunk = {type: 'tool_use', toolCall: {id: 't1', name: 'lookup', input: {}}}; + + expect(await collectText(streamOf([TEXT, tool, {type: 'text', text: 'by word'}, STOP_DONE]))).toBe('word by word'); + }); +}); + +describe('collectToolCalls', () => { + it('appends the call of a tool_use chunk to the pending list and leaves it alone for text and stop chunks', () => { + const pending: ToolCall[] = []; + const call: ToolCall = {id: 't2', name: 'weather', input: {city: 'here'}}; + + collectToolCalls(TEXT, pending); + collectToolCalls({type: 'tool_use', toolCall: call}, pending); + collectToolCalls(STOP_FOR_TOOL, pending); + + expect(pending).toEqual([call]); + }); +}); + +describe('extractStopReason', () => { + it('reads the reason off a stop chunk and answers null for a text chunk', () => { + expect([extractStopReason(STOP_FOR_TOOL), extractStopReason(STOP_DONE), extractStopReason(TEXT)]).toEqual([ + 'tool_use', + 'end_turn', + null, + ]); + }); +}); + +describe('extractUsage', () => { + it('reads the usage off a stop chunk that reports one, and answers undefined for a stop without usage and for a text chunk', () => { + const usage = {inputTokens: 3, outputTokens: 4, totalTokens: 7}; + + expect([extractUsage({...STOP_DONE, usage}), extractUsage(STOP_DONE), extractUsage(TEXT)]).toEqual([ + usage, + undefined, + undefined, + ]); + }); +}); diff --git a/src/toolLoop.test.ts b/src/toolLoop.test.ts new file mode 100644 index 0000000..424c4a7 --- /dev/null +++ b/src/toolLoop.test.ts @@ -0,0 +1,181 @@ +import {createChatOrchestrator} from './orchestration/chatOrchestrator.js'; +import {ToolRegistry} from './orchestration/tools/registry.js'; +import type {ToolResponse} from './orchestration/tools/registry.js'; +import {PromptBuilder} from './infrastructure/builders/promptBuilder.js'; +import {createMockProvider} from './providers/mock/mockProvider.js'; +import type {MockTurn} from './providers/mock/mockProvider.js'; +import {collectChunks} from './providers/providerTestSupport.js'; +import type {Message, SendMessageParams, ToolCall, ToolCallContent} from './types/ai.js'; +import type {ChatSession, SessionEntry} from './types/session.js'; + +// specs/hal-engine-architecture ยง Tool Execution Loop: the cases the package's own scripted mock can drive. + +const SENT_AT = '2026-01-01T00:00:00.000Z'; +const promptBuilder = new PromptBuilder({identity: 'You are a test assistant.'}); + +const askSession = (): ChatSession => ({ + sessionId: 's1', + userId: 'u1', + entries: [{role: 'user', content: 'ask', timestamp: SENT_AT}], +}); +const upserted = (content: string): SessionEntry => ({ + role: 'assistant', + content, + timestamp: SENT_AT, + isStreaming: false, +}); + +/** Two registered tools whose executors record the order they started in; what each answers is the caller's. */ +function twoTools(answers: {first: ToolResponse; second: ToolResponse}): { + registry: ToolRegistry; + ran: string[]; +} { + const ran: string[] = []; + const registry = new ToolRegistry(); + for (const name of ['first', 'second'] as const) { + registry.register({name, description: `the ${name} tool`, inputSchema: {type: 'object'}}, async () => { + ran.push(name); + return answers[name]; + }); + } + return {registry, ran}; +} + +const CALL_A: ToolCall = {id: 'call-a', name: 'first', input: {a: 1}}; +const CALL_B: ToolCall = {id: 'call-b', name: 'second', input: {b: 2}}; + +const toolUseIdsOf = (message: Message): string[][] => { + if (message.role !== 'assistant' || !Array.isArray(message.content)) return []; + const toolUses = message.content.filter((block): block is ToolCallContent => block.type === 'tool_use'); + return [toolUses.map(block => block.toolUseId)]; +}; + +const assistantToolUseIds = (messages: Message[]): string[][] => messages.flatMap(toolUseIdsOf); + +// The package's scripted mock, keeping every request it was sent. +const recordingMock = (turns: MockTurn[]) => { + const sent: SendMessageParams[] = []; + const provider = createMockProvider({type: 'mock', turns, onSendMessage: params => void sent.push(params)}); + return {provider, sent}; +}; + +describe('ChatOrchestrator tool loop, a round with more than one call', () => { + it('runs both executors of a two-call round and sends the next round one assistant message with two tool_use items and one user message with two tool_result items whose ids match, in script order', async () => { + const {provider, sent} = recordingMock([{toolCalls: [CALL_A, CALL_B]}, {text: 'done'}]); + const {registry, ran} = twoTools({first: {result: 'one'}, second: {result: 'two'}}); + + await collectChunks(createChatOrchestrator(provider, promptBuilder, registry).processMessageStream(askSession())); + const {messages} = sent[1]; + const [assistant, user] = messages.slice(-2); + + expect({calls: sent.length, ran, assistant, user}).toEqual({ + calls: 2, + ran: ['first', 'second'], + assistant: { + role: 'assistant', + content: [ + {type: 'tool_use', toolUseId: 'call-a', name: 'first', input: {a: 1}}, + {type: 'tool_use', toolUseId: 'call-b', name: 'second', input: {b: 2}}, + ], + }, + user: { + role: 'user', + content: [ + {type: 'tool_result', toolUseId: 'call-a', content: 'one'}, + {type: 'tool_result', toolUseId: 'call-b', content: 'two'}, + ], + }, + }); + }); + + it('yields exactly one suppress_output chunk, after the tool_result chunk, when one of two executors suppresses', async () => { + const provider = createMockProvider({type: 'mock', turns: [{toolCalls: [CALL_A, CALL_B]}, {text: 'done'}]}); + const {registry} = twoTools({ + first: {result: 'one', clientMessages: [{type: 'entry_commit', index: 0}]}, + second: {result: 'two', suppressAssistantResponse: true}, + }); + + const chunks = await collectChunks( + createChatOrchestrator(provider, promptBuilder, registry).processMessageStream(askSession()) + ); + + expect(chunks.map(c => c.type)).toEqual([ + 'tool_use', + 'tool_use', + 'stop', + 'tool_result', + 'suppress_output', + 'text', + 'stop', + ]); + }); + + it('gives two upserted entries sequential indices from the session length at call time, and both are readable from the session afterwards', async () => { + const [one, two] = [upserted('first card'), upserted('second card')]; + const provider = createMockProvider({type: 'mock', turns: [{toolCalls: [CALL_A, CALL_B]}, {text: 'done'}]}); + const {registry} = twoTools({ + first: { + result: 'one', + clientMessages: [ + {type: 'entry_upsert', index: 999, entry: one}, + {type: 'entry_upsert', index: 999, entry: two}, + ], + }, + second: {result: 'two'}, + }); + const session = askSession(); + const lengthAtCall = session.entries.length; + + const chunks = await collectChunks( + createChatOrchestrator(provider, promptBuilder, registry).processMessageStream(session) + ); + + expect({ + toolResult: chunks.find(c => c.type === 'tool_result'), + appended: session.entries.slice(lengthAtCall), + }).toEqual({ + toolResult: { + type: 'tool_result', + clientMessages: [ + {type: 'entry_upsert', index: lengthAtCall, entry: one}, + {type: 'entry_upsert', index: lengthAtCall + 1, entry: two}, + ], + }, + appended: [one, two], + }); + }); + + it('yields no tool_result chunk for a round whose executors return no client messages', async () => { + const provider = createMockProvider({type: 'mock', turns: [{toolCalls: [CALL_A]}, {text: 'done'}]}); + const {registry} = twoTools({first: {result: 'one'}, second: {result: 'two'}}); + + const chunks = await collectChunks( + createChatOrchestrator(provider, promptBuilder, registry).processMessageStream(askSession()) + ); + + expect(chunks.map(c => c.type)).toEqual(['tool_use', 'stop', 'text', 'stop']); + }); + + it('carries no pending call from one round into the next: three rounds leave two assistant messages with one tool_use item each, and each executor ran once', async () => { + const {provider, sent} = recordingMock([{toolCalls: [CALL_A]}, {toolCalls: [CALL_B]}, {text: 'done'}]); + const {registry, ran} = twoTools({first: {result: 'one'}, second: {result: 'two'}}); + + await collectChunks(createChatOrchestrator(provider, promptBuilder, registry).processMessageStream(askSession())); + + expect({calls: sent.length, ran, toolUseIds: assistantToolUseIds(sent[2].messages)}).toEqual({ + calls: 3, + ran: ['first', 'second'], + toolUseIds: [['call-a'], ['call-b']], + }); + }); + + it('ends a scripted tool_use turn after one round with no registry: one provider call, the tool_use and stop chunks alone', async () => { + const {provider, sent} = recordingMock([{toolCalls: [CALL_A]}, {text: 'never'}]); + + const chunks = await collectChunks( + createChatOrchestrator(provider, promptBuilder).processMessageStream(askSession()) + ); + + expect({calls: sent.length, types: chunks.map(c => c.type)}).toEqual({calls: 1, types: ['tool_use', 'stop']}); + }); +});