diff --git a/milestones/README.md b/milestones/README.md index 323d8e4..aad0fab 100644 --- a/milestones/README.md +++ b/milestones/README.md @@ -5,11 +5,13 @@ 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:** **[manim-font-quality.md](manim-font-quality.md)** — -`manim.font` / `quality` / `manim_path` must be YAML strings at config -load. +**Active:** **[generation-model-strings.md](generation-model-strings.md)** — +`narration_from_source` / `manim_scene_generation` model and prompt keys +must be YAML strings at config load. **Shipped:** +- **[manim-font-quality.md](manim-font-quality.md)** — + `manim.font` / `quality` / `manim_path` must be YAML strings (#106). - **[ai-timestamp-strings.md](ai-timestamp-strings.md)** — `ai.provider` / `timestamps.engine` / `tts.language` must be YAML strings (#105). diff --git a/milestones/generation-model-strings.md b/milestones/generation-model-strings.md new file mode 100644 index 0000000..8f1408f --- /dev/null +++ b/milestones/generation-model-strings.md @@ -0,0 +1,46 @@ +# Milestone: narration / scene-generation model and prompts must be strings + +**Status:** Active +**PR:** [#107](https://github.com/jmjava/documentation-generator/pull/107) +**Depends on:** `milestones/manim-font-quality.md` (PR #106), +`milestones/wizard-prompt-strings.md` (PR #100) + +## Problem + +`wizard.llm_model` / `wizard.system_prompt` and `tts.model` are already +typed at config load. These LLM keys were not: + +1. **`narration_from_source.model`** — `str()` turned a YAML list into + `"['gpt-4o']"` and sent it to the chat API. +2. **`narration_from_source.system_prompt`** — a list became a bracketed + string used as the narration system prompt. +3. **`manim_scene_generation.model` / `system_prompt` / + `scene_spec_system_prompt`** — same `str()` coercion into scene-spec + generation. + +## Goal + +Fail closed at `Config.from_yaml`. Missing keys still use code defaults +(`gpt-4o` / built-in prompts). Empty `system_prompt` strings remain +allowed (same as `wizard.system_prompt`). + +## Done when + +- [x] Present `narration_from_source.model` must be a non-empty YAML + string. +- [x] Present `narration_from_source.system_prompt` must be a YAML + string (empty allowed). +- [x] Present `manim_scene_generation.model` must be a non-empty YAML + string. +- [x] Present `manim_scene_generation.system_prompt` / + `scene_spec_system_prompt` must be YAML strings (empty allowed). +- [x] Tests for list values of those keys. +- [x] `ruff check src/ tests/` +- [x] `pytest tests/` +- [x] `docgen benchmark` (no clock change) + +## Out of scope + +- Per-segment `system_prompt` / `class_name` under those blocks. +- `wizard.default_guidance` type gating is separate. +- `env_file` / `repo_root` / `dirs.*` path strings are separate. diff --git a/milestones/manim-font-quality.md b/milestones/manim-font-quality.md index e5ac9da..de36d0b 100644 --- a/milestones/manim-font-quality.md +++ b/milestones/manim-font-quality.md @@ -1,7 +1,7 @@ # Milestone: manim.font / quality / manim_path must be strings -**Status:** Active -**PR:** [#106](https://github.com/jmjava/documentation-generator/pull/106) +**Status:** Shipped +**PR:** #106 **Depends on:** `milestones/ai-timestamp-strings.md` (PR #105) ## Problem diff --git a/src/docgen/config.py b/src/docgen/config.py index 0c027f6..f8e8bd4 100644 --- a/src/docgen/config.py +++ b/src/docgen/config.py @@ -308,6 +308,37 @@ def __post_init__(self) -> None: require_yaml_string(manim["quality"], label="manim.quality", source=src) if manim.get("manim_path") is not None: require_yaml_string(manim["manim_path"], label="manim.manim_path", source=src) + nfs = self._block("narration_from_source") + if nfs.get("model") is not None: + require_yaml_string( + nfs["model"], label="narration_from_source.model", source=src + ) + if nfs.get("system_prompt") is not None and not isinstance( + nfs["system_prompt"], str + ): + raise ConfigError( + f"{src}: narration_from_source.system_prompt must be a YAML string, " + f"not {type(nfs['system_prompt']).__name__}" + ) + msg = self._block("manim_scene_generation") + if msg.get("model") is not None: + require_yaml_string( + msg["model"], label="manim_scene_generation.model", source=src + ) + if msg.get("system_prompt") is not None and not isinstance( + msg["system_prompt"], str + ): + raise ConfigError( + f"{src}: manim_scene_generation.system_prompt must be a YAML string, " + f"not {type(msg['system_prompt']).__name__}" + ) + if msg.get("scene_spec_system_prompt") is not None and not isinstance( + msg["scene_spec_system_prompt"], str + ): + raise ConfigError( + f"{src}: manim_scene_generation.scene_spec_system_prompt must be a " + f"YAML string, not {type(msg['scene_spec_system_prompt']).__name__}" + ) ocr = self._sub_block(validation, "ocr", label="validation.ocr") if ocr.get("error_patterns") is not None: string_list_block( diff --git a/tests/test_config.py b/tests/test_config.py index 3209f9b..aade83d 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -535,3 +535,58 @@ def test_from_yaml_list_manim_path_raises(tmp_path: Path) -> None: p.write_text("manim:\n manim_path:\n - /usr/bin/manim\n", encoding="utf-8") with pytest.raises(ConfigError, match="manim.manim_path must be a YAML string"): Config.from_yaml(p) + + +def test_from_yaml_list_narration_from_source_model_raises(tmp_path: Path) -> None: + p = tmp_path / "docgen.yaml" + p.write_text("narration_from_source:\n model:\n - gpt-4o\n", encoding="utf-8") + with pytest.raises(ConfigError, match="narration_from_source.model must be a YAML string"): + Config.from_yaml(p) + + +def test_from_yaml_list_narration_from_source_system_prompt_raises(tmp_path: Path) -> None: + p = tmp_path / "docgen.yaml" + p.write_text( + "narration_from_source:\n system_prompt:\n - Write narration\n", + encoding="utf-8", + ) + with pytest.raises( + ConfigError, match="narration_from_source.system_prompt must be a YAML string" + ): + Config.from_yaml(p) + + +def test_from_yaml_list_manim_scene_generation_model_raises(tmp_path: Path) -> None: + p = tmp_path / "docgen.yaml" + p.write_text("manim_scene_generation:\n model:\n - gpt-4o\n", encoding="utf-8") + with pytest.raises( + ConfigError, match="manim_scene_generation.model must be a YAML string" + ): + Config.from_yaml(p) + + +def test_from_yaml_list_manim_scene_generation_system_prompt_raises( + tmp_path: Path, +) -> None: + p = tmp_path / "docgen.yaml" + p.write_text( + "manim_scene_generation:\n system_prompt:\n - Draw boxes\n", + encoding="utf-8", + ) + with pytest.raises( + ConfigError, match="manim_scene_generation.system_prompt must be a YAML string" + ): + Config.from_yaml(p) + + +def test_from_yaml_list_scene_spec_system_prompt_raises(tmp_path: Path) -> None: + p = tmp_path / "docgen.yaml" + p.write_text( + "manim_scene_generation:\n scene_spec_system_prompt:\n - Cover beats\n", + encoding="utf-8", + ) + with pytest.raises( + ConfigError, + match="manim_scene_generation.scene_spec_system_prompt must be a YAML string", + ): + Config.from_yaml(p)