Skip to main content

max / audiofiles

UI A+: unified repaint guard, ContentsRef seal, similarity-view refresh - any_worker_busy(): single source of truth for the repaint-while-busy guard (was a 4-flag inline OR in draw_browser that had already missed workers). A new worker that emits a terminal event must be listed here, not copy-pasted. - ContentsRef newtype around Arc<Vec<VfsNodeWithAnalysis>>: the per-frame file-list clone is now provably a refcount bump (the only Clone is the Arc one), sealing the "one type-change from a 100k deep clone per frame" footgun. Deref exposes the slice so read sites are unchanged. - Similarity-view staleness: refresh_contents previously early-returned in similarity mode, so a delete/move/rename inside it showed stale rows until exit. It now re-runs the same query (similarity_is_near_dup persists the mode), so the fresh results replace the view. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-22 23:22 UTC
Commit: 40f56378c35ce51e464a7128997b4f656006c4e5
Parent: 9c017f2
5 files changed, +79 insertions, -23 deletions
M crates/audiofiles-browser/src/editor.rs +3 -9
@@ -25,15 +25,9 @@ pub fn draw_browser(
25 25 if state.poll_workers() {
26 26 ctx.request_repaint();
27 27 }
28 - // Keep repainting while any background worker runs: egui sleeps after input
29 - // settles, so without this a worker's completion event sits in the channel
30 - // unread (and a spinner-less overlay looks stuck) until the next mouse move.
31 - // Every recv-loop worker that reports a single terminal event needs this.
32 - if state.classifier.busy.is_some()
33 - || state.forge.busy
34 - || state.loose_files_busy
35 - || state.latest_similarity_request.is_some()
36 - {
28 + // Keep repainting while any background worker runs (see any_worker_busy: the
29 + // single source of truth, so a newly added worker can't be forgotten here).
30 + if state.any_worker_busy() {
37 31 ctx.request_repaint();
38 32 }
39 33
M crates/audiofiles-browser/src/state/import_workflow.rs +16
@@ -445,6 +445,22 @@ impl BrowserState {
445 445 };
446 446 }
447 447
448 + /// Whether any background worker that reports a single terminal event is in
449 + /// flight. The single source of truth for the repaint guard in `draw_browser`:
450 + /// egui sleeps after input settles, so while a worker runs the frame loop must
451 + /// keep repainting or its completion event sits unread (and a spinner-less
452 + /// overlay looks stuck) until the next mouse move.
453 + ///
454 + /// **Invariant:** every worker whose completion arrives as one terminal event
455 + /// must be listed here. A new worker that isn't is a repaint bug — the prior
456 + /// inline OR in `draw_browser` already missed workers exactly this way.
457 + pub fn any_worker_busy(&self) -> bool {
458 + self.classifier.busy.is_some()
459 + || self.forge.busy
460 + || self.loose_files_busy
461 + || self.latest_similarity_request.is_some()
462 + }
463 +
448 464 /// Poll all backend workers for events. Returns `true` if any events were processed.
449 465 /// Called each frame to drain import, analysis, and export progress.
450 466 pub fn poll_workers(&mut self) -> bool {
M crates/audiofiles-browser/src/state/library.rs +3 -2
@@ -252,7 +252,7 @@ impl BrowserState {
252 252 let nodes = self.backend.find_nodes_by_hashes(vfs_id, &hash_refs)
253 253 .unwrap_or_default();
254 254 let count = nodes.len();
255 - self.contents = Arc::new(nodes);
255 + self.contents = ContentsRef::new(nodes);
256 256 self.active_collection = Some(id);
257 257 self.current_dir = None; // a collection view has no parent ".." row
258 258 self.similarity_search_hash = None;
@@ -321,7 +321,7 @@ impl BrowserState {
321 321 .find_nodes_by_hashes(vfs_id, hashes)
322 322 .unwrap_or_default();
323 323 let count = nodes.len();
324 - self.contents = Arc::new(nodes);
324 + self.contents = ContentsRef::new(nodes);
325 325 // The results replace the folder view, so there is no parent ".." row;
326 326 // clear current_dir to keep the row-offset bookkeeping honest.
327 327 self.current_dir = None;
@@ -329,6 +329,7 @@ impl BrowserState {
329 329 // guard stops and a later search starts from a clean slate.
330 330 self.latest_similarity_request = None;
331 331 self.similarity_search_hash = Some(source_hash.to_string());
332 + self.similarity_is_near_dup = is_near_dup;
332 333 self.similarity_source_name = self.backend.sample_original_name(source_hash).ok();
333 334 self.selection.clear();
334 335 self.status = format!("Found {count} {noun}");
M crates/audiofiles-browser/src/state/mod.rs +38 -2
@@ -25,6 +25,37 @@ use audiofiles_core::{CollectionId, NodeId, VfsId};
25 25 pub use audiofiles_core::vfs::VfsNodeWithAnalysis;
26 26 use parking_lot::Mutex;
27 27
28 + /// A cheap-to-clone handle to the current folder's contents.
29 + ///
30 + /// Wraps `Arc<Vec<VfsNodeWithAnalysis>>` so the per-frame `clone()` in the file
31 + /// list is always a refcount bump. The footgun this seals: the audit flagged the
32 + /// field as "one type-change from a 100k deep clone per frame" — with this newtype
33 + /// the only `Clone` is the `Arc` one, so a deep clone per frame is uncompilable.
34 + /// Deref exposes the slice, so read sites (`get`/`iter`/`len`/`is_empty`/indexing)
35 + /// are unchanged.
36 + #[derive(Clone, Default)]
37 + pub struct ContentsRef(Arc<Vec<VfsNodeWithAnalysis>>);
38 +
39 + impl ContentsRef {
40 + /// Wrap a freshly built contents list.
41 + pub fn new(items: Vec<VfsNodeWithAnalysis>) -> Self {
42 + Self(Arc::new(items))
43 + }
44 +
45 + /// Mutable access to the backing Vec for in-place edits (e.g. sort), cloning
46 + /// the Arc only if it is currently shared. Mirrors `Arc::make_mut`.
47 + pub fn make_mut(&mut self) -> &mut Vec<VfsNodeWithAnalysis> {
48 + Arc::make_mut(&mut self.0)
49 + }
50 + }
51 +
52 + impl std::ops::Deref for ContentsRef {
53 + type Target = [VfsNodeWithAnalysis];
54 + fn deref(&self) -> &[VfsNodeWithAnalysis] {
55 + &self.0
56 + }
57 + }
58 +
28 59 use crate::backend::{Backend, DirectBackend, ImportStrategyDesc};
29 60
30 61 use crate::import::{ImportedFolder, ImportStrategy};
@@ -97,7 +128,7 @@ pub struct BrowserState {
97 128 pub current_vfs_idx: usize,
98 129 pub current_dir: Option<NodeId>,
99 130 pub breadcrumb: Vec<VfsNode>,
100 - pub contents: Arc<Vec<VfsNodeWithAnalysis>>,
131 + pub contents: ContentsRef,
101 132 pub selection: Selection,
102 133 pub selected_tags: Arc<Vec<String>>,
103 134 /// Per-tag provenance for the selected sample: tag -> (source, rule_id). A tag
@@ -147,6 +178,10 @@ pub struct BrowserState {
147 178 /// results that don't match it are stale (the user fired another search
148 179 /// first) and are dropped, so a slow query can't replace the current view.
149 180 pub latest_similarity_request: Option<(String, bool)>,
181 + /// Whether the active similarity view is a near-duplicate search (vs. a
182 + /// feature-similarity search). Persisted so `refresh_contents` can re-run the
183 + /// correct query when the view is mutated (delete/move) in similarity mode.
184 + pub similarity_is_near_dup: bool,
150 185 /// Display name of the source sample for the active similarity / duplicate
151 186 /// search. Cached so the breadcrumb can render "Similar to: <name>" without
152 187 /// a backend lookup on every frame.
@@ -486,7 +521,7 @@ impl BrowserState {
486 521 current_vfs_idx,
487 522 current_dir: None,
488 523 breadcrumb: Vec::new(),
489 - contents: Arc::new(contents),
524 + contents: ContentsRef::new(contents),
490 525 selection: Selection::new(),
491 526 selected_tags: Arc::new(Vec::new()),
492 527 selected_tag_sources: std::collections::HashMap::new(),
@@ -508,6 +543,7 @@ impl BrowserState {
508 543 filter_tag_input: String::new(),
509 544 similarity_search_hash: None,
510 545 latest_similarity_request: None,
546 + similarity_is_near_dup: false,
511 547 similarity_source_name: None,
512 548 all_tags: Arc::new(all_tags),
513 549 tag_search: String::new(),
M crates/audiofiles-browser/src/state/navigation.rs +19 -10
@@ -19,15 +19,24 @@ impl BrowserState {
19 19
20 20 /// Reload the child node list and apply current sort/search.
21 21 pub fn refresh_contents(&mut self) {
22 - // In similarity mode, contents are managed by find_similar() — skip normal refresh.
23 - if self.similarity_search_hash.is_some() {
22 + // In similarity mode the view is an async query result, not a folder
23 + // listing. A mutation inside it (delete/move/rename) would otherwise leave
24 + // stale rows until the user exits the view, so re-run the same query; the
25 + // fresh results replace the view via apply_similarity_results. (Previously
26 + // this early-returned and did nothing, which was the staleness bug.)
27 + if let Some(hash) = self.similarity_search_hash.clone() {
28 + if self.similarity_is_near_dup {
29 + self.find_near_duplicates(&hash);
30 + } else {
31 + self.find_similar(&hash);
32 + }
24 33 return;
25 34 }
26 35
27 36 let vfs_id = match self.current_vfs_id() {
28 37 Some(id) => id,
29 38 None => {
30 - self.contents = Arc::new(Vec::new());
39 + self.contents = ContentsRef::new(Vec::new());
31 40 return;
32 41 }
33 42 };
@@ -38,32 +47,32 @@ impl BrowserState {
38 47 match filter.scope {
39 48 audiofiles_core::search::SearchScope::CurrentFolder => {
40 49 match self.backend.search_in_folder(&filter, vfs_id, self.current_dir) {
41 - Ok(results) => self.contents = Arc::new(results),
50 + Ok(results) => self.contents = ContentsRef::new(results),
42 51 Err(e) => {
43 52 error!("Search failed: {e}");
44 53 self.status = "Search error".to_string();
45 - self.contents = Arc::new(Vec::new());
54 + self.contents = ContentsRef::new(Vec::new());
46 55 }
47 56 }
48 57 }
49 58 audiofiles_core::search::SearchScope::Global => {
50 59 match self.backend.search_global(&filter) {
51 - Ok(results) => self.contents = Arc::new(results),
60 + Ok(results) => self.contents = ContentsRef::new(results),
52 61 Err(e) => {
53 62 error!("Global search failed: {e}");
54 63 self.status = "Search error".to_string();
55 - self.contents = Arc::new(Vec::new());
64 + self.contents = ContentsRef::new(Vec::new());
56 65 }
57 66 }
58 67 }
59 68 }
60 69 } else {
61 70 match self.backend.list_children_enriched(vfs_id, self.current_dir) {
62 - Ok(nodes) => self.contents = Arc::new(nodes),
71 + Ok(nodes) => self.contents = ContentsRef::new(nodes),
63 72 Err(e) => {
64 73 error!("Failed to list directory: {e}");
65 74 self.status = "Failed to load contents".to_string();
66 - self.contents = Arc::new(Vec::new());
75 + self.contents = ContentsRef::new(Vec::new());
67 76 }
68 77 }
69 78 }
@@ -105,7 +114,7 @@ impl BrowserState {
105 114 /// Sort contents by the current sort column and direction.
106 115 pub fn sort_contents(&mut self) {
107 116 // Directories always first
108 - Arc::make_mut(&mut self.contents).sort_by(|a, b| {
117 + self.contents.make_mut().sort_by(|a, b| {
109 118 let a_is_dir = a.node.node_type == NodeType::Directory;
110 119 let b_is_dir = b.node.node_type == NodeType::Directory;
111 120 if a_is_dir != b_is_dir {