Skip to content

Commit fdebd32

Browse files
committed
Delete: consume preview_id, oracle-aware walker (M3)
- `delete_volume_files_with_progress_inner` now calls `take_cached_scan_result(preview_id)` at the top. On hit, top-level files are recorded straight from `CopyScanResult` (no `is_directory` probe, no `list_directory` round-trip); top-level dirs recurse via the walker with `is_dir_hint = Some(true)` so the recursion never re-probes. - `scan_volume_recursive` consults `try_get_watched_listing(volume_id, path)` before every `list_directory`. Any subtree open in another pane and watcher-fresh is cache-fed; falls through to the volume call on miss, preserving the per-entry MTP progress callback. - No-preview path keeps the top-level `is_directory(source)` probe only when the source's parent isn't watcher-fresh. When the parent oracle hits, the `is_directory` flag comes from the cached `FileEntry`. - Cache-hit path emits a throttled scan-progress event per `progress_interval` while building the entry list, so the FE dialog stops looking frozen during the fast path (the original user-reported symptom). - New `delete_volume_reuse_tests.rs`: 4 integration tests covering preview consumption, no-preview walker correctness, parent-oracle replacing the `is_directory` probe, and the listing-closed-mid-walk fallthrough. - Docs: `write_operations/CLAUDE.md` data-flow diagram updated; new "Volume delete reuses the scan preview" Decision entry with the data-safety contract for stale cached entries.
1 parent 4918723 commit fdebd32

4 files changed

Lines changed: 674 additions & 40 deletions

File tree

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ network mounts, cross-filesystem moves, and name/path length limits.
2020
| `scan_preview.rs` | Scan preview subsystem for Copy dialog live stats: `start_scan_preview`, `cancel_scan_preview`, `is_scan_preview_complete`. Background scans (local and volume-based) with result caching. Emits `expected_files_total` / `expected_bytes_total` (sampled once at scan start from the drive index) on every `scan-preview-progress` event, alongside the running tallies and `current_dir`. |
2121
| `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`). |
2222
| `move_op.rs` | Same-fs: `fs::rename`. Cross-fs: copy to `.cmdr-staging-<uuid>`, atomic rename, delete sources. |
23-
| `delete.rs` | Scan, delete files first, then directories in reverse/deepest-first order. Not rollbackable. Also contains `delete_volume_files_with_progress` for non-local volumes (MTP): scans via `volume.list_directory(path, Some(&cb))` (per-entry throttled progress so the FE tally climbs mid-listing on slow MTP roundtrips), deletes via `volume.delete()` per item. Shared cumulative tally lives in an `Arc<VolumeScanTracker>` (atomics for files/dirs/bytes + `Mutex<Instant>` throttle) so the per-entry callback and the post-subtree snapshot agree across recursion levels. Both emit paths use `with_scan_meta(current_dir, dirs_done, None)` so the scanning UI shows the dir count and the directory the walker is currently in. |
23+
| `delete.rs` | Scan, delete files first, then directories in reverse/deepest-first order. Not rollbackable. Also contains `delete_volume_files_with_progress` for non-local volumes (MTP): consumes the scan preview via `take_cached_scan_result(preview_id)` first (top-level files come straight from `CopyScanResult` with no `is_directory` probe, top-level dirs recurse via the oracle-aware walker); on no-preview paths (MCP, programmatic) the top-level `is_directory(source)` probe stays unless the source's parent is watcher-fresh in `LISTING_CACHE`, in which case the type comes from the cached entry. The walker (`scan_volume_recursive`) consults `try_get_watched_listing(volume_id, path)` before every `list_directory`, so any subtree open in another pane is cache-fed. Scans via `volume.list_directory(path, Some(&cb))` (per-entry throttled progress so the FE tally climbs mid-listing on slow MTP roundtrips), deletes via `volume.delete()` per item. Shared cumulative tally lives in an `Arc<VolumeScanTracker>` (atomics for files/dirs/bytes + `Mutex<Instant>` throttle) so the per-entry callback and the post-subtree snapshot agree across recursion levels. Both emit paths use `with_scan_meta(current_dir, dirs_done, None)` so the scanning UI shows the dir count and the directory the walker is currently in. |
2424
| `eta.rs` | `EtaEstimator`: time-weighted EWMA per axis (bytes, files), τ ≈ 3 s. Combines via `max(ETA_bytes, ETA_files)`. One per `WriteOperationState`, fed by `state.enrich_progress` at every `write-progress` emit site. See [ETA + throughput](#eta--throughput) below. |
2525
| `trash.rs` | `move_to_trash_sync()` (macOS: ObjC `trashItemAtURL`; Linux: `trash` crate; reused by `commands/rename.rs`) and `trash_files_with_progress()` (batch trash with per-item progress, cancellation, partial failure). Uses `symlink_metadata()` for existence checks (handles dangling symlinks). |
2626
| `copy_strategy.rs` | Strategy selection per file: network FS → chunked copy; overwrite → temp+rename; macOS → `copyfile(3)`; Linux → `copy_file_range(2)`. |
@@ -34,6 +34,7 @@ network mounts, cross-filesystem moves, and name/path length limits.
3434
| `tests.rs` | Unit tests. |
3535
| `copy_integration_test.rs` | Copy operation integration tests (permissions, symlinks, xattrs, edge cases). |
3636
| `delete_integration_test.rs` | Delete operation integration tests. |
37+
| `delete_volume_reuse_tests.rs` | Volume-delete tests for scan-preview reuse and oracle fast paths (M3 of `fresh-listing-reuse-plan.md`). |
3738
| `move_integration_test.rs` | Same-fs and cross-fs move integration tests. |
3839
| `transaction_integration_test.rs` | CopyTransaction record/rollback/commit tests. |
3940
| `validation_integration_test.rs` | Validation functions, safety checks, path length, disk space tests. |
@@ -49,6 +50,10 @@ Frontend
4950
→ tokio::task::spawn_blocking (local I/O) or direct async (volume ops)
5051
→ validate (sources exist, dest writable, not same location, dest not inside source)
5152
→ scan phase: walk_dir_recursive, emit scan-progress events
53+
(delete on a volume also: `take_cached_scan_result(preview_id)` first;
54+
on hit, build the entry list from `per_path` — top-level files come
55+
straight from the cache, top-level dirs recurse via the oracle-aware
56+
walker; on miss, fall through to `scan_volume_recursive`)
5257
→ disk space check (statvfs)
5358
→ execute phase: per-file copy/delete
5459
→ throttled write-progress events (200ms default)
@@ -236,6 +241,9 @@ exits, partial files or staging directories may remain on disk. These use the `.
236241
**Decision**: `delete_files_start` routes to either `delete_files_with_progress` (local, uses `walkdir` + `fs::remove_file`) or `delete_volume_files_with_progress` (non-local, uses `Volume` trait) based on `volume_id`.
237242
**Why**: MTP volumes can't use `walkdir` or `fs::remove_*`. Rather than refactoring the existing local delete to go through the Volume trait (which would add overhead for local ops), we keep the fast local path and add a parallel volume-aware path. Both emit identical events so the frontend progress dialog works unchanged.
238243

244+
**Decision**: Volume delete reuses the scan preview and is oracle-aware on the no-preview path.
245+
**Why**: Before this, `delete_volume_files_with_progress_inner` ignored `config.preview_id` entirely and ran `scan_volume_recursive` again. On MTP that meant a second 17 s parent listing for a 135-photo `/DCIM/Camera` delete after the user had just paid that cost in the pre-flight dialog — and the second scan emitted no per-top-level-file progress, so the UI looked frozen. The fix has three parts. (1) `delete_volume_files_with_progress_inner` calls `take_cached_scan_result(preview_id)` at the top; on hit, top-level files are recorded from `CopyScanResult::total_bytes` with no `is_directory` probe and no `list_directory` round-trip, and top-level dirs recurse via the oracle-aware `scan_volume_recursive` (passing `is_dir_hint = Some(true)` so the recursion never re-probes). (2) The walker's internal `volume.list_directory(path, ...)` is now preceded by `try_get_watched_listing(volume_id, path)`; on hit, the cached entries replace the volume call entirely at every recursion level. (3) On the no-preview path (MCP triggers, programmatic deletes), the top-level `volume.is_directory(source)` probe stays only when the parent oracle misses — when a pane has the source's parent open and watcher-fresh, the type comes from the cached `FileEntry` and the probe is skipped. The cache-hit path also emits a throttled scan-progress event per `progress_interval` while building the entry list, so the FE dialog shows movement during the fast path instead of waiting for the delete phase to start. Pinned by `delete_volume_reuse_tests.rs`. Data-safety contract: stale-by-one cached entries can either silently skip a now-gone file (acceptable: the user already moved it elsewhere) or attempt to delete a missing one (the volume's `delete` errors cleanly). Neither direction can delete the wrong file because we feed `volume.delete(&entry.path)` exact paths the cache observed; a cached entry that races with a concurrent rename ends up addressing the old path the next call won't find.
246+
239247
**Decision**: Keep `exacl` crate for ACL copy in chunked copies (not custom FFI bindings).
240248
**Why**: `exacl` adds zero new transitive dependencies (all of its deps, `bitflags`, `log`, `scopeguard`, `uuid`, are already in our tree). It provides cross-platform ACL support (macOS, Linux, FreeBSD) and full ACL parsing/manipulation for potential future UI features. The crate appears unmaintained (last release Feb 2024) but ACL APIs are stable and don't change. Our usage is best-effort with graceful fallback: if `exacl` ever breaks, files still copy, they just lose ACLs. MIT licensed (compatible with BSL).
241249

0 commit comments

Comments
 (0)