Skip to content

Commit 338a54a

Browse files
committed
fix: address Round 1 cr-loop findings (subject-scope bucket-a)
Six-agent unbiased review surfaced behavioural and audit-hygiene regressions across the extracted base classes. Fixes: openai-base/adapters/chat-completions-text.ts - Defer RUN_FINISHED until end-of-stream so the trailing usage-only chunk that OpenAI emits AFTER finish_reason (when stream_options.include_usage is on) is captured. Previous fix emitted RUN_FINISHED with usage: undefined and dropped the trailing chunk. - Forward upstream finish_reason ('length', 'content_filter', etc.) instead of collapsing all non-tool reasons to 'stop'. - Surface tool-args JSON parse failures via logger.errors so a model emitting malformed JSON for tool arguments is debuggable instead of silently invoking the tool with input: {}. - Fix mapOptionsToRequest precedence: `{...modelOptions, temperature: options.temperature, ...}` clobbered `modelOptions.temperature` with undefined whenever the caller didn't set the top-level option. Now spreads top-level fields only when defined. - Throw on user message with zero content parts instead of silently sending content: '' (a paid request with no input). openai-base/adapters/responses-text.ts - Only reset accumulators on response.created. Previous code reset on response.failed/incomplete too, which left TEXT_MESSAGE_START events unbalanced when a response failed mid-stream. On terminal failure events we now emit TEXT_MESSAGE_END before RUN_ERROR. - Synthesize a terminal RUN_FINISHED if the stream ends without response.completed (truncated upstream connection), matching the chat-completions adapter's behavior so consumers always see a terminal event for every started run. - Surface tool-args JSON parse failures via logger.errors (parallel to chat-completions fix). - Distinguish refusals from unsupported content_part types in handleContentPart instead of always reporting 'Unknown refusal'. - Fix mapOptionsToRequest precedence: previously `...modelOptions` was spread LAST, silently shadowing the canonical top-level fields. Now matches the chat-completions adapter (modelOptions first, then defined top-level options), so callers tuning either backend see identical behaviour. - extractTextFromResponse now throws a distinct refusal error instead of returning '' and letting JSON.parse('') produce a confusing 'Failed to parse structured output as JSON. Content: ' message. openai-base/adapters/summarize.ts - generateId(this.name) for the result id (was hard-coded to ''). - Throw when chatStream emits RUN_ERROR instead of pretending a failed run succeeded with summary: ''. openai-base/adapters/{image,transcription,tts}.ts - Wrap SDK calls in try/catch + logger.errors with toRunErrorPayload so raw SDK errors (which can carry request metadata including auth headers) never reach user-supplied loggers. Matches the audit hygiene main #465 applied to ai-openai/text.ts before the extraction. - tts.ts: cross-runtime ArrayBuffer→base64 helper so the adapter works in browser/edge runtimes that lack Buffer. - transcription.ts: confidence calculation no longer treats avg_logprob === 0 (perfect-confidence) as missing. openai-base/tools/file-search-tool.ts - validateMaxNumResults(0) now correctly throws (was skipped because `if (maxNumResults && ...)` short-circuited on falsy zero). ai-openai/adapters/transcription.ts - shouldDefaultToVerbose returns true ONLY for whisper-1. The gpt-4o-transcribe* models reject 'verbose_json' with HTTP 400, so defaulting them to verbose was a guaranteed-failure regression. The previous logic was inverted. ai-openai/tools/{file-search,image-generation}-tool.ts - Drop locally duplicated validators; import from openai-base. The local copy of validateMaxNumResults had the same falsy-zero bug. ai-openai/tools/computer-use-tool.ts - Document that the brand discriminator ('computer_use') intentionally differs from the runtime tool name ('computer_use_preview'). The brand matches the model-meta capability union; the runtime name matches the OpenAI SDK literal that the special-tool dispatcher switches on. ai-utils/src/transforms.ts - Document transformNullsToUndefined's actual behavior (object keys are removed, top-level null becomes undefined, arrays preserve positional null). Restrict scope to JSON-shaped values; class instances/Date/Map/Set are not preserved by Object.entries recursion. ai-elevenlabs/src/utils/client.ts - Wire getElevenLabsApiKeyFromEnv and generateId through @tanstack/ai-utils so the dependency added during the merge is actually consumed (was unused after main's #504 SDK migration). ai-openrouter/package.json - Move @tanstack/ai from dependencies to devDependencies (it's already declared as a peerDependency). Avoids dual-instance hazards where the workspace package gets installed twice. Also bump the peer range to workspace:^ to match every other adapter. .changeset/refactor-providers-to-shared-packages.md - Correct the description: only ai-openai/ai-grok/ai-groq inherit from @tanstack/openai-base; the other six providers only consume @tanstack/ai-utils. Previous wording implied all nine adapters delegated to openai-base. Verification: - pnpm test:lib : 31 projects pass (116+ unit tests) - pnpm test:types : 32 projects pass - pnpm test:eslint : 31 projects pass - pnpm build : 33 projects pass
1 parent ff0c683 commit 338a54a

16 files changed

Lines changed: 384 additions & 159 deletions

File tree

.changeset/refactor-providers-to-shared-packages.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,4 @@
1010
'@tanstack/ai-elevenlabs': patch
1111
---
1212

13-
Internal refactor: delegate shared utilities to `@tanstack/ai-utils` and OpenAI-compatible adapter logic to `@tanstack/openai-base`. No breaking changes — all public APIs remain identical.
13+
Internal refactor: every provider now delegates `getApiKeyFromEnv` / `generateId` / `transformNullsToUndefined` / `ModelMeta` helpers to the new `@tanstack/ai-utils` package. `ai-openai`, `ai-grok`, and `ai-groq` additionally inherit OpenAI-compatible adapter base classes from the new `@tanstack/openai-base` package (Chat Completions / Responses text adapters, image / summarize / transcription / TTS / video adapters, schema converter, tool converters). The other providers (`ai-anthropic`, `ai-gemini`, `ai-ollama`, `ai-openrouter`, `ai-fal`, `ai-elevenlabs`) only consume `@tanstack/ai-utils` because they speak provider-native protocols, not OpenAI-compatible ones. No breaking changes — all public APIs remain identical.

packages/typescript/ai-elevenlabs/src/utils/client.ts

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
11
import { ElevenLabsClient } from '@elevenlabs/elevenlabs-js'
2+
import {
3+
getApiKeyFromEnv,
4+
generateId as sharedGenerateId,
5+
} from '@tanstack/ai-utils'
26
import type { ElevenLabsOutputFormat } from '../model-meta'
37

48
/**
@@ -39,14 +43,7 @@ function getEnvironment(): EnvObject | undefined {
3943
}
4044

4145
export function getElevenLabsApiKeyFromEnv(): string {
42-
const key = getEnvironment()?.ELEVENLABS_API_KEY
43-
if (!key) {
44-
throw new Error(
45-
'ELEVENLABS_API_KEY is required. Please set it in your environment ' +
46-
'variables or pass it explicitly to the adapter factory.',
47-
)
48-
}
49-
return key
46+
return getApiKeyFromEnv('ELEVENLABS_API_KEY')
5047
}
5148

5249
export function getElevenLabsAgentIdFromEnv(): string {
@@ -80,9 +77,9 @@ export function createElevenLabsClient(
8077
})
8178
}
8279

83-
export function generateId(prefix: string): string {
84-
return `${prefix}-${Date.now()}-${Math.random().toString(36).substring(2)}`
85-
}
80+
// Re-exported from `@tanstack/ai-utils` so existing callers keep working
81+
// while the implementation stays deduped across provider packages.
82+
export const generateId = sharedGenerateId
8683

8784
/**
8885
* Convert an ArrayBuffer to base64 in a cross-runtime way.

packages/typescript/ai-openai/src/adapters/transcription.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,10 @@ export class OpenAITranscriptionAdapter<
3535
}
3636

3737
protected override shouldDefaultToVerbose(model: string): boolean {
38-
return model !== 'whisper-1'
38+
// Only Whisper supports `verbose_json`. The gpt-4o-* transcribe models
39+
// accept only `json` and `text` and reject `verbose_json` with HTTP 400,
40+
// so they must NOT default to verbose. The previous logic was inverted.
41+
return model === 'whisper-1'
3942
}
4043
}
4144

packages/typescript/ai-openai/src/tools/computer-use-tool.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ export {
77
convertComputerUseToolToAdapterFormat,
88
} from '@tanstack/openai-base'
99

10+
// The brand discriminator (`computer_use`) intentionally differs from the
11+
// runtime tool name (`computer_use_preview`). The brand matches the model-meta
12+
// tool-capability union (`tools: ['computer_use', ...]`) used to gate which
13+
// models can construct this tool at compile time, while the runtime name
14+
// matches the OpenAI SDK's literal `'computer_use_preview'` that the
15+
// special-tool dispatcher in `convertToolsToProviderFormat` switches on.
1016
export type OpenAIComputerUseTool = ProviderTool<'openai', 'computer_use'>
1117

1218
/**

packages/typescript/ai-openai/src/tools/file-search-tool.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { validateMaxNumResults } from '@tanstack/openai-base'
12
import type { ProviderTool } from '@tanstack/ai'
23
import type { FileSearchToolConfig } from '@tanstack/openai-base'
34

@@ -7,12 +8,6 @@ export {
78
convertFileSearchToolToAdapterFormat,
89
} from '@tanstack/openai-base'
910

10-
const validateMaxNumResults = (maxNumResults: number | undefined) => {
11-
if (maxNumResults && (maxNumResults < 1 || maxNumResults > 50)) {
12-
throw new Error('max_num_results must be between 1 and 50.')
13-
}
14-
}
15-
1611
export type OpenAIFileSearchTool = ProviderTool<'openai', 'file_search'>
1712

1813
/**

packages/typescript/ai-openai/src/tools/image-generation-tool.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { validatePartialImages } from '@tanstack/openai-base'
12
import type { ProviderTool } from '@tanstack/ai'
23
import type { ImageGenerationToolConfig } from '@tanstack/openai-base'
34

@@ -12,12 +13,6 @@ export type OpenAIImageGenerationTool = ProviderTool<
1213
'image_generation'
1314
>
1415

15-
const validatePartialImages = (value: number | undefined) => {
16-
if (value !== undefined && (value < 0 || value > 3)) {
17-
throw new Error('partial_images must be between 0 and 3')
18-
}
19-
}
20-
2116
/**
2217
* Creates a standard Tool from ImageGenerationTool parameters, branded as an
2318
* OpenAI provider tool.

packages/typescript/ai-openrouter/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,15 @@
4444
],
4545
"dependencies": {
4646
"@openrouter/sdk": "0.12.14",
47-
"@tanstack/ai": "workspace:*",
4847
"@tanstack/ai-utils": "workspace:*"
4948
},
5049
"devDependencies": {
50+
"@tanstack/ai": "workspace:*",
5151
"@vitest/coverage-v8": "4.0.14",
5252
"vite": "^7.2.7",
5353
"zod": "^4.2.0"
5454
},
5555
"peerDependencies": {
56-
"@tanstack/ai": "workspace:*"
56+
"@tanstack/ai": "workspace:^"
5757
}
5858
}

packages/typescript/ai-utils/src/transforms.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,22 @@
11
/**
2-
* Recursively converts null values to undefined in an object.
3-
* Used after receiving structured output from OpenAI-compatible providers,
4-
* which return null for optional fields that were made nullable in the
5-
* JSON Schema strict mode transformation.
2+
* Recursively strip `null` values from a JSON-shaped value so optional fields
3+
* present as `null` in OpenAI-compatible structured output round-trip cleanly
4+
* through Zod schemas that expect `undefined` (or absence) instead of `null`.
5+
*
6+
* Behaviour:
7+
* - Top-level `null` becomes `undefined`.
8+
* - Object properties whose value is `null` are removed entirely (so
9+
* `'key' in result` is `false`). Zod's `.optional()` treats absent keys
10+
* the same as `undefined`, which is the round-trip we want; setting the
11+
* key to `undefined` would still register the property in `Object.keys`
12+
* and break some `.strict()`/`Object.keys`-based callers.
13+
* - Arrays recurse element-wise; `null` array elements stay `null` (an
14+
* array element is positional and removing it would shift indices).
15+
*
16+
* Scope: designed for `JSON.parse` output (plain objects, arrays, strings,
17+
* numbers, booleans, null). Class instances, `Date`, `Map`, `Set`, etc. are
18+
* NOT preserved — they are returned as fresh empty objects because the
19+
* function recurses via `Object.entries`. Don't pass non-JSON values.
620
*/
721
export function transformNullsToUndefined<T>(obj: T): T {
822
if (obj === null) {

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

Lines changed: 87 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -242,16 +242,19 @@ export class OpenAICompatibleChatCompletionsTextAdapter<
242242
let accumulatedContent = ''
243243
const timestamp = aguiState.timestamp
244244
let hasEmittedTextMessageStart = false
245+
let lastModel: string | undefined
245246
// Track usage from any chunk that carries it. With
246247
// `stream_options: { include_usage: true }` OpenAI emits a terminal chunk
247248
// whose `choices` is `[]` and only the `usage` field is populated; the
248-
// earlier `finish_reason` chunk does not include token counts. We capture
249-
// usage here so RUN_FINISHED can be emitted with accurate numbers
250-
// regardless of which chunk delivered them.
249+
// earlier `finish_reason` chunk does NOT include token counts. We must
250+
// therefore defer RUN_FINISHED until the iterator is exhausted so we can
251+
// pick up usage from the trailing chunk regardless of arrival order.
251252
let lastUsage:
252253
| OpenAI_SDK.Chat.Completions.ChatCompletionChunk['usage']
253254
| undefined
254-
let runFinishedEmitted = false
255+
let pendingFinishReason:
256+
| OpenAI_SDK.Chat.Completions.ChatCompletionChunk.Choice['finish_reason']
257+
| undefined
255258

256259
// Track tool calls being streamed (arguments come in chunks)
257260
const toolCallsInProgress = new Map<
@@ -271,6 +274,9 @@ export class OpenAICompatibleChatCompletionsTextAdapter<
271274
if (chunk.usage) {
272275
lastUsage = chunk.usage
273276
}
277+
if (chunk.model) {
278+
lastModel = chunk.model
279+
}
274280

275281
const choice = chunk.choices[0]
276282

@@ -373,22 +379,42 @@ export class OpenAICompatibleChatCompletionsTextAdapter<
373379
}
374380
}
375381

376-
// Handle finish reason
382+
// Handle finish reason. We DO emit TOOL_CALL_END and TEXT_MESSAGE_END
383+
// here because the corresponding _START events have already fired,
384+
// and tool execution downstream wants to begin as soon as possible.
385+
// RUN_FINISHED is deferred until the iterator is fully exhausted so
386+
// we can capture the trailing usage chunk that arrives AFTER this
387+
// chunk when stream_options.include_usage is on.
377388
if (choice.finish_reason) {
378389
// Emit all completed tool calls
379390
if (
380391
choice.finish_reason === 'tool_calls' ||
381392
toolCallsInProgress.size > 0
382393
) {
383394
for (const [, toolCall] of toolCallsInProgress) {
384-
// Parse arguments for TOOL_CALL_END
395+
// Parse arguments for TOOL_CALL_END. Surface parse failures via
396+
// the logger so a model emitting malformed JSON for tool args
397+
// is debuggable instead of silently invoking the tool with {}.
385398
let parsedInput: unknown = {}
386-
try {
387-
parsedInput = toolCall.arguments
388-
? JSON.parse(toolCall.arguments)
389-
: {}
390-
} catch {
391-
parsedInput = {}
399+
if (toolCall.arguments) {
400+
try {
401+
parsedInput = JSON.parse(toolCall.arguments)
402+
} catch (parseError) {
403+
options.logger.errors(
404+
`${this.name}.processStreamChunks tool-args JSON parse failed`,
405+
{
406+
error: toRunErrorPayload(
407+
parseError,
408+
`tool ${toolCall.name} (${toolCall.id}) returned malformed JSON arguments`,
409+
),
410+
source: `${this.name}.processStreamChunks`,
411+
toolCallId: toolCall.id,
412+
toolName: toolCall.name,
413+
rawArguments: toolCall.arguments,
414+
},
415+
)
416+
parsedInput = {}
417+
}
392418
}
393419

394420
// Emit AG-UI TOOL_CALL_END
@@ -404,12 +430,6 @@ export class OpenAICompatibleChatCompletionsTextAdapter<
404430
}
405431
}
406432

407-
const computedFinishReason =
408-
choice.finish_reason === 'tool_calls' ||
409-
toolCallsInProgress.size > 0
410-
? 'tool_calls'
411-
: 'stop'
412-
413433
// Emit TEXT_MESSAGE_END if we had text content
414434
if (hasEmittedTextMessageStart) {
415435
yield asChunk({
@@ -418,49 +438,45 @@ export class OpenAICompatibleChatCompletionsTextAdapter<
418438
model: chunk.model || options.model,
419439
timestamp,
420440
})
441+
hasEmittedTextMessageStart = false
421442
}
422443

423-
// Capture usage from this chunk if present. The terminal usage-only
424-
// chunk (when stream_options.include_usage is on) typically arrives
425-
// AFTER this finish_reason chunk, so prefer the later one when both
426-
// are available; here we just merge what we have so far.
427-
const usage = chunk.usage ?? lastUsage
428-
429-
// Emit AG-UI RUN_FINISHED
430-
yield asChunk({
431-
type: 'RUN_FINISHED',
432-
runId: aguiState.runId,
433-
model: chunk.model || options.model,
434-
timestamp,
435-
usage: usage
436-
? {
437-
promptTokens: usage.prompt_tokens || 0,
438-
completionTokens: usage.completion_tokens || 0,
439-
totalTokens: usage.total_tokens || 0,
440-
}
441-
: undefined,
442-
finishReason: computedFinishReason,
443-
})
444-
runFinishedEmitted = true
444+
// Remember the upstream finish_reason; RUN_FINISHED is emitted at
445+
// end-of-stream so we pick up the trailing usage-only chunk too.
446+
pendingFinishReason = choice.finish_reason
445447
}
446448
}
447449

448-
// If the stream ended without a `finish_reason` chunk (e.g. an aborted
449-
// stream that still managed to deliver the terminal usage chunk), emit
450-
// a synthetic RUN_FINISHED so callers always see a terminal event.
451-
if (!runFinishedEmitted && aguiState.hasEmittedRunStarted) {
450+
// Emit a single terminal RUN_FINISHED after the iterator is exhausted.
451+
// This both delivers accurate token counts (the trailing usage chunk
452+
// may arrive AFTER the finish_reason chunk) and gives consumers a
453+
// guaranteed terminal event even when the upstream cuts off mid-stream
454+
// (no finish_reason chunk ever arrives).
455+
if (aguiState.hasEmittedRunStarted) {
456+
// Make sure the text message lifecycle is closed even on early
457+
// termination paths where finish_reason never arrives.
452458
if (hasEmittedTextMessageStart) {
453459
yield asChunk({
454460
type: 'TEXT_MESSAGE_END',
455461
messageId: aguiState.messageId,
456-
model: options.model,
462+
model: lastModel || options.model,
457463
timestamp,
458464
})
459465
}
466+
467+
// Map upstream finish_reason to AG-UI's narrower vocabulary while
468+
// preserving the upstream value when it falls outside the AG-UI set.
469+
// Collapsing length / content_filter to 'stop' would hide why the
470+
// run terminated — surface it instead.
471+
const finishReason: string =
472+
pendingFinishReason === 'tool_calls' || toolCallsInProgress.size > 0
473+
? 'tool_calls'
474+
: (pendingFinishReason ?? 'stop')
475+
460476
yield asChunk({
461477
type: 'RUN_FINISHED',
462478
runId: aguiState.runId,
463-
model: options.model,
479+
model: lastModel || options.model,
464480
timestamp,
465481
usage: lastUsage
466482
? {
@@ -469,7 +485,7 @@ export class OpenAICompatibleChatCompletionsTextAdapter<
469485
totalTokens: lastUsage.total_tokens || 0,
470486
}
471487
: undefined,
472-
finishReason: toolCallsInProgress.size > 0 ? 'tool_calls' : 'stop',
488+
finishReason,
473489
})
474490
}
475491
} catch (error: unknown) {
@@ -528,13 +544,22 @@ export class OpenAICompatibleChatCompletionsTextAdapter<
528544

529545
const modelOptions = options.modelOptions
530546

547+
// Build the request so explicit top-level options win over modelOptions
548+
// when set, but `undefined` top-level options do NOT clobber values the
549+
// caller put in modelOptions. Keeping the merge nullish-aware fixes the
550+
// silent regression where a `modelOptions: { temperature: 0.7 }` setting
551+
// was overwritten with `temperature: undefined`.
531552
return {
532553
...modelOptions,
533554
model: options.model,
534555
messages,
535-
temperature: options.temperature,
536-
max_tokens: options.maxTokens,
537-
top_p: options.topP,
556+
...(options.temperature !== undefined && {
557+
temperature: options.temperature,
558+
}),
559+
...(options.maxTokens !== undefined && {
560+
max_tokens: options.maxTokens,
561+
}),
562+
...(options.topP !== undefined && { top_p: options.topP }),
538563
tools: tools as Array<OpenAI_SDK.Chat.Completions.ChatCompletionTool>,
539564
stream: true,
540565
}
@@ -609,9 +634,20 @@ export class OpenAICompatibleChatCompletionsTextAdapter<
609634
parts.push(converted)
610635
}
611636

637+
if (parts.length === 0) {
638+
// The original message had no content parts at all (e.g. content was
639+
// explicitly null or []). Sending an empty user message to OpenAI
640+
// produces a paid request with no signal — fail loud instead.
641+
throw new Error(
642+
`User message for ${this.name} has no content parts. ` +
643+
`Empty user messages would produce a paid request with no input; ` +
644+
`provide at least one text/image/audio part or omit the message.`,
645+
)
646+
}
647+
612648
return {
613649
role: 'user',
614-
content: parts.length > 0 ? parts : '',
650+
content: parts,
615651
}
616652
}
617653

0 commit comments

Comments
 (0)