New web based recorder (Instant Mode) #1363
Conversation
|
Warning Rate limit exceeded@richiemcilroy has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 35 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds a browser-based web recorder (UI, hooks, device prefs, upload clients), instant MP4 multipart upload + abort/delete S3 handling with retry, new Video RPCs and domain/schema types (webMP4, upload progress, instant recording), and integrates recorder UI into dashboard pages. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as WebRecorderDialog
participant Hooks as useWebRecorder & helpers
participant Browser as Media APIs
participant Backend
participant S3
User->>UI: Open dialog
UI->>Hooks: init (detect capabilities, load prefs)
Hooks->>Browser: enumerateDevices / getUserMedia / getDisplayMedia
Browser-->>Hooks: MediaStream(s)
User->>UI: Start recording
UI->>Hooks: startRecording
Hooks->>Hooks: MediaRecorder starts, buffer chunks
User->>UI: Stop recording
Hooks->>Hooks: stop -> final Blob / convert
Hooks->>Backend: VideoInstantCreate RPC → returns { videoId, upload }
Hooks->>S3: presign parts / upload parts
loop per part
Hooks->>S3: PUT part (presigned)
Hooks->>Backend: VideoUploadProgressUpdate RPC
end
Hooks->>S3: complete multipart upload
Backend-->>UI: video ready
sequenceDiagram
participant Upload as InstantMp4Uploader
participant Backend
participant S3
Upload->>Backend: initiateMultipartUpload (presign)
Backend-->>Upload: uploadId + presigned info
loop chunking
Upload->>Upload: buffer chunks
alt threshold reached
Upload->>Backend: presignMultipartPart
Backend-->>Upload: part URL
Upload->>S3: PUT part
S3-->>Upload: ETag
Upload->>Backend: update progress RPC
end
end
Upload->>Backend: completeMultipartUpload
Backend->>S3: complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas needing extra attention:
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
This reverts commit 4148b2b.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/app/(org)/dashboard/caps/components/WebRecorderDialog/instant-mp4-uploader.ts (1)
249-318: Abort active XHRs when cancelling uploads
cancel()now tears down the multipart session server-side, but any in-flightXMLHttpRequestkeeps streaming bytes until it finishes. That undermines the “instant stop” expectation and still burns bandwidth for the current part. Track active requests and callabort()so we cut the network transfer immediately.Apply this diff to capture and abort active requests:
@@ private readonly onChunkStateChange?: (chunks: ChunkUploadState[]) => void; private bufferedChunks: Blob[] = []; @@ private readonly chunkStates = new Map<number, ChunkUploadState>(); + private readonly activeXhrs = new Set<XMLHttpRequest>(); @@ return new Promise((resolve, reject) => { const xhr = new XMLHttpRequest(); + this.activeXhrs.add(xhr); xhr.open("PUT", url); xhr.responseType = "text"; @@ + xhr.onloadend = () => { + this.activeXhrs.delete(xhr); + }; @@ async cancel() { if (this.finished) return; this.finished = true; @@ const pendingUpload = this.uploadPromise.catch(() => { // Swallow errors during cancellation cleanup. }); + for (const xhr of this.activeXhrs) { + try { + xhr.abort(); + } catch { + // Best-effort abort; ignore failures. + } + } + this.activeXhrs.clear(); try { await abortMultipartUpload(this.videoId, this.uploadId);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/app/(org)/dashboard/caps/components/WebRecorderDialog/instant-mp4-uploader.ts(1 hunks)apps/web/app/api/upload/[...route]/multipart.ts(1 hunks)packages/web-backend/src/S3Buckets/S3BucketAccess.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
packages/web-backend/src/S3Buckets/S3BucketAccess.tsapps/web/app/api/upload/[...route]/multipart.tsapps/web/app/(org)/dashboard/caps/components/WebRecorderDialog/instant-mp4-uploader.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
packages/web-backend/src/S3Buckets/S3BucketAccess.tsapps/web/app/api/upload/[...route]/multipart.tsapps/web/app/(org)/dashboard/caps/components/WebRecorderDialog/instant-mp4-uploader.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/api/upload/[...route]/multipart.tsapps/web/app/(org)/dashboard/caps/components/WebRecorderDialog/instant-mp4-uploader.ts
🧬 Code graph analysis (2)
apps/web/app/api/upload/[...route]/multipart.ts (7)
apps/web/app/api/upload/utils.ts (1)
parseVideoIdOrFileKey(1-24)packages/web-backend/src/Videos/VideosRepo.ts (1)
VideosRepo(15-106)packages/web-backend/src/Videos/VideosPolicy.ts (1)
VideosPolicy(9-96)packages/web-backend/src/Database.ts (1)
Database(7-17)packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-211)packages/web-backend/src/Auth.ts (1)
makeCurrentUserLayer(48-50)apps/web/lib/server.ts (1)
runPromise(139-151)
apps/web/app/(org)/dashboard/caps/components/WebRecorderDialog/instant-mp4-uploader.ts (2)
apps/web/app/(org)/dashboard/caps/UploadingContext.tsx (1)
UploadStatus(8-30)apps/web/app/(org)/dashboard/caps/components/WebRecorderDialog/web-recorder-types.ts (2)
VideoId(15-15)ChunkUploadState(17-23)
⏰ 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: Analyze (rust)
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (6)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-dialog-header.tsx (1)
86-93: Improve accessibility of clickable plan badge.The plan badge uses a
<span>with anonClickhandler, which is not keyboard accessible. Users navigating with a keyboard cannot activate this element.Consider using a
<button>element:-<span +<button + type="button" + disabled={user.isPro} onClick={() => { if (!user.isPro) setUpgradeModalOpen(true); }} className={planClassName} > {planLabel} -</span> +</button>Note: You may need to adjust the
planClassNamestyling to ensure the button displays correctly (buttons have default browser styles that may need to be reset).apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useCameraDevices.ts (1)
1-63: Rename file to use kebab-case.The filename
useCameraDevices.tsviolates the project's coding guideline. TypeScript module files should use kebab-case naming.Rename the file:
useCameraDevices.ts → use-camera-devices.tsAlso update all imports referencing this file (e.g., in web-recorder-dialog.tsx and index.ts).
As per coding guidelines
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts (4)
605-627: Replace direct RPC call with mutation.Lines 614-618 directly call
EffectRuntime.runPromise(rpc.VideoDelete(...))in the error handler. Define adeleteVideoMutationusinguseEffectMutationat the hook level, then invoke the mutation function here.As per coding guidelines
694-775: Use mutation for video creation RPC.Lines 713-724 directly invoke
EffectRuntime.runPromise(rpc.VideoInstantCreate(...)). Refactor to useuseEffectMutationat the hook level per coding guidelines.As per coding guidelines
489-536: Apply mutation pattern for video creation and deletion.Lines 495-506 and 527-531 directly invoke
EffectRuntime.runPromise(rpc.VideoInstantCreate/VideoDelete(...)). UseuseEffectMutationto define mutations at the hook level, then call the returned functions here. This ensures proper integration with the Effect runtime and avoids directly callingrunPromise.As per coding guidelines
865-883: Apply mutation pattern in error cleanup.Lines 872-874 directly call
EffectRuntime.runPromise(rpc.VideoDelete(...))in the error handler. Use thedeleteVideoMutation(defined at hook level viauseEffectMutation) instead.As per coding guidelines
🧹 Nitpick comments (9)
apps/web/app/s/[videoId]/page.tsx (1)
270-270: Consider using the fetchedeffectiveCreatedAtfield.The
effectiveCreatedAtfield is now being fetched from the database but doesn't appear to be used anywhere in the component. Lines 729-731 still implement the same fallback logic manually (video.metadata?.customCreatedAt ? new Date(...) : video.createdAt).Since the database already computes this value, consider simplifying the logic at lines 729-731 to use
video.effectiveCreatedAtdirectly instead of duplicating the fallback logic in JavaScript.Apply this diff to use the database-computed field at lines 729-731:
<ShareHeader data={{ ...videoWithOrganizationInfo, - createdAt: video.metadata?.customCreatedAt - ? new Date(video.metadata.customCreatedAt) - : video.createdAt, + createdAt: video.effectiveCreatedAt, }}apps/web/actions/video/upload.ts (1)
325-367: Consider clarifying backoff strategy.The retry logic implements linear backoff (250ms, 500ms, 750ms) via
S3_DELETE_RETRY_BACKOFF_MS * attempt. The AI-generated summary describes this as "exponential-ish backoff", but the implementation is strictly linear. While linear backoff is acceptable for S3 operations, consider either:
- Updating documentation/comments to reflect the linear strategy, or
- Implementing true exponential backoff if that's the intent (e.g.,
Math.pow(2, attempt - 1) * baseDelay)packages/web-backend/src/Videos/index.ts (4)
146-148: Consider validating input instead of silently clamping.The code clamps
uploadedtototalusingMath.min, which silently corrects invalid client data. Consider logging a warning or rejecting the request ifinput.uploaded > input.total, as this indicates a client-side bug.-const uploaded = Math.min(input.uploaded, input.total); +if (input.uploaded > input.total) { + yield* Effect.logWarning( + `Upload progress exceeds total: ${input.uploaded} > ${input.total}` + ); +} +const uploaded = Math.min(input.uploaded, input.total);
179-192: Update operation may silently fail due to timestamp check.The WHERE clause at line 189 includes
Dz.lte(Db.videoUploads.updatedAt, updatedAt), which prevents updates if the incoming timestamp is older than the stored one. While this protects against stale updates, the method returnstrueregardless of whether rows were actually updated. Consider checking the affected row count and handling the case where no update occurred.
272-277: Empty strings passed as S3 metadata for missing optional fields.When optional fields like
durationSeconds,resolution,videoCodec, andaudioCodecare undefined, empty strings are passed as metadata values. Consider omitting these fields entirely from the metadata when they're not provided, as empty strings may cause confusion or processing issues downstream.-"x-amz-meta-duration": input.durationSeconds - ? input.durationSeconds.toString() - : "", -"x-amz-meta-resolution": input.resolution ?? "", -"x-amz-meta-videocodec": input.videoCodec ?? "", -"x-amz-meta-audiocodec": input.audioCodec ?? "", +...(input.durationSeconds && { + "x-amz-meta-duration": input.durationSeconds.toString(), +}), +...(input.resolution && { "x-amz-meta-resolution": input.resolution }), +...(input.videoCodec && { "x-amz-meta-videocodec": input.videoCodec }), +...(input.audioCodec && { "x-amz-meta-audiocodec": input.audioCodec }),
211-212: Consider a more descriptive error for organization mismatch.The policy check fails with a generic
PolicyDeniedErrorwhen the user's active organization doesn't match the input. Consider providing more context about why access was denied to aid debugging.-if (user.activeOrganizationId !== input.orgId) - return yield* Effect.fail(new Policy.PolicyDeniedError()); +if (user.activeOrganizationId !== input.orgId) + return yield* Effect.fail( + new Policy.PolicyDeniedError({ + message: "User's active organization does not match requested organization", + }) + );apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useDialogInteractions.ts (1)
31-44: Consider renamingshouldPreventDefaultfor clarity.The function name
shouldPreventDefaultcould be more descriptive. It returnstruewhen the target is whitelisted, meaning the dialog should not close. Consider renaming toshouldKeepDialogOpenorisInteractionAllowedto make the intent clearer.Apply this diff:
-const shouldPreventDefault = ( +const shouldKeepDialogOpen = ( target: Element | null | undefined, path: Array<EventTarget>, dialogContent: HTMLElement | null, ) => { if (!target) return false; return ( isWhitelisted(target, dialogContent) || path.some( (t) => t instanceof Element && isWhitelisted(t as Element, dialogContent), ) ); };Then update the usage in all three handlers:
- if (shouldPreventDefault(target, path, dialogContent)) { + if (shouldKeepDialogOpen(target, path, dialogContent)) { event.preventDefault(); }apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/instant-mp4-uploader.ts (1)
248-318: Make cancel actually abort the in-flight PUT
cancel()currently waits foruploadPromise, but the underlyingXMLHttpRequestkeeps streaming until it finishes or times out. On a slow link that means the user still waits for the full 5 MB chunk before cancel resolves. Please stash the active XHR instance socancel()can callabort()and mark the chunk accordingly. A sketch:@@ - private uploadPromise: Promise<void> = Promise.resolve(); + private uploadPromise: Promise<void> = Promise.resolve(); + private activeRequest: XMLHttpRequest | null = null; @@ private uploadBlobWithProgress({ url, partNumber, part }: { ... }) { return new Promise((resolve, reject) => { const xhr = new XMLHttpRequest(); + this.activeRequest = xhr; @@ xhr.onload = () => { + this.activeRequest = null; @@ xhr.onerror = () => { + this.activeRequest = null; @@ + xhr.onabort = () => { + this.activeRequest = null; + this.updateChunkState(partNumber, { status: "error" }); + reject(new Error(`Upload of part ${partNumber} aborted`)); + }; xhr.send(part); }); } @@ async cancel() { @@ - const pendingUpload = this.uploadPromise.catch(() => { + const pendingUpload = this.uploadPromise.catch(() => { // Swallow errors during cancellation cleanup. }); + if (this.activeRequest) { + this.activeRequest.abort(); + this.activeRequest = null; + }That keeps cancel responsive even while a part upload is mid-flight.
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/CameraPreviewWindow.tsx (1)
202-224: Re-clamp position on window resizeWe only clamp the preview window when size/shape/dimensions change. If the user docks it near the edge and then resizes the window smaller, the preview can end up partly off-screen because we never recompute against the new viewport. Consider extracting this block into a function wired to
window.resizeso we re-clamp whenever the viewport changes:useEffect(() => { - const metrics = getPreviewMetrics(size, shape, videoDimensions); - if (typeof window === "undefined") { - return; - } - const totalHeight = metrics.height + BAR_HEIGHT; - const maxX = Math.max(0, window.innerWidth - metrics.width); - const maxY = Math.max(0, window.innerHeight - totalHeight); - setPosition((prev) => { - const defaultX = window.innerWidth - metrics.width - WINDOW_PADDING; - const defaultY = window.innerHeight - totalHeight - WINDOW_PADDING; - const nextX = prev?.x ?? defaultX; - const nextY = prev?.y ?? defaultY; - return { - x: Math.max(0, Math.min(nextX, maxX)), - y: Math.max(0, Math.min(nextY, maxY)), - }; - }); -}, [size, shape, videoDimensions]); + const clampPosition = () => { + if (typeof window === "undefined") return; + const metrics = getPreviewMetrics(size, shape, videoDimensions); + const totalHeight = metrics.height + BAR_HEIGHT; + const maxX = Math.max(0, window.innerWidth - metrics.width); + const maxY = Math.max(0, window.innerHeight - totalHeight); + setPosition((prev) => { + const defaultX = window.innerWidth - metrics.width - WINDOW_PADDING; + const defaultY = window.innerHeight - totalHeight - WINDOW_PADDING; + const nextX = prev?.x ?? defaultX; + const nextY = prev?.y ?? defaultY; + return { + x: Math.max(0, Math.min(nextX, maxX)), + y: Math.max(0, Math.min(nextY, maxY)), + }; + }); + }; + clampPosition(); + window.addEventListener("resize", clampPosition); + return () => window.removeEventListener("resize", clampPosition); +}, [size, shape, videoDimensions]);That keeps the preview reachable after viewport changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (36)
apps/web/actions/video/upload.ts(3 hunks)apps/web/app/(org)/dashboard/caps/components/EmptyCapState.tsx(3 hunks)apps/web/app/(org)/dashboard/caps/components/index.ts(1 hunks)apps/web/app/(org)/dashboard/caps/components/sendProgressUpdate.ts(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/CameraPreviewWindow.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/CameraSelector.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/HowItWorksButton.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/HowItWorksPanel.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/InProgressRecordingBar.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/MicrophoneSelector.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/RecordingButton.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/RecordingModeSelector.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/SettingsButton.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/SettingsPanel.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/instant-mp4-uploader.ts(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/recording-conversion.ts(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/recording-upload.ts(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useCameraDevices.ts(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useDevicePreferences.ts(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useDialogInteractions.ts(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMediaPermission.ts(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMediaRecorderSetup.ts(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMicrophoneDevices.ts(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useRecordingTimer.ts(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useStreamManagement.ts(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useSurfaceDetection.ts(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-constants.ts(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-dialog-header.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-dialog.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-types.ts(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-utils.ts(1 hunks)apps/web/app/embed/[videoId]/page.tsx(1 hunks)apps/web/app/s/[videoId]/page.tsx(2 hunks)apps/web/package.json(1 hunks)packages/web-backend/src/Videos/index.ts(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/app/embed/[videoId]/page.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/(org)/dashboard/caps/components/sendProgressUpdate.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/web/app/(org)/dashboard/caps/components/index.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useRecordingTimer.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/MicrophoneSelector.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useDevicePreferences.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/HowItWorksPanel.tsxapps/web/app/(org)/dashboard/caps/components/EmptyCapState.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMicrophoneDevices.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-dialog-header.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMediaPermission.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/InProgressRecordingBar.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useSurfaceDetection.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useStreamManagement.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/instant-mp4-uploader.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/CameraPreviewWindow.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-dialog.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/RecordingButton.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useCameraDevices.tsapps/web/app/s/[videoId]/page.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/CameraSelector.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/RecordingModeSelector.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-constants.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/SettingsButton.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/HowItWorksButton.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.tsapps/web/actions/video/upload.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/SettingsPanel.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/recording-upload.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-utils.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMediaRecorderSetup.tspackages/web-backend/src/Videos/index.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useDialogInteractions.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-types.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/recording-conversion.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/app/(org)/dashboard/caps/components/index.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useRecordingTimer.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/MicrophoneSelector.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useDevicePreferences.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/HowItWorksPanel.tsxapps/web/app/(org)/dashboard/caps/components/EmptyCapState.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMicrophoneDevices.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-dialog-header.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMediaPermission.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/InProgressRecordingBar.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useSurfaceDetection.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useStreamManagement.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/instant-mp4-uploader.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/CameraPreviewWindow.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-dialog.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/RecordingButton.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useCameraDevices.tsapps/web/app/s/[videoId]/page.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/CameraSelector.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/RecordingModeSelector.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-constants.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/SettingsButton.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/HowItWorksButton.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.tsapps/web/actions/video/upload.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/SettingsPanel.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/recording-upload.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-utils.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMediaRecorderSetup.tspackages/web-backend/src/Videos/index.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useDialogInteractions.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-types.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/recording-conversion.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/(org)/dashboard/caps/components/index.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useRecordingTimer.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/MicrophoneSelector.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useDevicePreferences.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/HowItWorksPanel.tsxapps/web/app/(org)/dashboard/caps/components/EmptyCapState.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMicrophoneDevices.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-dialog-header.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMediaPermission.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/InProgressRecordingBar.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useSurfaceDetection.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useStreamManagement.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/instant-mp4-uploader.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/CameraPreviewWindow.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-dialog.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/RecordingButton.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useCameraDevices.tsapps/web/app/s/[videoId]/page.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/CameraSelector.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/RecordingModeSelector.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-constants.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/SettingsButton.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/HowItWorksButton.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.tsapps/web/actions/video/upload.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/SettingsPanel.tsxapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/recording-upload.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-utils.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMediaRecorderSetup.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useDialogInteractions.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-types.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/recording-conversion.ts
🧠 Learnings (5)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use kebab-case for filenames for TypeScript/JavaScript modules (e.g., `user-menu.tsx`).
Applied to files:
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useCameraDevices.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to packages/ui/**/*.{ts,tsx,js,jsx} : Component files in `packages/ui` should use PascalCase naming if they define React/Solid components.
Applied to files:
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useCameraDevices.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use PascalCase for React/Solid components.
Applied to files:
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useCameraDevices.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/**/*.{ts,tsx,js,jsx} : On the client, always use `useEffectQuery` or `useEffectMutation` from `@/lib/EffectRuntime`; never call `EffectRuntime.run*` directly in components.
Applied to files:
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/app/api/* : On the server, always run effects through `EffectRuntime.runPromise` after `provideOptionalAuth` to ensure cookies and per-request context are attached.
Applied to files:
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts
🧬 Code graph analysis (19)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/MicrophoneSelector.tsx (2)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMediaPermission.ts (1)
useMediaPermission(23-111)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-constants.ts (2)
NO_MICROPHONE_VALUE(2-2)NO_MICROPHONE(1-1)
apps/web/app/(org)/dashboard/caps/components/EmptyCapState.tsx (1)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-dialog.tsx (1)
WebRecorderDialog(39-349)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-dialog-header.tsx (1)
apps/web/app/(org)/dashboard/Contexts.tsx (1)
useDashboardContext(50-50)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/InProgressRecordingBar.tsx (1)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-types.ts (2)
RecorderPhase(1-9)ChunkUploadState(17-23)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useSurfaceDetection.ts (3)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-constants.ts (2)
DetectedDisplayRecordingMode(46-49)DETECTION_RETRY_DELAYS(119-119)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-utils.ts (2)
DetectedDisplayRecordingMode(7-7)detectRecordingModeFromTrack(36-76)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/RecordingModeSelector.tsx (1)
RecordingMode(18-18)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/instant-mp4-uploader.ts (2)
apps/web/app/(org)/dashboard/caps/UploadingContext.tsx (1)
UploadStatus(8-30)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-types.ts (2)
VideoId(15-15)ChunkUploadState(17-23)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/CameraPreviewWindow.tsx (2)
apps/desktop/src/utils/tauri.ts (2)
CameraPreviewSize(368-368)CameraPreviewShape(367-367)packages/ui/src/components/LoadingSpinner.tsx (1)
LoadingSpinner(1-42)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-dialog.tsx (7)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/RecordingModeSelector.tsx (2)
RecordingMode(18-18)RecordingModeSelector(26-112)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMicrophoneDevices.ts (1)
useMicrophoneDevices(5-61)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useCameraDevices.ts (1)
useCameraDevices(5-63)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useDevicePreferences.ts (1)
useDevicePreferences(15-147)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts (1)
useWebRecorder(70-1010)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useDialogInteractions.ts (1)
useDialogInteractions(52-119)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-constants.ts (2)
FREE_PLAN_MAX_RECORDING_MS(121-121)dialogVariants(6-31)
apps/web/app/s/[videoId]/page.tsx (1)
packages/database/schema.ts (1)
videos(291-363)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/CameraSelector.tsx (2)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMediaPermission.ts (1)
useMediaPermission(23-111)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-constants.ts (2)
NO_CAMERA_VALUE(4-4)NO_CAMERA(3-3)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-constants.ts (1)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-utils.ts (1)
DetectedDisplayRecordingMode(7-7)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/SettingsButton.tsx (1)
apps/web/app/(org)/dashboard/_components/AnimatedIcons/index.ts (1)
CogIcon(16-16)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts (15)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/RecordingModeSelector.tsx (1)
RecordingMode(18-18)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-types.ts (4)
RecorderPhase(1-9)VideoId(15-15)ChunkUploadState(17-23)PresignedPost(14-14)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-utils.ts (5)
RecorderCapabilities(9-14)detectCapabilities(16-34)DetectedDisplayRecordingMode(7-7)shouldRetryDisplayMediaWithoutPreferences(85-94)pickSupportedMimeType(78-83)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useStreamManagement.ts (1)
useStreamManagement(3-59)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useRecordingTimer.ts (1)
useRecordingTimer(3-102)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMediaRecorderSetup.ts (1)
useMediaRecorderSetup(4-93)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useSurfaceDetection.ts (1)
useSurfaceDetection(7-95)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/instant-mp4-uploader.ts (2)
InstantMp4Uploader(100-391)initiateMultipartUpload(42-52)apps/web/lib/EffectRuntime.ts (2)
useRpcClient(25-25)EffectRuntime(20-20)apps/web/app/(org)/dashboard/caps/UploadingContext.tsx (1)
useUploadingContext(41-48)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-constants.ts (8)
RECORDING_MODE_TO_DISPLAY_SURFACE(93-100)DetectedDisplayRecordingMode(46-49)DisplaySurfacePreference(51-55)DISPLAY_MEDIA_VIDEO_CONSTRAINTS(33-37)DISPLAY_MODE_PREFERENCES(57-79)MP4_MIME_TYPES(102-112)WEBM_MIME_TYPES(114-117)FREE_PLAN_MAX_RECORDING_MS(121-121)apps/web/app/(org)/dashboard/caps/components/sendProgressUpdate.ts (1)
sendProgressUpdate(5-24)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/recording-conversion.ts (2)
convertToMp4(95-137)captureThumbnail(8-93)apps/web/actions/video/upload.ts (2)
createVideoAndGetUploadUrl(138-265)deleteVideoResultFile(267-323)apps/web/lib/Requests/ThumbnailRequest.ts (1)
ThumbnailRequest(10-14)
apps/web/actions/video/upload.ts (6)
packages/web-domain/src/Video.ts (3)
Video(16-59)VideoId(12-12)VideoId(13-13)packages/database/auth/session.ts (1)
getCurrentUser(15-28)packages/database/schema.ts (1)
videos(291-363)packages/web-domain/src/S3Bucket.ts (1)
S3Bucket(7-15)packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-211)apps/web/lib/server.ts (1)
runPromise(139-151)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/recording-upload.ts (2)
apps/web/app/(org)/dashboard/caps/UploadingContext.tsx (1)
UploadStatus(8-30)apps/web/app/(org)/dashboard/caps/components/sendProgressUpdate.ts (1)
sendProgressUpdate(5-24)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-utils.ts (1)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-constants.ts (2)
DetectedDisplayRecordingMode(46-49)DISPLAY_SURFACE_TO_RECORDING_MODE(81-91)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMediaRecorderSetup.ts (1)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-types.ts (1)
RecorderErrorEvent(11-11)
packages/web-backend/src/Videos/index.ts (4)
packages/web-domain/src/Video.ts (5)
UploadProgressUpdateInput(70-75)Video(16-59)InstantRecordingCreateInput(82-92)VideoId(12-12)VideoId(13-13)packages/web-domain/src/Authentication.ts (1)
CurrentUser(8-16)packages/web-domain/src/Folder.ts (3)
Folder(37-45)FolderId(11-11)FolderId(12-12)packages/utils/src/lib/dub.ts (1)
dub(4-7)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/recording-conversion.ts (3)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/instant-mp4-uploader.ts (1)
finalize(343-373)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-types.ts (1)
VideoId(15-15)apps/web/app/(org)/dashboard/caps/UploadingContext.tsx (1)
UploadStatus(8-30)
⏰ 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: Analyze (rust)
🔇 Additional comments (38)
apps/web/package.json (1)
56-56: New Radix UI dialog and popover primitives align with web recorder UI requirements.Both dependencies are appropriate for the new web-based recorder modal dialogs and device selection UI mentioned in the PR objectives. They follow the existing project pattern of using Radix UI components and are correctly positioned in alphabetical order within the dependency list.
Please verify two aspects:
Version compatibility: These dependencies use major version 1 (e.g.,
^1.1.13,^1.0.7) while other Radix UI components in the project use major version 2 (e.g.,@radix-ui/react-dropdown-menu@^2.0.6). Confirm this version mix is intentional and compatible across the Radix UI ecosystem.Security: Verify that these specific versions have no known security vulnerabilities. You can check this via
npm auditor the GitHub Advisory Database.Also applies to: 58-58
apps/web/app/s/[videoId]/page.tsx (1)
462-462: Consistent with the earlier query.This addition mirrors the change at line 270 and ensures that
effectiveCreatedAtis available even after the video object is refreshed following transcription. The same suggestion applies: if you adopt the refactor to useeffectiveCreatedAtat lines 729-731, this field will be properly utilized.apps/web/actions/video/upload.ts (5)
14-19: LGTM!The
S3Bucketimport is correctly added to support the new video deletion functionality.
25-26: LGTM!The retry configuration constants are reasonable for S3 deletion operations.
369-379: LGTM!The error serialization helper safely handles both
Errorinstances and unknown error types for structured logging.
381-385: LGTM!The promise-based sleep helper is correctly implemented for introducing delays between retry attempts.
214-214: Verify intent: should desktop uploads still use "desktopMP4" instead of "webMP4"?The change affects all callers of
createVideoAndGetUploadUrl, including:
- New web recorder (intended change) ✓
- Desktop file uploads via UploadCapButton (line 127)
- Screenshots from uploads via UploadCapButton (line 458)
While both
"desktopMP4"and"webMP4"are handled identically throughout the codebase (Video.ts, ShareVideo, EmbedVideo, playlist route all treat them the same), this appears to conflate the source type semantics. Desktop file uploads should likely remain as"desktopMP4", with only the new web recorder creating"webMP4"videos.Also note: the desktop API endpoint (
apps/web/app/api/desktop/[...route]/video.ts) still creates"desktopMP4"videos (line 179), so there's now inconsistency between desktop uploads via the web UI vs. the desktop app.packages/web-backend/src/Videos/index.ts (9)
15-25: LGTM: Type aliases are well-structured.The type aliases and helper types correctly extract types from schemas and Options for use in method signatures.
54-67: LGTM: Improved error handling with explicit NotFoundError.The change to Option-based handling makes the error case explicit. Using
video.ownerIdfor the bucket prefix is more correct than relying on a separateuserIdparameter.
87-92: LGTM: Consistent Option-based error handling.The duplicate method now follows the same pattern as the delete method for error handling.
139-141: LGTM: Consistent Option-based return type.The method now returns
Option<Video.UploadProgress>, aligning with the Effect library patterns used throughout the codebase.
322-324: Download only supports MP4 sources; silently returns None for M3U8.The method now explicitly checks for
Mp4Sourceand returnsOption.none()for other source types (like M3U8). Ensure this behavior aligns with the UI expectations—callers must handle the None case gracefully. Consider whether returning an error would be clearer than None for unsupported source types.
333-350: LGTM: Consistent Option-based handling with proper prefix.The method correctly uses
video.ownerIdfor the S3 prefix and returnsOption.none()when the video or thumbnail is not found.
355-360: LGTM: Consistent Option-based error handling.The analytics method now follows the same Option-based pattern as other methods in the file.
172-172: ****The
modefield exists in thevideoUploadsschema atpackages/database/schema.ts:753as a varchar column with enum values["singlepart", "multipart"]. The code at line 172 correctly accesses this field without risk of runtime error.Likely an incorrect or invalid review comment.
258-264: Nullish coalescing may not respect explicitfalsevalue.The expression
input.supportsUploadProgress ?? truetreats an explicitfalsevalue the same astrue. If the client intentionally passesfalse, the upload progress record will still be created. Consider using a different default check if explicitfalseshould be respected.-if (input.supportsUploadProgress ?? true) +if (input.supportsUploadProgress !== false)Likely an incorrect or invalid review comment.
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/HowItWorksButton.tsx (1)
11-18: Concise button feels right.Simple markup, accessible text, and icon pairing make this affordance clear. Nice!
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/SettingsButton.tsx (1)
15-24: Visibility gate keeps things tidy.Returning
nullwhen hidden avoids stray focus targets while keeping the button styling tight.apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useDevicePreferences.ts (1)
24-135: Appreciate the defensive storage handling.The SSR guards and try/catch blocks keep this hook resilient across environments—good attention to detail.
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/HowItWorksPanel.tsx (1)
1-104: LGTM! Well-structured informational panel.The component is well-implemented with proper animations, accessibility considerations (aria-hidden spacer on line 70), and clear separation of concerns. The
HOW_IT_WORKS_ITEMSconstant with type assertion ensures type safety while maintaining readability.apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useDialogInteractions.ts (1)
57-112: LGTM! Outside interaction handling is implemented correctly.The three handlers (
handlePointerDownOutside,handleFocusOutside,handleInteractOutside) consistently prevent dialog dismissal when recording/busy or when interacting with whitelisted elements (selects, camera preview). The logic correctly maintains the dialog open for intended interactions.apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/RecordingButton.tsx (1)
1-56: LGTM! Clean and straightforward button implementation.The component properly handles the recording state transitions with appropriate callbacks and disabled states. The embedded
InstantIconSVG is well-structured with proper props spreading.apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/RecordingModeSelector.tsx (1)
1-112: LGTM! Well-typed selector with clear UX.The component demonstrates good practices with type-safe mode options, icon integration, and helpful guidance (fullscreen recommendation note on lines 98-103). The use of
Record<RecordingMode, ...>ensures all modes are covered.apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMediaRecorderSetup.ts (2)
55-75: LGTM! Race condition properly mitigated.The concurrent stop call issue from the previous review has been correctly addressed. The
isStoppingRefflag (lines 59, 61) prevents overlapping stop operations, and all cleanup paths (lines 30, 42, 51) properly reset the flag.
25-44: LGTM! Blob assembly and error handling are correct.The
onRecorderStopcallback properly handles the no-data case (lines 26-33) and correctly constructs the Blob with the appropriate MIME type fallback (line 36). State cleanup is thorough.apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/CameraSelector.tsx (1)
108-130: Nice permission gating on the triggerBlocking pointer/keyboard events while permission is pending keeps the UX accessible and avoids confusing dead dialogs. Looks solid.
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-types.ts (1)
13-23: Good reuse of domain-driven typesDeriving
PresignedPost,VideoId, and upload state from the domain module keeps everything type-safe and future-proof.apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/recording-conversion.ts (1)
8-136: Thorough conversion/thumbnail utilitiesAppreciate the defensive cleanup in
captureThumbnailand the progress plumbing inconvertToMp4; both will keep the UI responsive during long conversions.apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-dialog.tsx (2)
113-121: Camera auto-selection logic looks good.The effect auto-selects the first available camera when switching to camera mode without a selection. This only triggers when
useDevicePreferenceshasn't restored a saved preference, providing a sensible fallback for first-time users.
51-70: Audio preload and cleanup are well-implemented.The audio cues are preloaded on mount with proper cleanup on unmount, and the implementation guards against SSR with the
typeof windowcheck.apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/InProgressRecordingBar.tsx (2)
191-246: Async callback error handling is robust.All async handlers (
handleStop,handlePauseToggle,handleRestart) properly wrap callback results withPromise.resolve().catch()andtry-catch, preventing unhandled promise rejections as addressed in previous review feedback.
349-563: Inline chunk progress UI is well-structured.The
InlineChunkProgresscomponent provides clear visual feedback with a circular progress indicator and a detailed popover. The hover delay logic and chunk state mapping are implemented cleanly.apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useSurfaceDetection.ts (1)
7-95: Surface detection hook is well-designed.The hook encapsulates detection logic cleanly, with retry scheduling, track event listeners for readiness, and proper cleanup. Error handling in
clearDetectionTracking(lines 22-27) ensures resilience against misbehaving cleanup functions.apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts (2)
905-929: Free-plan auto-stop implementation is correct.The effect uses
freePlanAutoStopTriggeredRefto prevent duplicate stop calls, and thestopRecordingfunction itself guards against invalid phases (line 675). The typo "oast" mentioned in past comments has been corrected totoast.infoon line 922.
332-627: Complex startRecording function is well-structured.The function handles multiple recording modes (camera, display), device acquisition, stream composition, MediaRecorder setup, and instant MP4 upload initialization. Error handling includes cleanup of orphaned video records. The implementation is comprehensive and the flow is logical.
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-utils.ts (3)
16-34: Capability detection is robust.The function properly guards against SSR and checks for MediaRecorder, getUserMedia, and getDisplayMedia availability, returning a clear capability object for downstream decision-making.
36-76: Recording mode detection has appropriate fallbacks.The function first checks
displaySurfacefrom track settings, then falls back to label keyword matching. This provides resilient mode detection across different browser implementations.
78-94: MIME type selection and error classification are well-implemented.
pickSupportedMimeTypesafely checks MediaRecorder availability before testing candidates.shouldRetryDisplayMediaWithoutPreferencescorrectly identifiesOverconstrainedError,NotSupportedError, andTypeErroras recoverable, enabling graceful degradation when display preferences aren't supported.
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/MicrophoneSelector.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/recording-upload.ts
Outdated
Show resolved
Hide resolved
| const pauseTimer = useCallback( | ||
| (timestamp?: number) => { | ||
| const now = timestamp ?? performance.now(); | ||
| pauseStartRef.current = now; | ||
| syncDurationFromClock(now); | ||
| }, | ||
| [syncDurationFromClock], | ||
| ); | ||
|
|
||
| const resumeTimer = useCallback( | ||
| (timestamp?: number) => { | ||
| const now = timestamp ?? performance.now(); | ||
| commitPausedDuration(now); | ||
| syncDurationFromClock(now); | ||
| }, |
There was a problem hiding this comment.
Guard repeated pause calls to keep elapsed time accurate.
If pauseTimer is triggered twice before resumeTimer (React Strict Mode double-invocation or a double click), we overwrite pauseStartRef and lose the already accrued paused duration. That makes the elapsed timer jump forward by the time spent paused. Please make the pause call idempotent so we preserve the full paused span.
const pauseTimer = useCallback(
(timestamp?: number) => {
+ if (pauseStartRef.current !== null) {
+ return;
+ }
const now = timestamp ?? performance.now();
pauseStartRef.current = now;
syncDurationFromClock(now);
},
[syncDurationFromClock],
);🤖 Prompt for AI Agents
In
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useRecordingTimer.ts
around lines 74 to 88, pauseTimer can be called multiple times and overwrites
pauseStartRef, losing previously accrued paused duration; change pauseTimer to
be idempotent by checking if pauseStartRef.current is already set and returning
early if so (only set pauseStartRef.current and call syncDurationFromClock when
not already paused), so repeated pause calls don’t reset the pause start time
and elapsed time remains accurate.
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/actions/video/upload.ts (2)
299-309: Consider removing redundant transaction wrapper.The transaction contains only a single
DELETEoperation. Database transactions are primarily useful for ensuring atomicity across multiple operations, so wrapping a single delete is technically unnecessary.If you'd like to simplify, apply this diff:
- try { - await db().transaction(async (tx) => { - await tx.delete(videoUploads).where(eq(videoUploads.videoId, videoId)); - }); - } catch (error) { + try { + await db().delete(videoUploads).where(eq(videoUploads.videoId, videoId)); + } catch (error) {
333-375: Retry logic implements linear backoff.The backoff at line 367 is linear:
250ms * attempt(250ms, 500ms, 750ms). For three attempts with a 250ms base, linear backoff is acceptable, though exponential backoff (250ms * 2^(attempt-1)) is more common for S3 retry patterns and would yield 250ms, 500ms, 1000ms.If you prefer exponential backoff, apply this diff:
if (attempt < MAX_S3_DELETE_ATTEMPTS) { - await sleep(S3_DELETE_RETRY_BACKOFF_MS * attempt); + await sleep(S3_DELETE_RETRY_BACKOFF_MS * Math.pow(2, attempt - 1)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/actions/video/upload.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/web/actions/video/upload.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/actions/video/upload.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/actions/video/upload.ts
🧬 Code graph analysis (1)
apps/web/actions/video/upload.ts (6)
packages/web-domain/src/Video.ts (3)
Video(16-59)VideoId(12-12)VideoId(13-13)packages/database/auth/session.ts (1)
getCurrentUser(15-28)packages/database/schema.ts (2)
videos(291-363)videoUploads(747-754)packages/web-domain/src/S3Bucket.ts (1)
S3Bucket(7-15)packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-211)apps/web/lib/server.ts (1)
runPromise(139-151)
⏰ 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: Analyze (rust)
🔇 Additional comments (4)
apps/web/actions/video/upload.ts (4)
14-19: LGTM!The new S3Bucket import and retry constants are appropriately scoped for the delete functionality added in this file.
Also applies to: 25-26
267-331: Verify error handling strategy after partial success.The function correctly implements the separation of concerns requested in the past review:
- Database transaction (lines 299-309) commits first
- S3 deletion (lines 311-323) executes afterward
However, if S3 deletion fails after the database commit succeeds, the function throws an error (line 322) even though the
videoUploadsrow has been permanently deleted. This means:
- The caller receives a failure response
- The database state has changed (videoUploads row removed)
- The S3 object remains (orphaned)
Confirm this error propagation is intentional. Alternatively, consider logging the S3 failure without throwing, since the past review comment noted that "orphaned S3 objects can be cleaned up separately."
377-393: LGTM!The
serializeErrorandsleephelper functions are well-implemented:
serializeErrorsafely handles both Error instances and unknown error types for structured loggingsleepprovides a clean promise-based delay for retry backoff
214-214: The original review comment is incorrect.
createVideoAndGetUploadUrlis only called from web components (web recorder atuseWebRecorder.ts:791and file upload atUploadCapButton.tsx). Desktop recordings use a separate API endpoint (apps/web/app/api/desktop/[...route]/video.ts) that continues to setsource: { type: "desktopMP4" }when the desktop app sendsrecordingMode: "desktopMP4". No desktop flows are affected by this change.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/MicrophoneSelector.tsx (1)
74-84: Consider showing a loading indicator during permission request.While awaiting the permission prompt (line 75), the UI has no visual feedback, which might make it appear frozen to users. Consider adding a loading state that displays a spinner or temporarily updates the pill text to "Requesting...".
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/MicrophoneSelector.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/recording-upload.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/recording-upload.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/MicrophoneSelector.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/MicrophoneSelector.tsx
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/MicrophoneSelector.tsx
🧬 Code graph analysis (1)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/MicrophoneSelector.tsx (2)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMediaPermission.ts (1)
useMediaPermission(23-111)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-constants.ts (2)
NO_MICROPHONE_VALUE(2-2)NO_MICROPHONE(1-1)
⏰ 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: Analyze (rust)
🔇 Additional comments (3)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/MicrophoneSelector.tsx (3)
1-15: LGTM!Imports are clean and appropriate for a client component that manages media permissions and device selection.
17-26: LGTM!The interface is well-typed with appropriate nullable and optional properties.
113-190: Render logic and interactions are well-implemented.The component correctly:
- Maps
null↔NO_MICROPHONE_VALUEfor the select state- Blocks dropdown interactions when permission is needed
- Provides fallback labels for unlabeled microphones
- Uses
deviceIdas a unique key for list items
| @@ -0,0 +1,191 @@ | |||
| "use client"; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Rename file to follow kebab-case convention.
Per coding guidelines, TypeScript module filenames should use kebab-case. Rename MicrophoneSelector.tsx to microphone-selector.tsx and update imports in parent components accordingly.
As per coding guidelines.
🤖 Prompt for AI Agents
In
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/MicrophoneSelector.tsx
around line 1, the filename uses PascalCase but our guideline requires
kebab-case; rename the file to microphone-selector.tsx and update all imports
that reference MicrophoneSelector (including parent components and any index
barrels) to use the new kebab-case filename, ensuring TypeScript/Next.js paths
and any default/named exports remain unchanged so imports continue to resolve.
| <button | ||
| type="button" | ||
| className={clsx( | ||
| statusPillClassName, | ||
| "absolute right-[0.375rem] top-1/2 -translate-y-1/2 z-20", | ||
| )} | ||
| disabled={statusPillDisabled} | ||
| aria-disabled={statusPillDisabled} | ||
| onClick={handleStatusPillClick} | ||
| onKeyDown={handleStatusPillKeyDown} | ||
| > | ||
| {shouldRequestPermission | ||
| ? "Request permission" | ||
| : micEnabled | ||
| ? "On" | ||
| : "Off"} | ||
| </button> |
There was a problem hiding this comment.
Add an accessible label for the status pill button.
The status pill only displays "On", "Off", or "Request permission", which lacks context for screen reader users. Consider adding an aria-label attribute to clarify the button's purpose.
Apply this diff to improve accessibility:
<button
type="button"
className={clsx(
statusPillClassName,
"absolute right-[0.375rem] top-1/2 -translate-y-1/2 z-20",
)}
disabled={statusPillDisabled}
aria-disabled={statusPillDisabled}
+ aria-label={
+ shouldRequestPermission
+ ? "Request microphone permission"
+ : micEnabled
+ ? "Microphone is on, click to turn off"
+ : "Microphone is off"
+ }
onClick={handleStatusPillClick}
onKeyDown={handleStatusPillKeyDown}
>📝 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.
| <button | |
| type="button" | |
| className={clsx( | |
| statusPillClassName, | |
| "absolute right-[0.375rem] top-1/2 -translate-y-1/2 z-20", | |
| )} | |
| disabled={statusPillDisabled} | |
| aria-disabled={statusPillDisabled} | |
| onClick={handleStatusPillClick} | |
| onKeyDown={handleStatusPillKeyDown} | |
| > | |
| {shouldRequestPermission | |
| ? "Request permission" | |
| : micEnabled | |
| ? "On" | |
| : "Off"} | |
| </button> | |
| <button | |
| type="button" | |
| className={clsx( | |
| statusPillClassName, | |
| "absolute right-[0.375rem] top-1/2 -translate-y-1/2 z-20", | |
| )} | |
| disabled={statusPillDisabled} | |
| aria-disabled={statusPillDisabled} | |
| aria-label={ | |
| shouldRequestPermission | |
| ? "Request microphone permission" | |
| : micEnabled | |
| ? "Microphone is on, click to turn off" | |
| : "Microphone is off" | |
| } | |
| onClick={handleStatusPillClick} | |
| onKeyDown={handleStatusPillKeyDown} | |
| > | |
| {shouldRequestPermission | |
| ? "Request permission" | |
| : micEnabled | |
| ? "On" | |
| : "Off"} | |
| </button> |
🤖 Prompt for AI Agents
In
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/MicrophoneSelector.tsx
around lines 154 to 170, the status pill button only shows "On"/"Off"/"Request
permission" visually and needs an accessible label; add an aria-label prop that
clearly describes the control and its current state (e.g., "Microphone: On",
"Microphone: Off", or "Request microphone permission") and ensure it updates
with the same conditional logic used for the button text so screen readers get
full context.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts (1)
498-509: Refactor to use mutation for VideoInstantCreate RPC.Lines 498-509 directly call
EffectRuntime.runPromise(rpc.VideoInstantCreate(...)), violating the coding guideline: "On the client, always useuseEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime."Define a mutation at the hook level (similar to
deleteVideoat lines 195-197):const deleteVideo = useEffectMutation({ mutationFn: (id: VideoId) => rpc.VideoDelete(id), }); + const createInstantVideo = useEffectMutation({ + mutationFn: (params: Parameters<typeof rpc.VideoInstantCreate>[0]) => + rpc.VideoInstantCreate(params), + });Then replace the direct call:
- const creation = await EffectRuntime.runPromise( - rpc.VideoInstantCreate({ - orgId: Organisation.OrganisationId.make(organisationId), - folderId: Option.none(), - resolution, - width, - height, - videoCodec: "h264", - audioCodec: hasAudio ? "aac" : undefined, - supportsUploadProgress: true, - }), - ); + const creation = await createInstantVideo.mutateAsync({ + orgId: Organisation.OrganisationId.make(organisationId), + folderId: Option.none(), + resolution, + width, + height, + videoCodec: "h264", + audioCodec: hasAudio ? "aac" : undefined, + supportsUploadProgress: true, + });As per coding guidelines
🧹 Nitpick comments (1)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts (1)
898-922: Free-plan auto-stop implementation is correct.The typo flagged in the previous review ("oast" instead of "toast") has been fixed at line 915. The current implementation correctly uses
toast.info().The
freePlanAutoStopTriggeredRefguard prevents duplicate stop calls, andstopRecording()includes its own phase check (line 670:if (phase !== "recording" && phase !== "paused") return;), so race conditions are adequately handled.As an optional defensive improvement, you could add an explicit phase check before calling
stopRecording()to avoid queueing an async operation when unnecessary:if ( durationMs >= FREE_PLAN_MAX_RECORDING_MS && !freePlanAutoStopTriggeredRef.current ) { freePlanAutoStopTriggeredRef.current = true; toast.info( "Free plan recordings are limited to 5 minutes. Recording stopped automatically.", ); + if (phase !== "recording" && phase !== "paused") return; stopRecording().catch((error) => { console.error("Failed to stop recording at free plan limit", error); }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts
🧠 Learnings (2)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/**/*.{ts,tsx,js,jsx} : On the client, always use `useEffectQuery` or `useEffectMutation` from `@/lib/EffectRuntime`; never call `EffectRuntime.run*` directly in components.
Applied to files:
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/app/api/* : On the server, always run effects through `EffectRuntime.runPromise` after `provideOptionalAuth` to ensure cookies and per-request context are attached.
Applied to files:
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts
🧬 Code graph analysis (1)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts (15)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/RecordingModeSelector.tsx (1)
RecordingMode(18-18)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-types.ts (4)
RecorderPhase(1-9)VideoId(15-15)ChunkUploadState(17-23)PresignedPost(14-14)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-utils.ts (5)
RecorderCapabilities(9-14)detectCapabilities(16-34)DetectedDisplayRecordingMode(7-7)shouldRetryDisplayMediaWithoutPreferences(85-94)pickSupportedMimeType(78-83)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useStreamManagement.ts (1)
useStreamManagement(3-59)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useRecordingTimer.ts (1)
useRecordingTimer(3-102)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMediaRecorderSetup.ts (1)
useMediaRecorderSetup(4-93)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/instant-mp4-uploader.ts (2)
InstantMp4Uploader(100-391)initiateMultipartUpload(42-52)apps/web/lib/EffectRuntime.ts (3)
useRpcClient(25-25)useEffectMutation(23-23)EffectRuntime(20-20)apps/web/app/(org)/dashboard/caps/UploadingContext.tsx (1)
useUploadingContext(41-48)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-constants.ts (9)
RECORDING_MODE_TO_DISPLAY_SURFACE(93-100)DetectedDisplayRecordingMode(46-49)DisplaySurfacePreference(51-55)DISPLAY_MEDIA_VIDEO_CONSTRAINTS(33-37)ExtendedDisplayMediaStreamOptions(39-44)DISPLAY_MODE_PREFERENCES(57-79)MP4_MIME_TYPES(102-112)WEBM_MIME_TYPES(114-117)FREE_PLAN_MAX_RECORDING_MS(121-121)apps/web/app/(org)/dashboard/caps/components/sendProgressUpdate.ts (1)
sendProgressUpdate(5-24)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/recording-conversion.ts (2)
convertToMp4(95-137)captureThumbnail(8-93)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/recording-upload.ts (1)
uploadRecording(5-67)apps/web/actions/video/upload.ts (2)
createVideoAndGetUploadUrl(138-265)deleteVideoResultFile(267-331)apps/web/lib/Requests/ThumbnailRequest.ts (1)
ThumbnailRequest(10-14)
⏰ 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: Analyze (rust)
🔇 Additional comments (3)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts (3)
195-197: LGTM: Mutation pattern correctly implemented.The
deleteVideomutation is properly defined usinguseEffectMutation, following the coding guidelines for client-side RPC calls.
924-962: LGTM: Restart logic is well-structured.The
restartRecordingfunction properly handles the complex flow of stopping, cleaning up (with optional instant video preservation), deleting the previous result file, and restarting. TheisRestartingguard prevents concurrent restart operations.
964-1003: LGTM: Clean API surface.The returned object provides a clear, well-documented API for consumers, with appropriate boolean flags for UI state management and control functions for the recording lifecycle.
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/app/api/upload/[...route]/multipart.ts (1)
1-24: Run Biome to fix import orderingCI (“file not in the expected formatted state after Biome organize-imports”) is currently red. Please run
pnpm format(or Biome’s organize-imports) so the import block matches the expected ordering and the pipeline passes.
🧹 Nitpick comments (1)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts (1)
558-558: Consider refactoringsendProgressUpdateto use mutations.The
sendProgressUpdateutility function (imported at line 16) internally callsEffectRuntime.runPromise, which doesn't follow the pattern of usinguseEffectMutation. While this is a utility function rather than direct component code, aligning it with the mutation pattern would be more consistent with the coding guidelines.One approach: define a
VideoUploadProgressUpdatemutation at the hook level and pass its trigger function down toInstantMp4Uploaderinstead of the rawsendProgressUpdateutility. However, given that this pattern is used in multiple places and callbacks, refactoring may introduce complexity.As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts(1 hunks)apps/web/app/api/upload/[...route]/multipart.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.tsapps/web/app/api/upload/[...route]/multipart.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.tsapps/web/app/api/upload/[...route]/multipart.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.tsapps/web/app/api/upload/[...route]/multipart.ts
🧠 Learnings (2)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/**/*.{ts,tsx,js,jsx} : On the client, always use `useEffectQuery` or `useEffectMutation` from `@/lib/EffectRuntime`; never call `EffectRuntime.run*` directly in components.
Applied to files:
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/app/api/* : On the server, always run effects through `EffectRuntime.runPromise` after `provideOptionalAuth` to ensure cookies and per-request context are attached.
Applied to files:
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.tsapps/web/app/api/upload/[...route]/multipart.ts
🧬 Code graph analysis (2)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts (16)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/RecordingModeSelector.tsx (1)
RecordingMode(18-18)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-types.ts (4)
RecorderPhase(1-9)VideoId(15-15)ChunkUploadState(17-23)PresignedPost(14-14)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-utils.ts (5)
RecorderCapabilities(9-14)detectCapabilities(16-34)DetectedDisplayRecordingMode(7-7)shouldRetryDisplayMediaWithoutPreferences(85-94)pickSupportedMimeType(78-83)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useStreamManagement.ts (1)
useStreamManagement(3-59)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useRecordingTimer.ts (1)
useRecordingTimer(3-102)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMediaRecorderSetup.ts (1)
useMediaRecorderSetup(4-93)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useSurfaceDetection.ts (1)
useSurfaceDetection(7-95)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/instant-mp4-uploader.ts (2)
InstantMp4Uploader(100-391)initiateMultipartUpload(42-52)apps/web/lib/EffectRuntime.ts (2)
useRpcClient(25-25)useEffectMutation(23-23)apps/web/app/(org)/dashboard/caps/UploadingContext.tsx (1)
useUploadingContext(41-48)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-constants.ts (9)
RECORDING_MODE_TO_DISPLAY_SURFACE(93-100)DetectedDisplayRecordingMode(46-49)DisplaySurfacePreference(51-55)DISPLAY_MEDIA_VIDEO_CONSTRAINTS(33-37)ExtendedDisplayMediaStreamOptions(39-44)DISPLAY_MODE_PREFERENCES(57-79)MP4_MIME_TYPES(102-112)WEBM_MIME_TYPES(114-117)FREE_PLAN_MAX_RECORDING_MS(121-121)apps/web/app/(org)/dashboard/caps/components/sendProgressUpdate.ts (1)
sendProgressUpdate(5-24)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/recording-conversion.ts (2)
convertToMp4(95-137)captureThumbnail(8-93)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/recording-upload.ts (1)
uploadRecording(5-67)apps/web/actions/video/upload.ts (2)
createVideoAndGetUploadUrl(138-265)deleteVideoResultFile(267-331)apps/web/lib/Requests/ThumbnailRequest.ts (1)
ThumbnailRequest(10-14)
apps/web/app/api/upload/[...route]/multipart.ts (6)
apps/web/lib/server.ts (1)
runPromise(139-151)apps/web/app/api/upload/utils.ts (1)
parseVideoIdOrFileKey(1-24)packages/web-backend/src/Videos/VideosRepo.ts (1)
VideosRepo(15-106)packages/web-backend/src/Videos/VideosPolicy.ts (1)
VideosPolicy(9-96)packages/web-backend/src/Database.ts (1)
Database(7-17)packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-211)
🪛 GitHub Actions: CI
apps/web/app/api/upload/[...route]/multipart.ts
[error] 1-1: File content differs from formatting output. The file is not in the expected formatted state after Biome organize-imports.
[error] 1-1: The imports and exports are not sorted. Organize imports to fix ordering.
⏰ 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: Analyze (rust)
🔇 Additional comments (1)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts (1)
1-1016: Excellent implementation of the web recorder lifecycle.The hook demonstrates comprehensive handling of the recording flow, including:
- Robust capability detection and browser compatibility checks
- Proper resource cleanup (streams, timers, URLs) with cleanup effects
- Defensive guards against race conditions (refs for instant chunking, free-plan auto-stop, stopping state)
- Thorough error handling with user-facing toast notifications and fallback behaviors
- Correct use of
useEffectMutationpattern for RPC calls throughout- Free-plan auto-stop effect (lines 911-935) includes proper phase checks before triggering stop
The instant MP4 upload orchestration with fallback to WebM conversion is particularly well-designed, and the integration with multipart upload provides good user experience for chunked progress.
| const startRecording = async (options?: { reuseInstantVideo?: boolean }) => { | ||
| if (!organisationId) { | ||
| toast.error("Select an organization before recording."); | ||
| return; | ||
| } | ||
|
|
||
| if (recordingMode === "camera" && !selectedCameraId) { | ||
| toast.error("Select a camera before recording."); | ||
| return; | ||
| } | ||
|
|
||
| if (!isBrowserSupported) { | ||
| const fallbackMessage = | ||
| unsupportedReason ?? | ||
| "Recording isn't supported in this browser. Try another browser or use the desktop app."; | ||
| toast.error(fallbackMessage); | ||
| return; | ||
| } | ||
|
|
||
| setChunkUploads([]); | ||
| setIsSettingUp(true); | ||
|
|
||
| try { | ||
| let videoStream: MediaStream | null = null; | ||
| let firstTrack: MediaStreamTrack | null = null; | ||
|
|
||
| if (recordingMode === "camera") { | ||
| if (!selectedCameraId) { | ||
| throw new Error("Camera ID is required for camera-only mode"); | ||
| } | ||
| videoStream = await navigator.mediaDevices.getUserMedia({ | ||
| video: { | ||
| deviceId: { exact: selectedCameraId }, | ||
| frameRate: { ideal: 30 }, | ||
| width: { ideal: 1920 }, | ||
| height: { ideal: 1080 }, | ||
| }, | ||
| }); | ||
| cameraStreamRef.current = videoStream; | ||
| firstTrack = videoStream.getVideoTracks()[0] ?? null; | ||
| } else { | ||
| const desiredSurface = | ||
| RECORDING_MODE_TO_DISPLAY_SURFACE[ | ||
| recordingMode as DetectedDisplayRecordingMode | ||
| ]; | ||
| const videoConstraints: MediaTrackConstraints & { | ||
| displaySurface?: DisplaySurfacePreference; | ||
| } = { | ||
| ...DISPLAY_MEDIA_VIDEO_CONSTRAINTS, | ||
| displaySurface: desiredSurface, | ||
| }; | ||
|
|
||
| const baseDisplayRequest: ExtendedDisplayMediaStreamOptions = { | ||
| video: videoConstraints, | ||
| audio: false, | ||
| preferCurrentTab: recordingMode === "tab", | ||
| }; | ||
|
|
||
| const preferredOptions = DISPLAY_MODE_PREFERENCES[recordingMode]; | ||
|
|
||
| if (preferredOptions) { | ||
| const preferredDisplayRequest: DisplayMediaStreamOptions = { | ||
| ...baseDisplayRequest, | ||
| ...preferredOptions, | ||
| video: videoConstraints, | ||
| }; | ||
|
|
||
| try { | ||
| videoStream = await navigator.mediaDevices.getDisplayMedia( | ||
| preferredDisplayRequest, | ||
| ); | ||
| } catch (displayError) { | ||
| if (shouldRetryDisplayMediaWithoutPreferences(displayError)) { | ||
| console.warn( | ||
| "Display media preferences not supported, retrying without them", | ||
| displayError, | ||
| ); | ||
| videoStream = | ||
| await navigator.mediaDevices.getDisplayMedia( | ||
| baseDisplayRequest, | ||
| ); | ||
| } else { | ||
| throw displayError; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (!videoStream) { | ||
| videoStream = | ||
| await navigator.mediaDevices.getDisplayMedia(baseDisplayRequest); | ||
| } | ||
| displayStreamRef.current = videoStream; | ||
| firstTrack = videoStream.getVideoTracks()[0] ?? null; | ||
| } | ||
|
|
||
| const settings = firstTrack?.getSettings(); | ||
|
|
||
| if (recordingMode !== "camera") { | ||
| scheduleSurfaceDetection(firstTrack, settings); | ||
| } | ||
|
|
||
| dimensionsRef.current = { | ||
| width: settings?.width || undefined, | ||
| height: settings?.height || undefined, | ||
| }; | ||
|
|
||
| let micStream: MediaStream | null = null; | ||
| if (micEnabled && selectedMicId) { | ||
| try { | ||
| micStream = await navigator.mediaDevices.getUserMedia({ | ||
| audio: { | ||
| deviceId: { exact: selectedMicId }, | ||
| echoCancellation: true, | ||
| autoGainControl: true, | ||
| noiseSuppression: true, | ||
| }, | ||
| }); | ||
| } catch (micError) { | ||
| console.warn("Microphone permission denied", micError); | ||
| toast.warning("Microphone unavailable. Recording without audio."); | ||
| micStream = null; | ||
| } | ||
| } | ||
|
|
||
| if (micStream) { | ||
| micStreamRef.current = micStream; | ||
| } | ||
|
|
||
| const mixedStream = new MediaStream([ | ||
| ...videoStream.getVideoTracks(), | ||
| ...(micStream ? micStream.getAudioTracks() : []), | ||
| ]); | ||
|
|
||
| mixedStreamRef.current = mixedStream; | ||
| const hasAudio = mixedStream.getAudioTracks().length > 0; | ||
| setHasAudioTrack(hasAudio); | ||
|
|
||
| recordedChunksRef.current = []; | ||
| totalRecordedBytesRef.current = 0; | ||
| instantUploaderRef.current = null; | ||
| instantMp4ActiveRef.current = false; | ||
|
|
||
| const mp4Candidates = hasAudio | ||
| ? [...MP4_MIME_TYPES.withAudio, ...MP4_MIME_TYPES.videoOnly] | ||
| : [...MP4_MIME_TYPES.videoOnly, ...MP4_MIME_TYPES.withAudio]; | ||
| const supportedMp4MimeType = pickSupportedMimeType(mp4Candidates); | ||
| const webmCandidates = hasAudio | ||
| ? [...WEBM_MIME_TYPES.withAudio, ...WEBM_MIME_TYPES.videoOnly] | ||
| : [...WEBM_MIME_TYPES.videoOnly, ...WEBM_MIME_TYPES.withAudio]; | ||
| const fallbackMimeType = pickSupportedMimeType(webmCandidates); | ||
| const mimeType = supportedMp4MimeType ?? fallbackMimeType; | ||
| const useInstantMp4 = Boolean(supportedMp4MimeType); | ||
| instantMp4ActiveRef.current = useInstantMp4; | ||
| const shouldReuseInstantVideo = Boolean( | ||
| options?.reuseInstantVideo && videoCreationRef.current, | ||
| ); | ||
|
|
||
| if (useInstantMp4) { | ||
| let creationResult = videoCreationRef.current; | ||
| const width = dimensionsRef.current.width; | ||
| const height = dimensionsRef.current.height; | ||
| const resolution = width && height ? `${width}x${height}` : undefined; | ||
| if (!shouldReuseInstantVideo || !creationResult) { | ||
| const creation = unwrapExitOrThrow( | ||
| await videoInstantCreate.mutateAsync({ | ||
| orgId: Organisation.OrganisationId.make(organisationId), | ||
| folderId: Option.none(), | ||
| resolution, | ||
| width, | ||
| height, | ||
| videoCodec: "h264", | ||
| audioCodec: hasAudio ? "aac" : undefined, | ||
| supportsUploadProgress: true, | ||
| }), | ||
| ); | ||
| creationResult = { | ||
| id: creation.id, | ||
| upload: creation.upload, | ||
| shareUrl: creation.shareUrl, | ||
| }; | ||
| videoCreationRef.current = creationResult; | ||
| } | ||
| if (creationResult) { | ||
| setVideoId(creationResult.id); | ||
| pendingInstantVideoIdRef.current = creationResult.id; | ||
| } | ||
|
|
||
| let uploadId: string | null = null; | ||
| try { | ||
| if (!creationResult) | ||
| throw new Error("Missing instant recording context"); | ||
| uploadId = await initiateMultipartUpload(creationResult.id); | ||
| } catch (initError) { | ||
| const orphanId = creationResult?.id; | ||
| if (orphanId) { | ||
| await deleteVideo.mutateAsync(orphanId); | ||
| } | ||
| pendingInstantVideoIdRef.current = null; | ||
| videoCreationRef.current = null; | ||
| throw initError; | ||
| } | ||
|
|
||
| if (!creationResult) { | ||
| throw new Error("Instant recording metadata missing"); | ||
| } | ||
| instantUploaderRef.current = new InstantMp4Uploader({ | ||
| videoId: creationResult.id, | ||
| uploadId, | ||
| mimeType: supportedMp4MimeType ?? "", | ||
| setUploadStatus, | ||
| sendProgressUpdate: (uploaded, total) => | ||
| sendProgressUpdate(creationResult.id, uploaded, total), | ||
| onChunkStateChange: setChunkUploads, | ||
| }); | ||
| } else { | ||
| if (!shouldReuseInstantVideo) { | ||
| videoCreationRef.current = null; | ||
| pendingInstantVideoIdRef.current = null; | ||
| } | ||
| } | ||
|
|
||
| const recorder = new MediaRecorder( | ||
| mixedStream, | ||
| mimeType ? { mimeType } : undefined, | ||
| ); | ||
| recorder.ondataavailable = handleRecorderDataAvailable; | ||
| recorder.onstop = onRecorderStop; | ||
| recorder.onerror = onRecorderError; | ||
|
|
||
| const handleVideoEnded = () => { | ||
| stopRecordingRef.current?.().catch(() => { | ||
| /* ignore */ | ||
| }); | ||
| }; | ||
|
|
||
| firstTrack?.addEventListener("ended", handleVideoEnded, { once: true }); | ||
|
|
||
| mediaRecorderRef.current = recorder; | ||
| instantChunkModeRef.current = null; | ||
| lastInstantChunkAtRef.current = null; | ||
| clearInstantChunkGuard(); | ||
| stopInstantChunkInterval(); | ||
| if (useInstantMp4) { | ||
| let startedWithTimeslice = false; | ||
| try { | ||
| recorder.start(INSTANT_UPLOAD_REQUEST_INTERVAL_MS); | ||
| instantChunkModeRef.current = "timeslice"; | ||
| startedWithTimeslice = true; | ||
| } catch (startError) { | ||
| console.warn( | ||
| "Failed to start recorder with timeslice chunks, falling back to manual flush", | ||
| startError, | ||
| ); | ||
| } | ||
|
|
||
| if (startedWithTimeslice) { | ||
| scheduleInstantChunkGuard(); | ||
| } else { | ||
| recorder.start(); | ||
| beginManualInstantChunking(); | ||
| } | ||
| } else { | ||
| recorder.start(200); | ||
| } | ||
| onRecordingStart?.(); | ||
|
|
||
| startTimer(); | ||
| updatePhase("recording"); | ||
| } catch (err) { | ||
| const orphanVideoId = | ||
| instantMp4ActiveRef.current && videoCreationRef.current?.id | ||
| ? videoCreationRef.current.id | ||
| : null; | ||
| if (orphanVideoId) { | ||
| instantUploaderRef.current = null; | ||
| instantMp4ActiveRef.current = false; | ||
| videoCreationRef.current = null; | ||
| pendingInstantVideoIdRef.current = null; | ||
| await deleteVideo.mutateAsync(orphanVideoId); | ||
| } | ||
|
|
||
| console.error("Failed to start recording", err); | ||
| toast.error("Could not start recording."); | ||
| resetState(); | ||
| } finally { | ||
| setIsSettingUp(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Wrap startRecording in useCallback for consistency.
All other returned functions (pauseRecording, resumeRecording, stopRecording, restartRecording, resetState) are wrapped in useCallback, but startRecording is defined as a plain async function. This inconsistency could cause unnecessary re-renders in consuming components that use this function in dependency arrays.
Wrap the function declaration:
-const startRecording = async (options?: { reuseInstantVideo?: boolean }) => {
+const startRecording = useCallback(async (options?: { reuseInstantVideo?: boolean }) => {
if (!organisationId) {
toast.error("Select an organization before recording.");
return;
}
// ... rest of implementation
-};
+}, [/* add appropriate dependencies */]);Note: You'll need to carefully determine the complete dependency list. At minimum, it should include: organisationId, recordingMode, selectedCameraId, isBrowserSupported, unsupportedReason, and likely many of the callback refs and state setters.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts
around lines 347 to 634, startRecording is declared as a plain async function
while other exported controls are wrapped in useCallback, causing inconsistent
identity and possible unnecessary re-renders; change startRecording to be
created with useCallback(async (options?) => { ... }, [/* precise dependencies
*/]) and include all values read from outer scope (at minimum: organisationId,
recordingMode, selectedCameraId, isBrowserSupported, unsupportedReason,
micEnabled, selectedMicId, videoInstantCreate, initiateMultipartUpload,
deleteVideo, videoCreationRef.current, instantMp4ActiveRef.current,
pendingInstantVideoIdRef.current, instantUploaderRef.current, mediaRecorderRef,
cameraStreamRef, displayStreamRef, micStreamRef, mixedStreamRef, dimensionsRef,
setChunkUploads, setIsSettingUp, setHasAudioTrack, setVideoId, setUploadStatus,
setChunkUploads, onRecordingStart, startTimer, updatePhase,
scheduleSurfaceDetection, shouldRetryDisplayMediaWithoutPreferences,
pickSupportedMimeType, MP4_MIME_TYPES, WEBM_MIME_TYPES,
RECORDING_MODE_TO_DISPLAY_SURFACE, DISPLAY_MEDIA_VIDEO_CONSTRAINTS,
DISPLAY_MODE_PREFERENCES, Option, Organisation, unwrapExitOrThrow,
InstantMp4Uploader, INSTANT_UPLOAD_REQUEST_INTERVAL_MS,
handleRecorderDataAvailable, onRecorderStop, onRecorderError,
clearInstantChunkGuard, stopInstantChunkInterval, scheduleInstantChunkGuard,
beginManualInstantChunking, and any refs/state used inside) so the callback has
a complete dependency array and preserves stable identity.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts (1)
353-640:startRecordingstill not wrapped inuseCallback.This inconsistency was flagged in a previous review. All other control functions (
pauseRecording,resumeRecording,stopRecording,restartRecording) useuseCallback, butstartRecordingremains a plain async function. This can cause unnecessary re-renders in consuming components that includestartRecordingin dependency arrays.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts(1 hunks)apps/web/app/api/upload/[...route]/multipart.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/web/app/api/upload/[...route]/multipart.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/app/api/upload/[...route]/multipart.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/api/upload/[...route]/multipart.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts
🧠 Learnings (5)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/app/api/* : On the server, always run effects through `EffectRuntime.runPromise` after `provideOptionalAuth` to ensure cookies and per-request context are attached.
Applied to files:
apps/web/app/api/upload/[...route]/multipart.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/app/api/* : Convert effectful APIs to Next.js handlers using `apiToHandler(ApiLive)` and export only the returned `handler`; avoid calling `runPromise` directly in route files.
Applied to files:
apps/web/app/api/upload/[...route]/multipart.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/**/*.{ts,tsx,js,jsx} : On the client, always use `useEffectQuery` or `useEffectMutation` from `@/lib/EffectRuntime`; never call `EffectRuntime.run*` directly in components.
Applied to files:
apps/web/app/api/upload/[...route]/multipart.tsapps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/app/api/* : API routes in Next.js (`apps/web/app/api/*`) must use `effect/platform`'s `HttpApi` builder and follow the existing class/group/endpoint pattern; avoid ad-hoc handlers.
Applied to files:
apps/web/app/api/upload/[...route]/multipart.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/app/api/* : Acquire backend services inside `Effect.gen` and wire them using `Layer.provide`/`HttpApiBuilder.group`, translating errors to `HttpApiError` as appropriate.
Applied to files:
apps/web/app/api/upload/[...route]/multipart.ts
🧬 Code graph analysis (2)
apps/web/app/api/upload/[...route]/multipart.ts (5)
apps/web/lib/server.ts (1)
runPromise(139-151)apps/web/app/api/upload/utils.ts (1)
parseVideoIdOrFileKey(1-24)packages/web-backend/src/Videos/VideosRepo.ts (1)
VideosRepo(15-106)packages/web-backend/src/Videos/VideosPolicy.ts (1)
VideosPolicy(9-96)packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-211)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts (14)
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/RecordingModeSelector.tsx (1)
RecordingMode(18-18)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-types.ts (4)
RecorderPhase(1-9)VideoId(15-15)ChunkUploadState(17-23)PresignedPost(14-14)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-utils.ts (5)
RecorderCapabilities(9-14)detectCapabilities(16-34)DetectedDisplayRecordingMode(7-7)shouldRetryDisplayMediaWithoutPreferences(85-94)pickSupportedMimeType(78-83)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useStreamManagement.ts (1)
useStreamManagement(3-59)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useRecordingTimer.ts (1)
useRecordingTimer(3-102)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useMediaRecorderSetup.ts (1)
useMediaRecorderSetup(4-93)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useSurfaceDetection.ts (1)
useSurfaceDetection(7-95)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/instant-mp4-uploader.ts (2)
InstantMp4Uploader(100-391)initiateMultipartUpload(42-52)apps/web/lib/EffectRuntime.ts (2)
useRpcClient(25-25)useEffectMutation(23-23)apps/web/app/(org)/dashboard/caps/UploadingContext.tsx (1)
useUploadingContext(41-48)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/web-recorder-constants.ts (9)
RECORDING_MODE_TO_DISPLAY_SURFACE(93-100)DetectedDisplayRecordingMode(46-49)DisplaySurfacePreference(51-55)DISPLAY_MEDIA_VIDEO_CONSTRAINTS(33-37)ExtendedDisplayMediaStreamOptions(39-44)DISPLAY_MODE_PREFERENCES(57-79)MP4_MIME_TYPES(102-112)WEBM_MIME_TYPES(114-117)FREE_PLAN_MAX_RECORDING_MS(121-121)packages/web-domain/src/Organisation.ts (1)
Organisation(21-24)apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/recording-upload.ts (1)
uploadRecording(5-67)apps/web/actions/video/upload.ts (2)
createVideoAndGetUploadUrl(138-265)deleteVideoResultFile(267-331)
⏰ 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: Analyze (rust)
🔇 Additional comments (3)
apps/web/app/api/upload/[...route]/multipart.ts (3)
36-58: LGTM! Schema and validator properly structured.The abort request schema correctly mirrors the pattern used in other endpoints (supporting both
videoIdand deprecatedfileKey), and the validator typing withMiddlewareHandleris appropriately constrained.
514-575: Abort endpoint structure is sound.The endpoint correctly:
- Validates video ownership via policy
- Extracts and parses videoId/fileKey consistently with other endpoints
- Cleans up
videoUploadsrecords after aborting- Follows the error-handling pattern used in other multipart endpoints
The type assertion issue (lines 543-548) is flagged separately.
543-548: ****The abort method does exist on
bucket.multipartand the type assertion is safe. The multipart object in S3BucketAccess.ts defines anabortmethod with the same signature ascomplete, so the type assertion correctly reflects the actual available API. No runtime verification is needed.Likely an incorrect or invalid review comment.
| const stopRecording = useCallback(async () => { | ||
| stopInstantChunkInterval(); | ||
| clearInstantChunkGuard(); | ||
| instantChunkModeRef.current = null; | ||
| lastInstantChunkAtRef.current = null; | ||
| if (phase !== "recording" && phase !== "paused") return; | ||
|
|
||
| const orgId = organisationId; | ||
| if (!orgId) { | ||
| updatePhase("error"); | ||
| return; | ||
| } | ||
|
|
||
| const timestamp = performance.now(); | ||
| commitPausedDuration(timestamp); | ||
| const recordedDurationMs = syncDurationFromClock(timestamp); | ||
|
|
||
| const brandedOrgId = Organisation.OrganisationId.make(orgId); | ||
| let thumbnailBlob: Blob | null = null; | ||
| let thumbnailPreviewUrl: string | undefined; | ||
| let createdVideoId: VideoId | null = videoCreationRef.current?.id ?? null; | ||
| const instantUploader = instantUploaderRef.current; | ||
| const useInstantMp4 = Boolean(instantUploader); | ||
|
|
||
| try { | ||
| onRecordingStop?.(); | ||
| updatePhase("creating"); | ||
|
|
||
| const blob = await stopRecordingInternalWrapper(); | ||
| if (!blob) throw new Error("No recording available"); | ||
|
|
||
| const durationSeconds = Math.max( | ||
| 1, | ||
| Math.round(recordedDurationMs / 1000), | ||
| ); | ||
| const width = dimensionsRef.current.width; | ||
| const height = dimensionsRef.current.height; | ||
| const resolution = width && height ? `${width}x${height}` : undefined; | ||
|
|
||
| setUploadStatus({ status: "creating" }); | ||
|
|
||
| let creationResult = videoCreationRef.current; | ||
| if (!creationResult) { | ||
| const result = unwrapExitOrThrow( | ||
| await videoInstantCreate.mutateAsync({ | ||
| orgId: brandedOrgId, | ||
| folderId: Option.none(), | ||
| resolution, | ||
| durationSeconds, | ||
| width, | ||
| height, | ||
| videoCodec: "h264", | ||
| audioCodec: hasAudioTrack ? "aac" : undefined, | ||
| supportsUploadProgress: true, | ||
| }), | ||
| ); | ||
| creationResult = { | ||
| id: result.id, | ||
| upload: result.upload, | ||
| shareUrl: result.shareUrl, | ||
| }; | ||
| videoCreationRef.current = creationResult; | ||
| setVideoId(result.id); | ||
| } | ||
|
|
||
| createdVideoId = creationResult.id; | ||
|
|
||
| let mp4Blob: Blob; | ||
| if (useInstantMp4) { | ||
| mp4Blob = | ||
| blob.type === "video/mp4" | ||
| ? blob | ||
| : new File([blob], "result.mp4", { type: "video/mp4" }); | ||
| } else { | ||
| mp4Blob = await convertToMp4( | ||
| blob, | ||
| hasAudioTrack, | ||
| creationResult.id, | ||
| setUploadStatus, | ||
| updatePhase, | ||
| ); | ||
| } | ||
|
|
||
| thumbnailBlob = await captureThumbnail(mp4Blob, dimensionsRef.current); | ||
| thumbnailPreviewUrl = thumbnailBlob | ||
| ? URL.createObjectURL(thumbnailBlob) | ||
| : undefined; | ||
|
|
||
| updatePhase("uploading"); | ||
| setUploadStatus({ | ||
| status: "uploadingVideo", | ||
| capId: creationResult.id, | ||
| progress: 0, | ||
| thumbnailUrl: thumbnailPreviewUrl, | ||
| }); | ||
|
|
||
| if (useInstantMp4 && instantUploader) { | ||
| instantUploader.setThumbnailUrl(thumbnailPreviewUrl); | ||
| await instantUploader.finalize({ | ||
| finalBlob: mp4Blob, | ||
| durationSeconds, | ||
| width, | ||
| height, | ||
| thumbnailUrl: thumbnailPreviewUrl, | ||
| }); | ||
| instantUploaderRef.current = null; | ||
| instantMp4ActiveRef.current = false; | ||
| } else { | ||
| await uploadRecording( | ||
| mp4Blob, | ||
| creationResult.upload, | ||
| creationResult.id, | ||
| thumbnailPreviewUrl, | ||
| setUploadStatus, | ||
| ); | ||
| } | ||
|
|
||
| pendingInstantVideoIdRef.current = null; | ||
|
|
||
| if (thumbnailBlob) { | ||
| try { | ||
| const screenshotData = await createVideoAndGetUploadUrl({ | ||
| videoId: creationResult.id, | ||
| isScreenshot: true, | ||
| orgId: brandedOrgId, | ||
| }); | ||
|
|
||
| const screenshotFormData = new FormData(); | ||
| Object.entries(screenshotData.presignedPostData.fields).forEach( | ||
| ([key, value]) => { | ||
| screenshotFormData.append(key, value as string); | ||
| }, | ||
| ); | ||
| screenshotFormData.append( | ||
| "file", | ||
| thumbnailBlob, | ||
| "screen-capture.jpg", | ||
| ); | ||
|
|
||
| setUploadStatus({ | ||
| status: "uploadingThumbnail", | ||
| capId: creationResult.id, | ||
| progress: 90, | ||
| }); | ||
|
|
||
| await new Promise<void>((resolve, reject) => { | ||
| const xhr = new XMLHttpRequest(); | ||
| xhr.open("POST", screenshotData.presignedPostData.url); | ||
|
|
||
| xhr.upload.onprogress = (event) => { | ||
| if (event.lengthComputable) { | ||
| const percent = 90 + (event.loaded / event.total) * 10; | ||
| setUploadStatus({ | ||
| status: "uploadingThumbnail", | ||
| capId: creationResult.id, | ||
| progress: percent, | ||
| }); | ||
| } | ||
| }; | ||
|
|
||
| xhr.onload = () => { | ||
| if (xhr.status >= 200 && xhr.status < 300) { | ||
| resolve(); | ||
| } else { | ||
| reject( | ||
| new Error( | ||
| `Screenshot upload failed with status ${xhr.status}`, | ||
| ), | ||
| ); | ||
| } | ||
| }; | ||
|
|
||
| xhr.onerror = () => { | ||
| reject(new Error("Screenshot upload failed")); | ||
| }; | ||
|
|
||
| xhr.send(screenshotFormData); | ||
| }); | ||
|
|
||
| queryClient.refetchQueries({ | ||
| queryKey: ThumbnailRequest.queryKey(creationResult.id), | ||
| }); | ||
| } catch (thumbnailError) { | ||
| console.error("Failed to upload thumbnail", thumbnailError); | ||
| toast.warning("Recording uploaded, but thumbnail failed to upload."); | ||
| } | ||
| } | ||
|
|
||
| setUploadStatus(undefined); | ||
| updatePhase("completed"); | ||
| toast.success("Recording uploaded"); | ||
| if (creationResult.shareUrl) { | ||
| window.open(creationResult.shareUrl, "_blank", "noopener,noreferrer"); | ||
| } | ||
| router.refresh(); | ||
| } catch (err) { | ||
| console.error("Failed to process recording", err); | ||
| setUploadStatus(undefined); | ||
| updatePhase("error"); | ||
|
|
||
| const idToDelete = createdVideoId ?? videoId; | ||
| if (idToDelete) { | ||
| await deleteVideo.mutateAsync(idToDelete); | ||
| if (pendingInstantVideoIdRef.current === idToDelete) { | ||
| pendingInstantVideoIdRef.current = null; | ||
| } | ||
| } | ||
| } finally { | ||
| if (thumbnailPreviewUrl) { | ||
| URL.revokeObjectURL(thumbnailPreviewUrl); | ||
| } | ||
| } | ||
| }, [ | ||
| stopInstantChunkInterval, | ||
| phase, | ||
| organisationId, | ||
| hasAudioTrack, | ||
| videoId, | ||
| updatePhase, | ||
| setUploadStatus, | ||
| deleteVideo, | ||
| videoInstantCreate, | ||
| router, | ||
| stopRecordingInternalWrapper, | ||
| queryClient, | ||
| onRecordingStop, | ||
| commitPausedDuration, | ||
| syncDurationFromClock, | ||
| ]); |
There was a problem hiding this comment.
Add missing dependency clearInstantChunkGuard to useCallback array.
Line 685 calls clearInstantChunkGuard(), but the dependency array (lines 895–911) does not include it. This violates the exhaustive-deps rule and can cause stale closures if clearInstantChunkGuard is recreated.
Apply this diff:
}, [
stopInstantChunkInterval,
+ clearInstantChunkGuard,
phase,
organisationId,
hasAudioTrack,🤖 Prompt for AI Agents
In
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts
around lines 683 to 911, the useCallback for stopRecording calls
clearInstantChunkGuard() (line ~685) but the dependency array (lines ~895–911)
omits clearInstantChunkGuard; add clearInstantChunkGuard to that dependency
array so the callback updates when it changes (and run lint to confirm
exhaustive-deps compliance).
| const restartRecording = useCallback(async () => { | ||
| if (isRestarting) return; | ||
| if (phase !== "recording" && phase !== "paused") return; | ||
|
|
||
| const creationToReuse = videoCreationRef.current; | ||
| const shouldReuseInstantVideo = Boolean(creationToReuse); | ||
| setIsRestarting(true); | ||
|
|
||
| try { | ||
| try { | ||
| await stopRecordingInternalWrapper(); | ||
| } catch (error) { | ||
| console.warn("Failed to stop recorder before restart", error); | ||
| } | ||
|
|
||
| cleanupRecordingState({ preserveInstantVideo: shouldReuseInstantVideo }); | ||
| updatePhase("idle"); | ||
|
|
||
| if (shouldReuseInstantVideo && creationToReuse) { | ||
| await deleteVideoResultFile({ videoId: creationToReuse.id }); | ||
| } | ||
|
|
||
| await startRecording({ reuseInstantVideo: shouldReuseInstantVideo }); | ||
| } catch (error) { | ||
| console.error("Failed to restart recording", error); | ||
| toast.error("Could not restart recording. Please try again."); | ||
| cleanupRecordingState(); | ||
| updatePhase("idle"); | ||
| } finally { | ||
| setIsRestarting(false); | ||
| } | ||
| }, [ | ||
| cleanupRecordingState, | ||
| isRestarting, | ||
| phase, | ||
| startRecording, | ||
| stopRecordingInternalWrapper, | ||
| updatePhase, | ||
| ]); |
There was a problem hiding this comment.
Add missing dependency deleteVideoResultFile to useCallback array.
Line 962 invokes deleteVideoResultFile, but the dependency array (lines 974–981) omits it. Include it to satisfy the exhaustive-deps rule.
Apply this diff:
}, [
cleanupRecordingState,
+ deleteVideoResultFile,
isRestarting,
phase,📝 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 restartRecording = useCallback(async () => { | |
| if (isRestarting) return; | |
| if (phase !== "recording" && phase !== "paused") return; | |
| const creationToReuse = videoCreationRef.current; | |
| const shouldReuseInstantVideo = Boolean(creationToReuse); | |
| setIsRestarting(true); | |
| try { | |
| try { | |
| await stopRecordingInternalWrapper(); | |
| } catch (error) { | |
| console.warn("Failed to stop recorder before restart", error); | |
| } | |
| cleanupRecordingState({ preserveInstantVideo: shouldReuseInstantVideo }); | |
| updatePhase("idle"); | |
| if (shouldReuseInstantVideo && creationToReuse) { | |
| await deleteVideoResultFile({ videoId: creationToReuse.id }); | |
| } | |
| await startRecording({ reuseInstantVideo: shouldReuseInstantVideo }); | |
| } catch (error) { | |
| console.error("Failed to restart recording", error); | |
| toast.error("Could not restart recording. Please try again."); | |
| cleanupRecordingState(); | |
| updatePhase("idle"); | |
| } finally { | |
| setIsRestarting(false); | |
| } | |
| }, [ | |
| cleanupRecordingState, | |
| isRestarting, | |
| phase, | |
| startRecording, | |
| stopRecordingInternalWrapper, | |
| updatePhase, | |
| ]); | |
| const restartRecording = useCallback(async () => { | |
| if (isRestarting) return; | |
| if (phase !== "recording" && phase !== "paused") return; | |
| const creationToReuse = videoCreationRef.current; | |
| const shouldReuseInstantVideo = Boolean(creationToReuse); | |
| setIsRestarting(true); | |
| try { | |
| try { | |
| await stopRecordingInternalWrapper(); | |
| } catch (error) { | |
| console.warn("Failed to stop recorder before restart", error); | |
| } | |
| cleanupRecordingState({ preserveInstantVideo: shouldReuseInstantVideo }); | |
| updatePhase("idle"); | |
| if (shouldReuseInstantVideo && creationToReuse) { | |
| await deleteVideoResultFile({ videoId: creationToReuse.id }); | |
| } | |
| await startRecording({ reuseInstantVideo: shouldReuseInstantVideo }); | |
| } catch (error) { | |
| console.error("Failed to restart recording", error); | |
| toast.error("Could not restart recording. Please try again."); | |
| cleanupRecordingState(); | |
| updatePhase("idle"); | |
| } finally { | |
| setIsRestarting(false); | |
| } | |
| }, [ | |
| cleanupRecordingState, | |
| deleteVideoResultFile, | |
| isRestarting, | |
| phase, | |
| startRecording, | |
| stopRecordingInternalWrapper, | |
| updatePhase, | |
| ]); |
🤖 Prompt for AI Agents
In
apps/web/app/(org)/dashboard/caps/components/web-recorder-dialog/useWebRecorder.ts
around lines 943 to 981, the restartRecording useCallback calls
deleteVideoResultFile at line ~962 but the dependency array (lines ~974–981)
omits it; add deleteVideoResultFile to the dependency array so the hook
correctly re-subscribes when that function changes and to satisfy
exhaustive-deps.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/web/app/api/upload/[...route]/multipart.ts (2)
511-515: Redundant user layer provision (same as line 112-113).Same issue as the
/initiateendpoint: bothmakeCurrentUserLayer(user)andprovideOptionalAuthare applied, creating redundancy. Based on learnings.Apply this pattern instead:
}).pipe( Effect.provide(makeCurrentUserLayer(user)), - provideOptionalAuth, runPromiseAnyEnv, );
577-580: Redundant user layer provision (same as line 112-113).Same redundancy issue: both
makeCurrentUserLayer(user)andprovideOptionalAuthare applied. Based on learnings.Apply this pattern instead:
}).pipe( Effect.catchAll((error) => { // ... error handling }), Effect.provide(makeCurrentUserLayer(user)), - provideOptionalAuth, runPromiseAnyEnv, );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/app/api/upload/[...route]/multipart.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/web/app/api/upload/[...route]/multipart.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/app/api/upload/[...route]/multipart.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/api/upload/[...route]/multipart.ts
🧠 Learnings (5)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/app/api/* : On the server, always run effects through `EffectRuntime.runPromise` after `provideOptionalAuth` to ensure cookies and per-request context are attached.
Applied to files:
apps/web/app/api/upload/[...route]/multipart.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/app/api/* : Convert effectful APIs to Next.js handlers using `apiToHandler(ApiLive)` and export only the returned `handler`; avoid calling `runPromise` directly in route files.
Applied to files:
apps/web/app/api/upload/[...route]/multipart.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/**/*.{ts,tsx,js,jsx} : On the client, always use `useEffectQuery` or `useEffectMutation` from `@/lib/EffectRuntime`; never call `EffectRuntime.run*` directly in components.
Applied to files:
apps/web/app/api/upload/[...route]/multipart.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/app/api/* : API routes in Next.js (`apps/web/app/api/*`) must use `effect/platform`'s `HttpApi` builder and follow the existing class/group/endpoint pattern; avoid ad-hoc handlers.
Applied to files:
apps/web/app/api/upload/[...route]/multipart.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/app/api/* : Acquire backend services inside `Effect.gen` and wire them using `Layer.provide`/`HttpApiBuilder.group`, translating errors to `HttpApiError` as appropriate.
Applied to files:
apps/web/app/api/upload/[...route]/multipart.ts
🧬 Code graph analysis (1)
apps/web/app/api/upload/[...route]/multipart.ts (6)
apps/web/lib/server.ts (1)
runPromise(139-151)packages/web-backend/src/Auth.ts (2)
provideOptionalAuth(102-114)makeCurrentUserLayer(48-50)apps/web/app/api/upload/utils.ts (1)
parseVideoIdOrFileKey(1-24)packages/web-backend/src/Videos/VideosRepo.ts (1)
VideosRepo(15-106)packages/web-backend/src/Database.ts (1)
Database(7-17)packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-211)
⏰ 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: Analyze (rust)
🔇 Additional comments (4)
apps/web/app/api/upload/[...route]/multipart.ts (4)
36-58: LGTM! Abort validation follows established patterns.The abort request schema and validator correctly mirror the structure used in other endpoints (videoId with deprecated fileKey fallback).
148-149: Inconsistent auth pattern compared to other endpoints.This endpoint uses
provideOptionalAuthalone, while/initiate(line 112-113) and/complete(line 512-513) combinemakeCurrentUserLayer(user)withprovideOptionalAuth. SincewithAuthmiddleware already provides the user, prefer using onlymakeCurrentUserLayer(user)consistently across all endpoints for clarity. Based on learnings.Apply this diff for consistency:
-}).pipe(provideOptionalAuth, runPromiseAnyEnv); +}).pipe(Effect.provide(makeCurrentUserLayer(user)), runPromiseAnyEnv);⛔ Skipped due to learnings
Learnt from: CR Repo: CapSoftware/Cap PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-09-22T14:19:56.010Z Learning: Applies to apps/web/app/api/* : On the server, always run effects through `EffectRuntime.runPromise` after `provideOptionalAuth` to ensure cookies and per-request context are attached.
547-556: Type assertion is unnecessary but not unsafe—abort method exists with correct signature.The review comment incorrectly flags this as a critical safety issue. The
abortmethod is already defined inS3BucketAccess.ts(line 287) with the exact same signature as thecompletemethod. The type assertion at lines 548–553 is redundant but valid—it's redefining a method that already exists with the correct parameters and return type. The runtime call will work without issue.Likely an incorrect or invalid review comment.
32-35: Type-unsafe cast is pragmatic workaround with valid but manageable risk.The
runPromiseAnyEnvcast toanydoes bypass Effect's dependency verification. TherunPromisesignature requiresEffect.Effect<A, E, Layer.Layer.Success<typeof Dependencies>>, butprovideOptionalAuthfrom the external package@cap/web-backendis generically typed, creating a type mismatch that forces the cast.However, the actual layer provisions appear correct: endpoints properly provide
makeCurrentUserLayer(user)followed byprovideOptionalAuthbefore execution. The cast itself doesn't hide missing dependencies—it's a workaround for type incompatibility between the external package's generic signature and the server's strict Dependencies type.Recommendation: Rather than removing the cast without a viable alternative, consider:
- Whether
@cap/web-backend'sprovideOptionalAuthcan be updated to return a more specific type compatible withLayer.Layer.Success<typeof Dependencies>- Adding a type-safe wrapper if this pattern appears in multiple endpoints
For now, the cast is acceptable since layer provisions are correct and this pattern matches your server guidelines.
| Effect.provide(makeCurrentUserLayer(user)), | ||
| runPromise, | ||
| provideOptionalAuth, | ||
| runPromiseAnyEnv, |
There was a problem hiding this comment.
Redundant user layer provision.
The effect already provides the user via makeCurrentUserLayer(user) at line 112, then calls provideOptionalAuth at line 113 which will attempt to retrieve and provide the user again. Since withAuth middleware already supplies the user, only makeCurrentUserLayer(user) should be necessary—calling both creates redundancy and provideOptionalAuth requires Database which hasn't been provided yet. Based on learnings.
Apply this pattern instead:
}).pipe(
Effect.tapError(Effect.logError),
Effect.catchAll((e) => {
// ... error handling
}),
Effect.provide(makeCurrentUserLayer(user)),
- provideOptionalAuth,
runPromiseAnyEnv,
);🤖 Prompt for AI Agents
In apps/web/app/api/upload/[...route]/multipart.ts around lines 112 to 114, the
code redundantly provides the current user twice by calling both
makeCurrentUserLayer(user) and provideOptionalAuth (the latter also depends on
Database which isn't provided); remove provideOptionalAuth from the
Effect.provide chain and keep only makeCurrentUserLayer(user) (and the existing
runPromiseAnyEnv) so the user is supplied once by the middleware-provided layer
and no Database dependency is required.
| const videoIdFromFileKey = fileKey.split("/")[1]; | ||
| const videoIdRaw = "videoId" in body ? body.videoId : videoIdFromFileKey; | ||
| if (!videoIdRaw) return c.text("Video id not found", 400); | ||
| const videoId = Video.VideoId.make(videoIdRaw); |
There was a problem hiding this comment.
Early return breaks Effect error handling.
Returning c.text("Video id not found", 400) directly at line 530 bypasses the Effect error handling pipeline and returns a Promise instead of an Effect, which will cause type errors (the handler expects Effect to be returned).
Move this validation inside the Effect.gen:
- const videoIdFromFileKey = fileKey.split("/")[1];
- const videoIdRaw = "videoId" in body ? body.videoId : videoIdFromFileKey;
- if (!videoIdRaw) return c.text("Video id not found", 400);
- const videoId = Video.VideoId.make(videoIdRaw);
-
return Effect.gen(function* () {
+ const videoIdFromFileKey = fileKey.split("/")[1];
+ const videoIdRaw = "videoId" in body ? body.videoId : videoIdFromFileKey;
+ if (!videoIdRaw) {
+ c.status(400);
+ return c.text("Video id not found");
+ }
+ const videoId = Video.VideoId.make(videoIdRaw);
+
const repo = yield* VideosRepo;🤖 Prompt for AI Agents
In apps/web/app/api/upload/[...route]/multipart.ts around lines 528 to 531, the
early return c.text("Video id not found", 400) escapes the Effect pipeline and
returns a Promise instead of an Effect; remove that early return and move the
validation into the Effect.gen block: compute videoIdFromFileKey and videoIdRaw
before creating the Effect, then inside Effect.gen check if videoIdRaw is falsy
and yield an appropriate Effect failure (e.g., Effect.fail with a typed error or
an Effect.succeed(HttpResponse with 400) depending on our convention), otherwise
construct videoId with Video.VideoId.make(videoIdRaw) and continue the rest of
the Effect flow so the handler always returns an Effect.
This PR introduces a new web based recorder. Will be more reliable for devices that are maybe lower powered, or do not support Cap Desktop.
Summary by CodeRabbit
New Features
Improvements
UI