diff --git a/milestones/README.md b/milestones/README.md index 71ea156..e102ecf 100644 --- a/milestones/README.md +++ b/milestones/README.md @@ -5,11 +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:** **[empty-segments-all.md](empty-segments-all.md)** — -explicit empty `segments.all: []` must not fall through to -`segments.default` in yaml-generate. +**Active:** **[cli-segments-all.md](cli-segments-all.md)** — +`narration-generate --all` / `scene-spec-generate --all` must use +`Config.segments_all` (missing `all` falls back to `default`). **Shipped:** +- **[empty-segments-all.md](empty-segments-all.md)** — + explicit empty `segments.all: []` must not fall through to + `segments.default` in yaml-generate (#124). - **[concat-ffmpeg-timeout.md](concat-ffmpeg-timeout.md)** — concat must not leave a truncated full-demo mp4 after ffmpeg timeout or failure (#123). diff --git a/milestones/cli-segments-all.md b/milestones/cli-segments-all.md new file mode 100644 index 0000000..de78145 --- /dev/null +++ b/milestones/cli-segments-all.md @@ -0,0 +1,34 @@ +# Milestone: CLI --all must use Config.segments_all + +**Status:** Active +**PR:** [#125](https://github.com/jmjava/documentation-generator/pull/125) +**Depends on:** `milestones/empty-segments-all.md` (PR #124) + +## Problem + +``narration-generate --all`` and ``scene-spec-generate --all`` read +``raw["segments"]["all"] or []``. Missing ``all`` with a populated +``segments.default`` raised ``segments.all is empty`` even though +``Config.segments_all`` (TTS, timestamps, lint, pipeline) falls back +to ``default``. + +An explicit empty ``all: []`` must still fail (same as Config). + +## Goal + +Both commands use ``cfg.segments_all``. Missing ``all`` uses +``default``. Empty ``all: []`` still errors. + +## Done when + +- [x] ``--all`` with only ``default`` processes those ids. +- [x] ``--all`` with ``all: []`` still errors even if ``default`` has + ids. +- [x] `ruff check src/ tests/` +- [x] `pytest tests/` (717 passed, 1 skipped) +- [x] `docgen benchmark` (no clock change; meets baseline) + +## Out of scope + +- Changing TTS / timestamps empty-all contracts. +- Integer segment ids (already rejected at Config load). diff --git a/milestones/empty-segments-all.md b/milestones/empty-segments-all.md index 6bda136..920c1a1 100644 --- a/milestones/empty-segments-all.md +++ b/milestones/empty-segments-all.md @@ -1,6 +1,6 @@ # Milestone: empty segments.all must not fall through to default -**Status:** Active +**Status:** Shipped **PR:** [#124](https://github.com/jmjava/documentation-generator/pull/124) **Depends on:** `milestones/concat-ffmpeg-timeout.md` (PR #123), `milestones/yaml-generate-mappings.md` (PR #91) diff --git a/src/docgen/cli.py b/src/docgen/cli.py index 87b558a..7efc6d8 100644 --- a/src/docgen/cli.py +++ b/src/docgen/cli.py @@ -683,7 +683,7 @@ def _one(seg_str: str) -> None: click.echo(f" -> {out}" if all_segments else f"[narration-generate] wrote {out}") if all_segments: - ids = list((cfg.raw.get("segments") or {}).get("all") or []) + ids = list(cfg.segments_all) if not ids: raise click.ClickException("segments.all is empty in docgen.yaml") for seg_id in ids: @@ -933,10 +933,10 @@ def _one_sid(sid: str) -> None: ) if all_segments: - ids = list((cfg.raw.get("segments") or {}).get("all") or []) + ids = list(cfg.segments_all) if not ids: raise click.ClickException("segments.all is empty in docgen.yaml") - names = (cfg.raw.get("segment_names") or {}) + names = cfg.segment_names scripts_dir = cfg.base_dir / "scripts" failures: list[str] = [] for seg_id in ids: diff --git a/tests/test_narrate_from_source.py b/tests/test_narrate_from_source.py index f9996c6..59c379c 100644 --- a/tests/test_narrate_from_source.py +++ b/tests/test_narrate_from_source.py @@ -248,3 +248,62 @@ def test_narration_generate_cli_dry_run(tmp_path: Path) -> None: assert r.exit_code == 0, r.output assert "CLI out." in r.output assert not (tmp_path / "narration").exists() or not list((tmp_path / "narration").glob("*.md")) + + +def test_narration_generate_all_uses_default_when_all_missing(tmp_path: Path) -> None: + from click.testing import CliRunner + + from docgen.cli import main + + (tmp_path / ".git").mkdir() + (tmp_path / "docgen.yaml").write_text( + yaml.dump( + { + "segments": {"default": ["01"]}, + "segment_names": {"01": "01-demo"}, + "narration_from_source": {"context": {"paths": ["x.md"]}}, + } + ), + encoding="utf-8", + ) + (tmp_path / "x.md").write_text("# src\nbody", encoding="utf-8") + runner = CliRunner() + with patch("docgen.wizard.generate_narration_via_llm") as m: + m.return_value = "From default.\n" + r = runner.invoke( + main, + [ + "--config", + str(tmp_path / "docgen.yaml"), + "narration-generate", + "--all", + "--dry-run", + ], + ) + assert r.exit_code == 0, r.output + assert "segments.all is empty" not in (r.output + r.stderr) + assert "From default." in r.output + assert m.called + + +def test_narration_generate_all_empty_all_raises_even_with_default(tmp_path: Path) -> None: + from click.testing import CliRunner + + from docgen.cli import main + + (tmp_path / "docgen.yaml").write_text( + yaml.dump( + { + "segments": {"default": ["01"], "all": []}, + "segment_names": {"01": "01-demo"}, + } + ), + encoding="utf-8", + ) + runner = CliRunner() + r = runner.invoke( + main, + ["--config", str(tmp_path / "docgen.yaml"), "narration-generate", "--all"], + ) + assert r.exit_code != 0 + assert "segments.all is empty" in (r.output + r.stderr) diff --git a/tests/test_scene_spec_generate.py b/tests/test_scene_spec_generate.py index 5aaded9..32f0a10 100644 --- a/tests/test_scene_spec_generate.py +++ b/tests/test_scene_spec_generate.py @@ -368,3 +368,54 @@ def test_user_message_includes_computed_layout_stack_budgets() -> None: assert f"{budget_default:.2f}" in msg assert f"{budget_compact:.2f}" in msg assert "13.22" in msg # horizontal safe width (FRAME_WIDTH - 1.0) + + +def test_scene_spec_generate_all_uses_default_when_all_missing(tmp_path: Path) -> None: + from click.testing import CliRunner + + from docgen.cli import main + + p = tmp_path / "docgen.yaml" + p.write_text( + yaml.dump( + { + "dirs": {"narration": "narration", "animations": "animations"}, + "segments": {"default": ["01"]}, + "segment_names": {"01": "01-demo"}, + } + ), + encoding="utf-8", + ) + runner = CliRunner() + result = runner.invoke( + main, + ["--config", str(p), "scene-spec-generate", "--all", "--dry-run"], + ) + combined = result.output + result.stderr + assert "segments.all is empty" not in combined + assert "=== scene-spec-generate --segment 01 ===" in combined + + +def test_scene_spec_generate_all_empty_all_raises_even_with_default( + tmp_path: Path, +) -> None: + from click.testing import CliRunner + + from docgen.cli import main + + p = tmp_path / "docgen.yaml" + p.write_text( + yaml.dump( + { + "segments": {"default": ["01"], "all": []}, + "segment_names": {"01": "01-demo"}, + } + ), + encoding="utf-8", + ) + runner = CliRunner() + result = runner.invoke( + main, ["--config", str(p), "scene-spec-generate", "--all"] + ) + assert result.exit_code != 0 + assert "segments.all is empty" in (result.output + result.stderr)