Skip to content

sweep/2026-10-04: 50 fixes, 15 refactors (36→43 tests) - #3

Open
Chartres wants to merge 100 commits into
mainfrom
sweep/2026-10-04
Open

Chartres wants to merge 100 commits into
mainfrom
sweep/2026-10-04

Conversation

@Chartres

@Chartres Chartres commented Oct 3, 2026

Copy link
Copy Markdown
Owner

Sweep 2026-10-04 — mac-dir-stat

5 cycles · 100 commits · tests: 36 → 43

Findings fixed (50 total: 9H · 28M · 13L)

# File Symptom Sev
1 app.rs perform_delete no is_alive check; trashes wrong path if node died while dialog was open H
2 scanner/walk.rs APFS firmlinks (/Users /Applications etc.) treated as volume roots; fake Free Space injected H
3 scanner/walk.rs Dir not in dir_map after metadata fail; children get wrong full_path → Trash hits wrong file H
4 ui/context_menu.rs "Move to Trash" offered for scan root and synthetic nodes H
5 app.rs Cmd+Backspace no scan-root guard H
6 scanner/tree.rs remove_node no dead-node guard; double-call corrupts ancestor sizes M
7 app.rs poll_partial_refresh keeps stale ConfirmTrash / ConfirmBatchTrash ids after graft M
8 app.rs Cmd+Backspace / Enter not gated by wants_keyboard_input; search-clear opens trash dialog M
9 scanner/walk.rs TCC skip list applied even with Full Disk Access granted M
10 app.rs Batch-trash failures silently go to stderr only M
11 app.rs view_root / zoom_stack / selection not sanitized after delete; dead view_root M
12 scanner/mod.rs Panic in scan thread drops sender; UI stuck at "Scanning…" forever M
13 scanner/tree.rs remove_node on dead node — now idempotent; test added M
14 ui/treemap_view.rs Selecting "(none)" file type dimmed extensionless files (extension None ≠ "") M
15 app.rs poll_partial_refresh grafts without checking target alive; sets selected_node to dead id M
16 scanner/walk.rs /System/Volumes in skip_paths blocked the scan-root exception → empty tree on Data volume M
17 app.rs ConfirmBatchTrash not invalidated after graft M
18 scanner/walk.rs Incomplete firmlink list (/usr/local etc.); partial refresh on them doubled disk total M
19 cleanup.rs find_candidates could classify the scan root; "Trash All" would trash the entire scan root M
20 app.rs zoom_stack fallback after cleanup batch-trash pushed dead root node M
21 app.rs Scan / partial-refresh errors went only to eprintln; status_message never set M
22 scanner/tree.rs Synthetic node sentinel __ collided with real files like foo.__bak M
23 app.rs Cleanup candidate size stale after per-item deletes; batch total overstated M
24 platform/trash.rs empty_trash left zombie; Finder failure silent M
25 app.rs perform_batch_delete never reset view_root/zoom_stack; dead view_root after cleanup trash H
26 app.rs Cmd+Backspace allowed ConfirmTrash on synthetic nodes (context menu was guarded, keyboard was not) L
27 ui/dir_tree.rs + ui/context_menu.rs Zoom stack push without duplicate guard (only treemap_view had the fix) L
28 app.rs perform_delete kept dead ancestors in zoom_stack L
29 platform/finder.rs + platform/fda.rs Spawned open/osascript children never waited → zombie per click L
30 treemap/squarify.rs partial_cmp().unwrap() panics on NaN size L
31 cleanup.rs tests Fixed temp-dir names → collision when two test processes run concurrently L
32 flywheel.rs tests set_var in test mutated env across parallel threads L
33 flywheel.rs Duplicate state_dir() (already pub(crate) in state.rs after cycle 1 dedup) L
34 scanner/walk.rs /home and /net listed as APFS firmlinks (they are autofs; lookups can stall) L
35 scanner/walk.rs Redundant should_skip inside loop body (same check already ran before dir_map insert) L
36 app.rs status bar mixed tree file_count (includes synthetic nodes) with stale scan_progress.dirs L
37 app.rs + context_menu.rs Synthetic nodes reached Open / Show in Finder / Get Info / Enter-reveal L
38 scanner/walk.rs Partial refresh on non-root mount point injected synthetic nodes into tree mid-branch L
39 app.rs perform_batch_delete cleared selection only for exact id match; dead child nodes stayed selected L
40 scanner/walk.rs find_closest_ancestor re-parented orphans; full_path() returned wrong path → Trash hit wrong file M
41 scanner/tree.rs StringArena as u32 cast silently wraps at 4 GiB → corrupt full_path; now panics loudly L
... ui/treemap_view.rs, ui/search.rs, flywheel.rs, state.rs, ui/cleanup_window.rs, ui/widgets.rs various L fixes L

Contested / skipped (architecture-level)

  • Scan thread cancellation (threading redesign)
  • Hard link / sparse file / APFS clone size correctness (platform complexity)
  • find_candidates on UI thread (background task infra)
  • graft_under memory reclaim
  • Per-frame dir_tree clone / squarify stack depth
  • full_path for entries found via find_closest_ancestor when parent is missing (follow-up: now orphans are dropped instead of re-parented — safer)

Tests

36 → 43 (+7): added tests for remove_node idempotency, graft_under, clear_descendants, recompute_sizes_upward, find_candidates, scan-root orphan guard, and test isolation fixes (temp dirs, env races).

Refactors (15)

  • Reuse platform::dialogs::pick_folder in toolbar
  • Deduplicate state_dir() in fda.rs and flywheel.rs
  • ext_list.rs: iter/enumerate over index loop
  • treemap/color.rs: lerp closure
  • treemap/mod.rs: dead gap > 0 guard removed
  • toolbar.rs: chained display call
  • ui/cleanup_window.rs: single pass for batch selection
  • ui/dir_tree.rs: frame built once
  • platform/finder.rs: spawn_reaped helper (was already fixed inline, now unified)
  • ui/widgets.rs: reuse fg for label color
  • ui/help_window.rs: reuse tip() helper
  • flywheel.rs: dropped unused PathBuf import (only compiler warning)
  • ui/treemap_view.rs: 3 simplifications (redundant guard, find loop, is_some_and)

🤖 Generated with Claude Code

Chartres and others added 30 commits September 22, 2026 00:58
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…anic

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…lorMode import

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
RevealInFinder and MoveToTrash were never built by any caller — the
context menu performs both inline. Removing them makes the dispatch
match exhaustive, so a future variant fails to compile instead of
being silently swallowed by the catch-all arm.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- cleanup_selected pruning: collect-to-Vec then remove-in-loop is
  HashSet::retain, one line instead of twelve
- Option::is_some_and replaces map_or(false, ..) in two predicates
- collapse the nested hover-detection if into one condition

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Delete / empty-trash / batch-delete each hand-built the same modal:
hidden title bar, center anchor, heading + detail label, right-aligned
danger+ghost pair. Extracted to ui::widgets::confirm_dialog returning
Option<bool> (None = undecided, Some = chose), next to the buttons it
uses. Also flattens the Option<Option<_>> result plumbing into plain
Option<NodeId> / Option<Vec<NodeId>> / bool.

Only visual change: the single-delete dialog now uses the same 320/360
width as the other two.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tart_scan

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…an Done branch

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ot-owned)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-uploaded files)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… parent

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…uping

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ror in status_message

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Caches, .gradle

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…TRY=1

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ally happened

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
remove_node and clear_descendants each had their own walk marking
descendants dead — one recursive with a per-directory children clone, one
iterative. Both now share the iterative version (no clone, no recursion
depth limit). Also drop a dead `let new_id = …; let _ = new_id;` in
graft_under.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Six copies of the same four-line f64-rect → egui::Rect conversion in
treemap_view collapse to a single helper.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ogressInfo

Cmd+1/2/3 were three copies of the same three-statement body; now one loop
over (key, mode). ScanProgressInfo's zeroed literal was written out twice —
derive Default and spread it.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Chartres and others added 30 commits October 4, 2026 01:06
…ompute_sizes_upward

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…on, dead-node skip)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…eption fires

/System/Volumes was listed in build_skip_paths, which caused the TCC-path
loop to skip everything under it — including the scan root — even when the
scan-root exception at line 124 had already decided not to skip.  Scanning
/System/Volumes/Data produced an empty tree as a result.

The existing should_skip() guard (path.starts_with("/System/Volumes") &&
!path.starts_with(scan_root)) is the right place to handle this; the
skip_paths entry was redundant and wrong.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…g it

find_candidates was classifying the scan root node itself — if the user
scanned ~/Library/Caches the root would match "Application Caches" and the
Trash button could delete the entire scanned directory.

Fix: iterate the root's direct children instead of calling walk() on the
root, so only descendants are candidates.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ce nodes

is_volume_root() was firing for /usr, /usr/local, /System/Library/Caches,
/System/Library/Assets, and /System/Library/AssetsV2 because they have a
different st_dev from / (they are firmlinks into the Data volume) but were
not listed in APFS_FIRMLINKS.  A partial refresh on any of them would add a
duplicate Free Space synthetic node and double the displayed disk total.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
After a delete, the zoom_stack is pruned of dead nodes and if empty the
scan root is pushed back as the fallback.  If the scan root itself was the
deleted node (possible before the cleanup.rs scan-root guard) the pushed ID
was dead, leaving the UI in an invalid state.

Guard both perform_delete and perform_batch_delete with is_alive(root).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Errors from both poll_scan and poll_partial_refresh were only printed to
stderr; the user saw nothing in the UI.  Add status_message assignments so
the error appears in the status bar.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
starts_with("__") was used to identify __free_space__ and __skipped__
synthetic nodes, which would falsely match real file extensions like __bak,
causing them to be excluded from file_count and extension statistics.

Switch to exact equality checks in both child_totals and
collect_extensions_recursive.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CleanupCandidate.size was set at scan time and never updated.  After
deleting individual files inside a candidate directory the tree node's size
is correct (remove_node propagates upward) but the cached value remained
stale, so the batch-delete total overstated the space to be freed.

Replace retain() with retain_mut() and refresh c.size from the live tree.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ot thread

The comment said "background thread" but the implementation spawns an OS
process (osascript) and returns immediately.  Clarify that Ok(()) means the
process started, not that the trash was emptied.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…of three filters

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… conditionally

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…l from synthetic nodes

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nt ends

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…nset

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…! wrapper

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…st_ancestor

Re-parenting to a distant ancestor produced wrong full_path() results,
which could cause Trash/cleanup to target the wrong file. Now entries
whose parent is absent from dir_map are silently dropped. Deletes
find_closest_ancestor entirely and adds a unit test for the guard.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The jwalk process_read_dir callback on line 180 already gates every
entry; the identical check inside the loop was dead code.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… wrap

Casting buf.len() as u32 silently wraps at 4 GiB, corrupting
full_path() results. try_from + expect panics immediately with a
clear message so the bug is visible rather than silent data corruption.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ore layout recompute

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… instead of found flag

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…nto is_some_and

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant