Local upload tracking #1092
Local upload tracking #1092
Conversation
|
Warning Rate limit exceeded@oscartbeaumont has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 39 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 streaming multipart uploader plus a legacy uploader, desktop async API helpers for presign/complete/upload workflows, integrates upload state/progress into recording metadata and UI (feature-flagged), introduces an unpublished cap-api crate, and propagates related type/signature changes across desktop, web, and backend layers. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Desktop UI
participant App as Desktop App
participant API as Desktop API module
participant S3 as Storage
UI->>App: upload(video_id, file_path, meta, screenshot)
App->>API: upload_multipart_initiate(video_id)
API-->>App: upload_id
loop per chunk
App->>API: upload_multipart_presign_part(video_id, upload_id, part#, md5)
API-->>App: presigned_url
App->>S3: PUT part (presigned_url)
App-->>UI: emit UploadProgressEvent
end
App->>API: upload_multipart_complete(video_id, upload_id, parts, meta)
API-->>App: optional location
App->>API: upload_signed(screenshot_presign)
API-->>App: ok
App-->>UI: upload result (id, link)
sequenceDiagram
autonumber
participant Startup as App (startup)
participant Meta as Recording Meta Store
participant Up as Uploader
Startup->>Meta: list_recordings()
Meta-->>Startup: recordings with upload/status
alt any InProgress or Failed
Startup->>Up: resume_uploads(items)
Up-->>Startup: progress events (tauri)
else none
Startup-->>Startup: no pending uploads
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested reviewers
Poem
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 comment was marked as off-topic.
This comment was marked as off-topic.
|
Follow-up PRs:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/app/api/desktop/[...route]/video.ts (1)
82-96: Restore braces to fix syntax error.
if (video)now lacks a matching block opener, but the closing}remains. That leaves an unmatched brace and will fail to compile/parse.Apply this diff to restore the block:
- if (video) - return c.json({ + if (video) { + return c.json({ id: video.id, // All deprecated user_id: user.id, aws_region: "n/a", aws_bucket: "n/a", }); + }
🧹 Nitpick comments (2)
apps/web/actions/video/upload.ts (1)
158-282: Consider decomposing this function for improved maintainability.The
createVideoAndGetUploadUrlfunction handles multiple concerns: authorization, bucket configuration, video record creation, upload tracking initialization, presigned URL generation, third-party integrations (Dub), and cache revalidation.Extracting helper functions (e.g.,
initializeVideoRecord,setupUploadTracking,generatePresignedUrl) would improve testability and readability.apps/desktop/src/routes/(window-chrome)/settings/recordings.tsx (1)
79-80: Remove inline comment to follow repo conventionsPer the project guidelines for TS/TSX files, we avoid inline comments. Please drop the newly added comment above
refetchIntervalto stay compliant.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (26)
.github/workflows/ci.yml(1 hunks)apps/desktop/src-tauri/Cargo.toml(1 hunks)apps/desktop/src-tauri/src/api.rs(1 hunks)apps/desktop/src-tauri/src/general_settings.rs(3 hunks)apps/desktop/src-tauri/src/lib.rs(16 hunks)apps/desktop/src-tauri/src/main.rs(0 hunks)apps/desktop/src-tauri/src/recording.rs(23 hunks)apps/desktop/src-tauri/src/upload.rs(5 hunks)apps/desktop/src-tauri/src/upload_legacy.rs(1 hunks)apps/desktop/src-tauri/src/web_api.rs(1 hunks)apps/desktop/src/routes/(window-chrome)/settings/experimental.tsx(2 hunks)apps/desktop/src/routes/(window-chrome)/settings/general.tsx(1 hunks)apps/desktop/src/routes/(window-chrome)/settings/recordings.tsx(7 hunks)apps/desktop/src/utils/createEventListener.ts(1 hunks)apps/desktop/src/utils/tauri.ts(8 hunks)apps/web/actions/video/upload.ts(3 hunks)apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx(5 hunks)apps/web/app/api/desktop/[...route]/video.ts(1 hunks)crates/api/Cargo.toml(1 hunks)crates/api/src/lib.rs(1 hunks)crates/project/src/meta.rs(5 hunks)crates/recording/src/instant_recording.rs(1 hunks)crates/recording/src/studio_recording.rs(1 hunks)packages/ui-solid/src/auto-imports.d.ts(1 hunks)packages/web-backend/src/Videos/VideosRepo.ts(1 hunks)packages/web-backend/src/Videos/index.ts(0 hunks)
💤 Files with no reviewable changes (2)
- packages/web-backend/src/Videos/index.ts
- apps/desktop/src-tauri/src/main.rs
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
packages/web-backend/src/Videos/VideosRepo.tscrates/recording/src/instant_recording.rsapps/web/actions/video/upload.tsapps/desktop/src-tauri/src/general_settings.rspackages/ui-solid/src/auto-imports.d.tscrates/recording/src/studio_recording.rsapps/desktop/src/routes/(window-chrome)/settings/experimental.tsxapps/desktop/src/utils/createEventListener.tscrates/project/src/meta.rsapps/desktop/src-tauri/src/recording.rsapps/desktop/src-tauri/src/web_api.rsapps/desktop/src-tauri/src/lib.rsapps/desktop/src/routes/(window-chrome)/settings/general.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/desktop/src-tauri/src/upload.rsapps/web/app/api/desktop/[...route]/video.tsapps/desktop/src/utils/tauri.tscrates/api/src/lib.rsapps/desktop/src/routes/(window-chrome)/settings/recordings.tsxapps/desktop/src-tauri/src/upload_legacy.rsapps/desktop/src-tauri/src/api.rs
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{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/Videos/VideosRepo.tsapps/web/actions/video/upload.tspackages/ui-solid/src/auto-imports.d.tsapps/desktop/src/routes/(window-chrome)/settings/experimental.tsxapps/desktop/src/utils/createEventListener.tsapps/desktop/src/routes/(window-chrome)/settings/general.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/api/desktop/[...route]/video.tsapps/desktop/src/utils/tauri.tsapps/desktop/src/routes/(window-chrome)/settings/recordings.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:
packages/web-backend/src/Videos/VideosRepo.tsapps/web/actions/video/upload.tspackages/ui-solid/src/auto-imports.d.tsapps/desktop/src/routes/(window-chrome)/settings/experimental.tsxapps/desktop/src/utils/createEventListener.tsapps/desktop/src/routes/(window-chrome)/settings/general.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/api/desktop/[...route]/video.tsapps/desktop/src/utils/tauri.tsapps/desktop/src/routes/(window-chrome)/settings/recordings.tsx
crates/**/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
For desktop IPC, use tauri_specta derive/macros on Rust types/events; do not hand-roll bindings
Files:
crates/recording/src/instant_recording.rscrates/recording/src/studio_recording.rscrates/project/src/meta.rscrates/api/src/lib.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
crates/recording/src/instant_recording.rsapps/desktop/src-tauri/src/general_settings.rscrates/recording/src/studio_recording.rscrates/project/src/meta.rsapps/desktop/src-tauri/src/recording.rsapps/desktop/src-tauri/src/web_api.rsapps/desktop/src-tauri/src/lib.rsapps/desktop/src-tauri/src/upload.rscrates/api/src/lib.rsapps/desktop/src-tauri/src/upload_legacy.rsapps/desktop/src-tauri/src/api.rs
crates/*/src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Rust crates should place tests within the
src/and/or a siblingtests/directory for each crate insidecrates/*.
Files:
crates/recording/src/instant_recording.rscrates/recording/src/studio_recording.rscrates/project/src/meta.rscrates/api/src/lib.rs
apps/web/actions/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All Groq/OpenAI calls must be implemented in Next.js Server Actions under apps/web/actions; do not place AI calls elsewhere
Files:
apps/web/actions/video/upload.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Files:
apps/web/actions/video/upload.tsapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/api/desktop/[...route]/video.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.tsapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/api/desktop/[...route]/video.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: Do not manually import icons in the desktop app; rely on auto-imported icons
In the desktop app, use @tanstack/solid-query for server state management
Files:
apps/desktop/src/routes/(window-chrome)/settings/experimental.tsxapps/desktop/src/utils/createEventListener.tsapps/desktop/src/routes/(window-chrome)/settings/general.tsxapps/desktop/src/utils/tauri.tsapps/desktop/src/routes/(window-chrome)/settings/recordings.tsx
apps/web/app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components
Files:
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/api/desktop/[...route]/video.ts
**/tauri.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Never edit auto-generated IPC bindings file tauri.ts
Do not edit auto-generated files named
tauri.ts.
Files:
apps/desktop/src/utils/tauri.ts
🧬 Code graph analysis (14)
packages/web-backend/src/Videos/VideosRepo.ts (1)
packages/database/index.ts (1)
db(29-34)
crates/recording/src/instant_recording.rs (1)
apps/desktop/src/utils/tauri.ts (1)
InstantRecordingMeta(408-408)
apps/web/actions/video/upload.ts (3)
packages/utils/src/constants/plans.ts (1)
userIsPro(21-45)packages/database/index.ts (1)
db(29-34)packages/database/schema.ts (1)
videoUploads(656-662)
crates/recording/src/studio_recording.rs (2)
crates/rendering/src/project_recordings.rs (1)
Some(197-204)apps/desktop/src/utils/tauri.ts (1)
StudioRecordingStatus(462-462)
apps/desktop/src/routes/(window-chrome)/settings/experimental.tsx (1)
apps/desktop/src/routes/(window-chrome)/settings/Setting.tsx (1)
ToggleSetting(24-40)
crates/project/src/meta.rs (1)
apps/desktop/src/utils/tauri.ts (9)
UploadMeta(466-466)S3UploadMeta(449-449)VideoUploadInfo(478-478)InstantRecordingMeta(408-408)RecordingMeta(436-436)StudioRecordingStatus(462-462)StudioRecordingMeta(461-461)SingleSegment(458-458)MultipleSegments(417-417)
apps/desktop/src-tauri/src/recording.rs (3)
apps/desktop/src/utils/tauri.ts (9)
InstantRecordingMeta(408-408)MultipleSegments(417-417)Platform(427-427)RecordingMeta(436-436)SharingMeta(456-456)StudioRecordingMeta(461-461)StudioRecordingStatus(462-462)UploadMeta(466-466)VideoUploadInfo(478-478)apps/desktop/src-tauri/src/upload.rs (5)
build_video_meta(268-291)compress_image(293-321)create_or_get_video(199-266)spawn(330-346)singlepart_uploader(646-687)crates/project/src/meta.rs (4)
default(54-60)default(313-315)load_for_project(147-153)output_path(187-192)
apps/desktop/src-tauri/src/lib.rs (4)
apps/desktop/src/utils/tauri.ts (11)
InstantRecordingMeta(408-408)RecordingMeta(436-436)SharingMeta(456-456)StudioRecordingMeta(461-461)StudioRecordingStatus(462-462)UploadMeta(466-466)VideoUploadInfo(478-478)SingleSegment(458-458)MultipleSegments(417-417)UploadProgress(468-468)GeneralSettingsStore(391-391)apps/desktop/src-tauri/src/upload.rs (5)
create_or_get_video(199-266)upload_image(155-197)upload_video(58-139)build_video_meta(268-291)spawn(330-346)apps/desktop/src-tauri/src/upload_legacy.rs (4)
create_or_get_video(384-446)upload_image(328-382)upload_video(205-326)build_video_meta(497-520)crates/project/src/meta.rs (4)
status(218-226)path(143-145)path(329-331)path(380-382)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (2)
packages/ui-solid/src/ProgressCircle.tsx (1)
ProgressCircle(55-112)apps/web/components/VideoThumbnail.tsx (1)
VideoThumbnail(56-125)
apps/desktop/src-tauri/src/upload.rs (5)
apps/desktop/src/utils/tauri.ts (6)
UploadProgress(468-468)VideoUploadInfo(478-478)RecordingMeta(436-436)S3UploadMeta(449-449)UploadMeta(466-466)UploadProgressEvent(469-469)crates/project/src/meta.rs (5)
path(143-145)path(329-331)path(380-382)load_for_project(147-153)status(218-226)apps/desktop/src-tauri/src/upload_legacy.rs (4)
upload_video(205-326)compress_image(576-603)new(123-129)create_or_get_video(384-446)apps/desktop/src-tauri/src/general_settings.rs (1)
get(183-194)apps/desktop/src-tauri/src/api.rs (9)
upload_multipart_initiate(10-44)upload_multipart_complete(111-162)upload_multipart_presign_part(46-88)resp(40-40)resp(84-84)resp(158-158)resp(210-210)upload_signed(182-214)desktop_video_progress(216-244)
apps/desktop/src/utils/tauri.ts (1)
packages/web-domain/src/Video.ts (1)
UploadProgress(61-68)
apps/desktop/src/routes/(window-chrome)/settings/recordings.tsx (4)
apps/desktop/src/utils/tauri.ts (3)
RecordingMetaWithMetadata(437-437)events(283-327)UploadProgress(468-468)apps/desktop/src/utils/createEventListener.ts (1)
createTauriEventListener(30-44)packages/ui-solid/src/ProgressCircle.tsx (1)
ProgressCircle(55-112)apps/desktop/src-tauri/src/upload.rs (2)
progress(393-403)progress(728-828)
apps/desktop/src-tauri/src/upload_legacy.rs (2)
apps/desktop/src/utils/tauri.ts (4)
UploadProgress(468-468)VideoUploadInfo(478-478)S3UploadMeta(449-449)Video(471-471)apps/desktop/src-tauri/src/upload.rs (18)
serde_json(261-261)total(691-691)total(698-700)total(708-710)total(718-720)spawn(330-346)response(242-242)upload_video(58-139)create_or_get_video(199-266)build_video_meta(268-291)progress(393-403)progress(728-828)compress_image(293-321)run(348-424)size(694-694)size(702-704)size(712-714)size(722-724)
apps/desktop/src-tauri/src/api.rs (1)
apps/desktop/src-tauri/src/upload.rs (4)
total(691-691)total(698-700)total(708-710)total(718-720)
⏰ 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). (4)
- GitHub Check: Clippy
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (9)
.github/workflows/ci.yml (1)
96-101: Clippy component pin looks good.Explicitly installing the clippy component avoids missing-toolchain issues on fresh runners. Thanks for tightening this up.
packages/ui-solid/src/auto-imports.d.ts (1)
94-95: LGTM! Icon declarations align with upload tracking features.The addition of
IconPhRecordFillandIconPhWarningBoldsupports the PR's goal of displaying recording and error states in the UI. The declarations follow the existing pattern and are auto-generated, ensuring consistency.apps/web/actions/video/upload.ts (1)
167-167: LGTM! Feature flag approach for upload progress tracking.The optional parameter with a conservative default (
false) ensures backward compatibility while allowing opt-in for the new upload progress feature. The TODO comment appropriately signals the experimental nature of this capability.Also applies to: 177-178
apps/desktop/src-tauri/src/web_api.rs (1)
28-30: Header rename aligns with backend check.Server-side progress gating already reads
X-Cap-Desktop-Version, so this update restores parity. Looks good.apps/desktop/src/utils/createEventListener.ts (1)
34-43: Abort guard prevents stale callbacks.Setting the
abortedflag before awaitingunlistenstops late-fire events from touching torn-down components. Nice catch.crates/api/src/lib.rs (1)
1-4: Lib scaffolding looks good.Crate doc/TODO setup is fine for the initial scaffold.
apps/desktop/src-tauri/src/general_settings.rs (1)
100-165: New uploader flag wired correctly.Default helper + serde settings match existing pattern, so persisted configs behave as expected.
crates/api/Cargo.toml (1)
1-11: Cargo manifest scaffold looks fine.Package metadata and reqwest dependency are consistent with the new crate setup.
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (1)
295-552: CapCard progress handling looks solid.The z-index tweak plus disabled states for active uploads achieve the intended UX without regressions.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/components/VideoThumbnail.tsx (1)
76-80: Add missing effect dependency.
setImageStatuscomes from props, so the hooks linter flags this empty dependency array. Include it to keep lint/tests clean.- useEffect(() => { + useEffect(() => { if (imageRef.current?.complete && imageRef.current.naturalWidth !== 0) { setImageStatus("success"); } - }, []); + }, [setImageStatus]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/desktop/src-tauri/src/lib.rs(16 hunks)apps/desktop/src-tauri/src/recording.rs(23 hunks)apps/desktop/src-tauri/src/upload.rs(5 hunks)apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx(8 hunks)apps/web/app/(org)/dashboard/spaces/[spaceId]/components/VideoCard.tsx(3 hunks)apps/web/components/VideoThumbnail.tsx(2 hunks)packages/web-backend/src/Videos/VideosRepo.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
packages/web-backend/src/Videos/VideosRepo.tsapps/web/components/VideoThumbnail.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/VideoCard.tsxapps/desktop/src-tauri/src/upload.rsapps/desktop/src-tauri/src/recording.rsapps/desktop/src-tauri/src/lib.rs
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{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/Videos/VideosRepo.tsapps/web/components/VideoThumbnail.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/VideoCard.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:
packages/web-backend/src/Videos/VideosRepo.tsapps/web/components/VideoThumbnail.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/VideoCard.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Files:
apps/web/components/VideoThumbnail.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/VideoCard.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/components/VideoThumbnail.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/VideoCard.tsx
apps/web/app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components
Files:
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/VideoCard.tsx
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/upload.rsapps/desktop/src-tauri/src/recording.rsapps/desktop/src-tauri/src/lib.rs
🧬 Code graph analysis (6)
packages/web-backend/src/Videos/VideosRepo.ts (1)
packages/database/index.ts (1)
db(29-34)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (1)
apps/web/components/VideoThumbnail.tsx (1)
ImageLoadingStatus(8-8)
apps/web/app/(org)/dashboard/spaces/[spaceId]/components/VideoCard.tsx (1)
apps/web/components/VideoThumbnail.tsx (1)
ImageLoadingStatus(8-8)
apps/desktop/src-tauri/src/upload.rs (6)
apps/desktop/src/utils/tauri.ts (7)
UploadProgress(468-468)VideoUploadInfo(478-478)GeneralSettingsStore(391-391)RecordingMeta(436-436)S3UploadMeta(449-449)UploadMeta(466-466)UploadProgressEvent(469-469)apps/desktop/src-tauri/src/lib.rs (13)
std(1486-1504)std(1528-1550)tauri_specta(1894-2008)app(1146-1147)app(2307-2307)app(2344-2344)app(2350-2350)app(2550-2551)None(2687-2687)new(344-346)new(763-767)new(1426-1456)clone(338-340)crates/project/src/meta.rs (4)
path(143-145)path(329-331)path(380-382)load_for_project(147-153)apps/desktop/src-tauri/src/upload_legacy.rs (4)
upload_video(205-326)compress_image(576-603)new(123-129)upload_image(328-382)apps/desktop/src-tauri/src/general_settings.rs (1)
get(183-194)apps/desktop/src-tauri/src/api.rs (9)
upload_multipart_initiate(10-44)upload_multipart_complete(111-162)upload_multipart_presign_part(46-88)resp(40-40)resp(84-84)resp(158-158)resp(210-210)upload_signed(182-214)desktop_video_progress(216-244)
apps/desktop/src-tauri/src/recording.rs (3)
apps/desktop/src/utils/tauri.ts (10)
InstantRecordingMeta(408-408)MultipleSegments(417-417)Platform(427-427)RecordingMeta(436-436)SharingMeta(456-456)StudioRecordingMeta(461-461)StudioRecordingStatus(462-462)UploadMeta(466-466)GeneralSettingsStore(391-391)VideoUploadInfo(478-478)apps/desktop/src-tauri/src/upload.rs (5)
build_video_meta(268-291)compress_image(293-321)upload_video(58-139)spawn(330-346)singlepart_uploader(647-688)crates/project/src/meta.rs (3)
default(54-60)default(313-315)load_for_project(147-153)
apps/desktop/src-tauri/src/lib.rs (3)
apps/desktop/src/utils/tauri.ts (11)
InstantRecordingMeta(408-408)RecordingMeta(436-436)SharingMeta(456-456)StudioRecordingMeta(461-461)StudioRecordingStatus(462-462)UploadMeta(466-466)VideoUploadInfo(478-478)SingleSegment(458-458)MultipleSegments(417-417)UploadProgress(468-468)GeneralSettingsStore(391-391)apps/desktop/src-tauri/src/upload.rs (4)
create_or_get_video(199-266)upload_video(58-139)build_video_meta(268-291)spawn(330-346)crates/project/src/meta.rs (5)
status(218-226)path(143-145)path(329-331)path(380-382)load_for_project(147-153)
⏰ 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). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/actions/video/upload.ts (1)
243-246: Transaction atomicity concern persists when supportsUploadProgress is enabled.The past review flagged that the
videosinsert (line 241) andvideoUploadsinsert (now lines 244-246) lack transaction atomicity. While gating thevideoUploadsinsert behindsupportsUploadProgressreduces the frequency of this issue, the underlying risk remains: when the flag istrue, if presigned URL generation fails (lines 251-257), both database records may be orphaned without cleanup.Consider wrapping lines 241-246 in
db().transaction(...)whensupportsUploadProgressis enabled, or implement compensating cleanup in the error path.- await db().insert(videos).values(videoData); - - if (supportsUploadProgress) - await db().insert(videoUploads).values({ - videoId: idToUse, - }); + if (supportsUploadProgress) { + await db().transaction(async (tx) => { + await tx.insert(videos).values(videoData); + await tx.insert(videoUploads).values({ + videoId: idToUse, + }); + }); + } else { + await db().insert(videos).values(videoData); + }
📜 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 (6)
apps/web/actions/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All Groq/OpenAI calls must be implemented in Next.js Server Actions under apps/web/actions; do not place AI calls elsewhere
Files:
apps/web/actions/video/upload.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Files:
apps/web/actions/video/upload.ts
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/web/actions/video/upload.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{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 (2)
packages/utils/src/constants/plans.ts (1)
userIsPro(21-45)packages/database/schema.ts (1)
videoUploads(656-662)
⏰ 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). (4)
- GitHub Check: Clippy
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/components/VideoThumbnail.tsx (1)
76-80: AddsetImageStatusto the dependency array.The
useEffectcallssetImageStatusbut omits it from the dependency array, violating React's exhaustive-deps rule and potentially causing stale closures.Apply this diff:
useEffect(() => { if (imageRef.current?.complete && imageRef.current.naturalWidth !== 0) { setImageStatus("success"); } -}, []); +}, [setImageStatus]);
♻️ Duplicate comments (1)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (1)
515-517: Remove inline comment to comply with repo policy.The inline comment
// "block group",violates the coding guidelines that disallow inline comments.Apply this diff:
className={clsx( "relative", - // "block group", anyCapSelected && "cursor-pointer pointer-events-none", )}As per coding guidelines
🧹 Nitpick comments (1)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (1)
524-553: Consider extracting the upload overlay into a dedicated component.The conditional upload progress/error overlay spans 30 lines and duplicates structural elements (aspect-video container, rounded corners, absolute positioning). Extracting it into a separate component (e.g.,
CapCardUploadOverlay) would reduce nesting, improve readability, and make the overlay logic reusable.For example, create a new component:
interface CapCardUploadOverlayProps { uploadProgress: { status: string; progress: number } | null; } function CapCardUploadOverlay({ uploadProgress }: CapCardUploadOverlayProps) { if (!uploadProgress) return null; return ( <div className="relative inset-0 w-full h-full z-20"> <div className="overflow-hidden relative mx-auto w-full h-full rounded-t-xl border-b border-gray-3 aspect-video bg-black z-5"> <div className="flex absolute inset-0 justify-center items-center rounded-t-xl"> {uploadProgress.status === "failed" ? ( <div className="flex flex-col items-center"> <div className="flex justify-center items-center mb-2 w-8 h-8 bg-red-500 rounded-full"> <FontAwesomeIcon icon={faVideo} className="text-white size-3" /> </div> <p className="text-[13px] text-center text-white">Upload failed</p> </div> ) : ( <div className="relative size-20 md:size-16"> <ProgressCircle progressTextClassName="md:!text-[11px]" subTextClassName="!mt-0 md:!text-[7px] !text-[10px] mb-1" className="md:scale-[1.5] scale-[1.2]" progress={uploadProgress.progress} /> </div> )} </div> </div> </div> ); }Then replace lines 524-553 with:
-{imageStatus !== "success" && uploadProgress ? ( - <div className="relative inset-0 w-full h-full z-20"> - <div className="overflow-hidden relative mx-auto w-full h-full rounded-t-xl border-b border-gray-3 aspect-video bg-black z-5"> - <div className="flex absolute inset-0 justify-center items-center rounded-t-xl"> - {uploadProgress.status === "failed" ? ( - <div className="flex flex-col items-center"> - <div className="flex justify-center items-center mb-2 w-8 h-8 bg-red-500 rounded-full"> - <FontAwesomeIcon - icon={faVideo} - className="text-white size-3" - /> - </div> - <p className="text-[13px] text-center text-white"> - Upload failed - </p> - </div> - ) : ( - <div className="relative size-20 md:size-16"> - <ProgressCircle - progressTextClassName="md:!text-[11px]" - subTextClassName="!mt-0 md:!text-[7px] !text-[10px] mb-1" - className="md:scale-[1.5] scale-[1.2]" - progress={uploadProgress.progress} - /> - </div> - )} - </div> - </div> - </div> -) : null} +{imageStatus !== "success" && <CapCardUploadOverlay uploadProgress={uploadProgress} />}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src-tauri/src/recording.rs(23 hunks)apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx(8 hunks)apps/web/components/VideoThumbnail.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/desktop/src-tauri/src/recording.rsapps/web/components/VideoThumbnail.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/recording.rs
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Files:
apps/web/components/VideoThumbnail.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{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/components/VideoThumbnail.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.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/components/VideoThumbnail.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.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/components/VideoThumbnail.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx
apps/web/app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components
Files:
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx
🧬 Code graph analysis (2)
apps/desktop/src-tauri/src/recording.rs (3)
apps/desktop/src/utils/tauri.ts (17)
InstantRecordingMeta(408-408)MultipleSegments(417-417)Platform(427-427)ProjectConfiguration(432-432)RecordingMeta(436-436)SharingMeta(456-456)StudioRecordingMeta(461-461)StudioRecordingStatus(462-462)TimelineConfiguration(464-464)TimelineSegment(465-465)UploadMeta(466-466)ZoomMode(482-482)ZoomSegment(483-483)CaptureWindowWithThumbnail(367-367)VideoUploadInfo(478-478)RecordingMode(438-438)RecordingEvent(435-435)apps/desktop/src-tauri/src/upload.rs (6)
build_video_meta(268-291)compress_image(293-321)create_or_get_video(199-266)upload_video(58-139)spawn(330-346)singlepart_uploader(647-688)crates/project/src/meta.rs (2)
load_for_project(147-153)output_path(187-192)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (3)
apps/web/app/Layout/features.ts (1)
useFeatureFlag(32-34)apps/web/components/VideoThumbnail.tsx (1)
ImageLoadingStatus(8-8)packages/ui-solid/src/ProgressCircle.tsx (1)
ProgressCircle(55-112)
⏰ 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). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (15)
apps/web/components/VideoThumbnail.tsx (2)
6-6: LGTM!The removal of the unused
useStateimport resolves the issue flagged in the previous review.
8-8: LGTM!The new
ImageLoadingStatustype and props correctly externalize the image loading state management, allowing parent components to coordinate upload overlay and thumbnail visibility logic.Also applies to: 17-18
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (3)
35-38: LGTM!The integration of
ImageLoadingStatusand external state management for the thumbnail aligns well with the coordinated upload overlay UI introduced in this PR.Also applies to: 46-46, 179-180
372-375: LGTM!Disabling Download and Duplicate actions during active uploads when the feature flag is enabled prevents user actions that could conflict with ongoing uploads.
Also applies to: 433-436
565-568: LGTM!The use of the valid Tailwind utility
hiddeninstead ofdisplay-nonecorrectly hides the thumbnail when the upload overlay should be visible, and the props passed toVideoThumbnailenable external state coordination.Also applies to: 571-572
apps/desktop/src-tauri/src/recording.rs (10)
2-2: LGTM!The new imports support upload tracking, progressive upload spawning, and thumbnail compression. All appear necessary and are used in the changed code segments.
Also applies to: 5-8, 19-19, 37-37, 45-45
55-55: LGTM!Removing
Option<>wrapper fromprogressive_uploadsimplifies the API and eliminates potentialNonecases, as the uploader is now always created upfront for instant recordings.Also applies to: 148-148
1082-1147: LGTM!The implementation correctly pre-creates the video upload, constructs the shareable link, initializes
RecordingMetawith the newuploadfield (set toNoneat start), and saves metadata eagerly before recording begins. Error handling is proper throughout, and the Windows-safe timestamp formatting prevents path issues.
1519-1554: Past review concerns addressed: no unwrap() on error paths.The error handling now correctly uses
if let Ok(mut project_meta) = RecordingMeta::load_for_project(...)and.save_for_project().map_err(...).ok()to handle failures gracefully without panicking. This resolves the critical issue flagged in previous reviews whereunwrap()could crash the desktop app when already handling an upload error.
1646-1718: LGTM!The progressive upload error handling and fallback logic are well-structured. The code checks if progressive upload succeeded, falls back to
upload_videoif needed, and updatesRecordingMetawith failure status using proper error handling (nounwrap()calls). Thumbnail upload is handled separately with appropriate error logging.
1732-1739: LGTM!Metadata loading and saving after recording finish uses proper error handling with
if let Ok(mut meta) = ...and.map_err(...)?to propagate errors. No unwrap() calls that could panic.
1211-1211: LGTM!Progressive upload is correctly spawned upfront for instant recordings, with necessary app handle and recording directory cloned before moving into the spawn closure. The uploader receives the finish signal channel to coordinate recording completion.
Also applies to: 1216-1216, 1275-1281
1912-1917: LGTM!The refactored let-chain pattern improves readability and is idiomatic Rust. The logic correctly merges overlapping or near-overlapping intervals by extending the last merged interval when appropriate.
932-932: LGTM!Passing function references directly to
spawn_blockingis cleaner and equivalent to wrapping in a closure. The refactor improves code clarity.Also applies to: 940-940
271-271: LGTM!The
Cursorimport is necessary for PNG encoding on macOS. Theclamp_channelfunction correctly clamps float values to the 0-255 range for u8 conversion. Extractingrecording_dirbefore callinghandle_recording_endenables error path metadata updates.Also applies to: 523-523, 1430-1430, 1432-1432
|
Known issues:
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (1)
515-517: Remove inline comment to comply with repo policy.The inline comment on line 516 violates the coding guidelines that disallow comments in this repository.
Apply this diff:
className={clsx( "relative", - // "block group", anyCapSelected && "cursor-pointer pointer-events-none", )}As per coding guidelines
🧹 Nitpick comments (2)
apps/desktop/src-tauri/src/upload.rs (2)
584-644: Suggest explicit retry limit for consistency.The multipart uploader correctly computes MD5 checksums, presigns parts, and uploads with retry on server errors. However, unlike
singlepart_uploader(line 667), this function does not explicitly setmax_retries_per_request, relying on the default instead.Apply this diff to add an explicit retry limit consistent with the singlepart uploader:
let url = Uri::from_str(&presigned_url).map_err(|err| format!("uploader/part/{part_number}/invalid_url: {err:?}"))?; let resp = reqwest::Client::builder() .retry(reqwest::retry::for_host(url.host().unwrap_or("<unknown>").to_string()).classify_fn(|req_rep| { if req_rep.status().is_some_and(|s| s.is_server_error()) { req_rep.retryable() } else { req_rep.success() } - })) + }) + .max_retries_per_request(5) + .max_extra_load(5.0)) .build()
729-830: Consider logging progress API errors for debugging.The progress tracking function correctly manages delayed API updates, continuous frontend reemits, and cleanup on stream completion. Task cancellation is properly handled with
abort().However, API call failures at line 765, 773, and 793 are silently ignored with
.ok(). While best-effort progress reporting may be acceptable, logging these errors could aid debugging of progress reporting issues mentioned in the PR objectives.Consider adding error logging for progress API calls:
api::desktop_video_progress(&app_clone, &video_id_clone, uploaded, total).await.ok(); + .map_err(|e| tracing::warn!("Progress update failed: {}", e)) + .ok();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src-tauri/src/upload.rs(5 hunks)apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx(8 hunks)apps/web/app/(org)/dashboard/spaces/[spaceId]/components/VideoCard.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/(org)/dashboard/spaces/[spaceId]/components/VideoCard.tsx
🧰 Additional context used
📓 Path-based instructions (7)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Files:
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx
apps/web/app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components
Files:
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/desktop/src-tauri/src/upload.rs
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{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/CapCard/CapCard.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/CapCard/CapCard.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/CapCard/CapCard.tsx
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/upload.rs
🧬 Code graph analysis (2)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (2)
apps/web/app/Layout/features.ts (1)
useFeatureFlag(32-34)apps/web/components/VideoThumbnail.tsx (1)
ImageLoadingStatus(8-8)
apps/desktop/src-tauri/src/upload.rs (3)
apps/desktop/src/utils/tauri.ts (7)
UploadProgress(468-468)VideoUploadInfo(478-478)GeneralSettingsStore(391-391)RecordingMeta(436-436)S3UploadMeta(449-449)UploadMeta(466-466)UploadProgressEvent(469-469)apps/desktop/src-tauri/src/upload_legacy.rs (6)
upload_video(205-326)compress_image(576-603)new(123-129)create_or_get_video(384-446)run(622-805)build_video_meta(497-520)apps/desktop/src-tauri/src/api.rs (9)
upload_multipart_initiate(10-44)upload_multipart_complete(111-162)upload_multipart_presign_part(46-88)resp(40-40)resp(84-84)resp(158-158)resp(210-210)upload_signed(182-214)desktop_video_progress(216-244)
⏰ 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). (3)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (21)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (8)
32-32: LGTM!The imports are correctly structured and align with the feature flag and thumbnail loading status changes introduced in this PR.
Also applies to: 36-39
179-180: LGTM!The feature flag and image status state are properly initialized with correct types.
301-301: LGTM!The z-index adjustments establish a proper stacking hierarchy: container (z-10) < upload overlay (z-20) < dropdown (z-[51]).
Also applies to: 323-323
372-375: LGTM!The disabled condition correctly prevents downloads during active mutations and when uploads are in progress (behind the feature flag).
433-436: LGTM!The disabled condition correctly prevents duplication during active mutations and when uploads are in progress (behind the feature flag).
519-521: LGTM!The simplified
isDeletingcheck correctly prevents navigation during deletion.
524-553: LGTM!The upload progress overlay correctly handles both error and progress states with appropriate z-index layering and responsive sizing.
565-568: LGTM!The containerClass correctly uses the valid Tailwind
hiddenutility to toggle thumbnail visibility, and theimageStatusprops are properly passed to enable external control of loading state.Also applies to: 571-572
apps/desktop/src-tauri/src/upload.rs (13)
3-56: LGTM!The imports, struct definitions, and constants are well-organized. The use of
StringforuploadedandtotalinUploadProgressEventis appropriate for JSON serialization across the Tauri IPC boundary.
58-139: LGTM!The function correctly handles feature-flag routing, streams file data efficiently, and runs video and thumbnail uploads concurrently. Error propagation is appropriate.
141-153: LGTM!The helper function correctly opens a file and constructs a streaming reader with proper error handling.
155-197: LGTM!The function follows the same feature-flag pattern as
upload_videoand correctly handles image uploads.
199-266: LGTM!The function properly constructs API requests, handles authentication errors, and deserializes responses with appropriate error handling.
268-291: LGTM!The function correctly extracts video metadata using ffmpeg with appropriate error handling.
293-321: LGTM!The function correctly offloads CPU-intensive image processing to a blocking task and produces a compressed thumbnail with appropriate quality settings.
323-426: LGTM! Past review concern addressed.The
InstantMultipartUpload::runmethod correctly implements the multipart upload flow with feature-flag routing. Line 405 now sorts the collected parts bypart_numberin ascending order before callingupload_multipart_complete, which addresses the previous review comment about AWS requiring parts in correct sequence.The recording metadata is properly persisted at upload initiation and completion.
428-459: LGTM!The
Chunkstruct andfrom_file_to_chunksfunction provide a clean abstraction for reading files in chunks. The#[allow(unused)]attribute suggests this is reserved for future use.
461-582: LGTM! Intentional design for header rewriting.The function correctly implements a streaming chunker for growing files, with the clever approach of deferring part 1 to allow for MP4 header updates after recording completes. The re-read of the first chunk at the end (lines 552-575) ensures any header rewrites are captured in the uploaded file.
The realtime completion signaling and file size polling logic are appropriate for the use case.
646-689: LGTM!The single-part uploader correctly handles presigned URL uploads with appropriate retry configuration, timeouts, and error handling.
691-727: LGTM!The
UploadedChunktrait provides a clean abstraction for progress tracking, with correct implementations for multiple types.
832-855: LGTM!The Tauri channel progress tracker correctly computes and sends progress ratios to the frontend.
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/desktop/src-tauri/src/recording.rs (1)
652-660: Fix clippy: avoid?on explicitreturn Err(...)
return Err(...)?;triggers clippy’s needless_question_mark. Return the Err directly.Apply this diff:
- return Err("Recording not in progress".to_string())?; + return Err("Recording not in progress".to_string());
♻️ Duplicate comments (3)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (1)
515-517: Remove inline comment to satisfy repo policy.Inline comments are disallowed in this repo; please drop the
// "block group",entry.
As per coding guidelinesclassName={clsx( "relative", - // "block group", anyCapSelected && "cursor-pointer pointer-events-none",apps/desktop/src-tauri/src/recording.rs (1)
753-781: Failure path now persists status to metaOn errors, Studio MultipleSegments.status -> Failed { error }, Instant -> Failed { error }, and saved without panicking. This resolves prior unwrap risks.
apps/desktop/src-tauri/src/lib.rs (1)
2470-2584: Robust resume logic for uploads and crashed recordings
- Marks Studio/Instant InProgress as Failed on startup.
- Resumes Multipart via InstantMultipartUpload::spawn.
- Resumes SinglePart by rebuilding meta and uploading; persists Failed on any error, Complete + Sharing on success.
This addresses “stuck uploading” issues flagged previously.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
apps/desktop/src-tauri/Cargo.toml(1 hunks)apps/desktop/src-tauri/src/lib.rs(16 hunks)apps/desktop/src-tauri/src/recording.rs(20 hunks)apps/desktop/src/utils/tauri.ts(8 hunks)apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx(8 hunks)crates/recording/src/studio_recording.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src-tauri/Cargo.toml
🧰 Additional context used
📓 Path-based instructions (11)
crates/**/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
For desktop IPC, use tauri_specta derive/macros on Rust types/events; do not hand-roll bindings
Files:
crates/recording/src/studio_recording.rs
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
crates/recording/src/studio_recording.rsapps/desktop/src/utils/tauri.tsapps/desktop/src-tauri/src/recording.rsapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/desktop/src-tauri/src/lib.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
crates/recording/src/studio_recording.rsapps/desktop/src-tauri/src/recording.rsapps/desktop/src-tauri/src/lib.rs
crates/*/src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Rust crates should place tests within the
src/and/or a siblingtests/directory for each crate insidecrates/*.
Files:
crates/recording/src/studio_recording.rs
**/tauri.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Never edit auto-generated IPC bindings file tauri.ts
Do not edit auto-generated files named
tauri.ts.
Files:
apps/desktop/src/utils/tauri.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: Do not manually import icons in the desktop app; rely on auto-imported icons
In the desktop app, use @tanstack/solid-query for server state management
Files:
apps/desktop/src/utils/tauri.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/desktop/src/utils/tauri.tsapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.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/desktop/src/utils/tauri.tsapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Files:
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx
apps/web/app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components
Files:
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.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/CapCard/CapCard.tsx
🧬 Code graph analysis (5)
crates/recording/src/studio_recording.rs (1)
apps/desktop/src/utils/tauri.ts (1)
StudioRecordingStatus(465-465)
apps/desktop/src/utils/tauri.ts (1)
packages/web-domain/src/Video.ts (1)
UploadProgress(61-68)
apps/desktop/src-tauri/src/recording.rs (3)
apps/desktop/src/utils/tauri.ts (10)
InstantRecordingMeta(410-410)MultipleSegments(419-419)Platform(429-429)RecordingMeta(438-438)SharingMeta(459-459)StudioRecordingMeta(464-464)StudioRecordingStatus(465-465)TimelineSegment(468-468)UploadMeta(469-469)VideoUploadInfo(481-481)apps/desktop/src-tauri/src/upload.rs (5)
compress_image(293-321)create_or_get_video(199-266)upload_video(58-139)spawn(330-346)singlepart_uploader(647-689)crates/project/src/meta.rs (4)
default(54-60)default(313-315)load_for_project(147-153)output_path(187-192)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (2)
apps/web/app/Layout/features.ts (1)
useFeatureFlag(32-34)apps/web/components/VideoThumbnail.tsx (1)
ImageLoadingStatus(8-8)
apps/desktop/src-tauri/src/lib.rs (4)
apps/desktop/src/utils/tauri.ts (11)
InstantRecordingMeta(410-410)RecordingMeta(438-438)SharingMeta(459-459)StudioRecordingMeta(464-464)StudioRecordingStatus(465-465)UploadMeta(469-469)VideoUploadInfo(481-481)SingleSegment(461-461)MultipleSegments(419-419)UploadProgress(471-471)GeneralSettingsStore(393-393)apps/desktop/src-tauri/src/upload.rs (5)
create_or_get_video(199-266)upload_image(155-197)upload_video(58-139)build_video_meta(268-291)spawn(330-346)apps/desktop/src-tauri/src/upload_legacy.rs (5)
create_or_get_video(384-446)upload_image(328-382)upload_video(205-326)build_video_meta(497-520)new(123-129)crates/project/src/meta.rs (4)
status(218-226)path(143-145)path(329-331)path(380-382)
⏰ 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). (3)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (17)
crates/recording/src/studio_recording.rs (1)
552-553: Good addition: persist completion status in metadataSetting status to Complete on finish is correct and matches the new status model.
apps/desktop/src-tauri/src/recording.rs (7)
332-337: Windows-safe timestamp formattingUsing dots on Windows is correct to avoid invalid filename characters.
339-359: Initialize meta with explicit recording statusSetting Studio to InProgress and Instant to InProgress; upload: None. This enables reliable crash/finish handling.
498-505: Progressive upload spawn order: verify file readinessSpawning InstantMultipartUpload before actor build is fine if it waits on finish_upload_rx before touching the file. Please confirm the actor defers file reads until that signal.
550-572: Graceful error handling on actor spawn failureEmits UI error, persists failure via handle_recording_end, and returns early. Good defensive path.
876-945: Instant finish: correct progressive-upload-first strategy with thumbnail update and fallback
- Waits for progressive upload; on success uploads thumbnail only.
- On progressive failure, builds meta and runs full single-part upload; persists UploadMeta::Failed on errors.
This matches PR goals and avoids silent stalls.
959-967: Persist final meta and sharing on successLoads, updates, and saves meta; errors are surfaced. Looks good.
1139-1144: Segment merge useslet-chains; simpler and efficientThe merge logic is correct and clearer with
if let Some(last) … && ….apps/desktop/src/utils/tauri.ts (3)
128-136: Type updates align with backend (RecordingMetaWithMetadata)New return types for getRecordingMeta/listRecordings match lib.rs changes.
Also applies to: 134-136
305-329: New upload progress event exporteduploadProgressEvent wiring matches tauri_specta event added in Rust. Good.
393-393: Approve: GeneralSettingsStore extended & no stale references
GeneralSettingsStore.enableNewUploader, StudioRecordingStatus, UploadMeta, and UploadProgressEvent support the new uploader; no remaining references to RecordingMetaWithMode or legacy wrappers found.apps/desktop/src-tauri/src/lib.rs (6)
2008-2009: Expose upload progress eventEvent is registered for TS autogen; consistent with tauri.ts.
1418-1459: RecordingMetaWithMetadata: stable wrapper for UIMode/status derivation is correct for Studio/Instant. Good use of flatten to preserve original shape.
1470-1477: get_recording_meta now returns wrapperMatches TS binding changes; simplifies UI consumption.
1481-1523: list_recordings returns path + wrapper; sorted by created timeLooks good. Silent skip on load errors keeps UI resilient.
894-917: Block metadata reads for non-finalized studio recordingsReturning errors for Failed/InProgress prevents bogus duration estimation. Good guardrail.
1072-1080: Upload tracking and persistence on exported video
- Uses meta.output_path() to locate render.
- Persists UploadMeta::SinglePartUpload before upload, then Complete/Failed after.
- Clipboard update + notification on success. Solid flow.
Also applies to: 1115-1124, 1139-1147, 1162-1166
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/desktop/src-tauri/src/recording.rs (2)
898-920: Verify thumbnail upload error handling doesn't block the upload flow.The thumbnail upload for successful progressive uploads is done after
screenshot_task.awaitand usesok()to ignore errors (line 919). This means:
- Thumbnail upload failures won't fail the overall upload (good)
- Thumbnail upload is attempted even if screenshot creation failed (acceptable)
However, consider whether users should be notified of thumbnail upload failures, as this affects the video preview in the UI.
938-943: Save errors are silently ignored on upload failure path.When
upload_videofails, the code loads and updatesRecordingMetato persist the failure state (lines 938-943). However,save_for_project()failures are silently ignored viaok()(line 942). While this prevents cascading failures, users won't know if the failure state wasn't persisted.Consider logging the save error or using a pattern like:
.map_err(|e| error!("Failed to persist upload error state: {e}")) .ok();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src-tauri/src/recording.rs(20 hunks)crates/recording/src/instant_recording.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
crates/**/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
For desktop IPC, use tauri_specta derive/macros on Rust types/events; do not hand-roll bindings
Files:
crates/recording/src/instant_recording.rs
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
crates/recording/src/instant_recording.rsapps/desktop/src-tauri/src/recording.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
crates/recording/src/instant_recording.rsapps/desktop/src-tauri/src/recording.rs
crates/*/src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Rust crates should place tests within the
src/and/or a siblingtests/directory for each crate insidecrates/*.
Files:
crates/recording/src/instant_recording.rs
🧬 Code graph analysis (2)
crates/recording/src/instant_recording.rs (1)
apps/desktop/src/utils/tauri.ts (1)
InstantRecordingMeta(410-410)
apps/desktop/src-tauri/src/recording.rs (3)
apps/desktop/src/utils/tauri.ts (11)
InstantRecordingMeta(410-410)MultipleSegments(419-419)Platform(429-429)ProjectConfiguration(434-434)RecordingMeta(438-438)SharingMeta(459-459)StudioRecordingMeta(464-464)StudioRecordingStatus(465-465)TimelineConfiguration(467-467)UploadMeta(469-469)VideoUploadInfo(481-481)apps/desktop/src-tauri/src/upload.rs (4)
compress_image(293-321)create_or_get_video(199-266)spawn(330-346)singlepart_uploader(647-689)crates/project/src/meta.rs (4)
default(54-60)default(313-315)load_for_project(147-153)output_path(187-192)
⏰ 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). (4)
- GitHub Check: Clippy
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (10)
crates/recording/src/instant_recording.rs (1)
111-114: LGTM! API refactor correctly applied.The change from struct-like syntax to the
Completevariant is correct and aligns with the enum-basedInstantRecordingMetaAPI introduced incap_project. The field names and values are preserved as expected.apps/desktop/src-tauri/src/recording.rs (9)
293-331: Verify video pre-creation doesn't leave orphaned metadata on failure.The video is pre-created at lines 298-312, but if this fails, the function returns early with an error. However, since
RecordingMetais saved at line 362 (after this point), there's no orphaned metadata risk here. The flow is correct.
333-364: LGTM! Early metadata persistence enables crash recovery.The early creation and persistence of
RecordingMeta(before recording starts) is a good design choice. It ensures that:
- Windows path compatibility is handled with safe timestamp formatting
- The recording directory and metadata exist before any recording operations
- Failed or crashed recordings can be detected and handled on restart
499-505: LGTM! Progressive upload spawning is correct.The unconditional spawning of
InstantMultipartUploadfor instant recordings aligns with the API change makingprogressive_uploada required field. The placement within the actor spawn task ensures proper error propagation.
754-782: Error handling improved but save failures are still silent.The previous review flagged
unwrap()calls on error paths, and this has been addressed by usingok()instead (line 778). However, while this prevents panics, save failures are still silently ignored. The error is logged (line 776), which is acceptable for a failure-handling path where we're already in an error state and want to avoid cascading failures.Based on past review comments, the unwrap issue has been resolved.
877-893: LGTM! Progressive upload error handling is correct.The logic correctly handles both success and failure of the progressive upload:
- Success: logs and skips fallback upload
- Failure: logs error and proceeds to fallback upload_video
960-967: LGTM! Final metadata persistence is correct.On successful recording completion, the metadata is loaded, updated with the final state, and saved with proper error propagation (line 966). This ensures that recording metadata is persisted before proceeding.
1139-1147: LGTM! Let-chain pattern improves readability.The refactor to use Rust's let-chain syntax (
if let Some(last) = merged.last_mut() && condition) is more idiomatic and improves readability by reducing nesting.
1173-1181: LGTM! Test helper updated to match struct changes.The temporary
RecordingMetaconstruction in the test helper correctly includes the newupload: Nonefield to match the updated struct definition.
60-60: Instant/CompletedRecording::Instant variants include required progressive_upload All construction sites supply the mandatory field.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
apps/desktop/src-tauri/src/upload.rs (7)
42-47: Remove commented-out code.The commented
configfield should either be removed or uncommented if it's needed for future work. Commented code clutters the codebase and should be cleaned up.Apply this diff:
pub struct UploadedItem { pub link: String, pub id: String, - // #[allow(unused)] - // pub config: S3UploadMeta, }
49-54: Consider using numeric types for uploaded and total.The
uploadedandtotalfields are represented asString, which requires parsing on the receiving end and can lead to errors. Consider usingu64directly, as Serde and Tauri handle numeric serialization well.
59-140: Consider extracting repeated feature flag checking.The feature flag checking pattern (lines 67-88) is repeated in
upload_image(lines 157-170) andInstantMultipartUpload::run(lines 356-373). Consider extracting this into a helper function to reduce duplication and improve maintainability.Example refactor:
fn is_new_uploader_enabled(app: &AppHandle) -> bool { GeneralSettingsStore::get(app) .map_err(|err| error!("Error checking status of new uploader flow from settings: {err}")) .ok() .and_then(|v| v.map(|v| v.enable_new_uploader)) .unwrap_or(false) }
294-322: Consider image resize filter quality.The implementation uses
FilterType::Nearestfor resizing (line 304), which is fast but produces lower quality results. For thumbnail generation, consider usingFilterType::Lanczos3orFilterType::CatmullRomfor better visual quality, unless performance is critical here.
446-467: Remove or utilize unused function.The
from_file_to_chunksfunction is marked with#[allow(unused)]. If this function is not needed, remove it. If it's planned for future use, document the intention or create a TODO tracking item.
609-609: Verify MD5 computation for large chunks.Computing MD5 synchronously (line 609) for chunks up to 5MB could block the async runtime. Consider whether this is acceptable or if MD5 computation should be moved to a blocking task for better async performance.
664-676: Inconsistent retry configuration between single and multipart uploaders.
singlepart_uploaderhas more aggressive retry settings (max 5 retries, max_extra_load 5.0) compared tomultipart_uploaderwhich uses the default retry limits. Consider whether multipart uploads should have similar retry resilience, or document why they differ.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src-tauri/src/upload.rs(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/desktop/src-tauri/src/upload.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/upload.rs
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/upload.rs (3)
apps/desktop/src-tauri/src/upload_legacy.rs (2)
upload_video(205-326)run(622-805)apps/desktop/src-tauri/src/general_settings.rs (1)
get(183-194)apps/desktop/src-tauri/src/api.rs (9)
upload_multipart_initiate(10-44)upload_multipart_complete(111-162)upload_multipart_presign_part(46-88)resp(40-40)resp(84-84)resp(158-158)resp(210-210)upload_signed(182-214)desktop_video_progress(216-244)
⏰ 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). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (14)
apps/desktop/src-tauri/src/upload.rs (14)
3-40: LGTM!The imports are well-organized and appropriate for the streaming upload implementation.
67-72: Verify error handling for settings retrieval.When
GeneralSettingsStore::getfails, the error is logged but the function continues with a default value offalse. Ensure this is the intended behavior, as settings retrieval failures might indicate a more serious problem that should prevent upload.
142-154: LGTM!The file reader stream implementation is clean and handles errors appropriately.
269-292: LGTM!The video metadata extraction using ffmpeg is implemented correctly.
407-413: Verify deduplication strategy for multipart uploads.The deduplication keeps the last occurrence of each part number (lines 407-413). Ensure this is the intended behavior, as typically you'd want to keep the most recent upload of a part if it was reuploaded. The comment in the PR objectives mentions "First chunk may be skipped and needs reupload; requires deduplication of parts array", so this appears intentional.
Based on the PR context, this deduplication strategy aligns with the known issue about chunk reuploads.
378-390: Consider error handling consistency for RecordingMeta saves.The
save_for_project()call at line 387-390 logs errors with.ok()but continues execution. Later at lines 422-428, the save returns an error. Ensure this inconsistency is intentional. If saving metadata after initiating the upload isn't critical, document this behavior.
436-444: LGTM!The
Chunkstruct is well-defined with clear documentation.
469-589: Complex state machine requires careful testing.This function manages multiple state variables and has intricate logic for handling growing files during recording. Key behaviors to verify:
- Part numbering: starts at 1, first chunk is part 1 but yielded last
- Polling interval: 500ms between checks (lines 507, 585)
- First chunk handling: stored size at line 545, re-read at lines 558-582
Ensure thorough testing covers:
- Normal recording completion
- Recording failures/crashes
- Edge cases where file size changes between checks
- Behavior when realtime_upload_done signal is never sent
The implementation handles the complex requirements described in the PR objectives.
532-538: Verify partial read handling.The inner read loop (lines 532-538) accumulates bytes until
chunk_sizeis reached. Iffile.read()returns 0 (EOF) before reading the full chunk, the function breaks and yields a partial chunk. Verify this is the intended behavior, especially for the first chunk which might be incomplete if the file is still being written.
617-625: Review retry configuration for multipart uploads.The retry policy (lines 617-625) only retries on server errors (5xx). However, network issues, timeouts, and certain 4xx errors might also benefit from retries. The PR objectives mention "S3 retry policy on requests requires further testing". Consider expanding the retry conditions or documenting why only server errors are retried.
Based on PR objectives noting that "S3 retry policy on requests requires further testing; current retry behavior may be insufficient."
698-734: LGTM!The
UploadedChunktrait provides a clean abstraction for different chunk types, enabling generic progress tracking.
736-837: Complex progress tracking with multiple concurrent tasks.The progress function manages two types of background tasks:
pending_task: delayed API update after 2 seconds (line 779)reemit_task: continuous frontend events every 700ms (line 789)Key observations:
- Tasks are properly cancelled with
.abort()when new chunks arrive- The cleanup event (lines 826-830) with 0/0 values signals completion
- The reemit interval (700ms) and API delay (2 seconds) are hardcoded
Consider:
- Making intervals configurable or documenting why these values were chosen
- Ensuring
.abort()doesn't leave partial state (Tokio handles this safely, but worth verifying)The PR objectives mention "event emission frequency and frontend refetch tuning" as an open item.
788-802: Potential task leak if stream errors before completion.The
reemit_taskspawned at line 788 runs in an infinite loop. If the stream errors or is dropped before the cleanup logic (lines 819-821), this task may not be cancelled. Verify that the stream wrapper properly handles cleanup on error paths.
839-862: LGTM!The
tauri_progressfunction provides a clean channel-based progress tracking mechanism.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx (1)
467-472: Duplicated upload progress logic.This code duplicates the pattern in
HLSVideoPlayer.tsx(lines 283-288). See the review comment on that file for the recommended refactor to extract this into a shared hook.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx(1 hunks)apps/web/app/s/[videoId]/_components/HLSVideoPlayer.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Files:
apps/web/app/s/[videoId]/_components/HLSVideoPlayer.tsxapps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx
apps/web/app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components
Files:
apps/web/app/s/[videoId]/_components/HLSVideoPlayer.tsxapps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/web/app/s/[videoId]/_components/HLSVideoPlayer.tsxapps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{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/s/[videoId]/_components/HLSVideoPlayer.tsxapps/web/app/s/[videoId]/_components/CapVideoPlayer.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/s/[videoId]/_components/HLSVideoPlayer.tsxapps/web/app/s/[videoId]/_components/CapVideoPlayer.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/s/[videoId]/_components/HLSVideoPlayer.tsxapps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx
🧬 Code graph analysis (2)
apps/web/app/s/[videoId]/_components/HLSVideoPlayer.tsx (1)
apps/web/app/s/[videoId]/_components/ProgressCircle.tsx (1)
useUploadProgress(26-66)
apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx (1)
apps/web/app/s/[videoId]/_components/ProgressCircle.tsx (1)
useUploadProgress(26-66)
⏰ 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). (3)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src-tauri/src/upload.rs (1)
294-322: Extract duplicated image compression logic.This function is duplicated in
upload_legacy.rs(lines 575-602). Consider extracting it to a shared utility module to avoid code duplication.Create a new module (e.g.,
src/image_utils.rsor add to an existing utils module):pub async fn compress_image(path: PathBuf) -> Result<Vec<u8>, String> { // ... existing implementation ... }Then import and use it in both
upload.rsandupload_legacy.rs.
🧹 Nitpick comments (3)
apps/desktop/src-tauri/src/upload.rs (3)
504-506: Consider tuning the polling interval based on recording characteristics.The 100ms polling interval is used in two places: when file metadata access fails (line 504) and when waiting for new data (line 574). Depending on the recording bitrate and chunk size, this interval might be:
- Too aggressive (wasting CPU on frequent checks for slow recordings)
- Too slow (delaying chunk availability for fast recordings)
Consider making the polling interval configurable or adaptive based on observed file growth rates.
Also applies to: 574-574
813-824: Cleanup signal using "0, 0" values is non-obvious.The stream completion emits an
UploadProgressEventwithuploaded: "0"andtotal: "0"to signal the frontend to remove the event from its store. This convention is not self-documenting and could be misinterpreted as an error state or upload cancellation.Consider one of these alternatives:
- Add a boolean
completedfield toUploadProgressEventto explicitly signal completion- Use a separate event type for completion notifications
- Document this behavior prominently in the
UploadProgressEventstruct definitionExample with a completion flag:
#[derive(Clone, Serialize, Type, tauri_specta::Event)] pub struct UploadProgressEvent { video_id: String, uploaded: String, total: String, + completed: bool, }
772-791: Consider documenting the reemit interval choice.The continuous reemit task runs every 700ms to provide smooth progress updates to the frontend. While this interval seems reasonable, it would be helpful to document why this specific value was chosen (e.g., balancing frontend responsiveness vs. event frequency).
Consider extracting this as a named constant:
const PROGRESS_REEMIT_INTERVAL_MS: u64 = 700;Then use it in the interval creation:
- let mut interval = time::interval(Duration::from_millis(700)); + let mut interval = time::interval(Duration::from_millis(PROGRESS_REEMIT_INTERVAL_MS));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src-tauri/src/upload.rs(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/desktop/src-tauri/src/upload.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/upload.rs
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/upload.rs (4)
crates/utils/src/lib.rs (1)
spawn_actor(37-44)crates/project/src/meta.rs (5)
path(143-145)path(329-331)path(380-382)load_for_project(147-153)status(218-226)apps/desktop/src-tauri/src/upload_legacy.rs (6)
upload_video(205-326)compress_image(576-603)new(123-129)upload_image(328-382)create_or_get_video(384-446)build_video_meta(497-520)apps/desktop/src-tauri/src/api.rs (9)
upload_multipart_initiate(10-44)upload_multipart_complete(111-162)upload_multipart_presign_part(46-88)resp(40-40)resp(84-84)resp(158-158)resp(210-210)upload_signed(182-214)desktop_video_progress(216-244)
⏰ 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). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (4)
apps/desktop/src-tauri/src/upload.rs (4)
52-53: Consider numeric types for progress values.The
uploadedandtotalfields are declared asString, which requires conversion for numeric operations and increases the risk of parsing errors in consumers. Unless there's a specific serialization requirement for the frontend, usingu64would be more type-safe.If String types are required for JSON serialization to the frontend, please confirm this is intentional. Otherwise, apply this diff:
pub struct UploadProgressEvent { video_id: String, - uploaded: String, - total: String, + uploaded: u64, + total: u64, }Note: This change would require updating all
.to_string()calls when constructing these events throughout the file.
407-413: Deduplication strategy keeps last occurrence of each part.The deduplication logic uses
HashMapto keep only the last occurrence of each part number. This is likely intentional sincefrom_pending_file_to_chunksre-emits part 1 with the corrected MP4 header. However, this behavior should be verified as correct.Please confirm that keeping the last occurrence of duplicate part numbers is the intended behavior, especially for part 1 which is re-uploaded with the corrected MP4 header after recording completes.
598-598: base64::encode is still supported
base64 v0.22.1 retains theencodeconvenience function; no migration required.
387-390: Swallowed initial metadata save errors are intentional. The codebase consistently logs and ignores failures fromsave_for_project()during upload initiation—only the final save error is propagated.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src-tauri/src/upload.rs (1)
329-330: Remove implementation comment.Per coding guidelines, comments are not permitted.
Apply this diff:
impl InstantMultipartUpload { - /// starts a progressive (multipart) upload that runs until recording stops - /// and the file has stabilized (no additional data is being written). pub fn spawn(
♻️ Duplicate comments (1)
apps/desktop/src-tauri/src/upload.rs (1)
45-46: Remove commented-out code.The commented-out lines violate the coding guidelines that prohibit commented code.
Apply this diff:
pub struct UploadedItem { pub link: String, pub id: String, - // #[allow(unused)] - // pub config: S3UploadMeta, }
🧹 Nitpick comments (1)
apps/desktop/src-tauri/src/upload.rs (1)
446-467: Remove unused function or integrate into codebase.The
from_file_to_chunksfunction is marked#[allow(unused)]indicating it's dead code. Either integrate it into the codebase or remove it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src-tauri/src/upload.rs(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/desktop/src-tauri/src/upload.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/upload.rs
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/upload.rs (5)
apps/desktop/src/utils/tauri.ts (5)
UploadProgress(471-471)VideoUploadInfo(481-481)S3UploadMeta(452-452)UploadMeta(469-469)UploadProgressEvent(472-472)crates/project/src/meta.rs (5)
path(143-145)path(329-331)path(380-382)load_for_project(147-153)status(218-226)apps/desktop/src-tauri/src/upload_legacy.rs (6)
upload_video(205-326)compress_image(576-603)new(123-129)upload_image(328-382)create_or_get_video(384-446)build_video_meta(497-520)apps/desktop/src-tauri/src/general_settings.rs (1)
get(183-194)apps/desktop/src-tauri/src/api.rs (9)
upload_multipart_initiate(10-44)upload_multipart_complete(111-162)upload_multipart_presign_part(46-88)resp(40-40)resp(84-84)resp(158-158)resp(210-210)upload_signed(182-214)desktop_video_progress(216-244)
🪛 GitHub Check: Clippy
apps/desktop/src-tauri/src/upload.rs
[failure] 579-579: borrowed data escapes outside of function
error[E0521]: borrowed data escapes outside of function
--> apps/desktop/src-tauri/src/upload.rs:579:9
|
577 | fn retryable_client(host: &str) -> reqwest::ClientBuilder {
| ---- - let's call the lifetime of this reference '1
| |
| host is a reference that is only valid in the function body
578 | reqwest::Client::builder().retry(
579 | reqwest::retry::for_host(host)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| host escapes the function body here
| argument requires that '1 must outlive 'static
⏰ 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). (3)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (2)
apps/desktop/src-tauri/src/upload.rs (2)
407-413: Deduplication and sorting logic looks correct.The parts deduplication using HashMap and subsequent sorting properly addresses the previous review concern about part ordering for AWS CompleteMultipartUpload.
605-605: Remove debug logging comment.Per coding guidelines, comments are not permitted.
Apply this diff:
- debug!("Initializing multipart uploader for video {video_id:?}"); - try_stream! {Likely an incorrect or invalid review comment.
What this PR does:
CapCardwhich allows interaction while uploadingUploadCapButton. Will renable it in Effectify upload progress system in web #1032..capfiles to be eagerly created before the recording completes and also to track the upload progress internally.Remaining:
recordings.tsxrenders progress correctly -> uuid vs cap idUploadStatewithin the metadataTODO:
RecordingMetarestructure.RecordingMeta? By constructingRecordingMetaearlier we can also copy the eager behaviour of Cap web.RecordingMetaon recording start instead of end.upload.rsfrom_fileimplementation for uploadDoes- Is does abort early. Why doesn't my code?try_collectabort early or still run the whole stream???UploadProgressUpdaterintoprogressUploadProgressEventreliableupload.rsapi.rs. It's excessive.Channel<UploadProgress>and refactor frontend around new systemRecordingMetaRecordingMetaRecordingMetafrom multipart uploadInstantMultipartUploadand the regular single part updater???InstantMultipartUploadbe owned and managed by the actor instead of polling the filesystem?RecordingMetafrom fallback uploaderUploadStatecorrectly???UploadProgressEventis being emitted too infrequentlyUploadProgressEventcleanup uploads from the SolidJS store once they are completeRecordingMetaUploadStatecorrectly???UploadProgressEventtodo!()'s on the newRecordingMetaInnervariant.RecordingMetaon disk.ProgressCirclefor the actual valueProgressive upload failed: ...is not reported to the frontend. This must be reported and renders in the position of theProgressCircleReporting failed uploads to the DB backend???Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Chores