diff --git a/milestones/README.md b/milestones/README.md index aad0fab..95ca73c 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:** **[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. +**Active:** **[visual-map-field-strings.md](visual-map-field-strings.md)** — +`visual_map` type/scene/source and `segment_names` values must be YAML +strings at config load. **Shipped:** +- **[generation-model-strings.md](generation-model-strings.md)** — + `narration_from_source` / `manim_scene_generation` model and prompt + keys must be YAML strings (#107). - **[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)** — diff --git a/milestones/generation-model-strings.md b/milestones/generation-model-strings.md index 8f1408f..3295365 100644 --- a/milestones/generation-model-strings.md +++ b/milestones/generation-model-strings.md @@ -1,7 +1,7 @@ # Milestone: narration / scene-generation model and prompts must be strings -**Status:** Active -**PR:** [#107](https://github.com/jmjava/documentation-generator/pull/107) +**Status:** Shipped +**PR:** #107 **Depends on:** `milestones/manim-font-quality.md` (PR #106), `milestones/wizard-prompt-strings.md` (PR #100) diff --git a/milestones/visual-map-field-strings.md b/milestones/visual-map-field-strings.md new file mode 100644 index 0000000..7715e52 --- /dev/null +++ b/milestones/visual-map-field-strings.md @@ -0,0 +1,42 @@ +# Milestone: visual_map type/scene/source and segment_names values must be strings + +**Status:** Active +**PR:** [#108](https://github.com/jmjava/documentation-generator/pull/108) +**Depends on:** `milestones/generation-model-strings.md` (PR #107), +`milestones/visual-map-row-types.md` (PR #86), +`milestones/segment-id-strings.md` (PR #87) + +## Problem + +`visual_map` rows are already mappings and keys are already strings. +These inner / sibling wiring values were not: + +1. **`visual_map..type` / `scene` / `class` / `source`** — a YAML + list was `str()`’d (`"['manim']"`). Compose and pipeline then + **skipped** the segment (unknown type / missing class) instead of + failing at config load. `yaml-generate` treated `"['manim']"` as a + non-manim leftover and preserved the broken row. +2. **`segment_names` values** — keys are strings; a list stem became + `"['01-intro']"` and asset lookup used a bogus filename. + +Empty `type: ""` remains allowed (unmapped / yaml-generate fill). + +## Goal + +Fail closed at `Config.from_yaml`. Missing fields stay optional. + +## Done when + +- [x] Present `visual_map..type` / `scene` / `class` / `source` must + be YAML strings (empty allowed). +- [x] Present `segment_names` values must be non-empty YAML strings. +- [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 + +- `visual_map` mixed `sources:` remains a list of paths. +- `env_file` / `repo_root` / `dirs.*` path strings are separate. +- Per-segment generation `system_prompt` / `class_name` are separate. diff --git a/src/docgen/config.py b/src/docgen/config.py index f8e8bd4..334821a 100644 --- a/src/docgen/config.py +++ b/src/docgen/config.py @@ -219,8 +219,9 @@ def __post_init__(self) -> None: ) for i, item in enumerate(segs): require_yaml_string(item, label=f"concat.{name}[{i}]", source=src) - for key in self._block("segment_names"): - require_yaml_string(key, label="segment_names key", source=src) + for key, val in self._block("segment_names").items(): + sid = require_yaml_string(key, label="segment_names key", source=src) + require_yaml_string(val, label=f"segment_names.{sid}", source=src) pages_segs = self._block("pages").get("segments") if pages_segs is not None and not isinstance(pages_segs, dict): raise ConfigError( @@ -249,6 +250,13 @@ def __post_init__(self) -> None: f"{src}: visual_map.{sid_s} must be a YAML mapping, " f"not {type(spec).__name__}" ) + for fname in ("type", "scene", "class", "source"): + val = spec.get(fname) + if val is not None and not isinstance(val, str): + raise ConfigError( + f"{src}: visual_map.{sid_s}.{fname} must be a YAML string, " + f"not {type(val).__name__}" + ) wiz = self._block("wizard") if wiz.get("exclude_patterns") is not None: string_list_block( diff --git a/tests/test_config.py b/tests/test_config.py index aade83d..eaa301d 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -590,3 +590,44 @@ def test_from_yaml_list_scene_spec_system_prompt_raises(tmp_path: Path) -> None: match="manim_scene_generation.scene_spec_system_prompt must be a YAML string", ): Config.from_yaml(p) + + +def test_from_yaml_list_visual_map_type_raises(tmp_path: Path) -> None: + p = tmp_path / "docgen.yaml" + p.write_text('visual_map:\n "01":\n type:\n - manim\n', encoding="utf-8") + with pytest.raises(ConfigError, match="visual_map.01.type must be a YAML string"): + Config.from_yaml(p) + + +def test_from_yaml_list_visual_map_scene_raises(tmp_path: Path) -> None: + p = tmp_path / "docgen.yaml" + p.write_text( + 'visual_map:\n "01":\n type: manim\n scene:\n - OverviewScene\n', + encoding="utf-8", + ) + with pytest.raises(ConfigError, match="visual_map.01.scene must be a YAML string"): + Config.from_yaml(p) + + +def test_from_yaml_list_visual_map_source_raises(tmp_path: Path) -> None: + p = tmp_path / "docgen.yaml" + p.write_text( + 'visual_map:\n "01":\n type: still\n source:\n - slide.png\n', + encoding="utf-8", + ) + with pytest.raises(ConfigError, match="visual_map.01.source must be a YAML string"): + Config.from_yaml(p) + + +def test_from_yaml_empty_visual_map_type_allowed(tmp_path: Path) -> None: + p = tmp_path / "docgen.yaml" + p.write_text('visual_map:\n "01":\n type: ""\n', encoding="utf-8") + c = Config.from_yaml(p) + assert c.visual_map["01"]["type"] == "" + + +def test_from_yaml_list_segment_names_value_raises(tmp_path: Path) -> None: + p = tmp_path / "docgen.yaml" + p.write_text('segment_names:\n "01":\n - 01-intro\n', encoding="utf-8") + with pytest.raises(ConfigError, match="segment_names.01 must be a YAML string"): + Config.from_yaml(p)