Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions milestones/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:** **[ai-timestamp-strings.md](ai-timestamp-strings.md)** —
`ai.provider` / `timestamps.engine` / `tts.language` must be YAML strings
at config load.
**Active:** **[manim-font-quality.md](manim-font-quality.md)** —
`manim.font` / `quality` / `manim_path` must be YAML strings at config
load.

**Shipped:**
- **[ai-timestamp-strings.md](ai-timestamp-strings.md)** —
`ai.provider` / `timestamps.engine` / `tts.language` must be YAML
strings (#105).
- **[image-generation-strings.md](image-generation-strings.md)** —
`image_generation.model` / `size` / `quality` must be YAML strings (#104).
- **[image-empty-bytes.md](image-empty-bytes.md)** —
Expand Down
4 changes: 2 additions & 2 deletions milestones/ai-timestamp-strings.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Milestone: leftover AI / timestamps / TTS language keys must be strings

**Status:** Active
**PR:** [#105](https://github.com/jmjava/documentation-generator/pull/105)
**Status:** Shipped
**PR:** #105
**Depends on:** `milestones/image-generation-strings.md` (PR #104),
`milestones/tts-empty-audio.md` (PR #99),
`milestones/multi-host-ai-hardening.md`
Expand Down
39 changes: 39 additions & 0 deletions milestones/manim-font-quality.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Milestone: manim.font / quality / manim_path must be strings

**Status:** Active
**PR:** [#106](https://github.com/jmjava/documentation-generator/pull/106)
**Depends on:** `milestones/ai-timestamp-strings.md` (PR #105)

## Problem

`ai.provider` / `tts.model` / `image_generation.quality` are already
strings at config load. These Manim keys were not:

1. **`manim.font`** — `manim_font` did `str(...)`, so a YAML list became
`"['Liberation Sans']"` and Manim rendered with a bogus font.
2. **`manim.quality`** — a list was `str()`’d in `_quality_args` to
`"['1080p30']"`, missed the preset map, and **silently fell back** to
720p30 with a warning.
3. **`manim.manim_path`** — a list became `"['/usr/bin/manim']"` and
binary lookup failed later instead of at config load.

## Goal

Fail closed at `Config.from_yaml`. Missing keys still use defaults
(`Liberation Sans`, `1080p30`, no `manim_path`).

## Done when

- [x] Present `manim.font` / `manim.quality` / `manim.manim_path` 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

- Unknown `manim.quality` strings still warn and fall back to 720p30.
- `manim.min_font_size` remains an int (`int(...)` already TypeErrors on
a list).
- `wizard.default_guidance` type gating is separate.
7 changes: 7 additions & 0 deletions src/docgen/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,13 @@ def __post_init__(self) -> None:
require_yaml_string(ts["engine"], label="timestamps.engine", source=src)
if tts.get("language") is not None:
require_yaml_string(tts["language"], label="tts.language", source=src)
manim = self._block("manim")
if manim.get("font") is not None:
require_yaml_string(manim["font"], label="manim.font", source=src)
if manim.get("quality") is not 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)
ocr = self._sub_block(validation, "ocr", label="validation.ocr")
if ocr.get("error_patterns") is not None:
string_list_block(
Expand Down
21 changes: 21 additions & 0 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -514,3 +514,24 @@ def test_from_yaml_list_tts_language_raises(tmp_path: Path) -> None:
p.write_text("tts:\n language:\n - en\n", encoding="utf-8")
with pytest.raises(ConfigError, match="tts.language must be a YAML string"):
Config.from_yaml(p)


def test_from_yaml_list_manim_font_raises(tmp_path: Path) -> None:
p = tmp_path / "docgen.yaml"
p.write_text("manim:\n font:\n - Liberation Sans\n", encoding="utf-8")
with pytest.raises(ConfigError, match="manim.font must be a YAML string"):
Config.from_yaml(p)


def test_from_yaml_list_manim_quality_raises(tmp_path: Path) -> None:
p = tmp_path / "docgen.yaml"
p.write_text("manim:\n quality:\n - 1080p30\n", encoding="utf-8")
with pytest.raises(ConfigError, match="manim.quality must be a YAML string"):
Config.from_yaml(p)


def test_from_yaml_list_manim_path_raises(tmp_path: Path) -> None:
p = tmp_path / "docgen.yaml"
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)