Skip to content

Commit d6af8ee

Browse files
committed
fix: address Round 3 cr-loop findings (lifecycle + edge cases)
Round 3 confirmation surfaced subtler tool-call lifecycle and terminal-event idempotency gaps that earlier rounds missed. openai-base/adapters/chat-completions-text.ts - Move RUN_STARTED emission BEFORE the `if (!choice) continue` guard so a stream that arrives entirely as usage-only chunks (no choices) still emits RUN_STARTED. Without this the post-loop synthetic terminal block (which gates on hasEmittedRunStarted) skipped emission entirely, producing zero events for the run. - Skip non-started entries in the finish_reason TOOL_CALL_END loop and in the post-loop synthetic close. A delta with index but no id/name populates toolCallsInProgress without ever emitting TOOL_CALL_START; emitting TOOL_CALL_END for those entries violated AG-UI lifecycle (END without matching START) and produced empty toolCallId payloads consumers couldn't pair. - Clear toolCallsInProgress after emitting TOOL_CALL_END, and gate the synthetic finishReason on a separate pendingToolCount counter rather than `toolCallsInProgress.size > 0`. Previously, a tool-then-clean-stop run reported finishReason: 'tool_calls' because the map was never cleared after the in-loop emission. - parsedInput now coerces non-object JSON to {} (parallel to the Responses adapter's existing guard). A bare string/number/null in the arguments wire would otherwise propagate to tool execution as the TOOL_CALL_END.input. - Close any started tool calls in the post-loop synthetic block on truncated streams (no finish_reason ever arrives) so consumers see a matching TOOL_CALL_END for every TOOL_CALL_START. - convertMessage: throw on a single-text user message whose content is empty (paid request with no input). Previously the early-return bypassed the multipart fail-loud guard. - convertMessage: assistant tool-only messages now use `content: null` instead of `content: ''`, matching the OpenAI Chat Completions contract; empty-string content interacts oddly with tokenization on some backends. - convertContentPart image branch defaults the data-URI MIME to `application/octet-stream` when source.mimeType is undefined, instead of interpolating literal "undefined" into the URI. openai-base/adapters/responses-text.ts - response.failed / response.incomplete now ALWAYS emit RUN_ERROR (even when both `error` and `incomplete_details` are missing) and `return` out of the loop. Previously, an incomplete event with no detail silently coerced the run to RUN_FINISHED { finishReason: 'stop' } via the post-loop synthetic block, masking the failure. The early return also stops further chunks from being processed after a terminal event. - response.output_item.added: emit TOOL_CALL_START only when the metadata's started flag is false. A duplicate output_item.added for the same item id (retried streams / replay) no longer emits a second TOOL_CALL_START. - response.function_call_arguments.delta: drop the `args` field. Its `metadata ? undefined : chunk.delta` polarity was inverted (populated only on orphan-tool-call paths), the chat-completions adapter never emitted it, and consumers should accumulate `delta` themselves. - response.function_call_arguments.done: skip TOOL_CALL_END (and log) when the metadata's started flag is false, mirroring the chat- completions adapter's started-guard. Emitting END without a matching START broke AG-UI consumers' lifecycle pairing. - handleContentPart RUN_ERROR is now treated as terminal: set runFinishedEmitted = true and `return` out of the loop. Previously the loop kept processing and the synthetic terminal block could emit a second RUN_FINISHED after a refusal. - Mark hasStreamedContentDeltas / hasStreamedReasoningDeltas in the content_part.added handler so a subsequent matching content_part.done doesn't re-emit the same text (the existing skip guard for done events depended on these flags being true). - handleContentPart's reasoning_text branch caches the fallback stepId (`if (!stepId) stepId = generateId(...)`) instead of generating a fresh id on each call. Multiple reasoning content parts arriving without a preceding STEP_STARTED no longer emit different stepIds. - convertContentPartToInput image and audio: default the data-URI MIME to application/octet-stream when source.mimeType is undefined. openai-base/adapters/{chat-completions,responses}-tool-converter.ts - Shallow-copy the schemaConverter result before mutating additionalProperties. A subclass-supplied passthrough converter could otherwise have its caller's schema mutated by the `jsonSchema.additionalProperties = false` assignment. openai-base/adapters/video.ts - getVideoUrl SDK fall-through (content/getContent/download): convert the binary response to a data URL like the downloadContent path already does. Previously the bottom `return { url: response.url }` returned the API endpoint URL (or undefined for non-Response shapes) in place of a usable video URL. openai-base/package.json - Drop `zod` from peerDependencies and devDependencies. Grep confirms no zod imports anywhere in the package; the peer dep produced a spurious peer-warning on every consumer. Verification: - pnpm test:types : 32 projects pass - pnpm test:lib : 31 projects pass (116+ unit tests) - pnpm test:eslint : 31 projects pass - pnpm build : 33 projects pass Deferred bucket (b/c) follow-ups (not in this commit): - Include tests/ in ai-utils + openai-base tsconfig.json (requires fixing ~20 pre-existing test-file TS issues: EventType enum literals vs string literals, possibly-undefined narrowing, mock typing). - ai-fal peer-dep workspace:* → workspace:^ (pre-existing on main). - defineModelMeta gaps (output.cached, NaN, integer context_window). - transformNullsToUndefined runtime guard for non-plain objects.
1 parent 47f357a commit d6af8ee

7 files changed

Lines changed: 242 additions & 76 deletions

File tree

packages/typescript/openai-base/package.json

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,11 @@
4444
"openai": "^6.9.1"
4545
},
4646
"peerDependencies": {
47-
"@tanstack/ai": "workspace:^",
48-
"zod": "^4.0.0"
47+
"@tanstack/ai": "workspace:^"
4948
},
5049
"devDependencies": {
5150
"@tanstack/ai": "workspace:*",
5251
"@vitest/coverage-v8": "4.0.14",
53-
"vite": "^7.2.7",
54-
"zod": "^4.2.0"
52+
"vite": "^7.2.7"
5553
}
5654
}

packages/typescript/openai-base/src/adapters/chat-completions-text.ts

Lines changed: 93 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -278,11 +278,12 @@ export class OpenAICompatibleChatCompletionsTextAdapter<
278278
lastModel = chunk.model
279279
}
280280

281-
const choice = chunk.choices[0]
282-
283-
if (!choice) continue
284-
285-
// Emit RUN_STARTED on first chunk
281+
// Emit RUN_STARTED on the first chunk of any kind so callers see a
282+
// run lifecycle even on streams that arrive entirely as usage-only
283+
// (no choices). Without this, a usage-first stream would skip
284+
// RUN_STARTED via `if (!choice) continue` below and the post-loop
285+
// synthetic block would also skip RUN_FINISHED (it gates on
286+
// `hasEmittedRunStarted`).
286287
if (!aguiState.hasEmittedRunStarted) {
287288
aguiState.hasEmittedRunStarted = true
288289
yield asChunk({
@@ -293,6 +294,10 @@ export class OpenAICompatibleChatCompletionsTextAdapter<
293294
})
294295
}
295296

297+
const choice = chunk.choices[0]
298+
299+
if (!choice) continue
300+
296301
const delta = choice.delta
297302
const deltaContent = delta.content
298303
const deltaToolCalls = delta.tool_calls
@@ -386,19 +391,34 @@ export class OpenAICompatibleChatCompletionsTextAdapter<
386391
// we can capture the trailing usage chunk that arrives AFTER this
387392
// chunk when stream_options.include_usage is on.
388393
if (choice.finish_reason) {
389-
// Emit all completed tool calls
394+
// Track whether ANY tool call actually got a start event so we can
395+
// distinguish "tool-using run" from "stream had partial deltas but
396+
// never completed a tool call" — the latter must NOT report
397+
// tool_calls and must NOT emit TOOL_CALL_END for unstarted entries.
398+
let emittedAnyToolCallEnd = false
390399
if (
391400
choice.finish_reason === 'tool_calls' ||
392401
toolCallsInProgress.size > 0
393402
) {
394403
for (const [, toolCall] of toolCallsInProgress) {
404+
// Skip tool calls that never emitted TOOL_CALL_START — emitting
405+
// a stray TOOL_CALL_END here would violate AG-UI lifecycle
406+
// (END without matching START) for partial deltas where the
407+
// upstream never sent both id and name.
408+
if (!toolCall.started) continue
409+
395410
// Parse arguments for TOOL_CALL_END. Surface parse failures via
396411
// the logger so a model emitting malformed JSON for tool args
397412
// is debuggable instead of silently invoking the tool with {}.
413+
// Non-object JSON (e.g. a bare string or number) is also coerced
414+
// to {} so downstream tool execution doesn't receive a primitive
415+
// input, mirroring the Responses adapter's guard.
398416
let parsedInput: unknown = {}
399417
if (toolCall.arguments) {
400418
try {
401-
parsedInput = JSON.parse(toolCall.arguments)
419+
const parsed: unknown = JSON.parse(toolCall.arguments)
420+
parsedInput =
421+
parsed && typeof parsed === 'object' ? parsed : {}
402422
} catch (parseError) {
403423
options.logger.errors(
404424
`${this.name}.processStreamChunks tool-args JSON parse failed`,
@@ -427,8 +447,14 @@ export class OpenAICompatibleChatCompletionsTextAdapter<
427447
timestamp,
428448
input: parsedInput,
429449
})
450+
emittedAnyToolCallEnd = true
430451
}
452+
// Clear tool-call state after emission so a subsequent
453+
// `finish_reason: 'stop'` chunk (or the post-loop synthetic
454+
// block) doesn't see lingering entries and misreport the finish.
455+
toolCallsInProgress.clear()
431456
}
457+
void emittedAnyToolCallEnd
432458

433459
// Emit TEXT_MESSAGE_END if we had text content
434460
if (hasEmittedTextMessageStart) {
@@ -453,6 +479,35 @@ export class OpenAICompatibleChatCompletionsTextAdapter<
453479
// guaranteed terminal event even when the upstream cuts off mid-stream
454480
// (no finish_reason chunk ever arrives).
455481
if (aguiState.hasEmittedRunStarted) {
482+
// Close any started tool calls that never got finish_reason. A
483+
// truncated stream that emitted TOOL_CALL_START but never reached
484+
// finish_reason would otherwise leave consumers with an unbalanced
485+
// start. Skip non-started entries (no matching START to close).
486+
let pendingToolCount = 0
487+
for (const [, toolCall] of toolCallsInProgress) {
488+
if (!toolCall.started) continue
489+
let parsedInput: unknown = {}
490+
if (toolCall.arguments) {
491+
try {
492+
const parsed: unknown = JSON.parse(toolCall.arguments)
493+
parsedInput = parsed && typeof parsed === 'object' ? parsed : {}
494+
} catch {
495+
parsedInput = {}
496+
}
497+
}
498+
yield asChunk({
499+
type: 'TOOL_CALL_END',
500+
toolCallId: toolCall.id,
501+
toolCallName: toolCall.name,
502+
toolName: toolCall.name,
503+
model: lastModel || options.model,
504+
timestamp,
505+
input: parsedInput,
506+
})
507+
pendingToolCount += 1
508+
}
509+
toolCallsInProgress.clear()
510+
456511
// Make sure the text message lifecycle is closed even on early
457512
// termination paths where finish_reason never arrives.
458513
if (hasEmittedTextMessageStart) {
@@ -467,9 +522,12 @@ export class OpenAICompatibleChatCompletionsTextAdapter<
467522
// Map upstream finish_reason to AG-UI's narrower vocabulary while
468523
// preserving the upstream value when it falls outside the AG-UI set.
469524
// Collapsing length / content_filter to 'stop' would hide why the
470-
// run terminated — surface it instead.
525+
// run terminated — surface it instead. Use `tool_calls` only when
526+
// the upstream actually said so OR when we just closed pending tool
527+
// calls (truncated stream); a clean `stop` with no started tool
528+
// calls must NOT be remapped to `tool_calls`.
471529
const finishReason: string =
472-
pendingFinishReason === 'tool_calls' || toolCallsInProgress.size > 0
530+
pendingFinishReason === 'tool_calls' || pendingToolCount > 0
473531
? 'tool_calls'
474532
: (pendingFinishReason ?? 'stop')
475533

@@ -602,11 +660,17 @@ export class OpenAICompatibleChatCompletionsTextAdapter<
602660
: JSON.stringify(tc.function.arguments),
603661
},
604662
}))
663+
const hasToolCalls = !!toolCalls && toolCalls.length > 0
664+
const textContent = this.extractTextContent(message.content)
605665

666+
// Per the OpenAI Chat Completions contract, an assistant message that
667+
// only carries tool_calls should have `content: null` (or omit content)
668+
// rather than `content: ''`. Empty-string content interacts oddly with
669+
// tokenization on some backends; null is the documented shape.
606670
return {
607671
role: 'assistant',
608-
content: this.extractTextContent(message.content),
609-
...(toolCalls && toolCalls.length > 0 ? { tool_calls: toolCalls } : {}),
672+
content: hasToolCalls && !textContent ? null : textContent,
673+
...(hasToolCalls ? { tool_calls: toolCalls } : {}),
610674
}
611675
}
612676

@@ -615,9 +679,20 @@ export class OpenAICompatibleChatCompletionsTextAdapter<
615679

616680
// If only text, use simple string format
617681
if (contentParts.length === 1 && contentParts[0]?.type === 'text') {
682+
const text = contentParts[0].content
683+
if (text.length === 0) {
684+
// Single empty text part is the same fail-loud condition as below —
685+
// an empty paid request mask a real intent (caller passed `null`/'',
686+
// or an upstream step normalised everything to an empty string).
687+
throw new Error(
688+
`User message for ${this.name} has empty text content. ` +
689+
`Empty user messages would produce a paid request with no input; ` +
690+
`provide non-empty content or omit the message.`,
691+
)
692+
}
618693
return {
619694
role: 'user',
620-
content: contentParts[0].content,
695+
content: text,
621696
}
622697
}
623698

@@ -672,11 +747,15 @@ export class OpenAICompatibleChatCompletionsTextAdapter<
672747
| { detail?: 'auto' | 'low' | 'high' }
673748
| undefined
674749

675-
// For base64 data, construct a data URI using the mimeType from source
750+
// For base64 data, construct a data URI using the mimeType from source.
751+
// Default to a generic octet-stream MIME if the source didn't provide
752+
// one — interpolating `undefined` into the URI ("data:undefined;base64,
753+
// ...") would produce an invalid URI the API rejects.
676754
const imageValue = part.source.value
755+
const imageMime = part.source.mimeType || 'application/octet-stream'
677756
const imageUrl =
678757
part.source.type === 'data' && !imageValue.startsWith('data:')
679-
? `data:${part.source.mimeType};base64,${imageValue}`
758+
? `data:${imageMime};base64,${imageValue}`
680759
: imageValue
681760

682761
return {

packages/typescript/openai-base/src/adapters/chat-completions-tool-converter.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,13 @@ export function convertFunctionToolToChatCompletionsFormat(
3333
required: [],
3434
}) as JSONSchema
3535

36-
const jsonSchema = schemaConverter(inputSchema, inputSchema.required || [])
37-
38-
// Ensure additionalProperties is false for strict mode
36+
// Shallow-copy the converter's result before mutating: a subclass-supplied
37+
// schemaConverter has no contract requirement to return a fresh object,
38+
// and a passthrough `(s) => s` would otherwise have its caller's schema
39+
// mutated by the `additionalProperties = false` assignment below.
40+
const jsonSchema = {
41+
...schemaConverter(inputSchema, inputSchema.required || []),
42+
}
3943
jsonSchema.additionalProperties = false
4044

4145
return {

0 commit comments

Comments
 (0)