Skip to content

Commit 9445e61

Browse files
committed
File ops: Scanning UI no longer flashes back to 0
- Local delete (Shift+F8) and cross-FS local move (F6) now reuse the scan-preview cache via `config.preview_id` instead of re-scanning from 0. The transfer progress dialog goes straight to the active phase with the right totals. - Removed the scan-phase progress bar in `DeleteDialog` and `ScanPhaseBody`. A full-width bar during scanning visually read as "already deleting/copying". The scan UI is now just tallies, spinner, throughput, current dir. - In volume scan previews (MTP, SMB), the running file count no longer drops between parent groups or sibling subtrees. `run_oracle_aware_batch_scan` and `scan_subtree_with_oracle` now shift the volume backend's per-call local count by a cumulative baseline before forwarding to the FE.
1 parent e7660b3 commit 9445e61

11 files changed

Lines changed: 177 additions & 142 deletions

File tree

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,9 @@ volume-delete handler.
274274
**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`.
275275
**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.
276276

277+
**Decision**: Local delete and cross-FS local move reuse the scan-preview cache via `config.preview_id`.
278+
**Why**: Both `delete_files_with_progress_inner` and `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 `DeleteDialog` / `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 Deleting/Copying and the bar jumps to total"). Now both functions check `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 (e.g. empty `files` list from a volume preview cache, or no preview at all) the original `scan_sources` path stays — so MCP triggers, programmatic deletes, and forensic dialog flows still work. Delete also gets an initial `phase: Deleting` `WriteProgressEvent` emit after the cache hit so the FE switches to the active-phase UI with the right denominator; move already had this for Copying. Same template as `delete_volume_files_with_progress_inner`'s cache reuse, just for the local-FS paths.
279+
277280
**Decision**: Volume delete reuses the scan preview and is oracle-aware on the no-preview path.
278281
**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.
279282

@@ -309,6 +312,9 @@ and `statfs.f_fstypename` for APFS. See `copy_strategy.rs` for the implementatio
309312
**Gotcha**: On macOS, never use `statvfs` alone for disk space checks; use `NSURLVolumeAvailableCapacityForImportantUsageKey`
310313
**Why**: `statvfs` reports only physically free blocks. On APFS, purgeable space (iCloud caches, APFS snapshots) can account for tens of GB that macOS will reclaim on demand. Using `statvfs` causes the "insufficient space" error to reject copies that would actually succeed, and shows a different available-space number than the status bar (which uses the NSURL API). `validate_disk_space` in `helpers.rs` calls `crate::volumes::get_volume_space()` on macOS and falls back to `statvfs` on Linux.
311314

315+
**Gotcha**: Volume-side `on_progress` callbacks report counts LOCAL to the current scan operation, not cumulative.
316+
**Why**: `Volume::scan_for_copy_batch_with_progress` and `scan_subtree_with_oracle` both invoke `on_progress(count)` with a count local to the current `list_directory` call / subtree (starts at 1 each time). Forwarding that unchanged through `run_volume_scan_preview`'s closure made the FE's running tally drop visibly between parent groups, between sibling top-level dirs in a cache-hit branch, and between recursion frames inside `scan_subtree_with_oracle`. `run_oracle_aware_batch_scan` now wraps `on_progress` with a `baseline = aggregate.file_count` shift before each scan call (cold-cache batch + cache-hit subtree), and `scan_subtree_with_oracle` does the same at its own recursion site (`baseline = totals.file_count`). The visible FE count stays cumulative across the entire scan. Direct `on_progress(aggregate.file_count)` emit sites in `run_oracle_aware_batch_scan` (cache-hit per-file paths, fallthrough `scan_for_copy` after a name miss) stay unwrapped — they're already cumulative. Future scan call sites that delegate to a volume backend or to `scan_subtree_with_oracle` need the same baseline wrap.
317+
312318
**Gotcha**: Hardlink dedup doesn't straddle the oracle/walk boundary.
313319
**Why**: `walk_dir_recursive` dedupes hardlinks by inode for `total_bytes` (so a hardlink-heavy tree like cargo's `target/` reports correct bytes-to-free). `FileEntry` doesn't carry inode, so when the oracle supplies one half of a hardlink pair from the cached listing and a real walk supplies the other half, the dedup misses and bytes get over-counted. Direction is safe: over-counting → pessimistic ETA + conservative disk-space reject, never the other way. The walker's existing `walker_dedupes_hardlinks_by_inode` test still pins the same-side dedup. If true cross-boundary dedup ever becomes worth it, add `inode: Option<u64>` to `FileEntry`; not in this milestone.
314320

apps/desktop/src-tauri/src/file_system/write_operations/delete.rs

Lines changed: 68 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,47 @@ pub(super) fn delete_files_with_progress_inner(
3939
sources: &[PathBuf],
4040
config: &WriteOperationConfig,
4141
) -> Result<(), WriteOperationError> {
42-
// Phase 1: Scan to get file count (delete uses default sorting)
43-
let scan_result = scan_sources(
44-
sources,
45-
state,
46-
events,
47-
operation_id,
48-
WriteOperationType::Delete,
49-
config.sort_column,
50-
config.sort_order,
51-
)?;
42+
// Phase 1: Scan (or reuse cached preview results)
43+
let scan_result = if let Some(preview_id) = &config.preview_id {
44+
// Volume scans cache aggregate stats with an empty `files` list; the
45+
// per-file delete loop needs the file list, so treat an empty-files
46+
// cache hit the same as a miss and fall through to a fresh local scan.
47+
if let Some(cached) = take_cached_scan_result(preview_id).filter(|c| !c.files.is_empty()) {
48+
log::debug!(
49+
"delete_files_with_progress: reusing cached scan for operation_id={}, preview_id={}, files={}, bytes={}",
50+
operation_id,
51+
preview_id,
52+
cached.file_count,
53+
cached.total_bytes
54+
);
55+
cached
56+
} else {
57+
log::warn!(
58+
"preview_id={} cache miss despite frontend coordination, starting fresh scan for operation_id={}",
59+
preview_id,
60+
operation_id
61+
);
62+
scan_sources(
63+
sources,
64+
state,
65+
events,
66+
operation_id,
67+
WriteOperationType::Delete,
68+
config.sort_column,
69+
config.sort_order,
70+
)?
71+
}
72+
} else {
73+
scan_sources(
74+
sources,
75+
state,
76+
events,
77+
operation_id,
78+
WriteOperationType::Delete,
79+
config.sort_column,
80+
config.sort_order,
81+
)?
82+
};
5283

5384
// Handle dry-run mode (delete has no conflicts)
5485
if config.dry_run {
@@ -69,6 +100,33 @@ pub(super) fn delete_files_with_progress_inner(
69100
let mut bytes_done = 0u64;
70101
let mut last_progress_time = Instant::now();
71102

103+
// Emit initial Deleting-phase event with totals. Important when reusing a
104+
// cached preview: no scanning events were emitted by the BE, so the FE
105+
// still has scan-phase tallies on screen. This event flips the FE to the
106+
// active-phase UI with the correct denominator on file/byte progress.
107+
state.emit_progress_via_sink(
108+
events,
109+
WriteProgressEvent::new(
110+
operation_id.to_string(),
111+
WriteOperationType::Delete,
112+
WriteOperationPhase::Deleting,
113+
None,
114+
0,
115+
scan_result.file_count,
116+
0,
117+
scan_result.total_bytes,
118+
),
119+
);
120+
update_operation_status(
121+
operation_id,
122+
WriteOperationPhase::Deleting,
123+
None,
124+
0,
125+
scan_result.file_count,
126+
0,
127+
scan_result.total_bytes,
128+
);
129+
72130
let mut tracker = SourceItemTracker::new(&scan_result.files);
73131

74132
// Delete files

apps/desktop/src-tauri/src/file_system/write_operations/move_op.rs

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use std::time::Instant;
88

99
use super::copy::copy_single_item;
1010
use super::helpers::{is_same_filesystem, remove_dir_all_in_background, resolve_conflict, spawn_async_sync};
11-
use super::scan::{SourceItemTracker, handle_dry_run, scan_sources};
11+
use super::scan::{SourceItemTracker, handle_dry_run, scan_sources, take_cached_scan_result};
1212
use super::state::{CopyTransaction, OperationIntent, WriteOperationState, load_intent, update_operation_status};
1313
use super::types::{
1414
ConflictResolution, IoResultExt, OperationEventSink, TauriEventSink, WriteCancelledEvent, WriteCompleteEvent,
@@ -321,16 +321,47 @@ fn move_with_staging(
321321
destination: &Path,
322322
config: &WriteOperationConfig,
323323
) -> Result<(), WriteOperationError> {
324-
// Phase 1: Scan (move uses default sorting - order doesn't matter much for move)
325-
let scan_result = scan_sources(
326-
sources,
327-
state,
328-
events,
329-
operation_id,
330-
WriteOperationType::Move,
331-
config.sort_column,
332-
config.sort_order,
333-
)?;
324+
// Phase 1: Scan (or reuse cached preview results)
325+
let scan_result = if let Some(preview_id) = &config.preview_id {
326+
// Volume scans cache aggregate stats with an empty `files` list; the
327+
// per-file move loop needs the file list, so treat an empty-files
328+
// cache hit the same as a miss and fall through to a fresh local scan.
329+
if let Some(cached) = take_cached_scan_result(preview_id).filter(|c| !c.files.is_empty()) {
330+
log::debug!(
331+
"move_with_staging: reusing cached scan for operation_id={}, preview_id={}, files={}, bytes={}",
332+
operation_id,
333+
preview_id,
334+
cached.file_count,
335+
cached.total_bytes
336+
);
337+
cached
338+
} else {
339+
log::warn!(
340+
"preview_id={} cache miss despite frontend coordination, starting fresh scan for operation_id={}",
341+
preview_id,
342+
operation_id
343+
);
344+
scan_sources(
345+
sources,
346+
state,
347+
events,
348+
operation_id,
349+
WriteOperationType::Move,
350+
config.sort_column,
351+
config.sort_order,
352+
)?
353+
}
354+
} else {
355+
scan_sources(
356+
sources,
357+
state,
358+
events,
359+
operation_id,
360+
WriteOperationType::Move,
361+
config.sort_column,
362+
config.sort_order,
363+
)?
364+
};
334365

335366
// Create staging directory
336367
let staging_dir = destination.join(format!(".cmdr-staging-{}", operation_id));

apps/desktop/src-tauri/src/file_system/write_operations/scan.rs

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -272,15 +272,35 @@ pub(super) async fn scan_subtree_with_oracle(
272272
}
273273
let child_path = PathBuf::from(&entry.path);
274274
if entry.is_directory && !entry.is_symlink {
275-
// Recurse — oracle re-applies inside this call.
276-
let child_totals = Box::pin(scan_subtree_with_oracle(
277-
volume,
278-
volume_id,
279-
&child_path,
280-
is_cancelled,
281-
on_progress,
282-
))
283-
.await?;
275+
// Recurse — oracle re-applies inside this call. The recursive
276+
// emit reports counts local to the child subtree (starting at 1),
277+
// so wrap `on_progress` with a baseline of the current
278+
// `totals.file_count`; without this the FE display visibly drops
279+
// back to 1 every time the walker descends into a new sibling dir.
280+
let baseline = totals.file_count;
281+
let child_totals = match on_progress {
282+
Some(cb) => {
283+
let shifted = move |n: usize| cb(baseline + n);
284+
Box::pin(scan_subtree_with_oracle(
285+
volume,
286+
volume_id,
287+
&child_path,
288+
is_cancelled,
289+
Some(&shifted),
290+
))
291+
.await?
292+
}
293+
None => {
294+
Box::pin(scan_subtree_with_oracle(
295+
volume,
296+
volume_id,
297+
&child_path,
298+
is_cancelled,
299+
None,
300+
))
301+
.await?
302+
}
303+
};
284304
totals.file_count += child_totals.file_count;
285305
// The directory itself plus all its descendant dirs.
286306
totals.dir_count += 1 + child_totals.dir_count;

apps/desktop/src-tauri/src/file_system/write_operations/scan_preview.rs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -447,8 +447,14 @@ pub(super) async fn run_oracle_aware_batch_scan(
447447
};
448448

449449
if entry.is_directory && !entry.is_symlink {
450+
// `scan_subtree_with_oracle` emits a count local to this
451+
// subtree (starting at 1). Shift by `aggregate.file_count`
452+
// so the FE display stays cumulative across multiple
453+
// top-level dirs in this call.
454+
let baseline = aggregate.file_count;
455+
let shifted = |n: usize| on_progress(baseline + n);
450456
let subtree: SubtreeTotals =
451-
scan_subtree_with_oracle(volume, volume_id, source, is_cancelled, Some(on_progress)).await?;
457+
scan_subtree_with_oracle(volume, volume_id, source, is_cancelled, Some(&shifted)).await?;
452458
aggregate.file_count += subtree.file_count;
453459
// `scan_for_copy_batch`'s aggregate.dir_count counts descendants
454460
// only, not the top-level path itself. Match that convention
@@ -484,8 +490,17 @@ pub(super) async fn run_oracle_aware_batch_scan(
484490
// Cold cache for this parent. Delegate to the volume's own batch
485491
// scan: it preserves the MTP parent-grouping and SMB pipelined-stat
486492
// optimizations for cold paths.
493+
//
494+
// The volume's callback reports a count LOCAL to its current
495+
// `list_directory` call (starts at 1). Shift by `aggregate.file_count`
496+
// before forwarding so the FE display stays cumulative as we walk
497+
// multiple parent groups — without this, every new group's first
498+
// entry drops the visible count back to 1, then climbs to the
499+
// group's local total before the next group restarts.
500+
let baseline = aggregate.file_count;
501+
let shifted = |n: usize| on_progress(baseline + n);
487502
let group_result = volume
488-
.scan_for_copy_batch_with_progress(paths_in_group, Some(on_progress))
503+
.scan_for_copy_batch_with_progress(paths_in_group, Some(&shifted))
489504
.await?;
490505
aggregate.file_count += group_result.aggregate.file_count;
491506
aggregate.dir_count += group_result.aggregate.dir_count;

apps/desktop/src/lib/file-operations/CLAUDE.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,11 @@ F8/Shift+F8 (trash/delete). Transfer and delete operations share `TransferProgre
4343
- Dynamic stage indicator: "Scanning" → "Copying" (+ "Cleaning up" for cross-FS move)
4444
- **Scanning-phase UI** (both `waitingForScan` and `phase === 'scanning'` paths): rendered via the
4545
`ScanPhaseBody.svelte` child component. Shows source path, running tallies (`bytesFound / filesFound / dirsFound`),
46-
FE-computed throughput from `ScanThroughput` (`scan-throughput.ts`), and, when the backend supplies
47-
`expectedFilesTotal` / `expectedBytesTotal` from the drive index, a `ProgressBar` capped at 100% with "X% of
48-
estimated" text. Current directory (`event.currentDir`) renders above the filename so the user sees where in the
49-
tree the walker is. Title is reframed per operation: "Verifying before copy…", "Counting items to delete…", etc.
46+
FE-computed throughput from `ScanThroughput` (`scan-throughput.ts`), and a spinner. Current directory
47+
(`event.currentDir`) renders above the filename so the user sees where in the tree the walker is. Title is reframed
48+
per operation: "Verifying before copy…", "Counting items to delete…", etc. The backend still emits
49+
`expectedFilesTotal` / `expectedBytesTotal` on scan events but the FE ignores them — the bar this used to drive was
50+
visually indistinguishable from the destructive-phase bar and read as "already deleting".
5051
- Conflict resolution inline (if using `Stop` mode instead of dry-run). The per-file dialog has a 2-column grid: left
5152
column is the single-file action (`Skip` / `Rename` / `Overwrite`), right column is the apply-to-all variant
5253
(`Skip all` / `Rename all` / `Overwrite all`). A 4th row holds the two conditional bulk actions —

0 commit comments

Comments
 (0)