From daf365827b502b866bf8f478d0e12284e32c7cbe Mon Sep 17 00:00:00 2001 From: Joe Amditis <6799804+jamditis@users.noreply.github.com> Date: Wed, 22 Jul 2026 08:10:30 -0400 Subject: [PATCH 1/5] Disambiguate output stems so same-basename inputs don't overwrite Clip names are derived from the input file stem plus a segment index, so two inputs with the same basename (a/computah.wav and b/computah.wav) wrote the same clip names and the second file silently overwrote the first. process() still summed stats["kept"] across both, so it reported more clips than landed on disk and training examples vanished with no error -- the exact hazard for the multi-folder and globbed workflows the script is built for. Fix at the source rather than per-caller: _unique_stems() assigns a distinct output stem per input file, in input order. A stem that is already unique is kept as-is, so the common single-folder case is unchanged; a collision gets the lowest numeric suffix not otherwise taken, and singletons are reserved first so a disambiguated "computah-1" can never land on a real "computah-1" input. Both the segmented and the background write paths route through it. process() now returns the count of files actually written (a set of paths) instead of the requested segment total, so the reported number equals the files on disk by construction. Covers the duplicate-basename case in test_prep_wake_samples.py for both the positive (segmented) and background paths, with the two inputs carrying different burst counts so an overwrite shows up as fewer files than clips. Mutation-checked: reverting only the source fails exactly the two new checks. Closes #7 wake-20260722T0805-774163 --- prep_wake_samples.py | 45 +++++++++++++++++++++++++++----- test_prep_wake_samples.py | 54 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 6 deletions(-) diff --git a/prep_wake_samples.py b/prep_wake_samples.py index 527f9f6..f2db7fe 100644 --- a/prep_wake_samples.py +++ b/prep_wake_samples.py @@ -28,6 +28,7 @@ import subprocess import sys import tempfile +from collections import Counter from math import gcd from pathlib import Path @@ -206,6 +207,36 @@ class — pointing a whole mixed folder at --label positive would mislabel the return uniq +def _unique_stems(files: list[Path]) -> list[str]: + """A distinct output stem per input file, in input order. + + Clip names are derived from the input stem, so two inputs that share a + basename (a/computah.wav and b/computah.wav) would write the same clip + names and the second file would silently overwrite the first. A stem that + is already unique across the inputs is kept as-is; a colliding one gets the + lowest numeric suffix not otherwise taken. Singletons are reserved first so + a disambiguated "computah-1" can never land on a real "computah-1" input. + + A clip's final name is "-N_.wav" when disambiguated (the "-N" + from here, the "_" segment index from _write_clip) or "_.wav" + when the stem was already unique. + """ + counts = Counter(f.stem for f in files) + used = {f.stem for f in files if counts[f.stem] == 1} + stems: list[str] = [] + for f in files: + if counts[f.stem] == 1: + stems.append(f.stem) + continue + n = 1 + while f"{f.stem}-{n}" in used: + n += 1 + stem = f"{f.stem}-{n}" + used.add(stem) + stems.append(stem) + return stems + + def process( inputs: list[Path], out_dir: Path, @@ -221,28 +252,30 @@ def process( print(f"no audio files found at {joined}", file=sys.stderr) return 0 - total = 0 + stems = _unique_stems(files) + written: set[Path] = set() all_durs: list[float] = [] - for f in files: + for f, stem in zip(files, stems): audio = load_mono_16k(f) dur = len(audio) / TARGET_SR if label == "background": # Continuous negative audio: keep it whole, just normalized. - _write_clip(out_dir, f.stem, 0, audio) - total += 1 + written.add(_write_clip(out_dir, stem, 0, audio)) 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): - _write_clip(out_dir, f.stem, i, clip) + written.add(_write_clip(out_dir, stem, i, clip)) all_durs.append(len(clip) / TARGET_SR) - total += stats["kept"] note = "" if stats["too_short"] or stats["too_long"]: note = f" (dropped {stats['too_short']} short, {stats['too_long']} long)" print(f" {f.name}: {dur:.1f}s -> {stats['kept']} clips{note}") + # Count files actually on disk, not segments requested, so the report is + # honest even if two inputs ever resolve to the same clip name. + total = len(written) print(f"\nwrote {total} {label} clip(s) to {out_dir}") if all_durs: a = np.array(all_durs) diff --git a/test_prep_wake_samples.py b/test_prep_wake_samples.py index 0e2b191..fe429e4 100644 --- a/test_prep_wake_samples.py +++ b/test_prep_wake_samples.py @@ -143,6 +143,59 @@ def test_empty_input_no_crash(d: Path) -> None: check(total == 0, f"empty input returns 0 without crashing (got {total})") +def test_duplicate_basename_no_overwrite(d: Path) -> None: + """Two inputs sharing a basename must not overwrite each other's clips. + + Clip names come from the input stem, so a/computah.wav and b/computah.wav + once wrote the same names and the second file silently clobbered the first + while the reported total still counted both. The two files carry different + burst counts (2 and 3) so any overwrite shows up as fewer files than clips. + """ + a = d / "a" + b = d / "b" + a.mkdir() + b.mkdir() + sf.write( + a / "computah.wav", + make_bursts(2, 0.5, 0.8, prep.TARGET_SR), + prep.TARGET_SR, + subtype="PCM_16", + ) + sf.write( + b / "computah.wav", + make_bursts(3, 0.5, 0.8, prep.TARGET_SR), + prep.TARGET_SR, + subtype="PCM_16", + ) + + out = d / "dup_positive" + total = prep.process( + [a / "computah.wav", b / "computah.wav"], out, "positive", 0.3, 0.2, 3.0 + ) + files = sorted(out.glob("*.wav")) + check( + total == 5 and len(files) == 5, + f"5 clips from two same-basename inputs, none overwritten " + f"(total={total}, files={len(files)})", + ) + check( + len({f.name for f in files}) == len(files), + f"every clip name is distinct ({[f.name for f in files]})", + ) + + # The background path derives names the same way, so it shares the bug. + out_bg = d / "dup_background" + total_bg = prep.process( + [a / "computah.wav", b / "computah.wav"], out_bg, "background", 0.3, 0.2, 3.0 + ) + bg_files = sorted(out_bg.glob("*.wav")) + check( + total_bg == 2 and len(bg_files) == 2, + f"two same-basename background files stay two files " + f"(total={total_bg}, files={len(bg_files)})", + ) + + def main() -> int: test_segment_count() with tempfile.TemporaryDirectory(prefix="prep-wake-") as tmp: @@ -152,6 +205,7 @@ def main() -> int: test_background_kept_whole(d) test_explicit_files_only(d) test_empty_input_no_crash(d) + test_duplicate_basename_no_overwrite(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 From e86f01f505bd35d5c8a675fdf15acfcf8f86ca88 Mon Sep 17 00:00:00 2001 From: Joe Amditis <6799804+jamditis@users.noreply.github.com> Date: Wed, 22 Jul 2026 09:19:21 -0400 Subject: [PATCH 2/5] Refuse to overwrite an existing clip in _write_clip _unique_stems keeps in-run basenames distinct, but two collisions slip past it and silently drop a training example: a case-insensitive filesystem where a/Computah.wav and b/computah.wav resolve to one path, and a rerun into a populated out_dir. A casefold reservation key would fix the first but wrongly disambiguate distinct stems on the case-sensitive filesystem this runs on. Guarding the write instead is platform-correct for both and fails loud on any residual collision rather than clobbering. --- prep_wake_samples.py | 10 ++++++++++ test_prep_wake_samples.py | 22 ++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/prep_wake_samples.py b/prep_wake_samples.py index f2db7fe..7fdce98 100644 --- a/prep_wake_samples.py +++ b/prep_wake_samples.py @@ -89,6 +89,16 @@ def _write_clip(out_dir: Path, stem: str, idx: int, audio: np.ndarray) -> Path: out_dir.mkdir(parents=True, exist_ok=True) pcm = (np.clip(audio, -1.0, 1.0) * 32767.0).astype(np.int16) path = out_dir / f"{stem}_{idx:03d}.wav" + if path.exists(): + # Never silently clobber a clip already on disk. _unique_stems keeps + # in-run basenames distinct, but two collisions slip past it: a + # case-insensitive filesystem (a/Computah.wav and b/computah.wav map to + # one path) and a rerun into a populated out_dir. Fail loud so a real + # ambiguity surfaces instead of a training example vanishing. + raise FileExistsError( + f"refusing to overwrite {path}: another clip already claimed this " + f"name (case-insensitive basename collision or a prior run's output)" + ) sf.write(path, pcm, TARGET_SR, subtype="PCM_16") return path diff --git a/test_prep_wake_samples.py b/test_prep_wake_samples.py index fe429e4..8af074b 100644 --- a/test_prep_wake_samples.py +++ b/test_prep_wake_samples.py @@ -196,6 +196,27 @@ def test_duplicate_basename_no_overwrite(d: Path) -> None: ) +def test_write_clip_refuses_overwrite(d: Path) -> None: + """_write_clip must never overwrite a clip already on disk. + + _unique_stems disambiguates same-basename inputs on a case-sensitive + filesystem, but a case-insensitive one (Computah vs computah collapse to one + path) or a rerun into a populated dir can still target an existing file. The + guard turns that into a loud FileExistsError instead of a silent data loss, + so the check writes the same clip name twice and expects the second to raise. + """ + out = d / "overwrite_guard" + audio = make_bursts(1, 0.5, 0.8, prep.TARGET_SR) + first = prep._write_clip(out, "dup", 0, audio) + check(first.exists(), f"first _write_clip lands on disk ({first.name})") + try: + prep._write_clip(out, "dup", 0, audio) + except FileExistsError: + check(True, "second write to the same clip name raises FileExistsError") + else: + check(False, "second write to the same clip name should have raised") + + def main() -> int: test_segment_count() with tempfile.TemporaryDirectory(prefix="prep-wake-") as tmp: @@ -206,6 +227,7 @@ def main() -> int: test_explicit_files_only(d) test_empty_input_no_crash(d) test_duplicate_basename_no_overwrite(d) + test_write_clip_refuses_overwrite(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 From 7d03b5437af6686e4d7f432d61fe27d800faac1b Mon Sep 17 00:00:00 2001 From: Joe Amditis <6799804+jamditis@users.noreply.github.com> Date: Wed, 22 Jul 2026 09:24:06 -0400 Subject: [PATCH 3/5] Scope the overwrite guard to in-run collisions only The previous hard path.exists() guard broke the documented refresh: the docs tell people to rerun prep_wake_samples straight into samples/, and that now aborted on the first reused name and could leave a half-written dataset. The invariant is narrower than 'never overwrite': two inputs in one run must not clobber each other, but a prior run's leftover is a fine overwrite. _write_unique tracks the inodes written this run, so a case-insensitive FS collision (distinct-cased stems share an inode) still fails loud while a rerun refreshes cleanly. --- prep_wake_samples.py | 38 +++++++++++++++++-------- test_prep_wake_samples.py | 58 +++++++++++++++++++++++++++++---------- 2 files changed, 70 insertions(+), 26 deletions(-) diff --git a/prep_wake_samples.py b/prep_wake_samples.py index 7fdce98..86cd462 100644 --- a/prep_wake_samples.py +++ b/prep_wake_samples.py @@ -89,17 +89,32 @@ def _write_clip(out_dir: Path, stem: str, idx: int, audio: np.ndarray) -> Path: out_dir.mkdir(parents=True, exist_ok=True) pcm = (np.clip(audio, -1.0, 1.0) * 32767.0).astype(np.int16) path = out_dir / f"{stem}_{idx:03d}.wav" - if path.exists(): - # Never silently clobber a clip already on disk. _unique_stems keeps - # in-run basenames distinct, but two collisions slip past it: a - # case-insensitive filesystem (a/Computah.wav and b/computah.wav map to - # one path) and a rerun into a populated out_dir. Fail loud so a real - # ambiguity surfaces instead of a training example vanishing. + sf.write(path, pcm, TARGET_SR, subtype="PCM_16") + return path + + +def _write_unique( + out_dir: Path, stem: str, idx: int, audio: np.ndarray, seen: set[int] +) -> Path: + """Write a clip, refusing to clobber one this same run already wrote. + + Rerunning the script into a populated dir is a supported refresh, so a + leftover from a *prior* run is fine to overwrite. What must never happen is + two inputs in *one* run mapping to the same file and the second silently + replacing the first. `seen` holds the inode of every clip written this run; + the filesystem resolves a case-insensitive collision (a/Computah.wav and + b/computah.wav) to that same inode, so this catches it where a path-string + compare would not, and stays quiet on a case-sensitive FS where the two are + genuinely different files. + """ + path = out_dir / f"{stem}_{idx:03d}.wav" + if path.exists() and path.stat().st_ino in seen: raise FileExistsError( - f"refusing to overwrite {path}: another clip already claimed this " - f"name (case-insensitive basename collision or a prior run's output)" + f"two inputs map to {path.name} in one run " + f"(same basename, or a case-insensitive filesystem collision)" ) - sf.write(path, pcm, TARGET_SR, subtype="PCM_16") + _write_clip(out_dir, stem, idx, audio) + seen.add(path.stat().st_ino) return path @@ -264,19 +279,20 @@ def process( stems = _unique_stems(files) written: set[Path] = set() + seen_inodes: set[int] = set() all_durs: list[float] = [] for f, stem in zip(files, stems): audio = load_mono_16k(f) dur = len(audio) / TARGET_SR if label == "background": # Continuous negative audio: keep it whole, just normalized. - written.add(_write_clip(out_dir, stem, 0, audio)) + written.add(_write_unique(out_dir, stem, 0, audio, seen_inodes)) 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_clip(out_dir, stem, i, clip)) + written.add(_write_unique(out_dir, stem, i, clip, seen_inodes)) all_durs.append(len(clip) / TARGET_SR) note = "" if stats["too_short"] or stats["too_long"]: diff --git a/test_prep_wake_samples.py b/test_prep_wake_samples.py index 8af074b..13f0a69 100644 --- a/test_prep_wake_samples.py +++ b/test_prep_wake_samples.py @@ -196,25 +196,52 @@ def test_duplicate_basename_no_overwrite(d: Path) -> None: ) -def test_write_clip_refuses_overwrite(d: Path) -> None: - """_write_clip must never overwrite a clip already on disk. - - _unique_stems disambiguates same-basename inputs on a case-sensitive - filesystem, but a case-insensitive one (Computah vs computah collapse to one - path) or a rerun into a populated dir can still target an existing file. The - guard turns that into a loud FileExistsError instead of a silent data loss, - so the check writes the same clip name twice and expects the second to raise. +def test_rerun_refreshes_populated_dir(d: Path) -> None: + """Rerunning into a dir that already holds a prior run's clips must work. + + The docs tell people to run prep_wake_samples straight into samples/, so a + refresh overwrites last run's clips rather than aborting. Running process + twice into the same out_dir must succeed both times and leave the same file + count -- a prior-run leftover is an intended overwrite, not a collision. + """ + src = d / "rerun_src" + src.mkdir() + sf.write( + src / "take.wav", + make_bursts(3, 0.5, 0.8, prep.TARGET_SR), + prep.TARGET_SR, + subtype="PCM_16", + ) + out = d / "rerun_out" + first = prep.process([src / "take.wav"], out, "positive", 0.3, 0.2, 3.0) + second = prep.process([src / "take.wav"], out, "positive", 0.3, 0.2, 3.0) + files = sorted(out.glob("*.wav")) + check( + first == second == 3 and len(files) == 3, + f"rerun into a populated dir refreshes cleanly " + f"(first={first}, second={second}, files={len(files)})", + ) + + +def test_in_run_collision_raises(d: Path) -> None: + """Two clips in one run mapping to the same file must fail loud. + + _unique_stems keeps in-run basenames distinct, but this is the backstop for + a case-insensitive filesystem where distinct-cased stems resolve to one + inode. Writing the same name twice with a shared seen-set (as one run would) + must raise instead of silently dropping the first clip. """ - out = d / "overwrite_guard" + out = d / "in_run_collision" audio = make_bursts(1, 0.5, 0.8, prep.TARGET_SR) - first = prep._write_clip(out, "dup", 0, audio) - check(first.exists(), f"first _write_clip lands on disk ({first.name})") + seen: set[int] = set() + first = prep._write_unique(out, "dup", 0, audio, seen) + check(first.exists(), f"first clip of the run lands on disk ({first.name})") try: - prep._write_clip(out, "dup", 0, audio) + prep._write_unique(out, "dup", 0, audio, seen) except FileExistsError: - check(True, "second write to the same clip name raises FileExistsError") + check(True, "a second clip mapping to the same file in one run raises") else: - check(False, "second write to the same clip name should have raised") + check(False, "an in-run collision should have raised FileExistsError") def main() -> int: @@ -227,7 +254,8 @@ def main() -> int: test_explicit_files_only(d) test_empty_input_no_crash(d) test_duplicate_basename_no_overwrite(d) - test_write_clip_refuses_overwrite(d) + test_rerun_refreshes_populated_dir(d) + test_in_run_collision_raises(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 From 7763614e660cf4aa88c4e6a66ab958e06cab61ef Mon Sep 17 00:00:00 2001 From: Joe Amditis <6799804+jamditis@users.noreply.github.com> Date: Wed, 22 Jul 2026 10:17:32 -0400 Subject: [PATCH 4/5] Keep one colliding input on the bare stem when refreshing Suffixing every member of a colliding basename group meant a refresh that added a second computah.wav renamed the original too, stranding the last run's computah_###.wav. eval_wake_threshold reads every clip in the directory, so those orphans quietly trained and scored as duplicates. One member of each group now keeps the bare stem, picked by resolved path rather than input order: reordering the same files must not move the bare stem to a different one, or the strand comes right back. Reserving every input stem up front (not just singletons) keeps two overlapping groups from both claiming computah-1. Clips this run did not write now draw a warning naming the files and the fix, since deleting the user's dir would be too eager. --- prep_wake_samples.py | 77 +++++++++++++++++++++++++------ test_prep_wake_samples.py | 97 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 159 insertions(+), 15 deletions(-) diff --git a/prep_wake_samples.py b/prep_wake_samples.py index 86cd462..ebe13dc 100644 --- a/prep_wake_samples.py +++ b/prep_wake_samples.py @@ -238,28 +238,53 @@ def _unique_stems(files: list[Path]) -> list[str]: Clip names are derived from the input stem, so two inputs that share a basename (a/computah.wav and b/computah.wav) would write the same clip names and the second file would silently overwrite the first. A stem that - is already unique across the inputs is kept as-is; a colliding one gets the - lowest numeric suffix not otherwise taken. Singletons are reserved first so - a disambiguated "computah-1" can never land on a real "computah-1" input. + is already unique across the inputs is kept as-is. Within a colliding group + one file keeps the bare stem and the rest get the lowest numeric suffix not + otherwise taken. Every input stem is reserved up front so a disambiguated + "computah-1" can never land on a real "computah-1" input -- including one + that is itself part of another colliding group. + + One member of the group keeps the bare stem so that refreshing a samples dir + stays a refresh: adding a second computah.wav renames only the new file and + leaves the original's clips to be overwritten in place. Suffixing every + member instead would orphan the previous run's computah_###.wav, and + eval_wake_threshold reads every clip in the directory, so those orphans would + quietly train and score as duplicates. + + Which member keeps the bare stem is decided by resolved path, not by input + order, so the mapping depends only on WHICH files are present. Passing the + same two files in the other order must not move the bare stem to the other + file -- that would strand the first run's clips just as surely. A clip's final name is "-N_.wav" when disambiguated (the "-N" from here, the "_" segment index from _write_clip) or "_.wav" when the stem was already unique. """ counts = Counter(f.stem for f in files) - used = {f.stem for f in files if counts[f.stem] == 1} - stems: list[str] = [] - for f in files: + used = {f.stem for f in files} + assigned: dict[int, str] = {} + groups: dict[str, list[int]] = {} + for i, f in enumerate(files): if counts[f.stem] == 1: - stems.append(f.stem) - continue - n = 1 - while f"{f.stem}-{n}" in used: - n += 1 - stem = f"{f.stem}-{n}" - used.add(stem) - stems.append(stem) - return stems + assigned[i] = f.stem + else: + groups.setdefault(f.stem, []).append(i) + + # Sorted group order, and resolved-path order within a group, so every + # assignment below is a function of the input set alone. + for stem, idxs in sorted(groups.items()): + ranked = sorted(idxs, key=lambda j: str(files[j].resolve())) + for rank, i in enumerate(ranked): + if rank == 0: + assigned[i] = stem + continue + n = 1 + while f"{stem}-{n}" in used: + n += 1 + name = f"{stem}-{n}" + used.add(name) + assigned[i] = name + return [assigned[i] for i in range(len(files))] def process( @@ -303,6 +328,28 @@ def process( # honest even if two inputs ever resolve to the same clip name. total = len(written) print(f"\nwrote {total} {label} clip(s) to {out_dir}") + + # Clips 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. Deleting them here would be + # too eager (the dir is the user's), so say so and name the fix. + # Match what eval_wake_threshold._clips() reads, not just what we write, so + # the warning covers every file that will actually be scored. + stale = sorted( + p + for p in out_dir.iterdir() + if p.suffix.lower() in AUDIO_EXTS and p not in written + ) + if stale: + shown = ", ".join(p.name for p in stale[:3]) + if len(stale) > 3: + shown += f", +{len(stale) - 3} more" + print( + f"warning: {len(stale)} clip(s) in {out_dir} are left from an earlier " + f"run ({shown}). Training and eval read every clip in this directory, " + f"so delete them or clear {out_dir} and rerun.", + file=sys.stderr, + ) if all_durs: a = np.array(all_durs) print( diff --git a/test_prep_wake_samples.py b/test_prep_wake_samples.py index 13f0a69..7ded992 100644 --- a/test_prep_wake_samples.py +++ b/test_prep_wake_samples.py @@ -244,6 +244,101 @@ def test_in_run_collision_raises(d: Path) -> None: check(False, "an in-run collision should have raised FileExistsError") +def test_refresh_after_adding_collider_leaves_no_orphans(d: Path) -> None: + """Adding a second same-basename input must not orphan the first run's clips. + + Run one input, then rerun with a second file of the same basename into the + same dir. If every colliding input got suffixed, the first run's + computah_###.wav would be stranded next to an identical computah-1_###.wav -- + and eval_wake_threshold._clips() reads every clip in the directory, so the + stale pair would silently double-count. The first input keeps the bare stem, + so its clips are overwritten in place and only the new file is suffixed. + """ + a = d / "refresh_a" + b = d / "refresh_b" + a.mkdir() + b.mkdir() + sf.write( + a / "computah.wav", + make_bursts(2, 0.5, 0.8, prep.TARGET_SR), + prep.TARGET_SR, + subtype="PCM_16", + ) + sf.write( + b / "computah.wav", + make_bursts(3, 0.5, 0.8, prep.TARGET_SR), + prep.TARGET_SR, + subtype="PCM_16", + ) + + 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 + ) + + files = sorted(p.name for p in out.glob("*.wav")) + # 2 clips from a (bare stem, overwritten in place) + 3 from b (suffixed). + check( + len(files) == 5, + f"refresh with an added collider leaves no orphaned clips ({files})", + ) + check( + any(n.startswith("computah_") for n in files), + f"the first colliding input keeps the bare stem ({files})", + ) + + +def test_stem_assignment_ignores_input_order(d: Path) -> None: + """The same colliding files must map to the same stems in any order. + + If the bare stem went to whichever file was listed first, rerunning with the + inputs reordered would hand it to the other file: the previous run's + computah_###.wav would stop being rewritten and would linger as a duplicate + that eval_wake_threshold still reads. Ranking by resolved path makes the + mapping a function of which files are present, not how they were passed. + """ + a = d / "order_a" + b = d / "order_b" + a.mkdir() + b.mkdir() + for folder in (a, b): + sf.write( + folder / "computah.wav", + make_bursts(2, 0.5, 0.8, prep.TARGET_SR), + prep.TARGET_SR, + subtype="PCM_16", + ) + + forward = prep._unique_stems([a / "computah.wav", b / "computah.wav"]) + reverse = prep._unique_stems([b / "computah.wav", a / "computah.wav"]) + # Same file -> same stem regardless of position, so reverse is forward flipped. + check( + forward == list(reversed(reverse)), + f"stem assignment is order-independent (forward={forward}, reverse={reverse})", + ) + check( + sorted(forward) == ["computah", "computah-1"], + f"one file keeps the bare stem, the other is suffixed ({sorted(forward)})", + ) + + # Two colliding groups where one group's bare stem is the name the other + # group's suffix search wants. Reserving only singletons hands "computah-1" + # to both, and the run later dies on a bogus same-basename FileExistsError. + two_groups = prep._unique_stems( + [ + a / "computah.wav", + b / "computah.wav", + a / "computah-1.wav", + b / "computah-1.wav", + ] + ) + check( + len(set(two_groups)) == len(two_groups), + f"overlapping collider groups still get distinct stems ({two_groups})", + ) + + def main() -> int: test_segment_count() with tempfile.TemporaryDirectory(prefix="prep-wake-") as tmp: @@ -256,6 +351,8 @@ def main() -> int: test_duplicate_basename_no_overwrite(d) test_rerun_refreshes_populated_dir(d) test_in_run_collision_raises(d) + test_refresh_after_adding_collider_leaves_no_orphans(d) + test_stem_assignment_ignores_input_order(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 From aae0d6d6685b5ea7e00abe29bca7c0f4e13ea64b Mon Sep 17 00:00:00 2001 From: Joe Amditis <6799804+jamditis@users.noreply.github.com> Date: Wed, 22 Jul 2026 15:24:50 -0400 Subject: [PATCH 5/5] Report a bad --output up front, and survive a run with no clips The stale-clip scan reads the whole output directory, not just what this run wrote, because training and eval score every clip sitting there. It was written to mirror eval_wake_threshold._clips() and its comment said so, but it copied that function's body without its precondition: _clips() returns empty when the directory is absent, and the copy went straight to iterdir(). Only _write_clip() creates out_dir, so a run that produced no clips (a silent take, or every segment dropped by --min-dur or --max-dur) left it missing, and the scan raised FileNotFoundError after all the decode work was already spent. Skipping the scan beats creating the directory here. A missing directory holds no clips from an earlier run, and creating one would leave an empty directory behind for every run that produced nothing. The precondition now sits in _stale_clips(), next to the read it guards, because keeping the two apart is what let the copy lose it. Guarding on is_dir() alone would also swallow an --output that names an existing file, a real mistake worth naming rather than passing over in silence. That check runs before any audio is decoded, so one message covers it instead of whichever exception the run happened to reach first. #82 tracks what is still duplicated: the two modules keep separate AUDIO_EXTS sets, so this warning can name a file eval would never score. wake-20260722T1502-062c47 --- prep_wake_samples.py | 42 ++++++++++++++++++++++++++++++++------- test_prep_wake_samples.py | 37 ++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 7 deletions(-) diff --git a/prep_wake_samples.py b/prep_wake_samples.py index ebe13dc..e99bf77 100644 --- a/prep_wake_samples.py +++ b/prep_wake_samples.py @@ -287,6 +287,28 @@ def _unique_stems(files: list[Path]) -> list[str]: return [assigned[i] for i in range(len(files))] +def _stale_clips(out_dir: Path, written: set[Path]) -> list[Path]: + """Audio files in `out_dir` this run did not write, sorted; empty if absent. + + Mirrors `eval_wake_threshold._clips()`, precondition included: a directory + that does not exist holds nothing. The precondition lives here, with the + read, because the earlier inline copy dropped it and a run that writes no + clips never creates out_dir (only `_write_clip` does), so the scan died on + a missing directory. + + The mirror is close but not exact: this module's `AUDIO_EXTS` is the wider + set, so the warning can name a file eval would skip. #82 tracks giving both + modules one definition instead of two copies. + """ + if not out_dir.is_dir(): + return [] + return sorted( + p + for p in out_dir.iterdir() + if p.suffix.lower() in AUDIO_EXTS and p not in written + ) + + def process( inputs: list[Path], out_dir: Path, @@ -296,6 +318,18 @@ def process( max_dur_s: float, ) -> int: """Process every input file; return the total clip count written.""" + # 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: + # mkdir() raises FileExistsError once a clip is ready to write, and a run that + # writes nothing gets as far as the stale scan below. + if out_dir.exists() and not out_dir.is_dir(): + print( + f"error: --output {out_dir} exists and is not a directory; " + f"point --output at a directory, or move that file", + file=sys.stderr, + ) + return 0 + files = _inputs(inputs) if not files: joined = ", ".join(str(p) for p in inputs) @@ -333,13 +367,7 @@ def process( # input set. Training and eval read every clip in the directory, so they # would score as extra data nobody asked for. Deleting them here would be # too eager (the dir is the user's), so say so and name the fix. - # Match what eval_wake_threshold._clips() reads, not just what we write, so - # the warning covers every file that will actually be scored. - stale = sorted( - p - for p in out_dir.iterdir() - if p.suffix.lower() in AUDIO_EXTS and p not in written - ) + stale = _stale_clips(out_dir, written) if stale: shown = ", ".join(p.name for p in stale[:3]) if len(stale) > 3: diff --git a/test_prep_wake_samples.py b/test_prep_wake_samples.py index 7ded992..d852d31 100644 --- a/test_prep_wake_samples.py +++ b/test_prep_wake_samples.py @@ -143,6 +143,41 @@ def test_empty_input_no_crash(d: Path) -> None: check(total == 0, f"empty input returns 0 without crashing (got {total})") +def test_zero_clips_no_crash(d: Path) -> None: + """Audio that yields no clips must report cleanly, not raise. + + Distinct from test_empty_input_no_crash: the input file IS found, so this + runs past the no-files early return and reaches the stale-clip scan. Only + _write_clip() creates out_dir, so a run that writes nothing never creates + it and the scan reads a directory that is not there. + """ + audio = make_bursts(n=3, burst_s=0.6, gap_s=1.0, sr=prep.TARGET_SR) + src = d / "all_dropped.wav" + sf.write(src, audio, prep.TARGET_SR, subtype="PCM_16") + out = d / "out_dropped" + # A --min-dur longer than every burst, so all three segments drop as short. + total = prep.process([src], out, "positive", 0.3, 5.0, 10.0) + check(total == 0, f"all-dropped input returns 0 without crashing (got {total})") + check(not out.exists(), "a run that writes no clips leaves no output dir") + + +def test_output_path_not_a_directory(d: Path) -> None: + """An --output naming an existing file reports it, rather than raising. + + The mistake used to surface as one of two exceptions depending on whether a + clip was ready to write (FileExistsError from mkdir) or not + (NotADirectoryError from the stale scan), both after decoding every input. + """ + audio = make_bursts(n=2, burst_s=0.6, gap_s=1.0, sr=prep.TARGET_SR) + src = d / "into_a_file.wav" + sf.write(src, audio, prep.TARGET_SR, subtype="PCM_16") + occupied = d / "not_a_dir" + occupied.write_text("keep me") + total = prep.process([src], occupied, "positive", 0.3, 0.2, 3.0) + check(total == 0, f"an occupied --output returns 0, not a traceback (got {total})") + check(occupied.read_text() == "keep me", "the file at --output is left untouched") + + def test_duplicate_basename_no_overwrite(d: Path) -> None: """Two inputs sharing a basename must not overwrite each other's clips. @@ -348,6 +383,8 @@ def main() -> int: test_background_kept_whole(d) test_explicit_files_only(d) test_empty_input_no_crash(d) + test_zero_clips_no_crash(d) + test_output_path_not_a_directory(d) test_duplicate_basename_no_overwrite(d) test_rerun_refreshes_populated_dir(d) test_in_run_collision_raises(d)