max / audiofiles
- Co-Authored-By
- Claude Opus 5 (1M context) <noreply@anthropic.com>
11 files changed,
+44487 insertions,
-72 deletions
| @@ -122,6 +122,12 @@ | |||
| 122 | 122 | ||
| 123 | 123 | TRAINING_CLASSES = ["kick", "snare", "hihat", "cymbal", "clap", "tom", "percussion"] | |
| 124 | 124 | ||
| 125 | + | # The one dataset that fills samples/training/, and so the one the labels are | |
| 126 | + | # derived from. Recorded in the manifest as `training_source` because the | |
| 127 | + | # `datasets` list answers a different question -- what was fetched -- and the | |
| 128 | + | # two diverge as soon as anything else is downloaded into the same corpus. | |
| 129 | + | TRAINING_DATASET = "reverb-drums" | |
| 130 | + | ||
| 125 | 131 | ||
| 126 | 132 | def classify_name(path: Path) -> tuple[str | None, str]: | |
| 127 | 133 | """Map a source file to a training class from its name and parent dirs. | |
| @@ -324,6 +330,44 @@ | |||
| 324 | 330 | return counts | |
| 325 | 331 | ||
| 326 | 332 | ||
| 333 | + | def load_manifest(path: Path) -> dict: | |
| 334 | + | """Read the existing manifest, or start an empty one. | |
| 335 | + | ||
| 336 | + | A run fetches the datasets it was asked for and rebuilds only what those | |
| 337 | + | datasets feed, so it knows about a slice of the corpus rather than all of | |
| 338 | + | it. Writing a manifest built from that slice alone is how the attribution | |
| 339 | + | got lost: a `--datasets nsynth` run left samples/training/ full of | |
| 340 | + | reverb-drums files and replaced the credit with a dataset the labels never | |
| 341 | + | came from. Both are CC-BY 4.0 so the licence class survived it, but CC-BY | |
| 342 | + | asks for credit to the work actually used. Merge instead. | |
| 343 | + | ||
| 344 | + | A corrupt manifest is not fatal: this run is about to rewrite it with | |
| 345 | + | whatever it knows, and refusing to proceed would leave the bad file in | |
| 346 | + | place. The old contents are lost, which is the reason for the warning. | |
| 347 | + | """ | |
| 348 | + | if not path.exists(): | |
| 349 | + | return {} | |
| 350 | + | try: | |
| 351 | + | doc = json.loads(path.read_text()) | |
| 352 | + | except (OSError, json.JSONDecodeError) as e: | |
| 353 | + | print(f" warning: cannot read {path} ({e}); starting a fresh manifest") | |
| 354 | + | return {} | |
| 355 | + | return doc if isinstance(doc, dict) else {} | |
| 356 | + | ||
| 357 | + | ||
| 358 | + | def merge_datasets(old: list, new: list) -> list: | |
| 359 | + | """Union two dataset lists by name, with this run's entry winning. | |
| 360 | + | ||
| 361 | + | This run's entry wins because it was just fetched from the URL it names, | |
| 362 | + | whereas the recorded one may predate a registry edit. Sorted so a re-run | |
| 363 | + | over the same corpus produces a byte-identical manifest. | |
| 364 | + | """ | |
| 365 | + | by_name = {d["name"]: d for d in old if isinstance(d, dict) and "name" in d} | |
| 366 | + | for d in new: | |
| 367 | + | by_name[d["name"]] = d | |
| 368 | + | return [by_name[k] for k in sorted(by_name)] | |
| 369 | + | ||
| 370 | + | ||
| 327 | 371 | def main() -> None: | |
| 328 | 372 | ap = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) | |
| 329 | 373 | ap.add_argument("--dest", type=Path, default=Path("/media/max/T9/af-corpus")) | |
| @@ -357,7 +401,10 @@ | |||
| 357 | 401 | samples = args.dest / "samples" | |
| 358 | 402 | downloads.mkdir(parents=True, exist_ok=True) | |
| 359 | 403 | ||
| 360 | - | manifest = {"datasets": [], "layout": str(samples)} | |
| 404 | + | manifest_path = args.dest / "MANIFEST.json" | |
| 405 | + | manifest = load_manifest(manifest_path) | |
| 406 | + | manifest["layout"] = str(samples) | |
| 407 | + | fetched: list[dict] = [] | |
| 361 | 408 | ||
| 362 | 409 | for name in wanted: | |
| 363 | 410 | d = DATASETS[name] | |
| @@ -372,16 +419,20 @@ | |||
| 372 | 419 | download(extra_url, extra_path) | |
| 373 | 420 | if extra_path.suffix.lower() in (".zip", ".7z", ".gz"): | |
| 374 | 421 | extract(extra_path, raw / name, marker=raw / name / Path(extra_name).stem) | |
| 375 | - | manifest["datasets"].append( | |
| 376 | - | {"name": name, "license": d["license"], "source": d["url"]} | |
| 377 | - | ) | |
| 422 | + | fetched.append({"name": name, "license": d["license"], "source": d["url"]}) | |
| 378 | 423 | ||
| 379 | - | if not args.no_build and "reverb-drums" in wanted: | |
| 424 | + | manifest["datasets"] = merge_datasets(manifest.get("datasets", []), fetched) | |
| 425 | + | ||
| 426 | + | if not args.no_build and TRAINING_DATASET in wanted: | |
| 380 | 427 | print("\n=== building training layout ===") | |
| 381 | - | counts = build_training(raw / "reverb-drums", samples) | |
| 428 | + | counts = build_training(raw / TRAINING_DATASET, samples) | |
| 382 | 429 | for k, v in counts.items(): | |
| 383 | 430 | print(f" {k:<14} {v}") | |
| 384 | 431 | manifest["training_counts"] = counts | |
| 432 | + | # Names the dataset the labels came from, which is what anything | |
| 433 | + | # shipping those labels has to credit. The bundled .afcl generator | |
| 434 | + | # reads this key and refuses to build without it. | |
| 435 | + | manifest["training_source"] = TRAINING_DATASET | |
| 385 | 436 | ||
| 386 | 437 | # Runs off the built layout, not off a dataset, so it is gated on the loops | |
| 387 | 438 | # existing rather than on which datasets were requested. | |
| @@ -400,8 +451,8 @@ | |||
| 400 | 451 | ||
| 401 | 452 | # Provenance matters here: the corpus mixes licenses, and anything that | |
| 402 | 453 | # feeds a model needs that recorded rather than reconstructed later. | |
| 403 | - | (args.dest / "MANIFEST.json").write_text(json.dumps(manifest, indent=2)) | |
| 404 | - | print(f"\nwrote {args.dest / 'MANIFEST.json'}") | |
| 454 | + | manifest_path.write_text(json.dumps(manifest, indent=2)) | |
| 455 | + | print(f"\nwrote {manifest_path}") | |
| 405 | 456 | print(f"point the bench at it: export AF_BENCH_CORPUS={samples}") | |
| 406 | 457 | ||
| 407 | 458 |
| @@ -60,6 +60,8 @@ | |||
| 60 | 60 | /// carries no audio, so the manifest is what records whose labels these derive | |
| 61 | 61 | /// from. A missing or unreadable manifest is fatal rather than defaulted, because | |
| 62 | 62 | /// a layer that ships without attribution is the one outcome worth failing over. | |
| 63 | + | /// Same for a manifest that does not name its `training_source`: see | |
| 64 | + | /// [`license_note`] for why the `datasets` list alone is not evidence. | |
| 63 | 65 | /// | |
| 64 | 66 | /// Looked up in `corpus_root` and then in its parent, because `AF_BENCH_CORPUS` | |
| 65 | 67 | /// points at the `samples/` directory while `corpus.py` writes the manifest one | |
| @@ -93,49 +95,61 @@ | |||
| 93 | 95 | return Err(format!("{} lists no datasets", path.display())); | |
| 94 | 96 | } | |
| 95 | 97 | ||
| 96 | - | // The manifest must actually describe the training data, not merely exist. | |
| 98 | + | // Credit the dataset the LABELS came from, not everything ever fetched into | |
| 99 | + | // this corpus. | |
| 97 | 100 | // | |
| 98 | - | // `corpus.py` populates `samples/training/` only from `reverb-drums`, and | |
| 99 | - | // records `training_counts` in the same pass. It also OVERWRITES the manifest | |
| 100 | - | // rather than merging it, so a later `--datasets nsynth` run leaves the | |
| 101 | - | // training folders in place while replacing the attribution with a dataset | |
| 102 | - | // the labels did not come from. That is what the corpus on the T9 looks like | |
| 103 | - | // today: 1,049 reverb-drums one-shots under `training/`, and a manifest | |
| 104 | - | // naming only nsynth. | |
| 101 | + | // `datasets` answers "what was downloaded", and the two questions diverge the | |
| 102 | + | // moment a second dataset lands: the T9 corpus holds nsynth and fsl10k | |
| 103 | + | // alongside reverb-drums, and only reverb-drums fills `samples/training/`. | |
| 104 | + | // Crediting all three would name two works this layer does not derive from, | |
| 105 | + | // and an earlier version of corpus.py overwrote the manifest per run, so the | |
| 106 | + | // credit was at one point reverb-drums labels attributed solely to nsynth. | |
| 107 | + | // Both are CC-BY 4.0, so the licence class survived it, but CC-BY asks for | |
| 108 | + | // credit to the work actually used. | |
| 105 | 109 | // | |
| 106 | - | // Both are CC-BY 4.0, so this is not a licence-class error, but CC-BY | |
| 107 | - | // requires crediting the work actually used. Refuse rather than emit a | |
| 108 | - | // confident wrong credit into a file meant to ship. | |
| 109 | - | if doc.get("training_counts").is_none() { | |
| 110 | + | // `training_source` is written by corpus.py in the same pass that fills the | |
| 111 | + | // training folders, so its absence proves the manifest does not describe the | |
| 112 | + | // labels. Refuse rather than emit a confident wrong credit into a file meant | |
| 113 | + | // to ship. | |
| 114 | + | let source_name = doc | |
| 115 | + | .get("training_source") | |
| 116 | + | .and_then(|v| v.as_str()) | |
| 117 | + | .ok_or_else(|| { | |
| 118 | + | format!( | |
| 119 | + | "{} has no `training_source`, so it does not say which dataset the \ | |
| 120 | + | labels came from and its `datasets` list cannot be trusted as the \ | |
| 121 | + | attribution. Re-run scripts/corpus.py over this corpus to record it.", | |
| 122 | + | path.display() | |
| 123 | + | ) | |
| 124 | + | })?; | |
| 125 | + | ||
| 126 | + | let entry = datasets | |
| 127 | + | .iter() | |
| 128 | + | .find(|d| d.get("name").and_then(|v| v.as_str()) == Some(source_name)) | |
| 129 | + | .ok_or_else(|| { | |
| 130 | + | format!( | |
| 131 | + | "{} names `{source_name}` as the training source but has no dataset \ | |
| 132 | + | entry for it, so there is no licence or URL to credit", | |
| 133 | + | path.display() | |
| 134 | + | ) | |
| 135 | + | })?; | |
| 136 | + | ||
| 137 | + | let license = entry.get("license").and_then(|v| v.as_str()).unwrap_or(""); | |
| 138 | + | let source = entry.get("source").and_then(|v| v.as_str()).unwrap_or(""); | |
| 139 | + | if license.is_empty() { | |
| 110 | 140 | return Err(format!( | |
| 111 | - | "{} has no `training_counts`, so it does not describe the labelled \ | |
| 112 | - | training data and its `datasets` list cannot be trusted as the \ | |
| 113 | - | attribution. Rebuild the corpus with reverb-drums (which is what \ | |
| 114 | - | fills samples/training/) so the manifest and the labels agree.", | |
| 141 | + | "{} has no license for the training source `{source_name}`", | |
| 115 | 142 | path.display() | |
| 116 | 143 | )); | |
| 117 | 144 | } | |
| 145 | + | let credit = if source.is_empty() { | |
| 146 | + | format!("{source_name} ({license})") | |
| 147 | + | } else { | |
| 148 | + | format!("{source_name} ({license}), {source}") | |
| 149 | + | }; | |
| 118 | 150 | ||
| 119 | - | let mut parts = Vec::new(); | |
| 120 | - | for d in datasets { | |
| 121 | - | let name = d.get("name").and_then(|v| v.as_str()).unwrap_or(""); | |
| 122 | - | let license = d.get("license").and_then(|v| v.as_str()).unwrap_or(""); | |
| 123 | - | let source = d.get("source").and_then(|v| v.as_str()).unwrap_or(""); | |
| 124 | - | if name.is_empty() || license.is_empty() { | |
| 125 | - | return Err(format!( | |
| 126 | - | "{} has a dataset entry missing name or license", | |
| 127 | - | path.display() | |
| 128 | - | )); | |
| 129 | - | } | |
| 130 | - | if source.is_empty() { | |
| 131 | - | parts.push(format!("{name} ({license})")); | |
| 132 | - | } else { | |
| 133 | - | parts.push(format!("{name} ({license}), {source}")); | |
| 134 | - | } | |
| 135 | - | } | |
| 136 | 151 | Ok(format!( | |
| 137 | - | "Labels derived from: {}. This layer carries feature vectors and labels only, no audio.", | |
| 138 | - | parts.join("; ") | |
| 152 | + | "Labels derived from: {credit}. This layer carries feature vectors and labels only, no audio." | |
| 139 | 153 | )) | |
| 140 | 154 | } | |
| 141 | 155 | ||
| @@ -413,46 +427,81 @@ | |||
| 413 | 427 | std::fs::create_dir_all(&samples).unwrap(); | |
| 414 | 428 | std::fs::write( | |
| 415 | 429 | dir.path().join(MANIFEST), | |
| 416 | - | r#"{"training_counts":{"kick":2},"datasets":[{"name":"nsynth","license":"CC-BY 4.0"}]}"#, | |
| 430 | + | r#"{"training_source":"reverb-drums", | |
| 431 | + | "datasets":[{"name":"reverb-drums","license":"CC-BY 4.0"}]}"#, | |
| 417 | 432 | ) | |
| 418 | 433 | .unwrap(); | |
| 419 | - | assert!(license_note(&samples).unwrap().contains("nsynth")); | |
| 434 | + | assert!(license_note(&samples).unwrap().contains("reverb-drums")); | |
| 420 | 435 | } | |
| 421 | 436 | ||
| 422 | 437 | #[test] | |
| 423 | - | fn license_note_names_every_dataset_and_its_licence() { | |
| 438 | + | fn license_note_credits_the_training_source_and_its_licence() { | |
| 424 | 439 | let dir = tempfile::tempdir().unwrap(); | |
| 425 | 440 | std::fs::write( | |
| 426 | 441 | dir.path().join(MANIFEST), | |
| 427 | - | r#"{"training_counts":{"kick":2},"datasets":[ | |
| 428 | - | {"name":"nsynth","license":"CC-BY 4.0","source":"http://example.invalid/nsynth"}, | |
| 442 | + | r#"{"training_source":"reverb-drums","datasets":[ | |
| 443 | + | {"name":"reverb-drums","license":"CC-BY 4.0","source":"http://example.invalid/rd"}, | |
| 429 | 444 | {"name":"other","license":"CC0 1.0"} | |
| 430 | 445 | ]}"#, | |
| 431 | 446 | ) | |
| 432 | 447 | .unwrap(); | |
| 433 | 448 | let note = license_note(dir.path()).unwrap(); | |
| 434 | - | assert!(note.contains("nsynth (CC-BY 4.0)")); | |
| 435 | - | assert!(note.contains("http://example.invalid/nsynth")); | |
| 436 | - | assert!(note.contains("other (CC0 1.0)")); | |
| 449 | + | assert!(note.contains("reverb-drums (CC-BY 4.0)")); | |
| 450 | + | assert!(note.contains("http://example.invalid/rd")); | |
| 437 | 451 | assert!(note.contains("no audio")); | |
| 438 | 452 | } | |
| 439 | 453 | ||
| 440 | 454 | #[test] | |
| 441 | - | fn license_note_rejects_a_dataset_missing_its_licence() { | |
| 455 | + | fn license_note_credits_only_the_training_source() { | |
| 456 | + | // The real corpus holds nsynth and fsl10k too, and neither contributed a | |
| 457 | + | // label. Naming them would credit works this layer does not derive from. | |
| 442 | 458 | let dir = tempfile::tempdir().unwrap(); | |
| 443 | 459 | std::fs::write( | |
| 444 | 460 | dir.path().join(MANIFEST), | |
| 445 | - | r#"{"training_counts":{"kick":2},"datasets":[{"name":"nsynth"}]}"#, | |
| 461 | + | r#"{"training_source":"reverb-drums","datasets":[ | |
| 462 | + | {"name":"reverb-drums","license":"CC-BY 4.0"}, | |
| 463 | + | {"name":"nsynth","license":"CC-BY 4.0"}, | |
| 464 | + | {"name":"fsl10k","license":"CC-BY 4.0"} | |
| 465 | + | ]}"#, | |
| 466 | + | ) | |
| 467 | + | .unwrap(); | |
| 468 | + | let note = license_note(dir.path()).unwrap(); | |
| 469 | + | assert!(note.contains("reverb-drums"), "{note}"); | |
| 470 | + | assert!(!note.contains("nsynth"), "{note}"); | |
| 471 | + | assert!(!note.contains("fsl10k"), "{note}"); | |
| 472 | + | } | |
| 473 | + | ||
| 474 | + | #[test] | |
| 475 | + | fn license_note_rejects_a_training_source_missing_its_licence() { | |
| 476 | + | let dir = tempfile::tempdir().unwrap(); | |
| 477 | + | std::fs::write( | |
| 478 | + | dir.path().join(MANIFEST), | |
| 479 | + | r#"{"training_source":"reverb-drums","datasets":[{"name":"reverb-drums"}]}"#, | |
| 446 | 480 | ) | |
| 447 | 481 | .unwrap(); | |
| 448 | 482 | assert!(license_note(dir.path()).is_err()); | |
| 449 | 483 | } | |
| 450 | 484 | ||
| 485 | + | #[test] | |
| 486 | + | fn license_note_rejects_a_training_source_with_no_dataset_entry() { | |
| 487 | + | // Nothing to credit: the manifest names a source it has no licence or | |
| 488 | + | // URL for, so there is no attribution to write. | |
| 489 | + | let dir = tempfile::tempdir().unwrap(); | |
| 490 | + | std::fs::write( | |
| 491 | + | dir.path().join(MANIFEST), | |
| 492 | + | r#"{"training_source":"reverb-drums", | |
| 493 | + | "datasets":[{"name":"nsynth","license":"CC-BY 4.0"}]}"#, | |
| 494 | + | ) | |
| 495 | + | .unwrap(); | |
| 496 | + | let err = license_note(dir.path()).unwrap_err(); | |
| 497 | + | assert!(err.contains("reverb-drums"), "{err}"); | |
| 498 | + | } | |
| 499 | + | ||
| 451 | 500 | #[test] | |
| 452 | 501 | fn license_note_rejects_a_manifest_that_does_not_describe_the_training_data() { | |
| 453 | - | // The live failure: corpus.py overwrites the manifest, so a later fetch | |
| 454 | - | // of a different dataset leaves training/ intact while replacing the | |
| 455 | - | // attribution. Without `training_counts` the datasets list is not | |
| 502 | + | // The live failure: corpus.py used to overwrite the manifest, so a later | |
| 503 | + | // fetch of a different dataset left training/ intact while replacing the | |
| 504 | + | // attribution. Without `training_source` the datasets list is not | |
| 456 | 505 | // evidence of where the labels came from. | |
| 457 | 506 | let dir = tempfile::tempdir().unwrap(); | |
| 458 | 507 | std::fs::write( | |
| @@ -461,7 +510,7 @@ | |||
| 461 | 510 | ) | |
| 462 | 511 | .unwrap(); | |
| 463 | 512 | let err = license_note(dir.path()).unwrap_err(); | |
| 464 | - | assert!(err.contains("training_counts"), "{err}"); | |
| 513 | + | assert!(err.contains("training_source"), "{err}"); | |
| 465 | 514 | } | |
| 466 | 515 | ||
| 467 | 516 | #[test] |
| @@ -308,14 +308,17 @@ | |||
| 308 | 308 | // someone had just imported would be an expensive surprise. | |
| 309 | 309 | let vault = std::env::var("AF_BENCH_VAULT") | |
| 310 | 310 | .map_or_else(|_| std::env::temp_dir().join("af-afcl-gen"), PathBuf::from); | |
| 311 | + | // Defaults into audiofiles-core's own assets/, which is where the layer | |
| 312 | + | // is embedded from. Keeping the artifact inside the crate that | |
| 313 | + | // `include_str!`s it means the path never reaches outside the crate | |
| 314 | + | // directory, and the blob travels with the crate rather than with the | |
| 315 | + | // workspace root. | |
| 311 | 316 | let out = std::env::var("AF_AFCL_OUT").map_or_else( | |
| 312 | 317 | |_| { | |
| 313 | 318 | PathBuf::from(env!("CARGO_MANIFEST_DIR")) | |
| 314 | 319 | .parent() | |
| 315 | 320 | .unwrap() | |
| 316 | - | .parent() | |
| 317 | - | .unwrap() | |
| 318 | - | .join("assets/official.afcl") | |
| 321 | + | .join("audiofiles-core/assets/official.afcl") | |
| 319 | 322 | }, | |
| 320 | 323 | PathBuf::from, | |
| 321 | 324 | ); |
| @@ -102,6 +102,15 @@ | |||
| 102 | 102 | RowHeight => "row_height": Synced, | |
| 103 | 103 | ColumnConfig => "column_config": Synced, | |
| 104 | 104 | SampleTombstoneRetainDays => "sample_tombstone_retain_days": Synced, | |
| 105 | + | /// `FEATURE_VERSION` the bundled official `.afcl` was imported under, absent | |
| 106 | + | /// if it never has been. Synced, and load-bearingly so: the layer itself | |
| 107 | + | /// replicates through `classifier_layers`, so a device-local marker would | |
| 108 | + | /// have the second device import a second copy of a layer it already has, | |
| 109 | + | /// and would resurrect the layer on every device but the one the user | |
| 110 | + | /// removed it on. The value is the version rather than a bare flag so a | |
| 111 | + | /// later `FEATURE_VERSION` bump can tell a stale bundled layer from a | |
| 112 | + | /// current one without a second key. | |
| 113 | + | OfficialLayerImported => "official_layer_imported": Synced, | |
| 105 | 114 | ||
| 106 | 115 | // --- Local: local paths and safety gates. Never sync these. --- | |
| 107 | 116 | /// Reference-in-place vault mode. Local safety gate. |
| @@ -6,6 +6,29 @@ | |||
| 6 | 6 | /// Create a BrowserState backed by a temporary directory with an in-memory-like | |
| 7 | 7 | /// on-disk SQLite database. Each test gets full isolation. | |
| 8 | 8 | fn make_state() -> (BrowserState, tempfile::TempDir) { | |
| 9 | + | let dir = tempfile::TempDir::new().unwrap(); | |
| 10 | + | // Skip the bundled official layer. A real fresh vault imports it (that is | |
| 11 | + | // what `bundled_official_layer_is_imported_on_first_run` covers), but here it | |
| 12 | + | // would put 1,049 exemplars and a second layer into every test library, so | |
| 13 | + | // every layer-count and exemplar-count assertion would be measuring the | |
| 14 | + | // bundle rather than what the test set up. Pre-setting the marker on the | |
| 15 | + | // vault's database is what `official::ensure_imported` reads, so opening it | |
| 16 | + | // once before the backend does is enough. | |
| 17 | + | { | |
| 18 | + | let db = audiofiles_core::db::Database::open(dir.path().join("audiofiles.db")).unwrap(); | |
| 19 | + | db.set_config( | |
| 20 | + | audiofiles_core::config_key::ConfigKey::OfficialLayerImported, | |
| 21 | + | "test-skip", | |
| 22 | + | ) | |
| 23 | + | .unwrap(); | |
| 24 | + | } | |
| 25 | + | let shared = Arc::new(SharedState::new()); | |
| 26 | + | let state = BrowserState::new(dir.path(), shared, 44100.0, "Vault").unwrap(); | |
| 27 | + | (state, dir) | |
| 28 | + | } | |
| 29 | + | ||
| 30 | + | /// A state whose vault gets the real first-run treatment, bundled layer included. | |
| 31 | + | fn make_state_first_run() -> (BrowserState, tempfile::TempDir) { | |
| 9 | 32 | let dir = tempfile::TempDir::new().unwrap(); | |
| 10 | 33 | let shared = Arc::new(SharedState::new()); | |
| 11 | 34 | let state = BrowserState::new(dir.path(), shared, 44100.0, "Vault").unwrap(); | |
| @@ -2236,6 +2259,19 @@ | |||
| 2236 | 2259 | assert!(state.classifier.head_info.is_none()); | |
| 2237 | 2260 | } | |
| 2238 | 2261 | ||
| 2262 | + | #[test] | |
| 2263 | + | fn bundled_official_layer_is_imported_on_first_run() { | |
| 2264 | + | // Covers the wiring rather than the mechanism (`analysis::official` has | |
| 2265 | + | // the mechanism's tests): opening a fresh vault is what triggers it, and | |
| 2266 | + | // that only happens in the backend's constructor. | |
| 2267 | + | let (mut state, _dir) = make_state_first_run(); | |
| 2268 | + | state.refresh_layers(); | |
| 2269 | + | assert_eq!(state.classifier.layers.len(), 1); | |
| 2270 | + | let layer = &state.classifier.layers[0]; | |
| 2271 | + | assert_eq!(layer.kind, "official"); | |
| 2272 | + | assert!(layer.exemplar_count > 0); | |
| 2273 | + | } | |
| 2274 | + | ||
| 2239 | 2275 | #[test] | |
| 2240 | 2276 | fn classifier_afcl_export_import_round_trip() { | |
| 2241 | 2277 | // Source library: tag two samples, then export to a .afcl file. |
| @@ -968,14 +968,15 @@ | |||
| 968 | 968 | ); | |
| 969 | 969 | } | |
| 970 | 970 | ||
| 971 | - | // Imported layers | |
| 971 | + | // Classifier layers. Not "Imported layers": the bundled official | |
| 972 | + | // layer shows up here too, and the user did not import it. | |
| 972 | 973 | let layers = std::mem::take(&mut state.classifier.layers); | |
| 973 | 974 | if layers.is_empty() { | |
| 974 | 975 | state.classifier.layers = layers; // restore (empty), keep field consistent | |
| 975 | 976 | return; | |
| 976 | 977 | } | |
| 977 | 978 | ui.add_space(theme::space::peer()); | |
| 978 | - | widgets::subsection_label(ui, "Imported layers"); | |
| 979 | + | widgets::subsection_label(ui, "Classifier layers"); | |
| 979 | 980 | ui.label( | |
| 980 | 981 | egui::RichText::new( | |
| 981 | 982 | "Imported rules arrive disabled \u{2014} review them in Tag Rules above before \ | |
| @@ -1062,14 +1063,18 @@ | |||
| 1062 | 1063 | } | |
| 1063 | 1064 | }); | |
| 1064 | 1065 | if confirming { | |
| 1065 | - | ui.label( | |
| 1066 | - | egui::RichText::new( | |
| 1067 | - | "Removing deletes this layer's exemplars and imported rules. You'll \ | |
| 1068 | - | need the .afcl file to add it again.", | |
| 1069 | - | ) | |
| 1070 | - | .small() | |
| 1071 | - | .color(theme::danger()), | |
| 1072 | - | ); | |
| 1066 | + | // The bundled layer has no file behind it, so telling the | |
| 1067 | + | // user to re-import one would send them looking for | |
| 1068 | + | // something that does not exist. It is imported once and | |
| 1069 | + | // stays removed on every device. | |
| 1070 | + | let warning = if layer.kind == "official" { | |
| 1071 | + | "Removing deletes this layer's exemplars. The bundled layer is added \ | |
| 1072 | + | once, so this will not come back." | |
| 1073 | + | } else { | |
| 1074 | + | "Removing deletes this layer's exemplars and imported rules. You'll \ | |
| 1075 | + | need the .afcl file to add it again." | |
| 1076 | + | }; | |
| 1077 | + | ui.label(egui::RichText::new(warning).small().color(theme::danger())); | |
| 1073 | 1078 | } | |
| 1074 | 1079 | } | |
| 1075 | 1080 | state.classifier.layers = layers; |
| @@ -313,6 +313,15 @@ | |||
| 313 | 313 | ||
| 314 | 314 | // Import | |
| 315 | 315 | ||
| 316 | + | /// Parse a `.afcl` JSON string without importing it. | |
| 317 | + | /// | |
| 318 | + | /// Split out so the bundled layer ([`super::official`]) can be parsed and checked | |
| 319 | + | /// on its own, and so it goes through the same parser as a user's file rather | |
| 320 | + | /// than a second copy of it. | |
| 321 | + | pub fn parse(json: &str) -> Result<Afcl> { | |
| 322 | + | serde_json::from_str(json).map_err(|e| CoreError::Serialization(e.to_string())) | |
| 323 | + | } | |
| 324 | + | ||
| 316 | 325 | /// Parse and import a `.afcl` JSON string as a new removable layer. | |
| 317 | 326 | #[instrument(skip_all)] | |
| 318 | 327 | pub fn import_from_string( | |
| @@ -320,8 +329,7 @@ | |||
| 320 | 329 | json: &str, | |
| 321 | 330 | source: Option<&str>, | |
| 322 | 331 | ) -> Result<ImportSummary> { | |
| 323 | - | let afcl: Afcl = | |
| 324 | - | serde_json::from_str(json).map_err(|e| CoreError::Serialization(e.to_string()))?; | |
| 332 | + | let afcl = parse(json)?; | |
| 325 | 333 | import(db, &afcl, source) | |
| 326 | 334 | } | |
| 327 | 335 |