Skip to content

Commit d0de44b

Browse files
cursoragentjmjava
andcommitted
Fail closed when visual_map type/scene/source or segment_names stem is a list
YAML lists were str()'d into unknown visual types (compose skipped the segment; yaml-generate preserved the broken row) or bogus asset stems. Require those present fields to be YAML strings at config load. Empty visual_map type remains allowed. Co-authored-by: jmjava <jmjava@gmail.com>
1 parent 063a607 commit d0de44b

5 files changed

Lines changed: 101 additions & 7 deletions

File tree

‎milestones/README.md‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,14 @@ repositories that install `docgen` and maintain their own demo bundle. The
55
library no longer ships an in-repo dogfood; consumers are the integration test
66
of record.
77

8-
**Active:** **[generation-model-strings.md](generation-model-strings.md)** —
9-
`narration_from_source` / `manim_scene_generation` model and prompt keys
10-
must be YAML strings at config load.
8+
**Active:** **[visual-map-field-strings.md](visual-map-field-strings.md)** —
9+
`visual_map` type/scene/source and `segment_names` values must be YAML
10+
strings at config load.
1111

1212
**Shipped:**
13+
- **[generation-model-strings.md](generation-model-strings.md)** —
14+
`narration_from_source` / `manim_scene_generation` model and prompt
15+
keys must be YAML strings (#107).
1316
- **[manim-font-quality.md](manim-font-quality.md)** —
1417
`manim.font` / `quality` / `manim_path` must be YAML strings (#106).
1518
- **[ai-timestamp-strings.md](ai-timestamp-strings.md)** —

‎milestones/generation-model-strings.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# Milestone: narration / scene-generation model and prompts must be strings
22

3-
**Status:** Active
4-
**PR:** [#107](https://github.com/jmjava/documentation-generator/pull/107)
3+
**Status:** Shipped
4+
**PR:** #107
55
**Depends on:** `milestones/manim-font-quality.md` (PR #106),
66
`milestones/wizard-prompt-strings.md` (PR #100)
77

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# Milestone: visual_map type/scene/source and segment_names values must be strings
2+
3+
**Status:** Active
4+
**PR:** pending
5+
**Depends on:** `milestones/generation-model-strings.md` (PR #107),
6+
`milestones/visual-map-row-types.md` (PR #86),
7+
`milestones/segment-id-strings.md` (PR #87)
8+
9+
## Problem
10+
11+
`visual_map` rows are already mappings and keys are already strings.
12+
These inner / sibling wiring values were not:
13+
14+
1. **`visual_map.<id>.type` / `scene` / `class` / `source`** — a YAML
15+
list was `str()`’d (`"['manim']"`). Compose and pipeline then
16+
**skipped** the segment (unknown type / missing class) instead of
17+
failing at config load. `yaml-generate` treated `"['manim']"` as a
18+
non-manim leftover and preserved the broken row.
19+
2. **`segment_names` values** — keys are strings; a list stem became
20+
`"['01-intro']"` and asset lookup used a bogus filename.
21+
22+
Empty `type: ""` remains allowed (unmapped / yaml-generate fill).
23+
24+
## Goal
25+
26+
Fail closed at `Config.from_yaml`. Missing fields stay optional.
27+
28+
## Done when
29+
30+
- [x] Present `visual_map.<id>.type` / `scene` / `class` / `source` must
31+
be YAML strings (empty allowed).
32+
- [x] Present `segment_names` values must be non-empty YAML strings.
33+
- [x] Tests for list values of those keys.
34+
- [ ] `ruff check src/ tests/`
35+
- [ ] `pytest tests/`
36+
- [ ] `docgen benchmark` (no clock change)
37+
38+
## Out of scope
39+
40+
- `visual_map` mixed `sources:` remains a list of paths.
41+
- `env_file` / `repo_root` / `dirs.*` path strings are separate.
42+
- Per-segment generation `system_prompt` / `class_name` are separate.

‎src/docgen/config.py‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,8 +219,9 @@ def __post_init__(self) -> None:
219219
)
220220
for i, item in enumerate(segs):
221221
require_yaml_string(item, label=f"concat.{name}[{i}]", source=src)
222-
for key in self._block("segment_names"):
223-
require_yaml_string(key, label="segment_names key", source=src)
222+
for key, val in self._block("segment_names").items():
223+
sid = require_yaml_string(key, label="segment_names key", source=src)
224+
require_yaml_string(val, label=f"segment_names.{sid}", source=src)
224225
pages_segs = self._block("pages").get("segments")
225226
if pages_segs is not None and not isinstance(pages_segs, dict):
226227
raise ConfigError(
@@ -249,6 +250,13 @@ def __post_init__(self) -> None:
249250
f"{src}: visual_map.{sid_s} must be a YAML mapping, "
250251
f"not {type(spec).__name__}"
251252
)
253+
for field in ("type", "scene", "class", "source"):
254+
val = spec.get(field)
255+
if val is not None and not isinstance(val, str):
256+
raise ConfigError(
257+
f"{src}: visual_map.{sid_s}.{field} must be a YAML string, "
258+
f"not {type(val).__name__}"
259+
)
252260
wiz = self._block("wizard")
253261
if wiz.get("exclude_patterns") is not None:
254262
string_list_block(

‎tests/test_config.py‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,3 +590,44 @@ def test_from_yaml_list_scene_spec_system_prompt_raises(tmp_path: Path) -> None:
590590
match="manim_scene_generation.scene_spec_system_prompt must be a YAML string",
591591
):
592592
Config.from_yaml(p)
593+
594+
595+
def test_from_yaml_list_visual_map_type_raises(tmp_path: Path) -> None:
596+
p = tmp_path / "docgen.yaml"
597+
p.write_text('visual_map:\n "01":\n type:\n - manim\n', encoding="utf-8")
598+
with pytest.raises(ConfigError, match="visual_map.01.type must be a YAML string"):
599+
Config.from_yaml(p)
600+
601+
602+
def test_from_yaml_list_visual_map_scene_raises(tmp_path: Path) -> None:
603+
p = tmp_path / "docgen.yaml"
604+
p.write_text(
605+
'visual_map:\n "01":\n type: manim\n scene:\n - OverviewScene\n',
606+
encoding="utf-8",
607+
)
608+
with pytest.raises(ConfigError, match="visual_map.01.scene must be a YAML string"):
609+
Config.from_yaml(p)
610+
611+
612+
def test_from_yaml_list_visual_map_source_raises(tmp_path: Path) -> None:
613+
p = tmp_path / "docgen.yaml"
614+
p.write_text(
615+
'visual_map:\n "01":\n type: still\n source:\n - slide.png\n',
616+
encoding="utf-8",
617+
)
618+
with pytest.raises(ConfigError, match="visual_map.01.source must be a YAML string"):
619+
Config.from_yaml(p)
620+
621+
622+
def test_from_yaml_empty_visual_map_type_allowed(tmp_path: Path) -> None:
623+
p = tmp_path / "docgen.yaml"
624+
p.write_text('visual_map:\n "01":\n type: ""\n', encoding="utf-8")
625+
c = Config.from_yaml(p)
626+
assert c.visual_map["01"]["type"] == ""
627+
628+
629+
def test_from_yaml_list_segment_names_value_raises(tmp_path: Path) -> None:
630+
p = tmp_path / "docgen.yaml"
631+
p.write_text('segment_names:\n "01":\n - 01-intro\n', encoding="utf-8")
632+
with pytest.raises(ConfigError, match="segment_names.01 must be a YAML string"):
633+
Config.from_yaml(p)

0 commit comments

Comments
 (0)