Skip to content

Commit f415d2c

Browse files
cursoragentjmjava
andcommitted
Fail closed when nested validation tunables are not YAML numbers
bool is a subclass of int, so av_sync.tolerance_sec: true became 1.0s OCR slack and ocr.min_confidence: true became 1. Co-authored-by: jmjava <jmjava@gmail.com>
1 parent f0b0c58 commit f415d2c

5 files changed

Lines changed: 155 additions & 4 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-numeric-tunables.md](generation-numeric-tunables.md)** —
9-
narration / scene-generation temperature and context-byte tunables must
10-
be YAML numbers (`true` used to become `1.0` / 1-byte context).
8+
**Active:** **[validation-numeric-tunables.md](validation-numeric-tunables.md)** —
9+
nested validation OCR / layout / av_sync / timing / story_end numerics
10+
must be YAML numbers (`true` used to become `1`).
1111

1212
**Shipped:**
13+
- **[generation-numeric-tunables.md](generation-numeric-tunables.md)** —
14+
narration / scene-generation temperature and context-byte tunables
15+
must be YAML numbers (#117).
1316
- **[hint-segment-create-bool.md](hint-segment-create-bool.md)** —
1417
hint `docgen.segment.create` must be a YAML boolean (#116).
1518
- **[numeric-config-tunables.md](numeric-config-tunables.md)** —

‎milestones/generation-numeric-tunables.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Milestone: LLM generation numeric tunables must be YAML numbers
22

3-
**Status:** Active
3+
**Status:** Shipped
44
**PR:** [#117](https://github.com/jmjava/documentation-generator/pull/117)
55
**Depends on:** `milestones/hint-segment-create-bool.md` (PR #116),
66
`milestones/numeric-config-tunables.md` (PR #115),
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# Milestone: nested validation numerics must be YAML numbers
2+
3+
**Status:** Active
4+
**PR:** (this PR)
5+
**Depends on:** `milestones/generation-numeric-tunables.md` (PR #117),
6+
`milestones/numeric-config-tunables.md` (PR #115)
7+
8+
## Problem
9+
10+
Top-level ``validation.max_drift_sec`` / ``max_freeze_ratio`` are
11+
already typed. Nested check tunables still go through ``int()`` /
12+
``float()``. A YAML **bool** is a subclass of ``int``, so:
13+
14+
1. ``validation.av_sync.tolerance_sec: true`` became **1.0s** OCR
15+
slack (vs default 3.0).
16+
2. ``validation.ocr.min_confidence: true`` became **1**.
17+
3. ``validation.story_end.max_early_sec: true`` became **1s**.
18+
19+
A YAML list or string raised ``TypeError`` / ``ValueError`` inside
20+
validate, not ``ConfigError`` at load.
21+
22+
## Goal
23+
24+
Fail closed at ``Config.from_yaml``. Present values of:
25+
26+
- ``validation.ocr.sample_interval_sec`` / ``min_confidence``
27+
- ``validation.layout.min_spacing_px`` / ``edge_margin_px``
28+
- ``validation.av_sync.tolerance_sec`` / ``min_anchors_per_segment`` /
29+
``max_anchors_per_segment``
30+
- ``validation.timing_sync.max_tail_gap_sec`` / ``max_end_overrun_sec``
31+
- ``validation.story_end.max_early_sec`` / ``max_early_ratio``
32+
33+
must be YAML numbers (int or float, not bool). Missing keys keep
34+
defaults.
35+
36+
## Done when
37+
38+
- [x] Present nested tunables must be YAML numbers.
39+
- [x] Tests for bool ``tolerance_sec`` / ``max_early_sec``, list
40+
``min_confidence``, quoted ``max_tail_gap_sec``, and valid
41+
numbers.
42+
- [x] `ruff check src/ tests/`
43+
- [x] `pytest tests/`
44+
- [x] `docgen benchmark` (no clock change)
45+
46+
## Out of scope
47+
48+
- ``validation.*.enabled`` / ``layout.check_overlap`` /
49+
``prefer_scene_spec_labels`` boolean gates.
50+
- Coercing numeric strings into numbers.

‎src/docgen/config.py‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -607,6 +607,42 @@ def __post_init__(self) -> None:
607607
label="validation.narration_lint.post_tts_deny_patterns",
608608
source=src,
609609
)
610+
for nkey in ("sample_interval_sec", "min_confidence"):
611+
if ocr.get(nkey) is not None:
612+
require_yaml_number(
613+
ocr[nkey], label=f"validation.ocr.{nkey}", source=src
614+
)
615+
layout = self._sub_block(validation, "layout", label="validation.layout")
616+
for nkey in ("min_spacing_px", "edge_margin_px"):
617+
if layout.get(nkey) is not None:
618+
require_yaml_number(
619+
layout[nkey], label=f"validation.layout.{nkey}", source=src
620+
)
621+
for nkey in (
622+
"tolerance_sec",
623+
"min_anchors_per_segment",
624+
"max_anchors_per_segment",
625+
):
626+
if avs.get(nkey) is not None:
627+
require_yaml_number(
628+
avs[nkey], label=f"validation.av_sync.{nkey}", source=src
629+
)
630+
ts_sync = self._sub_block(
631+
validation, "timing_sync", label="validation.timing_sync"
632+
)
633+
for nkey in ("max_tail_gap_sec", "max_end_overrun_sec"):
634+
if ts_sync.get(nkey) is not None:
635+
require_yaml_number(
636+
ts_sync[nkey],
637+
label=f"validation.timing_sync.{nkey}",
638+
source=src,
639+
)
640+
story = self._sub_block(validation, "story_end", label="validation.story_end")
641+
for nkey in ("max_early_sec", "max_early_ratio"):
642+
if story.get(nkey) is not None:
643+
require_yaml_number(
644+
story[nkey], label=f"validation.story_end.{nkey}", source=src
645+
)
610646

611647
def _source_label(self) -> str:
612648
return self.yaml_path.name if self.yaml_path else "docgen.yaml"

‎tests/test_config.py‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -948,3 +948,65 @@ def test_from_yaml_generation_numeric_tunables_allowed(tmp_path: Path) -> None:
948948
assert c.raw["narration_from_source"]["max_context_bytes"] == 90000
949949
assert c.raw["manim_scene_generation"]["temperature"] == 0.4
950950
assert c.raw["manim_scene_generation"]["max_whisper_words_in_prompt"] == 12
951+
952+
953+
def test_from_yaml_bool_av_sync_tolerance_raises(tmp_path: Path) -> None:
954+
p = tmp_path / "docgen.yaml"
955+
p.write_text("validation:\n av_sync:\n tolerance_sec: true\n", encoding="utf-8")
956+
with pytest.raises(
957+
ConfigError, match="validation.av_sync.tolerance_sec must be a YAML number"
958+
):
959+
Config.from_yaml(p)
960+
961+
962+
def test_from_yaml_list_ocr_min_confidence_raises(tmp_path: Path) -> None:
963+
p = tmp_path / "docgen.yaml"
964+
p.write_text(
965+
"validation:\n ocr:\n min_confidence:\n - 40\n",
966+
encoding="utf-8",
967+
)
968+
with pytest.raises(
969+
ConfigError, match="validation.ocr.min_confidence must be a YAML number"
970+
):
971+
Config.from_yaml(p)
972+
973+
974+
def test_from_yaml_bool_story_end_max_early_sec_raises(tmp_path: Path) -> None:
975+
p = tmp_path / "docgen.yaml"
976+
p.write_text("validation:\n story_end:\n max_early_sec: true\n", encoding="utf-8")
977+
with pytest.raises(
978+
ConfigError, match="validation.story_end.max_early_sec must be a YAML number"
979+
):
980+
Config.from_yaml(p)
981+
982+
983+
def test_from_yaml_string_timing_sync_tail_gap_raises(tmp_path: Path) -> None:
984+
p = tmp_path / "docgen.yaml"
985+
p.write_text(
986+
'validation:\n timing_sync:\n max_tail_gap_sec: "3.0"\n',
987+
encoding="utf-8",
988+
)
989+
with pytest.raises(
990+
ConfigError, match="validation.timing_sync.max_tail_gap_sec must be a YAML number"
991+
):
992+
Config.from_yaml(p)
993+
994+
995+
def test_from_yaml_validation_nested_numerics_allowed(tmp_path: Path) -> None:
996+
p = tmp_path / "docgen.yaml"
997+
p.write_text(
998+
"validation:\n"
999+
" ocr:\n sample_interval_sec: 3\n min_confidence: 50\n"
1000+
" layout:\n min_spacing_px: 12\n edge_margin_px: 18\n"
1001+
" av_sync:\n tolerance_sec: 2.5\n min_anchors_per_segment: 3\n"
1002+
" timing_sync:\n max_tail_gap_sec: 4.0\n max_end_overrun_sec: 1.5\n"
1003+
" story_end:\n max_early_sec: 30\n max_early_ratio: 0.4\n",
1004+
encoding="utf-8",
1005+
)
1006+
c = Config.from_yaml(p)
1007+
assert c.ocr_config["sample_interval_sec"] == 3
1008+
assert c.ocr_config["min_confidence"] == 50
1009+
assert c.layout_config["min_spacing_px"] == 12
1010+
assert c.av_sync_config["tolerance_sec"] == 2.5
1011+
assert c.timing_sync_config["max_tail_gap_sec"] == 4.0
1012+
assert c.story_end_config["max_early_sec"] == 30

0 commit comments

Comments
 (0)