From f47b155c49470e6b0c8482dc2ae961e30fa49d53 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 8 Sep 2026 00:49:27 +0000 Subject: [PATCH 1/2] Fail closed when whisper prompt caps coerce bools to 1 Timing enrichment already type-checks max_whisper_segment_text_chars. The count caps still used int(... or 0), so true became 1 and truncated the word stream the scene-spec LLM uses for wait_word. Co-authored-by: jmjava --- milestones/README.md | 8 ++- milestones/pages-ffprobe-returncode.md | 2 +- milestones/whisper-prompt-caps.md | 46 ++++++++++++++ src/docgen/manim_scene_support.py | 40 +++++++++---- tests/test_manim_scene_support.py | 83 ++++++++++++++++++++++++++ 5 files changed, 162 insertions(+), 17 deletions(-) create mode 100644 milestones/whisper-prompt-caps.md diff --git a/milestones/README.md b/milestones/README.md index cf5ef1d..6cd5d05 100644 --- a/milestones/README.md +++ b/milestones/README.md @@ -5,11 +5,13 @@ 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:** **[pages-ffprobe-returncode.md](pages-ffprobe-returncode.md)** — -pages duration badges must not trust ffprobe stdout when the probe -exits non-zero. +**Active:** **[whisper-prompt-caps.md](whisper-prompt-caps.md)** — +timing-enrichment whisper count caps must not coerce bools to 1. **Shipped:** +- **[pages-ffprobe-returncode.md](pages-ffprobe-returncode.md)** — + pages duration badges must not trust ffprobe stdout when the probe + exits non-zero (#143). - **[merge-settings-numerics.md](merge-settings-numerics.md)** — narration / scene-generation merge must not coerce bool temperature or model values (#142). diff --git a/milestones/pages-ffprobe-returncode.md b/milestones/pages-ffprobe-returncode.md index 984bfe7..73f4240 100644 --- a/milestones/pages-ffprobe-returncode.md +++ b/milestones/pages-ffprobe-returncode.md @@ -1,6 +1,6 @@ # Milestone: pages duration probes must honor ffprobe returncode -**Status:** Active +**Status:** Shipped **PR:** [#143](https://github.com/jmjava/documentation-generator/pull/143) **Depends on:** `milestones/ffprobe-returncode.md` (PR #135), `milestones/pages-config-strings.md` (PR #111) diff --git a/milestones/whisper-prompt-caps.md b/milestones/whisper-prompt-caps.md new file mode 100644 index 0000000..d5db94f --- /dev/null +++ b/milestones/whisper-prompt-caps.md @@ -0,0 +1,46 @@ +# Milestone: Whisper prompt caps must not coerce bools + +**Status:** Active +**PR:** (this PR) +**Depends on:** `milestones/pages-ffprobe-returncode.md` (PR #143), +`milestones/merge-settings-numerics.md` (PR #142), +`milestones/generation-zero-values.md` (PR #122), +`milestones/generation-numeric-tunables.md` (PR #117) + +## Problem + +`Config.from_yaml` already requires YAML numbers for +`max_whisper_segments_in_prompt` / `max_whisper_words_in_prompt` / +`max_whisper_segment_text_chars` (#117). Timing enrichment already +type-checks **chars** (#122). The two count caps still do: + +```python +max_seg = int(root.get("max_whisper_segments_in_prompt", 0) or 0) +max_words = int(root.get("max_whisper_words_in_prompt", 0) or 0) +``` + +`bool` is a subclass of `int`: `max_whisper_words_in_prompt: true` +becomes **1** and truncates the word stream the LLM uses for +`wait_word`. A quoted `"12"` becomes 12 instead of raising. + +#142 called this out as out of scope of merge-settings numerics. + +## Goal + +Fail closed on present non-number caps. Missing/null still means 0 +(send the full stream). Explicit `0` still means all tokens. + +## Done when + +- [ ] Bool / string whisper count caps raise `SceneGenerationError` +- [ ] `max_whisper_words_in_prompt: 0` still lists every word +- [ ] Chars type-check stays (shared helper) +- [ ] `ruff check src/ tests/` +- [ ] `pytest tests/` +- [ ] `docgen benchmark` (no clock change; meets baseline) + +## Out of scope + +- `Config.from_yaml` already gates these keys for CLI load +- Validate `_check_streams` / `_check_drift` ffprobe `returncode` +- Whisper API `start`/`end` `float(... or 0.0)` in `ai_client` diff --git a/src/docgen/manim_scene_support.py b/src/docgen/manim_scene_support.py index c8a557f..1554f00 100644 --- a/src/docgen/manim_scene_support.py +++ b/src/docgen/manim_scene_support.py @@ -741,6 +741,18 @@ def format_pacing_schedule_markdown(segments: list[dict], pace_indices: list[int return "\n".join(lines) +def _nonneg_yaml_int(raw: Any, *, default: int, label: str) -> int: + """Missing/null → *default*; a present value must be a YAML number (not bool).""" + if raw is None: + return default + if isinstance(raw, bool) or not isinstance(raw, (int, float)): + raise SceneGenerationError( + f"manim_scene_generation.{label} must be a YAML number, " + f"not {type(raw).__name__} ({raw!r})" + ) + return max(0, int(raw)) + + def _load_timing_words_from_cfg(cfg: "Config", seg_name: str) -> list[dict]: """Return the ``words`` list from ``animations/timing.json`` for ``seg_name``.""" from docgen.timestamps import TimestampError, load_bundle_timing @@ -790,19 +802,21 @@ def build_timing_enrichment_for_prompt( """Whisper timing for the scene-spec LLM: word stream first (`wait_word`), else segments (`wait_segment`).""" root = manim_scene_generation_root(cfg) seg_block = manim_scene_generation_segment_block(cfg, seg_id) - max_seg = int(root.get("max_whisper_segments_in_prompt", 0) or 0) - max_words = int(root.get("max_whisper_words_in_prompt", 0) or 0) - raw_chars = root.get("max_whisper_segment_text_chars", 200) - if raw_chars is None: - max_chars = 200 - else: - # ``0 or 200`` used to ignore an explicit 0 (no truncation). - if isinstance(raw_chars, bool) or not isinstance(raw_chars, (int, float)): - raise SceneGenerationError( - "manim_scene_generation.max_whisper_segment_text_chars must be a " - f"YAML number, not {type(raw_chars).__name__} ({raw_chars!r})" - ) - max_chars = max(0, int(raw_chars)) + max_seg = _nonneg_yaml_int( + root.get("max_whisper_segments_in_prompt"), + default=0, + label="max_whisper_segments_in_prompt", + ) + max_words = _nonneg_yaml_int( + root.get("max_whisper_words_in_prompt"), + default=0, + label="max_whisper_words_in_prompt", + ) + max_chars = _nonneg_yaml_int( + root.get("max_whisper_segment_text_chars"), + default=200, + label="max_whisper_segment_text_chars", + ) whisper_words = _load_timing_words_from_cfg(cfg, seg_name) n_words_total = len(whisper_words) diff --git a/tests/test_manim_scene_support.py b/tests/test_manim_scene_support.py index 119eb4a..725c25f 100644 --- a/tests/test_manim_scene_support.py +++ b/tests/test_manim_scene_support.py @@ -316,6 +316,89 @@ def test_build_timing_enrichment_bool_max_chars_raises(tmp_path: Path) -> None: build_timing_enrichment_for_prompt(cfg, "08", "08-extras", segs) +def _write_extras_timing(tmp_path: Path, n_words: int = 4) -> list[dict]: + anim = tmp_path / "animations" + anim.mkdir(parents=True, exist_ok=True) + words = [ + {"start": float(i), "end": float(i) + 0.5, "word": f"w{i}"} + for i in range(n_words) + ] + segs = [ + { + "start": 0.0, + "end": float(n_words), + "text": " ".join(w["word"] for w in words), + } + ] + (anim / "timing.json").write_text( + json.dumps({"08-extras": {"segments": segs, "words": words}}), + encoding="utf-8", + ) + return segs + + +def test_build_timing_enrichment_bool_max_words_raises(tmp_path: Path) -> None: + cfg = Config.minimal(tmp_path) + cfg.raw["manim_scene_generation"] = {"max_whisper_words_in_prompt": True} + segs = _write_extras_timing(tmp_path, 4) + with pytest.raises(SceneGenerationError, match="max_whisper_words_in_prompt"): + build_timing_enrichment_for_prompt(cfg, "08", "08-extras", segs) + + +def test_build_timing_enrichment_string_max_words_raises(tmp_path: Path) -> None: + cfg = Config.minimal(tmp_path) + cfg.raw["manim_scene_generation"] = {"max_whisper_words_in_prompt": "12"} + segs = _write_extras_timing(tmp_path, 4) + with pytest.raises(SceneGenerationError, match="max_whisper_words_in_prompt"): + build_timing_enrichment_for_prompt(cfg, "08", "08-extras", segs) + + +def test_build_timing_enrichment_bool_max_segments_raises(tmp_path: Path) -> None: + cfg = Config.minimal(tmp_path) + cfg.raw["manim_scene_generation"] = {"max_whisper_segments_in_prompt": True} + segs = [ + {"start": 0.0, "end": 1.0, "text": "alpha"}, + {"start": 1.0, "end": 2.0, "text": "bravo"}, + ] + with pytest.raises(SceneGenerationError, match="max_whisper_segments_in_prompt"): + build_timing_enrichment_for_prompt(cfg, "08", "08-extras", segs) + + +def test_build_timing_enrichment_zero_max_words_lists_all(tmp_path: Path) -> None: + cfg = _write_cfg( + tmp_path, + { + "manim_scene_generation": { + "max_whisper_words_in_prompt": 0, + "segments": {"08": {"class_name": "ExtrasScene"}}, + }, + }, + ) + segs = _write_extras_timing(tmp_path, 4) + out = build_timing_enrichment_for_prompt(cfg, "08", "08-extras", segs) + assert '"word_index": 0' in out + assert '"word_index": 3' in out + assert "listed 1 of 4" not in out + + +def test_build_timing_enrichment_positive_max_words_truncates(tmp_path: Path) -> None: + cfg = _write_cfg( + tmp_path, + { + "manim_scene_generation": { + "max_whisper_words_in_prompt": 2, + "segments": {"08": {"class_name": "ExtrasScene"}}, + }, + }, + ) + segs = _write_extras_timing(tmp_path, 4) + out = build_timing_enrichment_for_prompt(cfg, "08", "08-extras", segs) + assert '"word_index": 0' in out + assert '"word_index": 1' in out + assert '"word_index": 3' not in out + assert "listed 2 of 4 tokens" in out + + # ── Class-name derivation ────────────────────────────────────────────────── From 51cf51bd54a6500c61e4193463165b52a2d58f3e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 8 Sep 2026 00:50:17 +0000 Subject: [PATCH 2/2] Record PR #144 and local gate results for whisper-prompt-caps ruff green; pytest 804 passed, 1 skipped; docgen benchmark meets baseline. Co-authored-by: jmjava --- milestones/whisper-prompt-caps.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/milestones/whisper-prompt-caps.md b/milestones/whisper-prompt-caps.md index d5db94f..f22a750 100644 --- a/milestones/whisper-prompt-caps.md +++ b/milestones/whisper-prompt-caps.md @@ -1,7 +1,7 @@ # Milestone: Whisper prompt caps must not coerce bools **Status:** Active -**PR:** (this PR) +**PR:** [#144](https://github.com/jmjava/documentation-generator/pull/144) **Depends on:** `milestones/pages-ffprobe-returncode.md` (PR #143), `milestones/merge-settings-numerics.md` (PR #142), `milestones/generation-zero-values.md` (PR #122), @@ -32,12 +32,12 @@ Fail closed on present non-number caps. Missing/null still means 0 ## Done when -- [ ] Bool / string whisper count caps raise `SceneGenerationError` -- [ ] `max_whisper_words_in_prompt: 0` still lists every word -- [ ] Chars type-check stays (shared helper) -- [ ] `ruff check src/ tests/` -- [ ] `pytest tests/` -- [ ] `docgen benchmark` (no clock change; meets baseline) +- [x] Bool / string whisper count caps raise `SceneGenerationError` +- [x] `max_whisper_words_in_prompt: 0` still lists every word +- [x] Chars type-check stays (shared helper) +- [x] `ruff check src/ tests/` +- [x] `pytest tests/` (804 passed, 1 skipped) +- [x] `docgen benchmark` (no clock change; meets baseline) ## Out of scope