Fail closed when Manim timing helpers see corrupt timing.json

milestones/README.md

@@ -5,10 +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:** **[wizard-json-object.md](wizard-json-object.md)** —
-wizard POST/PUT bodies must be JSON objects; bool fields must be booleans.
+**Active:** **[bootstrap-timing-helpers.md](bootstrap-timing-helpers.md)** —
+Manim `_load_timing` helpers must fail closed on corrupt `timing.json`.
 
 **Shipped:**
+- **[wizard-json-object.md](wizard-json-object.md)** —
+  wizard POST/PUT bodies must be JSON objects; bool fields must be booleans
+  (#129).
 - **[wizard-state-segments.md](wizard-state-segments.md)** —
   wizard `.docgen-state.json` `segments` must be a mapping of objects
   (#128).

milestones/bootstrap-timing-helpers.md

@@ -0,0 +1,42 @@
+# Milestone: Manim bootstrap timing loaders must type timing.json
+
+**Status:** Active  
+**PR:** [#130](https://github.com/jmjava/documentation-generator/pull/130)  
+**Depends on:** `milestones/timing-inner-lists.md` (PR #127),
+`milestones/wizard-json-object.md` (PR #129)
+
+## Problem
+
+Library ``load_bundle_timing`` now rejects non-object stems and non-array
+``words`` / ``segments``. Compiled ``scenes.py`` still used:
+
+```python
+data.get(segment_key, {}).get("segments", [])
+block = data.get(segment_key) or {}
+```
+
+A list stem raises ``AttributeError`` at Manim ``construct()``. A string
+``words`` field is coerced to empty and the board is unpaced.
+
+``scene-compile`` did not refresh those helpers.
+
+## Goal
+
+Bootstrap ``_load_timing`` / ``_load_timing_words`` raise ``TypeError`` on
+corrupt stem / inner types. Missing file or missing stem still returns
+``[]``. ``refresh_bootstrap_helpers`` and ``helper_api_violations`` treat
+the old bodies as stale.
+
+## Done when
+
+- [x] New helpers reject list stems and non-array ``words`` / ``segments``
+- [x] ``refresh_bootstrap_helpers`` rewrites stale loaders
+- [x] ``helper_api_violations`` flags stale loaders
+- [x] `ruff check src/ tests/`
+- [x] `pytest tests/` (750 passed, 1 skipped)
+- [x] `docgen benchmark` (helper change; meets baseline, no bump)
+
+## Out of scope
+
+- Changing the corrupt-JSON → empty reset in wizard ``load_state``
+- Requiring numeric ``start`` / ``end`` on every word row

milestones/wizard-json-object.md

@@ -1,6 +1,6 @@
 # Milestone: wizard POST bodies must be JSON objects
 
-**Status:** Active  
+**Status:** Shipped  
 **PR:** [#129](https://github.com/jmjava/documentation-generator/pull/129)  
 **Depends on:** `milestones/wizard-state-segments.md` (PR #128)
 

src/docgen/manim_scene_support.py

@@ -147,7 +147,31 @@ def _load_timing(segment_key: str) -> list[dict]:
     if not timing_path.exists():
         return []
     data = json.loads(timing_path.read_text())
-    return data.get(segment_key, {}).get("segments", [])
+    if not isinstance(data, dict):
+        raise TypeError(
+            f"timing.json root must be a JSON object, not {type(data).__name__}"
+        )
+    block = data.get(segment_key)
+    if block is None:
+        return []
+    if not isinstance(block, dict):
+        raise TypeError(
+            f"timing.json[{segment_key!r}] must be a JSON object, not {type(block).__name__}"
+        )
+    segs = block.get("segments")
+    if segs is None:
+        return []
+    if not isinstance(segs, list):
+        raise TypeError(
+            f"timing.json[{segment_key!r}].segments must be a JSON array, not {type(segs).__name__}"
+        )
+    for i, item in enumerate(segs):
+        kind = "null" if item is None else type(item).__name__
+        if not isinstance(item, dict):
+            raise TypeError(
+                f"timing.json[{segment_key!r}].segments[{i}] must be a JSON object, not {kind}"
+            )
+    return list(segs)
 
 
 def _load_timing_words(segment_key: str) -> list[dict]:
@@ -156,9 +180,31 @@ def _load_timing_words(segment_key: str) -> list[dict]:
     if not timing_path.exists():
         return []
     data = json.loads(timing_path.read_text())
-    block = data.get(segment_key) or {}
+    if not isinstance(data, dict):
+        raise TypeError(
+            f"timing.json root must be a JSON object, not {type(data).__name__}"
+        )
+    block = data.get(segment_key)
+    if block is None:
+        return []
+    if not isinstance(block, dict):
+        raise TypeError(
+            f"timing.json[{segment_key!r}] must be a JSON object, not {type(block).__name__}"
+        )
     words = block.get("words")
-    return list(words) if isinstance(words, list) else []
+    if words is None:
+        return []
+    if not isinstance(words, list):
+        raise TypeError(
+            f"timing.json[{segment_key!r}].words must be a JSON array, not {type(words).__name__}"
+        )
+    for i, item in enumerate(words):
+        kind = "null" if item is None else type(item).__name__
+        if not isinstance(item, dict):
+            raise TypeError(
+                f"timing.json[{segment_key!r}].words[{i}] must be a JSON object, not {kind}"
+            )
+    return list(words)
 
 
 def _box(label, color, w=2.2, h=0.75, fs=18, subtitle="", shape="rounded"):
@@ -1248,6 +1294,10 @@ def helper_needs_refresh(tree: ast.AST, name: str) -> bool:
             if timed is None:
                 return True
             return "not_past" not in _fn_arg_names(timed)
+        if name in {"_load_timing", "_load_timing_words"} and isinstance(
+            node, ast.FunctionDef
+        ) and node.name == name:
+            return "must be a JSON object" not in ast.unparse(node)
         if name == "_image" and isinstance(node, ast.FunctionDef) and node.name == "_image":
             return False
     return False
@@ -1267,7 +1317,7 @@ def _replace_top_level_def(text: str, node: ast.AST, new_src: str) -> str:
 
 
 def refresh_bootstrap_helpers(scenes_path: Path) -> list[str]:
-    """Replace stale ``_box`` / ``_arrow`` / ``_TimedScene`` with canonical bodies.
+    """Replace stale ``_box`` / ``_arrow`` / ``_TimedScene`` / timing loaders.
 
     Does not touch generated scene classes. Missing ``_image`` is still handled
     by :func:`ensure_image_helper`. Returns the names that were rewritten.
@@ -1287,7 +1337,12 @@ def refresh_bootstrap_helpers(scenes_path: Path) -> list[str]:
     # Replace from the bottom of the file so earlier line numbers stay valid.
     nodes: list[tuple[int, str, ast.AST]] = []
     for node in tree.body:
-        if isinstance(node, ast.FunctionDef) and node.name in {"_box", "_arrow"}:
+        if isinstance(node, ast.FunctionDef) and node.name in {
+            "_box",
+            "_arrow",
+            "_load_timing",
+            "_load_timing_words",
+        }:
             if helper_needs_refresh(tree, node.name):
                 nodes.append((node.lineno, node.name, node))
         elif isinstance(node, ast.ClassDef) and node.name == "_TimedScene":

src/docgen/scene_asset_validate.py

@@ -203,19 +203,19 @@ def helper_api_violations(scenes_text: str) -> list[str]:
     for node in tree.body:
         if isinstance(node, (ast.FunctionDef, ast.ClassDef)):
             defined.add(node.name)
-    if not defined.intersection({"_box", "_arrow", "_TimedScene"}):
+    if not defined.intersection({"_box", "_arrow", "_TimedScene", "_load_timing", "_load_timing_words"}):
         return []
     issues: list[str] = []
     if "MANIM_FONT" not in scenes_text:
         issues.append(
             "font: scenes.py is missing MANIM_FONT — run `docgen scene-compile` "
             "to refresh helpers (Pango default fonts drift across machines)"
         )
-    for name in ("_box", "_arrow", "_TimedScene"):
+    for name in ("_box", "_arrow", "_TimedScene", "_load_timing", "_load_timing_words"):
         if name in defined and helper_needs_refresh(tree, name):
             issues.append(
-                f"helpers: {name} is stale (missing shape / edge-to-edge / not_past) — "
-                "run `docgen scene-compile` to refresh helper bodies"
+                f"helpers: {name} is stale (missing shape / edge-to-edge / not_past / "
+                "typed timing.json loaders) — run `docgen scene-compile` to refresh helper bodies"
             )
     return issues
 

tests/test_manim_scene_support.py

@@ -31,6 +31,7 @@
     refresh_bootstrap_helpers,
     resolve_pace_segment_indices,
     sync_audio_tail_waits_in_scenes,
+    _bootstrap_helper_source,
 )
 
 
@@ -765,6 +766,72 @@ def test_refresh_bootstrap_helpers_noop_when_current(tmp_path: Path) -> None:
     assert p.read_text(encoding="utf-8") == before
 
 
+def _exec_timing_loaders(tmp_path: Path, payload: object) -> dict:
+    (tmp_path / "timing.json").write_text(json.dumps(payload), encoding="utf-8")
+    src = (
+        _bootstrap_helper_source("_load_timing")
+        + "\n"
+        + _bootstrap_helper_source("_load_timing_words")
+    )
+    ns: dict = {"Path": Path, "json": json, "__file__": str(tmp_path / "scenes.py")}
+    exec(compile(src, str(tmp_path / "scenes.py"), "exec"), ns)
+    return ns
+
+
+def test_load_timing_helpers_reject_list_stem(tmp_path: Path) -> None:
+    ns = _exec_timing_loaders(tmp_path, {"01-x": [{"start": 0.0}]})
+    with pytest.raises(TypeError, match=r"timing.json\['01-x'\] must be a JSON object"):
+        ns["_load_timing"]("01-x")
+    with pytest.raises(TypeError, match=r"timing.json\['01-x'\] must be a JSON object"):
+        ns["_load_timing_words"]("01-x")
+
+
+def test_load_timing_helpers_reject_non_array_inner_fields(tmp_path: Path) -> None:
+    ns = _exec_timing_loaders(tmp_path, {"01-x": {"words": "x", "segments": {}}})
+    with pytest.raises(TypeError, match=r"\.words must be a JSON array"):
+        ns["_load_timing_words"]("01-x")
+    with pytest.raises(TypeError, match=r"\.segments must be a JSON array"):
+        ns["_load_timing"]("01-x")
+
+
+def test_load_timing_helpers_accept_object_rows(tmp_path: Path) -> None:
+    ns = _exec_timing_loaders(
+        tmp_path,
+        {
+            "01-x": {
+                "words": [{"word": "hi", "start": 0.0, "end": 0.1}],
+                "segments": [{"text": "hi", "start": 0.0, "end": 0.1}],
+            }
+        },
+    )
+    assert ns["_load_timing_words"]("01-x")[0]["word"] == "hi"
+    assert ns["_load_timing"]("01-x")[0]["text"] == "hi"
+    assert ns["_load_timing"]("missing") == []
+    assert ns["_load_timing_words"]("missing") == []
+
+
+def test_refresh_bootstrap_helpers_upgrades_stale_timing_loaders(tmp_path: Path) -> None:
+    p = tmp_path / "scenes.py"
+    p.write_text(
+        "from manim import *\n"
+        "def _load_timing(segment_key):\n"
+        "    data = json.loads(Path('timing.json').read_text())\n"
+        "    return data.get(segment_key, {}).get('segments', [])\n"
+        "def _load_timing_words(segment_key):\n"
+        "    data = json.loads(Path('timing.json').read_text())\n"
+        "    block = data.get(segment_key) or {}\n"
+        "    words = block.get('words')\n"
+        "    return list(words) if isinstance(words, list) else []\n",
+        encoding="utf-8",
+    )
+    changed = refresh_bootstrap_helpers(p)
+    assert set(changed) == {"_load_timing", "_load_timing_words"}
+    text = p.read_text(encoding="utf-8")
+    assert "must be a JSON object" in text
+    assert "data.get(segment_key, {}).get('segments'" not in text
+
+
+
 def test_ensure_bootstrap_refreshes_stale_helpers(tmp_path: Path) -> None:
     p = tmp_path / "scenes.py"
     p.write_text(

tests/test_scene_asset_validate.py

@@ -213,6 +213,20 @@ def timed_play(self, *a, run_time=1.0):
     assert any("stale" in i for i in issues)
 
 
+def test_helper_api_flags_stale_timing_loaders() -> None:
+    stale = '''
+MANIM_FONT = "Liberation Sans"
+def _load_timing(segment_key):
+    return data.get(segment_key, {}).get("segments", [])
+def _load_timing_words(segment_key):
+    block = data.get(segment_key) or {}
+    return block.get("words")
+'''
+    issues = helper_api_violations(stale)
+    assert any("_load_timing is stale" in i for i in issues)
+    assert any("_load_timing_words is stale" in i for i in issues)
+
+
 def test_helper_api_clean_for_current_bootstrap() -> None:
     assert helper_api_violations(BOOTSTRAP_HEADER) == []