Fail closed when per-segment generation prompts are a list

milestones/README.md

@@ -5,10 +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:** **[path-config-strings.md](path-config-strings.md)** —
-`env_file` / `repo_root` / `dirs.*` must be YAML strings at config load.
+**Active:** **[generation-segment-strings.md](generation-segment-strings.md)** —
+per-segment narration / scene-generation prompts must be YAML strings
+at config load.
 
 **Shipped:**
+- **[path-config-strings.md](path-config-strings.md)** —
+  `env_file` / `repo_root` / `dirs.*` must be YAML strings (#109).
 - **[visual-map-field-strings.md](visual-map-field-strings.md)** —
   `visual_map` type/scene/source and `segment_names` values must be YAML
   strings (#108).

milestones/generation-segment-strings.md

@@ -0,0 +1,39 @@
+# Milestone: per-segment generation prompts must be strings
+
+**Status:** Active  
+**PR:** [#110](https://github.com/jmjava/documentation-generator/pull/110)  
+**Depends on:** `milestones/generation-model-strings.md` (PR #107),
+`milestones/path-config-strings.md` (PR #109)
+
+## Problem
+
+Root `narration_from_source.system_prompt` / `manim_scene_generation.model`
+are already typed at config load. Per-segment keys were not:
+
+1. **`narration_from_source.segments.<id>.system_prompt` / `topic`** —
+   `str()` turned a YAML list into a bracketed string used as the
+   narration system prompt or topic line.
+2. **`manim_scene_generation.segments.<id>.class_name` /
+   `system_prompt` / `scene_spec_system_prompt`** — same coercion into
+   the Manim class name or scene-spec system prompt.
+
+Empty `system_prompt` strings remain allowed (same as the root keys).
+
+## Goal
+
+Fail closed at `Config.from_yaml` while walking those segment maps.
+
+## Done when
+
+- [x] Present per-segment `system_prompt` / `topic` /
+      `scene_spec_system_prompt` must be YAML strings (empty allowed).
+- [x] Present per-segment `class_name` must be a non-empty YAML string.
+- [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
+
+- `wizard.default_guidance` type gating is separate.
+- `pages.docs_dir` / `title` path and copy strings are separate.

milestones/path-config-strings.md

@@ -1,7 +1,7 @@
 # Milestone: env_file / repo_root / dirs paths must be strings
 
-**Status:** Active  
-**PR:** [#109](https://github.com/jmjava/documentation-generator/pull/109)  
+**Status:** Shipped  
+**PR:** #109  
 **Depends on:** `milestones/visual-map-field-strings.md` (PR #108)
 
 ## Problem

src/docgen/config.py

@@ -78,6 +78,14 @@ def mapping_sub_block(
     return val
 
 
+def require_optional_yaml_string(value: Any, *, label: str, source: str) -> None:
+    """Allow missing/null; a present value must be a YAML string (empty OK)."""
+    if value is not None and not isinstance(value, str):
+        raise ConfigError(
+            f"{source}: {label} must be a YAML string, not {type(value).__name__}"
+        )
+
+
 def require_yaml_string(value: Any, *, label: str, source: str) -> str:
     """Require a YAML string so unquoted ``01`` is not silently coerced to ``\"1\"``."""
     if isinstance(value, str):
@@ -146,6 +154,27 @@ def require_hint_and_context_lists(
                 f"{source}: {prefix}.segments.{sid_s} must be a YAML mapping, "
                 f"not {type(spec).__name__}"
             )
+        require_optional_yaml_string(
+            spec.get("system_prompt"),
+            label=f"{prefix}.segments.{sid_s}.system_prompt",
+            source=source,
+        )
+        require_optional_yaml_string(
+            spec.get("topic"),
+            label=f"{prefix}.segments.{sid_s}.topic",
+            source=source,
+        )
+        require_optional_yaml_string(
+            spec.get("scene_spec_system_prompt"),
+            label=f"{prefix}.segments.{sid_s}.scene_spec_system_prompt",
+            source=source,
+        )
+        if spec.get("class_name") is not None:
+            require_yaml_string(
+                spec["class_name"],
+                label=f"{prefix}.segments.{sid_s}.class_name",
+                source=source,
+            )
         require_hint_and_context_lists(spec, prefix=f"{prefix}.segments.{sid_s}", source=source)
 
 
@@ -287,10 +316,9 @@ def __post_init__(self) -> None:
                 f"{src}: tts.instructions must be a YAML string, "
                 f"not {type(inst).__name__}"
             )
-        if wiz.get("system_prompt") is not None and not isinstance(wiz["system_prompt"], str):
-            raise ConfigError(
-                f"{src}: wizard.system_prompt must be a YAML string, "
-                f"not {type(wiz['system_prompt']).__name__}"
+        if wiz.get("system_prompt") is not None:
+            require_optional_yaml_string(
+                wiz["system_prompt"], label="wizard.system_prompt", source=src
             )
         if wiz.get("llm_model") is not None:
             require_yaml_string(wiz["llm_model"], label="wizard.llm_model", source=src)
@@ -325,31 +353,28 @@ def __post_init__(self) -> 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__}"
+        if nfs.get("system_prompt") is not None:
+            require_optional_yaml_string(
+                nfs["system_prompt"],
+                label="narration_from_source.system_prompt",
+                source=src,
             )
         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("system_prompt") is not None:
+            require_optional_yaml_string(
+                msg["system_prompt"],
+                label="manim_scene_generation.system_prompt",
+                source=src,
             )
-        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__}"
+        if msg.get("scene_spec_system_prompt") is not None:
+            require_optional_yaml_string(
+                msg["scene_spec_system_prompt"],
+                label="manim_scene_generation.scene_spec_system_prompt",
+                source=src,
             )
         if self.raw.get("env_file") is not None:
             require_yaml_string(self.raw["env_file"], label="env_file", source=src)

tests/test_config.py

@@ -652,3 +652,68 @@ def test_from_yaml_list_dirs_narration_raises(tmp_path: Path) -> None:
     p.write_text("dirs:\n  narration:\n    - narration\n", encoding="utf-8")
     with pytest.raises(ConfigError, match="dirs.narration must be a YAML string"):
         Config.from_yaml(p)
+
+
+def test_from_yaml_list_nfs_segment_system_prompt_raises(tmp_path: Path) -> None:
+    p = tmp_path / "docgen.yaml"
+    p.write_text(
+        'narration_from_source:\n  segments:\n    "01":\n      system_prompt:\n'
+        "        - Write narration\n",
+        encoding="utf-8",
+    )
+    with pytest.raises(
+        ConfigError,
+        match="narration_from_source.segments.01.system_prompt must be a YAML string",
+    ):
+        Config.from_yaml(p)
+
+
+def test_from_yaml_list_nfs_segment_topic_raises(tmp_path: Path) -> None:
+    p = tmp_path / "docgen.yaml"
+    p.write_text(
+        'narration_from_source:\n  segments:\n    "01":\n      topic:\n        - Overview\n',
+        encoding="utf-8",
+    )
+    with pytest.raises(
+        ConfigError,
+        match="narration_from_source.segments.01.topic must be a YAML string",
+    ):
+        Config.from_yaml(p)
+
+
+def test_from_yaml_list_msg_segment_class_name_raises(tmp_path: Path) -> None:
+    p = tmp_path / "docgen.yaml"
+    p.write_text(
+        'manim_scene_generation:\n  segments:\n    "01":\n      class_name:\n'
+        "        - OverviewScene\n",
+        encoding="utf-8",
+    )
+    with pytest.raises(
+        ConfigError,
+        match="manim_scene_generation.segments.01.class_name must be a YAML string",
+    ):
+        Config.from_yaml(p)
+
+
+def test_from_yaml_list_msg_segment_scene_spec_prompt_raises(tmp_path: Path) -> None:
+    p = tmp_path / "docgen.yaml"
+    p.write_text(
+        'manim_scene_generation:\n  segments:\n    "01":\n'
+        "      scene_spec_system_prompt:\n        - Cover beats\n",
+        encoding="utf-8",
+    )
+    with pytest.raises(
+        ConfigError,
+        match="manim_scene_generation.segments.01.scene_spec_system_prompt must be a YAML string",
+    ):
+        Config.from_yaml(p)
+
+
+def test_from_yaml_empty_nfs_segment_system_prompt_allowed(tmp_path: Path) -> None:
+    p = tmp_path / "docgen.yaml"
+    p.write_text(
+        'narration_from_source:\n  segments:\n    "01":\n      system_prompt: ""\n',
+        encoding="utf-8",
+    )
+    c = Config.from_yaml(p)
+    assert c.raw["narration_from_source"]["segments"]["01"]["system_prompt"] == ""