Skip to content

Commit 256d876

Browse files
kwaakwaa
authored
refactor!: check should stop before execute tool (#320)
* refactor!: check should stop before execute tool * [autofix.ci] apply automated fixes * chore: update test
1 parent cccf481 commit 256d876

18 files changed

Lines changed: 362 additions & 120 deletions

File tree

‎packages-ext/responses/src/utils/responses.ts‎

Lines changed: 38 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import type { OpenResponsesOptions } from '../types/open-responses-options'
66
import type { StopCondition } from '../types/stop-when'
77

88
import { objCamelToSnake, postJSON, trampoline } from '@xsai/shared'
9-
import { computeTotalUsage, executeTool, resolvePrepareStep } from '@xsai/shared-chat'
9+
import { computeTotalUsage, executeTool, resolvePrepareStep, toCompletionToolCall } from '@xsai/shared-chat'
1010
import { closeControllers, createControlledStream, errorControllers, EventSourceParserStream, JsonMessageTransformStream } from '@xsai/shared-stream'
1111

1212
import { normalizeInput } from './normalize-input'
@@ -230,11 +230,7 @@ export const responses = (options: ResponsesOptions): ResponsesResult => {
230230
.getReader()
231231
}
232232

233-
const pushFunctionCallOutput = async (functionCall: FunctionCall, step: {
234-
events: Event[]
235-
toolCalls: CompletionToolCall[]
236-
toolResults: CompletionToolResult[]
237-
}) => {
233+
const executeFunctionCall = async (functionCall: FunctionCall) => {
238234
const { completionToolCall, completionToolResult, result } = await executeTool({
239235
abortSignal: options.abortSignal,
240236
messages: [],
@@ -253,35 +249,31 @@ export const responses = (options: ResponsesOptions): ResponsesResult => {
253249
type: 'function_call_output',
254250
}
255251

256-
step.toolCalls.push(completionToolCall)
257-
step.toolResults.push(completionToolResult)
258-
input.push(normalizeOutput(functionCallOutput))
259-
step.events.push({
260-
...completionToolCall,
261-
type: 'tool-call.done',
262-
}, {
263-
...completionToolResult,
264-
type: 'tool-result.done',
265-
})
252+
return { completionToolCall, completionToolResult, functionCallOutput }
266253
}
267254

268-
const handleOutputItemDone = async (event: Extract<FullEvent, { type: 'response.output_item.done' }>, step: {
255+
const handleOutputItemDone = (event: Extract<FullEvent, { type: 'response.output_item.done' }>, step: {
269256
events: Event[]
257+
functionCalls: FunctionCall[]
270258
toolCalls: CompletionToolCall[]
271-
toolResults: CompletionToolResult[]
272259
}) => {
273260
if (event.item == null)
274261
return
275262

276263
input.push(normalizeOutput(event.item))
277264

278265
if (event.item.type === 'function_call') {
279-
await pushFunctionCallOutput(event.item, step)
266+
step.functionCalls.push(event.item)
267+
const toolCall = toCompletionToolCall(toToolCall(event.item))
268+
step.toolCalls.push(toolCall)
269+
step.events.push({ ...toolCall, type: 'tool-call.done' })
280270
}
281271
}
282272

273+
// eslint-disable-next-line sonarjs/cognitive-complexity
283274
const doStream = async () => {
284275
const reader = await createReader()
276+
const functionCalls: FunctionCall[] = []
285277
const toolCalls: CompletionToolCall[] = []
286278
const toolResults: CompletionToolResult[] = []
287279

@@ -293,7 +285,7 @@ export const responses = (options: ResponsesOptions): ResponsesResult => {
293285

294286
let shouldContinue = false
295287
const events = mapFullEvent(event)
296-
const step = { events, toolCalls, toolResults }
288+
const step = { events, functionCalls, toolCalls, toolResults }
297289

298290
// eslint-disable-next-line ts/switch-exhaustiveness-check
299291
switch (event.type) {
@@ -306,13 +298,31 @@ export const responses = (options: ResponsesOptions): ResponsesResult => {
306298
toolResults,
307299
})
308300

309-
shouldContinue = input.at(-1)?.type === 'function_call_output'
310-
&& options.abortSignal?.aborted !== true
311-
&& !shouldStop(stopWhen, {
312-
input,
313-
step: completionStep,
314-
steps: [...steps, completionStep],
315-
})
301+
if (options.abortSignal?.aborted === true)
302+
throw options.abortSignal.reason ?? new Error('This operation was aborted')
303+
304+
const stop = shouldStop(stopWhen, {
305+
input,
306+
step: completionStep,
307+
steps: [...steps, completionStep],
308+
})
309+
310+
if (!stop && functionCalls.length > 0) {
311+
const stepDoneEvent = events.pop()
312+
const results = await Promise.all(functionCalls.map(executeFunctionCall))
313+
314+
toolCalls.length = 0
315+
for (const { completionToolCall, completionToolResult, functionCallOutput } of results) {
316+
toolCalls.push(completionToolCall)
317+
toolResults.push(completionToolResult)
318+
input.push(normalizeOutput(functionCallOutput))
319+
events.push({ ...completionToolResult, type: 'tool-result.done' })
320+
}
321+
if (stepDoneEvent != null)
322+
events.push(stepDoneEvent)
323+
}
324+
325+
shouldContinue = functionCalls.length > 0 && !stop && !options.abortSignal?.aborted
316326

317327
pushStep(completionStep)
318328

@@ -333,7 +343,7 @@ export const responses = (options: ResponsesOptions): ResponsesResult => {
333343
})
334344
break
335345
case 'response.output_item.done':
336-
await handleOutputItemDone(event, step)
346+
handleOutputItemDone(event, step)
337347
break
338348
default:
339349
break

‎packages-ext/responses/test/index.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { tool } from '@xsai/tool'
55
import { describe, expect, it } from 'vitest'
66
import { z } from 'zod'
77

8-
import { responses } from '../src'
8+
import { responses, stepCountAtLeast } from '../src'
99

1010
const normalizeParsedJSON = (value: unknown): unknown => {
1111
if (Array.isArray(value))
@@ -104,6 +104,7 @@ describe('@xsai-ext/responses basic', async () => {
104104
instructions: 'You are a helpful assistant.',
105105
model: 'qwen3.5:0.8b',
106106
reasoning: { effort: 'low' },
107+
stopWhen: stepCountAtLeast(2),
107108
toolChoice: 'required',
108109
tools: [add],
109110
})

‎packages-ext/telemetry/src/wrapped/generate-text.ts‎

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
import type { CompletionStep, CompletionToolCall, CompletionToolResult, GenerateTextOptions, GenerateTextResponse, GenerateTextResult, Message, TrampolineFn, WithUnknown } from 'xsai'
1+
import type { CompletionStep, CompletionToolCall, CompletionToolResult, GenerateTextOptions, GenerateTextResponse, GenerateTextResult, Message, ToolCall, TrampolineFn, WithUnknown } from 'xsai'
22

33
import type { WithTelemetry } from '../types/options'
44

5-
import { chat, computeTotalUsage, executeTool, InvalidResponseError, normalizeChatCompletionUsage, resolvePrepareStep, responseJSON, shouldStop, stepCountAtLeast, trampoline } from 'xsai'
5+
import { chat, computeTotalUsage, executeTool, InvalidResponseError, normalizeChatCompletionUsage, resolvePrepareStep, responseJSON, shouldStop, stepCountAtLeast, toCompletionToolCall, trampoline } from 'xsai'
66

77
import { getTracer } from '../utils/get-tracer'
88
import { recordSpan } from '../utils/record-span'
@@ -60,17 +60,36 @@ export const generateText = async (options: WithUnknown<WithTelemetry<GenerateTe
6060
})
6161
}
6262

63-
const toolCalls: CompletionToolCall[] = []
63+
const { finish_reason: finishReason, message } = choices[0]
64+
const msgToolCalls: ToolCall[] = message?.tool_calls ?? []
65+
const toolCalls: CompletionToolCall[] = msgToolCalls.map(toCompletionToolCall)
6466
const toolResults: CompletionToolResult[] = []
6567

66-
const { finish_reason: finishReason, message } = choices[0]
67-
const msgToolCalls = message?.tool_calls ?? []
6868
const stopWhen = options.stopWhen ?? stepCountAtLeast(1)
6969

7070
messages.push(message)
7171
span.setAttribute('gen_ai.output.messages', JSON.stringify([message]))
7272

73-
if (msgToolCalls.length > 0) {
73+
const step: CompletionStep<true> = {
74+
finishReason,
75+
text: Array.isArray(message.content)
76+
? message.content.filter(m => m.type === 'text').map(m => m.text).join('\n')
77+
: message.content,
78+
toolCalls,
79+
toolResults,
80+
usage,
81+
}
82+
83+
if (options.abortSignal?.aborted === true)
84+
throw options.abortSignal.reason ?? new Error('This operation was aborted')
85+
86+
const stop = shouldStop(stopWhen, {
87+
input: messages,
88+
step,
89+
steps: [...steps, step],
90+
})
91+
92+
if (!stop && msgToolCalls.length > 0) {
7493
const results = await Promise.all(
7594
msgToolCalls.map(async toolCall => executeTool({
7695
abortSignal: options.abortSignal,
@@ -80,6 +99,7 @@ export const generateText = async (options: WithUnknown<WithTelemetry<GenerateTe
8099
})),
81100
)
82101

102+
toolCalls.length = 0
83103
for (const { completionToolCall, completionToolResult, result } of results) {
84104
toolCalls.push(completionToolCall)
85105
toolResults.push(completionToolResult)
@@ -91,22 +111,7 @@ export const generateText = async (options: WithUnknown<WithTelemetry<GenerateTe
91111
}
92112
}
93113

94-
const step: CompletionStep<true> = {
95-
finishReason,
96-
text: Array.isArray(message.content)
97-
98-
? message.content.filter(m => m.type === 'text').map(m => m.text).join('\n')
99-
: message.content,
100-
toolCalls,
101-
toolResults,
102-
usage,
103-
}
104-
const stop = shouldStop(stopWhen, {
105-
input: messages,
106-
step,
107-
steps: [...steps, step],
108-
})
109-
const willContinue = toolCalls.length > 0 && !stop && options.abortSignal?.aborted !== true
114+
const willContinue = toolCalls.length > 0 && !stop && !options.abortSignal?.aborted
110115

111116
steps.push(step)
112117

‎packages-ext/telemetry/src/wrapped/stream-text.ts‎

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import type { WithTelemetry } from '../types/options'
44
import type { StreamTextChunkResult } from '../types/stream-text-chunk'
55

66
import { closeControllers, createControlledStream, errorControllers, EventSourceParserStream, JsonMessageTransformStream } from '@xsai/shared-stream'
7-
import { chat, computeTotalUsage, executeTool, normalizeChatCompletionUsage, objCamelToSnake, resolvePrepareStep, shouldStop, stepCountAtLeast, trampoline } from 'xsai'
7+
import { chat, computeTotalUsage, executeTool, normalizeChatCompletionUsage, objCamelToSnake, resolvePrepareStep, shouldStop, stepCountAtLeast, toCompletionToolCall, trampoline } from 'xsai'
88

99
import { getTracer } from '../utils/get-tracer'
1010
import { recordSpan } from '../utils/record-span'
@@ -105,8 +105,6 @@ export const streamText = (options: WithUnknown<WithTelemetry<StreamTextOptions>
105105
}
106106

107107
const tool_calls: ToolCall[] = []
108-
const toolCalls: CompletionToolCall[] = []
109-
const toolResults: CompletionToolResult[] = []
110108
let finishReason: FinishReason = 'other'
111109
let reasoningStarted = false
112110
let textStarted = false
@@ -221,9 +219,30 @@ export const streamText = (options: WithUnknown<WithTelemetry<StreamTextOptions>
221219
messages.push(message)
222220
span.setAttribute('gen_ai.output.messages', JSON.stringify([message]))
223221

224-
if (tool_calls.length !== 0) {
225-
const validToolCalls = tool_calls.filter((tc): tc is ToolCall => tc != null)
222+
const validToolCalls = tool_calls.filter((tc): tc is ToolCall => tc != null)
223+
const toolCalls: CompletionToolCall[] = validToolCalls.map(toCompletionToolCall)
224+
const toolResults: CompletionToolResult[] = []
225+
226+
if (options.abortSignal?.aborted === true)
227+
throw options.abortSignal.reason ?? new Error('This operation was aborted')
228+
229+
for (const toolCall of toolCalls)
230+
pushEvent({ ...toolCall, type: 'tool-call.done' })
226231

232+
const step: CompletionStep = {
233+
finishReason,
234+
text,
235+
toolCalls,
236+
toolResults,
237+
usage,
238+
}
239+
const stop = shouldStop(stopWhen, {
240+
input: messages,
241+
step,
242+
steps: [...steps, step],
243+
})
244+
245+
if (!stop && validToolCalls.length > 0) {
227246
const results = await Promise.all(
228247
validToolCalls.map(async toolCall => executeTool({
229248
abortSignal: options.abortSignal,
@@ -233,6 +252,7 @@ export const streamText = (options: WithUnknown<WithTelemetry<StreamTextOptions>
233252
})),
234253
)
235254

255+
toolCalls.length = 0
236256
for (const { completionToolCall, completionToolResult, result } of results) {
237257
toolCalls.push(completionToolCall)
238258
toolResults.push(completionToolResult)
@@ -242,24 +262,11 @@ export const streamText = (options: WithUnknown<WithTelemetry<StreamTextOptions>
242262
tool_call_id: completionToolCall.toolCallId,
243263
})
244264

245-
pushEvent({ ...completionToolCall, type: 'tool-call.done' })
246265
pushEvent({ ...completionToolResult, type: 'tool-result.done' })
247266
}
248267
}
249268

250-
const step: CompletionStep = {
251-
finishReason,
252-
text,
253-
toolCalls,
254-
toolResults,
255-
usage,
256-
}
257-
const stop = shouldStop(stopWhen, {
258-
input: messages,
259-
step,
260-
steps: [...steps, step],
261-
})
262-
const willContinue = toolCalls.length > 0 && !stop && options.abortSignal?.aborted !== true
269+
const willContinue = validToolCalls.length > 0 && !stop && !options.abortSignal?.aborted
263270
pushStep(step)
264271
pushEvent({ type: 'step.done', usage })
265272

‎packages/generate-object/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
},
4141
"dependencies": {
4242
"@xsai/generate-text": "workspace:",
43+
"@xsai/shared": "workspace:",
4344
"xsschema": "workspace:"
4445
},
4546
"devDependencies": {

‎packages/generate-object/src/index.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { GenerateTextOptions, GenerateTextResult } from '@xsai/generate-tex
22
import type { Schema } from 'xsschema'
33

44
import { generateText } from '@xsai/generate-text'
5+
import { InvalidResponseError } from '@xsai/shared'
56
import { strictJsonSchema, toJsonSchema, validate } from 'xsschema'
67

78
import { wrap } from './_wrap'
@@ -48,7 +49,13 @@ export async function generateObject<T extends Schema>(options: GenerateObjectOp
4849
schemaName: undefined, // Remove schemaName from options
4950
strict: undefined, // Remove strict from options
5051
}).then(async ({ finishReason, messages, steps, text, toolCalls, toolResults, totalUsage, usage }) => {
51-
const json: unknown = JSON.parse(text!)
52+
if (text == null || text.length === 0) {
53+
throw new InvalidResponseError('Cannot generate an object from an empty response.', {
54+
reason: 'empty_body',
55+
})
56+
}
57+
58+
const json: unknown = JSON.parse(text)
5259

5360
if (options.output === 'array') {
5461
return {

‎packages/generate-object/test/index.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,48 @@ describe('@xsai/generate-object', () => {
4949
await expect(g).rejects.toThrow()
5050
})
5151

52+
it('should throw if the response has no text', async () => {
53+
const fetch: typeof globalThis.fetch = async () => new Response(JSON.stringify({
54+
choices: [{
55+
finish_reason: 'tool_calls',
56+
index: 0,
57+
message: {
58+
content: '',
59+
role: 'assistant',
60+
tool_calls: [{
61+
function: {
62+
arguments: '{}',
63+
name: 'lookup',
64+
},
65+
id: 'call_1',
66+
type: 'function',
67+
}],
68+
},
69+
}],
70+
created: 1,
71+
id: 'chatcmpl_1',
72+
model: 'test-model',
73+
object: 'chat.completion',
74+
system_fingerprint: 'fingerprint',
75+
usage: {
76+
completion_tokens: 1,
77+
prompt_tokens: 1,
78+
total_tokens: 2,
79+
},
80+
}))
81+
82+
await expect(generateObject({
83+
baseURL: 'https://example.com/v1/',
84+
fetch,
85+
messages: [{ content: 'lookup', role: 'user' }],
86+
model: 'test-model',
87+
schema: v.object({ answer: v.string() }),
88+
})).rejects.toMatchObject({
89+
code: 'invalid_response',
90+
reason: 'empty_body',
91+
})
92+
})
93+
5294
it('object', async () => {
5395
const { object } = await generateObject({
5496
baseURL: 'http://localhost:11434/v1/',

0 commit comments

Comments
 (0)