ADR-227: Add BackgroundNoiseAugmentor and MicrophoneNoiseAugmentor stages

[jodavis-claude]

Work item: ADR-227 — Implement BackgroundNoiseAugmentor and MicrophoneNoiseAugmentor, two ModifierStage subclasses that add environmental noise and microphone noise to AudioSample manifests, along with their DVC entry-point scripts.

Changes

• ml/pipeline/speech/background_noise_stage.py — New file: NoiseProvider protocol and BackgroundNoiseAugmentor(ModifierStage[AudioSample, AudioSample]). choose() always called for hash stability; all three applied_values keys always present; noise_start_s bounds derived from runtime file durations via AudioReader.
• ml/pipeline/speech/mic_noise_stage.py — New file: MicrophoneNoiseAugmentor(ModifierStage[AudioSample, AudioSample]). Generates Gaussian noise seeded from output_seed. Single applied_values key: mic_noise_amplitude.
• ml/pipeline/stages/speech_05_add_background_noise.py — New entry-point: --input-manifest-dir, --noise-dir, --output-dir; constructs BackgroundNoiseAugmentor with _DirectoryNoiseProvider.
• ml/pipeline/stages/speech_06_add_mic_noise.py — New entry-point: --input-manifest-dir, --output-dir; constructs MicrophoneNoiseAugmentor.
• ml/pipeline/stages/params.py — Added AddBackgroundNoiseParams and AddMicNoiseParams dataclasses; extended PipelineParams and PipelineParams.load() to parse the new sections.
• ml/params.yaml — Added stages.add_background_noise and stages.add_mic_noise sections with default values.
• ml/pipeline/speech/_doc_speech.md — Updated stages table to include both new stages; added design decisions for NoiseProvider, hash-stable noise file selection, runtime duration bounds, and Gaussian mic noise seeding.
• ml/test/pipeline/speech/test_background_noise_stage.py — New unit tests for BackgroundNoiseAugmentor (applied_values keys, noise_start_s clamping, derive_id format, generate_output, skip path).
• ml/test/pipeline/speech/test_mic_noise_stage.py — New unit tests for MicrophoneNoiseAugmentor (applied_values keys, derive_id format, generate_output, skip path).
• ml/test/pipeline/stages/test_params.py — Added TestAddBackgroundNoiseParamsLoad and TestAddMicNoiseParamsLoad test classes.

Design decisions

• choose() is always called regardless of should_vary. The noise filename is always selected and stored in applied_values for hash stability. Only noise_start_s and noise_volume are set to 0.0 when the variation is not applied. All three keys are always present.
• noise_start_s bounds are derived at runtime from file durations. AudioReader is called for both the noise file and the audio sample. When the noise file is shorter than the audio sample, max_start_s is forced to 0.0.
• Async duration reads in a synchronous context. _get_applied_values is synchronous per the base class contract. A _read_duration_s() helper detects whether a running event loop exists and uses ThreadPoolExecutor + asyncio.run() accordingly, satisfying both test and production paths.
• Noise mixing clips to [-1.0, 1.0]. The spec did not specify clipping; this is the standard approach and was recommended in the brief.
• Gaussian mic noise seeded from output_seed via np.random.default_rng(output_seed).normal(0, amplitude, len(samples)) for reproducibility.
• dvc.yaml wiring is out of scope — deferred to ADR-231 per the brief.

[jodavis-claude]

ADR-227 Review: BackgroundNoiseAugmentor and MicrophoneNoiseAugmentor

Build: clean (0 warnings, 0 errors). All 271 Python unit tests pass. All .NET unit and E2E tests pass.

All exit criteria from the brief are met: both stage classes are present, all three applied_values keys are always present in BackgroundNoiseAugmentor, choose() always runs for hash stability, noise_start_s/noise_volume are 0.0 when not applied, both _derive_id formats match the spec, entry-point scripts are present, and the required unit tests are all implemented.

Issues requiring changes

Priority 1 — Correctness

1. _read_duration_s blocks the event loop thread (background_noise_stage.py line 61). future.result() is called from the event loop thread, blocking it for the duration of the thread-pool execution. CONTRIBUTING.md prohibits blocking calls on async code paths. The implementation does work correctly (the worker thread creates its own event loop, avoiding deadlock), but the pattern is the Python equivalent of .Result on a Task. The executor.submit form should also use lambda: asyncio.run(_read()) instead of asyncio.run, _read() (the pre-created coroutine form, though valid, is non-obvious). The deeper fix is to move duration reads into _generate_output where an event loop is available for proper await.

2. list_files()[0].parent raises IndexError on empty noise directory (background_noise_stage.py lines 106 and 151). The noise directory is inferred from list_files()[0].parent rather than being stored directly. If the directory is empty, [0] raises IndexError with no useful error message. list_files() is also called three times across _get_applied_values and _generate_output with no caching, re-globbing the filesystem on each call. The fix is to store _noise_dir directly in the constructor (or cache the first list_files() result).

Priority 4 — Documentation / style (non-blocking)

3. _DirectoryNoiseProvider.list_files() gives no actionable error on empty directory (speech_05_add_background_noise.py line 30). The error eventually surfaces as ValueError("options must be non-empty") from VariationGenerator.choose() — a confusing message when the root cause is a misconfigured --noise-dir. Adding a guard in list_files() or __init__ would make misconfiguration immediately diagnosable.

Priority 5 — Style (non-blocking)

4. from typing import Any is imported but unused in test_background_noise_stage.py (line 9) and test_mic_noise_stage.py (line 9).
5. import pytest is imported but unused in test_background_noise_stage.py (line 12) and test_mic_noise_stage.py (line 11).

The two correctness issues (#1 and #2) require changes before merge.

[github-actions]

Test Results

401 tests  ±0   401 ✅ ±0   2m 3s ⏱️ -44s
  5 suites ±0     0 💤 ±0 
  5 files   ±0     0 ❌ ±0 

Results for commit 852e879. ± Comparison against base commit 82e5bb7.

♻️ This comment has been updated with latest results.

[jodavis-claude]

Placeholder — submitting to clear pending review state before adding replies.

[jodavis-claude]

Sign-off review — all 6 prior threads resolved; no new issues found.

Thread outcomes:

1. Priority 1 — executor.submit coroutine form / future.result() blocking the event loop (PRRT_kwDOQYcoLM6KZwqe): Developer applied the lambda fix correctly (executor.submit(lambda: asyncio.run(_read()))). Rationale for not restructuring into _generate_output is accepted — moving the duration reads there would diverge the skip-check hash from the _get_applied_values hash, causing every sample to regenerate on every run. The future.result() block is the least-bad option under the current ModifierStage contract. Thread resolved.

2. Priority 1 — Redundant list_files()[0].parent + IndexError risk on line 106 (PRRT_kwDOQYcoLM6KZwxo): Fixed. noise_dir: Path added as constructor parameter; self._noise_dir stored and used throughout. list_files()[0].parent calls eliminated. Thread resolved (outdated).

3. Priority 1 — Third list_files()[0].parent call in _generate_output (PRRT_kwDOQYcoLM6KZw1l): Fixed. Same self._noise_dir / noise_file pattern applied. Thread resolved (outdated).

4. Style — unused from typing import Any in test files (PRRT_kwDOQYcoLM6KZw4j): Fixed. Import removed from both test files. Thread resolved (outdated).

5. Style — unused import pytest in test files (PRRT_kwDOQYcoLM6KZw7K): Fixed. Import removed from both test files. Thread resolved (outdated).

6. Correctness (minor) — _DirectoryNoiseProvider.list_files() did not validate empty directory (PRRT_kwDOQYcoLM6KZw-2): Fixed. list_files() now raises ValueError(f"No WAV files found in noise directory: {self._noise_dir}") before returning an empty list. Thread resolved (outdated).

Modified files scanned for new issues: ml/pipeline/speech/background_noise_stage.py, ml/pipeline/stages/speech_05_add_background_noise.py, ml/test/pipeline/speech/test_background_noise_stage.py, ml/test/pipeline/speech/test_mic_noise_stage.py — no new Priority 1–4 issues found.

Approved for human review.

[jodavis-claude]

Sign-off review — all 6 prior threads resolved; no new Priority 1–4 issues found in modified files.

Thread outcomes:

1. Priority 1 — executor.submit coroutine form / future.result() blocking (PRRT_kwDOQYcoLM6KZwqe): Lambda fix verified — executor.submit(lambda: asyncio.run(_read())) creates the coroutine inside the worker thread. Developer rationale for not restructuring into _generate_output accepted: moving noise_start_s computation there would diverge the skip-check hash and break the ModifierStage contract. Resolved.
2. Priority 1 — Redundant list_files()[0].parent + IndexError risk at line 106 (PRRT_kwDOQYcoLM6KZwxo): Fixed. noise_dir: Path constructor parameter added; self._noise_dir used throughout. Resolved (outdated).
3. Priority 1 — Third list_files()[0].parent call in _generate_output (PRRT_kwDOQYcoLM6KZw1l): Fixed. self._noise_dir / noise_file pattern applied. Resolved (outdated).
4. Style — unused from typing import Any in test files (PRRT_kwDOQYcoLM6KZw4j): Fixed. Import removed from both test files. Resolved (outdated).
5. Style — unused import pytest in test files (PRRT_kwDOQYcoLM6KZw7K): Fixed. Import removed from both test files. Resolved (outdated).
6. Correctness (minor) — _DirectoryNoiseProvider.list_files() no empty-directory validation (PRRT_kwDOQYcoLM6KZw-2): Fixed. Raises ValueError with directory path if no WAV files found. Resolved (outdated).

Scan of modified files (background_noise_stage.py, speech_05_add_background_noise.py, test_background_noise_stage.py, test_mic_noise_stage.py): no new issues found. The lambda closure correctly captures per-call audio_reader and path bindings; noise mixing arithmetic (add + clip + astype float32) is correct; empty-directory validation is present and actionable.

[Copilot]

Pull request overview

This PR adds two new ML pipeline augmentation stages for speech data generation: one for mixing in environmental background noise and one for adding Gaussian microphone noise, including CLI entry-points, params wiring, docs, and unit tests.

Changes:

• Added BackgroundNoiseAugmentor (noise file selection + bounded start offset + mixing/clipping) and MicrophoneNoiseAugmentor (seeded Gaussian noise + clipping).
• Added stage entry-point scripts and new params.yaml sections + params parsing dataclasses.
• Added/updated documentation and unit tests covering applied-values, ID derivation, generation, and params loading.

Confidence: 92/100 (main uncertainty: whether the pipeline guarantees a single, fixed sample rate across generated speech WAVs and noise WAVs; current code does not enforce it).

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
ml/pipeline/speech/background_noise_stage.py	New background-noise mixing stage + duration-bounded noise slicing
ml/pipeline/speech/mic_noise_stage.py	New microphone Gaussian-noise stage (seeded, reproducible)
ml/pipeline/stages/speech_05_add_background_noise.py	New CLI entry-point wiring for background noise stage
ml/pipeline/stages/speech_06_add_mic_noise.py	New CLI entry-point wiring for mic noise stage
ml/pipeline/stages/params.py	Added params dataclasses + parsing for the two new stages
ml/params.yaml	Added default config sections for background noise and mic noise
ml/pipeline/speech/_doc_speech.md	Documented new stages and design decisions
ml/test/pipeline/speech/test_background_noise_stage.py	Unit tests for background noise stage behavior
ml/test/pipeline/speech/test_mic_noise_stage.py	Unit tests for mic noise stage behavior
ml/test/pipeline/stages/test_params.py	Extended params load tests for new stage config sections