fix: coerce duration props to Number() in renderers#183
fix: coerce duration props to Number() in renderers#183SecurityQQ wants to merge 1 commit intomainfrom
Conversation
Defense-in-depth fix: if user code passes duration as a string (e.g. "5" instead of 5), coerce it to a number before sending to the gateway. Prevents 'expected number, received string' Zod validation errors.
📝 Walkthroughwalkthroughduration props across music, video, and resolve renderers are being standardized to use changes
estimated code review effort🎯 2 (simple) | ⏱️ ~8 minutes poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/react/renderers/music.ts`:
- Line 30: Extract a single const duration computed once (e.g. const duration =
props.duration === undefined || props.duration === null ? undefined :
Number(props.duration)) and replace both occurrences of the current non-strict
checks and coercions (the two uses of props.duration != null and
Number(props.duration)) with that duration variable; ensure you use strict
equality (===) to check for undefined/null rather than != and then reuse
duration wherever the previous checks/coercions appeared so cache keys and
requests stay consistent.
In `@src/react/renderers/video.ts`:
- Line 153: The duration fallback currently uses "Number(props.duration) || 5",
which treats an explicit 0 as falsy and falls back to 5; update the assignment
that sets the duration (the "duration" property using props.duration) to use a
nullish check so an explicit 0 is preserved (for example, use props.duration !==
undefined/nullish-coalescing to choose between Number(props.duration) and 5) —
locate the "duration" property assignment in src/react/renderers/video.ts and
replace the || fallback with a nullish/undefined check such as using
props.duration ?? 5 or an explicit undefined check around
Number(props.duration).
In `@src/react/resolve.ts`:
- Line 650: The duration fallback currently uses "Number(props.duration) || 5"
which treats explicit 0 as falsy and overwrites it; update the duration
assignment in resolve.ts to first detect missing values (null/undefined) and
only then default to 5, otherwise coerce props.duration to a Number and fallback
to 5 if that coercion produces NaN/Infinity; reference the duration property and
props.duration so you change the expression that computes duration (replace the
current ||-based expression with a null/undefined check plus a Number.isFinite
check to preserve explicit 0).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 45ead561-203a-4c8f-bed6-742ae3116309
📒 Files selected for processing (3)
src/react/renderers/music.tssrc/react/renderers/video.tssrc/react/resolve.ts
| prompt, | ||
| model: model.modelId, | ||
| duration: props.duration, | ||
| duration: props.duration != null ? Number(props.duration) : undefined, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/react/renderers/music.ts | head -50Repository: vargHQ/sdk
Length of output: 2005
switch to strict equality and extract the duration once
lines 30 and 40 both use != null and duplicate the same coercion. violates the "always use strict equality (===)" guideline, and the duplication risks cache key + request misalignment if the logic changes later. extract const duration once and reuse it instead. meow 🐱
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/react/renderers/music.ts` at line 30, Extract a single const duration
computed once (e.g. const duration = props.duration === undefined ||
props.duration === null ? undefined : Number(props.duration)) and replace both
occurrences of the current non-strict checks and coercions (the two uses of
props.duration != null and Number(props.duration)) with that duration variable;
ensure you use strict equality (===) to check for undefined/null rather than !=
and then reuse duration wherever the previous checks/coercions appeared so cache
keys and requests stay consistent.
| model, | ||
| prompt: resolvedPrompt, | ||
| duration: props.duration ?? 5, | ||
| duration: Number(props.duration) || 5, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/react/renderers/video.ts | sed -n '145,165p'Repository: vargHQ/sdk
Length of output: 909
🏁 Script executed:
rg -n "duration.*\|\|" src/react/renderers/ -A 2 -B 2Repository: vargHQ/sdk
Length of output: 390
🏁 Script executed:
rg -n "Number.*\|\|" src/ --type ts --type tsx | head -20Repository: vargHQ/sdk
Length of output: 81
🏁 Script executed:
rg "Number.*\|\|" src/ -A 2 -B 2Repository: vargHQ/sdk
Length of output: 2699
fix zero duration fallback bug
line 153 has the same 0-to-5 regression as the resolve path: Number(props.duration) || 5 ignores explicit zero durations.
suggested patch
- duration: Number(props.duration) || 5,
+ duration: Number.isFinite(Number(props.duration)) ? Number(props.duration) : 5,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| duration: Number(props.duration) || 5, | |
| duration: Number.isFinite(Number(props.duration)) ? Number(props.duration) : 5, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/react/renderers/video.ts` at line 153, The duration fallback currently
uses "Number(props.duration) || 5", which treats an explicit 0 as falsy and
falls back to 5; update the assignment that sets the duration (the "duration"
property using props.duration) to use a nullish check so an explicit 0 is
preserved (for example, use props.duration !== undefined/nullish-coalescing to
choose between Number(props.duration) and 5) — locate the "duration" property
assignment in src/react/renderers/video.ts and replace the || fallback with a
nullish/undefined check such as using props.duration ?? 5 or an explicit
undefined check around Number(props.duration).
| model: model as Parameters<typeof generateVideoRaw>[0]["model"], | ||
| prompt: resolvedPrompt, | ||
| duration: (props.duration as number) ?? 5, | ||
| duration: Number(props.duration) || 5, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "resolve.ts" -path "*/react/*" -type fRepository: vargHQ/sdk
Length of output: 76
🏁 Script executed:
cat -n ./src/react/resolve.ts | sed -n '640,660p'Repository: vargHQ/sdk
Length of output: 910
🏁 Script executed:
# find the type definition for props in this function
rg -B 20 "duration: Number\(props\.duration\)" ./src/react/resolve.ts | head -50Repository: vargHQ/sdk
Length of output: 622
🏁 Script executed:
# find the function signature that contains line 650
rg -B 100 "duration: Number\(props\.duration\)" ./src/react/resolve.ts | grep -E "^export |^async |function |props:" | head -20Repository: vargHQ/sdk
Length of output: 36
🏁 Script executed:
# get the full function signature starting from the beginning
head -n 650 ./src/react/resolve.ts | tail -n 100 | grep -E "async|function|props|duration" | head -30Repository: vargHQ/sdk
Length of output: 258
🏁 Script executed:
# find function definition that contains line 650
ast-grep --pattern 'export function $_(props: $_) {
$$$
duration: Number(props.duration) || 5,
$$$
}'Repository: vargHQ/sdk
Length of output: 36
🏁 Script executed:
# get broader context to find the function signature
sed -n '500,650p' ./src/react/resolve.ts | grep -E "^export|^async|^function|^\s*\(props" | tail -5Repository: vargHQ/sdk
Length of output: 96
🏁 Script executed:
# get the function signature
sed -n '500,650p' ./src/react/resolve.ts | head -n 150Repository: vargHQ/sdk
Length of output: 4827
🏁 Script executed:
# check the generateVideoRaw function to understand duration expectations
rg "function generateVideoRaw|export.*generateVideoRaw" -A 10 ./src/react/resolve.tsRepository: vargHQ/sdk
Length of output: 36
🏁 Script executed:
# find generateVideoRaw definition
rg "generateVideoRaw" . --type ts --type tsx | head -20Repository: vargHQ/sdk
Length of output: 81
🏁 Script executed:
# find generateVideoRaw definition
rg "generateVideoRaw" . -g "*.ts" | head -20Repository: vargHQ/sdk
Length of output: 468
🏁 Script executed:
# find and examine the generate-video file
find . -name "generate-video.ts" -path "*/ai-sdk/*" | head -5Repository: vargHQ/sdk
Length of output: 84
🏁 Script executed:
# get the function signature and docs for generateVideo
head -n 150 ./src/ai-sdk/generate-video.tsRepository: vargHQ/sdk
Length of output: 3198
🏁 Script executed:
# check if there are tests for duration handling
rg "duration" ./src/react/resolve.ts -B 2 -A 2 | grep -E "duration|test|describe" | head -20Repository: vargHQ/sdk
Length of output: 1270
🏁 Script executed:
# look at tests to see if duration: 0 is used anywhere
rg "duration.*0" . -g "*.ts" | grep -E "test|duration.*:.*0" | head -15Repository: vargHQ/sdk
Length of output: 1338
fix duration coercion that overwrites explicit 0 values
line 650 uses Number(props.duration) || 5, so passing 0 or "0" gets silently converted to 5. this loses the distinction between missing vs explicitly provided values.
suggested patch
+ const parsedDuration =
+ props.duration === null || props.duration === undefined
+ ? undefined
+ : Number(props.duration);
+
const { video } = await generateVideo({
@@
- duration: Number(props.duration) || 5,
+ duration: Number.isFinite(parsedDuration) ? parsedDuration : 5,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/react/resolve.ts` at line 650, The duration fallback currently uses
"Number(props.duration) || 5" which treats explicit 0 as falsy and overwrites
it; update the duration assignment in resolve.ts to first detect missing values
(null/undefined) and only then default to 5, otherwise coerce props.duration to
a Number and fallback to 5 if that coercion produces NaN/Infinity; reference the
duration property and props.duration so you change the expression that computes
duration (replace the current ||-based expression with a null/undefined check
plus a Number.isFinite check to preserve explicit 0).
1 participant
Summary
Number()coercion fordurationprops in video, music, and resolve renderersduration="5"(string) instead ofduration={5}(number), the SDK now coerces it before sending to the gatewayRoot Cause
The
durationprop flows from JSX evaluation through renderers to the gateway client without any type coercion. The gateway'sVideoRequestandMusicRequestschemas usez.number().optional()(no coercion), so string durations cause"expected number, received string"validation errors.Files Changed
src/react/renderers/video.ts:153—props.duration ?? 5->Number(props.duration) || 5src/react/resolve.ts:650—(props.duration as number) ?? 5->Number(props.duration) || 5src/react/renderers/music.ts:30,40—props.duration->props.duration != null ? Number(props.duration) : undefinedBehavior
Number(5)->5(no change for correct inputs)Number("5")->5(fixes string inputs)Number(undefined)->NaN,NaN || 5->5(correct fallback)Number("auto")->NaN,NaN || 5->5(safe fallback for "auto")Related
fix/duration-string-to-numberbranch fixes the AI prompt to not generate string durations