fix: prevent hydration mismatch in ThemeToggle #611
Conversation
👷 Deploy request for appcut pending review.Visit the deploys page to approve it
|
|
@aasthamewar is attempting to deploy a commit to the OpenCut OSS Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis PR predominantly reformats and reorganizes code across many files. It also activates previously commented hooks and stores, updates an icon host list, re-exports editor utilities, adds a client-mount guard in ThemeToggle, and introduces a new dependency import for video caching. Migrations JSON files receive formatting-only edits. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as UI (Sounds Panel)
participant Store as useSoundsStore (Zustand)
participant API as External SFX API
participant Media as Media Store
participant Timeline as Timeline Store
UI->>Store: setQuery / toggleFilter / search()
Store->>API: Fetch sounds (query, filters, page)
API-->>Store: Results or error
Store-->>UI: Update state (results, loading, error)
UI->>Store: addSoundToTimeline(soundId)
Store->>API: Fetch audio data
API-->>Store: ArrayBuffer or error
Store->>Media: createMediaFile(audio)
Media-->>Store: MediaFile ref
Store->>Timeline: insert audio clip
Timeline-->>UI: Timeline updated
Store-->>UI: Success or error feedback
sequenceDiagram
autonumber
participant User as User
participant Comp as ThemeToggle
participant DOM as Browser
Note over Comp: Initial render (SSR)
Comp->>Comp: mounted=false
User->>DOM: Load page
DOM-->>Comp: useEffect fires
Comp->>Comp: set mounted=true
Comp-->>User: Render label using theme (client-only)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/web/src/components/editor/export-button.tsx (2)
79-81: Bug: includeAudio default coerced to true when false.
|| trueoverrides an explicit false; use nullish coalescing.- const [includeAudio, setIncludeAudio] = useState<boolean>( - DEFAULT_EXPORT_OPTIONS.includeAudio || true - ); + const [includeAudio, setIncludeAudio] = useState<boolean>( + DEFAULT_EXPORT_OPTIONS.includeAudio ?? true + );
148-151: Add accessible name and type to close button.Icon-only button needs an accessible label; also add type to avoid implicit submit.
As per coding guidelines
- <Button variant="text" size="icon" onClick={handleClose}> + <Button + variant="text" + size="icon" + type="button" + aria-label="Close" + onClick={handleClose} + > <X className="!size-5 text-foreground/85" /> </Button>apps/web/src/components/editor/media-panel/tabbar.tsx (1)
64-65: Fix invalid Tailwind class ordering
size-[1.1rem]!is invalid; the important!must prefix the utility.Apply:
- <tab.icon className="size-[1.1rem]! opacity-100 hover:opacity-75" /> + <tab.icon className="!size-[1.1rem] opacity-100 hover:opacity-75" />apps/web/src/components/ui/input.tsx (1)
88-89: Invalid Tailwind arbitrary size value
!size-[0.85]lacks a unit. Userem(or switch toh/wclasses) for valid CSS.Apply one of:
- <X className="!size-[0.85]" /> + <X className="!size-[0.85rem]" />or
- <X className="!size-[0.85]" /> + <X className="h-3.5 w-3.5" />
🧹 Nitpick comments (42)
apps/web/src/lib/editor-utils.ts (1)
3-8: Consideras constfor immutable presetsMarks presets as read‑only and aligns with our TS guideline to prefer
as constfor literals.Apply:
-const DEFAULT_CANVAS_PRESETS = [ +const DEFAULT_CANVAS_PRESETS = [ { name: "16:9", width: 1920, height: 1080 }, { name: "9:16", width: 1080, height: 1920 }, { name: "1:1", width: 1080, height: 1080 }, { name: "4:3", width: 1440, height: 1080 }, -]; +] as const;As per coding guidelines
apps/web/src/hooks/use-highlight-scroll.ts (1)
1-1: MemoizeregisterElementto avoid ref churn; optionally smooth scrollStabilizes the ref callback across renders; smoother UX when scrolling to highlighted items.
Apply:
-import { useEffect, useState, useRef } from "react"; +import { useEffect, useState, useRef, useCallback } from "react"; @@ - const registerElement = (id: string, element: HTMLElement | null) => { + const registerElement = useCallback((id: string, element: HTMLElement | null) => { if (element) { elementRefs.current.set(id, element); } else { elementRefs.current.delete(id); } - }; + }, []); @@ - target?.scrollIntoView({ block: "center" }); + target?.scrollIntoView({ block: "center", behavior: "smooth" });Also applies to: 11-17, 24-26
apps/web/src/lib/schemas/waitlist.ts (1)
3-5: Normalize email input (trim/lowercase) before validationReduces duplicate signups due to casing/whitespace differences.
Apply:
export const exportWaitlistSchema = z.object({ - email: z.string().email().max(320), + email: z.string().trim().toLowerCase().email().max(320), });apps/web/src/components/editor/timeline/timeline-marker.tsx (1)
13-18: ExtractformatTimeout of JSX IIFERemoves nested IIFE in JSX and improves readability/perf by avoiding per-render function creation inside markup.
Apply:
export function TimelineMarker({ time, zoomLevel, interval, isMainMarker, }: TimelineMarkerProps) { - return ( + const formatTime = (seconds: number) => { + const hours = Math.floor(seconds / 3600); + const minutes = Math.floor((seconds % 3600) / 60); + const secs = seconds % 60; + if (hours > 0) { + return `${hours}:${minutes.toString().padStart(2, "0")}:${Math.floor(secs) + .toString() + .padStart(2, "0")}`; + } + if (minutes > 0) { + return `${minutes}:${Math.floor(secs).toString().padStart(2, "0")}`; + } + if (interval >= 1) { + return `${Math.floor(secs)}s`; + } + return `${secs.toFixed(1)}s`; + }; + return ( <div @@ > - {(() => { - const formatTime = (seconds: number) => { - const hours = Math.floor(seconds / 3600); - const minutes = Math.floor((seconds % 3600) / 60); - const secs = seconds % 60; - - if (hours > 0) { - return `${hours}:${minutes - .toString() - .padStart(2, "0")}:${Math.floor(secs) - .toString() - .padStart(2, "0")}`; - } - if (minutes > 0) { - return `${minutes}:${Math.floor(secs) - .toString() - .padStart(2, "0")}`; - } - if (interval >= 1) { - return `${Math.floor(secs)}s`; - } - return `${secs.toFixed(1)}s`; - }; - return formatTime(time); - })()} + {formatTime(time)} </span>Also applies to: 19-66
apps/web/src/hooks/use-infinite-scroll.ts (1)
1-1: Throttle near-bottom triggers and use type-only import for React event typePrevents duplicate onLoadMore calls when the user is already at the bottom and aligns with our TS import guidelines.
Apply:
-import { useRef, useCallback } from "react"; +import { useRef, useCallback, useEffect } from "react"; +import type { UIEvent } from "react"; @@ const scrollAreaRef = useRef<HTMLDivElement>(null); + const isPendingRef = useRef(false); @@ - const handleScroll = useCallback( - (event: React.UIEvent<HTMLDivElement>) => { + const handleScroll = useCallback( + (event: UIEvent<HTMLDivElement>) => { if (!enabled) return; @@ - if (nearBottom && hasMore && !isLoading) { - onLoadMore(); + if (nearBottom && hasMore && !isLoading && !isPendingRef.current) { + isPendingRef.current = true; + onLoadMore(); } }, [onLoadMore, hasMore, isLoading, threshold, enabled] ); + useEffect(() => { + if (!isLoading) isPendingRef.current = false; + }, [isLoading]);As per coding guidelines
Also applies to: 18-19, 21-22, 27-31, 33-35
apps/web/src/components/editor/media-panel/views/captions.tsx (1)
191-194: Remove console. and replaceanywith a typed segment; prefer for...of over forEach*
- console.log/error present (Line 111, Line 180, Line 176); our guidelines disallow console in app code.
segments.forEach((segment: any) => ...)violates “don’t use any”.- Prefer for...of instead of Array.forEach.
Example changes (outside the selected lines):
// Define a type near the top of this file or import from a shared types module type TranscriptionSegment = { text: string; start: number; end: number }; // Replace: - segments.forEach((segment: any) => { + for (const segment of segments as TranscriptionSegment[]) { // Replace console.* with proper logger or guard in dev only - console.log("Transcription completed:", { text, segments }); + if (process.env.NODE_ENV !== "production") { + // Replace with your logger if available + // logger.debug("Transcription completed", { text, segments }); +}Also ensure Button components render with type="button" to avoid implicit submit in forms (either pass type on each usage or default it in the Button component). As per coding guidelines
apps/web/src/hooks/use-sound-search.ts (1)
96-106: Cancel in-flight searches with AbortController (real cancellation, not just ignore flag)Prevents wasted network work and late-arriving updates when the query changes quickly.
Apply:
@@ - const timeoutId = setTimeout(async () => { + const controller = new AbortController(); + const timeoutId = setTimeout(async () => { try { setSearching(true); setSearchError(null); resetPagination(); - const response = await fetch( - `/api/sounds/search?q=${encodeURIComponent(query)}&type=effects&page=1` - ); + const response = await fetch( + `/api/sounds/search?q=${encodeURIComponent(query)}&type=effects&page=1`, + { signal: controller.signal } + ); @@ - return () => { - clearTimeout(timeoutId); - ignore = true; - }; + return () => { + clearTimeout(timeoutId); + controller.abort(); + ignore = true; + };As per coding guidelines
Also applies to: 129-133
apps/web/src/components/ui/editable-timecode.tsx (1)
5-5: Use type-only import for TimeCodeAligns with our TS import rules.
-import { formatTimeCode, parseTimeCode, TimeCode } from "@/lib/time"; +import type { TimeCode } from "@/lib/time"; +import { formatTimeCode, parseTimeCode } from "@/lib/time";As per coding guidelines
apps/web/src/stores/playback-store.ts (1)
39-45: Avoid console usage in stores; gate or replace with a loggerReplace
console.errorwith a project logger or gate it to dev to avoid noisy prod logs.As per coding guidelines
- if (!projectFps) - console.error( - "Project FPS is not set, assuming " + DEFAULT_FPS + "fps" - ); + if (!projectFps) { + // Replace with app logger if available + if (process.env.NODE_ENV !== "production") { + // eslint-disable-next-line no-console + console.warn(`Project FPS is not set, assuming ${DEFAULT_FPS}fps`); + } + }apps/web/src/components/editor/panel-preset-selector.tsx (2)
43-51: Add accessible name and button type on the trigger; hide decorative iconsProvide an accessible name and set
type="button". Mark icons as decorative.As per coding guidelines
- <Button + <Button variant="secondary" size="sm" className="h-8 px-2 flex items-center gap-1 text-xs" - title="Panel Presets" + aria-label="Panel presets" + title="Panel Presets" + type="button" > - <LayoutPanelTop className="h-4 w-4" /> - <ChevronDown className="h-3 w-3" /> + <LayoutPanelTop className="h-4 w-4" aria-hidden="true" /> + <ChevronDown className="h-3 w-3" aria-hidden="true" /> </Button>
77-86: Reset button: specify type, label, and hide decorative iconEnsure it’s not treated as submit in forms and is screen‑reader friendly.
As per coding guidelines
- <Button + <Button variant="secondary" size="icon" className="h-6 w-6 shrink-0 opacity-60 hover:opacity-100" onClick={(e) => handleResetPreset(preset, e)} + aria-label={`Reset ${PRESET_LABELS[preset]} preset`} + type="button" title={`Reset ${PRESET_LABELS[preset]} preset`} > - <RotateCcw className="h-3 w-3" /> + <RotateCcw className="h-3 w-3" aria-hidden="true" /> </Button>apps/web/src/app/api/sounds/search/route.ts (3)
93-98: Parse x-forwarded-for safely (use first IP) for rate limitingUse the first IP in the list to avoid header spoofing issues.
- const ip = request.headers.get("x-forwarded-for") ?? "anonymous"; - const { success } = await baseRateLimit.limit(ip); + const xff = request.headers.get("x-forwarded-for") ?? ""; + const ip = xff.split(",")[0]?.trim() || "anonymous"; + const { success } = await baseRateLimit.limit(ip);
182-189: Remove console logging in API routes (use a logger or omit)Avoid
consolein server handlers; it’s noisy and often flagged in linting.As per coding guidelines
- if (!response.ok) { - const errorText = await response.text(); - console.error("Freesound API error:", response.status, errorText); - return NextResponse.json( - { error: "Failed to search sounds" }, - { status: response.status } - ); - } + if (!response.ok) { + return NextResponse.json( + { error: "Failed to search sounds" }, + { status: response.status } + ); + }
195-199: Remove remaining console.error callsReplace with structured logging (if available) or remove.
As per coding guidelines
- console.error( - "Invalid Freesound API response:", - freesoundValidation.error - ); + // TODO: integrate with app logger (invalid Freesound API response)- console.error( - "Invalid API response structure:", - responseValidation.error - ); + // TODO: integrate with app logger (invalid API response structure)- console.error("Error searching sounds:", error); + // TODO: integrate with app logger (route error)Also applies to: 247-250, 259-259
apps/web/src/components/ui/split-button.tsx (3)
3-3: Import HTMLAttributes as a type (split type/value imports).Enables pass-through div props without widening runtime imports.
As per coding guidelines
-import { ReactNode, forwardRef } from "react"; +import { forwardRef } from "react"; +import type { ReactNode, HTMLAttributes } from "react";
6-9: Allow pass-through div attributes on SplitButton.Extend standard div attributes so {...props} is type-safe and useful.
As per coding guidelines
-interface SplitButtonProps { - children: ReactNode; - className?: string; -} +interface SplitButtonProps extends HTMLAttributes<HTMLDivElement> { + children: ReactNode; +}
38-58: Add explicit type="button" to prevent implicit submit.Prevents accidental form submission; required by our guidelines.
As per coding guidelines
<Button ref={ref} variant="text" + type="button" className={cn(apps/web/src/app/api/waitlist/export/route.ts (1)
77-79: Avoid console logging in API routes.Use structured logging or remove to meet guidelines.
As per coding guidelines
- console.error("Waitlist API error:", error); + // TODO: replace with structured logger (e.g., Sentry/otel) or remove in productionapps/web/src/components/editor/export-button.tsx (3)
235-239: Add explicit type to button.Prevent implicit submit in forms.
As per coding guidelines
- <Button onClick={handleExport} className="w-full gap-2"> + <Button type="button" onClick={handleExport} className="w-full gap-2">
254-261: Cancel button should be a non-submit button.Add type to follow button guidelines.
As per coding guidelines
- <Button + <Button variant="outline" className="rounded-md w-full" + type="button" onClick={() => {}} >
294-312: Add explicit type to small action buttons.Meets button-type guideline and avoids form-submit side effects.
As per coding guidelines
- <Button + <Button variant="outline" size="sm" className="flex-1 text-xs h-8" + type="button" onClick={handleCopy} > @@ - <Button + <Button variant="outline" size="sm" className="flex-1 text-xs h-8" + type="button" onClick={onRetry} >apps/web/src/constants/text-constants.ts (1)
4-24: Preferas constwithsatisfiesover explicit type annotation.Keeps literal precision without widening and follows our style preference.
Based on learnings
-export const DEFAULT_TEXT_ELEMENT: Omit<TextElement, "id"> = { +export const DEFAULT_TEXT_ELEMENT = { type: "text", name: "Text", content: "Default Text", fontSize: 48, fontFamily: "Arial", color: "#ffffff", backgroundColor: "transparent", textAlign: "center", fontWeight: "normal", fontStyle: "normal", textDecoration: "none", x: 0, y: 0, rotation: 0, opacity: 1, duration: TIMELINE_CONSTANTS.DEFAULT_TEXT_DURATION, startTime: 0, trimStart: 0, trimEnd: 0, -}; +} as const satisfies Omit<TextElement, "id">;apps/web/src/components/icons.tsx (1)
3-20: Add <title> to SVGs for accessibility.Icons should include a descriptive title unless adjacent text names them.
As per coding guidelines
- <svg className={className} viewBox="0 0 24 24"> + <svg className={className} viewBox="0 0 24 24"> + <title>Google icon</title> @@ - > + > + <title>GitHub icon</title> @@ - > + > + <title>Vercel logo</title> @@ - > + > + <title>Background pattern</title> @@ - > + > + <title>Marble icon</title> @@ - > + > + <title>DataBuddy icon</title> @@ - > + > + <title>Socials icon</title> @@ - > + > + <title>Transition up icon</title>Also applies to: 26-38, 43-53, 58-113, 124-139, 150-165, 175-227, 239-266
apps/web/src/app/api/transcribe/route.ts (2)
115-122: Add timeout/abort to external fetch to avoid hanging requests.Prevents request threads from stalling on upstream issues.
- const response = await fetch(env.MODAL_TRANSCRIPTION_URL, { - method: "POST", - headers: { - "Content-Type": "application/json", - }, - body: JSON.stringify(modalRequestBody), - }); + const ac = new AbortController(); + const timeoutId = setTimeout(() => ac.abort(), 30_000); + let response: Response; + try { + response = await fetch(env.MODAL_TRANSCRIPTION_URL, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(modalRequestBody), + signal: ac.signal, + }); + } finally { + clearTimeout(timeoutId); + }
66-70: Remove console logs and unused variable.Replace ad-hoc logging with structured logs (or remove) and drop unused
origin.As per coding guidelines
- console.error( - "Missing environment variables:", - JSON.stringify(transcriptionCheck.missingVars) - ); + // TODO: replace with structured logging (Sentry/otel) if needed @@ - console.error("Modal API error:", response.status, errorText); + // TODO: replace with structured logging (Sentry/otel) if needed @@ - console.log("Raw Modal response:", JSON.stringify(rawResult, null, 2)); + // Debug logging removed; consider structured logs behind a debug flag @@ - const origin = request.headers.get("origin"); + // const origin = request.headers.get("origin"); // unusedAlso applies to: 123-126, 145-146, 57-58
apps/web/src/stores/text-properties-store.ts (1)
11-14: Prefer inference withas constover explicit annotationDrop the explicit
ReadonlyArray<TextPropertiesTabMeta>and keepas constto align with our TS guideline: avoid annotations on literals and preferas const.Apply:
-export const TEXT_PROPERTIES_TABS: ReadonlyArray<TextPropertiesTabMeta> = [ +export const TEXT_PROPERTIES_TABS = [ { value: "transform", label: "Transform" }, { value: "style", label: "Style" }, ] as const;apps/web/src/components/editor/layout-guide-overlay.tsx (1)
9-15: Addsizestonext/imagewithfillto avoid layout shiftWhen using
fill, adding asizeshint helps prevent CLS and optimizes image selection.Apply:
<Image src="/platform-guides/tiktok-blueprint.png" alt="TikTok layout guide" className="absolute inset-0 w-full h-full object-contain" draggable={false} + sizes="100vw" fill />apps/web/src/app/api/get-upload-url/route.ts (1)
88-90: Optional: bindContent-Typein signature to limit uploadsYou can pre-bind a content type so clients can’t upload arbitrary blobs. Client must then send the same header on PUT.
Example:
const contentType = `audio/${fileExtension === "m4a" ? "mp4" : fileExtension}`; const signed = await client.sign( new Request(url, { method: "PUT", headers: { "Content-Type": contentType } }), { aws: { signQuery: true } } );apps/web/src/components/ui/tooltip.tsx (1)
55-69: Mark decorative SVG as aria-hiddenThe caret is decorative; hide it from assistive tech.
Apply:
- {variant === "sidebar" && ( - <svg + {variant === "sidebar" && ( + <svg + aria-hidden="true" width="6" height="10" viewBox="0 0 6 10" fill="none" xmlns="http://www.w3.org/2000/svg" className="absolute left-[-6px] top-1/2 -translate-y-1/2" >apps/web/src/components/theme-toggle.tsx (1)
12-33: Hydration fix LGTM; add a stable accessible nameRendering the label only after mount avoids mismatches. Add a static
aria-labelso the button is accessible pre-hydration.Apply:
return ( <Button size="icon" variant="text" className="h-7" + aria-label="Toggle theme" onClick={() => setTheme(theme === "dark" ? "light" : "dark")} > <Sun className="!size-[1.1rem]" /> {mounted && ( <span className="sr-only">{theme === "dark" ? "Light" : "Dark"}</span> )} </Button> );apps/web/src/hooks/use-edge-auto-scroll.ts (1)
72-79: Optional: make scroll speed frame‑rate independent.Currently speed is “pixels per frame” and varies with refresh rate. Consider scaling by delta time (from
performance.now()) to normalize across displays.apps/web/src/components/language-select.tsx (3)
56-74: Use the provided containerRef for outside click detection (avoid unused prop).
containerRefis passed but unused. Prefer checking clicks outside the container if available, falling back to the button.Apply this diff:
- const handleClickOutside = (event: MouseEvent) => { - if ( - buttonRef.current && - !buttonRef.current.contains(event.target as Node) - ) { + const handleClickOutside = (event: MouseEvent) => { + const target = event.target as Node; + const container = containerRef?.current ?? buttonRef.current; + if (container && !container.contains(target)) { setIsClosing(true); setTimeout(() => setIsClosing(false), 600); setExpanded(false); buttonRef.current?.blur(); } };
165-166: Mark decorative chevron icon as hidden from assistive tech.Add
aria-hiddenso screen readers ignore the purely decorative icon.Apply this diff:
- <ChevronDown className="text-muted-foreground size-4" /> + <ChevronDown aria-hidden="true" focusable="false" className="text-muted-foreground size-4" />
41-44: Remove unused “isClosing” state (dead code).
isClosingis set but not used to affect UI or logic.Apply this diff:
- const [isClosing, setIsClosing] = useState(false); + // removed: isClosing (unused) ... - setIsClosing(true); - setTimeout(() => setIsClosing(false), 600); + // no-op: removed unused isClosingAlso applies to: 63-65
apps/web/src/stores/sounds-store.ts (2)
241-249: Infer audio MIME/extension from response headers (avoid hardcoding .mp3).Preview URLs may serve non-MP3 content; use server-provided content-type to name the file correctly.
Apply this diff:
- const blob = await response.blob(); - const file = new File([blob], `${sound.name}.mp3`, { - type: "audio/mpeg", - }); + const blob = await response.blob(); + const contentType = response.headers.get("content-type") || "audio/mpeg"; + const ext = + contentType.includes("mpeg") ? "mp3" : + contentType.includes("ogg") ? "ogg" : + contentType.includes("wav") ? "wav" : + contentType.includes("aac") ? "aac" : "mp3"; + const safeName = `${sound.name}`.trim() || `sound-${sound.id}`; + const file = new File([blob], `${safeName}.${ext}`, { + type: contentType, + });
159-160: Replace console.error with app logger or structured error reporting.Project guidelines discourage console usage. Route errors through a logging utility or telemetry.
Example diff (dev-only logging if no logger exists):
- console.error("Failed to load saved sounds:", error); + if (process.env.NODE_ENV !== "production") { + // eslint-disable-next-line no-console + console.error("Failed to load saved sounds:", error); + }Repeat similarly for other sites.
Also applies to: 175-176, 193-193, 224-224
apps/web/src/types/sounds.ts (1)
1-39: Optional: preferexport typeoverinterfacefor type-only exports.To align with the guidelines to use
export typefor types, consider converting these interfaces to type aliases. Example for one type:-export interface SoundEffect { +export type SoundEffect = { id: number; name: string; description: string; url: string; previewUrl?: string; downloadUrl?: string; duration: number; filesize: number; type: string; channels: number; bitrate: number; bitdepth: number; samplerate: number; username: string; tags: string[]; license: string; created: string; downloads: number; rating: number; ratingCount: number; -} +};Repeat for SavedSound and SavedSoundsData.
Please confirm the codebase preference here; if interfaces are standard across the repo, feel free to ignore. As per coding guidelines.
apps/web/src/lib/video-cache.ts (2)
74-75: Replace console. with the project’s logger or remove.*Console usage is disallowed by guidelines; switch to your logging utility or surface via return values/events.
As per coding guidelines.
Also applies to: 100-101, 151-153
46-47: Optional: clamp out-of-bounds seeks.If time can exceed media duration, seekToTime may return null. Consider clamping time to [0, duration] if track duration is known.
Confirm whether upstream callers guarantee time within bounds; if not, clamping here improves robustness.
apps/web/src/lib/export.ts (3)
51-53: Avoidanyby declaring WebKit prefix in global types.Add a .d.ts to type webkitAudioContext and drop the
anycast:global.d.ts:
declare global { interface Window { webkitAudioContext?: typeof AudioContext; } } export {};Then the diff above (using window.webkitAudioContext) compiles cleanly.
As per coding guidelines.
89-89: Replace console. with a logger.*Use your logging utility instead of console.warn/error.
As per coding guidelines.
Also applies to: 288-288
139-144: Optional: prevent clipping when mixing samples.Straight summation can clip. Consider soft limiting or clamping per sample:
- outputData[outputIndex] += sourceData[sourceIndex]; + const mixed = outputData[outputIndex] + sourceData[sourceIndex]; + outputData[outputIndex] = Math.max(-1, Math.min(1, mixed));Or normalize after mixing for better performance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (41)
apps/web/migrations/meta/0003_snapshot.json(6 hunks)apps/web/migrations/meta/_journal.json(1 hunks)apps/web/src/app/api/get-upload-url/route.ts(1 hunks)apps/web/src/app/api/sounds/search/route.ts(1 hunks)apps/web/src/app/api/transcribe/route.ts(1 hunks)apps/web/src/app/api/waitlist/export/route.ts(1 hunks)apps/web/src/components/editor/export-button.tsx(1 hunks)apps/web/src/components/editor/layout-guide-overlay.tsx(1 hunks)apps/web/src/components/editor/media-panel/tabbar.tsx(3 hunks)apps/web/src/components/editor/media-panel/views/captions.tsx(1 hunks)apps/web/src/components/editor/panel-base-view.tsx(1 hunks)apps/web/src/components/editor/panel-preset-selector.tsx(1 hunks)apps/web/src/components/editor/timeline/timeline-marker.tsx(1 hunks)apps/web/src/components/footer.tsx(1 hunks)apps/web/src/components/icons.tsx(2 hunks)apps/web/src/components/keyboard-shortcuts-help.tsx(1 hunks)apps/web/src/components/language-select.tsx(1 hunks)apps/web/src/components/theme-toggle.tsx(1 hunks)apps/web/src/components/ui/editable-timecode.tsx(1 hunks)apps/web/src/components/ui/font-picker.tsx(1 hunks)apps/web/src/components/ui/input-with-back.tsx(1 hunks)apps/web/src/components/ui/input.tsx(1 hunks)apps/web/src/components/ui/split-button.tsx(1 hunks)apps/web/src/components/ui/tooltip.tsx(4 hunks)apps/web/src/constants/text-constants.ts(1 hunks)apps/web/src/hooks/use-edge-auto-scroll.ts(1 hunks)apps/web/src/hooks/use-highlight-scroll.ts(1 hunks)apps/web/src/hooks/use-infinite-scroll.ts(1 hunks)apps/web/src/hooks/use-sound-search.ts(1 hunks)apps/web/src/lib/editor-utils.ts(1 hunks)apps/web/src/lib/export.ts(1 hunks)apps/web/src/lib/iconify-api.ts(0 hunks)apps/web/src/lib/schemas/waitlist.ts(1 hunks)apps/web/src/lib/transcription-utils.ts(1 hunks)apps/web/src/lib/video-cache.ts(1 hunks)apps/web/src/lib/zk-encryption.ts(1 hunks)apps/web/src/stores/playback-store.ts(1 hunks)apps/web/src/stores/sounds-store.ts(1 hunks)apps/web/src/stores/text-properties-store.ts(1 hunks)apps/web/src/types/media.ts(1 hunks)apps/web/src/types/sounds.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/src/lib/iconify-api.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use TypeScript namespaces.
Don't use non-null assertions with the!postfix operator.
Don't use parameter properties in class constructors.
Don't use user-defined types.
Useas constinstead of literal types and type annotations.
Use eitherT[]orArray<T>consistently.
Initialize each enum member value explicitly.
Useexport typefor types.
Useimport typefor types.
Make sure all enum members are literal values.
Don't use TypeScript const enum.
Don't declare empty interfaces.
Don't let variables evolve into any type through reassignments.
Don't use the any type.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use implicit any type on variable declarations.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use the TypeScript directive @ts-ignore.
Use consistent accessibility modifiers on class properties and methods.
Use function types instead of object types with call signatures.
Don't use void type outside of generic or return types.
**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Use either T[] or Array consistently
Don't use the any type
Files:
apps/web/src/types/media.tsapps/web/src/app/api/transcribe/route.tsapps/web/src/components/icons.tsxapps/web/src/components/footer.tsxapps/web/src/stores/text-properties-store.tsapps/web/src/stores/playback-store.tsapps/web/src/components/editor/panel-preset-selector.tsxapps/web/src/hooks/use-infinite-scroll.tsapps/web/src/types/sounds.tsapps/web/src/lib/schemas/waitlist.tsapps/web/src/components/ui/input-with-back.tsxapps/web/src/components/editor/media-panel/views/captions.tsxapps/web/src/hooks/use-sound-search.tsapps/web/src/components/ui/split-button.tsxapps/web/src/lib/zk-encryption.tsapps/web/src/components/ui/font-picker.tsxapps/web/src/hooks/use-highlight-scroll.tsapps/web/src/constants/text-constants.tsapps/web/src/components/ui/editable-timecode.tsxapps/web/src/lib/video-cache.tsapps/web/src/app/api/sounds/search/route.tsapps/web/src/lib/transcription-utils.tsapps/web/src/components/keyboard-shortcuts-help.tsxapps/web/src/components/editor/layout-guide-overlay.tsxapps/web/src/app/api/waitlist/export/route.tsapps/web/src/hooks/use-edge-auto-scroll.tsapps/web/src/components/editor/export-button.tsxapps/web/src/components/ui/input.tsxapps/web/src/components/theme-toggle.tsxapps/web/src/lib/export.tsapps/web/src/components/editor/panel-base-view.tsxapps/web/src/app/api/get-upload-url/route.tsapps/web/src/components/ui/tooltip.tsxapps/web/src/lib/editor-utils.tsapps/web/src/components/editor/media-panel/tabbar.tsxapps/web/src/components/editor/timeline/timeline-marker.tsxapps/web/src/stores/sounds-store.tsapps/web/src/components/language-select.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,ts,tsx}: Don't use the return value of React.render.
Don't use consecutive spaces in regular expression literals.
Don't use theargumentsobject.
Don't use primitive type aliases or misleading types.
Don't use the comma operator.
Don't write functions that exceed a given Cognitive Complexity score.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use uselessthisaliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names that aren't base 10 or use underscore separators.
Remove redundant terms from logical expressions.
Use while loops instead of...
Files:
apps/web/src/types/media.tsapps/web/src/app/api/transcribe/route.tsapps/web/src/components/icons.tsxapps/web/src/components/footer.tsxapps/web/src/stores/text-properties-store.tsapps/web/src/stores/playback-store.tsapps/web/src/components/editor/panel-preset-selector.tsxapps/web/src/hooks/use-infinite-scroll.tsapps/web/src/types/sounds.tsapps/web/src/lib/schemas/waitlist.tsapps/web/src/components/ui/input-with-back.tsxapps/web/src/components/editor/media-panel/views/captions.tsxapps/web/src/hooks/use-sound-search.tsapps/web/src/components/ui/split-button.tsxapps/web/src/lib/zk-encryption.tsapps/web/src/components/ui/font-picker.tsxapps/web/src/hooks/use-highlight-scroll.tsapps/web/src/constants/text-constants.tsapps/web/src/components/ui/editable-timecode.tsxapps/web/src/lib/video-cache.tsapps/web/src/app/api/sounds/search/route.tsapps/web/src/lib/transcription-utils.tsapps/web/src/components/keyboard-shortcuts-help.tsxapps/web/src/components/editor/layout-guide-overlay.tsxapps/web/src/app/api/waitlist/export/route.tsapps/web/src/hooks/use-edge-auto-scroll.tsapps/web/src/components/editor/export-button.tsxapps/web/src/components/ui/input.tsxapps/web/src/components/theme-toggle.tsxapps/web/src/lib/export.tsapps/web/src/components/editor/panel-base-view.tsxapps/web/src/app/api/get-upload-url/route.tsapps/web/src/components/ui/tooltip.tsxapps/web/src/lib/editor-utils.tsapps/web/src/components/editor/media-panel/tabbar.tsxapps/web/src/components/editor/timeline/timeline-marker.tsxapps/web/src/stores/sounds-store.tsapps/web/src/components/language-select.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{ts,tsx,js,jsx}: Don't use the comma operator
Use for...of statements instead of Array.forEach
Don't initialize variables to undefined
Use .flatMap() instead of map().flat() when possible
Don't assign a value to itself
Avoid unused imports and variables
Don't use await inside loops
Don't hardcode sensitive data like API keys and tokens
Don't use the delete operator
Don't use global eval()
Use String.slice() instead of String.substr() and String.substring()
Don't use else blocks when the if block breaks early
Put default function parameters and optional function parameters last
Use new when throwing an error
Use String.trimStart() and String.trimEnd() over String.trimLeft() and String.trimRight()
Files:
apps/web/src/types/media.tsapps/web/src/app/api/transcribe/route.tsapps/web/src/components/icons.tsxapps/web/src/components/footer.tsxapps/web/src/stores/text-properties-store.tsapps/web/src/stores/playback-store.tsapps/web/src/components/editor/panel-preset-selector.tsxapps/web/src/hooks/use-infinite-scroll.tsapps/web/src/types/sounds.tsapps/web/src/lib/schemas/waitlist.tsapps/web/src/components/ui/input-with-back.tsxapps/web/src/components/editor/media-panel/views/captions.tsxapps/web/src/hooks/use-sound-search.tsapps/web/src/components/ui/split-button.tsxapps/web/src/lib/zk-encryption.tsapps/web/src/components/ui/font-picker.tsxapps/web/src/hooks/use-highlight-scroll.tsapps/web/src/constants/text-constants.tsapps/web/src/components/ui/editable-timecode.tsxapps/web/src/lib/video-cache.tsapps/web/src/app/api/sounds/search/route.tsapps/web/src/lib/transcription-utils.tsapps/web/src/components/keyboard-shortcuts-help.tsxapps/web/src/components/editor/layout-guide-overlay.tsxapps/web/src/app/api/waitlist/export/route.tsapps/web/src/hooks/use-edge-auto-scroll.tsapps/web/src/components/editor/export-button.tsxapps/web/src/components/ui/input.tsxapps/web/src/components/theme-toggle.tsxapps/web/src/lib/export.tsapps/web/src/components/editor/panel-base-view.tsxapps/web/src/app/api/get-upload-url/route.tsapps/web/src/components/ui/tooltip.tsxapps/web/src/lib/editor-utils.tsapps/web/src/components/editor/media-panel/tabbar.tsxapps/web/src/components/editor/timeline/timeline-marker.tsxapps/web/src/stores/sounds-store.tsapps/web/src/components/language-select.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{jsx,tsx}: Don't useaccessKeyattribute on any HTML element.
Don't setaria-hidden="true"on focusable elements.
Don't add ARIA roles, states, and properties to elements that don't support them.
Don't use distracting elements like<marquee>or<blink>.
Only use thescopeprop on<th>elements.
Don't assign non-interactive ARIA roles to interactive HTML elements.
Make sure label elements have text content and are associated with an input.
Don't assign interactive ARIA roles to non-interactive HTML elements.
Don't assigntabIndexto non-interactive HTML elements.
Don't use positive integers fortabIndexproperty.
Don't include "image", "picture", or "photo" in img alt prop.
Don't use explicit role property that's the same as the implicit/default role.
Make static elements with click handlers use a valid role attribute.
Always include atitleelement for SVG elements.
Give all elements requiring alt text meaningful information for screen readers.
Make sure anchors have content that's accessible to screen readers.
AssigntabIndexto non-interactive HTML elements witharia-activedescendant.
Include all required ARIA attributes for elements with ARIA roles.
Make sure ARIA properties are valid for the element's supported roles.
Always include atypeattribute for button elements.
Make elements with interactive roles and handlers focusable.
Give heading elements content that's accessible to screen readers (not hidden witharia-hidden).
Always include alangattribute on the html element.
Always include atitleattribute for iframe elements.
AccompanyonClickwith at least one of:onKeyUp,onKeyDown, oronKeyPress.
AccompanyonMouseOver/onMouseOutwithonFocus/onBlur.
Include caption tracks for audio and video elements.
Use semantic elements instead of role attributes in JSX.
Make sure all anchors are valid and navigable.
Ensure all ARIA properties (aria-*) are valid.
Use valid, non-abstract ARIA roles for elements with...
Files:
apps/web/src/components/icons.tsxapps/web/src/components/footer.tsxapps/web/src/components/editor/panel-preset-selector.tsxapps/web/src/components/ui/input-with-back.tsxapps/web/src/components/editor/media-panel/views/captions.tsxapps/web/src/components/ui/split-button.tsxapps/web/src/components/ui/font-picker.tsxapps/web/src/components/ui/editable-timecode.tsxapps/web/src/components/keyboard-shortcuts-help.tsxapps/web/src/components/editor/layout-guide-overlay.tsxapps/web/src/components/editor/export-button.tsxapps/web/src/components/ui/input.tsxapps/web/src/components/theme-toggle.tsxapps/web/src/components/editor/panel-base-view.tsxapps/web/src/components/ui/tooltip.tsxapps/web/src/components/editor/media-panel/tabbar.tsxapps/web/src/components/editor/timeline/timeline-marker.tsxapps/web/src/components/language-select.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{tsx,jsx}: Always include a title element for icons unless there's text beside the icon
Always include a type attribute for button elements
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress
Accompany onMouseOver/onMouseOut with onFocus/onBlur
Don't import React itself
Don't define React components inside other components
Don't use both children and dangerouslySetInnerHTML on the same element
Don't insert comments as text nodes
Use <>...</> instead of ...
Files:
apps/web/src/components/icons.tsxapps/web/src/components/footer.tsxapps/web/src/components/editor/panel-preset-selector.tsxapps/web/src/components/ui/input-with-back.tsxapps/web/src/components/editor/media-panel/views/captions.tsxapps/web/src/components/ui/split-button.tsxapps/web/src/components/ui/font-picker.tsxapps/web/src/components/ui/editable-timecode.tsxapps/web/src/components/keyboard-shortcuts-help.tsxapps/web/src/components/editor/layout-guide-overlay.tsxapps/web/src/components/editor/export-button.tsxapps/web/src/components/ui/input.tsxapps/web/src/components/theme-toggle.tsxapps/web/src/components/editor/panel-base-view.tsxapps/web/src/components/ui/tooltip.tsxapps/web/src/components/editor/media-panel/tabbar.tsxapps/web/src/components/editor/timeline/timeline-marker.tsxapps/web/src/components/language-select.tsx
🧠 Learnings (7)
📚 Learning: 2025-08-09T09:03:49.797Z
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-09T09:03:49.797Z
Learning: Applies to **/*.{jsx,tsx} : Don't use `<img>` elements in Next.js projects.
Applied to files:
apps/web/src/components/footer.tsx
📚 Learning: 2025-08-09T09:03:49.797Z
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-09T09:03:49.797Z
Learning: Applies to **/*.{ts,tsx} : Use `export type` for types.
Applied to files:
apps/web/src/stores/text-properties-store.ts
📚 Learning: 2025-08-09T09:03:49.797Z
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-09T09:03:49.797Z
Learning: Applies to **/*.{jsx,tsx} : Include caption tracks for audio and video elements.
Applied to files:
apps/web/src/components/editor/media-panel/views/captions.tsx
📚 Learning: 2025-08-09T09:03:49.797Z
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-09T09:03:49.797Z
Learning: Applies to **/*.{jsx,tsx} : Use a recommended display strategy with Google Fonts.
Applied to files:
apps/web/src/components/ui/font-picker.tsx
📚 Learning: 2025-08-09T09:03:49.797Z
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-09T09:03:49.797Z
Learning: Applies to **/*.{ts,tsx} : Use `as const` instead of literal types and type annotations.
Applied to files:
apps/web/src/constants/text-constants.ts
📚 Learning: 2025-08-09T09:03:49.797Z
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-09T09:03:49.797Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use standard constants instead of approximated literals.
Applied to files:
apps/web/src/constants/text-constants.ts
📚 Learning: 2025-08-09T09:03:49.797Z
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-09T09:03:49.797Z
Learning: Applies to **/*.{jsx,tsx} : Make sure label elements have text content and are associated with an input.
Applied to files:
apps/web/src/components/ui/input.tsx
🧬 Code graph analysis (20)
apps/web/src/app/api/transcribe/route.ts (3)
apps/web/src/lib/rate-limit.ts (1)
baseRateLimit(11-16)apps/web/src/lib/transcription-utils.ts (1)
isTranscriptionConfigured(3-13)apps/web/src/env.ts (1)
env(7-45)
apps/web/src/stores/playback-store.ts (1)
apps/web/src/stores/project-store.ts (1)
DEFAULT_FPS(12-12)
apps/web/src/components/editor/panel-preset-selector.tsx (1)
apps/web/src/stores/panel-store.ts (2)
PanelPreset(4-8)usePanelStore(68-225)
apps/web/src/hooks/use-sound-search.ts (1)
apps/web/src/stores/sounds-store.ts (1)
useSoundsStore(74-282)
apps/web/src/components/ui/split-button.tsx (2)
apps/web/src/components/ui/button.tsx (2)
ButtonProps(41-45)Button(61-61)apps/web/src/lib/utils.ts (1)
cn(6-8)
apps/web/src/components/ui/font-picker.tsx (1)
apps/web/src/components/ui/select.tsx (1)
SelectTrigger(171-171)
apps/web/src/constants/text-constants.ts (2)
apps/web/src/types/timeline.ts (1)
TextElement(25-40)apps/web/src/constants/timeline-constants.ts (1)
TIMELINE_CONSTANTS(72-79)
apps/web/src/components/ui/editable-timecode.tsx (3)
apps/web/src/lib/time.ts (3)
TimeCode(5-5)formatTimeCode(8-29)parseTimeCode(31-122)apps/web/src/stores/project-store.ts (1)
DEFAULT_FPS(12-12)apps/web/src/lib/utils.ts (1)
cn(6-8)
apps/web/src/app/api/sounds/search/route.ts (2)
apps/web/src/lib/rate-limit.ts (1)
baseRateLimit(11-16)apps/web/src/env.ts (1)
env(7-45)
apps/web/src/lib/transcription-utils.ts (1)
apps/web/src/env.ts (1)
env(7-45)
apps/web/src/components/editor/layout-guide-overlay.tsx (1)
apps/web/src/stores/editor-store.ts (1)
useEditorStore(39-91)
apps/web/src/app/api/waitlist/export/route.ts (4)
apps/web/src/lib/schemas/waitlist.ts (2)
exportWaitlistSchema(3-5)exportWaitlistResponseSchema(7-10)apps/web/src/lib/rate-limit.ts (1)
baseRateLimit(11-16)packages/db/src/index.ts (2)
db(20-20)eq(27-27)packages/db/src/schema.ts (1)
exportWaitlist(61-70)
apps/web/src/components/ui/input.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(6-8)
apps/web/src/components/editor/panel-base-view.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(6-8)
apps/web/src/app/api/get-upload-url/route.ts (4)
apps/web/src/app/api/transcribe/route.ts (1)
POST(52-189)apps/web/src/lib/rate-limit.ts (1)
baseRateLimit(11-16)apps/web/src/lib/transcription-utils.ts (1)
isTranscriptionConfigured(3-13)apps/web/src/env.ts (1)
env(7-45)
apps/web/src/lib/editor-utils.ts (1)
apps/web/src/types/editor.ts (1)
CanvasSize(5-8)
apps/web/src/components/editor/media-panel/tabbar.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(6-8)
apps/web/src/components/editor/timeline/timeline-marker.tsx (2)
apps/web/src/lib/utils.ts (1)
cn(6-8)apps/web/src/constants/timeline-constants.ts (1)
TIMELINE_CONSTANTS(72-79)
apps/web/src/stores/sounds-store.ts (3)
apps/web/src/types/sounds.ts (2)
SoundEffect(1-22)SavedSound(24-34)apps/web/src/lib/storage/storage-service.ts (4)
storageService(479-479)isSoundSaved(445-453)removeSavedSound(429-443)saveSoundEffect(392-427)apps/web/src/stores/timeline-store.ts (1)
useTimelineStore(253-1853)
apps/web/src/components/language-select.tsx (1)
apps/web/src/components/editor/media-panel/views/captions.tsx (1)
languages(21-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vercel Agent Review
🔇 Additional comments (12)
apps/web/src/components/keyboard-shortcuts-help.tsx (1)
245-245: LGTMFormatting-only newline at EOF; no functional changes.
apps/web/src/components/ui/font-picker.tsx (1)
23-25: LGTMFormatting-only change; behavior unchanged.
apps/web/src/types/media.ts (1)
1-17: LGTM.Types are clear and appropriate for client-side media handling.
apps/web/src/lib/zk-encryption.ts (1)
15-47: LGTM: AES‑GCM with 256‑bit key and 96‑bit IV is correct.API and semantics look solid; helpers are straightforward.
apps/web/src/components/footer.tsx (1)
22-28: LGTM on Next/Image usageAlt text, width/height, and className are set correctly; no functional changes.
apps/web/src/components/language-select.tsx (1)
118-139: LGTM on structure and a11y basics.Buttons have type, icons are paired with visible labels, and focus handling is present.
Also applies to: 171-204
apps/web/src/stores/sounds-store.ts (3)
227-281: Overall flow looks solid.Good UX: toasts, optimistic media add, timeline integration, and error propagation.
Consider adding an AbortController + timeout to
fetch(audioUrl)to avoid hanging requests.
211-225: storageService.clearSavedSounds exists Verified implementation in apps/web/src/lib/storage/storage-service.ts (line 455); no action required.
250-261: MediaStore revokes object URLs on removal MediaStore’s removal and clear handlers callURL.revokeObjectURLfor all storedurlandthumbnailUrl, preventing memory leaks.apps/web/src/types/sounds.ts (1)
1-39: LGTM — structure unchanged.Types/interfaces remain consistent; this looks like formatting-only.
apps/web/migrations/meta/_journal.json (1)
34-34: EOF newline normalization — OK.Trailing newline addition is benign and improves POSIX tooling compatibility.
apps/web/migrations/meta/0003_snapshot.json (1)
96-99: Formatting-only normalization — OK.Array fields compacted to single-line and EOF newline normalized. No semantic changes.
Also applies to: 144-145, 210-212, 221-222, 283-284, 351-351
| // Check transcription configuration | ||
| const transcriptionCheck = isTranscriptionConfigured(); | ||
| if (!transcriptionCheck.configured) { | ||
| console.error( | ||
| "Missing environment variables:", | ||
| JSON.stringify(transcriptionCheck.missingVars) | ||
| ); | ||
|
|
||
| return NextResponse.json( | ||
| { | ||
| error: "Transcription not configured", | ||
| message: `Auto-captions require environment variables: ${transcriptionCheck.missingVars.join(", ")}. Check README for setup instructions.`, | ||
| }, | ||
| { status: 503 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
Wrong configuration guard: this endpoint checks transcription config instead of storage config
This route only needs R2/Cloudflare vars. Using isTranscriptionConfigured() can 503 the endpoint when MODAL_TRANSCRIPTION_URL is missing, even though uploads don’t require it.
Apply:
- // Check transcription configuration
- const transcriptionCheck = isTranscriptionConfigured();
- if (!transcriptionCheck.configured) {
- console.error(
- "Missing environment variables:",
- JSON.stringify(transcriptionCheck.missingVars)
- );
-
- return NextResponse.json(
- {
- error: "Transcription not configured",
- message: `Auto-captions require environment variables: ${transcriptionCheck.missingVars.join(", ")}. Check README for setup instructions.`,
- },
- { status: 503 }
- );
- }
+ // Check R2 storage configuration
+ const missingVars: string[] = [];
+ if (!env.CLOUDFLARE_ACCOUNT_ID) missingVars.push("CLOUDFLARE_ACCOUNT_ID");
+ if (!env.R2_ACCESS_KEY_ID) missingVars.push("R2_ACCESS_KEY_ID");
+ if (!env.R2_SECRET_ACCESS_KEY) missingVars.push("R2_SECRET_ACCESS_KEY");
+ if (!env.R2_BUCKET_NAME) missingVars.push("R2_BUCKET_NAME");
+ if (missingVars.length > 0) {
+ console.error("Missing environment variables:", JSON.stringify(missingVars));
+ return NextResponse.json(
+ {
+ error: "Upload storage not configured",
+ message: `Uploads require environment variables: ${missingVars.join(", ")}.`,
+ },
+ { status: 503 }
+ );
+ }📝 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.
| // Check transcription configuration | |
| const transcriptionCheck = isTranscriptionConfigured(); | |
| if (!transcriptionCheck.configured) { | |
| console.error( | |
| "Missing environment variables:", | |
| JSON.stringify(transcriptionCheck.missingVars) | |
| ); | |
| return NextResponse.json( | |
| { | |
| error: "Transcription not configured", | |
| message: `Auto-captions require environment variables: ${transcriptionCheck.missingVars.join(", ")}. Check README for setup instructions.`, | |
| }, | |
| { status: 503 } | |
| ); | |
| } | |
| // Check R2 storage configuration | |
| const missingVars: string[] = []; | |
| if (!env.CLOUDFLARE_ACCOUNT_ID) missingVars.push("CLOUDFLARE_ACCOUNT_ID"); | |
| if (!env.R2_ACCESS_KEY_ID) missingVars.push("R2_ACCESS_KEY_ID"); | |
| if (!env.R2_SECRET_ACCESS_KEY) missingVars.push("R2_SECRET_ACCESS_KEY"); | |
| if (!env.R2_BUCKET_NAME) missingVars.push("R2_BUCKET_NAME"); | |
| if (missingVars.length > 0) { | |
| console.error("Missing environment variables:", JSON.stringify(missingVars)); | |
| return NextResponse.json( | |
| { | |
| error: "Upload storage not configured", | |
| message: `Uploads require environment variables: ${missingVars.join(", ")}.`, | |
| }, | |
| { status: 503 } | |
| ); | |
| } |
🤖 Prompt for AI Agents
In apps/web/src/app/api/get-upload-url/route.ts around lines 32 to 47, the route
currently calls isTranscriptionConfigured() which can incorrectly 503 the
endpoint when transcription variables are missing; change the guard to use the
storage configuration checker (e.g., isStorageConfigured()) so only
R2/Cloudflare storage vars are validated, log the missing storage vars via
console.error(JSON.stringify(...)) and return a 503 NextResponse.json with an
error and a message that lists the missing storage vars (not transcription vars)
and points to README setup instructions.
| const url = new URL( | ||
| `https://${env.R2_BUCKET_NAME}.${env.CLOUDFLARE_ACCOUNT_ID}.r2.cloudflarestorage.com/${fileName}` | ||
| ); | ||
|
|
||
| url.searchParams.set("X-Amz-Expires", "3600"); // 1 hour expiry | ||
|
|
||
| const signed = await client.sign(new Request(url, { method: "PUT" }), { | ||
| aws: { signQuery: true }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify R2 endpoint style to prevent signature mismatch
Ensure the host/path style matches Cloudflare R2 S3 signing rules. Many setups use https://<ACCOUNT_ID>.r2.cloudflarestorage.com/<BUCKET>/<KEY> rather than <BUCKET>.<ACCOUNT_ID>.../<KEY>. If incorrect, presigned PUTs will 403.
Run to compare usage patterns in-repo:
If we’re mixing styles across endpoints, standardize to one that matches our bucket configuration (virtual-hosted vs path-style) and test a PUT with the presigned URL.
🏁 Script executed:
#!/bin/bash
rg -n -C2 'r2\.cloudflarestorage\.com' | sed -n '1,200p'Length of output: 891
Switch to path-style R2 endpoint for presigned URLs
Replace the virtual-hosted URL in apps/web/src/app/api/get-upload-url/route.ts (lines 82–84):
// from:
new URL(
`https://${env.R2_BUCKET_NAME}.${env.CLOUDFLARE_ACCOUNT_ID}.r2.cloudflarestorage.com/${fileName}`
);
// to:
new URL(
`https://${env.CLOUDFLARE_ACCOUNT_ID}.r2.cloudflarestorage.com/${env.R2_BUCKET_NAME}/${fileName}`
);This matches your Python usage and Cloudflare R2’s signing rules to avoid 403s.
🤖 Prompt for AI Agents
In apps/web/src/app/api/get-upload-url/route.ts around lines 82 to 90, the code
builds a virtual-hosted–style R2 URL using the bucket as a subdomain which can
cause 403s when signing; change it to path-style by constructing the URL with
the account host then appending the bucket and filename (i.e., host:
${env.CLOUDFLARE_ACCOUNT_ID}.r2.cloudflarestorage.com and path:
/${env.R2_BUCKET_NAME}/${fileName}), keep the same query param and signing call
so the presigned PUT uses the path-style endpoint.
| if (commercial_only) { | ||
| params.append( | ||
| "filter", | ||
| 'license:("Attribution" OR "Creative Commons 0" OR "Attribution Noncommercial" OR "Attribution Commercial")' | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
Commercial-only filter includes noncommercial license; fix license query
When commercial_only is true, exclude noncommercial licenses. Freesound’s commercial-friendly licenses are “Creative Commons 0” and “Attribution”.
As per coding guidelines
- if (commercial_only) {
- params.append(
- "filter",
- 'license:("Attribution" OR "Creative Commons 0" OR "Attribution Noncommercial" OR "Attribution Commercial")'
- );
- }
+ if (commercial_only) {
+ // Only include licenses that permit commercial use
+ params.append("filter", 'license:("Creative Commons 0" OR "Attribution")');
+ }Please verify Freesound’s license values and their commercial allowances:
🌐 Web query:
What license values does Freesound use (e.g., "Creative Commons 0", "Attribution", "Attribution Noncommercial") and which allow commercial use?
💡 Result:
Short answer
- Freesound uses: CC0 (Public Domain), CC BY (Attribution), CC BY-NC (Attribution-NonCommercial), plus a small number of legacy CC Sampling+ uploads. [1][2]
Which allow commercial use
- Allow commercial use: CC0 and CC BY. (Standard CC BY-SA and CC BY-ND also permit commercial use, but they are not the primary current Freesound licenses.) [2][3]
- Do NOT allow commercial use: CC BY-NC, CC BY-NC-SA, CC BY-NC-ND (the “NC” variants). Sampling+ is a legacy Freesound license that restricts commercial redistribution of unmodified samples. [1][2][3]
References
- Freesound blog / site licensing notes. [1]
- Freesound (Wikipedia summary of Freesound’s licensing history). [2]
- Creative Commons license descriptions (which components allow or forbid commercial use). [3]
Restrict commercial_only filter to CC0 and CC BY
apps/web/src/app/api/sounds/search/route.ts lines 167–171
- if (commercial_only) {
- params.append(
- "filter",
- 'license:("Attribution" OR "Creative Commons 0" OR "Attribution Noncommercial" OR "Attribution Commercial")'
- );
- }
+ if (commercial_only) {
+ // only include licenses permitting commercial use
+ params.append("filter", 'license:("Creative Commons 0" OR "Attribution")');
+ }🤖 Prompt for AI Agents
In apps/web/src/app/api/sounds/search/route.ts around lines 167 to 171, the
commercial_only branch currently allows multiple licenses but should be
restricted to only CC0 and CC BY; replace the existing filter value with one
that only includes "Creative Commons 0" and "Attribution" (CC BY) so the
params.append("filter", ...) call only filters for those two license strings.
| const modalRequestBody: any = { | ||
| filename, | ||
| language, | ||
| }; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove any and define a concrete payload type.
Avoids unsafe typing and matches our TS rules.
As per coding guidelines
- const modalRequestBody: any = {
+ const modalRequestBody: {
+ filename: string;
+ language?: string;
+ decryptionKey?: string;
+ iv?: string;
+ } = {
filename,
language,
};📝 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 modalRequestBody: any = { | |
| filename, | |
| language, | |
| }; | |
| const modalRequestBody: { | |
| filename: string; | |
| language?: string; | |
| decryptionKey?: string; | |
| iv?: string; | |
| } = { | |
| filename, | |
| language, | |
| }; |
🤖 Prompt for AI Agents
In apps/web/src/app/api/transcribe/route.ts around lines 103–107,
modalRequestBody is typed as any; define a concrete payload type (e.g.,
interface TranscribeRequest { filename: string; language: string; }) and replace
the any with that type. Update the modalRequestBody declaration to use the new
type, ensure any optional/nullable fields are reflected, and adjust any
downstream usages/signatures to accept the typed object.
| import { NextRequest, NextResponse } from "next/server"; | ||
| import { baseRateLimit } from "@/lib/rate-limit"; | ||
| import { db, exportWaitlist, eq } from "@opencut/db"; | ||
| import { randomUUID } from "crypto"; | ||
| import { | ||
| exportWaitlistSchema, | ||
| exportWaitlistResponseSchema, | ||
| } from "@/lib/schemas/waitlist"; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that eq is exported from @opencut/db.
If not re-exported, this import will fail at build.
🏁 Script executed:
#!/bin/bash
# Check if `eq` is re-exported from @opencut/db
rg -nP -C2 '\bexport\s+\*\s+from\s+"drizzle-orm"' packages
rg -nP -C2 '\bexport\s+{[^}]*\beq\b[^}]*}' packages
rg -nP -C2 '\bfrom\s+"@opencut/db"' apps | sed -n '1,200p'Length of output: 446
Export eq from @opencut/db or import it directly from drizzle-orm. @opencut/db does not currently re-export eq, so the import in route.ts will fail at build.
🤖 Prompt for AI Agents
In apps/web/src/app/api/waitlist/export/route.ts around lines 1 to 8, the import
list includes eq from @opencut/db which does not re-export it; update the import
to either remove eq from @opencut/db and import eq directly from "drizzle-orm"
(e.g., add an import { eq } from "drizzle-orm") or alternatively add eq to the
exports of the @opencut/db package and then import it from there; modify the
top-of-file imports accordingly so the build no longer fails.
| export function findBestCanvasPreset(aspectRatio: number): CanvasSize { | ||
| // Calculate aspect ratio for each preset and find the closest match | ||
| let bestMatch = DEFAULT_CANVAS_PRESETS[0]; // Default to 16:9 HD | ||
| let smallestDifference = Math.abs( | ||
| aspectRatio - bestMatch.width / bestMatch.height | ||
| ); | ||
|
|
There was a problem hiding this comment.
Guard invalid aspect ratios to avoid divide-by-zero/Infinity
aspectRatio can be 0/negative/NaN; 1920 / aspectRatio would yield Infinity/NaN and break layout.
Apply:
export function findBestCanvasPreset(aspectRatio: number): CanvasSize {
- // Calculate aspect ratio for each preset and find the closest match
- let bestMatch = DEFAULT_CANVAS_PRESETS[0]; // Default to 16:9 HD
+ // Guard invalid inputs to avoid divide-by-zero/Infinity and pick a sane default
+ const DEFAULT_PRESET = DEFAULT_CANVAS_PRESETS[0]; // 16:9 HD
+ if (!Number.isFinite(aspectRatio) || aspectRatio <= 0) {
+ return { width: DEFAULT_PRESET.width, height: DEFAULT_PRESET.height };
+ }
+ // Calculate aspect ratio for each preset and find the closest match
+ let bestMatch = DEFAULT_PRESET;
let smallestDifference = Math.abs(
aspectRatio - bestMatch.width / bestMatch.height
);Also applies to: 32-45
🤖 Prompt for AI Agents
In apps/web/src/lib/editor-utils.ts around lines 15-21 (and also apply the same
guard to lines 32-45): validate the incoming aspectRatio before using it in
divisions; if aspectRatio is NaN, not finite, or <= 0, immediately fall back to
the default preset (e.g., DEFAULT_CANVAS_PRESETS[0]) or a safe default aspect
ratio and return that preset instead of performing calculations that use
1/aspectRatio or width/aspectRatio; update the subsequent matching logic to
assume a validated positive finite aspectRatio so no divide-by-zero/Infinity
occurs.
| const audioContext = new ( | ||
| window.AudioContext || (window as any).webkitAudioContext | ||
| )(); |
There was a problem hiding this comment.
Close or reuse AudioContext to avoid resource leaks / context limit errors.
A new AudioContext per export without closing can hit browser limits and leak memory. Either reuse a singleton or close it before returning.
Targeted minimal changes:
- const audioContext = new (
- window.AudioContext || (window as any).webkitAudioContext
- )();
+ const AudioCtx = window.AudioContext || window.webkitAudioContext;
+ const audioContext = new AudioCtx();Also add closes:
@@
- if (audioElements.length === 0) {
- return null; // No audio to mix
- }
+ if (audioElements.length === 0) {
+ await audioContext.close();
+ return null; // No audio to mix
+ }
@@
- return outputBuffer;
+ await audioContext.close();
+ return outputBuffer;If you prefer a singleton:
- Create a module-level getAudioContext() returning a shared instance and close it when the app tears down.
📝 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 audioContext = new ( | |
| window.AudioContext || (window as any).webkitAudioContext | |
| )(); | |
| // --- original creation site (around line 51) --- | |
| - const audioContext = new ( | |
| - window.AudioContext || (window as any).webkitAudioContext | |
| const AudioCtx = window.AudioContext || window.webkitAudioContext; | |
| const audioContext = new AudioCtx(); | |
| // … (code that fills `audioElements`) … | |
| // --- early-exit when there’s nothing to mix --- | |
| - if (audioElements.length === 0) { | |
| - return null; // No audio to mix | |
| if (audioElements.length === 0) { | |
| await audioContext.close(); | |
| return null; // No audio to mix | |
| } | |
| // … (mixing logic, building `outputBuffer`) … | |
| // --- final return site --- | |
| await audioContext.close(); | |
| return outputBuffer; |
🤖 Prompt for AI Agents
In apps/web/src/lib/export.ts around lines 51-53, the code instantiates a new
AudioContext for each export which can leak resources and hit browser context
limits; update the file to either (A) implement a module-level getAudioContext()
that returns a shared singleton (create it lazily using window.AudioContext ||
window.webkitAudioContext) and use that singleton for exports, or (B) if you
must create a local context, ensure you call audioContext.close() in a finally
block (or after the work completes) to release resources; pick one approach,
update callers to use the shared getter if using the singleton, and add
teardown/close logic when the app or export finishes.
| export function isTranscriptionConfigured() { | ||
| const missingVars = []; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Avoid implicit any[] for missingVars
Empty array defaults to any[]. Annotate to comply with TS rules.
Apply:
export function isTranscriptionConfigured() {
- const missingVars = [];
+ const missingVars: string[] = [];As per coding guidelines
📝 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 isTranscriptionConfigured() { | |
| const missingVars = []; | |
| export function isTranscriptionConfigured() { | |
| const missingVars: string[] = []; |
🤖 Prompt for AI Agents
In apps/web/src/lib/transcription-utils.ts around lines 3–5, the local variable
missingVars is initialized as an empty array which TypeScript infers as any[];
annotate it to avoid implicit any. Change the declaration to explicitly type it
(e.g., missingVars: string[] = []) so the array elements are typed as strings
and comply with TS rules; update any subsequent pushes or checks to work with
the string[] type.
| async getFrameAt( | ||
| mediaId: string, | ||
| file: File, | ||
| time: number | ||
| ): Promise<WrappedCanvas | null> { | ||
| await this.ensureSink(mediaId, file); | ||
|
|
||
| const sinkData = this.sinks.get(mediaId); | ||
| if (!sinkData) return null; | ||
|
|
||
| if ( | ||
| sinkData.currentFrame && | ||
| this.isFrameValid(sinkData.currentFrame, time) | ||
| ) { | ||
| return sinkData.currentFrame; | ||
| } | ||
|
|
||
| if ( | ||
| sinkData.iterator && | ||
| sinkData.currentFrame && | ||
| time >= sinkData.lastTime && | ||
| time < sinkData.lastTime + 2.0 | ||
| ) { | ||
| const frame = await this.iterateToTime(sinkData, time); | ||
| if (frame) return frame; | ||
| } | ||
|
|
||
| return await this.seekToTime(sinkData, time); | ||
| } |
There was a problem hiding this comment.
Fix race conditions on shared iterator/seeks (concurrent calls can corrupt state).
Multiple overlapping getFrameAt() calls for the same mediaId can concurrently call iterator.next()/return() or replace sinkData.iterator, leading to nondeterministic behavior or runtime errors. Serialize per-media operations.
Apply:
@@
export class VideoCache {
private sinks = new Map<string, VideoSinkData>();
private initPromises = new Map<string, Promise<void>>();
+ private opChains = new Map<string, Promise<void>>();
+
+ private async runExclusive<T>(
+ mediaId: string,
+ task: () => Promise<T>
+ ): Promise<T> {
+ const prev = this.opChains.get(mediaId) ?? Promise.resolve();
+ let release: (() => void) | undefined;
+ const next = new Promise<void>((r) => {
+ release = r;
+ });
+ this.opChains.set(mediaId, prev.finally(() => next));
+ try {
+ await prev;
+ return await task();
+ } finally {
+ if (release) release();
+ if (this.opChains.get(mediaId) === next) this.opChains.delete(mediaId);
+ }
+ }
@@
- async getFrameAt(
+ async getFrameAt(
mediaId: string,
file: File,
time: number
): Promise<WrappedCanvas | null> {
- await this.ensureSink(mediaId, file);
-
- const sinkData = this.sinks.get(mediaId);
- if (!sinkData) return null;
-
- if (
- sinkData.currentFrame &&
- this.isFrameValid(sinkData.currentFrame, time)
- ) {
- return sinkData.currentFrame;
- }
-
- if (
- sinkData.iterator &&
- sinkData.currentFrame &&
- time >= sinkData.lastTime &&
- time < sinkData.lastTime + 2.0
- ) {
- const frame = await this.iterateToTime(sinkData, time);
- if (frame) return frame;
- }
-
- return await this.seekToTime(sinkData, time);
+ return this.runExclusive(mediaId, async () => {
+ await this.ensureSink(mediaId, file);
+ const sinkData = this.sinks.get(mediaId);
+ if (!sinkData) return null;
+
+ if (sinkData.currentFrame && this.isFrameValid(sinkData.currentFrame, time)) {
+ return sinkData.currentFrame;
+ }
+
+ if (
+ sinkData.iterator &&
+ sinkData.currentFrame &&
+ time >= sinkData.lastTime &&
+ time < sinkData.lastTime + 2.0
+ ) {
+ const frame = await this.iterateToTime(sinkData, time);
+ if (frame) return frame;
+ }
+
+ return this.seekToTime(sinkData, time);
+ });
}📝 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 getFrameAt( | |
| mediaId: string, | |
| file: File, | |
| time: number | |
| ): Promise<WrappedCanvas | null> { | |
| await this.ensureSink(mediaId, file); | |
| const sinkData = this.sinks.get(mediaId); | |
| if (!sinkData) return null; | |
| if ( | |
| sinkData.currentFrame && | |
| this.isFrameValid(sinkData.currentFrame, time) | |
| ) { | |
| return sinkData.currentFrame; | |
| } | |
| if ( | |
| sinkData.iterator && | |
| sinkData.currentFrame && | |
| time >= sinkData.lastTime && | |
| time < sinkData.lastTime + 2.0 | |
| ) { | |
| const frame = await this.iterateToTime(sinkData, time); | |
| if (frame) return frame; | |
| } | |
| return await this.seekToTime(sinkData, time); | |
| } | |
| export class VideoCache { | |
| private sinks = new Map<string, VideoSinkData>(); | |
| private initPromises = new Map<string, Promise<void>>(); | |
| private opChains = new Map<string, Promise<void>>(); | |
| private async runExclusive<T>( | |
| mediaId: string, | |
| task: () => Promise<T> | |
| ): Promise<T> { | |
| const prev = this.opChains.get(mediaId) ?? Promise.resolve(); | |
| let release: (() => void) | undefined; | |
| const next = new Promise<void>((r) => { | |
| release = r; | |
| }); | |
| this.opChains.set(mediaId, prev.finally(() => next)); | |
| try { | |
| await prev; | |
| return await task(); | |
| } finally { | |
| if (release) release(); | |
| if (this.opChains.get(mediaId) === next) this.opChains.delete(mediaId); | |
| } | |
| } | |
| async getFrameAt( | |
| mediaId: string, | |
| file: File, | |
| time: number | |
| ): Promise<WrappedCanvas | null> { | |
| return this.runExclusive(mediaId, async () => { | |
| await this.ensureSink(mediaId, file); | |
| const sinkData = this.sinks.get(mediaId); | |
| if (!sinkData) return null; | |
| if ( | |
| sinkData.currentFrame && | |
| this.isFrameValid(sinkData.currentFrame, time) | |
| ) { | |
| return sinkData.currentFrame; | |
| } | |
| if ( | |
| sinkData.iterator && | |
| sinkData.currentFrame && | |
| time >= sinkData.lastTime && | |
| time < sinkData.lastTime + 2.0 | |
| ) { | |
| const frame = await this.iterateToTime(sinkData, time); | |
| if (frame) return frame; | |
| } | |
| return this.seekToTime(sinkData, time); | |
| }); | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/web/src/lib/video-cache.ts around lines 19 to 47, concurrent
getFrameAt() calls for the same mediaId can race when they access/replace
sinkData.iterator, call iterator.next()/return(), or mutate
sinkData.currentFrame; serialize those per-media operations by adding a per-sink
mutex/lock (e.g., a Promise-based lock or queue stored on sinkData) and acquire
it at the start of getFrameAt() (or at least around any blocks that call
iterateToTime/seekToTime and touch iterator/currentFrame), perform the
iterator/seek logic while holding the lock, then release the lock so overlapping
calls are executed serially and state cannot be corrupted.
| clearVideo(mediaId: string): void { | ||
| const sinkData = this.sinks.get(mediaId); | ||
| if (sinkData) { | ||
| if (sinkData.iterator) { | ||
| sinkData.iterator.return(); | ||
| } | ||
|
|
||
| this.sinks.delete(mediaId); | ||
| } | ||
|
|
||
| this.initPromises.delete(mediaId); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Avoid unhandled promise rejections and release sink resources.
return() is async; not awaiting it can surface unhandled rejections. Also consider disposing the CanvasSink to free resources if supported.
- if (sinkData.iterator) {
- sinkData.iterator.return();
- }
+ if (sinkData.iterator) {
+ sinkData.iterator.return().catch(() => {});
+ }
+ // If mediabunny exposes a close/dispose on CanvasSink, call it:
+ // (sinkData.sink as unknown as { close?: () => void }).close?.();Does mediabunny’s CanvasSink (or underlying track) provide a close/dispose method you should call here to release decoders/contexts?
🌐 Web query:
Does the mediabunny CanvasSink or Input/Track types expose a close() or dispose() method that should be called to release resources?
💡 Result:
Short answer: No—CanvasSink and the Input/InputTrack types do not expose a close()/dispose() you must call.
Details and supporting docs:
- CanvasSink (a stateless sink that yields canvases) has no close/dispose API; its async iterators automatically clean up internal resources when the iterator is finished or broken out of. [1]
- Input / InputTrack are lightweight/read-only objects (creating an Input performs no reads); there is no documented close()/dispose() on Input or InputTrack. [2]
- What does require explicit closing: media sources used for writing (mediaSource.close()) and individual decoded samples (VideoSample/AudioSample.close())—those APIs are documented and should be used to release resources. [3]
Sources:
[1] CanvasSink / Media sinks docs.
[2] Reading media files / Input docs.
[3] Media sources (mediaSource.close) and Packets & samples (videoSample.close / audioSample.close).
Handle iterator.return() rejections and drop dispose suggestion
In apps/web/src/lib/video-cache.ts clearVideo, replace:
-if (sinkData.iterator) {
- sinkData.iterator.return();
-}
+if (sinkData.iterator) {
+ sinkData.iterator.return().catch(() => {});
+}CanvasSink/InputTrack have no close()/dispose() API.
📝 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.
| clearVideo(mediaId: string): void { | |
| const sinkData = this.sinks.get(mediaId); | |
| if (sinkData) { | |
| if (sinkData.iterator) { | |
| sinkData.iterator.return(); | |
| } | |
| this.sinks.delete(mediaId); | |
| } | |
| this.initPromises.delete(mediaId); | |
| } | |
| clearVideo(mediaId: string): void { | |
| const sinkData = this.sinks.get(mediaId); | |
| if (sinkData) { | |
| if (sinkData.iterator) { | |
| sinkData.iterator.return().catch(() => {}); | |
| } | |
| this.sinks.delete(mediaId); | |
| } | |
| this.initPromises.delete(mediaId); | |
| } |
🤖 Prompt for AI Agents
In apps/web/src/lib/video-cache.ts around lines 156 to 167, handle possible
rejections from sinkData.iterator.return() and remove any attempted
dispose/close calls; specifically, check that sinkData.iterator?.return is a
function, call it and attach a rejection handler (either await inside an async
wrapper or call sinkData.iterator.return()?.catch(() => {}) so the rejection is
swallowed/logged), and ensure you do not call any nonexistent close()/dispose()
APIs on CanvasSink/InputTrack — just delete the sink from this.sinks and clean
up initPromises as before.
|
Hi 👋 just a gentle follow-up. |
1 participant
What does this PR do?
Fixes a hydration error caused by mismatch between server and client rendering in the ThemeToggle component.
The
themevalue was undefined on the server, causing the server to render one value and the client another.Changes
mountedstate to delay rendering of theme-dependent text until after hydration.Steps to Test
Linked Issue
Closes #<610>
Summary by CodeRabbit
New Features
Improvements
Bug Fixes