Skip to content

Commit cd6b57a

Browse files
committed
fix: address CodeRabbit review comments on openai-base extraction
- Fix schema-converter default required parameter and null-widening for nested types - Fix removeEmptyRequired to recurse into anyOf/oneOf/allOf/additionalProperties (groq) - Forward modelOptions, request headers/signal in chat-completions-text adapter - Remove stream_options leak into non-streaming structured output calls - Use call_id instead of internal id for tool call correlation (responses-text) - Make transcription verbose_json default provider-agnostic via protected override - Add runtime guards for ArrayBuffer and atob in transcription adapter - Derive TTS outputFormat from merged request after modelOptions spread - Add downloadContent probe and fix expires_at seconds-to-milliseconds (video) - Fix mcp-tool type ordering so metadata cannot override type: 'mcp' - Add tests proving all fixes work
1 parent 23252bb commit cd6b57a

15 files changed

Lines changed: 619 additions & 55 deletions

File tree

packages/typescript/ai-groq/src/utils/schema-converter.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,28 @@ function removeEmptyRequired(schema: Record<string, any>): Record<string, any> {
3636
result.items = removeEmptyRequired(result.items)
3737
}
3838

39+
// Recurse into combinator arrays (anyOf, oneOf, allOf)
40+
for (const keyword of ['anyOf', 'oneOf', 'allOf'] as const) {
41+
if (Array.isArray(result[keyword])) {
42+
result[keyword] = result[keyword].map((entry: Record<string, any>) =>
43+
typeof entry === 'object' && entry !== null
44+
? removeEmptyRequired(entry)
45+
: entry,
46+
)
47+
}
48+
}
49+
50+
// Recurse into additionalProperties if it's a schema object
51+
if (
52+
result.additionalProperties &&
53+
typeof result.additionalProperties === 'object' &&
54+
!Array.isArray(result.additionalProperties)
55+
) {
56+
result.additionalProperties = removeEmptyRequired(
57+
result.additionalProperties,
58+
)
59+
}
60+
3961
return result
4062
}
4163

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
import { describe, expect, it } from 'vitest'
2+
import { makeGroqStructuredOutputCompatible } from '../src/utils/schema-converter'
3+
4+
describe('makeGroqStructuredOutputCompatible', () => {
5+
it('should remove empty required arrays inside anyOf variants', () => {
6+
const schema = {
7+
type: 'object',
8+
properties: {
9+
value: {
10+
anyOf: [
11+
{
12+
type: 'object',
13+
properties: {},
14+
required: [],
15+
},
16+
{ type: 'null' },
17+
],
18+
},
19+
},
20+
required: ['value'],
21+
}
22+
23+
const result = makeGroqStructuredOutputCompatible(schema, ['value'])
24+
25+
// Empty required inside anyOf variant should be removed
26+
const objectVariant = result.properties.value.anyOf.find(
27+
(v: any) => v.type === 'object',
28+
)
29+
expect(objectVariant.required).toBeUndefined()
30+
})
31+
32+
it('should remove empty required arrays inside oneOf variants', () => {
33+
const schema = {
34+
type: 'object',
35+
properties: {
36+
data: {
37+
type: 'object',
38+
properties: {
39+
inner: { type: 'string' },
40+
},
41+
required: ['inner'],
42+
},
43+
},
44+
required: ['data'],
45+
}
46+
47+
// First create a schema that would produce empty required after processing
48+
const result = makeGroqStructuredOutputCompatible(schema, ['data'])
49+
50+
// Should not have empty required arrays anywhere
51+
const checkNoEmptyRequired = (obj: any): void => {
52+
if (obj && typeof obj === 'object') {
53+
if (Array.isArray(obj.required)) {
54+
expect(obj.required.length).toBeGreaterThan(0)
55+
}
56+
for (const value of Object.values(obj)) {
57+
if (typeof value === 'object' && value !== null) {
58+
checkNoEmptyRequired(value)
59+
}
60+
}
61+
}
62+
}
63+
checkNoEmptyRequired(result)
64+
})
65+
66+
it('should remove empty required in additionalProperties', () => {
67+
const schema = {
68+
type: 'object',
69+
properties: {
70+
meta: {
71+
type: 'object',
72+
properties: {
73+
name: { type: 'string' },
74+
},
75+
required: ['name'],
76+
additionalProperties: {
77+
type: 'object',
78+
properties: {},
79+
required: [],
80+
},
81+
},
82+
},
83+
required: ['meta'],
84+
}
85+
86+
const result = makeGroqStructuredOutputCompatible(schema, ['meta'])
87+
88+
// meta should have required with allPropertyNames
89+
expect(result.properties.meta.required).toEqual(['name'])
90+
// additionalProperties' empty required should be removed
91+
if (
92+
result.properties.meta.additionalProperties &&
93+
typeof result.properties.meta.additionalProperties === 'object'
94+
) {
95+
expect(
96+
result.properties.meta.additionalProperties.required,
97+
).toBeUndefined()
98+
}
99+
})
100+
})

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ export class OpenAITranscriptionAdapter<
3333
constructor(config: OpenAITranscriptionConfig, model: TModel) {
3434
super(toCompatibleConfig(config), model, 'openai')
3535
}
36+
37+
protected override shouldDefaultToVerbose(model: string): boolean {
38+
return model !== 'whisper-1'
39+
}
3640
}
3741

3842
/**

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

Lines changed: 42 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,19 @@ export class OpenAICompatibleChatCompletionsTextAdapter<
7373
}
7474

7575
try {
76-
const stream = await this.client.chat.completions.create({
77-
...requestParams,
78-
stream: true,
79-
})
76+
const stream = await this.client.chat.completions.create(
77+
{
78+
...requestParams,
79+
stream: true,
80+
stream_options: { include_usage: true },
81+
},
82+
{
83+
headers: (options.request as RequestInit | undefined)?.headers as
84+
| Record<string, string>
85+
| undefined,
86+
signal: (options.request as RequestInit | undefined)?.signal,
87+
},
88+
)
8089

8190
yield* this.processStreamChunks(stream, options, aguiState)
8291
} catch (error: unknown) {
@@ -138,18 +147,32 @@ export class OpenAICompatibleChatCompletionsTextAdapter<
138147
)
139148

140149
try {
141-
const response = await this.client.chat.completions.create({
142-
...requestParams,
143-
stream: false,
144-
response_format: {
145-
type: 'json_schema',
146-
json_schema: {
147-
name: 'structured_output',
148-
schema: jsonSchema,
149-
strict: true,
150+
// Strip stream_options which is only valid for streaming calls
151+
const {
152+
stream_options: _,
153+
stream: __,
154+
...cleanParams
155+
} = requestParams as any
156+
const response = await this.client.chat.completions.create(
157+
{
158+
...cleanParams,
159+
stream: false,
160+
response_format: {
161+
type: 'json_schema',
162+
json_schema: {
163+
name: 'structured_output',
164+
schema: jsonSchema,
165+
strict: true,
166+
},
150167
},
151168
},
152-
})
169+
{
170+
headers: (chatOptions.request as RequestInit | undefined)?.headers as
171+
| Record<string, string>
172+
| undefined,
173+
signal: (chatOptions.request as RequestInit | undefined)?.signal,
174+
},
175+
)
153176

154177
// Extract text content from the response
155178
const rawText = response.choices[0]?.message.content || ''
@@ -436,15 +459,19 @@ export class OpenAICompatibleChatCompletionsTextAdapter<
436459
messages.push(this.convertMessage(message))
437460
}
438461

462+
const modelOptions = options.modelOptions as
463+
| Record<string, any>
464+
| undefined
465+
439466
return {
440467
model: options.model,
441468
messages,
442469
temperature: options.temperature,
443470
max_tokens: options.maxTokens,
444471
top_p: options.topP,
472+
...modelOptions,
445473
tools: tools as Array<OpenAI_SDK.Chat.Completions.ChatCompletionTool>,
446474
stream: true,
447-
stream_options: { include_usage: true },
448475
}
449476
}
450477

packages/typescript/openai-base/src/adapters/responses-text.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ export class OpenAICompatibleResponsesTextAdapter<
7878
// We assign our own indices as we encounter unique tool call IDs.
7979
const toolCallMetadata = new Map<
8080
string,
81-
{ index: number; name: string; started: boolean }
81+
{ index: number; name: string; callId: string; started: boolean }
8282
>()
8383
const requestParams = this.mapOptionsToRequest(options)
8484
const timestamp = Date.now()
@@ -274,7 +274,7 @@ export class OpenAICompatibleResponsesTextAdapter<
274274
stream: AsyncIterable<OpenAI_SDK.Responses.ResponseStreamEvent>,
275275
toolCallMetadata: Map<
276276
string,
277-
{ index: number; name: string; started: boolean }
277+
{ index: number; name: string; callId: string; started: boolean }
278278
>,
279279
options: TextOptions,
280280
aguiState: {
@@ -557,18 +557,21 @@ export class OpenAICompatibleResponsesTextAdapter<
557557
if (chunk.type === 'response.output_item.added') {
558558
const item = chunk.item
559559
if (item.type === 'function_call' && item.id) {
560+
// Use call_id for tool call correlation (required for function_call_output)
561+
const callId = (item as any).call_id || item.id
560562
// Store the function name for later use
561563
if (!toolCallMetadata.has(item.id)) {
562564
toolCallMetadata.set(item.id, {
563565
index: chunk.output_index,
564566
name: item.name || '',
567+
callId,
565568
started: false,
566569
})
567570
}
568571
// Emit TOOL_CALL_START
569572
yield {
570573
type: 'TOOL_CALL_START',
571-
toolCallId: item.id,
574+
toolCallId: callId,
572575
toolName: item.name || '',
573576
model: model || options.model,
574577
timestamp,
@@ -586,7 +589,7 @@ export class OpenAICompatibleResponsesTextAdapter<
586589
const metadata = toolCallMetadata.get(chunk.item_id)
587590
yield {
588591
type: 'TOOL_CALL_ARGS',
589-
toolCallId: chunk.item_id,
592+
toolCallId: metadata?.callId || chunk.item_id,
590593
model: model || options.model,
591594
timestamp,
592595
delta: chunk.delta,
@@ -600,6 +603,7 @@ export class OpenAICompatibleResponsesTextAdapter<
600603
// Get the function name from metadata (captured in output_item.added)
601604
const metadata = toolCallMetadata.get(item_id)
602605
const name = metadata?.name || ''
606+
const callId = metadata?.callId || item_id
603607

604608
// Parse arguments
605609
let parsedInput: unknown = {}
@@ -611,7 +615,7 @@ export class OpenAICompatibleResponsesTextAdapter<
611615

612616
yield {
613617
type: 'TOOL_CALL_END',
614-
toolCallId: item_id,
618+
toolCallId: callId,
615619
toolName: name,
616620
model: model || options.model,
617621
timestamp,

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

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ export class OpenAICompatibleTranscriptionAdapter<
6161
// Call API - use verbose_json to get timestamps when available
6262
const useVerbose =
6363
responseFormat === 'verbose_json' ||
64-
(!responseFormat && model !== 'whisper-1')
64+
(!responseFormat && this.shouldDefaultToVerbose(model))
6565

6666
if (useVerbose) {
6767
const response = await this.client.audio.transcriptions.create({
@@ -116,7 +116,7 @@ export class OpenAICompatibleTranscriptionAdapter<
116116
}
117117

118118
// If ArrayBuffer, convert to File
119-
if (audio instanceof ArrayBuffer) {
119+
if (typeof ArrayBuffer !== 'undefined' && audio instanceof ArrayBuffer) {
120120
return new File([audio], 'audio.mp3', { type: 'audio/mpeg' })
121121
}
122122

@@ -129,6 +129,11 @@ export class OpenAICompatibleTranscriptionAdapter<
129129
const base64Data = parts[1] || ''
130130
const mimeMatch = header?.match(/data:([^;]+)/)
131131
const mimeType = mimeMatch?.[1] || 'audio/mpeg'
132+
if (typeof atob !== 'function') {
133+
throw new Error(
134+
'atob is not available in this environment. Use a File, Blob, or ArrayBuffer input instead.',
135+
)
136+
}
132137
const binaryStr = atob(base64Data)
133138
const bytes = new Uint8Array(binaryStr.length)
134139
for (let i = 0; i < binaryStr.length; i++) {
@@ -139,6 +144,11 @@ export class OpenAICompatibleTranscriptionAdapter<
139144
}
140145

141146
// Assume raw base64
147+
if (typeof atob !== 'function') {
148+
throw new Error(
149+
'atob is not available in this environment. Use a File, Blob, or ArrayBuffer input instead.',
150+
)
151+
}
142152
const binaryStr = atob(audio)
143153
const bytes = new Uint8Array(binaryStr.length)
144154
for (let i = 0; i < binaryStr.length; i++) {
@@ -150,6 +160,14 @@ export class OpenAICompatibleTranscriptionAdapter<
150160
throw new Error('Invalid audio input type')
151161
}
152162

163+
/**
164+
* Whether the adapter should default to verbose_json when no response format is specified.
165+
* Override in provider-specific subclasses for model-specific behavior.
166+
*/
167+
protected shouldDefaultToVerbose(_model: string): boolean {
168+
return false
169+
}
170+
153171
protected mapResponseFormat(
154172
format?: 'json' | 'text' | 'srt' | 'verbose_json' | 'vtt',
155173
): OpenAI_SDK.Audio.TranscriptionCreateParams['response_format'] {

packages/typescript/openai-base/src/adapters/tts.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ export class OpenAICompatibleTTSAdapter<
6262
const arrayBuffer = await response.arrayBuffer()
6363
const base64 = Buffer.from(arrayBuffer).toString('base64')
6464

65-
const outputFormat = format || 'mp3'
65+
const outputFormat = (request.response_format as string) || 'mp3'
6666
const contentType = this.getContentType(outputFormat)
6767

6868
return {

0 commit comments

Comments
 (0)