Skip to content

Commit 96343ac

Browse files
cursoragentjmjava
andcommitted
Fail closed when manim.font, quality, or manim_path is a list
YAML lists were str()'d into a bogus font name, a quality string that silently fell back to 720p30, or a nonexistent manim binary path. Require those present keys to be non-empty YAML strings at config load. Co-authored-by: jmjava <jmjava@gmail.com>
1 parent 8a84289 commit 96343ac

5 files changed

Lines changed: 75 additions & 5 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:** **[ai-timestamp-strings.md](ai-timestamp-strings.md)** —
9-
`ai.provider` / `timestamps.engine` / `tts.language` must be YAML strings
10-
at config load.
8+
**Active:** **[manim-font-quality.md](manim-font-quality.md)** —
9+
`manim.font` / `quality` / `manim_path` must be YAML strings at config
10+
load.
1111

1212
**Shipped:**
13+
- **[ai-timestamp-strings.md](ai-timestamp-strings.md)** —
14+
`ai.provider` / `timestamps.engine` / `tts.language` must be YAML
15+
strings (#105).
1316
- **[image-generation-strings.md](image-generation-strings.md)** —
1417
`image_generation.model` / `size` / `quality` must be YAML strings (#104).
1518
- **[image-empty-bytes.md](image-empty-bytes.md)** —

‎milestones/ai-timestamp-strings.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# Milestone: leftover AI / timestamps / TTS language keys must be strings
22

3-
**Status:** Active
4-
**PR:** [#105](https://github.com/jmjava/documentation-generator/pull/105)
3+
**Status:** Shipped
4+
**PR:** #105
55
**Depends on:** `milestones/image-generation-strings.md` (PR #104),
66
`milestones/tts-empty-audio.md` (PR #99),
77
`milestones/multi-host-ai-hardening.md`

‎milestones/manim-font-quality.md‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# Milestone: manim.font / quality / manim_path must be strings
2+
3+
**Status:** Active
4+
**PR:** pending
5+
**Depends on:** `milestones/ai-timestamp-strings.md` (PR #105)
6+
7+
## Problem
8+
9+
`ai.provider` / `tts.model` / `image_generation.quality` are already
10+
strings at config load. These Manim keys were not:
11+
12+
1. **`manim.font`** — `manim_font` did `str(...)`, so a YAML list became
13+
`"['Liberation Sans']"` and Manim rendered with a bogus font.
14+
2. **`manim.quality`** — a list was `str()`’d in `_quality_args` to
15+
`"['1080p30']"`, missed the preset map, and **silently fell back** to
16+
720p30 with a warning.
17+
3. **`manim.manim_path`** — a list became `"['/usr/bin/manim']"` and
18+
binary lookup failed later instead of at config load.
19+
20+
## Goal
21+
22+
Fail closed at `Config.from_yaml`. Missing keys still use defaults
23+
(`Liberation Sans`, `1080p30`, no `manim_path`).
24+
25+
## Done when
26+
27+
- [x] Present `manim.font` / `manim.quality` / `manim.manim_path` must be
28+
non-empty YAML strings.
29+
- [x] Tests for list values of those keys.
30+
- [ ] `ruff check src/ tests/`
31+
- [ ] `pytest tests/`
32+
- [ ] `docgen benchmark` (no clock change)
33+
34+
## Out of scope
35+
36+
- Unknown `manim.quality` strings still warn and fall back to 720p30.
37+
- `manim.min_font_size` remains an int (`int(...)` already TypeErrors on
38+
a list).
39+
- `wizard.default_guidance` type gating is separate.

‎src/docgen/config.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,13 @@ def __post_init__(self) -> None:
301301
require_yaml_string(ts["engine"], label="timestamps.engine", source=src)
302302
if tts.get("language") is not None:
303303
require_yaml_string(tts["language"], label="tts.language", source=src)
304+
manim = self._block("manim")
305+
if manim.get("font") is not None:
306+
require_yaml_string(manim["font"], label="manim.font", source=src)
307+
if manim.get("quality") is not None:
308+
require_yaml_string(manim["quality"], label="manim.quality", source=src)
309+
if manim.get("manim_path") is not None:
310+
require_yaml_string(manim["manim_path"], label="manim.manim_path", source=src)
304311
ocr = self._sub_block(validation, "ocr", label="validation.ocr")
305312
if ocr.get("error_patterns") is not None:
306313
string_list_block(

‎tests/test_config.py‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,3 +514,24 @@ def test_from_yaml_list_tts_language_raises(tmp_path: Path) -> None:
514514
p.write_text("tts:\n language:\n - en\n", encoding="utf-8")
515515
with pytest.raises(ConfigError, match="tts.language must be a YAML string"):
516516
Config.from_yaml(p)
517+
518+
519+
def test_from_yaml_list_manim_font_raises(tmp_path: Path) -> None:
520+
p = tmp_path / "docgen.yaml"
521+
p.write_text("manim:\n font:\n - Liberation Sans\n", encoding="utf-8")
522+
with pytest.raises(ConfigError, match="manim.font must be a YAML string"):
523+
Config.from_yaml(p)
524+
525+
526+
def test_from_yaml_list_manim_quality_raises(tmp_path: Path) -> None:
527+
p = tmp_path / "docgen.yaml"
528+
p.write_text("manim:\n quality:\n - 1080p30\n", encoding="utf-8")
529+
with pytest.raises(ConfigError, match="manim.quality must be a YAML string"):
530+
Config.from_yaml(p)
531+
532+
533+
def test_from_yaml_list_manim_path_raises(tmp_path: Path) -> None:
534+
p = tmp_path / "docgen.yaml"
535+
p.write_text("manim:\n manim_path:\n - /usr/bin/manim\n", encoding="utf-8")
536+
with pytest.raises(ConfigError, match="manim.manim_path must be a YAML string"):
537+
Config.from_yaml(p)

0 commit comments

Comments
 (0)