Skip to content

Commit 0c65cc7

Browse files
tombeckenhamclaude
andcommitted
fix: address CodeRabbit review findings
- fal image/video: spread modelOptions after derived media fields so explicit user overrides win (matches documented intent) - openai video: validate effective size (size ?? modelOptions.size) - generate-fal-image-field-map: run arity check for default-selected fields too - ts-react-media example: correct reference-image support comment (Gemini multimodal models, not NanoBanana) - e2e VideoGenUI: reject on malformed data URL instead of resolving '' Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 349eb0d commit 0c65cc7

6 files changed

Lines changed: 25 additions & 14 deletions

File tree

examples/ts-react-media/src/components/ImageGenerator.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,8 @@ export default function ImageGenerator({
197197
Reference Images
198198
</label>
199199
<span className="text-xs text-gray-500">
200-
Supported by Gemini native (NanoBanana) models only
200+
Supported by Gemini multimodal models only
201+
(gemini-3.1-flash-image-preview, gemini-3-pro-image-preview)
201202
</span>
202203
</div>
203204
<div className="flex flex-wrap gap-2">

packages/ai-fal/src/adapters/image.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,14 +102,15 @@ export class FalImageAdapter<TModel extends FalModel> extends BaseImageAdapter<
102102
resolved: ResolvedMediaPrompt,
103103
): FalModelInput<TModel> {
104104
const sizeParams = mapSizeToFalFormat(options.size)
105-
// Order matters: modelOptions first (so user overrides win for
106-
// mask_url / control_image_url / reference_image_urls), then size,
107-
// then derived image-input fields, then prompt / num_images.
105+
// Order matters: size and derived image-input fields first, then
106+
// modelOptions (so explicit user overrides win for mask_url /
107+
// control_image_url / reference_image_urls), then the call-controlled
108+
// prompt / num_images, which always take precedence.
108109
const inputFields = mapImageInputsToFalFields(this.model, resolved.images)
109110
const input = {
110-
...options.modelOptions,
111111
...sizeParams,
112112
...inputFields,
113+
...options.modelOptions,
113114
// Media-only prompts (e.g. upscalers, background removal) omit the
114115
// prompt field entirely rather than sending an empty string.
115116
...(resolved.text ? { prompt: resolved.text } : {}),

packages/ai-fal/src/adapters/video.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,11 +162,13 @@ export class FalVideoAdapter<TModel extends FalModel> extends BaseVideoAdapter<
162162
const audioFields = mapAudioInputsToFalFields(resolved.audios)
163163

164164
const input = {
165-
...modelOptions,
166165
...sizeParams,
167166
...inputImageFields,
168167
...videoFields,
169168
...audioFields,
169+
// modelOptions applied after derived media fields so explicit user
170+
// overrides (video_url, reference_video_urls, audio_url, ...) win.
171+
...modelOptions,
170172
// Media-only prompts omit the prompt field rather than sending an
171173
// empty string (e.g. pure image-to-video endpoints).
172174
...(resolved.text ? { prompt: resolved.text } : {}),

packages/ai-openai/src/adapters/video.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,8 @@ export class OpenAIVideoAdapter<
9292
): Promise<VideoJobResult> {
9393
const { model, size, duration, modelOptions } = options
9494

95-
validateVideoSize(model, size)
95+
const resolvedSize = size ?? modelOptions?.size
96+
validateVideoSize(model, resolvedSize)
9697
const seconds = duration ?? modelOptions?.seconds
9798
validateVideoSeconds(model, seconds)
9899

@@ -129,10 +130,8 @@ export class OpenAIVideoAdapter<
129130
}
130131
// `VideoCreateParams.size` is `size?: VideoSize` (no `| undefined`), so we
131132
// narrow before assignment instead of casting from a `T | undefined` source.
132-
if (size) {
133-
request.size = size
134-
} else if (modelOptions?.size) {
135-
request.size = modelOptions.size
133+
if (resolvedSize) {
134+
request.size = resolvedSize
136135
}
137136
if (seconds !== undefined) {
138137
// `toApiSeconds` returns `OpenAIVideoSeconds | undefined`; we already

scripts/generate-fal-image-field-map.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,10 +221,12 @@ function computeOverrides(
221221
for (const role of ROLE_ORDER) {
222222
if (VIDEO_ONLY_ROLES.has(role) && !producesVideo) continue
223223
const chosen = CANDIDATES[role].find((candidate) => fields.has(candidate))
224-
if (!chosen || chosen === DEFAULTS[role]) continue
224+
if (!chosen) continue
225225

226226
// Arity sanity check: the runtime mapper decides array-wrapping from
227-
// the static LIST_FIELDS set, so the actual type must agree.
227+
// the static LIST_FIELDS set, so the actual type must agree. Run this
228+
// for default-selected fields too, otherwise a default field's type
229+
// could drift silently.
228230
const actualIsList = isList.get(chosen) ?? false
229231
const assumedIsList = LIST_FIELDS.has(chosen)
230232
if (actualIsList !== assumedIsList) {
@@ -235,6 +237,7 @@ function computeOverrides(
235237
`and LIST_FIELDS in image-inputs.ts.`,
236238
)
237239
}
240+
if (chosen === DEFAULTS[role]) continue
238241
entry[role] = chosen
239242
}
240243
if (Object.keys(entry).length > 0) overrides.set(endpointId, entry)

testing/e2e/src/components/VideoGenUI.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,12 @@ function fileToBase64(file: File): Promise<string> {
2727
reject(new Error('Unexpected FileReader result'))
2828
return
2929
}
30-
resolve(result.split(',')[1] ?? '')
30+
const base64 = result.split(',')[1]
31+
if (!base64) {
32+
reject(new Error(`Unexpected data URL format: ${result.slice(0, 32)}…`))
33+
return
34+
}
35+
resolve(base64)
3136
}
3237
reader.onerror = () => reject(new Error('Failed to read file'))
3338
reader.readAsDataURL(file)

0 commit comments

Comments
 (0)