max / audiofiles
- Co-Authored-By
- Claude Opus 5 (1M context) <noreply@anthropic.com>
9 files changed,
+1057 insertions,
-2 deletions
| @@ -83,6 +83,13 @@ | |||
| 83 | 83 | exemplar::preview_sample(db, &hash, &index, k) | |
| 84 | 84 | .map(|outcome| ClassifierJobResult::Suggested { hash, outcome }) | |
| 85 | 85 | } | |
| 86 | + | ClassifierJob::SuggestLibrary { k } => { | |
| 87 | + | // Deliberately not routed through the trained head the way AutoApply | |
| 88 | + | // is. The head is a distillation of the exemplars and does not carry | |
| 89 | + | // the driving neighbours, and the queue is the surface where the user | |
| 90 | + | // decides; it should show what the layer itself says. | |
| 91 | + | exemplar::preview_library(db, k).map(ClassifierJobResult::LibrarySuggested) | |
| 92 | + | } | |
| 86 | 93 | }; | |
| 87 | 94 | match result { | |
| 88 | 95 | Ok(r) => ClassifierWorkerEvent::Done(r), | |
| @@ -188,6 +195,53 @@ | |||
| 188 | 195 | } | |
| 189 | 196 | } | |
| 190 | 197 | ||
| 198 | + | #[test] | |
| 199 | + | fn suggest_library_job_completes_and_writes_nothing() { | |
| 200 | + | let dir = tempfile::TempDir::new().unwrap(); | |
| 201 | + | let db_path = dir.path().join("audiofiles.db"); | |
| 202 | + | seed(&db_path); | |
| 203 | + | // One unlabelled sample sitting in the seeded kick cluster. | |
| 204 | + | { | |
| 205 | + | let db = Database::open(&db_path).unwrap(); | |
| 206 | + | db.conn() | |
| 207 | + | .execute( | |
| 208 | + | "INSERT INTO samples (hash, original_name, file_extension, file_size, import_date, last_modified) \ | |
| 209 | + | VALUES ('q', 'q', 'wav', 1, 0, 0)", | |
| 210 | + | [], | |
| 211 | + | ) | |
| 212 | + | .unwrap(); | |
| 213 | + | let v = serde_json::to_string(&vec![0.02; NUM_FEATURES]).unwrap(); | |
| 214 | + | db.conn() | |
| 215 | + | .execute( | |
| 216 | + | "INSERT INTO sample_features (hash, feat_version, vector, computed_at) VALUES ('q', ?1, ?2, 0)", | |
| 217 | + | rusqlite::params![FEATURE_VERSION, v], | |
| 218 | + | ) | |
| 219 | + | .unwrap(); | |
| 220 | + | } | |
| 221 | + | let before = tag_count(&db_path); | |
| 222 | + | ||
| 223 | + | let job = ClassifierJob::SuggestLibrary { k: 5 }; | |
| 224 | + | match run_to_completion(db_path.clone(), job) { | |
| 225 | + | ClassifierWorkerEvent::Done(ClassifierJobResult::LibrarySuggested(queue)) => { | |
| 226 | + | assert!(!queue.is_empty(), "q should be offered the kick tag"); | |
| 227 | + | assert_eq!(queue.samples_with_suggestions, 1); | |
| 228 | + | } | |
| 229 | + | _ => panic!("expected a library review queue"), | |
| 230 | + | } | |
| 231 | + | assert_eq!( | |
| 232 | + | tag_count(&db_path), | |
| 233 | + | before, | |
| 234 | + | "the review pass must not write a tag" | |
| 235 | + | ); | |
| 236 | + | } | |
| 237 | + | ||
| 238 | + | fn tag_count(db_path: &std::path::Path) -> i64 { | |
| 239 | + | let db = Database::open(db_path).unwrap(); | |
| 240 | + | db.conn() | |
| 241 | + | .query_row("SELECT COUNT(*) FROM tags", [], |r| r.get(0)) | |
| 242 | + | .unwrap() | |
| 243 | + | } | |
| 244 | + | ||
| 191 | 245 | #[test] | |
| 192 | 246 | fn export_job_writes_file_on_worker() { | |
| 193 | 247 | let dir = tempfile::TempDir::new().unwrap(); |
| @@ -203,6 +203,42 @@ | |||
| 203 | 203 | Database::open_in_memory().unwrap() | |
| 204 | 204 | } | |
| 205 | 205 | ||
| 206 | + | #[test] | |
| 207 | + | fn accepting_a_group_twice_adds_nothing_the_second_time() { | |
| 208 | + | // Re-running the review queue re-offers nothing already accepted, but the | |
| 209 | + | // gesture has to be safe even when it does. | |
| 210 | + | let db = setup(); | |
| 211 | + | insert_fake_sample(&db, "a"); | |
| 212 | + | insert_fake_sample(&db, "b"); | |
| 213 | + | assert_eq!( | |
| 214 | + | bulk_add_tag(&db, &["a", "b"], "instrument.drum.kick").unwrap(), | |
| 215 | + | 2 | |
| 216 | + | ); | |
| 217 | + | assert_eq!( | |
| 218 | + | bulk_add_tag(&db, &["a", "b"], "instrument.drum.kick").unwrap(), | |
| 219 | + | 0 | |
| 220 | + | ); | |
| 221 | + | assert_eq!(get_sample_tags(&db, "a").unwrap().len(), 1); | |
| 222 | + | } | |
| 223 | + | ||
| 224 | + | #[test] | |
| 225 | + | fn an_accepted_group_leaves_no_provenance_row() { | |
| 226 | + | // An accepted suggestion is the user's own tag. Were it written as | |
| 227 | + | // source='ml', a later remove_tags_by_source("ml") -- the "Undo | |
| 228 | + | // auto-tagging" button -- would silently delete tags the user explicitly | |
| 229 | + | // approved in the review queue. | |
| 230 | + | let db = setup(); | |
| 231 | + | insert_fake_sample(&db, "a"); | |
| 232 | + | bulk_add_tag(&db, &["a"], "instrument.bass").unwrap(); | |
| 233 | + | let provenance: i64 = db | |
| 234 | + | .conn() | |
| 235 | + | .query_row("SELECT COUNT(*) FROM tag_provenance", [], |r| r.get(0)) | |
| 236 | + | .unwrap(); | |
| 237 | + | assert_eq!(provenance, 0); | |
| 238 | + | assert_eq!(crate::rules::remove_tags_by_source(&db, "ml").unwrap(), 0); | |
| 239 | + | assert_eq!(get_sample_tags(&db, "a").unwrap().len(), 1); | |
| 240 | + | } | |
| 241 | + | ||
| 206 | 242 | #[test] | |
| 207 | 243 | fn discovery_queries_exclude_tombstoned_samples() { | |
| 208 | 244 | let db = setup(); |
| @@ -291,6 +291,9 @@ | |||
| 291 | 291 | /// O(library), so the detail-panel "Suggest tags" action runs it here rather | |
| 292 | 292 | /// than under the DB lock on the GUI thread. | |
| 293 | 293 | SuggestSample { hash: String, k: usize }, | |
| 294 | + | /// What the layer *would* suggest across the whole library, writing nothing. | |
| 295 | + | /// Feeds the review queue. The non-writing counterpart of [`Self::AutoApply`]. | |
| 296 | + | SuggestLibrary { k: usize }, | |
| 294 | 297 | } | |
| 295 | 298 | ||
| 296 | 299 | /// The result of a completed [`ClassifierJob`]. | |
| @@ -306,6 +309,8 @@ | |||
| 306 | 309 | hash: String, | |
| 307 | 310 | outcome: audiofiles_core::analysis::exemplar::MlOutcome, | |
| 308 | 311 | }, | |
| 312 | + | /// The review queue: every suggestion the layer would make, grouped by tag. | |
| 313 | + | LibrarySuggested(audiofiles_core::analysis::exemplar::LibrarySuggestions), | |
| 309 | 314 | } | |
| 310 | 315 | ||
| 311 | 316 | /// The core abstraction separating UI from data access. |
| @@ -7,7 +7,28 @@ | |||
| 7 | 7 | MatchMode, NewRule, Rule, RuleAction, RuleCondition, RuleField, RuleOp, | |
| 8 | 8 | }; | |
| 9 | 9 | ||
| 10 | - | use super::{BrowserState, RuleDraft}; | |
| 10 | + | use super::{BrowserState, ReviewCandidate, ReviewGroup, ReviewQueue, RuleDraft}; | |
| 11 | + | ||
| 12 | + | /// Which of a group's candidates an accept applies to. | |
| 13 | + | #[derive(Debug, Clone, Copy, PartialEq, Eq)] | |
| 14 | + | pub enum ReviewSelection { | |
| 15 | + | /// Every candidate in the group. | |
| 16 | + | All, | |
| 17 | + | /// Only those above the tag's auto threshold. | |
| 18 | + | Confident, | |
| 19 | + | /// Only those the user ticked. | |
| 20 | + | Checked, | |
| 21 | + | } | |
| 22 | + | ||
| 23 | + | impl ReviewSelection { | |
| 24 | + | fn matches(self, c: &ReviewCandidate) -> bool { | |
| 25 | + | match self { | |
| 26 | + | Self::All => true, | |
| 27 | + | Self::Confident => c.confident, | |
| 28 | + | Self::Checked => c.accepted, | |
| 29 | + | } | |
| 30 | + | } | |
| 31 | + | } | |
| 11 | 32 | ||
| 12 | 33 | impl BrowserState { | |
| 13 | 34 | /// Reload the cached rule list from the backend. | |
| @@ -219,6 +240,127 @@ | |||
| 219 | 240 | ); | |
| 220 | 241 | } | |
| 221 | 242 | ||
| 243 | + | /// Collect what the layer would suggest across the library, writing nothing, | |
| 244 | + | /// into the review queue. | |
| 245 | + | pub fn classifier_review_library(&mut self) { | |
| 246 | + | use audiofiles_core::analysis::exemplar::DEFAULT_K; | |
| 247 | + | self.classifier.last_review_accept = None; | |
| 248 | + | self.start_classifier_job( | |
| 249 | + | crate::backend::ClassifierJob::SuggestLibrary { k: DEFAULT_K }, | |
| 250 | + | "Collecting suggestions\u{2026}", | |
| 251 | + | ); | |
| 252 | + | } | |
| 253 | + | ||
| 254 | + | /// Resolve display names for the first `limit` candidates of one group, on | |
| 255 | + | /// first expand. | |
| 256 | + | /// | |
| 257 | + | /// One backend call per candidate, so it is scoped twice over: not for the | |
| 258 | + | /// whole queue (the collapsed view needs a count, not a name), and not for a | |
| 259 | + | /// whole group either (groups are complete, so one can hold tens of thousands | |
| 260 | + | /// while only a couple of screens are ever drawn). `limit` must match the | |
| 261 | + | /// UI's render window. | |
| 262 | + | pub fn ensure_review_names(&mut self, group_index: usize, limit: usize) { | |
| 263 | + | let Some(queue) = self.classifier.review.as_mut() else { | |
| 264 | + | return; | |
| 265 | + | }; | |
| 266 | + | let Some(group) = queue.groups.get_mut(group_index) else { | |
| 267 | + | return; | |
| 268 | + | }; | |
| 269 | + | if group.names_loaded { | |
| 270 | + | return; | |
| 271 | + | } | |
| 272 | + | let hashes: Vec<String> = group | |
| 273 | + | .candidates | |
| 274 | + | .iter() | |
| 275 | + | .take(limit) | |
| 276 | + | .map(|c| c.hash.clone()) | |
| 277 | + | .collect(); | |
| 278 | + | let names: Vec<Option<String>> = hashes | |
| 279 | + | .iter() | |
| 280 | + | .map(|h| self.backend.sample_original_name(h).ok()) | |
| 281 | + | .collect(); | |
| 282 | + | // Re-borrow: `self.backend` was needed above and the group borrow could | |
| 283 | + | // not be held across it. | |
| 284 | + | let Some(queue) = self.classifier.review.as_mut() else { | |
| 285 | + | return; | |
| 286 | + | }; | |
| 287 | + | let Some(group) = queue.groups.get_mut(group_index) else { | |
| 288 | + | return; | |
| 289 | + | }; | |
| 290 | + | for (candidate, name) in group.candidates.iter_mut().zip(names) { | |
| 291 | + | candidate.name = name; | |
| 292 | + | } | |
| 293 | + | group.names_loaded = true; | |
| 294 | + | } | |
| 295 | + | ||
| 296 | + | /// Accept some of a group's candidates, writing the tag to each. | |
| 297 | + | /// | |
| 298 | + | /// The accepted tag is written as the user's own (`bulk_add_tag`, no | |
| 299 | + | /// provenance row), not as `source = 'ml'`. That matters: "Undo auto-tagging" | |
| 300 | + | /// is `remove_tags_by_source("ml")`, and it must not reach into tags the user | |
| 301 | + | /// approved by hand here. | |
| 302 | + | /// | |
| 303 | + | /// Accepted candidates leave the group, so the queue shrinks as it is worked | |
| 304 | + | /// and a second pass over the same screen cannot double-accept. | |
| 305 | + | pub fn accept_review(&mut self, group_index: usize, which: ReviewSelection) { | |
| 306 | + | let Some(queue) = self.classifier.review.as_ref() else { | |
| 307 | + | return; | |
| 308 | + | }; | |
| 309 | + | let Some(group) = queue.groups.get(group_index) else { | |
| 310 | + | return; | |
| 311 | + | }; | |
| 312 | + | let tag = group.tag.clone(); | |
| 313 | + | let hashes: Vec<String> = group | |
| 314 | + | .candidates | |
| 315 | + | .iter() | |
| 316 | + | .filter(|c| which.matches(c)) | |
| 317 | + | .map(|c| c.hash.clone()) | |
| 318 | + | .collect(); | |
| 319 | + | if hashes.is_empty() { | |
| 320 | + | return; | |
| 321 | + | } | |
| 322 | + | ||
| 323 | + | let refs: Vec<&str> = hashes.iter().map(String::as_str).collect(); | |
| 324 | + | let n = match self.backend.bulk_add_tag(&refs, &tag) { | |
| 325 | + | Ok(n) => n, | |
| 326 | + | Err(e) => { | |
| 327 | + | self.classifier.last_error = Some(format!("Could not accept: {e}")); | |
| 328 | + | self.status = format!("Could not accept: {e}"); | |
| 329 | + | return; | |
| 330 | + | } | |
| 331 | + | }; | |
| 332 | + | ||
| 333 | + | let accepted: std::collections::HashSet<String> = hashes.into_iter().collect(); | |
| 334 | + | if let Some(queue) = self.classifier.review.as_mut() | |
| 335 | + | && let Some(group) = queue.groups.get_mut(group_index) | |
| 336 | + | { | |
| 337 | + | group.candidates.retain(|c| !accepted.contains(&c.hash)); | |
| 338 | + | } | |
| 339 | + | self.classifier.last_review_accept = Some(format!("Accepted {n} \u{00b7} {tag}")); | |
| 340 | + | self.status = format!("Accepted {n} tag{}.", if n == 1 { "" } else { "s" }); | |
| 341 | + | self.refresh_contents(); | |
| 342 | + | self.refresh_selected_tags(); | |
| 343 | + | } | |
| 344 | + | ||
| 345 | + | /// Discard a whole group without accepting any of it. | |
| 346 | + | /// | |
| 347 | + | /// Nothing is persisted: the layer is free to offer the same tag again on the | |
| 348 | + | /// next pass. A durable "never suggest this" belongs with the per-tag policy, | |
| 349 | + | /// not with a dismissal list (the previous dismissed-suggestions subsystem was | |
| 350 | + | /// removed for exactly that reason when the class column went). | |
| 351 | + | pub fn dismiss_review_group(&mut self, group_index: usize) { | |
| 352 | + | if let Some(queue) = self.classifier.review.as_mut() | |
| 353 | + | && group_index < queue.groups.len() | |
| 354 | + | { | |
| 355 | + | let group = queue.groups.remove(group_index); | |
| 356 | + | self.classifier.last_review_accept = Some(format!( | |
| 357 | + | "Dismissed {} \u{00b7} {}", | |
| 358 | + | group.candidates.len(), | |
| 359 | + | group.tag | |
| 360 | + | )); | |
| 361 | + | } | |
| 362 | + | } | |
| 363 | + | ||
| 222 | 364 | /// Apply a finished classifier job's result (called from `poll_workers`). The worker | |
| 223 | 365 | /// wrote to its own DB connection, so refresh the affected cached views here. | |
| 224 | 366 | pub fn on_classifier_job_done(&mut self, result: crate::backend::ClassifierJobResult) { | |
| @@ -289,6 +431,57 @@ | |||
| 289 | 431 | }; | |
| 290 | 432 | self.detail.selected_ml_suggestions = all; | |
| 291 | 433 | } | |
| 434 | + | R::LibrarySuggested(suggestions) => { | |
| 435 | + | let total = suggestions.len(); | |
| 436 | + | self.classifier.review = Some(ReviewQueue { | |
| 437 | + | groups: suggestions | |
| 438 | + | .groups | |
| 439 | + | .into_iter() | |
| 440 | + | .map(|g| ReviewGroup { | |
| 441 | + | tag: g.tag, | |
| 442 | + | candidates: g | |
| 443 | + | .samples | |
| 444 | + | .into_iter() | |
| 445 | + | .map(|s| ReviewCandidate { | |
| 446 | + | hash: s.hash, | |
| 447 | + | name: None, | |
| 448 | + | score: s.score, | |
| 449 | + | confident: s.action | |
| 450 | + | == audiofiles_core::analysis::exemplar::MlAction::AutoApply, | |
| 451 | + | accepted: false, | |
| 452 | + | }) | |
| 453 | + | .collect(), | |
| 454 | + | expanded: false, | |
| 455 | + | names_loaded: false, | |
| 456 | + | }) | |
| 457 | + | .collect(), | |
| 458 | + | samples_considered: suggestions.samples_considered, | |
| 459 | + | samples_with_suggestions: suggestions.samples_with_suggestions, | |
| 460 | + | }); | |
| 461 | + | self.status = if total == 0 { | |
| 462 | + | "No suggestions. Tag a few more samples and try again.".to_string() | |
| 463 | + | } else { | |
| 464 | + | format!( | |
| 465 | + | "{total} suggestion{} across {} tag{}.", | |
| 466 | + | if total == 1 { "" } else { "s" }, | |
| 467 | + | self.classifier | |
| 468 | + | .review | |
| 469 | + | .as_ref() | |
| 470 | + | .map_or(0, |q| q.groups.len()), | |
| 471 | + | if self | |
| 472 | + | .classifier | |
| 473 | + | .review | |
| 474 | + | .as_ref() | |
| 475 | + | .map_or(0, |q| q.groups.len()) | |
| 476 | + | == 1 | |
| 477 | + | { | |
| 478 | + | "" | |
| 479 | + | } else { | |
| 480 | + | "s" | |
| 481 | + | } | |
| 482 | + | ) | |
| 483 | + | }; | |
| 484 | + | } | |
| 292 | 485 | } | |
| 293 | 486 | } | |
| 294 | 487 |
| @@ -75,6 +75,7 @@ | |||
| 75 | 75 | mod analysis_workflow; | |
| 76 | 76 | mod bulk_ops; | |
| 77 | 77 | mod classifier; | |
| 78 | + | pub use classifier::ReviewSelection; | |
| 78 | 79 | mod edit_workflow; | |
| 79 | 80 | mod export_workflow; | |
| 80 | 81 | mod forge; |
| @@ -3375,3 +3375,169 @@ | |||
| 3375 | 3375 | ); | |
| 3376 | 3376 | } | |
| 3377 | 3377 | } | |
| 3378 | + | ||
| 3379 | + | /// The library review queue: what the suggest-only path does when the user acts. | |
| 3380 | + | mod review_queue { | |
| 3381 | + | use super::*; | |
| 3382 | + | use crate::state::ReviewSelection; | |
| 3383 | + | ||
| 3384 | + | /// A queue with one group of three, the first two confident. | |
| 3385 | + | fn seeded(state: &mut BrowserState) { | |
| 3386 | + | for h in ["a", "b", "c"] { | |
| 3387 | + | insert_fake_sample(state, h); | |
| 3388 | + | } | |
| 3389 | + | state.classifier.review = Some(ReviewQueue { | |
| 3390 | + | groups: vec![ReviewGroup { | |
| 3391 | + | tag: "instrument.drum.kick".to_string(), | |
| 3392 | + | candidates: vec![ | |
| 3393 | + | ReviewCandidate { | |
| 3394 | + | hash: "a".into(), | |
| 3395 | + | name: None, | |
| 3396 | + | score: 0.95, | |
| 3397 | + | confident: true, | |
| 3398 | + | accepted: false, | |
| 3399 | + | }, | |
| 3400 | + | ReviewCandidate { | |
| 3401 | + | hash: "b".into(), | |
| 3402 | + | name: None, | |
| 3403 | + | score: 0.90, | |
| 3404 | + | confident: true, | |
| 3405 | + | accepted: false, | |
| 3406 | + | }, | |
| 3407 | + | ReviewCandidate { | |
| 3408 | + | hash: "c".into(), | |
| 3409 | + | name: None, | |
| 3410 | + | score: 0.60, | |
| 3411 | + | confident: false, | |
| 3412 | + | accepted: false, | |
| 3413 | + | }, | |
| 3414 | + | ], | |
| 3415 | + | expanded: false, | |
| 3416 | + | names_loaded: false, | |
| 3417 | + | }], | |
| 3418 | + | samples_considered: 10, | |
| 3419 | + | samples_with_suggestions: 3, | |
| 3420 | + | }); | |
| 3421 | + | } | |
| 3422 | + | ||
| 3423 | + | fn tags_of(state: &BrowserState, hash: &str) -> Vec<String> { | |
| 3424 | + | state.backend.get_sample_tags(hash).unwrap() | |
| 3425 | + | } | |
| 3426 | + | ||
| 3427 | + | #[test] | |
| 3428 | + | fn accepting_a_group_tags_every_member() { | |
| 3429 | + | let (mut state, _dir) = make_state(); | |
| 3430 | + | seeded(&mut state); | |
| 3431 | + | state.accept_review(0, ReviewSelection::All); | |
| 3432 | + | ||
| 3433 | + | for h in ["a", "b", "c"] { | |
| 3434 | + | assert_eq!(tags_of(&state, h), vec!["instrument.drum.kick".to_string()]); | |
| 3435 | + | } | |
| 3436 | + | } | |
| 3437 | + | ||
| 3438 | + | #[test] | |
| 3439 | + | fn accepting_the_confident_band_leaves_the_rest_queued() { | |
| 3440 | + | let (mut state, _dir) = make_state(); | |
| 3441 | + | seeded(&mut state); | |
| 3442 | + | state.accept_review(0, ReviewSelection::Confident); | |
| 3443 | + | ||
| 3444 | + | assert_eq!(tags_of(&state, "a").len(), 1); | |
| 3445 | + | assert_eq!(tags_of(&state, "b").len(), 1); | |
| 3446 | + | assert!(tags_of(&state, "c").is_empty(), "below the auto threshold"); | |
| 3447 | + | ||
| 3448 | + | // And the accepted two are gone from the queue, so a second click of the | |
| 3449 | + | // same button cannot re-accept them. | |
| 3450 | + | let group = &state.classifier.review.as_ref().unwrap().groups[0]; | |
| 3451 | + | assert_eq!(group.candidates.len(), 1); | |
| 3452 | + | assert_eq!(group.candidates[0].hash, "c"); | |
| 3453 | + | } | |
| 3454 | + | ||
| 3455 | + | #[test] | |
| 3456 | + | fn accepting_checked_takes_only_the_ticked_ones() { | |
| 3457 | + | let (mut state, _dir) = make_state(); | |
| 3458 | + | seeded(&mut state); | |
| 3459 | + | state.classifier.review.as_mut().unwrap().groups[0].candidates[2].accepted = true; | |
| 3460 | + | state.accept_review(0, ReviewSelection::Checked); | |
| 3461 | + | ||
| 3462 | + | assert!(tags_of(&state, "a").is_empty()); | |
| 3463 | + | assert!(tags_of(&state, "b").is_empty()); | |
| 3464 | + | assert_eq!(tags_of(&state, "c").len(), 1); | |
| 3465 | + | } | |
| 3466 | + | ||
| 3467 | + | #[test] | |
| 3468 | + | fn an_accepted_tag_is_the_users_own_not_ml_sourced() { | |
| 3469 | + | // The load-bearing one. "Undo auto-tagging" is remove_tags_by_source("ml"), | |
| 3470 | + | // and it must not reach into what the user approved in the queue. | |
| 3471 | + | let (mut state, _dir) = make_state(); | |
| 3472 | + | seeded(&mut state); | |
| 3473 | + | state.accept_review(0, ReviewSelection::All); | |
| 3474 | + | ||
| 3475 | + | let removed = state.backend.remove_tags_by_source("ml").unwrap(); | |
| 3476 | + | assert_eq!(removed, 0); | |
| 3477 | + | for h in ["a", "b", "c"] { | |
| 3478 | + | assert_eq!( | |
| 3479 | + | tags_of(&state, h).len(), | |
| 3480 | + | 1, | |
| 3481 | + | "undoing auto-tagging deleted an accepted tag" | |
| 3482 | + | ); | |
| 3483 | + | } | |
| 3484 | + | } | |
| 3485 | + | ||
| 3486 | + | #[test] | |
| 3487 | + | fn accepting_nothing_is_a_no_op() { | |
| 3488 | + | let (mut state, _dir) = make_state(); | |
| 3489 | + | seeded(&mut state); | |
| 3490 | + | // Nothing ticked. | |
| 3491 | + | state.accept_review(0, ReviewSelection::Checked); | |
| 3492 | + | ||
| 3493 | + | assert!(tags_of(&state, "a").is_empty()); | |
| 3494 | + | let group = &state.classifier.review.as_ref().unwrap().groups[0]; | |
| 3495 | + | assert_eq!(group.candidates.len(), 3, "the queue is untouched"); | |
| 3496 | + | assert!(state.classifier.last_review_accept.is_none()); | |
| 3497 | + | } | |
| 3498 | + | ||
| 3499 | + | #[test] | |
| 3500 | + | fn dismissing_a_group_writes_nothing() { | |
| 3501 | + | let (mut state, _dir) = make_state(); | |
| 3502 | + | seeded(&mut state); | |
| 3503 | + | state.dismiss_review_group(0); | |
| 3504 | + | ||
| 3505 | + | assert!(state.classifier.review.as_ref().unwrap().groups.is_empty()); | |
| 3506 | + | for h in ["a", "b", "c"] { | |
| 3507 | + | assert!(tags_of(&state, h).is_empty()); | |
| 3508 | + | } | |
| 3509 | + | } | |
| 3510 | + | ||
| 3511 | + | #[test] | |
| 3512 | + | fn acting_on_a_group_that_is_gone_does_not_panic() { | |
| 3513 | + | // The indices come from a UI loop over the previous frame's queue, so a | |
| 3514 | + | // stale one is reachable rather than theoretical. | |
| 3515 | + | let (mut state, _dir) = make_state(); | |
| 3516 | + | seeded(&mut state); | |
| 3517 | + | state.accept_review(9, ReviewSelection::All); | |
| 3518 | + | state.dismiss_review_group(9); | |
| 3519 | + | state.ensure_review_names(9, 200); | |
| 3520 | + | assert_eq!( | |
| 3521 | + | state.classifier.review.as_ref().unwrap().groups.len(), | |
| 3522 | + | 1, | |
| 3523 | + | "the real group survived" | |
| 3524 | + | ); | |
| 3525 | + | } | |
| 3526 | + | ||
| 3527 | + | #[test] | |
| 3528 | + | fn names_resolve_on_expand() { | |
| 3529 | + | let (mut state, _dir) = make_state(); | |
| 3530 | + | seeded(&mut state); | |
| 3531 | + | assert!( | |
| 3532 | + | state.classifier.review.as_ref().unwrap().groups[0].candidates[0] | |
| 3533 | + | .name | |
| 3534 | + | .is_none() | |
| 3535 | + | ); | |
| 3536 | + | ||
| 3537 | + | state.ensure_review_names(0, 200); | |
| 3538 | + | ||
| 3539 | + | let group = &state.classifier.review.as_ref().unwrap().groups[0]; | |
| 3540 | + | assert!(group.names_loaded); | |
| 3541 | + | assert_eq!(group.candidates[0].name.as_deref(), Some("a.wav")); | |
| 3542 | + | } | |
| 3543 | + | } |
| @@ -333,6 +333,12 @@ | |||
| 333 | 333 | /// Tag input for adding a new per-tag policy. | |
| 334 | 334 | pub new_policy_tag: String, | |
| 335 | 335 | ||
| 336 | + | // --- Review queue (Layer B, suggest-only) --- | |
| 337 | + | /// The last library-wide review pass, grouped by tag. `None` = never run. | |
| 338 | + | pub review: Option<ReviewQueue>, | |
| 339 | + | /// Result line for the last accept, shown in-section. | |
| 340 | + | pub last_review_accept: Option<String>, | |
| 341 | + | ||
| 336 | 342 | // --- Optional trained head (Layer B speed-up) --- | |
| 337 | 343 | /// Summary of the persisted trained head, if any. `None` = no/stale head. | |
| 338 | 344 | pub head_info: Option<crate::backend::TrainedHeadInfo>, | |
| @@ -1108,6 +1114,58 @@ | |||
| 1108 | 1114 | pub accepted: bool, | |
| 1109 | 1115 | } | |
| 1110 | 1116 | ||
| 1117 | + | // --- Library review queue --- | |
| 1118 | + | ||
| 1119 | + | /// The last library-wide review pass, as the UI holds it. | |
| 1120 | + | /// | |
| 1121 | + | /// Mirrors `exemplar::LibrarySuggestions` with the per-group display state the | |
| 1122 | + | /// core type has no business carrying (expansion, resolved names, checkboxes). | |
| 1123 | + | pub struct ReviewQueue { | |
| 1124 | + | pub groups: Vec<ReviewGroup>, | |
| 1125 | + | pub samples_considered: usize, | |
| 1126 | + | pub samples_with_suggestions: usize, | |
| 1127 | + | } | |
| 1128 | + | ||
| 1129 | + | /// Every sample the layer would give one tag, best score first. | |
| 1130 | + | pub struct ReviewGroup { | |
| 1131 | + | pub tag: String, | |
| 1132 | + | /// Every candidate, not a page of them. Accepting the group has to mean the | |
| 1133 | + | /// whole group; only rendering is windowed. | |
| 1134 | + | pub candidates: Vec<ReviewCandidate>, | |
| 1135 | + | pub expanded: bool, | |
| 1136 | + | /// Display names are resolved on expand, for the rendered window only. One | |
| 1137 | + | /// backend call per candidate, so resolving a 44,000-row group to draw 200 of | |
| 1138 | + | /// them would freeze the frame. | |
| 1139 | + | pub names_loaded: bool, | |
| 1140 | + | } | |
| 1141 | + | ||
| 1142 | + | impl ReviewGroup { | |
| 1143 | + | /// Candidates that cleared the tag's auto threshold. | |
| 1144 | + | pub fn confident(&self) -> usize { | |
| 1145 | + | self.candidates.iter().filter(|c| c.confident).count() | |
| 1146 | + | } | |
| 1147 | + | ||
| 1148 | + | pub fn checked(&self) -> usize { | |
| 1149 | + | self.candidates.iter().filter(|c| c.accepted).count() | |
| 1150 | + | } | |
| 1151 | + | } | |
| 1152 | + | ||
| 1153 | + | /// One sample's candidacy for a tag. | |
| 1154 | + | pub struct ReviewCandidate { | |
| 1155 | + | pub hash: String, | |
| 1156 | + | /// Resolved on expand; the hash stands in until then. | |
| 1157 | + | pub name: Option<String>, | |
| 1158 | + | pub score: f64, | |
| 1159 | + | /// Cleared the tag's auto threshold. Bands the list so "accept the confident | |
| 1160 | + | /// ones" is one gesture. Nothing is applied on either side of the band until | |
| 1161 | + | /// the user says so. | |
| 1162 | + | pub confident: bool, | |
| 1163 | + | /// Ticked for the "accept checked" path. Deliberately defaults to false: | |
| 1164 | + | /// a screen arriving with 340 boxes pre-ticked is auto-apply wearing a | |
| 1165 | + | /// checkbox, which is the thing this queue exists to avoid. | |
| 1166 | + | pub accepted: bool, | |
| 1167 | + | } | |
| 1168 | + | ||
| 1111 | 1169 | // --- Sort --- | |
| 1112 | 1170 | ||
| 1113 | 1171 | #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] |
| @@ -8,7 +8,7 @@ | |||
| 8 | 8 | ||
| 9 | 9 | use super::theme; | |
| 10 | 10 | use super::widgets; | |
| 11 | - | use crate::state::BrowserState; | |
| 11 | + | use crate::state::{BrowserState, ReviewSelection}; | |
| 12 | 12 | ||
| 13 | 13 | /// Fields offered in the condition builder (path/name + tags first, then DSP). | |
| 14 | 14 | const FIELDS: &[RuleField] = &[ | |
| @@ -560,6 +560,9 @@ | |||
| 560 | 560 | ); | |
| 561 | 561 | } | |
| 562 | 562 | ||
| 563 | + | ui.add_space(theme::space::peer()); | |
| 564 | + | draw_review_queue(ui, state); | |
| 565 | + | ||
| 563 | 566 | ui.add_space(theme::space::peer()); | |
| 564 | 567 | draw_trained_head(ui, state); | |
| 565 | 568 | ||
| @@ -625,6 +628,229 @@ | |||
| 625 | 628 | }); | |
| 626 | 629 | } | |
| 627 | 630 | ||
| 631 | + | /// Rows drawn per expanded group. | |
| 632 | + | /// | |
| 633 | + | /// A drawing and name-resolution budget, not a cap on the group: every button | |
| 634 | + | /// acts on all of it. Names cost one backend call each, so resolving a | |
| 635 | + | /// 44,000-row group to draw a couple of screens would stall the frame. | |
| 636 | + | const REVIEW_RENDER_ROWS: usize = 200; | |
| 637 | + | ||
| 638 | + | /// "Review suggestions" subsection: what the layer would tag, grouped by tag, | |
| 639 | + | /// accepted in bulk. | |
| 640 | + | /// | |
| 641 | + | /// Grouped by tag rather than by sample on purpose, and it is the whole reason | |
| 642 | + | /// the screen works. "Are these 340 all kicks?" is a question a scan of one list | |
| 643 | + | /// answers; the same 340 rows one sample at a time is 340 questions nobody | |
| 644 | + | /// finishes, which pushes the user to an undifferentiated accept-all and turns | |
| 645 | + | /// the queue back into auto-apply with extra steps. | |
| 646 | + | fn draw_review_queue(ui: &mut egui::Ui, state: &mut BrowserState) { | |
| 647 | + | widgets::subsection_label(ui, "Review suggestions"); | |
| 648 | + | ui.label( | |
| 649 | + | egui::RichText::new( | |
| 650 | + | "Collects what auto-tagging would apply, without applying any of it. \ | |
| 651 | + | Accept a whole group at once, or pick from it.", | |
| 652 | + | ) | |
| 653 | + | .small() | |
| 654 | + | .color(theme::content_muted()), | |
| 655 | + | ); | |
| 656 | + | ui.add_space(theme::space::bound()); | |
| 657 | + | ||
| 658 | + | let busy = state.classifier.busy.is_some(); | |
| 659 | + | ui.horizontal(|ui| { | |
| 660 | + | if ui | |
| 661 | + | .add_enabled(!busy, egui::Button::new("Review suggestions")) | |
| 662 | + | .on_hover_text("Find suggestions across the library. Writes nothing.") | |
| 663 | + | .clicked() | |
| 664 | + | { | |
| 665 | + | state.classifier_review_library(); | |
| 666 | + | } | |
| 667 | + | if state.classifier.review.is_some() | |
| 668 | + | && ui.add_enabled(!busy, egui::Button::new("Clear")).clicked() | |
| 669 | + | { | |
| 670 | + | state.classifier.review = None; | |
| 671 | + | state.classifier.last_review_accept = None; | |
| 672 | + | } | |
| 673 | + | }); | |
| 674 | + | ||
| 675 | + | if let Some(msg) = &state.classifier.last_review_accept { | |
| 676 | + | ui.label( | |
| 677 | + | egui::RichText::new(msg) | |
| 678 | + | .small() | |
| 679 | + | .color(theme::content_muted()), | |
| 680 | + | ); | |
| 681 | + | } | |
| 682 | + | ||
| 683 | + | let Some(queue) = &state.classifier.review else { | |
| 684 | + | return; | |
| 685 | + | }; | |
| 686 | + | ||
| 687 | + | ui.add_space(theme::space::bound()); | |
| 688 | + | ui.label( | |
| 689 | + | egui::RichText::new(format!( | |
| 690 | + | "{} of {} sample{} have suggestions.", | |
| 691 | + | queue.samples_with_suggestions, | |
| 692 | + | queue.samples_considered, | |
| 693 | + | if queue.samples_considered == 1 { | |
| 694 | + | "" | |
| 695 | + | } else { | |
| 696 | + | "s" | |
| 697 | + | } | |
| 698 | + | )) | |
| 699 | + | .small() | |
| 700 | + | .color(theme::content_muted()), | |
| 701 | + | ); | |
| 702 | + | if queue.groups.is_empty() { | |
| 703 | + | ui.label( | |
| 704 | + | egui::RichText::new( | |
| 705 | + | "Nothing to review. Tag a few more samples so there is something to match against.", | |
| 706 | + | ) | |
| 707 | + | .small() | |
| 708 | + | .color(theme::content_muted()), | |
| 709 | + | ); | |
| 710 | + | return; | |
| 711 | + | } | |
| 712 | + | ||
| 713 | + | // Deferred so the loop can borrow `state.classifier.review` immutably while | |
| 714 | + | // the actions below need it mutably. | |
| 715 | + | let mut expand: Option<usize> = None; | |
| 716 | + | let mut accept: Option<(usize, ReviewSelection)> = None; | |
| 717 | + | let mut dismiss: Option<usize> = None; | |
| 718 | + | let mut toggle: Option<(usize, usize)> = None; | |
| 719 | + | ||
| 720 | + | for (gi, group) in queue.groups.iter().enumerate() { | |
| 721 | + | ui.add_space(theme::space::bound()); | |
| 722 | + | widgets::inset_well(ui, |ui| { | |
| 723 | + | ui.horizontal(|ui| { | |
| 724 | + | let arrow = if group.expanded { | |
| 725 | + | "\u{25be}" | |
| 726 | + | } else { | |
| 727 | + | "\u{25b8}" | |
| 728 | + | }; | |
| 729 | + | if ui | |
| 730 | + | .small_button(format!("{arrow} {}", group.tag)) | |
| 731 | + | .on_hover_text("Show the samples in this group") | |
| 732 | + | .clicked() | |
| 733 | + | { | |
| 734 | + | expand = Some(gi); | |
| 735 | + | } | |
| 736 | + | ui.label( | |
| 737 | + | egui::RichText::new(format!("{}", group.candidates.len())) | |
| 738 | + | .small() | |
| 739 | + | .color(theme::content_muted()), | |
| 740 | + | ); | |
| 741 | + | let confident = group.confident(); | |
| 742 | + | if confident > 0 { | |
| 743 | + | ui.label( | |
| 744 | + | egui::RichText::new(format!("{confident} confident")) | |
| 745 | + | .small() | |
| 746 | + | .color(theme::content_muted()), | |
| 747 | + | ); | |
| 748 | + | } | |
| 749 | + | }); | |
| 750 | + | ui.horizontal(|ui| { | |
| 751 | + | if ui | |
| 752 | + | .small_button(format!("Accept all {}", group.candidates.len())) | |
| 753 | + | .clicked() | |
| 754 | + | { | |
| 755 | + | accept = Some((gi, ReviewSelection::All)); | |
| 756 | + | } | |
| 757 | + | let confident = group.confident(); | |
| 758 | + | if confident > 0 | |
| 759 | + | && confident < group.candidates.len() | |
| 760 | + | && ui | |
| 761 | + | .small_button(format!("Accept {confident} confident")) | |
| 762 | + | .on_hover_text("Only those above this tag's auto threshold") | |
| 763 | + | .clicked() | |
| 764 | + | { | |
| 765 | + | accept = Some((gi, ReviewSelection::Confident)); | |
| 766 | + | } | |
| 767 | + | let checked = group.checked(); | |
| 768 | + | if checked > 0 | |
| 769 | + | && ui | |
| 770 | + | .small_button(format!("Accept {checked} checked")) | |
| 771 | + | .clicked() | |
| 772 | + | { | |
| 773 | + | accept = Some((gi, ReviewSelection::Checked)); | |
| 774 | + | } | |
| 775 | + | if widgets::danger_small_button(ui, "Dismiss").clicked() { | |
| 776 | + | dismiss = Some(gi); | |
| 777 | + | } | |
| 778 | + | }); | |
| 779 | + | ||
| 780 | + | if !group.expanded { | |
| 781 | + | return; | |
| 782 | + | } | |
| 783 | + | ui.add_space(theme::space::bound()); | |
| 784 | + | if group.candidates.len() > REVIEW_RENDER_ROWS { | |
| 785 | + | // The window is a drawing budget, not a limit on the answer: the | |
| 786 | + | // buttons above still act on the whole group. Said out loud so the | |
| 787 | + | // list does not read as the complete set. | |
| 788 | + | ui.label( | |
| 789 | + | egui::RichText::new(format!( | |
| 790 | + | "Showing the {REVIEW_RENDER_ROWS} strongest of {}. The buttons above \ | |
| 791 | + | still apply to all {}.", | |
| 792 | + | group.candidates.len(), | |
| 793 | + | group.candidates.len() | |
| 794 | + | )) | |
| 795 | + | .small() | |
| 796 | + | .color(theme::content_muted()), | |
| 797 | + | ); | |
| 798 | + | } | |
| 799 | + | egui::ScrollArea::vertical() | |
| 800 | + | .max_height(220.0) | |
| 801 | + | .id_salt(("review-group", gi)) | |
| 802 | + | .show(ui, |ui| { | |
| 803 | + | for (ci, c) in group.candidates.iter().take(REVIEW_RENDER_ROWS).enumerate() { | |
| 804 | + | ui.horizontal(|ui| { | |
| 805 | + | let mut checked = c.accepted; | |
| 806 | + | if ui.checkbox(&mut checked, "").changed() { | |
| 807 | + | toggle = Some((gi, ci)); | |
| 808 | + | } | |
| 809 | + | ui.label( | |
| 810 | + | egui::RichText::new(c.name.as_deref().unwrap_or(&c.hash)).small(), | |
| 811 | + | ); | |
| 812 | + | ui.with_layout( | |
| 813 | + | egui::Layout::right_to_left(egui::Align::Center), | |
| 814 | + | |ui| { | |
| 815 | + | ui.label( | |
| 816 | + | egui::RichText::new(format!("{:.0}%", c.score * 100.0)) | |
| 817 | + | .small() | |
| 818 | + | .color(if c.confident { | |
| 819 | + | theme::content() | |
| 820 | + | } else { | |
| 821 | + | theme::content_muted() | |
| 822 | + | }), | |
| 823 | + | ); | |
| 824 | + | }, | |
| 825 | + | ); | |
| 826 | + | }); | |
| 827 | + | } | |
| 828 | + | }); | |
| 829 | + | }); | |
| 830 | + | } | |
| 831 | + | ||
| 832 | + | if let Some(gi) = expand { | |
| 833 | + | if let Some(q) = state.classifier.review.as_mut() | |
| 834 | + | && let Some(g) = q.groups.get_mut(gi) | |
| 835 | + | { | |
| 836 | + | g.expanded = !g.expanded; | |
| 837 | + | } | |
| 838 | + | state.ensure_review_names(gi, REVIEW_RENDER_ROWS); | |
| 839 | + | } | |
| 840 | + | if let Some((gi, ci)) = toggle | |
| 841 | + | && let Some(q) = state.classifier.review.as_mut() | |
| 842 | + | && let Some(c) = q.groups.get_mut(gi).and_then(|g| g.candidates.get_mut(ci)) | |
| 843 | + | { | |
| 844 | + | c.accepted = !c.accepted; | |
| 845 | + | } | |
| 846 | + | if let Some((gi, which)) = accept { | |
| 847 | + | state.accept_review(gi, which); | |
| 848 | + | } | |
| 849 | + | if let Some(gi) = dismiss { | |
| 850 | + | state.dismiss_review_group(gi); | |
| 851 | + | } | |
| 852 | + | } | |
| 853 | + | ||
| 628 | 854 | /// "Trained model" subsection: the optional distilled head that speeds up the | |
| 629 | 855 | /// library-wide auto-tagging pass on large libraries. Optional, without it, auto-tagging | |
| 630 | 856 | /// uses nearest-neighbor matching directly. |
| @@ -464,6 +464,184 @@ | |||
| 464 | 464 | Ok(outcome) | |
| 465 | 465 | } | |
| 466 | 466 | ||
| 467 | + | // Library-wide review queue | |
| 468 | + | ||
| 469 | + | /// Tag namespaces where a sample has exactly one correct answer, so the review | |
| 470 | + | /// queue offers only the best-scoring candidate under the prefix. | |
| 471 | + | /// | |
| 472 | + | /// A sample is one coarse family; it is legitimately several instruments (a "Bass | |
| 473 | + | /// Guitar Loop" is correctly both `instrument.bass` and `instrument.guitar`), so | |
| 474 | + | /// this cannot be the default. Listing a prefix here says the tags below it | |
| 475 | + | /// compete rather than accumulate, which is what makes a queue reviewable: the | |
| 476 | + | /// same sample appearing under two families is a question the user cannot answer | |
| 477 | + | /// from a list, because the answer depends on the other entry. | |
| 478 | + | /// | |
| 479 | + | /// See wiki `af-coarse-families` for why the families got their own namespace | |
| 480 | + | /// rather than living under `instrument.*`, which could never carry this. | |
| 481 | + | pub const EXCLUSIVE_PREFIXES: &[&str] = &["family"]; | |
| 482 | + | ||
| 483 | + | fn is_exclusive(tag: &str) -> Option<&'static str> { | |
| 484 | + | EXCLUSIVE_PREFIXES | |
| 485 | + | .iter() | |
| 486 | + | .copied() | |
| 487 | + | .find(|p| tag.strip_prefix(p).is_some_and(|r| r.starts_with('.'))) | |
| 488 | + | } | |
| 489 | + | ||
| 490 | + | /// One sample's candidacy for a tag, inside a [`SuggestionGroup`]. | |
| 491 | + | #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] | |
| 492 | + | pub struct QueuedSuggestion { | |
| 493 | + | pub hash: String, | |
| 494 | + | pub score: f64, | |
| 495 | + | /// Whether the score cleared this tag's auto threshold. Nothing is applied | |
| 496 | + | /// either way here; it bands the queue so "accept the confident ones" is one | |
| 497 | + | /// gesture rather than a scroll. | |
| 498 | + | pub action: MlAction, | |
| 499 | + | } | |
| 500 | + | ||
| 501 | + | /// Every sample the layer would suggest one tag for, best score first. | |
| 502 | + | /// | |
| 503 | + | /// Complete: nothing is capped here. A cap belongs to rendering, not to the | |
| 504 | + | /// answer, because the gesture this whole queue exists for is "accept these | |
| 505 | + | /// 44,000", and a group that only carries the 500 it could draw makes that | |
| 506 | + | /// gesture a lie. See `ui::classifier` for the render window. | |
| 507 | + | #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] | |
| 508 | + | pub struct SuggestionGroup { | |
| 509 | + | pub tag: String, | |
| 510 | + | pub samples: Vec<QueuedSuggestion>, | |
| 511 | + | } | |
| 512 | + | ||
| 513 | + | /// A library-wide, non-writing pass over the exemplar layer. | |
| 514 | + | #[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize)] | |
| 515 | + | pub struct LibrarySuggestions { | |
| 516 | + | /// Grouped by tag, largest group first. Grouping is the point: "are these 340 | |
| 517 | + | /// all kicks?" is answerable at a glance, where the same 340 rows one sample | |
| 518 | + | /// at a time is not. | |
| 519 | + | pub groups: Vec<SuggestionGroup>, | |
| 520 | + | /// Samples the index scored (analysed, non-deleted). | |
| 521 | + | pub samples_considered: usize, | |
| 522 | + | /// Samples that produced at least one suggestion. | |
| 523 | + | pub samples_with_suggestions: usize, | |
| 524 | + | } | |
| 525 | + | ||
| 526 | + | impl LibrarySuggestions { | |
| 527 | + | /// Total queued suggestions across every group. | |
| 528 | + | #[must_use] | |
| 529 | + | pub fn len(&self) -> usize { | |
| 530 | + | self.groups.iter().map(|g| g.samples.len()).sum() | |
| 531 | + | } | |
| 532 | + | ||
| 533 | + | #[must_use] | |
| 534 | + | pub fn is_empty(&self) -> bool { | |
| 535 | + | self.groups.iter().all(|g| g.samples.is_empty()) | |
| 536 | + | } | |
| 537 | + | } | |
| 538 | + | ||
| 539 | + | /// Drop all but the best-scoring candidate under each exclusive prefix. | |
| 540 | + | fn collapse_exclusive(mut suggestions: Vec<MlSuggestion>) -> Vec<MlSuggestion> { | |
| 541 | + | // Best first, so the first hit per prefix is the one to keep. | |
| 542 | + | suggestions.sort_by(|a, b| b.score.total_cmp(&a.score)); | |
| 543 | + | let mut kept_prefix: Vec<&'static str> = Vec::new(); | |
| 544 | + | suggestions.retain(|s| match is_exclusive(&s.tag) { | |
| 545 | + | None => true, | |
| 546 | + | Some(p) => { | |
| 547 | + | if kept_prefix.contains(&p) { | |
| 548 | + | false | |
| 549 | + | } else { | |
| 550 | + | kept_prefix.push(p); | |
| 551 | + | true | |
| 552 | + | } | |
| 553 | + | } | |
| 554 | + | }); | |
| 555 | + | suggestions | |
| 556 | + | } | |
| 557 | + | ||
| 558 | + | /// Build the index once and collect what the layer *would* suggest across the | |
| 559 | + | /// library, **writing nothing**. | |
| 560 | + | /// | |
| 561 | + | /// The non-writing counterpart of [`auto_apply_library`], and the feed for the | |
| 562 | + | /// review queue. It exists because the bundled layer ships suggest-only: it | |
| 563 | + | /// answers for a library it has never seen, so it proposes and the user accepts, | |
| 564 | + | /// rather than writing tags into someone's library unasked. (Auto-apply stays | |
| 565 | + | /// available for a user's own labels, which is what [`auto_apply_library`] is.) | |
| 566 | + | /// | |
| 567 | + | /// Groups are complete, deliberately. An earlier cut capped each at 500 and | |
| 568 | + | /// measurement killed it: over 50k samples that dropped 44,000 of 45,000 | |
| 569 | + | /// suggestions, so "Accept all" could only ever take 500 and clearing one group | |
| 570 | + | /// meant ninety re-runs of a five-second pass. The cap was bounding the wrong | |
| 571 | + | /// thing. Rendering is where a window belongs. | |
| 572 | + | /// | |
| 573 | + | /// Cost is index size times candidate count, the same shape as | |
| 574 | + | /// [`auto_apply_library`]: measured at 11 ms over 1k samples, 268 ms over 10k and | |
| 575 | + | /// 5.0 s over 50k. That is why it is a worker job with a spinner and never | |
| 576 | + | /// something the GUI thread calls. | |
| 577 | + | #[instrument(skip_all)] | |
| 578 | + | pub fn preview_library(db: &Database, k: usize) -> Result<LibrarySuggestions> { | |
| 579 | + | let index = build_index(db)?; | |
| 580 | + | if index.is_empty() { | |
| 581 | + | return Ok(LibrarySuggestions::default()); | |
| 582 | + | } | |
| 583 | + | let hashes: Vec<String> = { | |
| 584 | + | let mut stmt = db.conn().prepare( | |
| 585 | + | "SELECT s.hash FROM live_samples s | |
| 586 | + | JOIN sample_features f ON f.hash = s.hash AND f.feat_version = ?1", | |
| 587 | + | )?; | |
| 588 | + | stmt.query_map([FEATURE_VERSION], |row| row.get(0))? | |
| 589 | + | .collect::<std::result::Result<Vec<_>, _>>()? | |
| 590 | + | }; | |
| 591 | + | ||
| 592 | + | let samples_considered = hashes.len(); | |
| 593 | + | let mut samples_with_suggestions = 0usize; | |
| 594 | + | let mut by_tag: HashMap<String, Vec<QueuedSuggestion>> = HashMap::new(); | |
| 595 | + | for hash in hashes { | |
| 596 | + | let outcome = preview_sample(db, &hash, &index, k)?; | |
| 597 | + | // Both bands: `apply_policy` already dropped anything below review, and | |
| 598 | + | // already skipped tags the sample carries, so everything here is a real | |
| 599 | + | // proposal the user has not answered. | |
| 600 | + | let mut all = outcome.auto_applied; | |
| 601 | + | all.extend(outcome.pending_review); | |
| 602 | + | let all = collapse_exclusive(all); | |
| 603 | + | if all.is_empty() { | |
| 604 | + | continue; | |
| 605 | + | } | |
| 606 | + | samples_with_suggestions += 1; | |
| 607 | + | for s in all { | |
| 608 | + | by_tag.entry(s.tag).or_default().push(QueuedSuggestion { | |
| 609 | + | hash: hash.clone(), | |
| 610 | + | score: s.score, | |
| 611 | + | action: s.action, | |
| 612 | + | }); | |
| 613 | + | } | |
| 614 | + | } | |
| 615 | + | ||
| 616 | + | let mut groups: Vec<SuggestionGroup> = by_tag | |
| 617 | + | .into_iter() | |
| 618 | + | .map(|(tag, mut samples)| { | |
| 619 | + | // Best first, so the render window shows the strongest candidates and | |
| 620 | + | // "accept the confident ones" reads down the list. | |
| 621 | + | samples.sort_by(|a, b| { | |
| 622 | + | b.score | |
| 623 | + | .total_cmp(&a.score) | |
| 624 | + | .then_with(|| a.hash.cmp(&b.hash)) | |
| 625 | + | }); | |
| 626 | + | SuggestionGroup { tag, samples } | |
| 627 | + | }) | |
| 628 | + | .collect(); | |
| 629 | + | // Biggest group first: the largest is where bulk accept saves the most, and | |
| 630 | + | // ties break by tag so two runs over one library order identically. | |
| 631 | + | groups.sort_by(|a, b| { | |
| 632 | + | b.samples | |
| 633 | + | .len() | |
| 634 | + | .cmp(&a.samples.len()) | |
| 635 | + | .then_with(|| a.tag.cmp(&b.tag)) | |
| 636 | + | }); | |
| 637 | + | ||
| 638 | + | Ok(LibrarySuggestions { | |
| 639 | + | groups, | |
| 640 | + | samples_considered, | |
| 641 | + | samples_with_suggestions, | |
| 642 | + | }) | |
| 643 | + | } | |
| 644 | + | ||
| 467 | 645 | /// Build the index once and auto-apply above-threshold tags to every analyzed, | |
| 468 | 646 | /// non-deleted sample. Returns the total number of tags applied. Heavy, intended | |
| 469 | 647 | /// for an explicit "suggest across library" action. | |
| @@ -718,4 +896,142 @@ | |||
| 718 | 896 | assert!(outcome.auto_applied.is_empty()); | |
| 719 | 897 | assert!(!outcome.pending_review.is_empty()); | |
| 720 | 898 | } | |
| 899 | + | ||
| 900 | + | // --- Library review queue --- | |
| 901 | + | ||
| 902 | + | #[test] | |
| 903 | + | fn preview_library_writes_nothing() { | |
| 904 | + | // The whole point of the suggest-only path: it must be safe to run against | |
| 905 | + | // a library repeatedly without changing it. | |
| 906 | + | let db = Database::open_in_memory().unwrap(); | |
| 907 | + | insert(&db, "k1", &vec_at(0.0), &["instrument.drum.kick"]); | |
| 908 | + | insert(&db, "k2", &vec_at(0.01), &["instrument.drum.kick"]); | |
| 909 | + | insert(&db, "q", &vec_at(0.02), &[]); | |
| 910 | + | ||
| 911 | + | let before: usize = db | |
| 912 | + | .conn() | |
| 913 | + | .query_row("SELECT COUNT(*) FROM tags", [], |r| r.get::<_, i64>(0)) | |
| 914 | + | .unwrap() as usize; | |
| 915 | + | let queue = preview_library(&db, DEFAULT_K).unwrap(); | |
| 916 | + | let after: usize = db | |
| 917 | + | .conn() | |
| 918 | + | .query_row("SELECT COUNT(*) FROM tags", [], |r| r.get::<_, i64>(0)) | |
| 919 | + | .unwrap() as usize; | |
| 920 | + | ||
| 921 | + | assert!(!queue.is_empty(), "q should be offered the kick tag"); | |
| 922 | + | assert_eq!(before, after, "preview must not write a single tag"); | |
| 923 | + | } | |
| 924 | + | ||
| 925 | + | #[test] | |
| 926 | + | fn preview_library_groups_by_tag_and_counts_samples() { | |
| 927 | + | let db = Database::open_in_memory().unwrap(); | |
| 928 | + | insert(&db, "k1", &vec_at(0.0), &["instrument.drum.kick"]); | |
| 929 | + | insert(&db, "k2", &vec_at(0.01), &["instrument.drum.kick"]); | |
| 930 | + | // Two unlabelled samples sitting in the kick cluster. | |
| 931 | + | insert(&db, "q1", &vec_at(0.02), &[]); | |
| 932 | + | insert(&db, "q2", &vec_at(0.03), &[]); | |
| 933 | + | ||
| 934 | + | let queue = preview_library(&db, DEFAULT_K).unwrap(); | |
| 935 | + | let kick = queue | |
| 936 | + | .groups | |
| 937 | + | .iter() | |
| 938 | + | .find(|g| g.tag == "instrument.drum.kick") | |
| 939 | + | .expect("a kick group"); | |
| 940 | + | assert_eq!(kick.samples.len(), 2, "both unlabelled samples queued"); | |
| 941 | + | // The labelled samples already carry the tag, so they are not re-offered. | |
| 942 | + | assert!(kick.samples.iter().all(|s| s.hash.starts_with('q'))); | |
| 943 | + | assert_eq!(queue.samples_considered, 4); | |
| 944 | + | assert_eq!(queue.samples_with_suggestions, 2); | |
| 945 | + | } | |
| 946 | + | ||
| 947 | + | #[test] | |
| 948 | + | fn a_group_carries_every_candidate_so_accept_all_can_mean_all() { | |
| 949 | + | // Capping here is what an earlier cut got wrong: a group holding 500 of | |
| 950 | + | // 45,000 makes "Accept all" a lie, and clearing it needs ninety re-runs. | |
| 951 | + | let db = Database::open_in_memory().unwrap(); | |
| 952 | + | insert(&db, "k1", &vec_at(0.0), &["instrument.drum.kick"]); | |
| 953 | + | for i in 0..40 { | |
| 954 | + | insert(&db, &format!("q{i}"), &vec_at(0.001 * f64::from(i)), &[]); | |
| 955 | + | } | |
| 956 | + | let queue = preview_library(&db, DEFAULT_K).unwrap(); | |
| 957 | + | assert_eq!(queue.groups[0].samples.len(), 40); | |
| 958 | + | // And sorted best-first, so a render window shows the strongest. | |
| 959 | + | let scores: Vec<f64> = queue.groups[0].samples.iter().map(|s| s.score).collect(); | |
| 960 | + | assert!( | |
| 961 | + | scores.windows(2).all(|w| w[0] >= w[1]), | |
| 962 | + | "group must be sorted best first" | |
| 963 | + | ); | |
| 964 | + | } | |
| 965 | + | ||
| 966 | + | #[test] | |
| 967 | + | fn preview_library_offers_one_family_per_sample() { | |
| 968 | + | // Families are mutually exclusive: the same sample under two of them is a | |
| 969 | + | // question the user cannot answer from a list. | |
| 970 | + | let db = Database::open_in_memory().unwrap(); | |
| 971 | + | insert(&db, "a", &vec_at(0.0), &["family.low"]); | |
| 972 | + | insert(&db, "b", &vec_at(10.0), &["family.drum-bright"]); | |
| 973 | + | insert(&db, "q", &vec_at(4.0), &[]); | |
| 974 | + | set_policy(&db, "family.low", 0.2, 0.99).unwrap(); | |
| 975 | + | set_policy(&db, "family.drum-bright", 0.2, 0.99).unwrap(); | |
| 976 | + | ||
| 977 | + | // Both families score above review for this query, so without collapsing | |
| 978 | + | // it would appear in two groups. | |
| 979 | + | let index = build_index(&db).unwrap(); | |
| 980 | + | let raw = preview_sample(&db, "q", &index, DEFAULT_K).unwrap(); | |
| 981 | + | assert_eq!( | |
| 982 | + | raw.auto_applied.len() + raw.pending_review.len(), | |
| 983 | + | 2, | |
| 984 | + | "precondition: the sample really does score for both" | |
| 985 | + | ); | |
| 986 | + | ||
| 987 | + | let queue = preview_library(&db, DEFAULT_K).unwrap(); | |
| 988 | + | let offers: Vec<&str> = queue | |
| 989 | + | .groups | |
| 990 | + | .iter() | |
| 991 | + | .filter(|g| g.samples.iter().any(|s| s.hash == "q")) | |
| 992 | + | .map(|g| g.tag.as_str()) | |
| 993 | + | .collect(); | |
| 994 | + | assert_eq!(offers.len(), 1, "one family only, got {offers:?}"); | |
| 995 | + | // And it is the nearer one: q at 4.0 sits closer to a at 0.0 than b at 10.0. | |
| 996 | + | assert_eq!(offers[0], "family.low"); | |
| 997 | + | } | |
| 998 | + | ||
| 999 | + | #[test] | |
| 1000 | + | fn non_exclusive_tags_still_stack_on_one_sample() { | |
| 1001 | + | // The mirror of the test above. A sample is legitimately several | |
| 1002 | + | // instruments, so collapsing must not apply outside the declared prefixes. | |
| 1003 | + | let db = Database::open_in_memory().unwrap(); | |
| 1004 | + | insert(&db, "a", &vec_at(0.0), &["instrument.bass"]); | |
| 1005 | + | insert(&db, "b", &vec_at(10.0), &["instrument.guitar"]); | |
| 1006 | + | insert(&db, "q", &vec_at(5.0), &[]); | |
| 1007 | + | set_policy(&db, "instrument.bass", 0.2, 0.99).unwrap(); | |
| 1008 | + | set_policy(&db, "instrument.guitar", 0.2, 0.99).unwrap(); | |
| 1009 | + | ||
| 1010 | + | let queue = preview_library(&db, DEFAULT_K).unwrap(); | |
| 1011 | + | let offers = queue | |
| 1012 | + | .groups | |
| 1013 | + | .iter() | |
| 1014 | + | .filter(|g| g.samples.iter().any(|s| s.hash == "q")) | |
| 1015 | + | .count(); | |
| 1016 | + | assert_eq!(offers, 2, "both instruments stay on offer"); | |
| 1017 | + | } | |
| 1018 | + | ||
| 1019 | + | #[test] | |
| 1020 | + | fn exclusive_prefix_matches_on_a_segment_boundary() { | |
| 1021 | + | // `familytree.x` must not be treated as a family tag. | |
| 1022 | + | assert_eq!(is_exclusive("family.low"), Some("family")); | |
| 1023 | + | assert_eq!(is_exclusive("family.drum-bright"), Some("family")); | |
| 1024 | + | assert_eq!(is_exclusive("familytree.low"), None); | |
| 1025 | + | assert_eq!(is_exclusive("family"), None); | |
| 1026 | + | assert_eq!(is_exclusive("instrument.bass"), None); | |
| 1027 | + | } | |
| 1028 | + | ||
| 1029 | + | #[test] | |
| 1030 | + | fn an_empty_index_produces_an_empty_queue() { | |
| 1031 | + | let db = Database::open_in_memory().unwrap(); | |
| 1032 | + | insert(&db, "q", &vec_at(0.0), &[]); | |
| 1033 | + | let queue = preview_library(&db, DEFAULT_K).unwrap(); | |
| 1034 | + | assert!(queue.is_empty()); | |
| 1035 | + | assert_eq!(queue.len(), 0); | |
| 1036 | + | } | |
| 721 | 1037 | } |