Skip to content

Commit 067b96d

Browse files
committed
File ops: real progress for cross-volume transfers
- Cross-volume move now streams intra-file progress (the bug behind "Moving... 0 bytes / 0 files / 0 dirs" sitting on the FE for the entire upload). The `no_progress` no-op closure in `volume_move.rs` is replaced with `make_serial_per_file_progress` — same chunked progress callback `volume_copy` already used. - Scan phase emits climbing tallies as the walk progresses. `scan_volume_sources` now passes a throttled `ListingProgress` callback through `scan_for_copy_batch_with_progress` (the existing `_with_progress` trait variant), then a final throttle-bypassed emit with the aggregate totals. Without this, the FE showed "Scanning... 0 / 0 / 0" for the entire scan on programmatic / MCP-triggered ops with no `preview_id`. - Per-file milestone emit lives in `drive_transfer_serial_async`'s `Transferred` arm — one place, fires uniformly for serial copy + cross-volume move + same-volume rename. Concurrent copy got the same milestone in its `FuturesUnordered` `Ok` arm. Bypasses the throttle because it's a per-file event (bounded by file count) and the FE's files-done axis needs a Copying event with the bumped value — chunked emits carry the pre-iteration snapshot, so without this the axis would never cross `N/N` for the last file of any op. - Per-file progress callback builders (`make_serial_per_file_progress`, `make_concurrent_per_file_progress`) extracted into `transfer_driver.rs`, sharing one private `try_emit_throttled_progress` throttle gate. Replaces three near-identical inline closures (copy-serial, copy-concurrent, cross-volume-move) that all did the same dance. - Move closures simplified: dropped their own per-source emits (driver owns the milestone now) and the throttle-bypass workaround. `last_progress_time` mutex, `events` / `state` / `operation_id` clones, and `WriteProgressEvent` import all gone from `volume_move.rs`. - Six new regression tests pin the contracts: intra-file `Copying` emits for move + copy serial + copy concurrent; `Copying`-phase `files_done = N` milestone for serial + concurrent copy; `Scanning`-phase tallies during cross-volume move walk. 60/60 passes across 10 runs — not flaky. - Three pre-existing bulk-skip tests updated to filter by `phase == Copying` first (the `Scanning` phase now also carries `files_done > 0`, which collided with the `find(|p| p.files_done > 0)` pattern). Matches the original test intent more precisely. - `transfer/CLAUDE.md` picks up four Decision entries documenting the helper extraction, the per-file milestone living in the driver, the scan-tally wiring, and (kept from earlier in the series) why move's per-source emit historically bypassed the throttle before this commit consolidated it into the driver.
1 parent 3622fc9 commit 067b96d

7 files changed

Lines changed: 818 additions & 190 deletions

File tree

apps/desktop/src-tauri/src/file_system/write_operations/transfer/CLAUDE.md

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,14 @@ Frontend counterpart: [`apps/desktop/src/lib/file-operations/transfer/CLAUDE.md`
1212
|------|----------------|
1313
| `copy.rs` | `copy_files_with_progress`: scan → disk space check → per-file copy via `copy_single_item`. `CopyTransaction` for rollback. The per-source execute loop runs through `drive_transfer_serial_sync` (`transfer_driver.rs`); the closure captures `&mut transaction` / `&mut created_dirs` / `&mut tracker` / `&mut apply_to_all_resolution` and threads them into `copy_single_item`. Pre-flight scan / dry-run / disk-space / bulk-skip filtering stay outside the driver. Post-loop dispatch matches on `PostLoopIntent` (Completed / Cancelled / Failed) and reproduces the historic three-arm shape — including the post-completion `RollingBack` recheck for the click-during-the-last-millisecond race (commit `1de4255d`). |
1414
| `move_op.rs` | Same-fs: `fs::rename`. Cross-fs: copy to `.cmdr-staging-<uuid>`, atomic rename, delete sources. |
15-
| `transfer_driver.rs` | Shared per-source transfer driver for copy/move ops. Owns the bulk-skip prelude, per-iter cancellation check, conflict-resolve dispatch (async path), per-iter skip accounting, and paired progress/status emit. Two sibling entry points: `drive_transfer_serial_sync` (used by `copy.rs::copy_files_with_progress_inner`; closure captures `&mut CopyTransaction` / `&mut created_dirs` / `&mut SourceItemTracker`) and `drive_transfer_serial_async` (used by `volume_copy::copy_volumes_with_progress`'s serial path and both `volume_move` paths; driver owns top-level conflict detection + dispatch). The `FuturesUnordered` concurrent path in `volume_copy.rs` stays inline (1-of-4 abstraction; see plan § "Concurrent driver scope"). |
15+
| `transfer_driver.rs` | Shared per-source transfer driver for copy/move ops. Owns the bulk-skip prelude, per-iter cancellation check, conflict-resolve dispatch (async path), per-iter skip accounting, and paired progress/status emit. Two sibling entry points: `drive_transfer_serial_sync` (used by `copy.rs::copy_files_with_progress_inner`; closure captures `&mut CopyTransaction` / `&mut created_dirs` / `&mut SourceItemTracker`) and `drive_transfer_serial_async` (used by `volume_copy::copy_volumes_with_progress`'s serial path and both `volume_move` paths; driver owns top-level conflict detection + dispatch). Also houses `make_serial_per_file_progress` + `make_concurrent_per_file_progress`: per-file `on_progress` callback builders for `copy_single_path`, sharing one private `try_emit_throttled_progress` throttle gate. The `FuturesUnordered` concurrent path in `volume_copy.rs` stays inline (1-of-4 abstraction; see plan § "Concurrent driver scope"). |
1616
| `copy_strategy.rs` | Strategy selection per file: network FS → chunked copy; overwrite → temp+rename; macOS → `copyfile(3)`; Linux → `copy_file_range(2)`. |
1717
| `macos_copy.rs` | FFI to macOS `copyfile(3)`. Preserves xattrs, ACLs, resource forks, Finder metadata. Supports APFS `clonefile`. |
1818
| `linux_copy.rs` | Linux `copy_file_range(2)` with reflink support on btrfs/XFS. 4 MB chunks, cancellation between iterations. |
1919
| `chunked_copy.rs` | 1 MB chunked read/write, the default copy method for all non-APFS-clonefile copies on macOS and network copies on Linux. Checks cancellation between chunks. Copies xattrs, ACLs, timestamps. |
2020
| `volume_copy.rs` | Volume-to-volume copy (Local↔MTP↔SMB): `copy_between_volumes`, `scan_for_volume_copy`. Uses `OperationEventSink` (not `AppHandle` directly) for event emission. Handles conflict detection, resolution, progress, rollback (delete all copied files in reverse with progress), and partial-file cleanup on cancel. Shared `map_volume_error` helper. |
2121
| `volume_move.rs` | Volume-to-volume move: `move_between_volumes`, `move_within_same_volume`. Same-volume uses `Volume::rename`; cross-volume does copy+delete. |
22-
| `volume_preflight.rs` | Shared preflight scan for both volume copy and move: `scan_volume_sources` returns a `VolumePreflight { total_files, total_bytes, source_hints }`. Reuses a cached preview from `TransferDialog` when one is available; otherwise dispatches `volume.scan_for_copy_batch` (so MTP's group-by-parent and SMB's pipelined-stat optimizations still kick in). Emits one `WriteProgressEvent { phase: Scanning, … }` so the FE sees the scan stage. On pre-scan cancel, emits `write-cancelled` and returns `Err(Cancelled)` so the FE dialog closes cleanly. |
22+
| `volume_preflight.rs` | Shared preflight scan for both volume copy and move: `scan_volume_sources` returns a `VolumePreflight { total_files, total_bytes, source_hints }`. Reuses a cached preview from `TransferDialog` when one is available; otherwise dispatches `volume.scan_for_copy_batch_with_progress` (so MTP's group-by-parent and SMB's pipelined-stat optimizations still kick in). Emits `WriteProgressEvent { phase: Scanning, … }` events: one kickoff at `0/0/0`, throttled per-listing tallies as the walk progresses, and one final aggregate emit (throttle-bypassed) so the FE's scan-phase tally lands on the real total. On pre-scan cancel, emits `write-cancelled` and returns `Err(Cancelled)` so the FE dialog closes cleanly. |
2323
| `volume_conflict.rs`, `volume_strategy.rs` | Conflict resolution (Stop/Skip/Overwrite/Rename/OverwriteSmaller/OverwriteOlder) and copy strategy selection for volume operations. `volume_conflict.rs` mirrors the local-FS `reduce_conditional_resolution` from `../helpers.rs` with its own `reduce_volume_conditional_resolution` (async, uses size hints + `get_metadata` for mtime). |
2424
| `copy_integration_test.rs` | Copy operation integration tests (permissions, symlinks, xattrs, edge cases). |
2525
| `move_integration_test.rs` | Same-fs and cross-fs move integration tests. |
@@ -78,6 +78,15 @@ Frontend counterpart: [`apps/desktop/src/lib/file-operations/transfer/CLAUDE.md`
7878
**Decision**: Cross-FS local move reuses the scan-preview cache via `config.preview_id`.
7979
**Why**: `move_with_staging` used to ignore `preview_id` and always re-run `scan_sources`. The FE had just paid the cost of a full scan in `TransferDialog` (which emits cumulative `scan-preview-progress` events), so the second BE-side scan starting from `filesDone=0` made the count visibly reset in `TransferProgressDialog` ("scan again from 0, climb to N, then phase flips to Copying and the bar jumps to total"). Now the function checks `config.preview_id` first: on cache hit the `ScanResult` is consumed directly (same shape `copy.rs` uses), skipping the redundant scan and going straight to the active phase. On miss (no preview at all) the original `scan_sources` path stays — so MCP triggers and programmatic moves still work.
8080

81+
**Decision**: Per-file `on_progress` callbacks for `copy_single_path` come from shared builders in `transfer_driver.rs` (`make_serial_per_file_progress` for one-source-in-flight paths, `make_concurrent_per_file_progress` for the `FuturesUnordered` path). Both delegate to a private `try_emit_throttled_progress` core.
82+
**Why**: Three near-identical closures (serial-copy, concurrent-copy, cross-volume-move) used to inline the same dance: throttle-gate, atomic delta tracking, paired `WriteProgressEvent` + `update_operation_status` emit. The move site's variant degenerated to a no-op `Continue(())` callback because the original move code shipped `bytes_total = 0` and intra-file progress couldn't fill the bar; once `volume_preflight.rs` started populating `bytes_total`, the FE's Size bar pinned at 0 throughout a multi-minute cross-volume upload (observed against an SMB destination with a 3.7 GB file: "Moving... 0 bytes / 0 files / 0 dirs" the entire time). Centralizing the shape means a fix lands once and stays correct across all three sites. The concurrent variant carries an extra per-task `last_file_bytes: Arc<AtomicU64>` parameter so the orchestrating task can detect "volume never invoked on_progress" and credit the file's bytes as a compensation; the serial variant captures snapshots (`bytes_done_so_far`, `files_done_so_far`) from the driver's iteration context instead. Pinned by `cross_volume_move_emits_intra_file_progress`, `test_cross_volume_copy_serial_emits_intra_file_progress`, and `test_cross_volume_copy_concurrent_emits_intra_file_progress`.
83+
84+
**Decision**: Per-file milestone emit lives in `drive_transfer_serial_async`'s `Transferred` arm (one place, fires uniformly for serial copy + cross-volume move + same-volume rename).
85+
**Why**: Chunked `on_progress` emits inside `transfer_one` carry `files_done_so_far` (the driver's iteration snapshot taken before this file started), so for single-file ops the chunked path never crosses `N/N` — the user would see "Copying... 99% / 0 of 1 files" jump straight to the complete toast, never observing the final "1 of 1" milestone. The Transferred arm bumps the counters AND emits an unthrottled `WriteProgressEvent` with the bumped values, mirrored in the concurrent path's task-complete branch (`copy_volumes_with_progress`'s `Some(Ok(...))` arm reads the shared atomics and emits the same shape). The throttle bypass is deliberate: per-file milestones are bounded by file count (not noisy), and throttle suppression of this event is exactly the bug being fixed. Move closures used to do their own per-source emit; that's now redundant and removed. The `cross_volume_move_cancel_mid_batch_preserves_completed` test still passes because the driver's milestone observes `files_done >= N` between sources before the next-iter cancellation check fires.
86+
87+
**Decision**: `scan_volume_sources` emits climbing `Scanning`-phase tallies via `scan_for_copy_batch_with_progress`.
88+
**Why**: Without a cached `preview_id` (programmatic moves, MCP-triggered ops), the operation's scan phase used to emit a single `0/0/0` event up front, then sit silent through the entire `scan_for_copy_batch` call before flipping to `Copying`. On slow sources (cold MTP listing, large SMB tree) the FE shows "Scanning... 0 bytes / 0 files / 0 dirs" the whole duration. The scan-preview pipeline emits its own climbing tallies into the preview's event channel, not the operation's, so the operation needs to wire its own. The fix: pass a throttled `Fn(ListingProgress)` callback through `scan_for_copy_batch_with_progress` (the existing `_with_progress` trait variant) that emits a `Scanning`-phase event per tick, plus a final throttle-bypassed emit with the aggregate totals so a fast scan whose per-listing emits all got throttled still lands on the right number. The kickoff + final emits frame the per-tick stream. Pinned by `cross_volume_move_emits_scan_phase_tallies_during_walk`.
89+
8190
**Decision**: Volume move runs the same preflight scan as volume copy (extracted to `volume_preflight.rs`).
8291
**Why**: Both `move_volumes_with_progress` (cross-volume copy+delete) and `move_within_same_volume_with_progress` (rename) used to skip the scan phase entirely, sending `bytes_total = 0` on every progress event. The FE's `TransferProgressDialog` hides the Size progress bar behind `{#if bytesTotal > 0}`, so during an MTP→local move the user saw only the Files bar with no size feedback — even though `bytes_done` was being tracked correctly. The fix shares one helper (`scan_volume_sources`) between copy and move: reuses a cached `TransferDialog` preview when available (free in the common dialog-driven path), falls through to `volume.scan_for_copy_batch` otherwise. Move now gets the same `(total_files, total_bytes, source_hints)` triple copy has, so progress events carry the real `bytes_total`, the per-source `is_directory` probe inside the move loop is gone (hint comes from the scan), and the same-volume rename's per-iter `get_metadata` for size is gone (hint again). The previous `collect_known_directory_paths` helper (file-only-bulk-skip via per-source `get_metadata`) is replaced by `VolumePreflight::known_directory_paths()`. Behavior change to flag: programmatic moves with no `preview_id` (MCP, etc.) now pay one batch scan up front; for MTP this is ~one parent listing's RTT. Same cost copy has always paid; consistent across both ops. Pinned by `volume_move::tests::*_emits_bytes_total_on_progress`.
8392

0 commit comments

Comments
 (0)