diff --git a/.gitignore b/.gitignore index f26b60a..ebd613a 100644 --- a/.gitignore +++ b/.gitignore @@ -20,6 +20,7 @@ test_audio/* # Wake-word recordings and processed clips — personal voice data, never committed samples/ recordings/ +.prep-manifest.json # Local experiment/training scripts (machine-specific paths), not part of the published project experiments/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 6072ca8..219974b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,12 +11,43 @@ All notable changes to computah are recorded here. The format follows just re-recorded (a `_NNN.wav` clip whose stem this run wrote) from `--output`, so re-recording with fewer utterances no longer strands orphaned higher-numbered clips that training and eval would still read. Off by default - (deletion is opt-in) and deliberately narrow: a source recording, an unrelated - take's clips, or a hand-curated clip whose stem this run did not write is named - but never auto-deleted. Without `--clean` the run still warns and names the - leftover count. `test_prep_wake_samples.py` covers the re-run-with-fewer-clips - case with and without `--clean`, and that `--clean` spares files this run did - not record. + (deletion is opt-in). The source manifest below broadens cleanup to the + prep-owned clips of an intentionally omitted take while still sparing source + recordings and hand-curated audio. With no manifest, the narrow same-stem rule + remains as a warned legacy fallback; any present-but-unusable record fails + closed until removed. Without `--clean` the run still warns and names the + leftover count. `test_prep_wake_samples.py` covers the + re-run-with-fewer-clips case with and without `--clean`, and that source-aware + `--clean` spares files prep did not record. +- `prep_wake_samples.py` output manifest (#84): prep records each resolved source + path and the clips it owns in `.prep-manifest.json` in the output dir. + `--clean` uses that provenance to remove orphans the stem match cannot reach -- + clips from an input dropped between runs, and clips under a disambiguated stem + (`take-1_000.wav`) that a later run no longer produces. Before a destructive + run, prep names every omitted source and how many of its clips `--clean` will + remove. A run must share an exact recorded source path before it may write to + that dataset, so a different dataset with the same label and generic basename + cannot overwrite or clean it. + Malformed ownership maps, including clip names that are paths instead of safe + output filenames or clips claimed by multiple sources, are refused before any + audio is decoded or written. Manifests are read and written explicitly as + UTF-8 so non-ASCII recording paths round-trip across supported platforms. + Manifested stems remain reserved for their source while new output names are + assigned, so a new same-basename recording cannot overwrite the only good + clips from a source whose rerun is silent. A source this run attempted but got + no clips from retains those prior clips. With a readable source map, + hand-curated audio stays outside it and is never deleted. A non-clean legacy + refresh that leaves ambiguous same-stem files does not write a manifest; the + next `--clean` remains on the documented filename fallback instead of turning + a guess into durable provenance. A no-output run cannot claim an unrecorded + directory, and a first-time source that writes no clips cannot gain cleanup + authority over an existing dataset. Manifest writes use atomic replacement. + Only an absent manifest degrades `--clean` to the narrow stem match with an + explicit warning; that fallback cannot distinguish a prep leftover from a + hand-added `_NNN.wav`. An unreadable, malformed, or non-v2 manifest + fails closed until the user removes it to bootstrap ownership explicitly. + Existing pre-manifest directories must bootstrap with the complete source set; + clips under stems omitted from a partial first run remain deliberately unowned. - Configurable request endpointing (#15): the trailing-silence window that ends a captured request and the max-request cap that bounds a runaway are now config keys (`endpoint_silence_ms`, `max_request_ms`, milliseconds) instead of fixed constants, diff --git a/docs/recording-computah.md b/docs/recording-computah.md index 964a831..e1c825c 100644 --- a/docs/recording-computah.md +++ b/docs/recording-computah.md @@ -76,6 +76,18 @@ reports per-file segment counts and a duration summary, and writes 16 kHz mono i WAVs ready for verifier training. `samples/` is gitignored — the recordings are personal data and stay out of version control. +When adding manifests to an existing `samples/` directory, run the first refresh +with the complete source list for that label. A partial bootstrap cannot prove +ownership of legacy clips under omitted stems, so those files remain manual-only +leftovers. Move or clear the old output first if the complete source set is no +longer available. + +Once an output directory has a source manifest, an incremental run must include at +least one recording already listed there along with any new recording. Keep added +microphone takes under the same positive-file glob, or list them beside an existing +source explicitly. With `--clean`, pass every source whose clips the dataset should +keep; omitted sources are treated as intentionally dropped. + ## Training Training runs externally (openWakeWord 0.4.0 ships no train submodule) on a GPU host. diff --git a/docs/wake-threshold-tuning.md b/docs/wake-threshold-tuning.md index 5b624a8..62287d3 100644 --- a/docs/wake-threshold-tuning.md +++ b/docs/wake-threshold-tuning.md @@ -33,14 +33,67 @@ repo. The evaluation script below is committed; the audio it reads is not. Re-running prep into a populated `samples/` dir refreshes it in place. If a re-recording yields fewer clips than last time, the extra clips from the old run would linger and the training globs would still read them, so prep warns and -names the count. Pass `--clean` to remove those leftovers in the same run. It is -deliberately narrow: it only deletes leftover clips of a take this run -re-recorded (a `_NNN.wav` clip whose stem the run wrote), so a source -recording or a hand-curated clip for a different stem is named but left in place. +names the count. Pass `--clean` to remove those leftovers in the same run. + +With a readable source-aware manifest, `--clean` only deletes clips prep recorded +as its output. It removes the now-unused higher-numbered clips of a take this run +re-recorded, and the clips a previous run recorded for a take that is no longer +in the inputs. The `.prep-manifest.json` in the output dir maps each resolved +source path to the clips prep wrote for it. A source recording, a hand-curated +clip, and anything else absent from that map is named but left in place. +Recorded output stems also remain reserved for their source while the next run +assigns names. A new same-basename recording therefore cannot overwrite another +source's manifested clips before `--clean` has a chance to apply its ownership +rules. + +That record proves prep made a file, not that the file belongs to what you are +refreshing. A `--label` that disagrees with the record already in the directory +is refused outright, before anything is decoded: that is a mistyped `--output`, +and waiting until `--clean` would be too late, since the run would already have +overwritten that dataset's clips. Past that, a run must contain at least one +exact source path from the record before it may delete from the dataset or write +to it. A shared basename is not ownership. A directory with no record yet counts +as yours. If a non-clean legacy refresh leaves same-stem files behind, prep does +not write a manifest because it cannot prove whether those files are old prep +output or audio curated by hand. The follow-up `--clean` stays on the legacy +filename rule rather than recording that guess as provenance. A run that +produces no clips does not claim an unrecorded directory. Prep also spares the +prior clips of a source it tried and got nothing from (a silent or bad +re-recording): those are the only copy, and it will not trade them for a run +that produced nothing. A new source that produces no clips is not added to the +ownership record, so it cannot authorize a later cleanup until a successful run +actually contributes audio. + +When upgrading an existing pre-manifest `samples/` directory, bootstrap it with +the complete source list for that label. A partial first run can record only the +sources it receives; legacy clips under omitted stems then remain deliberately +unowned and future `--clean` runs will only warn about them. If the complete +source set is unavailable, move or clear the old output before starting a new +partial dataset. + +To add a new recording to an existing dataset without `--clean`, include at +least one source already recorded there in the same invocation. This gives prep +explicit ownership proof while it adds the new source to the manifest. If you +use `--clean`, include every source whose clips the dataset should keep: omitted +sources are treated as intentionally dropped and their prep-owned clips are +removed. Before decoding, prep warns with each omitted source path and the +number of its clips scheduled for removal. A safe incremental workflow is to add +recordings without `--clean`, then run a full-input `--clean` pass after +reviewing the complete source list. + +Manifest updates use atomic replacement. Only an absent manifest falls back to +the narrow same-stem cleanup rule, with a warning that source ownership +protection is unavailable. That fallback has no provenance: `--clean` can remove +any `_NNN.wav` file, including a hand-added file with that shape. +Check the warning list and move any such file before cleaning. An unreadable, +malformed, or non-v2 manifest fails closed before any decode or write because +neither its dataset label nor its source ownership can be trusted. After +reviewing the output directory, remove that manifest and rerun the full source +set to bootstrap ownership. ```bash -.venv/bin/python prep_wake_samples.py --input computah_normal.wav \ - --output samples/positive --label positive --clean +.venv/bin/python prep_wake_samples.py --input computah_normal.wav computah_styles.wav \ + computah_distance.wav --output samples/positive --label positive --clean ``` ## 2. Run the sweep diff --git a/prep_wake_samples.py b/prep_wake_samples.py index 9b0cf99..fb5080c 100644 --- a/prep_wake_samples.py +++ b/prep_wake_samples.py @@ -11,6 +11,9 @@ counts and a duration summary so a bad take (clipped words, no gaps, silence) is obvious before training. +Successful runs also write `.prep-manifest.json` in `--output`; it records the +dataset label and exact source ownership used to guard later refreshes and cleanup. + Recordings are personal data: write them under samples/ (gitignored), not into the tree. @@ -24,6 +27,8 @@ from __future__ import annotations import argparse +import json +import os import re import shutil import subprocess @@ -40,9 +45,9 @@ TARGET_SR = 16000 AUDIO_EXTS = {".wav", ".flac", ".ogg", ".m4a", ".mp3", ".aac", ".mp4", ".wma"} # prep writes each clip as "_NNN.wav" (see _write_unique; NNN is >=3 digits, -# since :03d is a minimum width). --clean pairs this shape with the current run's -# stems so it only removes a re-recorded take's own leftover clips, never a source -# take, an unrelated take's clips, or hand-added audio a user left in the output dir. +# since :03d is a minimum width). Without source ownership, --clean uses this +# shape plus the current run's stems as its narrow legacy fallback. With a v2 +# manifest, exact recorded clip names authorize cleanup instead. _CLIP_NAME = re.compile(r"(.+)_\d{3,}\.wav$") @@ -323,12 +328,248 @@ def _summarize(paths: list[Path], limit: int = 3) -> str: return shown +MANIFEST_NAME = ".prep-manifest.json" + + +def _source_key(path: Path) -> str: + """A stable local identity for one source recording.""" + return os.path.normcase(str(path.resolve())) + + +def _read_manifest( + out_dir: Path, +) -> tuple[str | None, set[str], dict[str, set[str]] | None]: + """The label, clips, and source ownership a previous prep run recorded here. + + An absent file returns ``(None, set(), None)`` and lets the caller use the + warned legacy stem rule. Every present-but-unreadable, malformed, or + unsupported record returns no source map; the caller sees that the file + exists and refuses to decode or write until ownership is bootstrapped + explicitly. + """ + manifest_path = out_dir / MANIFEST_NAME + + def warn_manifest_problem(reason: object) -> None: + print( + f"warning: could not use {manifest_path} ({reason}); source ownership " + "protection is unavailable, so this manifest cannot authorize a write", + file=sys.stderr, + ) + + try: + raw = json.loads(manifest_path.read_text(encoding="utf-8")) + except FileNotFoundError: + return None, set(), None + except (OSError, ValueError) as e: + warn_manifest_problem(e) + return None, set(), None + if not isinstance(raw, dict): + warn_manifest_problem("expected a JSON object") + return None, set(), None + clips = raw.get("clips") + if not isinstance(clips, list): + warn_manifest_problem("expected a clips list") + return None, set(), None + label = raw.get("label") + if not isinstance(label, str): + warn_manifest_problem("expected a string label") + return None, set(), None + + def safe_clip_name(name: object) -> bool: + return ( + isinstance(name, str) + and name == Path(name).name + and "/" not in name + and "\x00" not in name + and _CLIP_NAME.fullmatch(name) is not None + ) + + if not all(safe_clip_name(name) for name in clips): + warn_manifest_problem("clips must contain safe output filenames, not paths") + return label, set(), None + clip_names = set(clips) + version = raw.get("version") + if version != 2: + warn_manifest_problem(f"unsupported manifest version {version!r}; expected 2") + return label, clip_names, None + sources_raw = raw.get("sources") + if not isinstance(sources_raw, dict): + warn_manifest_problem("expected a sources map") + return label, clip_names, None + + sources: dict[str, set[str]] = {} + claimed_clips: set[str] = set() + for source, names in sources_raw.items(): + if not isinstance(source, str) or not isinstance(names, list): + warn_manifest_problem( + "expected every sources entry to map a path to a clip list" + ) + return label, clip_names, None + if not all(safe_clip_name(name) for name in names): + warn_manifest_problem("sources must own safe output filenames, not paths") + return label, clip_names, None + source_clips = set(names) + if claimed_clips & source_clips: + warn_manifest_problem( + "each recorded clip must have exactly one source owner" + ) + return label, clip_names, None + sources[source] = source_clips + claimed_clips.update(source_clips) + if claimed_clips != clip_names: + warn_manifest_problem("sources do not own exactly the recorded clips") + return label, clip_names, None + return label, clip_names, sources + + +def _write_manifest( + out_dir: Path, label: str, source_clips: dict[str, set[str]] +) -> None: + """Record each source and the clips it owns, for a later --clean run. + + Written after the clips are on disk, and a failure only warns: the clips are + the output that matters, and losing the manifest costs precision on a future + --clean, not correctness. A run that wrote nothing into a directory that does + not exist has nothing to record and does not create one. + + The caller verifies ownership and assembles the source map; this only writes. + """ + if not out_dir.is_dir(): + return + clips = set().union(*source_clips.values()) if source_clips else set() + payload = ( + json.dumps( + { + "version": 2, + "label": label, + "clips": sorted(clips), + "sources": { + source: sorted(names) + for source, names in sorted(source_clips.items()) + }, + }, + indent=2, + ) + + "\n" + ) + temporary: Path | None = None + try: + with tempfile.NamedTemporaryFile( + mode="w", + encoding="utf-8", + dir=out_dir, + prefix=f".{MANIFEST_NAME}.", + suffix=".tmp", + delete=False, + ) as tmp: + temporary = Path(tmp.name) + tmp.write(payload) + tmp.flush() + os.fsync(tmp.fileno()) + os.replace(temporary, out_dir / MANIFEST_NAME) + except OSError as e: + manifest_path = out_dir / MANIFEST_NAME + if manifest_path.exists(): + recovery = ( + "the previous manifest remains in force and clips written by this " + "run are unrecorded. Fix the write problem and rerun the full input " + f"set, or remove {manifest_path} to bootstrap ownership again" + ) + else: + recovery = ( + "no manifest was recorded, so clips written by this run have no " + "source ownership protection. Fix the write problem and rerun the " + "full input set" + ) + print( + f"warning: could not write {manifest_path} ({e}); {recovery}", + file=sys.stderr, + ) + finally: + if temporary is not None: + try: + temporary.unlink(missing_ok=True) + except OSError: + pass + + def _clip_stem(name: str) -> str | None: """The stem of a "_NNN.wav" clip name, or None if it isn't one.""" m = _CLIP_NAME.fullmatch(name) return m.group(1) if m else None +def _manifest_aware_stems( + files: list[Path], + source_keys: list[str], + prior_sources: dict[str, set[str]] | None, +) -> list[str]: + """Assign output stems without taking names owned by another source. + + The first run uses the path-stable basename rule. Once a source manifest + exists, a returning source keeps one of its recorded stems when possible, + and every prior stem stays reserved from other sources. That reservation is + needed before any clip is written: cleanup cannot save a silent source's + only good clips after a newcomer has already overwritten their filenames. + """ + generated = _unique_stems(files) + if prior_sources is None: + return generated + + source_stems: dict[str, set[str]] = {} + owners_by_stem: dict[str, set[str]] = {} + for source, names in prior_sources.items(): + stems = {stem for name in names if (stem := _clip_stem(name)) is not None} + source_stems[source] = stems + for stem in stems: + owners_by_stem.setdefault(stem, set()).add(source) + + assigned: dict[int, str] = {} + used: set[str] = set() + + # Anchor returning sources to a stem they already own. Prefer the stem the + # current basename rule would choose, then a sole prior stem. A malformed + # record in which two sources share a stem is never used as an anchor. + for i in sorted(range(len(files)), key=lambda j: str(files[j].resolve())): + source = source_keys[i] + owned = source_stems.get(source, set()) + preferred: str | None = None + if generated[i] in owned and owners_by_stem.get(generated[i]) == {source}: + preferred = generated[i] + elif len(owned) == 1: + candidate = next(iter(owned)) + if owners_by_stem.get(candidate) == {source}: + preferred = candidate + if preferred is not None and preferred not in used: + assigned[i] = preferred + used.add(preferred) + + # Keep the ordinary mapping for inputs whose candidate has no recorded + # owner. Anchors run first so a newcomer cannot claim a returning source's + # prior stem merely because it appeared earlier in the invocation. + for i in sorted(range(len(files)), key=lambda j: str(files[j].resolve())): + if i in assigned: + continue + candidate = generated[i] + if candidate not in owners_by_stem and candidate not in used: + assigned[i] = candidate + used.add(candidate) + + # Any remaining candidate collides with recorded ownership. Give it a fresh + # deterministic suffix, avoiding both prior stems and raw basenames that a + # different pending input may need. + pending = [i for i in range(len(files)) if i not in assigned] + unavailable = set(owners_by_stem) | used | {files[i].stem for i in pending} + for i in sorted(pending, key=lambda j: (files[j].stem, str(files[j].resolve()))): + n = 1 + while f"{files[i].stem}-{n}" in unavailable: + n += 1 + assigned[i] = f"{files[i].stem}-{n}" + unavailable.add(assigned[i]) + + return [assigned[i] for i in range(len(files))] + + def process( inputs: list[Path], out_dir: Path, @@ -340,11 +581,15 @@ def process( ) -> int: """Process every input file; return the total clip count written. - With ``clean`` set, a re-recorded take's leftover clips (a prior run's - now-unused higher-numbered clips for a stem this run wrote) are removed after - this run writes, so its own count matches what is on disk. It stays narrow on - purpose: an unrelated take's clips, a source recording, or hand-added audio is - named but never deleted, so a stray ``--output`` never silently wipes files. + With ``clean`` set, prep's own leftover clips are removed after this run + writes, so its count matches what is on disk. Two kinds qualify: a + re-recorded take's now-unused higher-numbered clips (matched by stem), and + clips a previous run recorded for a take that is not in this run's inputs at + all (matched against ``MANIFEST_NAME``, and only when this run contains a + source path recorded for that dataset). Everything else is named but never + deleted -- a source recording, hand-added audio, another dataset a stray + ``--output`` landed in, and the prior clips of a take this run attempted but + wrote nothing for. """ # Checked before any audio is decoded, so an occupied --output is named once # rather than surfacing as whichever exception the run happens to reach first: @@ -358,28 +603,100 @@ def process( ) return 0 + # Refused before any audio is decoded, because by the time --clean is + # evaluated the damage is already done: _write_unique would have overwritten + # this directory's take_000.wav with the new dataset's. A label mismatch is + # the signature of a mistyped --output, and there is no reading of it where + # writing positives into a directory of negatives is what the user meant. + manifest_present = (out_dir / MANIFEST_NAME).exists() + prior_label, _prior_clips, prior_sources = _read_manifest(out_dir) + if prior_label is not None and prior_label != label: + print( + f"error: --output {out_dir} holds {prior_label!r} clips, not {label!r}; " + f"nothing was written. Point --output at this run's directory, or clear " + f"that one (or remove its {MANIFEST_NAME}) to reuse it", + file=sys.stderr, + ) + return 0 + if manifest_present and prior_sources is None: + print( + f"error: --output {out_dir} has an unusable {MANIFEST_NAME}, so this " + "run cannot prove its label or source ownership; nothing was written. " + f"After reviewing the directory, remove {out_dir / MANIFEST_NAME} to " + "bootstrap ownership from the full input set, or clear the directory " + "first", + file=sys.stderr, + ) + return 0 + if ( + clean + and prior_sources is None + and out_dir.is_dir() + and any(path.suffix.lower() in AUDIO_EXTS for path in out_dir.iterdir()) + ): + print( + f"warning: {out_dir} has no {MANIFEST_NAME}; this --clean will use " + "legacy filename cleanup without source ownership protection and may " + "remove hand-added _NNN.wav files", + file=sys.stderr, + ) + files = _inputs(inputs) if not files: joined = ", ".join(str(p) for p in inputs) print(f"no audio files found at {joined}", file=sys.stderr) return 0 - stems = _unique_stems(files) + source_keys = [_source_key(f) for f in files] + # This guards writes as well as --clean: _write_unique intentionally refreshes + # prior filenames in place, so a different source with the same basename could + # overwrite take_000.wav before cleanup runs. Add a new source alongside one + # recorded source in the same invocation to prove which dataset owns the dir. + if prior_sources is not None and not (set(source_keys) & prior_sources.keys()): + recorded = sorted(prior_sources) + preview = ", ".join(repr(source) for source in recorded[:2]) + if len(recorded) > 2: + preview += f", +{len(recorded) - 2} more" + print( + f"error: --output {out_dir} belongs to a {label!r} dataset with " + f"different source recordings; nothing was written. Recorded sources " + f"include {preview}. Point --output at this run's directory, or clear " + f"that one (or remove its {MANIFEST_NAME}) to reuse it", + file=sys.stderr, + ) + return 0 + if clean and prior_sources is not None: + present_names = {path.name for path in _stale_clips(out_dir, set())} + for source in sorted(prior_sources.keys() - set(source_keys)): + count = len(prior_sources[source] & present_names) + if count: + print( + f"warning: source {source!r} is not in this run; --clean will " + f"remove its {count} prep-owned clip(s) from {out_dir}", + file=sys.stderr, + ) + + stems = _manifest_aware_stems(files, source_keys, prior_sources) written: set[Path] = set() + written_by_source: dict[str, set[Path]] = {source: set() for source in source_keys} seen_inodes: set[int] = set() all_durs: list[float] = [] - for f, stem in zip(files, stems): + for f, stem, source in zip(files, stems, source_keys): audio = load_mono_16k(f) dur = len(audio) / TARGET_SR if label == "background": # Continuous negative audio: keep it whole, just normalized. - written.add(_write_unique(out_dir, stem, 0, audio, seen_inodes)) + path = _write_unique(out_dir, stem, 0, audio, seen_inodes) + written.add(path) + written_by_source[source].add(path) print(f" {f.name}: {dur:.1f}s background -> 1 file") continue clips, stats = segment_on_silence(audio, min_gap_s, min_dur_s, max_dur_s) for i, clip in enumerate(clips): - written.add(_write_unique(out_dir, stem, i, clip, seen_inodes)) + path = _write_unique(out_dir, stem, i, clip, seen_inodes) + written.add(path) + written_by_source[source].add(path) all_durs.append(len(clip) / TARGET_SR) note = "" if stats["too_short"] or stats["too_long"]: @@ -391,50 +708,211 @@ def process( total = len(written) print(f"\nwrote {total} {label} clip(s) to {out_dir}") - # Files this run did not write are left from an earlier run over a different - # input set. Training and eval read every clip in the directory, so they - # would score as extra data nobody asked for. --clean removes the leftover - # clips of a take this run just re-recorded (same stem, now-unused index), so - # the reported count matches what is on disk. It is deliberately narrow: a - # clip whose stem this run did not write -- an unrelated take, a source - # recording, hand-added audio -- is named but never auto-deleted, because it - # is indistinguishable from data the user curated on purpose. + # Files this run did not write are left from an earlier input set. Once a + # manifest exists, cleanup is based on source ownership rather than inferred + # from output stems: prior clips belonging to a source this run successfully + # re-recorded, or to a source dropped from this run, are removable. Prior clips + # belonging to a source this run attempted but got no clips from are kept as + # the only good copy. Files absent from the source map are user-owned. # - # Scope by the stems this run actually WROTE a clip for, read back from - # `written`, not by the input list `stems`: an input that produced zero clips - # this run (silence, a bad re-recording, thresholds that dropped every - # segment) has no new clip to make its earlier good clips stale, so --clean - # must not delete them. Keying off written paths leaves them to linger and - # warn instead of wiping the only copy. + # Without a readable manifest, the older stem rule remains the narrow + # fallback. It can clean a re-recorded take's higher-numbered leftovers but + # cannot reach a dropped input safely. run_stems = { stem for stem in (_clip_stem(p.name) for p in written) if stem is not None } + current_sources = set(source_keys) + written_sources = {source for source, paths in written_by_source.items() if paths} stale = _stale_clips(out_dir, written) - if stale: - removable = ( - [p for p in stale if _clip_stem(p.name) in run_stems] if clean else [] - ) - for p in removable: - p.unlink() - if removable: - print( - f"removed {len(removable)} clip(s) left from an earlier run " - f"({_summarize(removable)})" - ) - lingering = [p for p in stale if p not in removable] - if lingering: - fix = ( - "these are not clips this run re-recorded, so --clean leaves them; " - "remove them by hand" - if clean - else "rerun with --clean to remove them, or clear the directory by hand" + removable: list[Path] = [] + if clean: + if prior_sources is None: + removable = [p for p in stale if _clip_stem(p.name) in run_stems] + else: + replaceable_sources = { + source + for source in prior_sources + if source not in current_sources or source in written_sources + } + removable_names = set().union( + *(prior_sources[source] for source in replaceable_sources) ) + removable = [p for p in stale if p.name in removable_names] + removed: list[Path] = [] + for path in removable: + try: + path.unlink() + except OSError as e: print( - f"warning: {len(lingering)} file(s) in {out_dir} are left from an " - f"earlier run ({_summarize(lingering)}). Training and eval read " - f"every clip in this directory, so {fix}.", + f"warning: could not remove {path} ({e}); leaving it recorded for " + "a later --clean", file=sys.stderr, ) + else: + removed.append(path) + if removed: + print( + f"removed {len(removed)} clip(s) left from an earlier run " + f"({_summarize(removed)})" + ) + lingering = [p for p in stale if p not in removed] + lingering_names = {p.name for p in lingering} + ambiguous_legacy = prior_sources is None and any( + _clip_stem(path.name) in run_stems for path in lingering + ) + + # Carry forward every still-present owned clip under its original source, + # then replace or add the clips written for current sources. A no-manifest + # run cannot claim same-stem leftovers: they may be old prep output or audio + # curated by hand. Leave the record absent until legacy --clean resolves + # that ambiguity, so v2 never turns an inferred filename match into durable + # provenance. + next_sources: dict[str, set[str]] = {} + if prior_sources is not None: + for source, names in prior_sources.items(): + kept = names & lingering_names + if kept: + next_sources[source] = kept + # Keep an already-owned current source in the record even when it wrote + # nothing. If its old clips are already absent, the empty entry still proves + # the next rerun comes from the same source instead of deadlocking the + # directory. A brand-new source earns no cleanup authority until it actually + # contributes a clip. + for source, paths in written_by_source.items(): + if not paths and (prior_sources is None or source not in prior_sources): + continue + next_sources.setdefault(source, set()).update(path.name for path in paths) + + # A no-output run cannot establish ownership in an unrecorded directory. + # Leaving it unclaimed lets a later successful rerun bootstrap the legacy + # clips instead of stranding them behind an empty manifest. + if (prior_sources is not None or written) and not ambiguous_legacy: + _write_manifest(out_dir, label, next_sources) + elif ambiguous_legacy: + print( + f"warning: {MANIFEST_NAME} was not written because same-stem legacy " + "files remain and prep cannot prove whether it created them; review " + "the warning below before rerunning with --clean", + file=sys.stderr, + ) + + if lingering: + # Why a file lingered decides what the user should do about it, and the + # cases want opposite advice. A take this run re-recorded and got nothing + # from kept its previous clips because they are the only copy -- telling + # that user to delete them by hand would undo the guard and lose the + # recording. Split the message rather than send one that is right for the + # common case and harmful for the case prep went out of its way to + # protect. + if prior_sources is None: + attempted = set(stems) + empty_takes = [ + p for p in lingering if _clip_stem(p.name) in attempted - run_stems + ] + else: + silent_names = set().union( + *( + prior_sources[source] + for source in current_sources - written_sources + if source in prior_sources + ) + ) + empty_takes = [p for p in lingering if p.name in silent_names] + owned_names = ( + set().union(*prior_sources.values()) if prior_sources is not None else set() + ) + failed_removals = [p for p in lingering if p in removable] + if empty_takes: + other_lingering = [ + p + for p in lingering + if p not in empty_takes and p not in failed_removals + ] + fix = ( + f"{_summarize(empty_takes)} are prior clips for a source this run " + "re-recorded but got no clips from, so prep kept them rather " + "than leave you with none; check the recording before removing " + "anything" + ) + if other_lingering: + if clean: + fix += ( + f". {_summarize(other_lingering)} have no prep ownership " + "record, so --clean leaves them; remove them by hand if " + "they are not wanted" + ) + else: + cleanable = [ + p + for p in other_lingering + if ( + p.name in owned_names + if prior_sources is not None + else _clip_stem(p.name) in run_stems + ) + ] + manual = [p for p in other_lingering if p not in cleanable] + if cleanable: + fix += ( + f". Rerun with --clean to remove prep-owned " + f"{_summarize(cleanable)}" + ) + if manual: + fix += ( + f". {_summarize(manual)} have no prep ownership record; " + "remove only those files by hand if they are not wanted" + ) + if failed_removals: + fix += ( + f". {_summarize(failed_removals)} were selected for cleanup " + "but could not be removed; address the earlier errors and rerun " + "--clean" + ) + elif clean: + unrecorded = [p for p in lingering if p not in failed_removals] + fixes: list[str] = [] + if unrecorded: + fixes.append( + "prep has no record of creating " + f"{_summarize(unrecorded)}, so --clean leaves them; remove " + "them by hand if they are not wanted" + ) + if failed_removals: + fixes.append( + f"{_summarize(failed_removals)} were selected for cleanup " + "but could not be removed; address the earlier errors and " + "rerun --clean" + ) + fix = ". ".join(fixes) + else: + if prior_sources is None: + fix = ( + "review these legacy filenames, then rerun with --clean or " + "remove only the unwanted files by hand" + ) + else: + cleanable = [p for p in lingering if p.name in owned_names] + manual = [p for p in lingering if p not in cleanable] + fixes = [] + if cleanable: + fixes.append( + f"rerun with --clean to remove prep-owned " + f"{_summarize(cleanable)}" + ) + if manual: + fixes.append( + f"{_summarize(manual)} have no prep ownership record; " + "remove them by hand if they are not wanted" + ) + fix = ". ".join(fixes) + print( + f"warning: {len(lingering)} file(s) in {out_dir} are left from an " + f"earlier run ({_summarize(lingering)}). Training and eval read every " + f"clip in this directory, so they count as data nobody asked for: " + f"{fix}.", + file=sys.stderr, + ) + if all_durs: a = np.array(all_durs) print( @@ -454,7 +932,11 @@ def main() -> int: nargs="+", help="audio file(s), a shell glob, or a folder of them", ) - p.add_argument("--output", required=True, help="output directory for clips") + p.add_argument( + "--output", + required=True, + help=f"output directory for clips and the {MANIFEST_NAME} ownership record", + ) p.add_argument( "--label", choices=("positive", "negative", "background"), @@ -476,9 +958,10 @@ def main() -> int: p.add_argument( "--clean", action="store_true", - help="remove a re-recorded take's leftover clips in --output (a prior run's " - "now-unused higher-numbered clips for a take in this run); leaves unrelated " - "or hand-added audio in place", + help="remove prep's own leftover clips in --output: a re-recorded take's " + "now-unused higher-numbered clips, plus clips a previous run recorded for a " + "take no longer in the inputs; when no manifest exists, falls back to " + "same-stem filenames, while an unusable manifest fails closed", ) args = p.parse_args() diff --git a/test_prep_wake_samples.py b/test_prep_wake_samples.py index bb3ea33..6994416 100644 --- a/test_prep_wake_samples.py +++ b/test_prep_wake_samples.py @@ -8,8 +8,12 @@ from __future__ import annotations +import io +import json +import os import sys import tempfile +from contextlib import redirect_stderr from pathlib import Path import numpy as np @@ -456,7 +460,12 @@ def test_refresh_after_adding_collider_leaves_no_orphans(d: Path) -> None: out = d / "refresh_out" prep.process([a / "computah.wav"], out, "positive", 0.3, 0.2, 3.0) prep.process( - [a / "computah.wav", b / "computah.wav"], out, "positive", 0.3, 0.2, 3.0 + [a / "computah.wav", b / "computah.wav"], + out, + "positive", + 0.3, + 0.2, + 3.0, ) files = sorted(p.name for p in out.glob("*.wav")) @@ -521,6 +530,1058 @@ def test_stem_assignment_ignores_input_order(d: Path) -> None: ) +def _burst_take(path: Path, n: int) -> None: + """Write a take of `n` well-separated utterances at `path`.""" + path.parent.mkdir(parents=True, exist_ok=True) + sf.write( + path, make_bursts(n, 0.5, 0.8, prep.TARGET_SR), prep.TARGET_SR, subtype="PCM_16" + ) + + +def test_manifest_cleans_dropped_input_orphans(d: Path) -> None: + """--clean removes the orphans of an input dropped between runs (#84 case 1). + + The stem rule alone cannot reach these: `other` is not in the rerun at all, + so no clip this run writes ever makes its old ones stale. The manifest is + what distinguishes them from audio a user put in the directory. + """ + src = d / "dropped_src" + _burst_take(src / "a" / "take.wav", 3) + _burst_take(src / "b" / "other.wav", 2) + out = d / "dropped_out" + prep.process( + [src / "a" / "take.wav", src / "b" / "other.wav"], + out, + "positive", + 0.3, + 0.2, + 3.0, + ) + check( + (out / prep.MANIFEST_NAME).is_file(), + "a run records what it wrote in the manifest", + ) + + errors = io.StringIO() + with redirect_stderr(errors): + prep.process( + [src / "a" / "take.wav"], + out, + "positive", + 0.3, + 0.2, + 3.0, + clean=True, + ) + present = sorted(p.name for p in out.glob("*.wav")) + check( + present == ["take_000.wav", "take_001.wav", "take_002.wav"], + f"--clean removes a dropped input's orphans ({present})", + ) + check( + prep._source_key(src / "b" / "other.wav") in errors.getvalue() + and "will remove" in errors.getvalue() + and "2 prep-owned clip" in errors.getvalue(), + "a dropped source and its deletion count are warned before cleanup", + ) + + +def test_manifest_cleans_changed_collider_stem_orphans(d: Path) -> None: + """--clean removes a dropped collider without renaming the survivor. + + Two `take.wav` inputs become stems `take` and `take-1`. Rerunning only the + second source keeps its manifested `take-1` stem so its refresh cannot take + another source's filenames; the manifest removes the dropped first source's + bare-stem clips. + """ + src = d / "collider_src" + _burst_take(src / "a" / "take.wav", 3) + _burst_take(src / "b" / "take.wav", 2) + out = d / "collider_out" + prep.process( + [src / "a" / "take.wav", src / "b" / "take.wav"], out, "positive", 0.3, 0.2, 3.0 + ) + first = sorted(p.name for p in out.glob("*.wav")) + check( + any(n.startswith("take-1_") for n in first), + f"the collider run writes a disambiguated stem ({first})", + ) + + prep.process([src / "b" / "take.wav"], out, "positive", 0.3, 0.2, 3.0, clean=True) + present = sorted(p.name for p in out.glob("*.wav")) + check( + present == ["take-1_000.wav", "take-1_001.wav"], + f"--clean drops the omitted collider without renaming the survivor ({present})", + ) + + +def test_manifest_tracks_a_silent_colliding_source_by_path(d: Path) -> None: + """A silent rerun keeps its own prior clips even when its stem changes. + + Two same-basename inputs need source-level ownership: when only the second + source returns and produces no clips, its old disambiguated clips are the + copy to preserve, while the dropped first source's clips can be cleaned. + Stem membership alone cannot tell those cases apart. + """ + src = d / "silent_collider_src" + first = src / "a" / "take.wav" + silent = src / "b" / "take.wav" + _burst_take(first, 3) + _burst_take(silent, 2) + out = d / "silent_collider_out" + prep.process([first, silent], out, "positive", 0.3, 0.2, 3.0) + + _label, _clips, sources = prep._read_manifest(out) + source_keys = {prep._source_key(first), prep._source_key(silent)} + check( + sources is not None and set(sources) == source_keys, + "the manifest records exact source ownership", + ) + silent_clips = sources[prep._source_key(silent)] if sources is not None else set() + + total = prep.process([silent], out, "positive", 0.3, 5.0, 10.0, clean=True) + present = sorted(p.name for p in out.glob("*.wav")) + check( + total == 0 and present == sorted(silent_clips), + f"a silent colliding source keeps its clips while the dropped source cleans " + f"({present})", + ) + + +def test_new_collider_cannot_overwrite_a_silent_sources_prior_stem(d: Path) -> None: + """A new collider must not take a silent source's manifested filenames. + + Cleanup is too late to protect the only good clips if another input has + already refreshed their paths in place. Manifested stems therefore stay + reserved for their source while output names are assigned. + """ + src = d / "stem_theft_src" + first = src / "a" / "take.wav" + silent = src / "b" / "take.wav" + newcomer = src / "c" / "take.wav" + _burst_take(first, 3) + _burst_take(silent, 2) + out = d / "stem_theft_out" + prep.process([first, silent], out, "positive", 0.3, 0.2, 3.0) + + _label, _clips, sources = prep._read_manifest(out) + silent_key = prep._source_key(silent) + silent_names = sources[silent_key] if sources is not None else set() + silent_bytes = {name: (out / name).read_bytes() for name in silent_names} + + sf.write( + silent, + np.ones(int(prep.TARGET_SR * 4.0), dtype=np.float32) * 0.5, + prep.TARGET_SR, + subtype="PCM_16", + ) + _burst_take(newcomer, 3) + prep.process([silent, newcomer], out, "positive", 0.3, 0.2, 1.0, clean=True) + _label, _clips, refreshed_sources = prep._read_manifest(out) + present = sorted(p.name for p in out.glob("*.wav")) + preserved = all( + (out / name).exists() and (out / name).read_bytes() == payload + for name, payload in silent_bytes.items() + ) + check( + preserved + and refreshed_sources is not None + and refreshed_sources.get(silent_key) == silent_names, + f"a newcomer cannot overwrite a silent source's only good clips ({present})", + ) + + +def test_manifest_never_authorizes_deleting_curated_audio(d: Path) -> None: + """The manifest widens --clean only to files prep recorded writing. + + A hand-added clip whose stem no run ever wrote is exactly the shape the new + rule reaches for -- an orphan of an unattempted stem -- so this pins that it + is still spared. Being absent from the manifest is the whole guarantee. + """ + src = d / "curated_src" + _burst_take(src / "take.wav", 3) + out = d / "curated_out" + prep.process([src / "take.wav"], out, "positive", 0.3, 0.2, 3.0) + _burst_take(out / "custom_001.wav", 1) + _burst_take(out / "my_recording.wav", 1) + + errors = io.StringIO() + with redirect_stderr(errors): + prep.process([src / "take.wav"], out, "positive", 0.3, 0.2, 3.0) + check( + "custom_001.wav" in errors.getvalue() + and "remove" in errors.getvalue() + and "by hand" in errors.getvalue() + and "rerun with --clean to remove them" not in errors.getvalue(), + "a non-clean refresh gives manual-only guidance for curated audio", + ) + + prep.process([src / "take.wav"], out, "positive", 0.3, 0.2, 3.0, clean=True) + present = sorted(p.name for p in out.glob("*.wav")) + check( + "custom_001.wav" in present and "my_recording.wav" in present, + f"--clean spares audio prep did not create ({present})", + ) + + +def test_manifest_spares_prior_clips_when_rerun_writes_nothing(d: Path) -> None: + """The manifest must not reopen the silent-re-recording data loss. + + Those clips ARE in the manifest, so a rule that removed every manifested + clip the run did not rewrite would delete the only copy of a take whose + re-recording came back silent. The stem has to be excluded because the run + attempted it, regardless of what the manifest says. + """ + src = d / "silent_src" + take = src / "take.wav" + _burst_take(take, 3) + out = d / "silent_out" + prep.process([take], out, "positive", 0.3, 0.2, 3.0) + + _burst_take(take, 3) + second = prep.process([take], out, "positive", 0.3, 5.0, 10.0, clean=True) + present = sorted(p.name for p in out.glob("*.wav")) + check( + second == 0 and present == ["take_000.wav", "take_001.wav", "take_002.wav"], + f"a manifested clip is still spared when its take wrote nothing ({present})", + ) + + +def test_nonclean_silent_rerun_protects_only_copy_in_warning(d: Path) -> None: + """A warning must not invite deletion of a silent source's prior clips.""" + src = d / "nonclean_silent_src" + take = src / "take.wav" + _burst_take(take, 3) + out = d / "nonclean_silent_out" + prep.process([take], out, "positive", 0.3, 0.2, 3.0) + + sf.write( + take, + np.ones(int(prep.TARGET_SR * 4.0), dtype=np.float32) * 0.5, + prep.TARGET_SR, + subtype="PCM_16", + ) + errors = io.StringIO() + with redirect_stderr(errors): + prep.process([take], out, "positive", 0.3, 0.2, 1.0) + present = sorted(p.name for p in out.glob("*.wav")) + check( + present == ["take_000.wav", "take_001.wav", "take_002.wav"] + and "check the recording" in errors.getvalue() + and "clear the directory" not in errors.getvalue() + and "rerun with --clean" not in errors.getvalue(), + f"a non-clean silent rerun protects its only-copy clips in guidance ({present})", + ) + + +def test_legacy_mixed_warning_keeps_cleanable_leftovers_automated(d: Path) -> None: + """Legacy leftovers beside a silent take remain --clean-removable.""" + src = d / "legacy_mixed_src" + good = src / "good.wav" + silent = src / "silent.wav" + _burst_take(good, 2) + _burst_take(silent, 2) + out = d / "legacy_mixed_out" + prep.process([good, silent], out, "positive", 0.3, 0.2, 3.0) + (out / prep.MANIFEST_NAME).unlink() + + _burst_take(good, 1) + sf.write( + silent, + np.ones(int(prep.TARGET_SR * 4.0), dtype=np.float32) * 0.5, + prep.TARGET_SR, + subtype="PCM_16", + ) + errors = io.StringIO() + with redirect_stderr(errors): + prep.process([good, silent], out, "positive", 0.3, 0.2, 1.0) + check( + "check the recording" in errors.getvalue() + and "Rerun with --clean to remove prep-owned good_001.wav" in errors.getvalue(), + "legacy mixed guidance protects silent clips and automates true leftovers", + ) + + +def test_manifest_empty_source_remains_refreshable(d: Path) -> None: + """An owned source with no clips must not deadlock its output directory.""" + src = d / "empty_source_src" + take = src / "take.wav" + _burst_take(take, 3) + out = d / "empty_source_out" + prep.process([take], out, "positive", 0.3, 0.2, 3.0) + for clip in out.glob("*.wav"): + clip.unlink() + + empty = prep.process([take], out, "positive", 0.3, 5.0, 10.0, clean=True) + refreshed = prep.process([take], out, "positive", 0.3, 0.2, 3.0) + present = sorted(p.name for p in out.glob("*.wav")) + check( + empty == 0 and refreshed == 3 and len(present) == 3, + f"an empty owned source accepts its next successful refresh ({present})", + ) + + +def test_new_zero_output_source_does_not_gain_cleanup_authority(d: Path) -> None: + """A source that contributed no clips cannot authorize a later clean.""" + src = d / "zero_owner_src" + good = src / "good.wav" + junk = src / "junk.wav" + _burst_take(good, 3) + out = d / "zero_owner_out" + prep.process([good], out, "positive", 0.3, 0.2, 3.0) + + sf.write( + junk, + np.ones(int(prep.TARGET_SR * 4.0), dtype=np.float32) * 0.5, + prep.TARGET_SR, + subtype="PCM_16", + ) + prep.process([good, junk], out, "positive", 0.3, 0.2, 1.0, clean=True) + _label, _clips, sources = prep._read_manifest(out) + junk_key = prep._source_key(junk) + check( + sources is not None and junk_key not in sources, + "a first-time zero-output source is not recorded as an owner", + ) + + errors = io.StringIO() + with redirect_stderr(errors): + prep.process([junk], out, "positive", 0.3, 0.2, 1.0, clean=True) + present = sorted(p.name for p in out.glob("*.wav")) + check( + present == ["good_000.wav", "good_001.wav", "good_002.wav"] + and "different source recordings" in errors.getvalue(), + f"a zero-output newcomer cannot later clean the real dataset ({present})", + ) + + +def test_manifest_spares_a_silent_take_alongside_a_good_one(d: Path) -> None: + """The `attempted` guard, with the manifest rule actually armed. + + When the whole run comes back empty nothing is written, so the manifest rule + never arms and the guard is never reached. The case that reaches it is a run + where one take records fine and another comes back silent: the good take + supplies the stem overlap that arms the rule, and then only `attempted` + stands between the silent take's earlier clips -- its only copy -- and + unlink(). Delete that clause and this is the test that goes red. + """ + src = d / "mixed_src" + good = src / "good.wav" + flaky = src / "flaky.wav" + _burst_take(good, 2) + _burst_take(flaky, 3) + out = d / "mixed_out" + prep.process([good, flaky], out, "positive", 0.3, 0.2, 3.0) + _burst_take(out / "custom_001.wav", 1) + before = sorted(p.name for p in out.glob("*.wav")) + check( + len(before) == 6, + f"both takes record and curated audio is present ({before})", + ) + + # Re-record both. `good` is fine; `flaky` comes back as one long unbroken + # tone -- a mic left running, say -- which the max-duration cap drops + # entirely, so it writes nothing while `good` writes and arms the rule. + _burst_take(good, 2) + sf.write( + flaky, + np.ones(int(prep.TARGET_SR * 4.0), dtype=np.float32) * 0.5, + prep.TARGET_SR, + subtype="PCM_16", + ) + errors = io.StringIO() + with redirect_stderr(errors): + prep.process([good, flaky], out, "positive", 0.3, 0.2, 1.0, clean=True) + present = sorted(p.name for p in out.glob("*.wav")) + check( + sorted(n for n in present if n.startswith("flaky_")) + == ["flaky_000.wav", "flaky_001.wav", "flaky_002.wav"], + f"--clean spares a silent take's only copy while cleaning beside it ({present})", + ) + check( + "check the recording" in errors.getvalue() + and "custom_001.wav" in errors.getvalue() + and "remove" in errors.getvalue() + and "by hand" in errors.getvalue(), + "mixed lingering guidance protects silent clips and identifies manual cleanup", + ) + + +def test_manifest_unreadable_is_refused(d: Path) -> None: + """A corrupt manifest cannot disable label and source ownership guards.""" + src = d / "corrupt_src" + _burst_take(src / "a" / "take.wav", 3) + _burst_take(src / "b" / "other.wav", 2) + out = d / "corrupt_out" + prep.process( + [src / "a" / "take.wav", src / "b" / "other.wav"], + out, + "positive", + 0.3, + 0.2, + 3.0, + ) + corrupt_payload = "{not json" + manifest = out / prep.MANIFEST_NAME + manifest.write_text(corrupt_payload) + + _burst_take(src / "a" / "take.wav", 2) + errors = io.StringIO() + with redirect_stderr(errors): + total = prep.process( + [src / "a" / "take.wav"], out, "positive", 0.3, 0.2, 3.0, clean=True + ) + present = sorted(p.name for p in out.glob("*.wav")) + check( + total == 0 + and present + == [ + "other_000.wav", + "other_001.wav", + "take_000.wav", + "take_001.wav", + "take_002.wav", + ], + f"a corrupt manifest refuses the run before any clip changes ({present})", + ) + check( + "manifest" in errors.getvalue() and "nothing was written" in errors.getvalue(), + "a corrupt manifest explains the fail-closed ownership refusal", + ) + check( + manifest.read_text() == corrupt_payload, + "a fallback run preserves the unreadable manifest for deliberate recovery", + ) + + +def test_manifest_wrong_shape_is_refused(d: Path) -> None: + """Readable JSON with an invalid manifest shape must fail closed.""" + src = d / "malformed_manifest_src" + take = src / "take.wav" + _burst_take(take, 3) + out = d / "malformed_manifest_out" + prep.process([take], out, "positive", 0.3, 0.2, 3.0) + (out / prep.MANIFEST_NAME).write_text("[]\n") + + _burst_take(take, 2) + errors = io.StringIO() + with redirect_stderr(errors): + total = prep.process([take], out, "positive", 0.3, 0.2, 3.0, clean=True) + check( + total == 0 + and "manifest" in errors.getvalue() + and "nothing was written" in errors.getvalue(), + "a wrong-shape manifest refuses to disable ownership protection", + ) + + +def test_manifest_bad_sources_warns_before_ownership_refusal(d: Path) -> None: + """A malformed source map must be diagnosed as corrupt, not legacy.""" + out = d / "bad_sources_manifest_out" + out.mkdir() + (out / prep.MANIFEST_NAME).write_text( + json.dumps( + { + "version": 2, + "label": "positive", + "clips": ["take_000.wav"], + "sources": "not a source map", + } + ) + ) + + errors = io.StringIO() + with redirect_stderr(errors): + prep._read_manifest(out) + check( + "manifest" in errors.getvalue() + and "sources" in errors.getvalue() + and "source ownership" in errors.getvalue(), + "a malformed sources map reports why ownership is unavailable", + ) + + +def test_manifest_rejects_overlapping_source_ownership(d: Path) -> None: + """Two sources cannot both authorize deletion of the same clip.""" + src = d / "overlapping_owners_src" + current = src / "b" / "take.wav" + _burst_take(current, 2) + out = d / "overlapping_owners_out" + out.mkdir() + _burst_take(out / "take_000.wav", 1) + manifest = out / prep.MANIFEST_NAME + manifest.write_text( + json.dumps( + { + "version": 2, + "label": "positive", + "clips": ["take_000.wav"], + "sources": { + str((src / "a" / "take.wav").resolve()): ["take_000.wav"], + prep._source_key(current): ["take_000.wav"], + }, + } + ) + ) + before = (out / "take_000.wav").read_bytes() + + errors = io.StringIO() + with redirect_stderr(errors): + total = prep.process([current], out, "positive", 0.3, 0.2, 3.0, clean=True) + check( + total == 0 + and (out / "take_000.wav").read_bytes() == before + and "ownership" in errors.getvalue() + and "nothing was written" in errors.getvalue(), + "overlapping source ownership fails closed before cleanup", + ) + + +def test_manifest_reader_requests_utf8(d: Path) -> None: + """Manifest paths must decode with the UTF-8 encoding used by the writer.""" + out = d / "utf8_manifest_out" + out.mkdir() + (out / prep.MANIFEST_NAME).write_text( + json.dumps( + { + "version": 2, + "label": "positive", + "clips": [], + "sources": {"/recordings/José/take.wav": []}, + }, + ensure_ascii=False, + ), + encoding="utf-8", + ) + original_read_text = prep.Path.read_text + requested: list[str | None] = [] + + def require_utf8(path: Path, *args: object, **kwargs: object) -> str: + requested.append(kwargs.get("encoding")) + if kwargs.get("encoding") != "utf-8": + raise UnicodeDecodeError("ascii", b"\x81", 0, 1, "simulated locale") + return original_read_text(path, *args, **kwargs) + + try: + prep.Path.read_text = require_utf8 + label, _clips, sources = prep._read_manifest(out) + finally: + prep.Path.read_text = original_read_text + check( + requested == ["utf-8"] and label == "positive" and sources is not None, + "the manifest reader explicitly decodes UTF-8", + ) + + +def test_manifest_source_warning_escapes_control_characters(d: Path) -> None: + """An untrusted source key cannot forge a destructive-warning line.""" + src = d / "escaped_source_warning_src" + current = src / "current.wav" + _burst_take(current, 2) + out = d / "escaped_source_warning_out" + out.mkdir() + _burst_take(out / "current_000.wav", 1) + _burst_take(out / "old_000.wav", 1) + hostile_source = "old.wav\nwarning: forged" + (out / prep.MANIFEST_NAME).write_text( + json.dumps( + { + "version": 2, + "label": "positive", + "clips": ["current_000.wav", "old_000.wav"], + "sources": { + prep._source_key(current): ["current_000.wav"], + hostile_source: ["old_000.wav"], + }, + } + ) + ) + + errors = io.StringIO() + with redirect_stderr(errors): + prep.process([current], out, "positive", 0.3, 0.2, 3.0, clean=True) + check( + "old.wav\\nwarning: forged" in errors.getvalue() + and hostile_source not in errors.getvalue(), + "a manifest source key is escaped in the pre-clean warning", + ) + + +def test_manifest_clip_names_cannot_escape_output_directory(d: Path) -> None: + """Manifest ownership must never turn a path into an output stem.""" + src = d / "hostile_manifest_src" + take = src / "take.wav" + _burst_take(take, 2) + out = d / "hostile_manifest_out" + out.mkdir() + escaped = d / "escaped_000.wav" + hostile_name = "../escaped_000.wav" + (out / prep.MANIFEST_NAME).write_text( + json.dumps( + { + "version": 2, + "label": "positive", + "clips": [hostile_name], + "sources": {prep._source_key(take): [hostile_name]}, + } + ) + ) + + errors = io.StringIO() + with redirect_stderr(errors): + total = prep.process([take], out, "positive", 0.3, 0.2, 3.0) + check( + total == 0 + and not escaped.exists() + and "manifest" in errors.getvalue() + and "nothing was written" in errors.getvalue(), + "a manifest path is refused before any write can escape --output", + ) + + +def test_manifest_clip_names_reject_embedded_nul(d: Path) -> None: + """A NUL-bearing manifest stem must fail closed instead of crashing.""" + src = d / "nul_manifest_src" + take = src / "take.wav" + _burst_take(take, 2) + out = d / "nul_manifest_out" + out.mkdir() + hostile_name = "a\u0000b_000.wav" + (out / prep.MANIFEST_NAME).write_text( + json.dumps( + { + "version": 2, + "label": "positive", + "clips": [hostile_name], + "sources": {prep._source_key(take): [hostile_name]}, + } + ) + ) + + errors = io.StringIO() + try: + with redirect_stderr(errors): + total = prep.process([take], out, "positive", 0.3, 0.2, 3.0) + except ValueError: + total = -1 + check( + total == 0 + and "manifest" in errors.getvalue() + and "nothing was written" in errors.getvalue(), + "an embedded NUL is refused before a manifest stem reaches the filesystem", + ) + + +def test_manifest_rejects_unsupported_version_with_v2_shape(d: Path) -> None: + """A version-1 declaration cannot gain v2 cleanup authority by shape.""" + src = d / "unsupported_version_src" + take = src / "take.wav" + _burst_take(take, 2) + out = d / "unsupported_version_out" + out.mkdir() + source_key = prep._source_key(take) + (out / prep.MANIFEST_NAME).write_text( + json.dumps( + { + "version": 1, + "label": "positive", + "clips": ["take_000.wav"], + "sources": {source_key: ["take_000.wav"]}, + } + ) + ) + + errors = io.StringIO() + with redirect_stderr(errors): + total = prep.process([take], out, "positive", 0.3, 0.2, 3.0) + check( + total == 0 + and not list(out.glob("*.wav")) + and "version" in errors.getvalue() + and "nothing was written" in errors.getvalue(), + "an unsupported manifest version is refused despite a v2-shaped map", + ) + + +def test_manifest_requires_a_string_label(d: Path) -> None: + """A v2 source map cannot authorize writes without a trusted label.""" + src = d / "missing_label_src" + take = src / "take.wav" + _burst_take(take, 2) + out = d / "missing_label_out" + out.mkdir() + (out / prep.MANIFEST_NAME).write_text( + json.dumps( + { + "version": 2, + "clips": ["take_000.wav"], + "sources": {prep._source_key(take): ["take_000.wav"]}, + } + ) + ) + + errors = io.StringIO() + with redirect_stderr(errors): + total = prep.process([take], out, "positive", 0.3, 0.2, 3.0) + check( + total == 0 + and not list(out.glob("*.wav")) + and "label" in errors.getvalue() + and "nothing was written" in errors.getvalue(), + "a v2 manifest without a string label fails closed", + ) + + +def test_posix_backslash_basename_round_trips_manifest(d: Path) -> None: + """A POSIX basename the writer accepts must remain readable next run.""" + if os.sep != "/": + check(True, "the POSIX backslash-basename check is not applicable") + return + + src = d / "backslash_basename_src" + take = src / "rec\\take.wav" + _burst_take(take, 2) + out = d / "backslash_basename_out" + first = prep.process([take], out, "positive", 0.3, 0.2, 3.0) + second = prep.process([take], out, "positive", 0.3, 0.2, 3.0) + _label, _clips, sources = prep._read_manifest(out) + check( + first == 2 + and second == 2 + and sources is not None + and prep._source_key(take) in sources, + "a POSIX backslash basename survives its manifest round trip", + ) + + +def test_manifest_replace_failure_describes_authoritative_old_record(d: Path) -> None: + """An atomic replace failure leaves the previous manifest in force.""" + out = d / "manifest_replace_failure_out" + out.mkdir() + old_payload = '{"version": 1, "label": "positive", "clips": []}\n' + manifest = out / prep.MANIFEST_NAME + manifest.write_text(old_payload) + original_replace = prep.os.replace + + def fail_replace(_source: Path, _target: Path) -> None: + raise OSError("simulated replace failure") + + errors = io.StringIO() + try: + prep.os.replace = fail_replace + with redirect_stderr(errors): + prep._write_manifest( + out, + "positive", + {"/recordings/take.wav": {"take_000.wav"}}, + ) + finally: + prep.os.replace = original_replace + + check( + manifest.read_text() == old_payload + and "previous manifest remains in force" in errors.getvalue() + and "unrecorded" in errors.getvalue(), + "a replace failure explains that the old record is still authoritative", + ) + + +def test_failed_unlink_summary_keeps_recorded_ownership(d: Path) -> None: + """A failed owned deletion must not be summarized as unrecorded audio.""" + src = d / "failed_unlink_src" + take = src / "take.wav" + _burst_take(take, 3) + out = d / "failed_unlink_out" + prep.process([take], out, "positive", 0.3, 0.2, 3.0) + _burst_take(take, 2) + + original_unlink = prep.Path.unlink + + def fail_one_unlink(path: Path, *args: object, **kwargs: object) -> None: + if path.name == "take_002.wav": + raise OSError("simulated unlink failure") + original_unlink(path, *args, **kwargs) + + errors = io.StringIO() + try: + prep.Path.unlink = fail_one_unlink + with redirect_stderr(errors): + prep.process([take], out, "positive", 0.3, 0.2, 3.0, clean=True) + finally: + prep.Path.unlink = original_unlink + + _label, _clips, sources = prep._read_manifest(out) + owned = sources.get(prep._source_key(take), set()) if sources is not None else set() + check( + "take_002.wav" in owned + and "selected for cleanup" in errors.getvalue() + and "could not be removed" in errors.getvalue() + and "no record of creating" not in errors.getvalue(), + "a failed unlink stays owned and gets accurate recovery guidance", + ) + + +def test_manifest_without_source_ownership_is_refused(d: Path) -> None: + """A readable legacy record cannot prove which dataset owns the output.""" + src = d / "legacy_manifest_src" + take = src / "take.wav" + _burst_take(take, 3) + out = d / "legacy_manifest_out" + prep.process([take], out, "positive", 0.3, 0.2, 3.0) + + manifest_path = out / prep.MANIFEST_NAME + manifest = json.loads(manifest_path.read_text()) + manifest["version"] = 1 + manifest.pop("sources") + manifest_path.write_text(json.dumps(manifest)) + before = sorted(p.name for p in out.glob("*.wav")) + + _burst_take(take, 1) + total = prep.process([take], out, "positive", 0.3, 0.2, 3.0, clean=True) + present = sorted(p.name for p in out.glob("*.wav")) + check( + total == 0 and present == before, + f"a manifest without source ownership cannot authorize a write ({present})", + ) + + +def test_manifest_bootstrap_keeps_legacy_leftovers_cleanable(d: Path) -> None: + """The first manifest must not strand clips from a pre-manifest run. + + A normal refresh without --clean warns that its higher-numbered leftovers + can be removed by rerunning with --clean. When that refresh is also the run + that would introduce the manifest, prep withholds the record while ambiguous + same-stem files remain. The follow-up clean therefore stays on the legacy + filename rule, removes those leftovers, and only then records v2 ownership. + """ + src = d / "bootstrap_src" + take = src / "take.wav" + _burst_take(take, 3) + out = d / "bootstrap_out" + prep.process([take], out, "positive", 0.3, 0.2, 3.0) + (out / prep.MANIFEST_NAME).unlink() # Simulate an output from before manifests. + + _burst_take(take, 2) + prep.process([take], out, "positive", 0.3, 0.2, 3.0) + prep.process([take], out, "positive", 0.3, 0.2, 3.0, clean=True) + present = sorted(p.name for p in out.glob("*.wav")) + check( + present == ["take_000.wav", "take_001.wav"], + f"the first manifest leaves its warned-about leftovers cleanable ({present})", + ) + + +def test_manifest_bootstrap_does_not_adopt_ambiguous_same_stem_audio( + d: Path, +) -> None: + """A non-clean legacy refresh must not claim ambiguous clips as prep-owned. + + Before a manifest exists, a same-stem clip can be a prep leftover or audio + curated by hand. Leaving the directory on the documented legacy path keeps + that ambiguity visible; recording the clip in v2 would make a later + source-aware --clean delete it under a false provenance claim. + """ + src = d / "bootstrap_curated_src" + take = src / "take.wav" + _burst_take(take, 3) + out = d / "bootstrap_curated_out" + prep.process([take], out, "positive", 0.3, 0.2, 3.0) + (out / prep.MANIFEST_NAME).unlink() + _burst_take(out / "take_007.wav", 1) + + _burst_take(take, 2) + prep.process([take], out, "positive", 0.3, 0.2, 3.0) + present = sorted(p.name for p in out.glob("*.wav")) + check( + not (out / prep.MANIFEST_NAME).exists() and "take_007.wav" in present, + f"ambiguous same-stem audio stays outside a source-aware record ({present})", + ) + + +def test_clean_without_manifest_warns_before_legacy_cleanup(d: Path) -> None: + """A provenance-free clean must announce its filename-only deletion rule.""" + src = d / "legacy_clean_src" + take = src / "take.wav" + _burst_take(take, 3) + out = d / "legacy_clean_out" + prep.process([take], out, "positive", 0.3, 0.2, 3.0) + (out / prep.MANIFEST_NAME).unlink() + _burst_take(out / "take_007.wav", 1) + + _burst_take(take, 2) + errors = io.StringIO() + with redirect_stderr(errors): + prep.process([take], out, "positive", 0.3, 0.2, 3.0, clean=True) + check( + "manifest" in errors.getvalue() + and "legacy filename cleanup" in errors.getvalue(), + "a no-manifest --clean warns before using the provenance-free rule", + ) + + +def test_manifest_spares_a_different_dataset_on_stray_output(d: Path) -> None: + """A mistyped --output must not let the manifest wipe another dataset. + + Provenance says prep created a file, not that the file belongs to the set + being refreshed, so the manifest rule only arms on a matching label with an + overlapping stem. Without that, every clip in a background dir is one prep + wrote and no positives rerun attempts, which would be enough to delete the + whole directory. + """ + src = d / "stray_src" + _burst_take(src / "noise.wav", 2) + bg = d / "stray_background" + prep.process([src / "noise.wav"], bg, "background", 0.3, 0.2, 3.0) + before = sorted(p.name for p in bg.glob("*.wav")) + check( + before == ["noise_000.wav"], f"the background dir starts populated ({before})" + ) + + # The typo: a positives run pointed at the background dir. + _burst_take(src / "computah.wav", 3) + prep.process([src / "computah.wav"], bg, "positive", 0.3, 0.2, 3.0, clean=True) + present = sorted(p.name for p in bg.glob("*.wav")) + check( + "noise_000.wav" in present, + f"--clean spares a dataset this run is not refreshing ({present})", + ) + + # And the guard has to survive the typo being repeated. If the stray run had + # relabeled the directory's record as "positive", this second one would find + # a matching label plus an overlapping stem and read the background clip as + # its own orphan. + _burst_take(src / "computah.wav", 2) + prep.process([src / "computah.wav"], bg, "positive", 0.3, 0.2, 3.0, clean=True) + present = sorted(p.name for p in bg.glob("*.wav")) + check( + "noise_000.wav" in present, + f"a repeated stray --output still spares the other dataset ({present})", + ) + + +def test_same_label_shared_stem_stray_is_refused_before_writing(d: Path) -> None: + """One generic stem match is not proof that two datasets are the same. + + A prior dataset and a stray run can both contain ``take.wav`` while coming + from different source paths. Treating that basename collision as ownership + lets --clean delete every other manifested stem in the prior dataset. The + output must remain untouched unless the run can prove it is refreshing the + recorded source set. + """ + src = d / "shared_stem_src" + original = src / "original" + stray = src / "stray" + _burst_take(original / "take.wav", 3) + _burst_take(original / "other.wav", 2) + _burst_take(stray / "take.wav", 1) + out = d / "shared_stem_out" + prep.process( + [original / "take.wav", original / "other.wav"], + out, + "positive", + 0.3, + 0.2, + 3.0, + ) + before = sorted(p.name for p in out.glob("*.wav")) + + total = prep.process( + [stray / "take.wav"], out, "positive", 0.3, 0.2, 3.0, clean=True + ) + present = sorted(p.name for p in out.glob("*.wav")) + check( + total == 0 and present == before, + f"a same-label basename collision cannot claim another dataset ({present})", + ) + + +def test_clean_disarmed_entirely_on_a_label_mismatch(d: Path) -> None: + """A label mismatch has to disarm the stem match too, not just the manifest. + + The stems collide in practice: `take.wav` is as plausible a negatives + filename as a positives one. Deciding this at --clean time is too late -- + by then the run has already overwritten take_000.wav with the wrong + dataset's audio -- so the check runs before any decode. + """ + src = d / "mismatch_src" + _burst_take(src / "take.wav", 3) + neg = d / "mismatch_neg" + prep.process([src / "take.wav"], neg, "negative", 0.3, 0.2, 3.0) + before = sorted(p.name for p in neg.glob("*.wav")) + check(len(before) == 3, f"the negatives dir starts with three clips ({before})") + + _burst_take(src / "take.wav", 1) + total = prep.process([src / "take.wav"], neg, "positive", 0.3, 0.2, 3.0, clean=True) + present = sorted(p.name for p in neg.glob("*.wav")) + check( + total == 0 and present == before, + f"a mismatched --output is refused before anything is written ({present})", + ) + + +def test_clean_spares_a_hand_added_clip_under_a_recorded_stem(d: Path) -> None: + """The stem match consults the record once there is one. + + A clip a user dropped in beside a manifested dataset shares the stem prep + writes, so shape alone cannot tell it from an orphan. Deleting it would break + the guarantee this whole change rests on: --clean removes only what prep + made. + """ + src = d / "handadd_src" + take = src / "take.wav" + _burst_take(take, 3) + out = d / "handadd_out" + prep.process([take], out, "positive", 0.3, 0.2, 3.0) + _burst_take(out / "take_007.wav", 1) # kept by hand, never prep's + + _burst_take(take, 2) + prep.process([take], out, "positive", 0.3, 0.2, 3.0, clean=True) + present = sorted(p.name for p in out.glob("*.wav")) + check( + "take_007.wav" in present and "take_002.wav" not in present, + f"--clean takes its own orphan and leaves the hand-added clip ({present})", + ) + + +def test_same_label_stray_is_refused_before_writing(d: Path) -> None: + """A no-overlap run must not pollute another same-label dataset. + + The manifest already proves the output belongs to a recorded source set. + With no source in common, the likely wrong --output is known before any + audio is decoded, so warning after clips have landed is too late. + """ + src = d / "seed_src" + _burst_take(src / "noise.wav", 2) + _burst_take(src / "computah.wav", 3) + out = d / "seed_out" + prep.process([src / "noise.wav"], out, "positive", 0.3, 0.2, 3.0) + + before = sorted(p.name for p in out.glob("*.wav")) + errors = io.StringIO() + with redirect_stderr(errors): + totals = [ + prep.process( + [src / "computah.wav"], + out, + "positive", + 0.3, + 0.2, + 3.0, + clean=True, + ) + for _ in range(2) + ] + present = sorted(p.name for p in out.glob("*.wav")) + check( + totals == [0, 0] and present == before, + f"a repeated same-label stray writes nothing ({present})", + ) + check( + repr(prep._source_key(src / "noise.wav")) in errors.getvalue(), + "a no-overlap refusal names an escaped recorded source", + ) + + def main() -> int: test_segment_count() with tempfile.TemporaryDirectory(prefix="prep-wake-") as tmp: @@ -541,6 +1602,39 @@ def main() -> int: test_in_run_collision_raises(d) test_refresh_after_adding_collider_leaves_no_orphans(d) test_stem_assignment_ignores_input_order(d) + test_manifest_cleans_dropped_input_orphans(d) + test_manifest_cleans_changed_collider_stem_orphans(d) + test_manifest_tracks_a_silent_colliding_source_by_path(d) + test_new_collider_cannot_overwrite_a_silent_sources_prior_stem(d) + test_manifest_never_authorizes_deleting_curated_audio(d) + test_manifest_spares_prior_clips_when_rerun_writes_nothing(d) + test_nonclean_silent_rerun_protects_only_copy_in_warning(d) + test_legacy_mixed_warning_keeps_cleanable_leftovers_automated(d) + test_manifest_empty_source_remains_refreshable(d) + test_new_zero_output_source_does_not_gain_cleanup_authority(d) + test_manifest_spares_a_silent_take_alongside_a_good_one(d) + test_manifest_unreadable_is_refused(d) + test_manifest_wrong_shape_is_refused(d) + test_manifest_bad_sources_warns_before_ownership_refusal(d) + test_manifest_rejects_overlapping_source_ownership(d) + test_manifest_reader_requests_utf8(d) + test_manifest_source_warning_escapes_control_characters(d) + test_manifest_clip_names_cannot_escape_output_directory(d) + test_manifest_clip_names_reject_embedded_nul(d) + test_manifest_rejects_unsupported_version_with_v2_shape(d) + test_manifest_requires_a_string_label(d) + test_posix_backslash_basename_round_trips_manifest(d) + test_manifest_replace_failure_describes_authoritative_old_record(d) + test_failed_unlink_summary_keeps_recorded_ownership(d) + test_manifest_without_source_ownership_is_refused(d) + test_manifest_bootstrap_keeps_legacy_leftovers_cleanable(d) + test_manifest_bootstrap_does_not_adopt_ambiguous_same_stem_audio(d) + test_clean_without_manifest_warns_before_legacy_cleanup(d) + test_manifest_spares_a_different_dataset_on_stray_output(d) + test_same_label_shared_stem_stray_is_refused_before_writing(d) + test_clean_disarmed_entirely_on_a_label_mismatch(d) + test_clean_spares_a_hand_added_clip_under_a_recorded_stem(d) + test_same_label_stray_is_refused_before_writing(d) n_pass = sum(1 for ok, _ in results if ok) print(f"=== {n_pass}/{len(results)} checks passed ===") return 0 if n_pass == len(results) else 1