Conversation
- Add S3-compatible storage provider (AWS S3, R2, MinIO, etc.) - Add file upload API endpoint (/api/upload) for images, videos, audio - Add Video layer with timeline sync, trimming, volume, and playback rate - Add Audio layer with waveform visualization, captions, and recording support - Add AI caption generation endpoint (/api/captions) via OpenRouter - Add enterTime/exitTime to LayerSchema for controlling layer visibility - Update canvas rendering to respect enter/exit time ranges - Update timeline UI with draggable duration bars and resize handles - Update properties panel with time range controls and split-at-playhead - Update project store with split, trim, and time range operations - Update video renderer to extract and mix audio tracks from layers via FFmpeg - Update image layer to support uploaded files alongside URL sources - Update AI mutations and schemas for new layer types and enter/exit time https://claude.ai/code/session_01HoPrQgyF2qCoqL2NPKBnfs
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds per-layer timing/trim fields and media-aware layers; S3-compatible storage with upload/presign/delete; client media capture/upload components; audio/video/image layers with timeline-synced playback and captions; thumbnail generator + queue; and server-side audio-track extraction/mixing in the renderer/export path. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Export as /api/export
participant ProjectStore as Project Store
participant Renderer as Video Renderer
participant FFmpeg as FFmpeg
Client->>Export: Request export
Export->>ProjectStore: Load project + projectData
ProjectStore-->>Export: projectData
Export->>Renderer: renderProjectToVideoStream(..., projectData)
Renderer->>Renderer: extractAudioTracks(projectData)
loop per audio track
Renderer->>FFmpeg: add audio input + trim/delay/volume filters
end
Renderer->>FFmpeg: amix audio + map video
FFmpeg-->>Renderer: output stream
Renderer-->>Export: stream response to Client
sequenceDiagram
participant Editor as Editor UI
participant Props as Properties Panel
participant Store as Project Store
participant Canvas as Canvas Renderer
participant Layer as Layer Component
Editor->>Props: Select layer
Props->>Store: get layer(id)
Store-->>Props: layer data (enterTime/exitTime)
Props->>Store: setLayerEnterTime(id, t)
Store-->>Canvas: notify update
Canvas->>Layer: render only if currentTime ∈ [enterTime, exitTime]
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/ai/schemas.ts (1)
206-228:⚠️ Potential issue | 🔴 Critical
EditLayerInputSchema.updatesis missingenterTime/exitTime, so Zod will strip them.
mutateEditLayerinsrc/lib/ai/mutations.ts(lines 291–296) readsinput.updates.enterTimeandinput.updates.exitTime, but those fields aren't declared in this schema. When the AI SDK validates tool input againstEditLayerInputSchema, Zod will silently drop unrecognized keys from theupdatesobject, making the timing edits unreachable.Proposed fix
export const EditLayerInputSchema = z.object({ layerId: z.string().describe('Layer ID or reference'), updates: z.object({ name: z.string().optional(), visible: z.boolean().optional(), locked: z.boolean().optional(), position: z .object({ x: z.number().optional(), y: z.number().optional() }) .optional(), scale: z .object({ x: z.number().optional(), y: z.number().optional() }) .optional(), rotation: z.number().optional().describe('Rotation in degrees'), opacity: z.number().min(0).max(1).optional(), - props: z.record(z.string(), z.unknown()).optional() + props: z.record(z.string(), z.unknown()).optional(), + enterTime: z.number().min(0).optional().describe('When the layer enters the timeline (seconds)'), + exitTime: z.number().min(0).optional().describe('When the layer exits the timeline (seconds)') }) });
🤖 Fix all issues with AI agents
In @.env.example:
- Around line 12-18: The .env example uses inline comments after the equals sign
which will be treated as part of the value by dotenv; remove the inline comments
and place explanatory comments on the line(s) above the variable declarations so
the variables (e.g., PRIVATE_S3_ENDPOINT and PRIVATE_S3_PUBLIC_URL) remain empty
strings when intended—ensure the variable lines are like PRIVATE_S3_ENDPOINT=
and PRIVATE_S3_PUBLIC_URL= with the explanatory text on the previous line.
In `@src/lib/components/editor/panels/properties-panel.svelte`:
- Around line 525-531: The ScrubInput for "media-end" currently shows 0 when
selectedLayer.props.mediaEndTime is unset; change the value prop to fall back to
the actual media duration or the layer's exit time before 0 so new media layers
show full length by default — e.g., compute value as
selectedLayer.props.mediaEndTime ?? selectedLayer.props.media?.duration ??
selectedLayer.props.exitTime ?? 0 and pass that to ScrubInput (retain use of
updateLayerProps('mediaEndTime', v) for onchange).
In `@src/lib/components/editor/timeline/timeline-layer.svelte`:
- Around line 64-130: The window mouse listeners added in
startDragEnter/startDragExit/startDragBar (mousemove -> handleDragMove and
mouseup -> handleDragEnd) can leak if the component unmounts mid-drag; add a
Svelte onDestroy handler that removes those same listeners and clears dragging
state (or calls handleDragEnd) to ensure cleanup: import onDestroy, and in the
onDestroy callback call window.removeEventListener('mousemove', handleDragMove)
and window.removeEventListener('mouseup', handleDragEnd) and reset
isDraggingEnter/isDraggingExit/isDraggingBar (or invoke handleDragEnd) so no
stale listeners remain after unmount.
In `@src/lib/layers/components/AudioLayer.svelte`:
- Around line 165-173: The {`#each` Array(barCount) as _, i} block in
AudioLayer.svelte is missing a key; update the each block to include a stable
key (use the index i or another stable identifier) so Svelte can properly
reconcile DOM when barCount/width changes — e.g. change the each signature to
include the key (i) for the block that generates barHeight and uses
waveformColor.
- Around line 82-84: The current layer lookup uses
projectStore.project.layers.find with the predicate (l.type === 'audio' &&
l.props.src === src), which is fragile when multiple audio layers share the same
src; update the component to accept the parent layer's unique id (pass id from
LayerWrapper via customProps or a dedicated prop) and change the lookup to find
by id (e.g., l.id === id) in AudioLayer (and mirror the same change in
VideoLayer) so each component resolves its exact layer and enterTime;
additionally add a stable key to the bars loop by changing the each block to
{`#each` Array(barCount) as _, i (i)} to avoid list-reconciliation issues.
In `@src/lib/layers/components/ImageLayer.svelte`:
- Line 37: The destructured prop fileKey in ImageLayer.svelte is unused; update
the destructuring line (where Props are unpacked: src, width, height, objectFit,
fileKey, fileName) to prefix the unused prop with an underscore (e.g., _fileKey)
so it satisfies the ESLint no-unused-vars rule and coding guidelines while
leaving the rest of the props (src, width, height, objectFit, fileName)
unchanged.
In `@src/lib/layers/components/VideoLayer.svelte`:
- Around line 58-87: The current $effect in VideoLayer.svelte uses a fragile
reverse lookup via projectStore.project.layers.find(l => l.type === 'video' &&
l.props.src === src) to get enterTime, which fails for duplicate src; change the
component to accept its own identity/timing (e.g., export let layerId or export
let enterTime) and use that value directly in the $effect instead of searching
by src; if you choose layerId, lookup the layer by id
(projectStore.project.layers.find(l => l.id === layerId)) to read enterTime,
otherwise use the passed enterTime prop, and remove the src-based lookup in the
$effect to ensure correct time sync for each VideoLayer instance.
In `@src/lib/schemas/animation.ts`:
- Around line 162-172: The schema's enterTime uses .default(0).optional() which
makes the default dead; change enterTime to match ai/schemas.ts by removing
.default(0) so it becomes z.number().min(0).optional(), and update the
JSDoc/comment to not claim "Defaults to 0" (note callers like layer-factory,
video-renderer and project.svelte handle the fallback with `?? 0`); if instead
you want the schema to apply the default at validation time, flip the chain to
.optional().default(0) on enterTime.
In `@src/lib/server/storage/index.ts`:
- Around line 218-233: fileExists currently swallows every error and returns
false; change its error handling so only S3 "Not Found" responses map to false.
In the catch of fileExists (which calls getClient() and HeadObjectCommand),
inspect the caught error (e.g., check error.name === 'NotFound' or
error.$metadata?.httpStatusCode === 404) and return false only for that case;
for any other error (network/auth/throttling), rethrow the error so callers can
handle transient or permission failures.
In `@src/lib/server/video-renderer.ts`:
- Around line 166-199: The per-track audio filter building (audioTracks.forEach
-> filterParts/audioInputs) currently only sets atrim start and never applies an
end, so tracks can play past their intended mediaEndTime/exitTime; compute an
effective end time for each track (use track.mediaEndTime and cap with
track.exitTime if present), then include that end in the atrim (e.g.,
atrim=start=...,end=... or convert to duration) before asetpts so the filter for
each track enforces both start and end bounds; update the logic that builds
filter (references: audioTracks, filterParts, audioInputs, atrim, enterTime,
mediaEndTime, exitTime) and ensure the rest of the pipeline (amix mapping)
continues to use the trimmed outputs ([a{i}]).
In `@src/lib/stores/project.svelte.ts`:
- Around line 384-416: The three setters (setLayerEnterTime, setLayerExitTime,
setLayerTimeRange) currently only clamp values to [0, duration] but do not
enforce enterTime < exitTime; update each to read the layer's current opposite
bound and ensure the invariant by clamping against it. For
setLayerEnterTime(layerId, enterTime) clamp enter to [0, duration] then set
enter = Math.min(clampedEnter, layer.exitTime - SMALL_EPS) so it stays strictly
before the existing exit; for setLayerExitTime(layerId, exitTime) clamp exit to
[0, duration] then set exit = Math.max(clampedExit, layer.enterTime +
SMALL_EPS); for setLayerTimeRange(layerId, enterTime, exitTime) clamp both to
[0, duration] and if enter >= exit resolve the conflict deterministically (e.g.,
push exit to enter + SMALL_EPS or pull enter to exit - SMALL_EPS, bounded by 0
and duration) so the final layer has enterTime < exitTime. Use a tiny constant
(SMALL_EPS, e.g., 1e-3) and reference the methods setLayerEnterTime,
setLayerExitTime, and setLayerTimeRange to locate the changes.
- Around line 444-454: The split creates non-unique IDs and duplicates
keyframes: replace the deterministic id assignment (id: layer.id + '_split')
with a call to nanoid() to generate a unique id for secondHalf and import nanoid
where needed, and change the keyframe filter logic so the halves do not both
include keyframes at the split time (e.g., keep first half using k.time <= time
and change secondHalf's filter from k.time >= time to k.time > time) — update
the secondHalf object (id and keyframes filters) accordingly, ensuring you
reference secondHalf and the existing layer.keyframes filtering logic.
In `@src/routes/api/captions/`+server.ts:
- Around line 22-29: The POST request handler currently trusts and casts request
JSON and doesn't enforce authentication; add a Zod schema (define schema first,
then infer types) for { audioUrl: string, language?: string, style?:
'subtitle'|'caption'|'lyrics' } and validate the incoming body with schema.parse
or safeParse inside the POST function before destructuring, and enforce auth by
checking locals.user?.id (return 401/unauthorized when missing); if locals is
unused elsewhere, rename the parameter to _locals or use it for the auth check
so ESLint stops flagging it.
- Around line 50-69: The prompt currently interpolates untrusted audioUrl
directly into the template in the generateText call (the
openrouter('moonshotai/kimi-k2.5') prompt), which allows prompt injection;
instead move all static instructions into a system-level prompt and pass the
audioUrl as a separate user message (or strictly sanitize/URL-encode audioUrl
before interpolation) so the model cannot treat the URL as instructions—update
the call that builds the prompt (the generateText invocation) to use a
messages-style API (system message = the caption generation rules, user message
= the audio URL and language/style parameters) or at minimum validate/escape
audioUrl to remove any textual payload before including it.
In `@src/routes/api/upload/`+server.ts:
- Line 18: MAX_REQUEST_SIZE is declared but unused; either enforce it or
remove/rename it. Fix option A: in the request handler (e.g., the
POST/handleUpload function in +server.ts) read
request.headers.get('content-length'), parse it to an integer, and if it exceeds
MAX_REQUEST_SIZE return a 413 Payload Too Large response before processing the
body. Fix option B: if you intend to reserve it for later, rename it to
_MAX_REQUEST_SIZE to avoid lint warnings. Ensure any enforcement uses the exact
constant name MAX_REQUEST_SIZE so the check is meaningful.
- Around line 43-46: The code unsafely casts mediaTypeHint to MediaType which
lets invalid values like "foo" bypass detection and later cause
ALLOWED_MIME_TYPES[mediaType] to be undefined; update the logic around
mediaTypeHint in the upload handler so you first validate that mediaTypeHint is
one of the allowed keys (e.g., exists in ALLOWED_MIME_TYPES or matches the
MediaType union) before using it; if the hint is invalid or absent, fall back to
detectMediaType(file.type), then pass the validated mediaType into
validateMediaType/ALLOWED_MIME_TYPES checks and return the 400 error only when
neither a valid hint nor a detected type is available.
- Around line 20-24: The POST request handler currently lets any caller upload
files; add an authentication guard that checks locals.user?.id before proceeding
(same pattern used in the export handler). Inside the exported POST function
(symbol: POST) after destructuring locals, if locals.user?.id is missing throw
an error(401, 'Unauthorized') or error(403, 'Forbidden') to reject
unauthenticated requests; keep the existing isStorageConfigured() check and only
continue to the upload logic when both storage is configured and locals.user?.id
exists.
🧹 Nitpick comments (4)
src/lib/server/video-renderer.ts (1)
48-52: Duplicate ternary branches — both sides are identical.Lines 49-52: The video and audio branches of the ternary produce the same expression. This is dead logic.
Simplify
- const muted = - layer.type === 'video' - ? (layer.props.muted as boolean) ?? false - : (layer.props.muted as boolean) ?? false; + const muted = (layer.props.muted as boolean) ?? false;src/lib/ai/mutations.ts (1)
290-296: Consider cross-validatingenterTime < exitTime.Both
mutateCreateLayerandmutateEditLayerset the two times independently. If the AI (or a future caller) setsenterTime >= exitTime, the layer silently disappears from the canvas (theisInTimeRangecheck incanvas.sveltewill always befalse). A simple guard would prevent confusing invisible-layer states.Suggested addition after the existing clamps
if (input.updates.enterTime !== undefined) { layer.enterTime = Math.max(0, input.updates.enterTime); } if (input.updates.exitTime !== undefined) { layer.exitTime = Math.max(0, input.updates.exitTime); } + // Ensure enterTime < exitTime + if (layer.enterTime !== undefined && layer.exitTime !== undefined && layer.enterTime >= layer.exitTime) { + layer.exitTime = layer.enterTime + 0.1; + }src/lib/server/storage/index.ts (2)
34-43: Silent fallback to default bucket/region when env vars are empty may mask misconfiguration.
getConfig()falls back to'devmotion-uploads'/'us-east-1'when environment variables are unset. Combined withaccessKeyId: '', this means an unconfigured deployment will get past initialization and only fail at the first S3 call with an opaque SDK error. Consider failing fast ingetClient()oruploadFile()by checkingisStorageConfigured()and throwing a clear error.Example: guard in getClient()
function getClient(): S3Client { if (_client) return _client; const config = getConfig(); + if (!config.accessKeyId || !config.secretAccessKey) { + throw new Error('S3 storage is not configured. Set PRIVATE_S3_ACCESS_KEY_ID and PRIVATE_S3_SECRET_ACCESS_KEY.'); + } _client = new S3Client({
164-172: Public URL construction with endpoint uses path-style — verify compatibility with virtual-hosted providers.Line 169 constructs the URL as
${endpoint}/${bucket}/${key}(path-style), which is correct whenforcePathStyleis set (MinIO, some R2 configs). However, if someone configures an endpoint for a virtual-hosted-style provider, this URL won't resolve. ThepublicUrlconfig option (line 166–167) serves as the escape hatch, so this is acceptable — just document thatpublicUrlshould be set for virtual-hosted providers.
| const layer = projectStore.project.layers.find( | ||
| (l) => l.type === 'audio' && l.props.src === src | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see how AudioLayer is imported and used in the codebase
rg -n -C3 'AudioLayer|VideoLayer' --type=svelte -g '!**/layers/components/**' | head -100Repository: epavanello/devmotion
Length of output: 94
🏁 Script executed:
# Check the current AudioLayer.svelte implementation
cat -n src/lib/layers/components/AudioLayer.svelte | head -150Repository: epavanello/devmotion
Length of output: 6450
🏁 Script executed:
# Look for registerLayer pattern in the codebase
rg -n 'registerLayer' --type=svelte --type=tsRepository: epavanello/devmotion
Length of output: 94
🏁 Script executed:
# Read the rest of AudioLayer.svelte to see the {`#each`} issue
cat -n src/lib/layers/components/AudioLayer.svelte | tail -60Repository: epavanello/devmotion
Length of output: 2259
🏁 Script executed:
# Search for AudioLayer instantiation without relying on file type
rg -n 'AudioLayer' --type-add 'svelte:include,*.svelte'Repository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
# Search for registerLayer pattern
rg -n 'registerLayer'Repository: epavanello/devmotion
Length of output: 210
🏁 Script executed:
# Check the layer registry file to understand how layers are registered
find . -name '*registry*' -type f | head -20Repository: epavanello/devmotion
Length of output: 1104
🏁 Script executed:
# Check the registry.ts file to understand how layers are registered
cat -n src/lib/layers/registry.tsRepository: epavanello/devmotion
Length of output: 2614
🏁 Script executed:
# Check CLAUDE.md for the registerLayer pattern explanation
cat -n CLAUDE.md | grep -A10 -B10 "registerLayer"Repository: epavanello/devmotion
Length of output: 1572
🏁 Script executed:
# Search for where layer components are instantiated (check renderer, canvas, etc)
rg -n "AudioLayer|VideoLayer|TextLayer" --type-add 'svelte:include,*.svelte' -A 3 -B 3Repository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
# Check if there's a central place where layer components are imported
find src -name "*.ts" -o -name "*.svelte" | xargs grep -l "AudioLayer\|VideoLayer" 2>/dev/nullRepository: epavanello/devmotion
Length of output: 48
🏁 Script executed:
# Check how layers are rendered/instantiated in the canvas
find src -name "*.svelte" -o -name "*.ts" | xargs grep -l "getLayerComponent\|layerRegistry" 2>/dev/null | head -10Repository: epavanello/devmotion
Length of output: 248
🏁 Script executed:
# Look for the canvas or renderer that instantiates layer components
find src -name "*canvas*" -o -name "*renderer*" | head -20Repository: epavanello/devmotion
Length of output: 232
🏁 Script executed:
# Check the Layer type definition to see what properties are available
rg -n "type Layer|interface Layer" --type-add 'ts:include,*.ts'Repository: epavanello/devmotion
Length of output: 893
🏁 Script executed:
# Verify the {`#each`} key pattern - check Svelte 5 requirements
rg -n "{`#each`.*as.*\(" --type-add 'svelte:include,*.svelte' -A 2 | head -50Repository: epavanello/devmotion
Length of output: 153
🏁 Script executed:
# Check Layer schema definition
cat -n src/lib/schemas/animation.ts | grep -A 30 "LayerSchema ="Repository: epavanello/devmotion
Length of output: 1269
🏁 Script executed:
# Look at canvas.svelte to see how layer components are instantiated
cat -n src/lib/components/editor/canvas/canvas.svelteRepository: epavanello/devmotion
Length of output: 14248
🏁 Script executed:
# Check for {`#each`} patterns in the codebase
rg -n "#each" --type-add 'svelte:include,*.svelte' | grep -v node_modules | head -30Repository: epavanello/devmotion
Length of output: 3234
🏁 Script executed:
# Check LayerWrapper to see how it passes data to the layer component
cat -n src/lib/layers/LayerWrapper.svelteRepository: epavanello/devmotion
Length of output: 8174
🏁 Script executed:
# Verify if the layer component receives the layer ID somehow
rg -n "LayerWrapper" -A 20 src/lib/components/editor/canvas/canvas.svelte | head -40Repository: epavanello/devmotion
Length of output: 1572
🏁 Script executed:
# Check what getLayerProps does with customProps
cat -n src/lib/engine/layer-rendering.ts | grep -A 30 "getLayerProps"Repository: epavanello/devmotion
Length of output: 379
🏁 Script executed:
# Verify the exact structure of what's passed to AudioLayer
rg -n "getLayerProps" -A 5 src/lib/components/editor/canvas/canvas.svelteRepository: epavanello/devmotion
Length of output: 680
🏁 Script executed:
# Check if there are any tests or actual usage patterns with multiple audio layers
rg -n "type.*audio" src/lib/schemas/animation.ts -A 5 -B 5Repository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
# Check if the layer ID could be passed through customProps or as a separate prop to the component
cat -n src/lib/layers/base.ts | head -100Repository: epavanello/devmotion
Length of output: 3619
🏁 Script executed:
# Verify if there are other layer components that have the same fragile lookup pattern
rg -n "projectStore.project.layers.find" src/lib/layers/components/Repository: epavanello/devmotion
Length of output: 361
Fix the fragile layer lookup—multiple audio layers with the same src will match the wrong instance.
Lines 82-84 and 135-137 find the parent layer via l.type === 'audio' && l.props.src === src. If two audio layers reference the same file, both component instances will resolve to the first match, producing incorrect enterTime offsets (and the same pattern exists in VideoLayer).
The layer ID is available in the parent LayerWrapper but is not currently passed to the layer component. Pass id through customProps or as a separate prop and look up by ID instead:
Suggested fix
let {
src,
label,
+ id,
...
}: Props = $props();
- const layer = projectStore.project.layers.find(
- (l) => l.type === 'audio' && l.props.src === src
- );
+ const layer = $derived(projectStore.project.layers.find((l) => l.id === id));The same fix applies to line 135 and to VideoLayer.
Also add a key to the {#each Array(barCount)} block on line 165 ({#each Array(barCount) as _, i (i)}).
🤖 Prompt for AI Agents
In `@src/lib/layers/components/AudioLayer.svelte` around lines 82 - 84, The
current layer lookup uses projectStore.project.layers.find with the predicate
(l.type === 'audio' && l.props.src === src), which is fragile when multiple
audio layers share the same src; update the component to accept the parent
layer's unique id (pass id from LayerWrapper via customProps or a dedicated
prop) and change the lookup to find by id (e.g., l.id === id) in AudioLayer (and
mirror the same change in VideoLayer) so each component resolves its exact layer
and enterTime; additionally add a stable key to the bars loop by changing the
each block to {`#each` Array(barCount) as _, i (i)} to avoid list-reconciliation
issues.
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In `@src/lib/components/editor/AudioRecorder.svelte`:
- Around line 67-71: When getUserMedia succeeds but new MediaRecorder(...)
throws, the acquired media stream is not stopped; update the catch block around
the MediaRecorder construction so after calling handleMediaError(err) and
setting recordingError you also stop and release the acquired stream from
getUserMedia (iterate stream.getTracks().forEach(t => t.stop()) and remove any
references) to prevent leaks—mirror the cleanup used in VideoRecorder's error
handling; ensure you reference the same stream variable returned by getUserMedia
and clear it after stopping.
- Around line 35-37: AudioRecorder.svelte currently hardcodes
'audio/webm;codecs=opus' when constructing MediaRecorder which will throw on
Safari; modify the recorder creation to mirror VideoRecorder's pattern: call the
existing getSupportedMimeType() helper (or implement it if missing) to obtain a
safe mimeType, pass { mimeType } to new MediaRecorder only when mimeType is
non-empty, otherwise construct MediaRecorder with just the mediaStream; also
update the blob creation (where chunks are combined) to use the same mimeType
value (new Blob(chunks, { type: mimeType })) so the resulting Blob uses the
supported type.
In `@src/lib/components/editor/CameraCapture.svelte`:
- Around line 117-121: The retakePhoto function currently sets state = 'idle'
(which tears down the video element) before calling startPreview(), causing
startPreview to bind srcObject to a missing element; fix by changing the
sequence so startPreview() is invoked before changing state (or remove the
explicit state = 'idle' and let the reactive $: effect handle toggling the video
element), updating the retakePhoto function (reference: retakePhoto,
startPreview, state, capturedImageUrl) so the preview is started while the video
element still exists.
- Around line 29-55: The preview assignment is racy: startPreview() assigns
videoEl.srcObject immediately but the <video> is only rendered after
state='preview', so add a reactive effect that watches videoEl and mediaStream
and sets videoEl.srcObject when both are present; in startPreview() (and
retakePhoto flow) stop relying on direct srcObject assignment — only set
mediaStream and state='preview' so the reactive effect will bind the stream once
the element is in DOM; reference the startPreview and retakePhoto functions and
the videoEl and mediaStream variables when applying this change.
In `@src/lib/server/video-renderer.ts`:
- Around line 193-200: The amix filter is using duration=longest which can make
audio outlast the video; update the audio filter pipeline in the block that
manipulates audioInputs/filterParts/outputOptions so the mixed stream is trimmed
to the video's duration by piping the amix output into an atrim using the
existing actualDuration variable (e.g., replace or augment the amix filter
string to feed into atrim=duration=${actualDuration} and map that trimmed stream
as [aout] before pushing '-map', '[aout]' and the audio codec/bitrate options).
Ensure the filter_complex string concatenation uses this new amix+atrim sequence
so the final audio length matches the video.
In `@src/lib/stores/project.svelte.ts`:
- Around line 399-426: The current setLayerEnterTime and setLayerExitTime
compute clampedEnterTime/clampedExitTime first then apply cross-validation which
can push values outside [0, project.duration]; instead perform the cross-field
validation first then clamp to the duration bounds: in setLayerEnterTime compute
desiredEnter = Math.min(enterTime, (exitTime ?? this.project.duration) - 0.1)
then clamp validEnterTime = Math.max(0, Math.min(desiredEnter,
this.project.duration)); similarly in setLayerExitTime compute desiredExit =
Math.max(exitTime, (enterTime ?? 0) + 0.1) then clamp validExitTime =
Math.max(0, Math.min(desiredExit, this.project.duration)); update the returned
layer with validEnterTime/validExitTime.
In `@src/routes/api/captions/`+server.ts:
- Around line 58-62: The server is fetching a user-supplied audioUrl directly
(see audioUrl and fetch(audioUrl) in the request handler) which exposes an SSRF
risk; add URL validation before the fetch by parsing audioUrl (new
URL(audioUrl)), then enforce either an allowlist of trusted hostnames/domains
(your storage domain) or reject hosts that resolve to
private/loopback/link-local IP ranges and metadata addresses (e.g.,
169.254.169.254) — you can implement this as a helper like
validateAudioUrl(audioUrl) invoked from the handler and return a 400/error if it
fails so fetch(audioUrl) is only called for validated targets.
In `@src/routes/api/upload/`[key]/+server.ts:
- Around line 15-56: The GET handler currently returns presigned URLs without
auth and logs sensitive data; protect it by checking locals.user (like the
upload endpoint does) before calling getSignedFileUrl(decodedKey, 3600) and
return a 401/403 when unauthenticated, or alternatively document the
public-media design and add rate-limiting to prevent enumeration; also remove
the console.log calls that print decodedKey and signedUrl (they leak presigned
credentials) and ensure only non-sensitive audit logging remains, keeping the
existing isStorageConfigured() and redirect(signedUrl) flow intact.
- Around line 61-84: The DELETE handler currently allows unauthenticated
deletes; update the DELETE RequestHandler signature to accept locals (async ({
params, locals })) and require locals.user?.id before proceeding — if missing
call error(401, 'Unauthorized') (or error(403, ...) per your policy); then use
the authenticated user id to authorize the delete (e.g., verify ownership before
calling deleteFile if your model ties files to users). Keep the existing storage
check (isStorageConfigured()) and use deleteFile(decodeURIComponent(key)) only
after authentication/authorization succeeds.
🟡 Minor comments (12)
package.json-80-80 (1)
80-80:⚠️ Potential issue | 🟡 MinorRemove unused
@ai-sdk/openaidependency.The
openaipackage (^6.18.0) is correctly used for direct Whisper API calls insrc/routes/api/captions/+server.ts. However,@ai-sdk/openai(^3.0.18) is never imported or used anywhere in the codebase. The project uses@ai-sdk/svelteand@openrouter/ai-sdk-providerfor AI SDK integration, but not@ai-sdk/openai. This should be removed from dependencies to reduce bloat.src/routes/api/upload/+server.ts-76-79 (1)
76-79:⚠️ Potential issue | 🟡 MinorFile size error returns 400 instead of 413.
Line 34 correctly uses
error(413, 'Payload Too Large')for the content-length check, but line 78 useserror(400, ...)for the same logical condition. Use 413 for consistency.Proposed fix
if (!validateFileSize(file.size, mediaType)) { - error(400, `File too large for ${mediaType} upload`); + error(413, `File too large for ${mediaType} upload`); }src/lib/stores/project.svelte.ts-474-482 (1)
474-482:⚠️ Potential issue | 🟡 MinorSplit produces keyframes with duplicate IDs across the two halves.
map((k) => ({ ...k }))on line 481 shallow-copies keyframes but preserves their originalidvalues. After splitting, keyframes at times ≤splitTimein the first half and those atsplitTimein the second half could share IDs with keyframes in other layers (the original's keyframes are not re-IDed either). SinceselectedKeyframeIdsis a global set, selecting a keyframe by ID could match in both layers.Proposed fix — assign new IDs to the second half's keyframes
keyframes: layer.keyframes .filter((k) => k.time > time) - .map((k) => ({ ...k })) + .map((k) => ({ ...k, id: nanoid() }))src/lib/server/thumbnail-generator.ts-36-40 (1)
36-40:⚠️ Potential issue | 🟡 MinorGuard against zero/negative
heightand zeroduration.
- Line 36:
projectData.width / projectData.heightwill produceInfinity(orNaNif both are 0) whenheightis 0, causing invalid thumbnail dimensions.- Lines 39-40: A zero-duration project yields
totalFrames = 0, which means no frames are written to FFmpeg. This will likely cause FFmpeg to error out or produce an empty/corrupt GIF.Consider validating inputs early or returning gracefully for degenerate projects.
🛡️ Proposed guard
+ if (projectData.width <= 0 || projectData.height <= 0 || projectData.duration <= 0) { + throw new Error(`Invalid project dimensions or duration for thumbnail generation`); + } + const aspectRatio = projectData.width / projectData.height;src/lib/components/editor/VideoRecorder.svelte-193-197 (1)
193-197:⚠️ Potential issue | 🟡 MinorCountdown can briefly show a negative value.
If the interval fires slightly after
MAX_DURATIONis reached but beforestopRecordingexecutes,MAX_DURATION - recordingDurationcould be negative. Clamp to 0:- {MAX_DURATION - recordingDuration}s remaining + {Math.max(0, MAX_DURATION - recordingDuration)}s remainingsrc/lib/components/editor/VideoRecorder.svelte-163-163 (1)
163-163:⚠️ Potential issue | 🟡 MinorUnused
svelte-ignorecomment flagged by ESLint.The
a11y_media_has_captionwarning doesn't trigger here (live preview video withmuted), so the ignore directive is unnecessary.- <!-- svelte-ignore a11y_media_has_caption -->src/routes/api/captions/+server.ts-51-51 (1)
51-51:⚠️ Potential issue | 🟡 Minor
styleis validated by the schema but never used.
styleis included inCaptionRequestSchemabut not destructured fromresult.dataon line 51, nor passed to the Whisper API. Either remove it from the schema or use it (e.g., include in the response metadata).Proposed fix
- const { audioUrl, language } = result.data; + const { audioUrl, language, style } = result.data;Or if not needed:
const CaptionRequestSchema = z.object({ audioUrl: z.string().url(), - language: z.string().optional(), - style: z.enum(['subtitle', 'caption', 'lyrics']).optional() + language: z.string().optional() });src/lib/components/editor/panels/properties-panel.svelte-585-594 (1)
585-594:⚠️ Potential issue | 🟡 MinorThe fallback chain references a non-existent property.
selectedLayer.props.media?.durationdoes not exist in either AudioLayer or VideoLayer schemas. Both definemediaStartTimeandmediaEndTimeas flat properties, not nested under amediaobject. This intermediate fallback will always beundefinedand silently skip toselectedLayer.exitTime.The media duration is available from the DOM element (via
audioEl.durationorvideoEl.durationin the respective layer components), not from props. Consider removing this dead code path or replacing it with the intended logic.Current code
value={(selectedLayer.props.mediaEndTime as number) ?? (selectedLayer.props.media?.duration as number) ?? selectedLayer.exitTime ?? projectStore.project.duration}src/routes/api/captions/+server.ts-86-89 (1)
86-89:⚠️ Potential issue | 🟡 MinorUse OpenAI SDK's proper
TranscriptionVerbosetype instead of unsafe cast.The code unnecessarily bypasses type safety with
as unknown as { segments: ... }. Sinceresponse_format: 'verbose_json'is specified, use the SDK's built-in type:transcription as OpenAI.Audio.TranscriptionVerbose. This gives you proper typing for thesegmentsfield without the unsafe cast and prevents runtime surprises if response shape changes.- const segments = ( - transcription as unknown as { segments: Array<{ start: number; end: number; text: string }> } - ).segments; + const segments = (transcription as OpenAI.Audio.TranscriptionVerbose).segments ?? [];src/lib/server/video-renderer.ts-146-151 (1)
146-151:⚠️ Potential issue | 🟡 MinorAudio inputs added before
actualDurationis known.The FFmpeg command inputs (including audio tracks) are added at Lines 146-151 using
config.duration, but the actual duration is resolved later from the page at Line 140 (actualDuration). The filter complex at Lines 164-201 is also built beforeactualDurationis available if you need it to cap audio length. Consider moving the audio track setup and filter construction after the page evaluation (Line 138-141).src/lib/components/editor/media-upload-utils.ts-68-72 (1)
68-72:⚠️ Potential issue | 🟡 Minor
formatDurationproduces fractional seconds for non-integer input.
seconds % 60preserves the fractional part, soformatDuration(65.7)returns"1:05.700000000000003"instead of"1:05". Floor the seconds remainder.Proposed fix
export function formatDuration(seconds: number): string { const mins = Math.floor(seconds / 60); - const secs = seconds % 60; + const secs = Math.floor(seconds % 60); return `${mins}:${secs.toString().padStart(2, '0')}`; }src/lib/components/editor/FileUpload.svelte-77-83 (1)
77-83:⚠️ Potential issue | 🟡 MinorSilent failure when
data.successis falsy.If the server returns
200 OKbut with{ success: false, ... }(e.g., a validation error), the function silently completes without callingonUploador settinguploadError. The user sees a successful upload with no feedback.Proposed fix
const data = await res.json(); if (data.success && data.file) { onUpload({ url: data.file.url, key: data.file.key, fileName: data.file.originalName }); + } else { + throw new Error(data?.message || 'Upload failed: unexpected response'); }
🧹 Nitpick comments (22)
src/lib/engine/layer-factory.ts (1)
33-40: The union type is structurally redundant.Since
CreateLayerOptionsalready containsx?andy?, the union{ x?: number; y?: number } | CreateLayerOptionsis equivalent to justCreateLayerOptionsunder structural typing. Consider simplifying the parameter type toCreateLayerOptions = {}alone — the JSDoc already explains that simple{x, y}is accepted.The
'enterTime' in positionduck-typing check (Line 39–40) is correct, including the edge case whereenterTimeis explicitlyundefined.Simplify parameter type
export function createLayer( type: LayerType, propsOverrides: Record<string, unknown> = {}, - position: { x?: number; y?: number } | CreateLayerOptions = {} + position: CreateLayerOptions = {} ): Layer {docs/current-feature.md (1)
1-1: This reads like a raw prompt, not feature documentation.This file appears to be the original AI prompt rather than structured documentation. Consider replacing it with proper feature documentation (e.g., acceptance criteria, architecture notes, or an ADR) or removing it before merge so it doesn't confuse future contributors.
src/lib/stores/project.svelte.ts (1)
151-171:removeLayeris now async — verify callers handle the returned Promise.Making this method
asyncmeans callers that don'tawaitit will fire-and-forget the file cleanup. The layer mutation itself happens after theawaiton line 167, so there's a window where the layer is still visible in the UI while the DELETE request is in-flight. Consider either:
- Moving the layer filter before the
await(optimistic removal), or- Ensuring all call sites
awaitthis method if they depend on the layer being gone.Line 158: the
as stringcast is redundant given thetypeofcheck on line 156.Suggested: optimistic removal
async removeLayer(layerId: string) { const layer = this.project.layers.find((l) => l.id === layerId); + // Remove layer from state immediately (optimistic) + this.project.layers = this.project.layers.filter((l) => l.id !== layerId); + if (this.selectedLayerId === layerId) { + this.selectedLayerId = null; + } + // Clean up uploaded files if the layer has a fileKey if (layer && layer.props.fileKey && typeof layer.props.fileKey === 'string') { try { - await fetch(`/api/upload/${encodeURIComponent(layer.props.fileKey as string)}`, { + await fetch(`/api/upload/${encodeURIComponent(layer.props.fileKey)}`, { method: 'DELETE' }); } catch (err) { console.warn('Failed to delete file from storage:', err); } } - - this.project.layers = this.project.layers.filter((l) => l.id !== layerId); - if (this.selectedLayerId === layerId) { - this.selectedLayerId = null; - } }src/lib/server/storage/index.ts (1)
200-205: Remove or reduce verbosity of presigned URL logging in production.Lines 200–205 log the bucket name, key, region, and endpoint on every signed-URL generation. In production this creates noise and leaks infrastructure details. Consider moving this to
debuglevel or removing it.Proposed fix
- console.log('Generating presigned URL for:', { - bucket: config.bucket, - key, - region: config.region, - endpoint: config.endpoint - });src/routes/api/upload/+server.ts (1)
54-65: UnnecessarydetectMediaTypecall when the hint is valid.On line 57,
detectMediaType(file.type)is called unconditionally inside theif (mediaTypeHint)branch, but the resultdetectedTypeis only used in theelsefallback (line 61). When the hint is valid (line 58), the detection result is discarded.Proposed fix
let mediaType: MediaType | null = null; if (mediaTypeHint) { - const detectedType = detectMediaType(file.type); if (mediaTypeHint === 'image' || mediaTypeHint === 'video' || mediaTypeHint === 'audio') { mediaType = mediaTypeHint as MediaType; } else { - mediaType = detectedType; + mediaType = detectMediaType(file.type); } } else { mediaType = detectMediaType(file.type); }src/lib/server/thumbnail-generator.ts (2)
45-130: Consolidate cleanup into afinallyblock to prevent resource leaks.Browser close and token invalidation are duplicated in both the success path (Lines 111-112) and error path (Lines 126-127). If
browser.close()on the success path throws, the token is never invalidated and the error propagates without proper cleanup. Afinallyblock avoids this duplication and ensures cleanup always runs.♻️ Proposed refactor using finally
try { // ... browser launch, frame capture, ffmpeg encoding ... - await browser.close(); - invalidateRenderToken(token); - const result = await uploadFile( gifBuffer, `${projectId}-thumbnail.gif`, 'image/gif', 'image', projectId ); await db.update(project).set({ thumbnailUrl: result.url }).where(eq(project.id, projectId)); return result.url; } catch (err) { - if (browser) await browser.close(); - invalidateRenderToken(token); console.error(`Failed to generate thumbnail for project ${projectId}:`, err); throw err; + } finally { + if (browser) { + await browser.close().catch((e) => + console.error('Failed to close browser:', e) + ); + } + invalidateRenderToken(token); }
64-65: Verify that Playwright'swaitForFunctionauto-awaits the returned Promise.
window.__DEVMOTION__?.readyreturns aPromise(set in the render page). This works because Playwright'swaitForFunctionauto-awaits Promise return values. However,window.__DEVMOTION__?.readycan short-circuit toundefined(beforeonMountruns), which is falsy — so it keeps polling until the Promise is set. Once set, the Promise object itself is truthy and Playwright awaits it. This is correct but subtle.A clearer approach would be to await the promise explicitly:
- await page.waitForFunction(() => window.__DEVMOTION__?.ready, { timeout: 30000 }); + await page.waitForFunction( + () => window.__DEVMOTION__?.ready?.then(() => true), + { timeout: 30000 } + );src/lib/server/thumbnail-queue.ts (2)
18-55: No deduplication — rapid saves will queue redundant thumbnail jobs.Each call to
saveProjectenqueues a new job. If a user saves frequently, multiple thumbnail generation jobs for the sameprojectIdqueue up, each spawning a Playwright browser and FFmpeg process. Consider replacing any pending job for the sameprojectIdwhen a new one arrives:♻️ Proposed deduplication in enqueue
enqueue(job: ThumbnailJob) { + // Replace any pending job for the same project + this.queue = this.queue.filter((j) => j.projectId !== job.projectId); this.queue.push(job); console.log(`[ThumbnailQueue] Enqueued job for project ${job.projectId}`); this.processQueue(); }
21-21:maxConcurrentis declared but never referenced.The processing logic uses only the
processingboolean. Either remove the unused field or implement actual concurrency control that references it.src/routes/(marketing)/gallery/+page.svelte (1)
58-73: Consider addingloading="lazy"and a fallback for broken thumbnail URLs.This is a gallery grid that can render many images at once. Adding lazy loading improves initial page load. Additionally, if a thumbnail URL becomes stale, the
<img>will render broken — anonerrorhandler falling back to the placeholder would be more resilient.♻️ Proposed enhancement
<img src={project.thumbnailUrl} alt={project.name} - class="absolute inset-0 h-full w-full object-cover" + class="absolute inset-0 h-full w-full object-cover" + loading="lazy" + onerror={(e) => { e.currentTarget.style.display = 'none'; }} />src/lib/components/editor/media-capture-types.ts (1)
6-22: Clean type definitions — consider Zod schemas if these validate external data.The types are well-structured. Since
MediaCaptureResultrepresents data returned from the upload API (external boundary), the coding guideline prefers defining a Zod schema first and inferring the type viaz.infer. If validation already happens inmedia-upload-utils.tsbefore constructing these objects, this is fine as-is.As per coding guidelines,
**/*.ts: "Define Zod schemas first, then infer TypeScript types from schemas usingz.infer<typeof SchemaName>" and "Validate external data with Zod schemas before use".src/lib/components/editor/timeline/timeline-layer.svelte (1)
94-122: Drag-enter handler reads reactiveexitTimeinstead of snapshot — potential micro-glitch.In
handleDragMove(line 99), when dragging the enter handle, the clamp uses the live reactiveexitTime. If something else updatesexitTimeconcurrently (e.g., a store sync), the constraint could shift mid-drag. Using the snapshotteddragStartExitwould be more predictable:- const newEnter = Math.max(0, Math.min(dragStartEnter + deltaTime, exitTime - 0.1)); + const newEnter = Math.max(0, Math.min(dragStartEnter + deltaTime, dragStartExit - 0.1));Similarly on line 103,
enterTimecould usedragStartEnterfor the exit handle. In practice this is unlikely to cause visible issues, so this is optional.src/lib/components/editor/VideoRecorder.svelte (2)
13-20: Props type duplicatesMediaCaptureResultfrommedia-capture-types.ts.The inline
{ url: string; key: string; fileName: string }type ononRecordingCompleteis identical to theMediaCaptureResultinterface defined inmedia-capture-types.ts. Reuse it for consistency and to keep the contract in one place.♻️ Proposed fix
+ import type { MediaCaptureResult } from './media-capture-types'; + interface Props { /** Callback when recording is complete and uploaded */ - onRecordingComplete: (result: { url: string; key: string; fileName: string }) => void; + onRecordingComplete: (result: MediaCaptureResult) => void; /** Optional project ID for organizing uploads */ projectId?: string; /** Include audio in video recording (default: true) */ audioEnabled?: boolean; }
56-67: Only webm MIME types listed — recording will fail on Safari.Safari's MediaRecorder doesn't support
video/webm. Consider addingvideo/mp4as a fallback to support Safari 14.6+.♻️ Add mp4 fallback
const mimeTypes = [ 'video/webm;codecs=vp9,opus', 'video/webm;codecs=vp8,opus', 'video/webm;codecs=vp9', 'video/webm;codecs=vp8', - 'video/webm' + 'video/webm', + 'video/mp4' ];Note: if
video/mp4is selected, the file extension on line 135 should also be adjusted accordingly rather than hardcoding'webm'.src/lib/components/editor/AudioRecorder.svelte (2)
12-17: Same type duplication asVideoRecorder— useMediaCaptureResult.The inline result type should be replaced with the shared
MediaCaptureResultfrommedia-capture-types.tsfor the same reason noted in VideoRecorder.
60-66: No maximum recording duration — could produce very large uploads.
VideoRecorderenforces a 5-minuteMAX_DURATIONwith auto-stop.AudioRecorderhas no such limit. Consider adding a similar safeguard to prevent runaway recordings and oversized uploads.src/lib/components/editor/FileUpload.svelte (1)
50-91: Upload logic duplicatesuploadMediaBlobfrommedia-upload-utils.ts.
handleFileSelectreimplements the sameFormDataconstruction,/api/uploadPOST, error parsing, and result extraction thatuploadMediaBlobalready provides. Consider reusing it to keep the upload flow consistent and avoid divergence (e.g., if the API response shape changes, you'd need to update both places).♻️ Proposed refactor using uploadMediaBlob
<script lang="ts"> import { Button } from '$lib/components/ui/button'; import { Upload, Trash2, Loader2, Check } from '@lucide/svelte'; import AudioRecorder from './AudioRecorder.svelte'; import VideoRecorder from './VideoRecorder.svelte'; import CameraCapture from './CameraCapture.svelte'; + import { uploadMediaBlob } from './media-upload-utils';async function handleFileSelect(e: Event) { const input = e.target as HTMLInputElement; const file = input.files?.[0]; if (!file) return; isUploading = true; uploadError = ''; try { - const formData = new FormData(); - formData.append('file', file); - formData.append('mediaType', mediaType); - if (projectId) { - formData.append('projectId', projectId); - } - - const res = await fetch('/api/upload', { - method: 'POST', - body: formData - }); - - if (!res.ok) { - const data = await res.json().catch(() => null); - throw new Error(data?.message || `Upload failed (${res.status})`); - } - - const data = await res.json(); - if (data.success && data.file) { - onUpload({ - url: data.file.url, - key: data.file.key, - fileName: data.file.originalName - }); - } + const result = await uploadMediaBlob(file, file.name, mediaType, projectId); + onUpload(result); } catch (err) { uploadError = err instanceof Error ? err.message : 'Upload failed'; } finally {src/lib/components/editor/CameraCapture.svelte (2)
139-147:URL.revokeObjectURLis a no-op on data URLs.
capturedImageUrlis a data URL fromcanvas.toDataURL()(Line 74), not a blob URL fromURL.createObjectURL(). TherevokeObjectURLcall is harmless but misleading. You can safely remove it or, if you want to save memory on large images, consider usingURL.createObjectURL(blob)for the review image instead.
162-162: Remove unusedsvelte-ignorecomment.Static analysis (ESLint) reports that the
a11y_media_has_captionwarning is not triggered here, making the ignore directive unnecessary. Themutedattribute on the<video>likely suppresses it.- <!-- svelte-ignore a11y_media_has_caption --> <videosrc/lib/server/video-renderer.ts (1)
44-69: Identical ternary branches and excessive type assertions inextractAudioTracks.Lines 50-52: The ternary produces the same expression for both
videoandaudio— simplify it. Lines 48-61 also contain severalas string/as number/as booleancasts, suggestinglayer.propsis loosely typed. If the schema allows narrowingpropsbylayer.type, that would eliminate the casts.♻️ Simplify muted check
- const muted = - layer.type === 'video' - ? ((layer.props.muted as boolean) ?? false) - : ((layer.props.muted as boolean) ?? false); + const muted = (layer.props.muted as boolean) ?? false;src/lib/components/editor/media-upload-utils.ts (2)
121-128:getSupportedMimeTypeisasyncbut never awaits — returns a wrapped promise unnecessarily.The function body is entirely synchronous. Marking it
asyncforces callers toawaitfor no reason and may mislead readers into thinking there's I/O involved.Proposed fix
-export async function getSupportedMimeType(types: string[]): Promise<string | null> { +export function getSupportedMimeType(types: string[]): string | null { for (const type of types) { if (typeof MediaRecorder !== 'undefined' && MediaRecorder.isTypeSupported(type)) { return type; } } return null; }
159-162: Biome:forEachcallback implicitly returns the value oftrack.stop().Per the static analysis hint, the arrow in
.forEach((track) => track.stop())returns the result ofstop(). While harmless here, it's flagged by the linter. Use a block body orfor...of.Proposed fix
export function stopMediaStream(stream: MediaStream | null): void { if (stream) { - stream.getTracks().forEach((track) => track.stop()); + for (const track of stream.getTracks()) { + track.stop(); + } } }
| setLayerEnterTime(layerId: string, enterTime: number) { | ||
| this.project.layers = this.project.layers.map((layer) => { | ||
| if (layer.id === layerId) { | ||
| const exitTime = layer.exitTime ?? this.project.duration; | ||
| const clampedEnterTime = Math.max(0, Math.min(enterTime, this.project.duration)); | ||
| // Ensure enter time is before exit time | ||
| const validEnterTime = Math.min(clampedEnterTime, exitTime - 0.1); | ||
| return { ...layer, enterTime: validEnterTime }; | ||
| } | ||
| return layer; | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Set the exit time for a layer (when it becomes hidden) | ||
| */ | ||
| setLayerExitTime(layerId: string, exitTime: number) { | ||
| this.project.layers = this.project.layers.map((layer) => { | ||
| if (layer.id === layerId) { | ||
| const enterTime = layer.enterTime ?? 0; | ||
| const clampedExitTime = Math.max(0, Math.min(exitTime, this.project.duration)); | ||
| // Ensure exit time is after enter time | ||
| const validExitTime = Math.max(clampedExitTime, enterTime + 0.1); | ||
| return { ...layer, exitTime: validExitTime }; | ||
| } | ||
| return layer; | ||
| }); | ||
| } |
There was a problem hiding this comment.
validEnterTime can go negative; validExitTime can exceed duration.
In setLayerEnterTime, after clamping to [0, duration], line 405 computes Math.min(clampedEnterTime, exitTime - 0.1). If exitTime is very small (e.g., 0.05), this produces a negative value, bypassing the earlier clamp. The symmetric problem exists in setLayerExitTime where enterTime + 0.1 can exceed duration.
Apply the bounds clamp after the cross-validation:
Proposed fix
setLayerEnterTime(layerId: string, enterTime: number) {
this.project.layers = this.project.layers.map((layer) => {
if (layer.id === layerId) {
const exitTime = layer.exitTime ?? this.project.duration;
const clampedEnterTime = Math.max(0, Math.min(enterTime, this.project.duration));
- const validEnterTime = Math.min(clampedEnterTime, exitTime - 0.1);
+ const validEnterTime = Math.max(0, Math.min(clampedEnterTime, exitTime - 0.1));
return { ...layer, enterTime: validEnterTime };
}
return layer;
});
} setLayerExitTime(layerId: string, exitTime: number) {
this.project.layers = this.project.layers.map((layer) => {
if (layer.id === layerId) {
const enterTime = layer.enterTime ?? 0;
const clampedExitTime = Math.max(0, Math.min(exitTime, this.project.duration));
- const validExitTime = Math.max(clampedExitTime, enterTime + 0.1);
+ const validExitTime = Math.min(this.project.duration, Math.max(clampedExitTime, enterTime + 0.1));
return { ...layer, exitTime: validExitTime };
}
return layer;
});
}📝 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.
| setLayerEnterTime(layerId: string, enterTime: number) { | |
| this.project.layers = this.project.layers.map((layer) => { | |
| if (layer.id === layerId) { | |
| const exitTime = layer.exitTime ?? this.project.duration; | |
| const clampedEnterTime = Math.max(0, Math.min(enterTime, this.project.duration)); | |
| // Ensure enter time is before exit time | |
| const validEnterTime = Math.min(clampedEnterTime, exitTime - 0.1); | |
| return { ...layer, enterTime: validEnterTime }; | |
| } | |
| return layer; | |
| }); | |
| } | |
| /** | |
| * Set the exit time for a layer (when it becomes hidden) | |
| */ | |
| setLayerExitTime(layerId: string, exitTime: number) { | |
| this.project.layers = this.project.layers.map((layer) => { | |
| if (layer.id === layerId) { | |
| const enterTime = layer.enterTime ?? 0; | |
| const clampedExitTime = Math.max(0, Math.min(exitTime, this.project.duration)); | |
| // Ensure exit time is after enter time | |
| const validExitTime = Math.max(clampedExitTime, enterTime + 0.1); | |
| return { ...layer, exitTime: validExitTime }; | |
| } | |
| return layer; | |
| }); | |
| } | |
| setLayerEnterTime(layerId: string, enterTime: number) { | |
| this.project.layers = this.project.layers.map((layer) => { | |
| if (layer.id === layerId) { | |
| const exitTime = layer.exitTime ?? this.project.duration; | |
| const clampedEnterTime = Math.max(0, Math.min(enterTime, this.project.duration)); | |
| // Ensure enter time is before exit time | |
| const validEnterTime = Math.max(0, Math.min(clampedEnterTime, exitTime - 0.1)); | |
| return { ...layer, enterTime: validEnterTime }; | |
| } | |
| return layer; | |
| }); | |
| } | |
| /** | |
| * Set the exit time for a layer (when it becomes hidden) | |
| */ | |
| setLayerExitTime(layerId: string, exitTime: number) { | |
| this.project.layers = this.project.layers.map((layer) => { | |
| if (layer.id === layerId) { | |
| const enterTime = layer.enterTime ?? 0; | |
| const clampedExitTime = Math.max(0, Math.min(exitTime, this.project.duration)); | |
| // Ensure exit time is after enter time | |
| const validExitTime = Math.min(this.project.duration, Math.max(clampedExitTime, enterTime + 0.1)); | |
| return { ...layer, exitTime: validExitTime }; | |
| } | |
| return layer; | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@src/lib/stores/project.svelte.ts` around lines 399 - 426, The current
setLayerEnterTime and setLayerExitTime compute clampedEnterTime/clampedExitTime
first then apply cross-validation which can push values outside [0,
project.duration]; instead perform the cross-field validation first then clamp
to the duration bounds: in setLayerEnterTime compute desiredEnter =
Math.min(enterTime, (exitTime ?? this.project.duration) - 0.1) then clamp
validEnterTime = Math.max(0, Math.min(desiredEnter, this.project.duration));
similarly in setLayerExitTime compute desiredExit = Math.max(exitTime,
(enterTime ?? 0) + 0.1) then clamp validExitTime = Math.max(0,
Math.min(desiredExit, this.project.duration)); update the returned layer with
validEnterTime/validExitTime.
| export const GET: RequestHandler = async ({ params }) => { | ||
| // Check if storage is configured | ||
| if (!isStorageConfigured()) { | ||
| error(503, 'File storage is not configured. Set S3 environment variables.'); | ||
| } | ||
|
|
||
| const { key } = params; | ||
|
|
||
| if (!key) { | ||
| error(400, 'No file key provided'); | ||
| } | ||
|
|
||
| try { | ||
| // Decode the key (it may contain URL-encoded characters) | ||
| const decodedKey = decodeURIComponent(key); | ||
|
|
||
| console.log('Attempting to get signed URL for key:', decodedKey); | ||
|
|
||
| // Generate presigned URL valid for 1 hour | ||
| const signedUrl = await getSignedFileUrl(decodedKey, 3600); | ||
|
|
||
| console.log('Generated signed URL:', signedUrl); | ||
|
|
||
| // Redirect to the presigned URL | ||
| throw redirect(302, signedUrl); | ||
| } catch (err) { | ||
| // Re-throw SvelteKit errors (redirect, error) | ||
| if (err && typeof err === 'object' && 'status' in err) { | ||
| throw err; | ||
| } | ||
|
|
||
| console.error('File access error:', err); | ||
| // Extract error message from various error types | ||
| let errorMessage = 'Unknown error'; | ||
| if (err instanceof Error) { | ||
| errorMessage = err.message; | ||
| } else if (err && typeof err === 'object') { | ||
| errorMessage = JSON.stringify(err); | ||
| } | ||
| error(500, `File access failed: ${errorMessage}`); | ||
| } | ||
| }; |
There was a problem hiding this comment.
GET handler lacks authentication — intentional for media serving?
The GET endpoint generates presigned URLs without checking locals.user. This means anyone with a key URL can access private bucket files. If this is intentional (e.g., for embedding media in public projects), consider at minimum:
- Rate limiting to prevent enumeration
- Documenting the design decision
If unintentional, add an auth guard similar to the upload endpoint.
Also, lines 31 and 36 log the key and full signed URL to stdout — remove these before production use as they leak presigned credentials.
🤖 Prompt for AI Agents
In `@src/routes/api/upload/`[key]/+server.ts around lines 15 - 56, The GET handler
currently returns presigned URLs without auth and logs sensitive data; protect
it by checking locals.user (like the upload endpoint does) before calling
getSignedFileUrl(decodedKey, 3600) and return a 401/403 when unauthenticated, or
alternatively document the public-media design and add rate-limiting to prevent
enumeration; also remove the console.log calls that print decodedKey and
signedUrl (they leak presigned credentials) and ensure only non-sensitive audit
logging remains, keeping the existing isStorageConfigured() and
redirect(signedUrl) flow intact.
| export const DELETE: RequestHandler = async ({ params }) => { | ||
| // Check if storage is configured | ||
| if (!isStorageConfigured()) { | ||
| error(503, 'File storage is not configured. Set S3 environment variables.'); | ||
| } | ||
|
|
||
| const { key } = params; | ||
|
|
||
| if (!key) { | ||
| error(400, 'No file key provided'); | ||
| } | ||
|
|
||
| try { | ||
| // Delete the file from storage | ||
| await deleteFile(decodeURIComponent(key)); | ||
|
|
||
| return json({ | ||
| success: true, | ||
| message: 'File deleted successfully' | ||
| }); | ||
| } catch (err) { | ||
| console.error('Delete error:', err); | ||
| error(500, `Delete failed: ${err instanceof Error ? err.message : 'Unknown error'}`); | ||
| } |
There was a problem hiding this comment.
Missing authentication on DELETE — any unauthenticated request can delete files.
The DELETE handler has no auth check, unlike the upload endpoint which requires locals.user?.id. An attacker who discovers or brute-forces a key can delete any uploaded media file.
Proposed fix
-export const DELETE: RequestHandler = async ({ params }) => {
+export const DELETE: RequestHandler = async ({ params, locals }) => {
+ if (!locals.user?.id) {
+ error(401, 'Unauthorized');
+ }
+
// Check if storage is configured
if (!isStorageConfigured()) {📝 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.
| export const DELETE: RequestHandler = async ({ params }) => { | |
| // Check if storage is configured | |
| if (!isStorageConfigured()) { | |
| error(503, 'File storage is not configured. Set S3 environment variables.'); | |
| } | |
| const { key } = params; | |
| if (!key) { | |
| error(400, 'No file key provided'); | |
| } | |
| try { | |
| // Delete the file from storage | |
| await deleteFile(decodeURIComponent(key)); | |
| return json({ | |
| success: true, | |
| message: 'File deleted successfully' | |
| }); | |
| } catch (err) { | |
| console.error('Delete error:', err); | |
| error(500, `Delete failed: ${err instanceof Error ? err.message : 'Unknown error'}`); | |
| } | |
| export const DELETE: RequestHandler = async ({ params, locals }) => { | |
| if (!locals.user?.id) { | |
| error(401, 'Unauthorized'); | |
| } | |
| // Check if storage is configured | |
| if (!isStorageConfigured()) { | |
| error(503, 'File storage is not configured. Set S3 environment variables.'); | |
| } | |
| const { key } = params; | |
| if (!key) { | |
| error(400, 'No file key provided'); | |
| } | |
| try { | |
| // Delete the file from storage | |
| await deleteFile(decodeURIComponent(key)); | |
| return json({ | |
| success: true, | |
| message: 'File deleted successfully' | |
| }); | |
| } catch (err) { | |
| console.error('Delete error:', err); | |
| error(500, `Delete failed: ${err instanceof Error ? err.message : 'Unknown error'}`); | |
| } |
🤖 Prompt for AI Agents
In `@src/routes/api/upload/`[key]/+server.ts around lines 61 - 84, The DELETE
handler currently allows unauthenticated deletes; update the DELETE
RequestHandler signature to accept locals (async ({ params, locals })) and
require locals.user?.id before proceeding — if missing call error(401,
'Unauthorized') (or error(403, ...) per your policy); then use the authenticated
user id to authorize the delete (e.g., verify ownership before calling
deleteFile if your model ties files to users). Keep the existing storage check
(isStorageConfigured()) and use deleteFile(decodeURIComponent(key)) only after
authentication/authorization succeeds.
|
The preview deployment for epavanello/devmotion:main failed. 🔴 Last updated at: 2026-02-06 18:12:11 CET |
…chemas with content duration and offset properties.
epavanello
changed the title
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Fix all issues with AI agents
In `@src/lib/components/editor/CameraCapture.svelte`:
- Around line 147-154: The cleanup currently calls
URL.revokeObjectURL(capturedImageUrl) inside the $effect teardown, but
capturedImageUrl is produced via canvasEl.toDataURL(...) (a data: URL), so
revokeObjectURL is a no-op and misleading; either remove the revokeObjectURL
call from the $effect cleanup, or change the capture flow to produce a revocable
blob URL by converting the canvas to a Blob and calling
URL.createObjectURL(blob) (update where capturedImageUrl is assigned) and then
keep URL.revokeObjectURL(capturedImageUrl) in the $effect teardown; adjust
references to capturedImageUrl and canvasEl.toDataURL accordingly and ensure
stopMediaStream(mediaStream) remains in the same teardown.
In `@src/lib/components/editor/FileUpload.svelte`:
- Around line 53-86: The extractMediaDuration promise can hang if neither
onloadedmetadata nor onerror fires; modify extractMediaDuration to add a timeout
(e.g., 10s) that will revoke the object URL and resolve(undefined) if neither
event fires, and ensure the timeout is cleared when onloadedmetadata or onerror
runs; apply this for both the video and audio branches, remove event listeners
and revoke URL in all resolution paths to avoid leaks, and reference the
existing mediaType check and the created elements (video/audio) so the timer and
cleanup are tied to those element handlers.
- Around line 88-136: handleFileSelect currently starts uploading immediately
without checking file.size; add client-side validation against the displayed
maxSizes (per mediaType) before setting isUploading. In handleFileSelect (before
setting isUploading = true and before calling extractMediaDuration), compute the
allowedMax = maxSizes[mediaType] (convert units to bytes if needed), compare
file.size to allowedMax, and if it exceeds set uploadError to a clear message,
clear the input.value, and return early. Ensure the same symbols are used (file,
mediaType, maxSizes, uploadError, isUploading, input) so the UI shows the error
and no network request is made.
In `@src/lib/components/editor/media-upload-utils.ts`:
- Around line 71-75: formatDuration currently uses the raw seconds parameter so
non-integer inputs produce fractional seconds (e.g., formatDuration(65.7) ->
"1:5.7"); update the function (formatDuration) to internally coerce seconds to
an integer (e.g., Math.floor or equivalent) before computing mins and secs, then
compute mins = Math.floor(secondsInt/60) and secs = secondsInt % 60 and format
secs with padStart as before so callers don't need to pre-floor values.
In `@src/lib/components/editor/panels/layers-panel.svelte`:
- Around line 180-190: The Popover.Close child snippet for the Delete button is
missing the props spread so the popover close behavior and ARIA wiring are not
applied; update the {`#snippet` child()} usage in the Popover.Close block to match
the Cancel button by using {`#snippet` child({ props })} and spread {...props}
onto the Delete <Button> (while keeping the onclick={() =>
deleteLayer(layer.id)}) so the popover will close and accessibility attributes
are preserved (adjust any references to deleteLayer or deletePopoverOpenLayerId
as needed).
- Around line 128-129: The Tailwind important modifier is using old v3 prefix
syntax; update the Svelte class directives in layers-panel.svelte that reference
deletePopoverOpenLayerId and layer.id to use the Tailwind v4 suffix form (e.g.,
change class:!max-w-40 and class:!opacity-100 to class:max-w-40! and
class:opacity-100! respectively) so the important styles are applied when
deletePopoverOpenLayerId === layer.id.
In `@src/lib/components/editor/panels/properties-panel.svelte`:
- Around line 448-454: The auto-set exit time logic incorrectly treats an
explicit 0 as missing because it uses a falsy check; change the condition that
decides whether to call projectStore.setLayerExitTime (currently using
!selectedLayer.exitTime) to a nullish/undefined check (e.g., check
selectedLayer.exitTime == null or selectedLayer.exitTime === undefined) so that
an explicit 0 is preserved; locate this in the block around
projectStore.updateLayer and projectStore.setLayerExitTime where
selectedLayer.enterTime and selectedLayer.exitTime are referenced.
- Around line 570-583: The computed max for the ScrubInput can be negative when
selectedLayer.contentDuration - selectedLayer.contentOffset >
projectStore.project.duration; update the max expression in the ScrubInput (the
block using selectedLayer.enterTime and onchange ->
projectStore.setLayerEnterTime) to clamp the result to a minimum of 0 (e.g.,
compute availableMax = projectStore.project.duration -
(selectedLayer.contentDuration - (selectedLayer.contentOffset ?? 0)) and use
Math.max(0, Math.min(projectStore.project.duration, availableMax))) so the max
never falls below the min of 0.
In `@src/lib/components/editor/project-switcher.svelte`:
- Around line 143-177: The DropdownMenu.Item that wraps the project editing UI
is auto-closing when interacting with nested controls; update the
DropdownMenu.Item (the one that contains the editing branch where
editingProjectId === proj.id and the input bound to editingName) to pass
closeOnSelect={false} so clicks/focus inside the input and the Buttons
(saveRename, cancelRename) don't dismiss the menu, and then ensure saveRename
explicitly closes the menu (or toggles the dropdown open state) after the rename
completes so the menu only closes on successful save.
In `@src/lib/components/editor/timeline/timeline-keyframe.svelte`:
- Around line 140-161: The mousedown handler currently sets isDragging = true
immediately; change handleMouseDown to record the initial mouse position and set
a "pendingDrag" state instead, add a mousemove listener (e.g., handleMouseMove)
that measures delta from the recorded start and only sets isDragging = true once
movement exceeds a small threshold (3–5px), and ensure handleMouseUp/cancel
removes the pending listeners and treats the interaction as a click if
isDragging was never set; update any logic that relied on immediate isDragging
(used by handleMouseDown/handleKeyDown and the Popover trigger) to account for
the delayed transition from pendingDrag to isDragging.
In `@src/lib/layers/components/AudioLayer.svelte`:
- Around line 147-154: currentCaption's timing ignores playbackRate causing
desync; update its calculation to mirror the audioTime formula used elsewhere:
compute audioTime = (contentOffsetProp ?? 0) + (projectStore.currentTime -
(enterTimeProp ?? 0)) * playbackRate and use that audioTime when finding active
caption in parsedCaptions (i.e., replace the current relativeTime-based lookup
with the audioTime-based lookup inside the currentCaption derived value).
In `@src/lib/layers/components/VideoLayer.svelte`:
- Around line 10-58: The schema defaults for width and height reference
projectStore but projectStore is imported only in the instance script; move the
projectStore import into the module script (the top <script context="module">
block) so the dependency is explicit for schema evaluation (used by schema and
calculateCoverDimensions), and then remove the duplicate projectStore import
from the instance script to avoid redundancy; ensure the import name remains
projectStore and that calculateCoverDimensions and ASPECT_RATIOS references
continue to resolve in the module scope.
In `@src/lib/schemas/animation.ts`:
- Around line 188-217: The LayerSchema refine closure currently checks
contentOffset/contentDuration and visibleDuration vs availableContent but misses
enforcing enterTime < exitTime; update the refine function (the anonymous
predicate passed to LayerSchema.refine) to additionally check when
data.enterTime !== undefined && data.exitTime !== undefined that data.enterTime
< data.exitTime, returning false if not, and ensure the existing error message
is adjusted or left to reflect this added ordering constraint so invalid {
enterTime: 5, exitTime: 2 } fails validation.
In `@src/lib/stores/project.svelte.ts`:
- Around line 151-165: The removeLayer method was made async but callers still
invoke it synchronously and the DELETE fetch doesn't validate HTTP errors or
needs the redundant cast removed; update every site that calls removeLayer (the
keyboard handler that triggers layer removal, the LayersPanel remove-button
call, and the onclick removal handler in the LayersPanel component) to await
store.removeLayer(...), propagate or handle rejections as appropriate, and in
removeLayer itself check the fetch response (const res = await fetch(...); if
(!res.ok) throw or log the error) before treating deletion as successful, and
remove the redundant "as string" cast since typeof already narrows
layer.props.fileKey to string.
🧹 Nitpick comments (12)
src/lib/components/ui/alert-dialog/alert-dialog-footer.svelte (1)
2-3: Import order: framework import should precede internal module.Per guidelines, imports should be ordered: External packages → SvelteKit/framework → Internal modules → Relative imports. Here
svelte/elements(framework) should come before$lib/utils.js(internal).Suggested fix
- import { cn, type WithElementRef } from '$lib/utils.js'; - import type { HTMLAttributes } from 'svelte/elements'; + import type { HTMLAttributes } from 'svelte/elements'; + import { cn, type WithElementRef } from '$lib/utils.js';As per coding guidelines,
**/*.{ts,js,svelte}: "Organize imports in order: External packages → SvelteKit → Internal modules → Relative imports".src/lib/components/ui/alert-dialog/alert-dialog-content.svelte (1)
2-6: Import order: framework and relative imports are out of sequence.Per guidelines (External → SvelteKit/framework → Internal → Relative),
svelteshould precede$lib/utils.js, and relative imports should come last.Suggested fix
import { AlertDialog as AlertDialogPrimitive } from 'bits-ui'; - import AlertDialogPortal from './alert-dialog-portal.svelte'; - import AlertDialogOverlay from './alert-dialog-overlay.svelte'; - import { cn, type WithoutChild, type WithoutChildrenOrChild } from '$lib/utils.js'; import type { ComponentProps } from 'svelte'; + import { cn, type WithoutChild, type WithoutChildrenOrChild } from '$lib/utils.js'; + import AlertDialogOverlay from './alert-dialog-overlay.svelte'; + import AlertDialogPortal from './alert-dialog-portal.svelte';As per coding guidelines,
**/*.{ts,js,svelte}: "Organize imports in order: External packages → SvelteKit → Internal modules → Relative imports".src/lib/components/editor/project-switcher.svelte (2)
58-72: Silent failure on empty name and no user feedback on error.Two concerns here:
- Line 60: If the user clears the input and clicks save, the function silently returns. Consider resetting to the original name or showing a validation hint.
- Line 68: On API failure, the error is only logged to console — the user gets no indication the rename failed, and the UI resets
renamingtofalseas if it succeeded (thougheditingProjectIdstays set since lines 65-66 are skipped).The error path is actually okay for keeping the editing state open (the
finallyonly clearsrenaming), but the user still sees no feedback. A toast or inline error indicator would improve UX.
148-157: Add anaria-labelto the rename input for accessibility.The text input has no accessible label. Screen reader users won't know this field is for renaming a project.
♿ Proposed fix
<input type="text" bind:value={editingName} class="h-7 min-w-0 flex-1 rounded border border-input bg-background px-2 text-sm" disabled={renaming} + aria-label="Rename project" onkeydown={(e) => { if (e.key === 'Enter') saveRename(e); if (e.key === 'Escape') cancelRename(e); }} autofocus />src/lib/components/editor/timeline/timeline-layer.svelte (1)
181-213: Consider addingaria-labelto the drag handles for accessibility.The enter/exit resize handles and the bar itself are interactive
<div>elements withonmousedownbut no ARIA attributes or keyboard support. Thesvelte-ignore a11y_no_static_element_interactionscomments suppress the warnings. While drag handles are typically mouse-only, addingaria-labelattributes (e.g., "Resize layer start") would improve screen reader discoverability if keyboard editing is later added.src/lib/components/editor/media-upload-utils.ts (3)
124-131:getSupportedMimeTypeisasyncbut contains noawait.
MediaRecorder.isTypeSupported()is synchronous. Theasynckeyword forces callers toawaitunnecessarily. If this is intentional for a future async codec-probing path, consider adding a comment; otherwise, dropasync.Proposed fix
-export async function getSupportedMimeType(types: string[]): Promise<string | null> { +export function getSupportedMimeType(types: string[]): string | null { for (const type of types) { if (typeof MediaRecorder !== 'undefined' && MediaRecorder.isTypeSupported(type)) { return type; } } return null; }
162-165: Static analysis:forEachcallback implicitly returnstrack.stop()result.Biome flags the arrow function in
forEachas returning a value. While harmless (forEach ignores returns), adding braces suppresses the lint and makes intent explicit.Proposed fix
export function stopMediaStream(stream: MediaStream | null): void { if (stream) { - stream.getTracks().forEach((track) => track.stop()); + stream.getTracks().forEach((track) => { track.stop(); }); } }
32-53: Uploadeddurationis the client-provided value, not server-derived.
uploadMediaBlobpassesdurationthrough theFormDatabut the returnedMediaCaptureResult.duration(Line 48) is the caller-supplied argument, not the server's response. If the server computes or validates duration (e.g., via ffprobe), consider returningdata.file.duration ?? durationto prefer the server value when available.src/lib/stores/project.svelte.ts (2)
606-633:splitLayertriggers two sequential reactive mutations tothis.project.layers.Lines 618 and 633 each assign to
this.project.layers, causing two reactive updates (and two debounced saves). Consider building the final array in a single assignment:Proposed single-pass approach
- // Update the first half (original layer) - keep same contentOffset, just trim exit time - this.project.layers = this.project.layers.map((l) => { - if (l.id === layerId) { - return { - ...l, - exitTime: time, - keyframes: l.keyframes.filter((k) => k.time <= time) - }; - } - return l; - }); - - // Add the second half after the original - const insertIndex = this.project.layers.findIndex((l) => l.id === layerId) + 1; - const layers = [...this.project.layers]; - layers.splice(insertIndex, 0, secondHalf); - this.project.layers = layers; + const newLayers: Layer[] = []; + for (const l of this.project.layers) { + if (l.id === layerId) { + newLayers.push({ + ...l, + exitTime: time, + keyframes: l.keyframes.filter((k) => k.time <= time) + }); + newLayers.push(secondHalf); + } else { + newLayers.push(l); + } + } + this.project.layers = newLayers;
392-478: Keyframe-shifting logic is duplicated between media and non-media branches.Lines 537–546 and 562–571 in
setLayerTimeRangecontain identical keyframe-shifting code. Consider extracting a small helper:function shiftKeyframes(keyframes: Keyframe[], delta: number, maxTime: number): Keyframe[] { if (delta === 0) return keyframes; return keyframes .map((kf) => ({ ...kf, time: Math.max(0, Math.min(maxTime, kf.time + delta)) })) .sort((a, b) => a.time - b.time); }This would also benefit readability in
setLayerEnterTimeandsetLayerExitTimeif keyframe shifting is added there later.Also applies to: 485-580
src/lib/components/editor/panels/background-picker.svelte (2)
329-329: Usingindexas the{#each}key for a mutable list.
ColorStophas no unique identifier, so(index)is the pragmatic fallback — but it can cause subtle rendering issues when stops are removed from the middle (Svelte may reuse the wrong DOM node). Since the list is small and removals go throughremoveColorStop(index), this is low-risk. If you ever add drag-to-reorder, consider adding a stable id to each stop.
34-81: Dual$derived+$effectsync pattern is redundant in Svelte 5.25+.Lines 34–61 declare
$derivedvalues that compute fromvalue. Lines 63–81 add an$effectthat re-assigns the same variables from the same dependency. In Svelte 5.25+, whenvaluechanges, the$derivedexpressions are automatically marked dirty and recalculate lazily on next read—even after being temporarily overridden by direct assignment (bind:value). The$effectblock duplicates this automatic re-sync behavior.Remove the
$effectblock and rely solely on$derived. The binding directives in the template will handle local overrides, and$derivedwill automatically re-evaluate whenvaluechanges.Diff
- // Sync local state when value changes from outside - $effect(() => { - activeTab = isSolid(value) ? 'solid' : 'gradient'; - if (isGradient(value)) { - gradientType = value.type; - stops = [...value.stops]; - if (value.type === 'linear') { - angle = value.angle; - } else if (value.type === 'radial') { - posX = value.position.x; - posY = value.position.y; - radialShape = value.shape; - } else if (value.type === 'conic') { - angle = value.angle; - posX = value.position.x; - posY = value.position.y; - } - } - });
| $effect(() => { | ||
| return () => { | ||
| stopMediaStream(mediaStream); | ||
| if (capturedImageUrl) { | ||
| URL.revokeObjectURL(capturedImageUrl); | ||
| } | ||
| }; | ||
| }); |
There was a problem hiding this comment.
URL.revokeObjectURL is a no-op on data URLs.
capturedImageUrl is assigned from canvasEl.toDataURL(...) (line 81), which produces a data: URI — not a blob URL created by URL.createObjectURL(). Calling revokeObjectURL on it is harmless but misleading and does not free any resources.
Either remove the call (data URLs are garbage-collected normally) or, if you want revocable URLs, switch capture to use URL.createObjectURL(blob) for the review preview.
Proposed fix (simplest: just remove the no-op)
$effect(() => {
return () => {
stopMediaStream(mediaStream);
- if (capturedImageUrl) {
- URL.revokeObjectURL(capturedImageUrl);
- }
};
});📝 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.
| $effect(() => { | |
| return () => { | |
| stopMediaStream(mediaStream); | |
| if (capturedImageUrl) { | |
| URL.revokeObjectURL(capturedImageUrl); | |
| } | |
| }; | |
| }); | |
| $effect(() => { | |
| return () => { | |
| stopMediaStream(mediaStream); | |
| }; | |
| }); |
🤖 Prompt for AI Agents
In `@src/lib/components/editor/CameraCapture.svelte` around lines 147 - 154, The
cleanup currently calls URL.revokeObjectURL(capturedImageUrl) inside the $effect
teardown, but capturedImageUrl is produced via canvasEl.toDataURL(...) (a data:
URL), so revokeObjectURL is a no-op and misleading; either remove the
revokeObjectURL call from the $effect cleanup, or change the capture flow to
produce a revocable blob URL by converting the canvas to a Blob and calling
URL.createObjectURL(blob) (update where capturedImageUrl is assigned) and then
keep URL.revokeObjectURL(capturedImageUrl) in the $effect teardown; adjust
references to capturedImageUrl and canvasEl.toDataURL accordingly and ensure
stopMediaStream(mediaStream) remains in the same teardown.
| async function extractMediaDuration(file: File): Promise<number | undefined> { | ||
| return new Promise((resolve) => { | ||
| const url = URL.createObjectURL(file); | ||
|
|
||
| if (mediaType === 'video') { | ||
| const video = document.createElement('video'); | ||
| video.preload = 'metadata'; | ||
| video.onloadedmetadata = () => { | ||
| URL.revokeObjectURL(url); | ||
| resolve(video.duration); | ||
| }; | ||
| video.onerror = () => { | ||
| URL.revokeObjectURL(url); | ||
| resolve(undefined); | ||
| }; | ||
| video.src = url; | ||
| } else if (mediaType === 'audio') { | ||
| const audio = document.createElement('audio'); | ||
| audio.preload = 'metadata'; | ||
| audio.onloadedmetadata = () => { | ||
| URL.revokeObjectURL(url); | ||
| resolve(audio.duration); | ||
| }; | ||
| audio.onerror = () => { | ||
| URL.revokeObjectURL(url); | ||
| resolve(undefined); | ||
| }; | ||
| audio.src = url; | ||
| } else { | ||
| URL.revokeObjectURL(url); | ||
| resolve(undefined); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
extractMediaDuration can hang indefinitely if metadata events never fire.
If neither onloadedmetadata nor onerror fires (e.g., corrupt file, unsupported codec), this promise never resolves, blocking the upload path forever. Add a timeout to ensure the promise always settles.
⏱️ Proposed fix: add a timeout
async function extractMediaDuration(file: File): Promise<number | undefined> {
return new Promise((resolve) => {
const url = URL.createObjectURL(file);
+ const timeout = setTimeout(() => {
+ URL.revokeObjectURL(url);
+ resolve(undefined);
+ }, 10_000);
if (mediaType === 'video') {
const video = document.createElement('video');
video.preload = 'metadata';
video.onloadedmetadata = () => {
+ clearTimeout(timeout);
URL.revokeObjectURL(url);
resolve(video.duration);
};
video.onerror = () => {
+ clearTimeout(timeout);
URL.revokeObjectURL(url);
resolve(undefined);
};
video.src = url;
} else if (mediaType === 'audio') {
const audio = document.createElement('audio');
audio.preload = 'metadata';
audio.onloadedmetadata = () => {
+ clearTimeout(timeout);
URL.revokeObjectURL(url);
resolve(audio.duration);
};
audio.onerror = () => {
+ clearTimeout(timeout);
URL.revokeObjectURL(url);
resolve(undefined);
};
audio.src = url;
} else {
+ clearTimeout(timeout);
URL.revokeObjectURL(url);
resolve(undefined);
}
});
}🤖 Prompt for AI Agents
In `@src/lib/components/editor/FileUpload.svelte` around lines 53 - 86, The
extractMediaDuration promise can hang if neither onloadedmetadata nor onerror
fires; modify extractMediaDuration to add a timeout (e.g., 10s) that will revoke
the object URL and resolve(undefined) if neither event fires, and ensure the
timeout is cleared when onloadedmetadata or onerror runs; apply this for both
the video and audio branches, remove event listeners and revoke URL in all
resolution paths to avoid leaks, and reference the existing mediaType check and
the created elements (video/audio) so the timer and cleanup are tied to those
element handlers.
| async function handleFileSelect(e: Event) { | ||
| const input = e.target as HTMLInputElement; | ||
| const file = input.files?.[0]; | ||
| if (!file) return; | ||
|
|
||
| isUploading = true; | ||
| uploadError = ''; | ||
|
|
||
| try { | ||
| // Extract duration for video/audio files | ||
| let duration: number | undefined; | ||
| if (mediaType === 'video' || mediaType === 'audio') { | ||
| duration = await extractMediaDuration(file); | ||
| } | ||
|
|
||
| const formData = new FormData(); | ||
| formData.append('file', file); | ||
| formData.append('mediaType', mediaType); | ||
| if (projectId) { | ||
| formData.append('projectId', projectId); | ||
| } | ||
|
|
||
| const res = await fetch('/api/upload', { | ||
| method: 'POST', | ||
| body: formData | ||
| }); | ||
|
|
||
| if (!res.ok) { | ||
| const data = await res.json().catch(() => null); | ||
| throw new Error(data?.message || `Upload failed (${res.status})`); | ||
| } | ||
|
|
||
| const data = await res.json(); | ||
| if (data.success && data.file) { | ||
| onUpload({ | ||
| url: data.file.url, | ||
| key: data.file.key, | ||
| fileName: data.file.originalName, | ||
| duration | ||
| }); | ||
| } | ||
| } catch (err) { | ||
| uploadError = err instanceof Error ? err.message : 'Upload failed'; | ||
| } finally { | ||
| isUploading = false; | ||
| // Reset input | ||
| if (input) input.value = ''; | ||
| } | ||
| } |
There was a problem hiding this comment.
No client-side file size validation before uploading.
The maxSizes hints are displayed (Line 238) but the file size is never checked before initiating the upload. A user could select a multi-GB file and wait for a long upload only to have the server reject it.
🛡️ Proposed fix: validate file size before uploading
+ const maxSizeBytes: Record<string, number> = {
+ image: 10 * 1024 * 1024,
+ video: 500 * 1024 * 1024,
+ audio: 100 * 1024 * 1024
+ };
async function handleFileSelect(e: Event) {
const input = e.target as HTMLInputElement;
const file = input.files?.[0];
if (!file) return;
+ if (file.size > maxSizeBytes[mediaType]) {
+ uploadError = `File too large. Maximum size is ${maxSizes[mediaType]}.`;
+ input.value = '';
+ return;
+ }
+
isUploading = true;
uploadError = '';📝 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.
| async function handleFileSelect(e: Event) { | |
| const input = e.target as HTMLInputElement; | |
| const file = input.files?.[0]; | |
| if (!file) return; | |
| isUploading = true; | |
| uploadError = ''; | |
| try { | |
| // Extract duration for video/audio files | |
| let duration: number | undefined; | |
| if (mediaType === 'video' || mediaType === 'audio') { | |
| duration = await extractMediaDuration(file); | |
| } | |
| const formData = new FormData(); | |
| formData.append('file', file); | |
| formData.append('mediaType', mediaType); | |
| if (projectId) { | |
| formData.append('projectId', projectId); | |
| } | |
| const res = await fetch('/api/upload', { | |
| method: 'POST', | |
| body: formData | |
| }); | |
| if (!res.ok) { | |
| const data = await res.json().catch(() => null); | |
| throw new Error(data?.message || `Upload failed (${res.status})`); | |
| } | |
| const data = await res.json(); | |
| if (data.success && data.file) { | |
| onUpload({ | |
| url: data.file.url, | |
| key: data.file.key, | |
| fileName: data.file.originalName, | |
| duration | |
| }); | |
| } | |
| } catch (err) { | |
| uploadError = err instanceof Error ? err.message : 'Upload failed'; | |
| } finally { | |
| isUploading = false; | |
| // Reset input | |
| if (input) input.value = ''; | |
| } | |
| } | |
| const maxSizeBytes: Record<string, number> = { | |
| image: 10 * 1024 * 1024, | |
| video: 500 * 1024 * 1024, | |
| audio: 100 * 1024 * 1024 | |
| }; | |
| async function handleFileSelect(e: Event) { | |
| const input = e.target as HTMLInputElement; | |
| const file = input.files?.[0]; | |
| if (!file) return; | |
| if (file.size > maxSizeBytes[mediaType]) { | |
| uploadError = `File too large. Maximum size is ${maxSizes[mediaType]}.`; | |
| input.value = ''; | |
| return; | |
| } | |
| isUploading = true; | |
| uploadError = ''; | |
| try { | |
| // Extract duration for video/audio files | |
| let duration: number | undefined; | |
| if (mediaType === 'video' || mediaType === 'audio') { | |
| duration = await extractMediaDuration(file); | |
| } | |
| const formData = new FormData(); | |
| formData.append('file', file); | |
| formData.append('mediaType', mediaType); | |
| if (projectId) { | |
| formData.append('projectId', projectId); | |
| } | |
| const res = await fetch('/api/upload', { | |
| method: 'POST', | |
| body: formData | |
| }); | |
| if (!res.ok) { | |
| const data = await res.json().catch(() => null); | |
| throw new Error(data?.message || `Upload failed (${res.status})`); | |
| } | |
| const data = await res.json(); | |
| if (data.success && data.file) { | |
| onUpload({ | |
| url: data.file.url, | |
| key: data.file.key, | |
| fileName: data.file.originalName, | |
| duration | |
| }); | |
| } | |
| } catch (err) { | |
| uploadError = err instanceof Error ? err.message : 'Upload failed'; | |
| } finally { | |
| isUploading = false; | |
| // Reset input | |
| if (input) input.value = ''; | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@src/lib/components/editor/FileUpload.svelte` around lines 88 - 136,
handleFileSelect currently starts uploading immediately without checking
file.size; add client-side validation against the displayed maxSizes (per
mediaType) before setting isUploading. In handleFileSelect (before setting
isUploading = true and before calling extractMediaDuration), compute the
allowedMax = maxSizes[mediaType] (convert units to bytes if needed), compare
file.size to allowedMax, and if it exceeds set uploadError to a clear message,
clear the input.value, and return early. Ensure the same symbols are used (file,
mediaType, maxSizes, uploadError, isUploading, input) so the UI shows the error
and no network request is made.
| export function formatDuration(seconds: number): string { | ||
| const mins = Math.floor(seconds / 60); | ||
| const secs = seconds % 60; | ||
| return `${mins}:${secs.toString().padStart(2, '0')}`; | ||
| } |
There was a problem hiding this comment.
formatDuration produces fractional seconds when input isn't an integer.
If called with e.g. formatDuration(65.7), secs is 5.7 → output is "1:5.7" instead of "1:05". The caller in AudioRecorder wraps with Math.floor, but other consumers may not. Floor the seconds inside the utility for robustness.
Proposed fix
export function formatDuration(seconds: number): string {
- const mins = Math.floor(seconds / 60);
- const secs = seconds % 60;
+ const totalSecs = Math.floor(seconds);
+ const mins = Math.floor(totalSecs / 60);
+ const secs = totalSecs % 60;
return `${mins}:${secs.toString().padStart(2, '0')}`;
}📝 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.
| export function formatDuration(seconds: number): string { | |
| const mins = Math.floor(seconds / 60); | |
| const secs = seconds % 60; | |
| return `${mins}:${secs.toString().padStart(2, '0')}`; | |
| } | |
| export function formatDuration(seconds: number): string { | |
| const totalSecs = Math.floor(seconds); | |
| const mins = Math.floor(totalSecs / 60); | |
| const secs = totalSecs % 60; | |
| return `${mins}:${secs.toString().padStart(2, '0')}`; | |
| } |
🤖 Prompt for AI Agents
In `@src/lib/components/editor/media-upload-utils.ts` around lines 71 - 75,
formatDuration currently uses the raw seconds parameter so non-integer inputs
produce fractional seconds (e.g., formatDuration(65.7) -> "1:5.7"); update the
function (formatDuration) to internally coerce seconds to an integer (e.g.,
Math.floor or equivalent) before computing mins and secs, then compute mins =
Math.floor(secondsInt/60) and secs = secondsInt % 60 and format secs with
padStart as before so callers don't need to pre-floor values.
| <Popover.Root> | ||
| <Popover.Trigger> | ||
| {#snippet child({ props })} | ||
| <button | ||
| class="absolute top-1/2 h-4 w-4 -translate-x-1/2 -translate-y-1/2 cursor-grab rounded-sm border-2 transition-transform hover:scale-110 active:cursor-grabbing" | ||
| class:bg-primary={isSelected} | ||
| class:bg-muted-foreground={!isSelected} | ||
| class:border-primary={isSelected} | ||
| class:border-background={!isSelected} | ||
| style="left: {position}px" | ||
| onmousedown={handleMouseDown} | ||
| onkeydown={handleKeyDown} | ||
| oncontextmenu={(e) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| }} | ||
| aria-label="Keyframe at {firstKeyframe.time.toFixed(2)}s" | ||
| {...props} | ||
| > | ||
| </button> | ||
| {/snippet} | ||
| </Popover.Trigger> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/lib/components/editor/timeline/timeline-keyframe.svelte | head -100Repository: epavanello/devmotion
Length of output: 3783
🏁 Script executed:
cat -n src/lib/components/editor/timeline/timeline-keyframe.svelte | sed -n '50,90p'Repository: epavanello/devmotion
Length of output: 1476
🏁 Script executed:
cat -n src/lib/components/editor/timeline/timeline-keyframe.svelte | sed -n '1,20p'Repository: epavanello/devmotion
Length of output: 951
Consider adding a minimum movement threshold before committing to drag behavior.
The code sets isDragging = true immediately on mousedown without checking for minimum movement distance. While browser behavior typically prevents click synthesis after actual mouse movement, adding an explicit threshold (e.g., 3–5 pixels) before committing to the drag would make the interaction more robust and match common UI patterns. This prevents unintended popover opens from accidental micro-movements during what the user intends as a simple click.
🤖 Prompt for AI Agents
In `@src/lib/components/editor/timeline/timeline-keyframe.svelte` around lines 140
- 161, The mousedown handler currently sets isDragging = true immediately;
change handleMouseDown to record the initial mouse position and set a
"pendingDrag" state instead, add a mousemove listener (e.g., handleMouseMove)
that measures delta from the recorded start and only sets isDragging = true once
movement exceeds a small threshold (3–5px), and ensure handleMouseUp/cancel
removes the pending listeners and treats the interaction as a click if
isDragging was never set; update any logic that relied on immediate isDragging
(used by handleMouseDown/handleKeyDown and the Popover trigger) to account for
the delayed transition from pendingDrag to isDragging.
| const currentCaption = $derived.by(() => { | ||
| if (!showCaptions || parsedCaptions.length === 0) return ''; | ||
| const enterTime = enterTimeProp ?? 0; | ||
| const contentOffset = contentOffsetProp ?? 0; | ||
| const relativeTime = projectStore.currentTime - enterTime + contentOffset; | ||
| const active = parsedCaptions.find((c) => relativeTime >= c.start && relativeTime < c.end); | ||
| return active?.text || ''; | ||
| }); |
There was a problem hiding this comment.
Caption timing ignores playbackRate — captions will desync at non-1x speeds.
Line 151 computes relativeTime = projectStore.currentTime - enterTime + contentOffset, but the audio sync effect (Line 104) computes audioTime = contentOffset + relativeTime * playbackRate. Caption lookup should use the same audioTime formula so captions stay aligned with the actual audio playback position.
🐛 Proposed fix
const currentCaption = $derived.by(() => {
if (!showCaptions || parsedCaptions.length === 0) return '';
const enterTime = enterTimeProp ?? 0;
const contentOffset = contentOffsetProp ?? 0;
- const relativeTime = projectStore.currentTime - enterTime + contentOffset;
+ const relativeTime = projectStore.currentTime - enterTime;
+ const audioTime = contentOffset + relativeTime * playbackRate;
- const active = parsedCaptions.find((c) => relativeTime >= c.start && relativeTime < c.end);
+ const active = parsedCaptions.find((c) => audioTime >= c.start && audioTime < c.end);
return active?.text || '';
});📝 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.
| const currentCaption = $derived.by(() => { | |
| if (!showCaptions || parsedCaptions.length === 0) return ''; | |
| const enterTime = enterTimeProp ?? 0; | |
| const contentOffset = contentOffsetProp ?? 0; | |
| const relativeTime = projectStore.currentTime - enterTime + contentOffset; | |
| const active = parsedCaptions.find((c) => relativeTime >= c.start && relativeTime < c.end); | |
| return active?.text || ''; | |
| }); | |
| const currentCaption = $derived.by(() => { | |
| if (!showCaptions || parsedCaptions.length === 0) return ''; | |
| const enterTime = enterTimeProp ?? 0; | |
| const contentOffset = contentOffsetProp ?? 0; | |
| const relativeTime = projectStore.currentTime - enterTime; | |
| const audioTime = contentOffset + relativeTime * playbackRate; | |
| const active = parsedCaptions.find((c) => audioTime >= c.start && audioTime < c.end); | |
| return active?.text || ''; | |
| }); |
🤖 Prompt for AI Agents
In `@src/lib/layers/components/AudioLayer.svelte` around lines 147 - 154,
currentCaption's timing ignores playbackRate causing desync; update its
calculation to mirror the audioTime formula used elsewhere: compute audioTime =
(contentOffsetProp ?? 0) + (projectStore.currentTime - (enterTimeProp ?? 0)) *
playbackRate and use that audioTime when finding active caption in
parsedCaptions (i.e., replace the current relativeTime-based lookup with the
audioTime-based lookup inside the currentCaption derived value).
| const schema = z.object({ | ||
| src: z.string().default('').describe('Video source URL or uploaded file URL'), | ||
| width: z | ||
| .number() | ||
| .min(1) | ||
| .max(5000) | ||
| .default( | ||
| () => | ||
| calculateCoverDimensions( | ||
| projectStore.project.width, | ||
| projectStore.project.height, | ||
| ASPECT_RATIOS.VIDEO_DEFAULT | ||
| ).width | ||
| ) | ||
| .describe('Width (px)'), | ||
| height: z | ||
| .number() | ||
| .min(1) | ||
| .max(5000) | ||
| .default( | ||
| () => | ||
| calculateCoverDimensions( | ||
| projectStore.project.width, | ||
| projectStore.project.height, | ||
| ASPECT_RATIOS.VIDEO_DEFAULT | ||
| ).height | ||
| ) | ||
| .describe('Height (px)'), | ||
| objectFit: z | ||
| .enum(['contain', 'cover', 'fill', 'none', 'scale-down']) | ||
| .default('cover') | ||
| .describe('Object fit mode'), | ||
| /** Playback volume (0-1) */ | ||
| volume: z.number().min(0).max(1).default(1).describe('Volume (0-1)'), | ||
| /** Whether the video is muted */ | ||
| muted: z.boolean().default(false).describe('Mute audio'), | ||
| /** Playback rate */ | ||
| playbackRate: z.number().min(0.1).max(4).default(1).describe('Playback rate'), | ||
| /** The storage key if file was uploaded (used for cleanup) */ | ||
| fileKey: z.string().default('').describe('Storage key (for uploaded files)'), | ||
| /** Original filename if uploaded */ | ||
| fileName: z.string().default('').describe('Original filename'), | ||
| /** Layer ID - passed by LayerWrapper for time sync */ | ||
| layerId: z.string().optional().describe('Layer ID (internal)'), | ||
| /** Enter time - passed by LayerWrapper for time sync */ | ||
| enterTime: z.number().optional().describe('Enter time (internal)'), | ||
| /** Content offset - passed by LayerWrapper for content trimming */ | ||
| contentOffset: z.number().optional().describe('Content offset (internal)') | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, view the VideoLayer.svelte file to see the actual imports and schema
cat -n src/lib/layers/components/VideoLayer.svelte | head -80Repository: epavanello/devmotion
Length of output: 3164
🏁 Script executed:
#!/bin/bash
# Check if projectStore is imported in instance script and other imports
cat -n src/lib/layers/components/VideoLayer.svelte | sed -n '69,85p'Repository: epavanello/devmotion
Length of output: 471
🏁 Script executed:
#!/bin/bash
# List all layer component files to understand the pattern
find src/lib/layers/components -name "*.svelte" -type fRepository: epavanello/devmotion
Length of output: 684
🏁 Script executed:
#!/bin/bash
# Check how other layer components handle imports and schema
for file in src/lib/layers/components/*.svelte; do
echo "=== $(basename "$file") ==="
head -80 "$file" | grep -A 50 "script module"
doneRepository: epavanello/devmotion
Length of output: 25762
🏁 Script executed:
#!/bin/bash
# Search for registerLayer usage in the codebase
rg "registerLayer" --type svelte --type ts -nRepository: epavanello/devmotion
Length of output: 94
🏁 Script executed:
#!/bin/bash
# Check the registry to understand how layers are registered
find src -name "*registry*" -type fRepository: epavanello/devmotion
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Search for registerLayer calls in TypeScript/JavaScript files
rg "registerLayer" src/ -t ts -t jsRepository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check the registry file to understand layer registration
cat -n src/lib/layers/registry.ts | head -100Repository: epavanello/devmotion
Length of output: 2614
Move projectStore import to the module script to clarify the dependency.
The schema defaults on lines 19–20 and 32–33 reference projectStore, which is currently imported only in the instance script (line 72). While this works because Svelte hoists all imports to module level, it obscures the actual dependency. ImageLayer.svelte imports projectStore in the module script where it's needed, which is the clearer pattern—follow that approach.
♻️ Proposed fix
<script module lang="ts">
import { z } from 'zod';
import type { LayerMeta } from '../registry';
import { Video } from '@lucide/svelte';
import { calculateCoverDimensions, ASPECT_RATIOS } from '$lib/utils/media';
+ import { projectStore } from '$lib/stores/project.svelte';Then remove the duplicate import from line 72.
🤖 Prompt for AI Agents
In `@src/lib/layers/components/VideoLayer.svelte` around lines 10 - 58, The schema
defaults for width and height reference projectStore but projectStore is
imported only in the instance script; move the projectStore import into the
module script (the top <script context="module"> block) so the dependency is
explicit for schema evaluation (used by schema and calculateCoverDimensions),
and then remove the duplicate projectStore import from the instance script to
avoid redundancy; ensure the import name remains projectStore and that
calculateCoverDimensions and ASPECT_RATIOS references continue to resolve in the
module scope.
src/lib/schemas/animation.ts
Outdated
| .refine( | ||
| (data) => { | ||
| // Validate contentOffset doesn't exceed contentDuration | ||
| if ( | ||
| data.contentDuration !== undefined && | ||
| data.contentOffset !== undefined && | ||
| data.contentOffset >= data.contentDuration | ||
| ) { | ||
| return false; | ||
| } | ||
| // Validate visible duration doesn't exceed available content | ||
| if ( | ||
| data.contentDuration !== undefined && | ||
| data.enterTime !== undefined && | ||
| data.exitTime !== undefined | ||
| ) { | ||
| const visibleDuration = data.exitTime - data.enterTime; | ||
| const contentOffset = data.contentOffset ?? 0; | ||
| const availableContent = data.contentDuration - contentOffset; | ||
| if (visibleDuration > availableContent) { | ||
| return false; | ||
| } | ||
| } | ||
| return true; | ||
| }, | ||
| { | ||
| message: | ||
| 'Content duration and offset constraints violated: offset must be < duration, and visible duration must fit within available content' | ||
| } | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/lib/schemas/animation.ts | sed -n '180,220p'Repository: epavanello/devmotion
Length of output: 1768
🏁 Script executed:
rg -n "enterTime|exitTime" --type=ts -A 2 -B 2 | head -80Repository: epavanello/devmotion
Length of output: 5792
🏁 Script executed:
# Search for any validation or constraint checks on enterTime/exitTime
rg -n "enterTime.*<|enterTime.*>|exitTime" --type=ts --type=svelte | grep -E "(enterTime|exitTime)" | head -40Repository: epavanello/devmotion
Length of output: 94
🏁 Script executed:
# Look for store mutations or handlers that might validate this
fd -e ts -e svelte | xargs grep -l "enterTime\|exitTime" | head -20Repository: epavanello/devmotion
Length of output: 521
🏁 Script executed:
# Check how layers are created/parsed from external data (API, imports, etc.)
rg -n "LayerSchema.*parse|LayerSchema.*safeParse|parse.*Layer" --type=ts -B 2 -A 2 | head -50Repository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
# Check if there are any API endpoints that accept/validate layer data
fd -e ts -path "*/routes/*" | xargs grep -l "Layer\|layer" 2>/dev/null | head -10Repository: epavanello/devmotion
Length of output: 232
🏁 Script executed:
# Look at the setLayerExitTime function to see if it validates enterTime < exitTime
sed -n '485,520p' src/lib/stores/project.svelte.tsRepository: epavanello/devmotion
Length of output: 1636
🏁 Script executed:
# Check if LayerSchema is used directly for parsing external data (API, imports)
rg -n "safeParse|\.parse\(" src/lib/schemas/animation.ts -A 3 -B 3Repository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
# Search for API endpoints or server-side code that might parse layers
rg -n "LayerSchema\|ProjectSchema" --type=ts -A 2 -B 2 | head -60Repository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
# Check setLayerEnterTime more closely to see how it validates enterTime vs exitTime
sed -n '401,478p' src/lib/stores/project.svelte.tsRepository: epavanello/devmotion
Length of output: 3206
🏁 Script executed:
# Check the full setLayerTimeRange function
sed -n '507,550p' src/lib/stores/project.svelte.tsRepository: epavanello/devmotion
Length of output: 1957
🏁 Script executed:
# Look for server-side endpoints or imports that might parse layer data
rg -n "import.*Layer\|from.*animation" --type=ts -A 1 -B 1 | head -50Repository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
# Check if there are any database/API operations that might load layers
fd -type f \( -name "*.ts" -o -name "*.svelte" \) -path "*/routes/*" -o -path "*/server/*" | xargs grep -l "layer" 2>/dev/null | head -5Repository: epavanello/devmotion
Length of output: 234
🏁 Script executed:
# Search for where LayerSchema is used (parse, safeParse, or in type context)
rg "LayerSchema" --type=ts -B 2 -A 2Repository: epavanello/devmotion
Length of output: 2856
🏁 Script executed:
# Check if there's any API endpoint that loads/saves projects
find src -name "*.ts" | xargs grep -l "POST\|GET\|DELETE" 2>/dev/null | head -5Repository: epavanello/devmotion
Length of output: 254
🏁 Script executed:
# Look at the entire Layer type and how it's used
grep -n "type Layer\|= z.infer" src/lib/schemas/animation.ts | head -20Repository: epavanello/devmotion
Length of output: 1299
🏁 Script executed:
# Check the server-side API endpoints to see if they parse/validate layer data
cat src/routes/api/captions/+server.tsRepository: epavanello/devmotion
Length of output: 3707
🏁 Script executed:
# Check if projects are loaded/parsed from database or API
rg "ProjectSchema\|LayerSchema" src/routes -A 3 -B 1Repository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
# Look for any JSON import/export functionality
rg "JSON\.parse\|JSON\.stringify" src/lib/stores --type=ts -B 2 -A 2 | head -40Repository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
# Check if there are any import/export functions that might load layer data from files
rg "export.*project\|import.*project" src/lib/stores -A 3 -B 1 | head -40Repository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
# Look for any AI mutation functions that might create layers
cat src/lib/ai/mutations.ts | head -100Repository: epavanello/devmotion
Length of output: 3072
🏁 Script executed:
# Check the createLayer function to see if it validates enterTime < exitTime
cat src/lib/engine/layer-factory.ts | head -80Repository: epavanello/devmotion
Length of output: 1959
🏁 Script executed:
# Look for any tests that might show expected behavior with enterTime/exitTime
rg -n "enterTime.*exitTime\|exitTime.*enterTime" src --type=ts -B 1 -A 1 | head -60Repository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
# Check how createLayer is used - does it always go through store mutations?
rg -n "createLayer" src/lib --type=ts -B 2 -A 2 | head -60Repository: epavanello/devmotion
Length of output: 1075
🏁 Script executed:
# Verify that ProjectSchema.safeParse is used anywhere to validate external data
rg -n "ProjectSchema\|LayerSchema.*safeParse\|safeParse.*project" src --type=tsRepository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
# Look for any direct instantiation of Layer objects without store mutations
rg -n "enterTime.*exitTime\|new Layer" src --type=ts -B 1 -A 1 | grep -v "setLayer\|store"Repository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
# Check if createLayer output passes LayerSchema validation
rg -n "projectDataSchema\|ProjectSchema" src/lib/schemas/animation.ts -A 2 -B 2Repository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
# Look at the complete animation.ts to see the full Layer type definition
sed -n '140,290p' src/lib/schemas/animation.tsRepository: epavanello/devmotion
Length of output: 5200
Add enterTime < exitTime validation to the LayerSchema refine block.
The refine currently validates content offset/duration constraints but doesn't check that enterTime < exitTime. While store mutations (setLayerEnterTime, setLayerExitTime) enforce this through Math.max(clamped, other ± 0.1) logic, the schema should validate it independently. Without this check, a layer with enterTime: 5, exitTime: 2 technically passes schema validation since the negative visibleDuration satisfies the content constraint. This creates a validation gap.
Proposed fix — add enterTime/exitTime ordering check
.refine(
(data) => {
+ // Validate enterTime < exitTime when both are defined
+ if (
+ data.enterTime !== undefined &&
+ data.exitTime !== undefined &&
+ data.exitTime <= data.enterTime
+ ) {
+ return false;
+ }
// Validate contentOffset doesn't exceed contentDuration
if (🤖 Prompt for AI Agents
In `@src/lib/schemas/animation.ts` around lines 188 - 217, The LayerSchema refine
closure currently checks contentOffset/contentDuration and visibleDuration vs
availableContent but misses enforcing enterTime < exitTime; update the refine
function (the anonymous predicate passed to LayerSchema.refine) to additionally
check when data.enterTime !== undefined && data.exitTime !== undefined that
data.enterTime < data.exitTime, returning false if not, and ensure the existing
error message is adjusted or left to reflect this added ordering constraint so
invalid { enterTime: 5, exitTime: 2 } fails validation.
| async removeLayer(layerId: string) { | ||
| // Find the layer to check if it has uploaded files to clean up | ||
| const layer = this.project.layers.find((l) => l.id === layerId); | ||
|
|
||
| // Clean up uploaded files if the layer has a fileKey | ||
| if (layer && layer.props.fileKey && typeof layer.props.fileKey === 'string') { | ||
| try { | ||
| await fetch(`/api/upload/${encodeURIComponent(layer.props.fileKey as string)}`, { | ||
| method: 'DELETE' | ||
| }); | ||
| } catch (err) { | ||
| console.warn('Failed to delete file from storage:', err); | ||
| // Continue with layer deletion even if file cleanup fails | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n --type=ts --type=svelte -C3 'removeLayer\b' -g '!**/project.svelte.ts'Repository: epavanello/devmotion
Length of output: 94
🏁 Script executed:
rg -n 'removeLayer\b' --type=ts -C3 -g '!**/project.svelte.ts'Repository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
rg -n 'removeLayer' -C3 -g '!**/project.svelte.ts'Repository: epavanello/devmotion
Length of output: 1233
🏁 Script executed:
cat -n src/lib/components/editor/keyboard-handler.svelte | sed -n '20,35p'Repository: epavanello/devmotion
Length of output: 683
🏁 Script executed:
cat -n src/lib/components/editor/panels/layers-panel.svelte | sed -n '30,45p'Repository: epavanello/devmotion
Length of output: 622
🏁 Script executed:
cat -n src/lib/components/editor/panels/layers-panel.svelte | sed -n '1,80p'Repository: epavanello/devmotion
Length of output: 3390
🏁 Script executed:
cat -n src/lib/components/editor/panels/layers-panel.svelte | sed -n '80,150p'Repository: epavanello/devmotion
Length of output: 3106
🏁 Script executed:
cat -n src/lib/components/editor/panels/layers-panel.svelte | sed -n '150,180p'Repository: epavanello/devmotion
Length of output: 1634
🏁 Script executed:
cat -n src/lib/components/editor/panels/layers-panel.svelte | sed -n '180,195p'Repository: epavanello/devmotion
Length of output: 721
🏁 Script executed:
cat -n src/lib/components/editor/keyboard-handler.svelte | sed -n '1,40p'Repository: epavanello/devmotion
Length of output: 1643
Update call sites to properly await the now-async removeLayer method.
Making removeLayer async means all three call sites (keyboard-handler.svelte:27, layers-panel.svelte:37, and the onclick handler at layers-panel.svelte:185) must be updated to await the result. Currently, they invoke it synchronously, causing file cleanup to silently become fire-and-forget with no error propagation.
Additionally, the fetch response status is never checked — HTTP errors (4xx/5xx) are silently ignored in the catch block. Validate the response with if (!res.ok) before considering the deletion successful.
Also remove the redundant as string cast on line 158, since the type is already narrowed by the typeof layer.props.fileKey === 'string' check on line 156.
🤖 Prompt for AI Agents
In `@src/lib/stores/project.svelte.ts` around lines 151 - 165, The removeLayer
method was made async but callers still invoke it synchronously and the DELETE
fetch doesn't validate HTTP errors or needs the redundant cast removed; update
every site that calls removeLayer (the keyboard handler that triggers layer
removal, the LayersPanel remove-button call, and the onclick removal handler in
the LayersPanel component) to await store.removeLayer(...), propagate or handle
rejections as appropriate, and in removeLayer itself check the fetch response
(const res = await fetch(...); if (!res.ok) throw or log the error) before
treating deletion as successful, and remove the redundant "as string" cast since
typeof already narrows layer.props.fileKey to string.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Dockerfile (1)
25-46:⚠️ Potential issue | 🟡 MinorRemove OPENAI_API_KEY and OPENROUTER_API_KEY from Docker build—these use
$env/dynamic/private.
OPENAI_API_KEYandOPENROUTER_API_KEYare imported via$env/dynamic/privatein the SvelteKit code (not static), meaning they're accessed only at runtime during request handling. They do not need to be present at build time and should not be baked into the Docker build layer viaARG/ENV. The runtimeenvironmentblock indocker-compose.yamlalready provides them.Remove lines 27 (OPENAI_API_KEY) and the OPENROUTER_API_KEY mapping from the Dockerfile builder stage.
Note: The S3 credentials and PRIVATE_DATABASE_URL do need to remain as static envs since they're used in server initialization code that executes at build time.
🤖 Fix all issues with AI agents
In `@docker-compose.yaml`:
- Around line 12-18: Remove the sensitive build-time args from the
docker-compose build args block so secrets like OPENAI_API_KEY,
PRIVATE_S3_BUCKET, PRIVATE_S3_REGION, PRIVATE_S3_ENDPOINT,
PRIVATE_S3_ACCESS_KEY_ID, PRIVATE_S3_SECRET_ACCESS_KEY, and
PRIVATE_S3_PUBLIC_URL are not passed into the build context; keep those values
only in the runtime environment section (the environment block) and ensure the
Dockerfile no longer defines corresponding ARG/ENV entries so build args are
unnecessary.
| - OPENAI_API_KEY=${OPENAI_API_KEY} | ||
| - PRIVATE_S3_BUCKET=${PRIVATE_S3_BUCKET} | ||
| - PRIVATE_S3_REGION=${PRIVATE_S3_REGION} | ||
| - PRIVATE_S3_ENDPOINT=${PRIVATE_S3_ENDPOINT} | ||
| - PRIVATE_S3_ACCESS_KEY_ID=${PRIVATE_S3_ACCESS_KEY_ID} | ||
| - PRIVATE_S3_SECRET_ACCESS_KEY=${PRIVATE_S3_SECRET_ACCESS_KEY} | ||
| - PRIVATE_S3_PUBLIC_URL=${PRIVATE_S3_PUBLIC_URL} |
There was a problem hiding this comment.
Build args propagate secrets into the Docker build context — same concern as the Dockerfile.
If the ARG/ENV lines are removed from the Dockerfile per the suggestion above, these build args entries should be removed as well. The runtime environment block (lines 26–32) is sufficient for server-side secrets.
🤖 Prompt for AI Agents
In `@docker-compose.yaml` around lines 12 - 18, Remove the sensitive build-time
args from the docker-compose build args block so secrets like OPENAI_API_KEY,
PRIVATE_S3_BUCKET, PRIVATE_S3_REGION, PRIVATE_S3_ENDPOINT,
PRIVATE_S3_ACCESS_KEY_ID, PRIVATE_S3_SECRET_ACCESS_KEY, and
PRIVATE_S3_PUBLIC_URL are not passed into the build context; keep those values
only in the runtime environment section (the environment block) and ensure the
Dockerfile no longer defines corresponding ARG/ENV entries so build args are
unnecessary.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/lib/ai/schemas.ts`:
- Around line 224-236: Add a cross-field validation to both
CreateLayerInputSchema and EditLayerInputSchema to ensure that when both
enterTime and exitTime are present, enterTime is strictly less than exitTime:
for each schema append a .refine() that checks (data.enterTime === undefined ||
data.exitTime === undefined) || data.enterTime < data.exitTime and provide a
clear error message/path (e.g., { message: 'enterTime must be less than
exitTime', path: ['enterTime','exitTime'] }) so validation fails when enterTime
>= exitTime.
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/components/editor/canvas/canvas.svelte (1)
179-184: 🛠️ Refactor suggestion | 🟠 MajorUse
$derived.byfor block-body derivations.
$derived(() => { ... })stores an arrow function as the derived value, which is why you needrecordingScale()on line 228. The idiomatic Svelte 5 pattern for a derived value that needs a block body is$derived.by(...), which would makerecordingScalea number directly.♻️ Proposed fix
- const recordingScale = $derived(() => { + const recordingScale = $derived.by(() => { if (typeof window === 'undefined') return 1; const scaleX = window.innerWidth / projectStore.project.width; const scaleY = window.innerHeight / projectStore.project.height; return Math.min(1, scaleX, scaleY); // Never scale up, only down });Then update the usage on line 228:
- style:transform={isRecording ? `scale(${recordingScale()})` : undefined} + style:transform={isRecording ? `scale(${recordingScale})` : undefined}
🤖 Fix all issues with AI agents
In `@src/lib/components/editor/panels/background-picker.svelte`:
- Around line 343-372: The each-block uses the numeric index as the key which
causes DOM reuse bugs when a stop is removed; change the key from (index) to a
stable unique identifier like stop.id (or a composite stable key such as
`${stop.color}-${stop.position}`) in the {`#each` stops as stop, index ( ... )}
and ensure new color stops are created with a unique id (update the code paths
that create/add stops so they assign an id); keep the existing handlers
(updateColorStop, removeColorStop) unchanged but reference stops by id in any
logic that expects stable identity.
In `@src/lib/components/editor/panels/input-pin.svelte`:
- Around line 12-23: The prop "property" is currently typed as string but is
passed into addKeyframe which expects AnimatableProperty; update the prop
declaration so property: AnimatableProperty instead of string (ensure the
AnimatableProperty type is imported/available in this component) so the
signature of property matches addKeyframe(property) and preserves compile-time
type safety.
In `@src/lib/components/editor/panels/inputs-wrapper.svelte`:
- Around line 2-4: Remove the unused imports Label and projectStore from the top
of the file: locate the import statements that bring in Label (import Label from
'$lib/components/ui/label/label.svelte') and projectStore (import { projectStore
} from '$lib/stores/project.svelte') and delete them; ensure no other code in
this module references the symbols Label or projectStore (rename or keep if used
elsewhere), then run lint to confirm ESLint no longer reports these unused
imports.
In `@src/lib/components/editor/panels/layers-panel.svelte`:
- Around line 183-194: The Popover.Close's internal close handler is being
overridden because onclick is placed after the spread {...props} in the snippet
inside Popover.Close; update the delete button's click handler to reset the
popover state instead of relying on the Popover's handler — inside the handler
used by the Button (the function passed to onclick that calls
deleteLayer(layer.id)), also set deletePopoverOpenLayerId = null (or call the
function that resets it) before/after deleteLayer to ensure the stale ID is
cleared; alternatively you can move the onclick so it appears before {...props}
but the preferred fix is to explicitly reset deletePopoverOpenLayerId in the
deleteLayer click handler referenced in the snippet child({ props }) / Button.
In `@src/lib/components/editor/panels/properties-panel.svelte`:
- Around line 97-102: Remove the debug console.log in the derived computation
for layerCustomPropertyComponents: inside the block that reads selectedLayer,
calls getLayerDefinition(selectedLayer.type) and assigns to definition, delete
the line console.log(definition) so the derived value no longer logs on every
re-render; keep the rest of the logic returning
Object.entries(definition.customPropertyComponents ?? {}).
- Line 51: Remove the unused import of InputPin from this module: delete the
line importing InputPin (the import statement referencing InputPin) since
InputPin is not referenced here (it's consumed internally by InputWrapper and
InputsWrapper); ensure there are no remaining references to InputPin in this
file after removal and run lint to confirm the ESLint warning is resolved.
- Around line 384-404: Rename the snippet definition basicProperyfield to
basicPropertyField (fix the missing "t" and use camelCase with capital F) and
update all call sites that invoke the snippet (e.g., any usages like
basicProperyfield({ ... })) to use basicPropertyField({ ... }) so definition and
references match; ensure you only rename the snippet identifier and keep the
parameter shape (label, name, labelExtra, content) unchanged.
- Around line 501-508: The InputWrapper is passing bare metadata.name (e.g.,
"content") to addKeyframe which misaligns with updateLayerProps and hasKeyframes
that expect properties prefixed with "props."; change the property prop when
rendering InputWrapper so it always supplies the normalized property key: e.g.,
compute propName = metadata.name.startsWith('props.') ? metadata.name :
`props.${metadata.name}` and pass property={propName} to InputWrapper (this
keeps addKeyframe, updateLayerProps and hasKeyframes consistent while leaving
propertyValueMap/opacity handling intact).
In `@src/lib/layers/components/VideoLayer.svelte`:
- Around line 76-89: The VideoLayer component destructures fileName from props
but fileName is not defined in the Zod schema that produces the Props type,
causing a silent reliance on enhancedProps from canvas.svelte; either add
fileName to the layer schema (matching how ImageLayer defines it) so Props
includes fileName, or remove the fileName destructuring from the VideoLayer
props if it is unused—update the Zod schema that builds Props (or the schema
used by VideoLayer) or remove references to fileName in VideoLayer.svelte and
related code paths (e.g., enhancedProps/canvas.svelte) to keep types consistent.
In `@src/lib/layers/properties/GenerateCaption.svelte`:
- Around line 40-44: The code uses layer.props inside the async continuation
which can overwrite concurrent updates; before making the fetch (or before the
await), capture a snapshot of the props (e.g., const prevProps = layer.props)
or, alternatively, after the fetch read the current layer props from the store
(via projectStore.getLayer or similar) and then call
projectStore.updateLayer(layer.id, { props: { ...prevPropsOrLatestProps,
captionText: data.captions, showCaptions: true } }) so you merge against a
stable props reference instead of the stale layer.props captured after the
await.
🧹 Nitpick comments (5)
src/lib/layers/properties/GenerateCaption.svelte (1)
1-5: Import order: external packages should come first.Per coding guidelines, imports should be ordered: External packages → SvelteKit → Internal modules → Relative imports.
@lucide/svelteis an external package but is listed after the$libimports.Suggested reorder
<script lang="ts"> + import { Sparkles } from '@lucide/svelte'; import Button from '$lib/components/ui/button/button.svelte'; import type { Layer } from '$lib/schemas/animation'; import { projectStore } from '$lib/stores/project.svelte'; - import { Sparkles } from '@lucide/svelte';As per coding guidelines,
**/*.{ts,js,svelte}: "Organize imports in order: External packages → SvelteKit → Internal modules → Relative imports".src/lib/components/editor/panels/properties-group.svelte (1)
1-15: Filename should use PascalCase for Svelte components.This file is named
properties-group.sveltebut should bePropertiesGroup.svelteper the project's naming convention. As per coding guidelines: "Use PascalCase for Svelte component filenames."src/lib/layers/base.ts (1)
147-153: Remove debug comment on Line 151.The comment
// THis is ok, dont change zodType.meta()?.descriptionlooks like a personal development note (with a typo). Clean it up before merging.🧹 Remove or reword the comment
const propertiesMeta: PropertyMetadata = { name: key, type: 'string', interpolationType: 'discrete', // Default to discrete (no interpolation), - // THis is ok, dont change zodType.meta()?.description + // .meta()?.description reads from the schema-level describe(), not the field registry description: zodType.meta()?.description };src/lib/components/editor/panels/input-pin.svelte (1)
1-27: Svelte component filename should be PascalCase.Per coding guidelines,
*.sveltefiles should use PascalCase. This file should be renamed frominput-pin.sveltetoInputPin.svelte. The same applies toinputs-wrapper.svelte→InputsWrapper.svelteandinput-wrapper.svelte→InputWrapper.svelte(pre-existing). As per coding guidelines, "Use PascalCase for Svelte component filenames."src/lib/components/editor/panels/properties-panel.svelte (1)
647-688:hasKeyframescan beundefinedwhenselectedLayeris null.
selectedLayer?.keyframes.some(...)returnsboolean | undefined, but theInputsWrapperfield type expectshasKeyframes: boolean. While unlikely at runtime (the block is gated by{#ifselectedLayer}), the non-null assertion or fallback would satisfy strict mode.♻️ Add fallback
- hasKeyframes: selectedLayer?.keyframes.some((k) => k.property === f.prop) + hasKeyframes: selectedLayer?.keyframes.some((k) => k.property === f.prop) ?? false
| {#each stops as stop, index (index)} | ||
| <div class="flex items-center gap-1"> | ||
| <Input | ||
| type="color" | ||
| value={stop.color} | ||
| oninput={(e) => updateColorStop(index, 'color', e.currentTarget.value)} | ||
| class="h-7 w-9 cursor-pointer p-0.5" | ||
| /> | ||
| <Input | ||
| type="number" | ||
| min="0" | ||
| max="100" | ||
| value={stop.position} | ||
| oninput={(e) => | ||
| updateColorStop(index, 'position', parseInt(e.currentTarget.value) || 0)} | ||
| class="h-7 w-14 text-xs" | ||
| /> | ||
| <span class="text-xs text-muted-foreground">%</span> | ||
| {#if stops.length > 2} | ||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| class="h-6 w-6 p-0" | ||
| onclick={() => removeColorStop(index)} | ||
| > | ||
| <Trash2 class="size-3" /> | ||
| </Button> | ||
| {/if} | ||
| </div> | ||
| {/each} |
There was a problem hiding this comment.
Using index as the {#each} key for color stops can cause UI glitches.
When a stop is removed (via removeColorStop), indices shift and Svelte may reuse DOM nodes incorrectly, leading to stale color/position values in the inputs. Use a stable identifier instead.
♻️ Proposed fix — use a composite key
- {`#each` stops as stop, index (index)}
+ {`#each` stops as stop, index (`${index}-${stop.color}-${stop.position}`)}Alternatively, assign a unique id to each stop when creating them for a fully stable key.
📝 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.
| {#each stops as stop, index (index)} | |
| <div class="flex items-center gap-1"> | |
| <Input | |
| type="color" | |
| value={stop.color} | |
| oninput={(e) => updateColorStop(index, 'color', e.currentTarget.value)} | |
| class="h-7 w-9 cursor-pointer p-0.5" | |
| /> | |
| <Input | |
| type="number" | |
| min="0" | |
| max="100" | |
| value={stop.position} | |
| oninput={(e) => | |
| updateColorStop(index, 'position', parseInt(e.currentTarget.value) || 0)} | |
| class="h-7 w-14 text-xs" | |
| /> | |
| <span class="text-xs text-muted-foreground">%</span> | |
| {#if stops.length > 2} | |
| <Button | |
| variant="ghost" | |
| size="sm" | |
| class="h-6 w-6 p-0" | |
| onclick={() => removeColorStop(index)} | |
| > | |
| <Trash2 class="size-3" /> | |
| </Button> | |
| {/if} | |
| </div> | |
| {/each} | |
| {`#each` stops as stop, index (`${index}-${stop.color}-${stop.position}`)} | |
| <div class="flex items-center gap-1"> | |
| <Input | |
| type="color" | |
| value={stop.color} | |
| oninput={(e) => updateColorStop(index, 'color', e.currentTarget.value)} | |
| class="h-7 w-9 cursor-pointer p-0.5" | |
| /> | |
| <Input | |
| type="number" | |
| min="0" | |
| max="100" | |
| value={stop.position} | |
| oninput={(e) => | |
| updateColorStop(index, 'position', parseInt(e.currentTarget.value) || 0)} | |
| class="h-7 w-14 text-xs" | |
| /> | |
| <span class="text-xs text-muted-foreground">%</span> | |
| {`#if` stops.length > 2} | |
| <Button | |
| variant="ghost" | |
| size="sm" | |
| class="h-6 w-6 p-0" | |
| onclick={() => removeColorStop(index)} | |
| > | |
| <Trash2 class="size-3" /> | |
| </Button> | |
| {/if} | |
| </div> | |
| {/each} |
🤖 Prompt for AI Agents
In `@src/lib/components/editor/panels/background-picker.svelte` around lines 343 -
372, The each-block uses the numeric index as the key which causes DOM reuse
bugs when a stop is removed; change the key from (index) to a stable unique
identifier like stop.id (or a composite stable key such as
`${stop.color}-${stop.position}`) in the {`#each` stops as stop, index ( ... )}
and ensure new color stops are created with a unique id (update the code paths
that create/add stops so they assign an id); keep the existing handlers
(updateColorStop, removeColorStop) unchanged but reference stops by id in any
logic that expects stable identity.
| property: string; | ||
| active: boolean; | ||
| label: string; | ||
| addKeyframe: (property: AnimatableProperty) => void; | ||
| } = $props(); | ||
| </script> | ||
|
|
||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| class="h-5 w-5 p-0" | ||
| onclick={() => addKeyframe(property)} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for AnimatableProperty definition
rg -n "AnimatableProperty" --type=ts --type=svelte -B2 -A2 | head -50Repository: epavanello/devmotion
Length of output: 94
🏁 Script executed:
#!/bin/bash
# Check the actual input-pin.svelte file around lines 12-23
cat -n src/lib/components/editor/panels/input-pin.svelte | sed -n '1,30p'Repository: epavanello/devmotion
Length of output: 877
🏁 Script executed:
#!/bin/bash
# Look for where AnimatableProperty is imported or defined
rg -n "type AnimatableProperty|interface AnimatableProperty|export.*AnimatableProperty" --type=ts --type=svelteRepository: epavanello/devmotion
Length of output: 94
🏁 Script executed:
#!/bin/bash
# Read the animation schema file to see AnimatableProperty definition
cat -n src/lib/schemas/animation.ts | head -100Repository: epavanello/devmotion
Length of output: 3043
🏁 Script executed:
#!/bin/bash
# Search for AnimatableProperty in any file
find . -type f \( -name "*.ts" -o -name "*.js" \) ! -path "*/node_modules/*" -exec grep -l "AnimatableProperty" {} \;Repository: epavanello/devmotion
Length of output: 183
🏁 Script executed:
#!/bin/bash
# Check the types/animation.ts file
cat -n src/lib/types/animation.ts | head -80Repository: epavanello/devmotion
Length of output: 1375
🏁 Script executed:
#!/bin/bash
# Search for where addKeyframe is defined
grep -rn "addKeyframe" --include="*.ts" --include="*.svelte" src/Repository: epavanello/devmotion
Length of output: 2406
property should be typed as AnimatableProperty for type safety.
Line 23 passes property to addKeyframe which expects AnimatableProperty (a constrained string type from the schema), but the prop is currently typed as the broader string type. This bypasses compile-time type checking. Change the prop type to match the callback's expectation.
♻️ Fix type annotation
const {
property,
active,
label,
addKeyframe
}: {
- property: string;
+ property: AnimatableProperty;
active: boolean;
label: string;
addKeyframe: (property: AnimatableProperty) => void;
} = $props();🤖 Prompt for AI Agents
In `@src/lib/components/editor/panels/input-pin.svelte` around lines 12 - 23, The
prop "property" is currently typed as string but is passed into addKeyframe
which expects AnimatableProperty; update the prop declaration so property:
AnimatableProperty instead of string (ensure the AnimatableProperty type is
imported/available in this component) so the signature of property matches
addKeyframe(property) and preserves compile-time type safety.
| import Label from '$lib/components/ui/label/label.svelte'; | ||
| import type { AnimatableProperty } from '$lib/schemas/animation'; | ||
| import { projectStore } from '$lib/stores/project.svelte'; |
There was a problem hiding this comment.
Remove unused imports Label and projectStore.
Both are flagged by ESLint as unused. They appear to be leftover from a refactor.
🧹 Proposed fix
<script lang="ts">
- import Label from '$lib/components/ui/label/label.svelte';
import type { AnimatableProperty } from '$lib/schemas/animation';
- import { projectStore } from '$lib/stores/project.svelte';
import type { Snippet } from 'svelte';
import InputPin from './input-pin.svelte';
import * as ButtonGroup from '$lib/components/ui/button-group';📝 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.
| import Label from '$lib/components/ui/label/label.svelte'; | |
| import type { AnimatableProperty } from '$lib/schemas/animation'; | |
| import { projectStore } from '$lib/stores/project.svelte'; | |
| import type { AnimatableProperty } from '$lib/schemas/animation'; |
🧰 Tools
🪛 ESLint
[error] 2-2: 'Label' is defined but never used. Allowed unused vars must match /^_/u.
(@typescript-eslint/no-unused-vars)
[error] 4-4: 'projectStore' is defined but never used. Allowed unused vars must match /^_/u.
(@typescript-eslint/no-unused-vars)
🤖 Prompt for AI Agents
In `@src/lib/components/editor/panels/inputs-wrapper.svelte` around lines 2 - 4,
Remove the unused imports Label and projectStore from the top of the file:
locate the import statements that bring in Label (import Label from
'$lib/components/ui/label/label.svelte') and projectStore (import { projectStore
} from '$lib/stores/project.svelte') and delete them; ensure no other code in
this module references the symbols Label or projectStore (rename or keep if used
elsewhere), then run lint to confirm ESLint no longer reports these unused
imports.
| <Popover.Close> | ||
| {#snippet child({ props })} | ||
| <Button | ||
| variant="destructive" | ||
| size="sm" | ||
| {...props} | ||
| onclick={() => deleteLayer(layer.id)} | ||
| > | ||
| Delete | ||
| </Button> | ||
| {/snippet} | ||
| </Popover.Close> |
There was a problem hiding this comment.
onclick after {...props} overrides Popover.Close's close handler — deletePopoverOpenLayerId stays stale.
In Svelte 5, the last attribute wins. Since onclick (line 189) comes after {...props} (line 188), the Popover.Close's internal close handler is replaced. The popover still disappears because deleteLayer removes the layer from the DOM, but deletePopoverOpenLayerId is never reset to null.
This is mostly harmless today (no layer will match the stale ID), but it's fragile. Either swap the order so {...props} merges after onclick, or explicitly reset the state:
Option A: reset state in the handler
<Button
variant="destructive"
size="sm"
{...props}
- onclick={() => deleteLayer(layer.id)}
+ onclick={() => {
+ deletePopoverOpenLayerId = null;
+ deleteLayer(layer.id);
+ }}
>Option B: place spread after onclick so Popover.Close handler wins too
<Button
variant="destructive"
size="sm"
- {...props}
onclick={() => deleteLayer(layer.id)}
+ {...props}
>(With this approach, Popover.Close closes the popover and you'd need to call deleteLayer elsewhere — less practical.)
Option A is the simpler fix.
🤖 Prompt for AI Agents
In `@src/lib/components/editor/panels/layers-panel.svelte` around lines 183 - 194,
The Popover.Close's internal close handler is being overridden because onclick
is placed after the spread {...props} in the snippet inside Popover.Close;
update the delete button's click handler to reset the popover state instead of
relying on the Popover's handler — inside the handler used by the Button (the
function passed to onclick that calls deleteLayer(layer.id)), also set
deletePopoverOpenLayerId = null (or call the function that resets it)
before/after deleteLayer to ensure the stale ID is cleared; alternatively you
can move the onclick so it appears before {...props} but the preferred fix is to
explicitly reset deletePopoverOpenLayerId in the deleteLayer click handler
referenced in the snippet child({ props }) / Button.
| {#snippet basicProperyfield({ | ||
| label, | ||
| name, | ||
| labelExtra, | ||
| content | ||
| }: { | ||
| label: string; | ||
| name?: string; | ||
| labelExtra?: Snippet; | ||
| content: Snippet; | ||
| })} | ||
| <div class="space-y-2"> | ||
| <div class="flex items-center justify-between"> | ||
| <Label for={metadata.name} class="text-xs">{metadata.description || metadata.name}</Label> | ||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| class="h-5 w-5 p-0" | ||
| onclick={() => addKeyframe(property)} | ||
| title={isInterpolatable | ||
| ? `Add keyframe for ${metadata.description || metadata.name}` | ||
| : `Add keyframe for ${metadata.description || metadata.name} (discrete - jumps to value)`} | ||
| > | ||
| <Pin class="size-3" fill={hasKeyframes ? 'currentColor' : 'none'} /> | ||
| </Button> | ||
| </div> | ||
| <Label for={name} class="text-xs" | ||
| >{label} | ||
| {#if labelExtra} | ||
| {@render labelExtra()} | ||
| {/if} | ||
| </Label> | ||
| {@render content()} | ||
| </div> | ||
| {/snippet} |
There was a problem hiding this comment.
Typo in snippet name: basicProperyfield → basicPropertyField.
The snippet name has a missing t in "Property" and inconsistent casing (field should be Field for camelCase consistency).
🧹 Proposed fix
-{`#snippet` basicProperyfield({
+{`#snippet` basicPropertyField({Also update the call sites (e.g., Line 845):
- {`@render` basicProperyfield({
+ {`@render` basicPropertyField({🤖 Prompt for AI Agents
In `@src/lib/components/editor/panels/properties-panel.svelte` around lines 384 -
404, Rename the snippet definition basicProperyfield to basicPropertyField (fix
the missing "t" and use camelCase with capital F) and update all call sites that
invoke the snippet (e.g., any usages like basicProperyfield({ ... })) to use
basicPropertyField({ ... }) so definition and references match; ensure you only
rename the snippet identifier and keep the parameter shape (label, name,
labelExtra, content) unchanged.
| <InputWrapper | ||
| id={metadata.name} | ||
| label={metadata.description || metadata.name} | ||
| property={metadata.name} | ||
| {addKeyframe} | ||
| > | ||
| {@render content()} | ||
| </InputWrapper> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate the properties-panel.svelte file and examine the context around lines 501-508
find . -name "properties-panel.svelte" -type fRepository: epavanello/devmotion
Length of output: 122
🏁 Script executed:
# Search for addKeyframe function definition and how it handles the property parameter
rg -n "addKeyframe" --type=ts --type=svelte -B5 -A10 | head -100Repository: epavanello/devmotion
Length of output: 94
🏁 Script executed:
# Look for the property.startsWith('props.') check mentioned at line 333
rg -n "property\.startsWith\('props\.\)" --type=ts --type=svelte -B3 -A3Repository: epavanello/devmotion
Length of output: 94
🏁 Script executed:
# Search for how keyframes are stored and how hasKeyframes checks them
rg -n "hasKeyframes" --type=ts --type=svelte -B2 -A5 | head -80Repository: epavanello/devmotion
Length of output: 94
🏁 Script executed:
# Read the properties-panel.svelte file around lines 501-508
cat -n ./src/lib/components/editor/panels/properties-panel.svelte | sed -n '495,515p'Repository: epavanello/devmotion
Length of output: 729
🏁 Script executed:
# Search for addKeyframe in the file using grep
grep -n "addKeyframe" ./src/lib/components/editor/panels/properties-panel.svelte | head -20Repository: epavanello/devmotion
Length of output: 416
🏁 Script executed:
# Look for the props. prefix check
grep -rn "property\.startsWith.*props\." . --include="*.ts" --include="*.svelte" --include="*.js"Repository: epavanello/devmotion
Length of output: 50377
🏁 Script executed:
# Search for hasKeyframes implementation
grep -rn "hasKeyframes" . --include="*.ts" --include="*.svelte" --include="*.js" -B2 -A5 | head -80Repository: epavanello/devmotion
Length of output: 6148
🏁 Script executed:
# Find InputWrapper and InputPin components to trace property flow
find . -name "InputWrapper*" -o -name "InputPin*" | grep -E "\.(svelte|ts)$"Repository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
# Look at the addKeyframe function around line 313
sed -n '310,350p' ./src/lib/components/editor/panels/properties-panel.svelteRepository: epavanello/devmotion
Length of output: 1336
🏁 Script executed:
# Check what metadata.name contains by looking at where it's assigned
grep -n "metadata\.name" ./src/lib/components/editor/panels/properties-panel.svelte | head -20Repository: epavanello/devmotion
Length of output: 1045
🏁 Script executed:
# Check how other properties are passed to InputWrapper (e.g., line 817)
sed -n '815,825p' ./src/lib/components/editor/panels/properties-panel.svelteRepository: epavanello/devmotion
Length of output: 480
🏁 Script executed:
# Look for where keyframes are created/stored and what property format they use
grep -n "keyframes\|AnimatableProperty" ./src/lib/components/editor/panels/properties-panel.svelte | head -40Repository: epavanello/devmotion
Length of output: 2174
🏁 Script executed:
# Find updateAnimatableValue function definition
grep -n "function updateAnimatableValue\|const updateAnimatableValue" ./src/lib/components/editor/panels/properties-panel.svelteRepository: epavanello/devmotion
Length of output: 101
🏁 Script executed:
# Get more context around line 290-310 to see the updateAnimatableValue call
sed -n '280,310p' ./src/lib/components/editor/panels/properties-panel.svelteRepository: epavanello/devmotion
Length of output: 957
🏁 Script executed:
# Search for where keyframes are actually created/pushed
grep -n "keyframes.push\|keyframes =" ./src/lib/components/editor/panels/properties-panel.svelteRepository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
# Get the full updateAnimatableValue function implementation
sed -n '255,280p' ./src/lib/components/editor/panels/properties-panel.svelteRepository: epavanello/devmotion
Length of output: 896
🏁 Script executed:
# Look for projectStore.addKeyframe or similar calls that actually create keyframes
grep -rn "addKeyframe\|Keyframe\|keyframes.push" ./src/lib --include="*.ts" --include="*.svelte" | grep -v node_modules | head -40Repository: epavanello/devmotion
Length of output: 4665
Pass props. prefix when creating custom property keyframes — mismatch between InputWrapper property lookup and addKeyframe handler.
When metadata.name (e.g., "content") is passed to InputWrapper, the addKeyframe function receives the bare name and:
-
Fails to retrieve the property value: Line 333 checks
property.startsWith('props.'). Since"content"doesn't match, it skips theselectedLayer.propslookup and defaults tocurrentValue = 0. -
Creates inconsistently-keyed keyframes: Calling
addKeyframe("content")stores a keyframe withproperty: "content", butupdateLayerProps(line 304) stores keyframes asproperty: "props.content". -
Breaks hasKeyframes detection: The check at line 30 (
k.property === property) looks fork.property === "content"but findsk.property === "props.content"(if modified via updateLayerProps), so the keyframe indicator never activates.
In contrast, the built-in "opacity" property (line 817) works correctly because it's in propertyValueMap.
🐛 Proposed fix
<InputWrapper
id={metadata.name}
label={metadata.description || metadata.name}
- property={metadata.name}
+ property={`props.${metadata.name}`}
{addKeyframe}
>
{`@render` content()}
</InputWrapper>📝 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.
| <InputWrapper | |
| id={metadata.name} | |
| label={metadata.description || metadata.name} | |
| property={metadata.name} | |
| {addKeyframe} | |
| > | |
| {@render content()} | |
| </InputWrapper> | |
| <InputWrapper | |
| id={metadata.name} | |
| label={metadata.description || metadata.name} | |
| property={`props.${metadata.name}`} | |
| {addKeyframe} | |
| > | |
| {`@render` content()} | |
| </InputWrapper> |
🤖 Prompt for AI Agents
In `@src/lib/components/editor/panels/properties-panel.svelte` around lines 501 -
508, The InputWrapper is passing bare metadata.name (e.g., "content") to
addKeyframe which misaligns with updateLayerProps and hasKeyframes that expect
properties prefixed with "props."; change the property prop when rendering
InputWrapper so it always supplies the normalized property key: e.g., compute
propName = metadata.name.startsWith('props.') ? metadata.name :
`props.${metadata.name}` and pass property={propName} to InputWrapper (this
keeps addKeyframe, updateLayerProps and hasKeyframes consistent while leaving
propertyValueMap/opacity handling intact).
| if (data.success && data.captions) { | ||
| projectStore.updateLayer(layer.id, { | ||
| props: { ...layer.props, captionText: data.captions, showCaptions: true } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Stale layer.props reference in async continuation.
After the await fetch(...) resolves, layer.props on line 42 reflects whatever the prop value is at that point. If the user (or another action) modifies this layer's props while the request is in flight, the spread { ...layer.props, ... } could silently overwrite those intermediate changes.
A safer approach is to capture layer.props before the await, or read the latest state from the store:
Option: snapshot props before the async call
isGeneratingCaptions = true;
captionError = '';
+ const currentProps = layer.props;
try {
const res = await fetch('/api/captions', {
...
});
...
const data = await res.json();
if (data.success && data.captions) {
projectStore.updateLayer(layer.id, {
- props: { ...layer.props, captionText: data.captions, showCaptions: true }
+ props: { ...currentProps, captionText: data.captions, showCaptions: true }
});
}🤖 Prompt for AI Agents
In `@src/lib/layers/properties/GenerateCaption.svelte` around lines 40 - 44, The
code uses layer.props inside the async continuation which can overwrite
concurrent updates; before making the fetch (or before the await), capture a
snapshot of the props (e.g., const prevProps = layer.props) or, alternatively,
after the fetch read the current layer props from the store (via
projectStore.getLayer or similar) and then call
projectStore.updateLayer(layer.id, { props: { ...prevPropsOrLatestProps,
captionText: data.captions, showCaptions: true } }) so you merge against a
stable props reference instead of the stale layer.props captured after the
await.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/components/ai/ai-chat.svelte (1)
68-75:⚠️ Potential issue | 🟠 MajorForward timing fields to
executeCreateLayerto prevent silent data loss.The
CreateLayerInputschema includes optional timing fields (enterTime,exitTime,contentDuration,contentOffset) that are properly handled bymutateCreateLayer, but they're not being passed through in the call at lines 69–75. If the AI model provides these values, they'll be silently dropped.Proposed fix
const input = toolCall.input as CreateLayerInput; result = executeCreateLayer({ type: layerType, name: input.name, position: input.position, props: input.props || {}, - animation: input.animation + animation: input.animation, + enterTime: input.enterTime, + exitTime: input.exitTime, + contentDuration: input.contentDuration, + contentOffset: input.contentOffset });
🤖 Fix all issues with AI agents
In `@src/lib/components/editor/panels/input-propery.svelte`:
- Line 1: The file name is misspelled; rename the component file from
input-propery.svelte to input-property.svelte and update every import/usage to
match the new name (search for "input-propery.svelte" and replace). Ensure any
exports, tests, routes, or index barrels that reference the old filename (e.g.,
import statements or component registrations) are updated to import
"./input-property.svelte" and run the build/tests to confirm no remaining
references.
- Around line 46-53: The onRemove handler resets contentDuration and
contentOffset but fails to clear exitTime, causing stale timing; update the
onRemove block (the arrow function passed to onRemove) to also reset exitTime
via projectStore.updateLayer(layer.id, { contentDuration: 0, contentOffset: 0,
exitTime: undefined }) or an appropriate null/zero value consistent with how
onUpload sets it, so the layer no longer retains the previous media-derived
exitTime after removal; ensure the update mirrors what's done in onUpload for
exitTime handling.
- Around line 82-92: The select handler currently uses e.currentTarget.value
which is always a string, so when metadata.options contains numeric values the
prop becomes a string; update the onchange handler used with metadata (and the
onUpdateProp call) to look up the selected option from metadata.options
(matching by stringified value) and then coerce the value to the original option
type (e.g., convert to number when the matched option.value is a number) before
calling onUpdateProp(metadata.name, coercedValue); reference metadata.options,
metadata.name, onUpdateProp and the select value handling to implement this
change.
In `@src/lib/components/editor/panels/properties-panel.svelte`:
- Around line 467-489: The ScrubInput max can become negative when
contentDuration < 0.1; change the max prop to use a clamped value (e.g. const
clampedMax = hasDuration ? Math.max(0, contentDuration - 0.1) : undefined and
then max={clampedMax}) and update the onchange logic to use that same clampedMax
when computing clamped = hasDuration ? Math.min(v, clampedMax) : Math.max(0, v)
and when computing maxVisibleDuration so projectStore.updateLayer and
projectStore.setLayerExitTime use a non-negative max; adjust references to
contentDuration - 0.1 accordingly in the ScrubInput block (ScrubInput,
contentOffset, contentDuration, hasDuration, onchange, projectStore.updateLayer,
projectStore.setLayerExitTime, selectedLayer.exitTime).
- Line 52: The import uses a misspelled component name and filename
("InputPropery" from "input-propery.svelte"); rename the component and file to
"InputProperty" and "input-property.svelte" and update every reference
accordingly (the import in the properties-panel component and any other
imports/exports that reference InputPropery or input-propery.svelte), ensuring
the Svelte component's default export/component tag matches the corrected name
"InputProperty".
In `@src/lib/components/editor/project-switcher.svelte`:
- Around line 59-73: The saveRename function currently swallows errors and only
console.logs them, leaving the user with no feedback; update saveRename (and the
component state) so on catch it surfaces a user-visible error (e.g., set a local
renameError string and/or call your toast/notification helper like showToast)
using the caught error message, keep editingProjectId/editingName unchanged so
the editor stays open for retry, and still set renaming = false in finally;
ensure renameProject and getUserProjects().refresh stay in the try block so
refresh only happens on success.
- Around line 42-45: The guard in handleOpenProject only blocks navigation to
the same project id; change behavior to either block all navigation while an
edit is active or explicitly discard the edit before navigating. Locate function
handleOpenProject and the editingProjectId state; either make the initial check
if (editingProjectId) return to prevent navigating away while any project is
being renamed, or if you intend to allow navigation, clear/reset
editingProjectId first (e.g., set editingProjectId = null or call the reset
method) before calling goto(resolve(`/p/${id}`)) so the in-progress edit is not
silently lost.
🧹 Nitpick comments (5)
src/lib/components/editor/panels/scrub-xyz.svelte (1)
2-3: Import order: external packages should precede internal modules.Per your coding guidelines, imports should be ordered: External packages → SvelteKit → Internal modules → Relative imports.
@lucide/svelte(external) should come before$lib/utils(internal).Proposed fix
- import { cn } from '$lib/utils'; - import { Move3d } from '@lucide/svelte'; + import { Move3d } from '@lucide/svelte'; + import { cn } from '$lib/utils';src/lib/components/editor/panels/input-propery.svelte (1)
61-61: Use??instead of||for thestepfallback.
metadata.step || 1treats0as falsy and would default to1. If a step of0is intentional (unlikely but possible) or if the intent is strictly to guard againstundefined/null, nullish coalescing is safer.- step={metadata.step || 1} + step={metadata.step ?? 1}src/lib/components/editor/project-switcher.svelte (1)
4-21: Import order doesn't follow the guideline.Per the coding guideline ("External packages → SvelteKit → Internal modules → Relative imports"),
$app/navigationand$app/paths(SvelteKit) should come before$lib/*(internal) imports. This was partially pre-existing but worth cleaning up now that the import block is being modified.src/lib/components/editor/panels/properties-panel.svelte (2)
242-252: Duplicate JSDoc blocks forupdateAnimatableValue.Two consecutive doc comments describe the same function. Keep only the more complete one (lines 247-252).
Proposed fix
- /** - * Helper to update an animatable property - handles keyframe logic - * If the property has keyframes, updates/creates keyframe at current time - * Otherwise calls updateBase to update the base value - */ /** * Helper to update an animatable property - handles keyframe logic * If the property has keyframes, updates/creates keyframe at current time * If updateBase is provided and no keyframes exist, calls updateBase * If updateBase is not provided, always creates/updates keyframe */
43-48: Stale empty lines from removed imports.Lines 43 and 46–48 are consecutive blank lines left behind after import removals. Consider collapsing to a single blank separator.
| @@ -0,0 +1,114 @@ | |||
| <script lang="ts"> | |||
There was a problem hiding this comment.
Filename typo: input-propery.svelte → input-property.svelte
The file is missing a "t" — propery should be property. This will cause confusion for anyone importing or referencing this component.
🤖 Prompt for AI Agents
In `@src/lib/components/editor/panels/input-propery.svelte` at line 1, The file
name is misspelled; rename the component file from input-propery.svelte to
input-property.svelte and update every import/usage to match the new name
(search for "input-propery.svelte" and replace). Ensure any exports, tests,
routes, or index barrels that reference the old filename (e.g., import
statements or component registrations) are updated to import
"./input-property.svelte" and run the build/tests to confirm no remaining
references.
| onRemove={() => { | ||
| onUpdateProp(metadata.name, ''); | ||
| onUpdateProp('fileKey', ''); | ||
| projectStore.updateLayer(layer.id, { | ||
| contentDuration: 0, | ||
| contentOffset: 0 | ||
| }); | ||
| }} |
There was a problem hiding this comment.
onRemove resets contentDuration and contentOffset but not exitTime.
The onUpload handler auto-sets exitTime when it's unset (Lines 39–43), but onRemove never resets it. After removing an uploaded file, the layer retains a stale exitTime derived from the old media duration. This can cause the layer to appear with incorrect timing on the timeline.
Proposed fix
onRemove={() => {
onUpdateProp(metadata.name, '');
onUpdateProp('fileKey', '');
projectStore.updateLayer(layer.id, {
contentDuration: 0,
- contentOffset: 0
+ contentOffset: 0,
+ exitTime: undefined
});
}}📝 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.
| onRemove={() => { | |
| onUpdateProp(metadata.name, ''); | |
| onUpdateProp('fileKey', ''); | |
| projectStore.updateLayer(layer.id, { | |
| contentDuration: 0, | |
| contentOffset: 0 | |
| }); | |
| }} | |
| onRemove={() => { | |
| onUpdateProp(metadata.name, ''); | |
| onUpdateProp('fileKey', ''); | |
| projectStore.updateLayer(layer.id, { | |
| contentDuration: 0, | |
| contentOffset: 0, | |
| exitTime: undefined | |
| }); | |
| }} |
🤖 Prompt for AI Agents
In `@src/lib/components/editor/panels/input-propery.svelte` around lines 46 - 53,
The onRemove handler resets contentDuration and contentOffset but fails to clear
exitTime, causing stale timing; update the onRemove block (the arrow function
passed to onRemove) to also reset exitTime via
projectStore.updateLayer(layer.id, { contentDuration: 0, contentOffset: 0,
exitTime: undefined }) or an appropriate null/zero value consistent with how
onUpload sets it, so the layer no longer retains the previous media-derived
exitTime after removal; ensure the update mirrors what's done in onUpload for
exitTime handling.
| {:else if metadata.type === 'select' && metadata.options} | ||
| <select | ||
| id={metadata.name} | ||
| value={typeof value === 'string' || typeof value === 'number' ? value : ''} | ||
| onchange={(e) => onUpdateProp(metadata.name, e.currentTarget.value)} | ||
| class="flex h-9 w-full rounded-md border border-input bg-transparent px-3 py-1 text-sm shadow-sm transition-colors focus-visible:ring-1 focus-visible:ring-ring focus-visible:outline-none" | ||
| > | ||
| {#each metadata.options as option (option.value)} | ||
| <option value={option.value}>{option.label}</option> | ||
| {/each} | ||
| </select> |
There was a problem hiding this comment.
Select onchange always passes a string, even for numeric option values.
e.currentTarget.value is always a string. If metadata.options contains numeric values, the prop will be updated with a string instead of the expected number. Consider coercing based on the original option type.
Proposed fix
onchange={(e) => {
- onUpdateProp(metadata.name, e.currentTarget.value)
+ const raw = e.currentTarget.value;
+ const match = metadata.options?.find((o) => String(o.value) === raw);
+ onUpdateProp(metadata.name, match ? match.value : raw);
}}🤖 Prompt for AI Agents
In `@src/lib/components/editor/panels/input-propery.svelte` around lines 82 - 92,
The select handler currently uses e.currentTarget.value which is always a
string, so when metadata.options contains numeric values the prop becomes a
string; update the onchange handler used with metadata (and the onUpdateProp
call) to look up the selected option from metadata.options (matching by
stringified value) and then coerce the value to the original option type (e.g.,
convert to number when the matched option.value is a number) before calling
onUpdateProp(metadata.name, coercedValue); reference metadata.options,
metadata.name, onUpdateProp and the select value handling to implement this
change.
| import type { Snippet } from 'svelte'; | ||
| import PropertiesGroup from './properties-group.svelte'; | ||
| import InputsWrapper from './inputs-wrapper.svelte'; | ||
| import InputPropery from './input-propery.svelte'; |
There was a problem hiding this comment.
Typo in component import: InputPropery → InputProperty.
The component name is missing a t. This affects both the import and likely the filename itself.
#!/bin/bash
# Check the actual filename of the component
fd "input-proper" --type f🤖 Prompt for AI Agents
In `@src/lib/components/editor/panels/properties-panel.svelte` at line 52, The
import uses a misspelled component name and filename ("InputPropery" from
"input-propery.svelte"); rename the component and file to "InputProperty" and
"input-property.svelte" and update every reference accordingly (the import in
the properties-panel component and any other imports/exports that reference
InputPropery or input-propery.svelte), ensuring the Svelte component's default
export/component tag matches the corrected name "InputProperty".
| <ScrubInput | ||
| id="content-offset" | ||
| value={contentOffset} | ||
| min={0} | ||
| max={hasDuration ? contentDuration - 0.1 : undefined} | ||
| step={0.1} | ||
| onchange={(v) => { | ||
| const clamped = hasDuration | ||
| ? Math.min(v, contentDuration - 0.1) | ||
| : Math.max(0, v); | ||
| projectStore.updateLayer(selectedLayer.id, { contentOffset: clamped }); | ||
|
|
||
| // Auto-adjust exitTime if it would exceed available content | ||
| if (hasDuration && selectedLayer.exitTime !== undefined) { | ||
| const enterTime = selectedLayer.enterTime ?? 0; | ||
| const maxVisibleDuration = contentDuration - clamped; | ||
| const maxExitTime = enterTime + maxVisibleDuration; | ||
| if (selectedLayer.exitTime > maxExitTime) { | ||
| projectStore.setLayerExitTime(selectedLayer.id, maxExitTime); | ||
| } | ||
| } | ||
| }} | ||
| /> |
There was a problem hiding this comment.
Content offset max edge case when contentDuration < 0.1.
Line 471 sets max to contentDuration - 0.1. If the source media duration is less than 0.1s (or if contentDuration was somehow set to a small positive value), max becomes negative while min is 0, which could produce an invalid range for ScrubInput.
Consider clamping: max={hasDuration ? Math.max(0, contentDuration - 0.1) : undefined}.
Proposed fix
<ScrubInput
id="content-offset"
value={contentOffset}
min={0}
- max={hasDuration ? contentDuration - 0.1 : undefined}
+ max={hasDuration ? Math.max(0, contentDuration - 0.1) : undefined}
step={0.1}📝 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.
| <ScrubInput | |
| id="content-offset" | |
| value={contentOffset} | |
| min={0} | |
| max={hasDuration ? contentDuration - 0.1 : undefined} | |
| step={0.1} | |
| onchange={(v) => { | |
| const clamped = hasDuration | |
| ? Math.min(v, contentDuration - 0.1) | |
| : Math.max(0, v); | |
| projectStore.updateLayer(selectedLayer.id, { contentOffset: clamped }); | |
| // Auto-adjust exitTime if it would exceed available content | |
| if (hasDuration && selectedLayer.exitTime !== undefined) { | |
| const enterTime = selectedLayer.enterTime ?? 0; | |
| const maxVisibleDuration = contentDuration - clamped; | |
| const maxExitTime = enterTime + maxVisibleDuration; | |
| if (selectedLayer.exitTime > maxExitTime) { | |
| projectStore.setLayerExitTime(selectedLayer.id, maxExitTime); | |
| } | |
| } | |
| }} | |
| /> | |
| <ScrubInput | |
| id="content-offset" | |
| value={contentOffset} | |
| min={0} | |
| max={hasDuration ? Math.max(0, contentDuration - 0.1) : undefined} | |
| step={0.1} | |
| onchange={(v) => { | |
| const clamped = hasDuration | |
| ? Math.min(v, contentDuration - 0.1) | |
| : Math.max(0, v); | |
| projectStore.updateLayer(selectedLayer.id, { contentOffset: clamped }); | |
| // Auto-adjust exitTime if it would exceed available content | |
| if (hasDuration && selectedLayer.exitTime !== undefined) { | |
| const enterTime = selectedLayer.enterTime ?? 0; | |
| const maxVisibleDuration = contentDuration - clamped; | |
| const maxExitTime = enterTime + maxVisibleDuration; | |
| if (selectedLayer.exitTime > maxExitTime) { | |
| projectStore.setLayerExitTime(selectedLayer.id, maxExitTime); | |
| } | |
| } | |
| }} | |
| /> |
🤖 Prompt for AI Agents
In `@src/lib/components/editor/panels/properties-panel.svelte` around lines 467 -
489, The ScrubInput max can become negative when contentDuration < 0.1; change
the max prop to use a clamped value (e.g. const clampedMax = hasDuration ?
Math.max(0, contentDuration - 0.1) : undefined and then max={clampedMax}) and
update the onchange logic to use that same clampedMax when computing clamped =
hasDuration ? Math.min(v, clampedMax) : Math.max(0, v) and when computing
maxVisibleDuration so projectStore.updateLayer and projectStore.setLayerExitTime
use a non-negative max; adjust references to contentDuration - 0.1 accordingly
in the ScrubInput block (ScrubInput, contentOffset, contentDuration,
hasDuration, onchange, projectStore.updateLayer, projectStore.setLayerExitTime,
selectedLayer.exitTime).
| function handleOpenProject(id: string) { | ||
| if (editingProjectId === id) return; // Don't navigate while editing | ||
| goto(resolve(`/p/${id}`)); | ||
| } |
There was a problem hiding this comment.
Navigation guard only blocks the project being edited, not all navigation.
editingProjectId === id only prevents clicking the same project row. Clicking a different project while one is being renamed will navigate away, silently discarding the in-progress edit. If this is intentional, consider at least resetting editingProjectId in that path. Otherwise, guard against any navigation while editing:
Suggested fix
function handleOpenProject(id: string) {
- if (editingProjectId === id) return; // Don't navigate while editing
+ if (editingProjectId) return; // Don't navigate while editing any project
goto(resolve(`/p/${id}`));
}📝 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.
| function handleOpenProject(id: string) { | |
| if (editingProjectId === id) return; // Don't navigate while editing | |
| goto(resolve(`/p/${id}`)); | |
| } | |
| function handleOpenProject(id: string) { | |
| if (editingProjectId) return; // Don't navigate while editing any project | |
| goto(resolve(`/p/${id}`)); | |
| } |
🤖 Prompt for AI Agents
In `@src/lib/components/editor/project-switcher.svelte` around lines 42 - 45, The
guard in handleOpenProject only blocks navigation to the same project id; change
behavior to either block all navigation while an edit is active or explicitly
discard the edit before navigating. Locate function handleOpenProject and the
editingProjectId state; either make the initial check if (editingProjectId)
return to prevent navigating away while any project is being renamed, or if you
intend to allow navigation, clear/reset editingProjectId first (e.g., set
editingProjectId = null or call the reset method) before calling
goto(resolve(`/p/${id}`)) so the in-progress edit is not silently lost.
| async function saveRename(e: Event) { | ||
| e.stopPropagation(); | ||
| if (!editingProjectId || !editingName.trim()) return; | ||
| renaming = true; | ||
| try { | ||
| await renameProject({ id: editingProjectId, name: editingName.trim() }); | ||
| getUserProjects().refresh(); | ||
| editingProjectId = null; | ||
| editingName = ''; | ||
| } catch (error) { | ||
| console.error('Failed to rename project:', error); | ||
| } finally { | ||
| renaming = false; | ||
| } | ||
| } |
There was a problem hiding this comment.
No user-visible feedback when rename fails.
saveRename catches the error and logs it to the console, but the user gets no indication that the rename failed — the UI simply stays in editing mode with renaming reset to false. Consider showing a toast or inline error so the user knows to retry.
🤖 Prompt for AI Agents
In `@src/lib/components/editor/project-switcher.svelte` around lines 59 - 73, The
saveRename function currently swallows errors and only console.logs them,
leaving the user with no feedback; update saveRename (and the component state)
so on catch it surfaces a user-visible error (e.g., set a local renameError
string and/or call your toast/notification helper like showToast) using the
caught error message, keep editingProjectId/editingName unchanged so the editor
stays open for retry, and still set renaming = false in finally; ensure
renameProject and getUserProjects().refresh stay in the try block so refresh
only happens on success.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/lib/components/editor/keyframe-card.svelte`:
- Around line 126-132: The Button currently spreads {...props} then sets
onclick={handleDelete}, which overrides the Popover.Close-provided click handler
and can prevent the popover from closing; update the Button to merge handlers
instead of replacing them by using Svelte event binding (on:click) and invoking
the original prop handler first (e.g., props.onclick or props.onClick) then
calling handleDelete — reference the Popover.Close block, the {`#snippet` child({
props })} usage, the Button element, the {...props} spread, and the handleDelete
function when making this change.
In `@src/lib/components/editor/timeline/timeline-keyframe.svelte`:
- Around line 147-153: The explicit onclick after the spread overrides
Popover.Close's handler; update the Button inside the snippet (child({ props }))
to compose both handlers instead of overriding: replace
onclick={handleDeleteAll} with a composed handler like onclick={(e) => {
props.onclick?.(e); handleDeleteAll(e); }} so the Popover.Close (props.onclick)
is invoked first and then handleDeleteAll; keep the {...props} spread as-is to
preserve other props.
🧹 Nitpick comments (2)
src/lib/components/editor/panels/inputs-wrapper.svelte (2)
14-27:labelsfield name is misleading — it holds a single label string.Every caller passes a single string (e.g.,
'X','Y','Z'). Consider renaming tolabelfor clarity and consistency withInputPin's ownlabelprop.
43-53: Label is rendered even when InputPin is hidden — intentional?When
field.propertyis falsy,InputPinis skipped but the<Label>on line 52 is still rendered. If these fields are only used in scenarios wherepropertyis always provided (as in the current consumers), this is fine. But if a field without a property is meant to be a pure spacer/slot, the dangling label may be unexpected.
| <Popover.Close> | ||
| {#snippet child({ props })} | ||
| <Button variant="destructive" size="sm" {...props} onclick={handleDelete}> | ||
| Delete | ||
| </Button> | ||
| {/snippet} | ||
| </Popover.Close> |
There was a problem hiding this comment.
Same onclick override issue as in timeline-keyframe — Popover.Close handler may be lost.
onclick={handleDelete} after {...props} overrides the close callback provided by Popover.Close. The popover might not dismiss if the component isn't immediately unmounted after deletion.
🐛 Proposed fix
<Popover.Close>
{`#snippet` child({ props })}
- <Button variant="destructive" size="sm" {...props} onclick={handleDelete}>
+ <Button variant="destructive" size="sm" {...props} onclick={(e) => {
+ handleDelete();
+ props.onclick?.(e);
+ }}>
Delete
</Button>
{/snippet}
</Popover.Close>📝 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.
| <Popover.Close> | |
| {#snippet child({ props })} | |
| <Button variant="destructive" size="sm" {...props} onclick={handleDelete}> | |
| Delete | |
| </Button> | |
| {/snippet} | |
| </Popover.Close> | |
| <Popover.Close> | |
| {`#snippet` child({ props })} | |
| <Button variant="destructive" size="sm" {...props} onclick={(e) => { | |
| handleDelete(); | |
| props.onclick?.(e); | |
| }}> | |
| Delete | |
| </Button> | |
| {/snippet} | |
| </Popover.Close> |
🤖 Prompt for AI Agents
In `@src/lib/components/editor/keyframe-card.svelte` around lines 126 - 132, The
Button currently spreads {...props} then sets onclick={handleDelete}, which
overrides the Popover.Close-provided click handler and can prevent the popover
from closing; update the Button to merge handlers instead of replacing them by
using Svelte event binding (on:click) and invoking the original prop handler
first (e.g., props.onclick or props.onClick) then calling handleDelete —
reference the Popover.Close block, the {`#snippet` child({ props })} usage, the
Button element, the {...props} spread, and the handleDelete function when making
this change.
| <Popover.Close> | ||
| {#snippet child({ props })} | ||
| <Button variant="destructive" size="sm" {...props} onclick={handleDeleteAll}> | ||
| Delete All | ||
| </Button> | ||
| {/snippet} | ||
| </Popover.Close> |
There was a problem hiding this comment.
onclick={handleDeleteAll} after {...props} overrides the Popover.Close handler.
In Svelte 5, the explicit onclick placed after the spread takes precedence over any onclick in props (provided by Popover.Close). The popover won't close on its own after deletion. It may still disappear because the parent unmounts the component after all keyframes are removed, but this is fragile — e.g., if only some keyframes are at this time point and others remain, the outer popover could linger.
Compose both handlers or reorder:
🐛 Proposed fix
<Popover.Close>
{`#snippet` child({ props })}
- <Button variant="destructive" size="sm" {...props} onclick={handleDeleteAll}>
+ <Button variant="destructive" size="sm" {...props} onclick={(e) => {
+ handleDeleteAll();
+ props.onclick?.(e);
+ }}>
Delete All
</Button>
{/snippet}
</Popover.Close>🤖 Prompt for AI Agents
In `@src/lib/components/editor/timeline/timeline-keyframe.svelte` around lines 147
- 153, The explicit onclick after the spread overrides Popover.Close's handler;
update the Button inside the snippet (child({ props })) to compose both handlers
instead of overriding: replace onclick={handleDeleteAll} with a composed handler
like onclick={(e) => { props.onclick?.(e); handleDeleteAll(e); }} so the
Popover.Close (props.onclick) is invoked first and then handleDeleteAll; keep
the {...props} spread as-is to preserve other props.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/components/editor/canvas/canvas.svelte (1)
155-160:⚠️ Potential issue | 🟠 Major
$derived(() => {...})stores a function, not a computed value — use$derived.by.
$derived(() => { ... })assigns the arrow function itself as the derived value, sorecordingScale()re-executes the body on every access without Svelte reactivity tracking for internal dependencies. Use$derived.by(() => { ... })to properly compute and cache the result, then reference it asrecordingScale(without parens) on line 204.Proposed fix
- const recordingScale = $derived(() => { + const recordingScale = $derived.by(() => { if (typeof window === 'undefined') return 1; const scaleX = window.innerWidth / projectStore.project.width; const scaleY = window.innerHeight / projectStore.project.height; return Math.min(1, scaleX, scaleY); // Never scale up, only down });And on line 204:
- style:transform={isRecording ? `scale(${recordingScale()})` : undefined} + style:transform={isRecording ? `scale(${recordingScale})` : undefined}
🤖 Fix all issues with AI agents
In `@src/lib/components/editor/AudioRecorder.svelte`:
- Around line 110-124: The uploadRecording function currently hardcodes '.webm'
when creating the filename (via generateTimestampedFileName) which can mismatch
the negotiated MIME type from startRecording; change uploadRecording to accept a
mimeType (or derived extension) parameter and use that to build the filename
(call generateTimestampedFileName with the correct extension derived from
mimeType instead of 'webm'), then forward that mime/extension into the
uploadMediaBlob call; also update the call site(s) (e.g., where
onstop/startRecording invokes uploadRecording) to pass the resolved MIME type or
its derived extension so uploaded files match the actual codec.
In `@src/lib/components/editor/panels/input-propery.svelte`:
- Around line 39-42: The code always calls
projectStore.setLayerExitTime(layer.id, enterTime + result.duration) which
overwrites any user-set exit time; change it to only set the exit time when the
layer has no existing exitTime (e.g., check layer.exitTime == null/undefined) so
auto-setting occurs only for unset exit times; update the block around
enterTime, layer.enterTime and projectStore.setLayerExitTime to perform this
guard and preserve user trims on re-upload or re-processing.
In `@src/lib/layers/components/VideoLayer.svelte`:
- Around line 124-135: The reactive effect using $effect skips applying a volume
of 0 because it checks `if (volume)` which treats 0 as falsy; change the guard
to test for null/undefined (e.g. `volume !== undefined && volume !== null`) so
`videoEl.volume` is set when volume is 0; also apply the same null/undefined
check for `playbackRate` (replace `if (playbackRate)` with an explicit
null/undefined check) to avoid skipping valid 0-like values, keeping references
to `videoEl`, `volume`, `muted`, and `playbackRate` inside the `$effect` block.
🧹 Nitpick comments (4)
src/lib/components/editor/canvas/layers-renderer.svelte (1)
1-1: Filename should use PascalCase per coding guidelines.The file is named
layers-renderer.sveltebut the coding guidelines require PascalCase for Svelte component filenames (e.g.,LayersRenderer.svelte). As per coding guidelines, "Use PascalCase for Svelte component filenames".src/lib/server/video-renderer.ts (2)
206-214:amixwithinputs=1emits an FFmpeg warning; guard the single-track case.When there's only one audio track, wrapping it through
amix=inputs=1is unnecessary and FFmpeg may emit a warning or behave unexpectedly. For a single track, just alias it directly to[aout].Proposed fix
// Mix all audio tracks together and trim to video duration if (audioInputs.length > 0) { - filterParts.push( - `${audioInputs.join('')}amix=inputs=${audioInputs.length}:duration=longest,atrim=duration=${actualDuration}[aout]` - ); + if (audioInputs.length === 1) { + filterParts.push( + `${audioInputs[0]}atrim=duration=${actualDuration}[aout]` + ); + } else { + filterParts.push( + `${audioInputs.join('')}amix=inputs=${audioInputs.length}:duration=longest,atrim=duration=${actualDuration}[aout]` + ); + } outputOptions.push('-filter_complex', filterParts.join(';')); outputOptions.push('-map', '0:v', '-map', '[aout]'); outputOptions.push('-c:a aac', '-b:a 192k'); }
105-106: Verbose progress logging on every frame will flood server logs.
console.log({ progress })fires for every frame captured. For a 30fps, 60-second render, that's ~1800 log lines. Consider logging at intervals (e.g., every 10% progress) or usingdebuglevel.Proposed fix
const emitProgress = (progress: RenderProgress) => { renderEmitter.emit(`progress:${renderId}`, progress); - console.log({ progress }); + if (progress.phase !== 'capturing' || progress.percent % 10 === 0) { + console.log({ progress }); + } };src/routes/render/[id]/+page.svelte (1)
45-58: Event listeners leak after timeout resolution.When the 5-second timeout fires and resolves the promise, the
loadeddataanderrorlisteners remain attached to the video element. While unlikely to cause problems in the render page context (short-lived), it's a minor hygiene issue.Proposed fix
const videoPromises = Array.from(videoElements).map((video) => { if (video.readyState >= 3) return Promise.resolve(); return new Promise<void>((resolve) => { const onReady = () => { video.removeEventListener('loadeddata', onReady); video.removeEventListener('error', onReady); + clearTimeout(timer); resolve(); }; video.addEventListener('loadeddata', onReady); video.addEventListener('error', onReady); - setTimeout(resolve, 5000); + const timer = setTimeout(() => { + video.removeEventListener('loadeddata', onReady); + video.removeEventListener('error', onReady); + resolve(); + }, 5000); }); });Apply the same pattern to the image promises.
| async function uploadRecording(blob: Blob, duration: number) { | ||
| isUploading = true; | ||
| recordingError = ''; | ||
|
|
||
| try { | ||
| const fileName = generateTimestampedFileName('recording', 'webm'); | ||
| const result = await uploadMediaBlob(blob, fileName, 'audio', projectId, duration); | ||
| onRecordingComplete(result); | ||
| } catch (err) { | ||
| recordingError = err instanceof Error ? err.message : 'Upload failed'; | ||
| } finally { | ||
| isUploading = false; | ||
| recordingDuration = 0; | ||
| } | ||
| } |
There was a problem hiding this comment.
Hardcoded .webm extension doesn't match the negotiated MIME type.
startRecording correctly negotiates the MIME type (which may resolve to audio/mp4 on Safari), but uploadRecording always generates a .webm filename. This produces a mismatched extension when the actual codec is mp4, which can break downstream processing or playback.
Pass the resolved MIME type (or derived extension) through to uploadRecording.
🐛 Proposed fix
- async function uploadRecording(blob: Blob, duration: number) {
+ async function uploadRecording(blob: Blob, duration: number, mimeType: string) {
isUploading = true;
recordingError = '';
try {
- const fileName = generateTimestampedFileName('recording', 'webm');
+ const ext = mimeType.includes('mp4') ? 'mp4' : mimeType.includes('ogg') ? 'ogg' : 'webm';
+ const fileName = generateTimestampedFileName('recording', ext);
const result = await uploadMediaBlob(blob, fileName, 'audio', projectId, duration);
onRecordingComplete(result);
} catch (err) {And update the call site in onstop:
- await uploadRecording(audioBlob, finalDuration);
+ await uploadRecording(audioBlob, finalDuration, mimeType);🤖 Prompt for AI Agents
In `@src/lib/components/editor/AudioRecorder.svelte` around lines 110 - 124, The
uploadRecording function currently hardcodes '.webm' when creating the filename
(via generateTimestampedFileName) which can mismatch the negotiated MIME type
from startRecording; change uploadRecording to accept a mimeType (or derived
extension) parameter and use that to build the filename (call
generateTimestampedFileName with the correct extension derived from mimeType
instead of 'webm'), then forward that mime/extension into the uploadMediaBlob
call; also update the call site(s) (e.g., where onstop/startRecording invokes
uploadRecording) to pass the resolved MIME type or its derived extension so
uploaded files match the actual codec.
| // Auto-set exit time based on content duration if layer has no exit time yet | ||
| const enterTime = layer.enterTime ?? 0; | ||
| projectStore.setLayerExitTime(layer.id, enterTime + result.duration); | ||
| } |
There was a problem hiding this comment.
setLayerExitTime is called unconditionally — overwrites user-set exit time on re-upload.
The comment says "Auto-set exit time based on content duration if layer has no exit time yet," but the code always calls projectStore.setLayerExitTime(layer.id, enterTime + result.duration). If the user has manually trimmed the exit time, re-uploading or re-processing will silently overwrite it.
Proposed fix: guard with existing exit time check
if (result.duration !== undefined) {
projectStore.updateLayer(layer.id, {
contentDuration: result.duration
});
// Auto-set exit time based on content duration if layer has no exit time yet
const enterTime = layer.enterTime ?? 0;
- projectStore.setLayerExitTime(layer.id, enterTime + result.duration);
+ if (layer.exitTime === undefined) {
+ projectStore.setLayerExitTime(layer.id, enterTime + result.duration);
+ }
}📝 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.
| // Auto-set exit time based on content duration if layer has no exit time yet | |
| const enterTime = layer.enterTime ?? 0; | |
| projectStore.setLayerExitTime(layer.id, enterTime + result.duration); | |
| } | |
| if (result.duration !== undefined) { | |
| projectStore.updateLayer(layer.id, { | |
| contentDuration: result.duration | |
| }); | |
| // Auto-set exit time based on content duration if layer has no exit time yet | |
| const enterTime = layer.enterTime ?? 0; | |
| if (layer.exitTime === undefined) { | |
| projectStore.setLayerExitTime(layer.id, enterTime + result.duration); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@src/lib/components/editor/panels/input-propery.svelte` around lines 39 - 42,
The code always calls projectStore.setLayerExitTime(layer.id, enterTime +
result.duration) which overwrites any user-set exit time; change it to only set
the exit time when the layer has no existing exitTime (e.g., check
layer.exitTime == null/undefined) so auto-setting occurs only for unset exit
times; update the block around enterTime, layer.enterTime and
projectStore.setLayerExitTime to perform this guard and preserve user trims on
re-upload or re-processing.
| $effect(() => { | ||
| if (!videoEl) { | ||
| return; | ||
| } | ||
| if (volume) { | ||
| videoEl.volume = volume; | ||
| } | ||
| videoEl.muted = muted; | ||
| if (playbackRate) { | ||
| videoEl.playbackRate = playbackRate; | ||
| } | ||
| }); |
There was a problem hiding this comment.
if (volume) skips setting volume when value is 0.
Since 0 is falsy, if (volume) prevents the video element from being set to volume 0 (fully silent via volume slider). Use a null/undefined check instead.
Proposed fix
$effect(() => {
if (!videoEl) {
return;
}
- if (volume) {
- videoEl.volume = volume;
- }
+ videoEl.volume = volume ?? 1;
videoEl.muted = muted;
- if (playbackRate) {
- videoEl.playbackRate = playbackRate;
- }
+ videoEl.playbackRate = playbackRate ?? 1;
});🤖 Prompt for AI Agents
In `@src/lib/layers/components/VideoLayer.svelte` around lines 124 - 135, The
reactive effect using $effect skips applying a volume of 0 because it checks `if
(volume)` which treats 0 as falsy; change the guard to test for null/undefined
(e.g. `volume !== undefined && volume !== null`) so `videoEl.volume` is set when
volume is 0; also apply the same null/undefined check for `playbackRate`
(replace `if (playbackRate)` with an explicit null/undefined check) to avoid
skipping valid 0-like values, keeping references to `videoEl`, `volume`,
`muted`, and `playbackRate` inside the `$effect` block.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/layers/base.ts (1)
126-132:⚠️ Potential issue | 🟡 MinorTypo in comment: "THis" → "This".
Line 130 has a capitalization typo.
Suggested fix
- // THis is ok, dont change zodType.meta()?.description + // This is ok, don't change zodType.meta()?.description
🤖 Fix all issues with AI agents
In `@src/lib/components/editor/FileUpload.svelte`:
- Around line 115-128: The code currently treats any ok HTTP response as success
but silently no-ops if the JSON payload lacks data.success or data.file; update
the post-response handling in FileUpload.svelte to validate the parsed JSON (the
`data` variable) and, if `!data.success || !data.file`, throw an Error or call
the existing error-handling path with a clear message (e.g., use `data.message`
if present or a fallback like "Upload returned unexpected payload"), so
`onUpload` is only called when `data.file` exists and the UI/spinner/error state
is correctly updated.
In `@src/lib/components/editor/panels/input-propery.svelte`:
- Around line 106-108: The code assumes metadata.meta.component exists when
metadata.meta?.widget === 'custom', which can be undefined and crash rendering;
update the conditional in input-propery.svelte to guard the custom branch by
checking metadata.meta?.component before creating {`@const` Component =
metadata.meta.component} and rendering <Component ...>, and if it's missing
either skip rendering the custom component or render a safe fallback (e.g., a
placeholder or error message) while still calling onUpdateProp and passing
layer/value props as before.
In `@src/lib/layers/components/AudioLayer.svelte`:
- Around line 107-132: The $effect in AudioLayer.svelte doesn't clean up the
HTMLAudioElement, so audioEl may keep playing after the component/layer is
destroyed; add a teardown that pauses and releases the audio and any resources:
inside the reactive effect (or via Svelte's onDestroy) stop playback
(audioEl.pause()), remove event listeners attached to audioEl (if any), clear
audioEl.src or revokeObjectURL if you assigned one, and null out audioEl
reference so the element can be GC'd; target the existing $effect block and
ensure a cleanup function runs on component teardown or when audioEl changes to
prevent orphaned audio playback.
- Around line 49-50: The module script is referencing CaptionsEditor in the call
to .register(fieldRegistry, { widget: 'custom', component: CaptionsEditor }) but
CaptionsEditor is currently imported only in the instance script; move the
import of CaptionsEditor into the module <script> so the module scope can access
it, update the module script to import CaptionsEditor before the .register call,
and remove the duplicate import from the instance <script> to avoid redundant
imports.
In `@src/routes/api/captions/`+server.ts:
- Around line 16-21: The schema declares a style field that is never used;
update the handler to either remove style from CaptionRequestSchema or consume
it: destructure style from the parsed request object (the result of
CaptionRequestSchema.parse/parseAsync) and forward it into the transcription
call (e.g., pass style as the prompt or style parameter to the function that
calls Whisper/transcribe — locate the transcription invocation in the handler
and include style there), or simply delete style from CaptionRequestSchema if
you don't intend to influence Whisper; ensure you reference the
CaptionRequestSchema and the transcription call (the function/method that sends
audio to Whisper) when making the change.
- Around line 75-81: The current contentType-to-extension fallback (using
variable ext derived from contentType) only checks for 'wav', 'ogg', and 'webm'
and defaults everything else to 'mp3'; update the mapping logic in the code that
computes ext to handle common audio MIME types (e.g., 'flac' -> 'flac', 'aac' ->
'aac', 'mp4'/'m4a' -> 'mp4' or 'm4a') or, better, derive the extension from the
MIME subtype by parsing contentType (split on '/' and use the subtype) and
normalize known aliases (e.g., 'mpeg' -> 'mp3', 'x-m4a'/'m4a' -> 'm4a'), or use
a small lookup map (e.g., { 'audio/flac':'flac', 'audio/aac':'aac',
'audio/mp4':'m4a', 'audio/mpeg':'mp3', ...}) to ensure Whisper receives the
correct extension when computing ext from contentType.
🧹 Nitpick comments (4)
src/lib/layers/properties/GenerateCaption.svelte (1)
41-42: Response payload is not validated with Zod.The API response is trusted via
data.success && data.wordswithout schema validation. Per coding guidelines, external data should be validated with Zod before use. Consider parsing the response with a small schema (e.g.,z.object({ success: z.literal(true), words: z.array(...) })).src/lib/layers/properties/CaptionsEditor.svelte (1)
9-22:CaptionWordtype is duplicated andvalueis unsafely cast.The
CaptionWordtype mirrors the shape defined inAudioLayer.svelte's schema (z.object({ word, start, end })). Consider extracting a shared schema/type to avoid drift. Additionally,value as CaptionWord[]on line 22 bypasses runtime validation — if the data shape doesn't match, updates will silently produce malformed objects.Suggested approach
+import { z } from 'zod'; + +const CaptionWordSchema = z.object({ + word: z.string(), + start: z.number(), + end: z.number() +}); + -type CaptionWord = { - word: string; - start: number; - end: number; -}; +type CaptionWord = z.infer<typeof CaptionWordSchema>; // Ensure value is an array of caption words -let words = $derived(Array.isArray(value) ? (value as CaptionWord[]) : []); +let words = $derived( + Array.isArray(value) ? value.filter((v): v is CaptionWord => CaptionWordSchema.safeParse(v).success) : [] +);Ideally, extract the
CaptionWordSchemainto a shared module soAudioLayer.svelteand this component both reference it.src/lib/ai/schemas.ts (1)
131-131: Missing blank line between the closing ofCreateLayerInputSchemaand the next JSDoc block.Line 131 (end of schema) flows directly into line 132 (start of
/** ... */), making it harder to visually separate the schema from the function. Minor readability nit.src/lib/layers/base.ts (1)
79-97:PropertyMetadatais a plain TypeScript type rather than a Zod schema.Per the coding guidelines, types should ideally be inferred from Zod schemas (
z.infer<typeof …>). SincePropertyMetadatais only used internally for UI generation metadata (not for validating external data), this is acceptable — but worth noting if it ever starts being used at a trust boundary.
| if (!res.ok) { | ||
| const data = await res.json().catch(() => null); | ||
| throw new Error(data?.message || `Upload failed (${res.status})`); | ||
| } | ||
|
|
||
| const data = await res.json(); | ||
| if (data.success && data.file) { | ||
| onUpload({ | ||
| url: data.file.url, | ||
| key: data.file.key, | ||
| fileName: data.file.originalName, | ||
| duration | ||
| }); | ||
| } |
There was a problem hiding this comment.
Silent no-op when server returns 200 but unexpected payload shape.
If the response is ok but data.success is falsy or data.file is missing, the upload completes silently without calling onUpload or reporting an error. The user sees the spinner stop with no feedback.
🛡️ Proposed fix
const data = await res.json();
if (data.success && data.file) {
onUpload({
url: data.file.url,
key: data.file.key,
fileName: data.file.originalName,
duration
});
+ } else {
+ throw new Error(data?.message || 'Upload failed: unexpected response');
}🤖 Prompt for AI Agents
In `@src/lib/components/editor/FileUpload.svelte` around lines 115 - 128, The code
currently treats any ok HTTP response as success but silently no-ops if the JSON
payload lacks data.success or data.file; update the post-response handling in
FileUpload.svelte to validate the parsed JSON (the `data` variable) and, if
`!data.success || !data.file`, throw an Error or call the existing
error-handling path with a clear message (e.g., use `data.message` if present or
a fallback like "Upload returned unexpected payload"), so `onUpload` is only
called when `data.file` exists and the UI/spinner/error state is correctly
updated.
| {:else if metadata.meta?.widget === 'custom'} | ||
| {@const Component = metadata.meta.component} | ||
| <Component {value} onChange={(newValue) => onUpdateProp(metadata.name, newValue)} {layer} /> |
There was a problem hiding this comment.
No guard against metadata.meta.component being undefined.
The widget === 'custom' check ensures metadata.meta exists, but metadata.meta.component could still be undefined, which would cause a runtime error when Svelte tries to render {@const Component = metadata.meta.component}.
🛡️ Proposed fix
{:else if metadata.meta?.widget === 'custom'}
- {`@const` Component = metadata.meta.component}
- <Component {value} onChange={(newValue) => onUpdateProp(metadata.name, newValue)} {layer} />
+ {`#if` metadata.meta.component}
+ {`@const` Component = metadata.meta.component}
+ <Component {value} onChange={(newValue) => onUpdateProp(metadata.name, newValue)} {layer} />
+ {/if}🤖 Prompt for AI Agents
In `@src/lib/components/editor/panels/input-propery.svelte` around lines 106 -
108, The code assumes metadata.meta.component exists when metadata.meta?.widget
=== 'custom', which can be undefined and crash rendering; update the conditional
in input-propery.svelte to guard the custom branch by checking
metadata.meta?.component before creating {`@const` Component =
metadata.meta.component} and rendering <Component ...>, and if it's missing
either skip rendering the custom component or render a safe fallback (e.g., a
placeholder or error message) while still calling onUpdateProp and passing
layer/value props as before.
| .describe('Caption words with timestamps') | ||
| .register(fieldRegistry, { widget: 'custom', component: CaptionsEditor }), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/lib/layers/components/AudioLayer.svelte | head -100Repository: epavanello/devmotion
Length of output: 4009
CaptionsEditor referenced in module script but imported only in instance script.
Line 50 (inside <script module>) uses CaptionsEditor in .register(fieldRegistry, { widget: 'custom', component: CaptionsEditor }), but CaptionsEditor is imported at line 79 in the instance <script> block. In Svelte 5, module and instance scripts have separate scopes — the module script cannot access instance-scoped imports. This causes a reference error at build time.
Move the import to the module script:
Proposed fix
<script module lang="ts">
import { z } from 'zod';
import type { LayerMeta } from '../registry';
import { fieldRegistry } from '../base';
import { Music } from '@lucide/svelte';
import GenerateCaption from '../properties/GenerateCaption.svelte';
+ import CaptionsEditor from '../properties/CaptionsEditor.svelte';Remove the duplicate import from line 79 in the instance script.
🤖 Prompt for AI Agents
In `@src/lib/layers/components/AudioLayer.svelte` around lines 49 - 50, The module
script is referencing CaptionsEditor in the call to .register(fieldRegistry, {
widget: 'custom', component: CaptionsEditor }) but CaptionsEditor is currently
imported only in the instance script; move the import of CaptionsEditor into the
module <script> so the module scope can access it, update the module script to
import CaptionsEditor before the .register call, and remove the duplicate import
from the instance <script> to avoid redundant imports.
| $effect(() => { | ||
| if (!audioEl || !src || !layer) return; | ||
|
|
||
| // Use enterTime passed as prop or fallback to 0 | ||
| const enterTime = layer.enterTime ?? 0; | ||
| const relativeTime = currentTime - enterTime; | ||
|
|
||
| // Apply content offset (where to start in the source audio) | ||
| const contentOffset = layer.contentOffset ?? 0; | ||
| const audioTime = contentOffset + relativeTime * (playbackRate ?? 1); | ||
|
|
||
| // Clamp to valid range (up to audio duration) | ||
| const maxTime = audioEl.duration || Infinity; | ||
| const clampedTime = Math.max(contentOffset, Math.min(audioTime, maxTime)); | ||
|
|
||
| if (isFinite(clampedTime) && Math.abs(audioEl.currentTime - clampedTime) > 0.05) { | ||
| audioEl.currentTime = clampedTime; | ||
| } | ||
|
|
||
| // Sync playback state | ||
| if (isPlaying && relativeTime >= 0 && audioTime < maxTime) { | ||
| if (audioEl.paused) audioEl.play().catch(() => {}); | ||
| } else { | ||
| if (!audioEl.paused) audioEl.pause(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Audio element is not cleaned up on component teardown.
When the component is destroyed (e.g., layer removed), the HTMLAudioElement may continue playing. The $effect doesn't return a cleanup function to pause/release the audio. This could cause orphaned audio playback.
Proposed fix
$effect(() => {
if (!audioEl || !src || !layer) return;
// ... existing sync logic ...
+
+ return () => {
+ if (audioEl && !audioEl.paused) audioEl.pause();
+ };
});🤖 Prompt for AI Agents
In `@src/lib/layers/components/AudioLayer.svelte` around lines 107 - 132, The
$effect in AudioLayer.svelte doesn't clean up the HTMLAudioElement, so audioEl
may keep playing after the component/layer is destroyed; add a teardown that
pauses and releases the audio and any resources: inside the reactive effect (or
via Svelte's onDestroy) stop playback (audioEl.pause()), remove event listeners
attached to audioEl (if any), clear audioEl.src or revokeObjectURL if you
assigned one, and null out audioEl reference so the element can be GC'd; target
the existing $effect block and ensure a cleanup function runs on component
teardown or when audioEl changes to prevent orphaned audio playback.
| const CaptionRequestSchema = z.object({ | ||
| fileKey: z.string().optional(), | ||
| audioUrl: z.string().optional(), | ||
| language: z.string().optional(), | ||
| style: z.enum(['subtitle', 'caption', 'lyrics']).optional() | ||
| }); |
There was a problem hiding this comment.
style field is validated but never used.
The CaptionRequestSchema accepts a style field (line 20), but it's not destructured on line 44 and is never referenced in the handler. Either remove it from the schema or use it (e.g., pass to Whisper's prompt parameter).
Fix: remove unused field
const CaptionRequestSchema = z.object({
fileKey: z.string().optional(),
audioUrl: z.string().optional(),
- language: z.string().optional(),
- style: z.enum(['subtitle', 'caption', 'lyrics']).optional()
+ language: z.string().optional()
});Also applies to: 44-44
🤖 Prompt for AI Agents
In `@src/routes/api/captions/`+server.ts around lines 16 - 21, The schema declares
a style field that is never used; update the handler to either remove style from
CaptionRequestSchema or consume it: destructure style from the parsed request
object (the result of CaptionRequestSchema.parse/parseAsync) and forward it into
the transcription call (e.g., pass style as the prompt or style parameter to the
function that calls Whisper/transcribe — locate the transcription invocation in
the handler and include style there), or simply delete style from
CaptionRequestSchema if you don't intend to influence Whisper; ensure you
reference the CaptionRequestSchema and the transcription call (the
function/method that sends audio to Whisper) when making the change.
| const ext = contentType.includes('wav') | ||
| ? 'wav' | ||
| : contentType.includes('ogg') | ||
| ? 'ogg' | ||
| : contentType.includes('webm') | ||
| ? 'webm' | ||
| : 'mp3'; |
There was a problem hiding this comment.
Content-type to extension mapping is incomplete.
Only wav, ogg, webm are checked; everything else defaults to mp3. Common audio types like flac, aac, mp4/m4a will be misidentified, which could cause Whisper to reject or misprocess the file.
Consider a more robust mapping
- const ext = contentType.includes('wav')
- ? 'wav'
- : contentType.includes('ogg')
- ? 'ogg'
- : contentType.includes('webm')
- ? 'webm'
- : 'mp3';
+ const extMap: Record<string, string> = {
+ wav: 'wav',
+ ogg: 'ogg',
+ webm: 'webm',
+ flac: 'flac',
+ mp4: 'm4a',
+ m4a: 'm4a',
+ mpeg: 'mp3'
+ };
+ const ext = Object.entries(extMap).find(([k]) => contentType.includes(k))?.[1] ?? 'mp3';📝 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.
| const ext = contentType.includes('wav') | |
| ? 'wav' | |
| : contentType.includes('ogg') | |
| ? 'ogg' | |
| : contentType.includes('webm') | |
| ? 'webm' | |
| : 'mp3'; | |
| const extMap: Record<string, string> = { | |
| wav: 'wav', | |
| ogg: 'ogg', | |
| webm: 'webm', | |
| flac: 'flac', | |
| mp4: 'm4a', | |
| m4a: 'm4a', | |
| mpeg: 'mp3' | |
| }; | |
| const ext = Object.entries(extMap).find(([k]) => contentType.includes(k))?.[1] ?? 'mp3'; |
🤖 Prompt for AI Agents
In `@src/routes/api/captions/`+server.ts around lines 75 - 81, The current
contentType-to-extension fallback (using variable ext derived from contentType)
only checks for 'wav', 'ogg', and 'webm' and defaults everything else to 'mp3';
update the mapping logic in the code that computes ext to handle common audio
MIME types (e.g., 'flac' -> 'flac', 'aac' -> 'aac', 'mp4'/'m4a' -> 'mp4' or
'm4a') or, better, derive the extension from the MIME subtype by parsing
contentType (split on '/' and use the subtype) and normalize known aliases
(e.g., 'mpeg' -> 'mp3', 'x-m4a'/'m4a' -> 'm4a'), or use a small lookup map
(e.g., { 'audio/flac':'flac', 'audio/aac':'aac', 'audio/mp4':'m4a',
'audio/mpeg':'mp3', ...}) to ensure Whisper receives the correct extension when
computing ext from contentType.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/lib/components/editor/toolbar.svelte`:
- Around line 107-121: The confirm prompt in openExportDialog currently reads
like a directive; update the confirmation text to a question to match OK/Cancel
semantics (e.g., change the string passed to confirm(...) to "Save unsaved
changes before exporting?") while keeping the same conditional flow using
projectStore.hasUnsavedChanges, user, and the existing handleSaveToCloud() call
so behavior remains unchanged.
In `@src/lib/layers/properties/GenerateCaption.svelte`:
- Around line 17-28: generateCaptions currently fires an API request even when
layer.props.fileKey is falsy; add an early guard at the start of
generateCaptions to check fileKey (from layer.props.fileKey) and if it's missing
set captionError to a helpful message and return (ensure isGeneratingCaptions is
not set true in that case), and also update the UI control that calls
generateCaptions to disable the button when fileKey is falsy so the action
cannot be invoked without an uploaded file.
In `@src/lib/stores/project.svelte.ts`:
- Around line 571-595: The validExitTime calculation in setLayerTimeRange can
exceed project bounds because it uses Math.max(clampedExitTime, clampedEnterTime
+ 0.1) without re-clamping; change it to clamp the result to the project
duration (e.g. validExitTime = Math.min(this.project.duration,
Math.max(clampedExitTime, clampedEnterTime + 0.1))) so validExitTime never
exceeds this.project.duration while preserving the minimum length relative to
clampedEnterTime.
In `@src/routes/api/captions/`+server.ts:
- Around line 1-7: The top doc comment in src/routes/api/captions/+server.ts
incorrectly states the caption format as "MM:SS.ms - MM:SS.ms | caption text"
whereas the actual handler returns word-level objects; update the comment to
describe the real response shape (an array of objects like { word: string,
start: number, end: number } representing word-level timings) and remove the
misleading timed-string example so the documentation matches the handler's
output.
| async function openExportDialog() { | ||
| // Check for unsaved changes | ||
| if (projectStore.hasUnsavedChanges && user) { | ||
| const shouldSave = confirm( | ||
| 'You have unsaved changes. Please save your project before exporting.' | ||
| ); | ||
|
|
||
| if (!shouldSave) { | ||
| return; | ||
| } | ||
| await handleSaveToCloud(); | ||
| } | ||
|
|
||
| showExportDialog = true; | ||
| } |
There was a problem hiding this comment.
Confirm dialog wording reads as a directive, not a question.
The confirm() text says "Please save your project before exporting" — which sounds like an instruction rather than a yes/no prompt. Consider rewording to something like "Save unsaved changes before exporting?" so the OK/Cancel buttons make intuitive sense.
🤖 Prompt for AI Agents
In `@src/lib/components/editor/toolbar.svelte` around lines 107 - 121, The confirm
prompt in openExportDialog currently reads like a directive; update the
confirmation text to a question to match OK/Cancel semantics (e.g., change the
string passed to confirm(...) to "Save unsaved changes before exporting?") while
keeping the same conditional flow using projectStore.hasUnsavedChanges, user,
and the existing handleSaveToCloud() call so behavior remains unchanged.
| async function generateCaptions() { | ||
| const fileKey = layer.props.fileKey; | ||
|
|
||
| isGeneratingCaptions = true; | ||
| captionError = ''; | ||
|
|
||
| try { | ||
| const res = await fetch('/api/captions', { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ fileKey }) | ||
| }); |
There was a problem hiding this comment.
No guard when fileKey is missing.
If the layer has no fileKey (e.g., not yet uploaded), the request fires with undefined, gets a 400 from the API, and shows a generic error. Consider disabling the button or returning early when fileKey is falsy for better UX.
Proposed fix
async function generateCaptions() {
const fileKey = layer.props.fileKey;
+ if (!fileKey) {
+ captionError = 'No audio file uploaded';
+ return;
+ }
isGeneratingCaptions = true;
captionError = '';📝 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.
| async function generateCaptions() { | |
| const fileKey = layer.props.fileKey; | |
| isGeneratingCaptions = true; | |
| captionError = ''; | |
| try { | |
| const res = await fetch('/api/captions', { | |
| method: 'POST', | |
| headers: { 'Content-Type': 'application/json' }, | |
| body: JSON.stringify({ fileKey }) | |
| }); | |
| async function generateCaptions() { | |
| const fileKey = layer.props.fileKey; | |
| if (!fileKey) { | |
| captionError = 'No audio file uploaded'; | |
| return; | |
| } | |
| isGeneratingCaptions = true; | |
| captionError = ''; | |
| try { | |
| const res = await fetch('/api/captions', { | |
| method: 'POST', | |
| headers: { 'Content-Type': 'application/json' }, | |
| body: JSON.stringify({ fileKey }) | |
| }); |
🤖 Prompt for AI Agents
In `@src/lib/layers/properties/GenerateCaption.svelte` around lines 17 - 28,
generateCaptions currently fires an API request even when layer.props.fileKey is
falsy; add an early guard at the start of generateCaptions to check fileKey
(from layer.props.fileKey) and if it's missing set captionError to a helpful
message and return (ensure isGeneratingCaptions is not set true in that case),
and also update the UI control that calls generateCaptions to disable the button
when fileKey is falsy so the action cannot be invoked without an uploaded file.
| // For non-media layers, use simple logic | ||
| const clampedEnterTime = Math.max(0, Math.min(enterTime, this.project.duration)); | ||
| const clampedExitTime = Math.max(0, Math.min(exitTime, this.project.duration)); | ||
| const validExitTime = Math.max(clampedExitTime, clampedEnterTime + 0.1); | ||
|
|
||
| // Shift keyframes if requested | ||
| const timeDelta = shiftKeyframes ? clampedEnterTime - oldEnterTime : 0; | ||
| const shiftedKeyframes = | ||
| timeDelta !== 0 | ||
| ? layer.keyframes | ||
| .map((kf) => ({ | ||
| ...kf, | ||
| time: Math.max(0, Math.min(this.project.duration, kf.time + timeDelta)) | ||
| })) | ||
| .sort((a, b) => a.time - b.time) | ||
| : layer.keyframes; | ||
|
|
||
| return { | ||
| ...layer, | ||
| enterTime: clampedEnterTime, | ||
| exitTime: validExitTime, | ||
| keyframes: shiftedKeyframes | ||
| }; | ||
| }); | ||
| } |
There was a problem hiding this comment.
Non-media validExitTime in setLayerTimeRange can exceed project duration.
Line 574: Math.max(clampedExitTime, clampedEnterTime + 0.1) — when clampedEnterTime is near this.project.duration, the + 0.1 can push validExitTime past the project's duration boundary. Apply Math.min(this.project.duration, ...) as an outer clamp.
Proposed fix
- const validExitTime = Math.max(clampedExitTime, clampedEnterTime + 0.1);
+ const validExitTime = Math.min(
+ this.project.duration,
+ Math.max(clampedExitTime, clampedEnterTime + 0.1)
+ );🤖 Prompt for AI Agents
In `@src/lib/stores/project.svelte.ts` around lines 571 - 595, The validExitTime
calculation in setLayerTimeRange can exceed project bounds because it uses
Math.max(clampedExitTime, clampedEnterTime + 0.1) without re-clamping; change it
to clamp the result to the project duration (e.g. validExitTime =
Math.min(this.project.duration, Math.max(clampedExitTime, clampedEnterTime +
0.1))) so validExitTime never exceeds this.project.duration while preserving the
minimum length relative to clampedEnterTime.
| /** | ||
| * AI Caption Generation API | ||
| * | ||
| * Generates timed captions/subtitles for audio files using OpenAI Whisper. | ||
| * Returns captions in the timed format expected by AudioLayer: | ||
| * "MM:SS.ms - MM:SS.ms | caption text" | ||
| */ |
There was a problem hiding this comment.
Doc comment describes a format that doesn't match the actual response.
Lines 5–6 say captions are returned as "MM:SS.ms - MM:SS.ms | caption text", but the handler actually returns word-level objects ({ word, start, end }). Update the doc to match the implementation.
🤖 Prompt for AI Agents
In `@src/routes/api/captions/`+server.ts around lines 1 - 7, The top doc comment
in src/routes/api/captions/+server.ts incorrectly states the caption format as
"MM:SS.ms - MM:SS.ms | caption text" whereas the actual handler returns
word-level objects; update the comment to describe the real response shape (an
array of objects like { word: string, start: number, end: number } representing
word-level timings) and remove the misleading timed-string example so the
documentation matches the handler's output.
2 participants
Summary by CodeRabbit
New Features
UI
Documentation
Chores