diff --git a/milestones/README.md b/milestones/README.md index cbec422..e51a7f8 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:** **[validation-numeric-tunables.md](validation-numeric-tunables.md)** — -nested validation OCR / layout / av_sync / timing / story_end numerics -must be YAML numbers (`true` used to become `1`). +**Active:** **[validation-enable-bools.md](validation-enable-bools.md)** — +validation / manim enable flags must be YAML booleans (`"false"` used +to leave checks on). **Shipped:** +- **[validation-numeric-tunables.md](validation-numeric-tunables.md)** — + nested validation OCR / layout / av_sync / timing / story_end + numerics must be YAML numbers (#118). - **[generation-numeric-tunables.md](generation-numeric-tunables.md)** — narration / scene-generation temperature and context-byte tunables must be YAML numbers (#117). diff --git a/milestones/validation-enable-bools.md b/milestones/validation-enable-bools.md new file mode 100644 index 0000000..80179ca --- /dev/null +++ b/milestones/validation-enable-bools.md @@ -0,0 +1,46 @@ +# Milestone: validation enable flags must be YAML booleans + +**Status:** Active +**PR:** [#119](https://github.com/jmjava/documentation-generator/pull/119) +**Depends on:** `milestones/validation-numeric-tunables.md` (PR #118), +`milestones/discovery-bool-flags.md` (PR #114) + +## Problem + +Nested validation numerics are typed. Enable flags still use Python +truthiness / ``bool()``: + +1. Quoted ``validation.av_sync.enabled: "false"`` is a non-empty + string, so the check **stayed on**. +2. ``manim.scene_lint: "false"`` kept scene lint on. +3. Integer ``subject_beat_coverage.enabled: 0`` disabled coverage via + ``bool(0)`` instead of failing closed like other discovery bools. + +YAML ``false`` / ``true`` already work. + +## Goal + +Fail closed at ``Config.from_yaml``. Present values of: + +- ``manim.scene_lint`` +- ``validation.layout.check_overlap`` +- ``validation.av_sync.enabled`` / ``prefer_scene_spec_labels`` +- ``validation.timing_sync.enabled`` +- ``validation.scene_assets.enabled`` +- ``validation.story_end.enabled`` +- ``validation.subject_beat_coverage.enabled`` + +must be YAML booleans. Missing keys keep defaults. + +## Done when + +- [x] Present enable flags must be YAML booleans. +- [x] Tests for quoted ``"false"``, integer ``0``, and real YAML bools. +- [x] `ruff check src/ tests/` +- [x] `pytest tests/` (686 passed, 1 skipped) +- [x] `docgen benchmark` (no clock change; meets baseline) + +## Out of scope + +- Changing which checks are soft in ``validate --pre-push``. +- Coercing ``"false"`` / ``0`` into booleans. diff --git a/milestones/validation-numeric-tunables.md b/milestones/validation-numeric-tunables.md index fdb6e2e..87e8fb6 100644 --- a/milestones/validation-numeric-tunables.md +++ b/milestones/validation-numeric-tunables.md @@ -1,6 +1,6 @@ # Milestone: nested validation numerics must be YAML numbers -**Status:** Active +**Status:** Shipped **PR:** [#118](https://github.com/jmjava/documentation-generator/pull/118) **Depends on:** `milestones/generation-numeric-tunables.md` (PR #117), `milestones/numeric-config-tunables.md` (PR #115) diff --git a/src/docgen/config.py b/src/docgen/config.py index ee70d33..393ed61 100644 --- a/src/docgen/config.py +++ b/src/docgen/config.py @@ -455,6 +455,10 @@ def __post_init__(self) -> None: label="manim.min_font_size", source=src, ) + if manim.get("scene_lint") is not None: + require_yaml_bool( + manim["scene_lint"], label="manim.scene_lint", source=src + ) compose = self._block("compose") if compose.get("ffmpeg_timeout_sec") is not None: require_yaml_number( @@ -643,6 +647,52 @@ def __post_init__(self) -> None: require_yaml_number( story[nkey], label=f"validation.story_end.{nkey}", source=src ) + if layout.get("check_overlap") is not None: + require_yaml_bool( + layout["check_overlap"], + label="validation.layout.check_overlap", + source=src, + ) + if avs.get("enabled") is not None: + require_yaml_bool( + avs["enabled"], label="validation.av_sync.enabled", source=src + ) + if avs.get("prefer_scene_spec_labels") is not None: + require_yaml_bool( + avs["prefer_scene_spec_labels"], + label="validation.av_sync.prefer_scene_spec_labels", + source=src, + ) + if ts_sync.get("enabled") is not None: + require_yaml_bool( + ts_sync["enabled"], + label="validation.timing_sync.enabled", + source=src, + ) + if story.get("enabled") is not None: + require_yaml_bool( + story["enabled"], + label="validation.story_end.enabled", + source=src, + ) + sa = self._sub_block( + validation, "scene_assets", label="validation.scene_assets" + ) + if sa.get("enabled") is not None: + require_yaml_bool( + sa["enabled"], label="validation.scene_assets.enabled", source=src + ) + sbc = self._sub_block( + validation, + "subject_beat_coverage", + label="validation.subject_beat_coverage", + ) + if sbc.get("enabled") is not None: + require_yaml_bool( + sbc["enabled"], + label="validation.subject_beat_coverage.enabled", + source=src, + ) def _source_label(self) -> str: return self.yaml_path.name if self.yaml_path else "docgen.yaml" diff --git a/tests/test_config.py b/tests/test_config.py index 6156f62..1938196 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1010,3 +1010,59 @@ def test_from_yaml_validation_nested_numerics_allowed(tmp_path: Path) -> None: assert c.av_sync_config["tolerance_sec"] == 2.5 assert c.timing_sync_config["max_tail_gap_sec"] == 4.0 assert c.story_end_config["max_early_sec"] == 30 + + +def test_from_yaml_string_av_sync_enabled_raises(tmp_path: Path) -> None: + p = tmp_path / "docgen.yaml" + p.write_text( + 'validation:\n av_sync:\n enabled: "false"\n', + encoding="utf-8", + ) + with pytest.raises( + ConfigError, match="validation.av_sync.enabled must be a YAML boolean" + ): + Config.from_yaml(p) + + +def test_from_yaml_string_scene_lint_raises(tmp_path: Path) -> None: + p = tmp_path / "docgen.yaml" + p.write_text('manim:\n scene_lint: "false"\n', encoding="utf-8") + with pytest.raises(ConfigError, match="manim.scene_lint must be a YAML boolean"): + Config.from_yaml(p) + + +def test_from_yaml_int_subject_beat_coverage_enabled_raises(tmp_path: Path) -> None: + p = tmp_path / "docgen.yaml" + p.write_text( + "validation:\n subject_beat_coverage:\n enabled: 0\n", + encoding="utf-8", + ) + with pytest.raises( + ConfigError, + match="validation.subject_beat_coverage.enabled must be a YAML boolean", + ): + Config.from_yaml(p) + + +def test_from_yaml_validation_enable_bools_allowed(tmp_path: Path) -> None: + p = tmp_path / "docgen.yaml" + p.write_text( + "manim:\n scene_lint: false\n" + "validation:\n" + " av_sync:\n enabled: false\n prefer_scene_spec_labels: true\n" + " timing_sync:\n enabled: true\n" + " scene_assets:\n enabled: true\n" + " story_end:\n enabled: false\n" + " layout:\n check_overlap: false\n" + " subject_beat_coverage:\n enabled: false\n", + encoding="utf-8", + ) + c = Config.from_yaml(p) + assert c.manim_scene_lint_enabled is False + assert c.av_sync_config["enabled"] is False + assert c.av_sync_config["prefer_scene_spec_labels"] is True + assert c.timing_sync_config["enabled"] is True + assert c.scene_assets_config["enabled"] is True + assert c.story_end_config["enabled"] is False + assert c.layout_config["check_overlap"] is False + assert c.subject_beat_coverage_enabled is False