ADR-225: TTS sample generation stage#228
Conversation
jodavis-claude
left a comment
There was a problem hiding this comment.
ADR-225 TTS Stage — First-pass review
All exit criteria are met: TtsProvider protocol, TtsSampleGenerator with correct _derive_id/_get_applied_values/_generate_output, EdgeTtsProvider with exponential back-off retry, entry-point voice filtering, 25 unit tests covering applied-value types, rate string formatting, skip path, and transcript/label invariants. Build and tests pass. No correctness, security, or performance issues found.
Issues
Priority 4
.claude/settings.json(line 7): Unrelated change (clearingextraKnownMarketplaces) bundled into this PR. Should be separated.- Missing
ml/pipeline/speech/_doc_speech.md:ml/pipeline/speech/is a new package. CONTRIBUTING.md: "When new subsystems are added, they should include a documentation file." The spec (_spec_OopPipeline.md) is pre-implementation and cannot serve this role. A_doc_*.mdcovering the speech subsystem's design intent (TtsProvider abstraction, EdgeTtsProvider retry contract, relationship to ModifierStage) should be added before this merges.
Priority 5 (style, non-blocking)
tts_stage.pyline 91: Multi-line docstring onEdgeTtsProvider. CLAUDE.md: one short line max; remove or collapse.speech_03_generate_samples.pyline 1: 10-line module docstring with usage block. Same policy; remove or collapse to one line.
jodavis-claude
left a comment
There was a problem hiding this comment.
Sign-off review — all prior issues resolved, no new issues found.
Prior threads (all resolved):
- [P4]
.claude/settings.json:extraKnownMarketplacescleared accidentally — reverted in05abd7b, marketplace entry restored. ✓ Resolved. - [P5]
tts_stage.pyEdgeTtsProvidermulti-line docstring — removed in5608fbd. ✓ Resolved. - [P5]
speech_03_generate_samples.py10-line module docstring — removed in07d468e. ✓ Resolved.
New file scan (4 modified files: .claude/settings.json, tts_stage.py, speech_03_generate_samples.py, _doc_speech.md):
- No new Priority 1–4 issues found. The
_doc_speech.mdaddition (commit2c156c3) covers the required design decisions per CONTRIBUTING.md.
Outcome: approved — ready for human review.
|
@copilot fix the test failure in this branch. It's probably a missing dependency in requirements.txt. Verify that the build passes with no warnings or errors and that all tests pass when you are done. |
Fixed in the latest commit. |
The ADR-225 implementation commit accidentally cleared extraKnownMarketplaces; restore the jodavis-agent-plugins marketplace entry that was present before. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CONTRIBUTING.md requires a documentation file for new subsystems. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CLAUDE.md: never write multi-paragraph docstrings. The retry strategy is self-evident from the constants and loop structure. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…les.py CLAUDE.md: never write multi-paragraph docstrings. The usage block duplicates what argparse --help already surfaces. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Speech and intent pipelines have separate number spaces. The speech pipeline starts at 01, so this stage is speech_01, not speech_03.
Work item
ADR-225 — Implement the TTS sample generation stage (
TtsSampleGenerator) and its DVC entry-point, convertingManifest[TextSample]intoManifest[AudioSample]by synthesizing speech via edge_tts with per-sample voice and speech-rate variation.Changes
ml/pipeline/speech/__init__.py— package marker for the newspeechsubpackageml/pipeline/speech/tts_stage.py—TtsProviderprotocol;TtsSampleGenerator(ModifierStage[TextSample, AudioSample])with_get_applied_values,_derive_id, and_generate_output;EdgeTtsProviderconcrete implementation that streams MP3 from edge_tts, converts to WAV via soundfile, and retries up to 3 times with exponential back-offml/pipeline/stages/speech_03_generate_samples.py— DVC entry-point: fetches and filters the en-US female voice list, loads params, constructs the stage, and drivesasyncio.run(stage.transform(...))ml/pipeline/stages/params.py— addedGenerateSamplesParamsdataclass (speech_rate_min,speech_rate_max) and wired it intoPipelineParams.load()fromstages.generate_speech_samplesml/params.yaml— addedstages.generate_speech_samplessection with defaultspeech_rate_min: -10/speech_rate_max: 20ml/dvc.yaml— addedspeech_03_generate_samplesstage withpersist: trueon the output directory (required byModifierStageseed-storage contract)ml/test/pipeline/speech/__init__.py— test package markerml/test/pipeline/speech/test_tts_stage.py— 25 unit tests covering applied-values keys/types,_derive_idformat, TTS provider call arguments,AudioSample.transcriptcorrectness, rate-string formatting, and skip-path behaviourml/test/pipeline/stages/test_params.py— updated_VALID_DATAfixture and added 3 tests forGenerateSamplesParamsloadingDesign decisions
MP3 → WAV conversion via soundfile. edge_tts streams raw MP3 bytes; soundfile 0.13.1 (already installed) reads MP3 natively from a
BytesIObuffer.EdgeTtsProviderdecodes to a numpy array and re-encodes as PCM_16 WAV — no additional runtime dependency required.speech_ratestored as raw int, rate string assembled at synthesis time. Keeping+5%out ofapplied_valuesmeans the content hash is stable even if the formatting convention changes; only the semantic value matters for skip-unchanged detection.Voice list filtered in the entry-point, not in the stage.
TtsSampleGeneratoraccepts a plainlist[str], making it trivially testable with a two-element stub list without any network calls.