feat(editor): Cap recording import for timeline stitching#1719
feat(editor): Cap recording import for timeline stitching#1719MinitJain wants to merge 8 commits intoCapSoftware:mainfrom
Conversation
Wrap trim handle state updates in batch() so project segment and previewTime update atomically before effects fire. Reorder effects in Editor.tsx so the config-update effect is created before the render-frame effect (SolidJS fires effects in creation order). Add skipRenderFrameForConfigUpdate flag so renderFrameEvent is suppressed when updateConfigAndRender already handles the render, eliminating the race condition where a stale-config frame was emitted before the async config update completed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…artifacts Spacedrive.framework is a pre-built CI artifact not present in local checkouts. Removing it allows the macOS bundle step to complete. Disabling createUpdaterArtifacts avoids the requirement for TAURI_SIGNING_PRIVATE_KEY which is only available in CI. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tware#1712) Implements the recording import feature from issue CapSoftware#1712. Users can now import additional Cap recordings into the editor — imported recordings are appended to the timeline and stitched together on export. Changes: - ExternalRecordingReference struct in project config - ProjectRecordingsMeta::new_with_external() loads primary + external segments - create_all_segments() in editor builds unified segment list - import_cap_recording Tauri command: validates resolution match, prevents duplicate imports, computes correct clip index offsets, appends timeline segments - Export and preview pipelines updated to pass external_recordings through - Timeline UI: "Import recording" button with folder picker + reload on success⚠️ Not locally tested — macOS Sequoia TCC blocks ScreenCaptureKit on unsigned dev binaries, preventing the app from recording. Build passes (cargo build + pnpm typecheck clean). Needs testing on a signed build or by a reviewer with a valid dev certificate. Known limitations: - External recording paths stored as absolute strings (breaks if folder is moved) - window.location.reload() on import (loses unsaved editor state) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| "bundle": { | ||
| "active": true, | ||
| "createUpdaterArtifacts": true, | ||
| "createUpdaterArtifacts": false, |
There was a problem hiding this comment.
createUpdaterArtifacts set to false
This change disables generation of auto-update artifacts (.sig / .tar.gz / .nsis.zip) at bundle time. If the Cap desktop app ships auto-update, this will silently break it for any release built from this commit — users on older versions won't receive the update. The previous value was true. Please verify this was intentional; if not, revert.
| "createUpdaterArtifacts": false, | |
| "createUpdaterArtifacts": true, |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src-tauri/tauri.conf.json
Line: 39
Comment:
**Likely unintentional: `createUpdaterArtifacts` set to `false`**
This change disables generation of auto-update artifacts (`.sig` / `.tar.gz` / `.nsis.zip`) at bundle time. If the Cap desktop app ships auto-update, this will silently break it for any release built from this commit — users on older versions won't receive the update. The previous value was `true`. Please verify this was intentional; if not, revert.
```suggestion
"createUpdaterArtifacts": true,
```
How can I resolve this? If you propose a fix, please make it concise.
apps/desktop/src-tauri/src/lib.rs
Outdated
| let clip_index_offset = (primary_recordings.segments.len() | ||
| + project_config | ||
| .external_recordings | ||
| .iter() | ||
| .map(|r| { | ||
| let p = std::path::PathBuf::from(&r.path); | ||
| RecordingMeta::load_for_project(&p) | ||
| .ok() | ||
| .and_then(|m| { | ||
| m.studio_meta().map(|s| match s { | ||
| cap_project::StudioRecordingMeta::SingleSegment { .. } => 1usize, | ||
| cap_project::StudioRecordingMeta::MultipleSegments { inner } => { | ||
| inner.segments.len() | ||
| } | ||
| }) | ||
| }) | ||
| .unwrap_or(0) | ||
| }) | ||
| .sum::<usize>()) as u32; |
There was a problem hiding this comment.
unwrap_or(0) corrupts clip indices when a prior external recording is missing
clip_index_offset is computed by summing the segment count of every previously-imported external recording. If any of those paths is missing or has been moved, load_for_project fails and the closure returns 0 for that recording. As a result the new recording's recording_clip indices are computed too low, silently colliding with the indices of an existing external recording in ProjectRecordingsMeta::segments. The timeline would then map multiple TimelineSegments to the wrong clips.
Consider propagating the error instead of swallowing it:
let ext_segment_counts = project_config
.external_recordings
.iter()
.enumerate()
.map(|(i, r)| {
let p = std::path::PathBuf::from(&r.path);
let m = RecordingMeta::load_for_project(&p)
.map_err(|e| format!("existing external recording {i}: {e}"))?;
Ok(m.studio_meta().map(|s| match s {
cap_project::StudioRecordingMeta::SingleSegment { .. } => 1usize,
cap_project::StudioRecordingMeta::MultipleSegments { inner } => inner.segments.len(),
}).unwrap_or(0))
})
.collect::<Result<Vec<_>, String>>()?;
let clip_index_offset = (primary_recordings.segments.len()
+ ext_segment_counts.iter().sum::<usize>()) as u32;Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src-tauri/src/lib.rs
Line: 2155-2173
Comment:
**Silent `unwrap_or(0)` corrupts clip indices when a prior external recording is missing**
`clip_index_offset` is computed by summing the segment count of every previously-imported external recording. If any of those paths is missing or has been moved, `load_for_project` fails and the closure returns `0` for that recording. As a result the new recording's `recording_clip` indices are computed too low, silently colliding with the indices of an existing external recording in `ProjectRecordingsMeta::segments`. The timeline would then map multiple `TimelineSegment`s to the wrong clips.
Consider propagating the error instead of swallowing it:
```rust
let ext_segment_counts = project_config
.external_recordings
.iter()
.enumerate()
.map(|(i, r)| {
let p = std::path::PathBuf::from(&r.path);
let m = RecordingMeta::load_for_project(&p)
.map_err(|e| format!("existing external recording {i}: {e}"))?;
Ok(m.studio_meta().map(|s| match s {
cap_project::StudioRecordingMeta::SingleSegment { .. } => 1usize,
cap_project::StudioRecordingMeta::MultipleSegments { inner } => inner.segments.len(),
}).unwrap_or(0))
})
.collect::<Result<Vec<_>, String>>()?;
let clip_index_offset = (primary_recordings.segments.len()
+ ext_segment_counts.iter().sum::<usize>()) as u32;
```
How can I resolve this? If you propose a fix, please make it concise.| </Show> | ||
| <button | ||
| class="flex items-center gap-1.5 px-3 py-2 mt-1 rounded-xl bg-blue-500 text-white text-xs font-medium disabled:opacity-50 self-start" | ||
| onClick={handleImportCapRecording} | ||
| disabled={isImporting()} | ||
| > | ||
| <Show | ||
| when={isImporting()} | ||
| fallback={<IconLucidePlus class="size-3" />} | ||
| > | ||
| <IconLucideLoader2 class="size-3 animate-spin" /> | ||
| </Show> |
There was a problem hiding this comment.
There are now two separate "Import recording" buttons that call the same handler: one added to the timeline toolbar (line ~878) and this one inside the track-rows scroll area. If this second one is intended as an empty-state CTA, it should be wrapped in a <Show> guard; otherwise having two identical call-to-action buttons in the same view is likely to confuse users.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/routes/editor/Timeline/index.tsx
Line: 1045-1056
Comment:
**Duplicate "Import recording" button**
There are now two separate "Import recording" buttons that call the same handler: one added to the timeline toolbar (line ~878) and this one inside the track-rows scroll area. If this second one is intended as an empty-state CTA, it should be wrapped in a `<Show>` guard; otherwise having two identical call-to-action buttons in the same view is likely to confuse users.
How can I resolve this? If you propose a fix, please make it concise.| const handleImportCapRecording = async () => { | ||
| const selected = await openDialog({ | ||
| directory: true, | ||
| title: "Select a Cap Recording to Import", | ||
| filters: [{ name: "Cap Recording", extensions: ["cap"] }], | ||
| }); |
There was a problem hiding this comment.
filters has no effect for directory pickers on most platforms
openDialog is called with directory: true, which opens a folder-selection dialog. The filters: [{ name: "Cap Recording", extensions: ["cap"] }] option is designed for file pickers and is ignored by OS-level folder dialogs on macOS and Windows. The .endsWith(".cap") check below correctly guards against bad input, but the filter gives a false impression of UI-side enforcement.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/routes/editor/Timeline/index.tsx
Line: 728-733
Comment:
**`filters` has no effect for directory pickers on most platforms**
`openDialog` is called with `directory: true`, which opens a folder-selection dialog. The `filters: [{ name: "Cap Recording", extensions: ["cap"] }]` option is designed for file pickers and is ignored by OS-level folder dialogs on macOS and Windows. The `.endsWith(".cap")` check below correctly guards against bad input, but the filter gives a false impression of UI-side enforcement.
How can I resolve this? If you propose a fix, please make it concise.Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Propagate error instead of unwrap_or(0) in clip_index_offset calculation to prevent silent index corruption when a prior external recording path is missing - Remove duplicate Import recording button from track-rows area - Remove no-op filters from directory picker openDialog call Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The |
|
Hey @richiemcilroy just wanted to clarify again. That said, The core logic — config serialization, segment stitching, and index offset calculation — has been reviewed manually. Happy to make any further changes if needed! |
1 participant
Summary
ExternalRecordingReferencestruct to project config andexternal_recordingsfield onProjectConfigurationProjectRecordingsMeta::new_with_external()to load primary + external recording segments togethercreate_all_segments()in the editor crate to build a unified segment list across all recordingsimport_cap_recordingTauri command: validates.capfolder, checks resolution match, prevents duplicate imports, computes correct clip index offsets, appends segments to the project timelineexternal_recordingsthroughCloses #1712
macOS Sequoia (Darwin 25) blocks ScreenCaptureKit on unsigned dev binaries —
GetShareableContent: The user declined TCCs for application, window, display capture— preventing the app from recording during development.Build is clean (
cargo build+pnpm typecheckpass). Needs testing on a signed build or by a reviewer with a valid Apple developer certificate.Known limitations
window.location.reload()on successful import loses any unsaved editor stateTest plan
.caprecording into an existing project.capfolder — should show error🤖 Generated with Claude Code
Greptile Summary
This PR adds Cap recording import/stitching to the timeline editor: a new
import_cap_recordingTauri command validates the source folder, checks resolution compatibility, deduplicates, computes clip index offsets, appendsTimelineSegments to the project config, and reloads the editor. Export and preview pipelines are extended viacreate_all_segments/new_with_externalto handle the concatenated segment list.tauri.conf.jsonflipscreateUpdaterArtifactsfromtruetofalseand removesSpacedrive.framework; both changes look unintentional and would break the auto-update pipeline for any release cut from this branch.clip_index_offsetcomputation silently uses0for any previously-imported external recording whose path is no longer resolvable (unwrap_or(0)), producing collidingrecording_clipindices that silently corrupt the project timeline on the next import.Confidence Score: 3/5
Not safe to merge as-is — two P1 issues need resolution before shipping.
The tauri.conf.json change disabling createUpdaterArtifacts would break the entire auto-update pipeline for any release built from this branch, and the silent unwrap_or(0) in clip index offset computation can corrupt the project config on subsequent imports when a prior external recording path is broken.
apps/desktop/src-tauri/tauri.conf.json (auto-update regression) and apps/desktop/src-tauri/src/lib.rs lines 2155–2173 (clip index offset corruption).
Vulnerabilities
No security concerns identified. External recording paths are validated for existence and correct metadata before processing. No user-supplied data reaches shell execution or SQL. The Tauri command is window-scoped and validates the calling window label before acting.
Important Files Changed
createUpdaterArtifactsflipped fromtruetofalse(breaks auto-update pipeline) andSpacedrive.frameworkremoved from macOS bundle — both look unintentional.import_cap_recordingTauri command with resolution/duplicate checks and timeline segment stitching; clip_index_offset silently falls back to 0 on broken external-recording paths, corrupting future imports.TrackRow.onImport/importingprops are dead code.new_with_externalwhich loads and appends external recording segments with a resolution guard;&PathBuf→&Pathcleanup is a nice improvement.create_all_segmentsiterates external recordings and delegates tocreate_segments;EditorInstance::newcorrectly switched tonew_with_externalandcreate_all_segments.ExternalRecordingReferencestruct andexternal_recordingsfield toProjectConfigurationwith#[serde(default)]— backward-compatible schema addition.create_all_segmentsandnew_with_externalwith external_recordings propagated through.Sequence Diagram
sequenceDiagram participant UI as Timeline UI participant TC as import_cap_recording (Tauri) participant FS as Filesystem participant PC as ProjectConfiguration participant EI as EditorInstance UI->>TC: importCapRecording(recording_path) TC->>FS: validate recording-meta.json TC->>FS: load ext RecordingMeta TC->>FS: load primary RecordingMeta TC->>TC: check resolution match (first segments) TC->>PC: load ProjectConfiguration TC->>TC: deduplicate check (path comparison) TC->>TC: compute clip_index_offset (primary.len + existing_externals.len) TC->>PC: push ExternalRecordingReference TC->>PC: append TimelineSegments (clip indices offset) TC->>PC: write ProjectConfiguration TC->>EI: EditorInstances::remove(window) TC->>UI: emit CapRecordingImported event UI->>UI: window.location.reload() UI->>EI: EditorInstance::new (loads new_with_external)Comments Outside Diff (1)
apps/desktop/src/routes/editor/Timeline/index.tsx, line 1071-1079 (link)TrackRowonImport/importingprops are dead codeonImportandimportingare added to theTrackRowprops interface and the overlay button is wired up insideTrackRow, but no caller in this diff ever passesonImportto aTrackRow. The overlay will never render. Either wire it up at the usage site or remove these props to avoid dead code.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix: apply Biome formatting to Timeline/..." | Re-trigger Greptile
(3/5) Reply to the agent's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!