ADR-228: SpectrogramStage, TokenStage, SetManifestSplitter and DVC entry-points

[jodavis-claude]

Summary

• Add SpectrogramStage — deterministic ModifierStage that computes log-mel spectrograms via librosa (offloaded to thread pool), pads/truncates to (n_mels, time_steps), and writes .npy output
• Add TokenStage — deterministic ModifierStage that converts transcripts to padded phoneme token sequences using an injected VocabResult; silently skips missing words; writes .json output
• Add SetManifestSplitter — splits the fully-augmented Manifest[AudioSample] into train/val/test subsets using conventions.split_manifest_path; validates percentages sum to 100
• Add DVC entry-points speech_07_compute_tokens.py, speech_08_compute_spectrograms.py, speech_09_create_set_manifests.py
• Extend PipelineParams with ComputeSpectrogramsParams, ComputeTokensParams, CreateSetManifestsParams
• Add 66 unit tests across 4 test files (347 total passing)

Test plan

• SpectrogramStage: _is_deterministic is a class variable set to True; _derive_id returns input_sample.id; _get_applied_values returns {}; output .npy shape is (n_mels, time_steps); parent_id equals input_sample.id; skip-unchanged path does not call reader on second run
• TokenStage: same determinism checks; output JSON has phonemes and tokens keys; tokens padded to input_token_length using ctc_blank_idx; missing words skipped silently; skip-unchanged path works
• SetManifestSplitter: percentages not summing to 100 raise ValueError; split counts match percentages; all input IDs appear in exactly one split; output files named train_manifest.json, val_manifest.json, test_manifest.json; same seed produces same splits
• PipelineParams: new param sections load correctly; missing sections raise
• scripts/validate-build passes (zero warnings)
• scripts/validate-tests passes (all 347 tests)

🤖 Generated with Claude Code

[github-actions]

Test Results

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

Results for commit 198e0ae. ± Comparison against base commit acbc760.

♻️ This comment has been updated with latest results.

[jodavis-claude]

Review — ADR-228: SpectrogramStage, TokenStage, SetManifestSplitter

Summary

The implementation is correct and well-structured. All exit criteria from the task brief are met: _is_deterministic = True as a class variable, _derive_id returns input_sample.id unchanged, _get_applied_values returns {}, spectrograms are the right shape, tokens are padded/truncated to input_token_length, SetManifestSplitter validates percentages and writes via conventions.split_manifest_path, and all three DVC entry-points follow the established pattern. The 66 new tests give solid coverage.

Two issues to address before merging:

Priority 3 — Performance

TokenStage._generate_output does blocking file I/O on the async event loop (token_stage.py:77–78). SpectrogramStage correctly offloads np.save via run_in_executor (line 82). TokenStage's json.dump should be consistent. Practical impact is low today (sequential processing, fast writes), but the inconsistency with the stated pattern should be fixed.

Priority 4 — Documentation

No _doc_*.md for ml/pipeline/speech/. CONTRIBUTING.md requires documentation files for new subsystems. ADR-228 completes the first major cohort of speech pipeline stages and is the natural point to add one, covering: the two concerns in the directory (augmentation vs. featurisation), the _is_deterministic/parent_id design decisions, and SetManifestSplitter design intent.

[jodavis-claude]

Sign-off review — ADR-228 (SpectrogramStage, TokenStage, SetManifestSplitter)

Both review threads from the first pass are resolved:

1. TokenStage blocking I/O (resolved) — _generate_output now uses loop.run_in_executor(None, lambda: output_path.write_text(json.dumps(payload), encoding="utf-8")), consistent with the np.save pattern in SpectrogramStage. import asyncio added. (commit 4b6c690)

2. _doc_speech.md missing (resolved) — New doc covers both cohorts (augmentation vs. featurisation), the _is_deterministic/parent_id/_derive_id design decisions for featurisation stages, spectrogram padding/truncation strategy, blocking compute offload rationale, and SetManifestSplitter intent (augmented manifest, seed=42, stdlib random.Random, conventions.split_manifest_path naming). Links to source files throughout. (commit f3cf630)

No new issues found in the two modified files (token_stage.py, _doc_speech.md).

Outcome: approved — ready for human review.

[Copilot]

Pull request overview

Adds new speech-pipeline components in ml/ to (a) derive phoneme token targets, (b) compute log-mel spectrogram features, and (c) split an augmented manifest into train/val/test sets, along with new DVC-style stage entry points and corresponding PipelineParams/params.yaml updates.

Changes:

• Added TokenStage, SpectrogramStage, and SetManifestSplitter plus three new stage entry-point scripts (speech_07/08/09_*).
• Extended PipelineParams (and tests) to load compute_spectrograms, compute_tokens, and create_set_manifests sections.
• Added unit tests for the new stages/splitter and updated ml/params.yaml defaults.

Confidence: 95/100

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
ml/pipeline/speech/token_stage.py	New deterministic stage writing per-sample token JSON outputs.
ml/pipeline/speech/spectrogram_stage.py	New deterministic stage writing per-sample spectrogram .npy outputs.
ml/pipeline/speech/set_splitter.py	New helper to split a manifest into train/val/test and write split manifests.
ml/pipeline/stages/speech_07_compute_tokens.py	CLI entry-point to run TokenStage over a manifest.
ml/pipeline/stages/speech_08_compute_spectrograms.py	CLI entry-point to run SpectrogramStage over a manifest.
ml/pipeline/stages/speech_09_create_set_manifests.py	CLI entry-point to split a manifest into set manifests.
ml/pipeline/stages/params.py	Adds dataclasses + loader wiring for the new stage parameter sections.
ml/params.yaml	Adds default config blocks for the new stage parameters.
ml/test/pipeline/stages/test_params.py	Extends params-loading tests for the new sections.
ml/test/pipeline/speech/test_token_stage.py	Adds unit tests for TokenStage.
ml/test/pipeline/speech/test_spectrogram_stage.py	Adds unit tests for SpectrogramStage.
ml/test/pipeline/speech/test_set_splitter.py	Adds unit tests for SetManifestSplitter.