diff --git a/milestones/README.md b/milestones/README.md index 2152d1c..cbddcd6 100644 --- a/milestones/README.md +++ b/milestones/README.md @@ -5,10 +5,14 @@ repositories that install `docgen` and maintain their own demo bundle. The library no longer ships an in-repo dogfood; consumers are the integration test of record. -**Active:** **[av-sync-scene-spec.md](av-sync-scene-spec.md)** — -Unreadable `*.scene.yaml` must fail `av_sync`, not fall back to transcript nouns. +**Active:** **[tts-empty-audio.md](tts-empty-audio.md)** — +TTS must not succeed with an empty mp3; `tts.model` / `voice` / `instructions` +must be YAML strings. **Shipped:** +- **[av-sync-scene-spec.md](av-sync-scene-spec.md)** — + Unreadable `*.scene.yaml` must fail `av_sync`, not fall back to transcript + nouns (#98). - **[layout-check-errors.md](layout-check-errors.md)** — Manim layout check crashes must fail, not skip as success (#97). - **[manim-unsafe-unicode.md](manim-unsafe-unicode.md)** — diff --git a/milestones/av-sync-scene-spec.md b/milestones/av-sync-scene-spec.md index de9ef58..e3dfba9 100644 --- a/milestones/av-sync-scene-spec.md +++ b/milestones/av-sync-scene-spec.md @@ -1,7 +1,7 @@ # Milestone: av_sync must not skip unreadable scene specs -**Status:** Active -**PR:** pending +**Status:** Shipped +**PR:** #98 **Depends on:** `milestones/layout-check-errors.md` (PR #97), `milestones/validate-fail-closed.md` diff --git a/milestones/tts-empty-audio.md b/milestones/tts-empty-audio.md new file mode 100644 index 0000000..f1b66c9 --- /dev/null +++ b/milestones/tts-empty-audio.md @@ -0,0 +1,40 @@ +# Milestone: TTS must not succeed with empty audio or list-valued strings + +**Status:** Active +**PR:** pending +**Depends on:** `milestones/av-sync-scene-spec.md` (PR #98), +`milestones/manim-lint-config.md` (PR #82) + +## Problem + +1. `TTSGenerator` printed **Wrote** after `synthesize_speech` even when the + provider wrote a **0-byte** mp3 (or no file). Downstream timestamps then + ran against empty audio. Chat already fails closed on empty text; TTS did + not. +2. xAI `_grok_tts` wrote whatever bytes came back, including empty. +3. `tts.model` / `tts.voice` / `tts.instructions` as a YAML **list** loaded + and were passed to the API (`instructions:` as a bullet list is a common + typo). `tts.instructions` is typed `str`. + +## Goal + +Fail closed at config load for non-string TTS fields. Fail closed after +synthesize if the mp3 is missing or empty (unlink the empty file). Grok TTS +raises on an empty HTTP body. + +## Done when + +- [x] Present `tts.model` / `tts.voice` must be non-empty YAML strings. +- [x] Present `tts.instructions` must be a YAML string (empty string allowed). +- [x] `TTSGenerator` raises `TTSError` and removes a 0-byte output file. +- [x] xAI TTS raises `AIError` on an empty body (does not write the file). +- [x] Tests for list-valued fields, empty provider output, empty Grok body. +- [x] `ruff check src/ tests/` +- [x] `pytest tests/` +- [x] `docgen benchmark` (no clock change) + +## Out of scope + +- Wizard PUT of empty narration still writes the file (lint/TTS catch later). +- `wizard.system_prompt` / `llm_model` type gating is separate. +- Missing `tts` keys still use the built-in defaults. diff --git a/src/docgen/ai_client.py b/src/docgen/ai_client.py index b98c8ea..9704a5b 100644 --- a/src/docgen/ai_client.py +++ b/src/docgen/ai_client.py @@ -626,6 +626,8 @@ def _grok_tts( accept="audio/mpeg", error_label="xAI", ) + if not body: + raise AIError("xAI TTS returned empty audio") output_path.parent.mkdir(parents=True, exist_ok=True) output_path.write_bytes(body) diff --git a/src/docgen/config.py b/src/docgen/config.py index 283aa7b..eeb9b7d 100644 --- a/src/docgen/config.py +++ b/src/docgen/config.py @@ -264,6 +264,17 @@ def __post_init__(self) -> None: label="wizard.scan_extensions", source=src, ) + tts = self._block("tts") + if tts.get("model") is not None: + require_yaml_string(tts["model"], label="tts.model", source=src) + if tts.get("voice") is not None: + require_yaml_string(tts["voice"], label="tts.voice", source=src) + inst = tts.get("instructions") + if inst is not None and not isinstance(inst, str): + raise ConfigError( + f"{src}: tts.instructions must be a YAML string, " + f"not {type(inst).__name__}" + ) ocr = self._sub_block(validation, "ocr", label="validation.ocr") if ocr.get("error_patterns") is not None: string_list_block( diff --git a/src/docgen/tts.py b/src/docgen/tts.py index 08fe051..1227d38 100644 --- a/src/docgen/tts.py +++ b/src/docgen/tts.py @@ -131,6 +131,13 @@ def _generate_one(self, seg_id: str, dry_run: bool) -> None: output_path=out_path, cfg=self.config, ) + if not out_path.is_file() or out_path.stat().st_size == 0: + if out_path.is_file(): + out_path.unlink() + raise TTSError( + f"TTS wrote no audio for {seg_id} ({out_path.name}) — " + "the provider returned an empty file" + ) print(f"[tts] Wrote {out_path}") new_duration = _probe_duration(out_path) diff --git a/tests/test_ai_client.py b/tests/test_ai_client.py index aa3fa06..01898d0 100644 --- a/tests/test_ai_client.py +++ b/tests/test_ai_client.py @@ -190,6 +190,29 @@ def _http(url: str, *, data: bytes, headers: dict, **_kwargs) -> bytes: assert out.read_bytes() == b"ID3fake" +def test_grok_tts_empty_body_raises(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("DOCGEN_AI_PROVIDER", "grok") + monkeypatch.setenv("XAI_API_KEY", "xai-test") + cfg = _cfg(tmp_path, {}) + out = tmp_path / "n.mp3" + + def _http(url: str, *, data: bytes, headers: dict, **_kwargs) -> bytes: + assert url.endswith("/tts") + return b"" + + with patch("docgen.ai_client._http_with_retries", side_effect=_http): + with pytest.raises(AIError, match="empty audio"): + synthesize_speech( + text="Hello", + model="gpt-4o-mini-tts", + voice="coral", + instructions="unused on grok", + output_path=out, + cfg=cfg, + ) + assert not out.exists() + + def test_grok_stt_maps_words(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("DOCGEN_AI_PROVIDER", "grok") monkeypatch.setenv("XAI_API_KEY", "xai-test") diff --git a/tests/test_config.py b/tests/test_config.py index cdd95f0..8b5d518 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -378,3 +378,27 @@ def test_from_yaml_string_manim_unsafe_unicode_raises(tmp_path: Path) -> None: p.write_text("manim:\n unsafe_unicode: \"\\u2192\"\n", encoding="utf-8") with pytest.raises(ConfigError, match="manim.unsafe_unicode must be a YAML list"): Config.from_yaml(p) + + +def test_from_yaml_list_tts_instructions_raises(tmp_path: Path) -> None: + p = tmp_path / "docgen.yaml" + p.write_text( + "tts:\n instructions:\n - Speak calmly\n - Pronounce YAML as camel\n", + encoding="utf-8", + ) + with pytest.raises(ConfigError, match="tts.instructions must be a YAML string"): + Config.from_yaml(p) + + +def test_from_yaml_list_tts_voice_raises(tmp_path: Path) -> None: + p = tmp_path / "docgen.yaml" + p.write_text("tts:\n voice:\n - coral\n", encoding="utf-8") + with pytest.raises(ConfigError, match="tts.voice must be a YAML string"): + Config.from_yaml(p) + + +def test_from_yaml_list_tts_model_raises(tmp_path: Path) -> None: + p = tmp_path / "docgen.yaml" + p.write_text("tts:\n model:\n - gpt-4o-mini-tts\n", encoding="utf-8") + with pytest.raises(ConfigError, match="tts.model must be a YAML string"): + Config.from_yaml(p) diff --git a/tests/test_tts.py b/tests/test_tts.py index 1bfd56b..32980bc 100644 --- a/tests/test_tts.py +++ b/tests/test_tts.py @@ -131,6 +131,30 @@ def test_tts_empty_segments_all_raises(tmp_path: Path) -> None: TTSGenerator(cfg).generate(dry_run=True) +def test_tts_empty_provider_output_raises(tmp_path: Path) -> None: + raw = { + "dirs": {"narration": "narration", "audio": "audio"}, + "segments": {"all": ["01"], "default": ["01"]}, + "segment_names": {"01": "01-intro"}, + "validation": {"narration_lint": {"block_tts_on_pre_lint": False}}, + } + p = tmp_path / "docgen.yaml" + p.write_text(yaml.dump(raw), encoding="utf-8") + narr = tmp_path / "narration" + narr.mkdir() + (narr / "01-intro.md").write_text("Spoken line.\n", encoding="utf-8") + cfg = Config.from_yaml(p) + + def _write_empty(*, output_path: Path, **_kwargs: object) -> None: + output_path.parent.mkdir(parents=True, exist_ok=True) + output_path.write_bytes(b"") + + with patch("docgen.ai_client.synthesize_speech", side_effect=_write_empty): + with pytest.raises(TTSError, match="wrote no audio"): + TTSGenerator(cfg).generate(segment="01") + assert not (tmp_path / "audio" / "01-intro.mp3").exists() + + def test_probe_duration_returns_none_for_missing_file(tmp_path): result = _probe_duration(tmp_path / "nonexistent.mp3") assert result is None