Fail closed when timing.json start/end are not JSON numbers

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-path-ref.md](wizard-path-ref.md)** —
-wizard `open-bundle` `path` and `tool/update` `ref` must be JSON strings.
+**Active:** **[timing-start-end.md](timing-start-end.md)** —
+`timing.json` word/segment `start` / `end` must be JSON numbers.
 
 **Shipped:**
+- **[wizard-path-ref.md](wizard-path-ref.md)** —
+  wizard `open-bundle` `path` and `tool/update` `ref` must be JSON strings
+  (#133).
 - **[wizard-json-parse.md](wizard-json-parse.md)** —
   wizard JSON bodies must parse; garbage JSON must not look like `{}`
   (#132).

milestones/timing-start-end.md

@@ -0,0 +1,39 @@
+# Milestone: timing.json word/segment start and end must be numbers
+
+**Status:** Active  
+**PR:** [#134](https://github.com/jmjava/documentation-generator/pull/134)  
+**Depends on:** `milestones/timing-inner-lists.md` (PR #127),
+`milestones/bootstrap-timing-helpers.md` (PR #130),
+`milestones/wizard-path-ref.md` (PR #133)
+
+## Problem
+
+PR #127 required ``words`` / ``segments`` to be arrays of objects. Each row
+still used ``float(row.get("start", 0.0))``.
+
+``start: true`` became ``1.0`` because ``bool`` is a subclass of ``int``.
+A missing ``start`` waited until ``0.0`` and dumped the board. A string
+``"0.5"`` looked like a timestamp instead of a type error.
+
+## Goal
+
+When a ``words`` / ``segments`` row is present, ``start`` and ``end`` must
+be JSON numbers (``int`` or ``float``, not ``bool``). Missing / null / string
+rows fail closed in ``load_bundle_timing`` and in Manim ``_load_timing`` /
+``_load_timing_words``. Empty ``[]`` / missing / null lists stay allowed.
+
+``scene-compile`` refreshes bootstrap helpers that only typed objects.
+
+## Done when
+
+- [x] ``load_bundle_timing`` rejects missing / null / bool / string ``start`` / ``end``
+- [x] Bootstrap loaders reject the same at Manim render
+- [x] Stale object-only loaders are refreshed
+- [x] `ruff check src/ tests/`
+- [x] `pytest tests/` (766 passed, 1 skipped)
+- [x] `docgen benchmark` (helper change; meets baseline, no bump)
+
+## Out of scope
+
+- ``wait_until_word`` still swallows ``TypeError`` / ``ValueError`` after load
+- Requiring ``end >= start``

milestones/wizard-path-ref.md

@@ -1,6 +1,6 @@
 # Milestone: wizard open-bundle path and tool/update ref must be strings
 
-**Status:** Active  
+**Status:** Shipped  
 **PR:** [#133](https://github.com/jmjava/documentation-generator/pull/133)  
 **Depends on:** `milestones/wizard-json-parse.md` (PR #132)
 

src/docgen/manim_scene_support.py

@@ -171,6 +171,20 @@ def _load_timing(segment_key: str) -> list[dict]:
             raise TypeError(
                 f"timing.json[{segment_key!r}].segments[{i}] must be a JSON object, not {kind}"
             )
+        for time_key in ("start", "end"):
+            if time_key not in item or item[time_key] is None:
+                tkind = "missing" if time_key not in item else "null"
+                raise TypeError(
+                    f"timing.json[{segment_key!r}].segments[{i}].{time_key} "
+                    f"must be a JSON number, not {tkind}"
+                )
+            val = item[time_key]
+            if isinstance(val, bool) or not isinstance(val, (int, float)):
+                tkind = type(val).__name__
+                raise TypeError(
+                    f"timing.json[{segment_key!r}].segments[{i}].{time_key} "
+                    f"must be a JSON number, not {tkind}"
+                )
     return list(segs)
 
 
@@ -204,6 +218,20 @@ def _load_timing_words(segment_key: str) -> list[dict]:
             raise TypeError(
                 f"timing.json[{segment_key!r}].words[{i}] must be a JSON object, not {kind}"
             )
+        for time_key in ("start", "end"):
+            if time_key not in item or item[time_key] is None:
+                tkind = "missing" if time_key not in item else "null"
+                raise TypeError(
+                    f"timing.json[{segment_key!r}].words[{i}].{time_key} "
+                    f"must be a JSON number, not {tkind}"
+                )
+            val = item[time_key]
+            if isinstance(val, bool) or not isinstance(val, (int, float)):
+                tkind = type(val).__name__
+                raise TypeError(
+                    f"timing.json[{segment_key!r}].words[{i}].{time_key} "
+                    f"must be a JSON number, not {tkind}"
+                )
     return list(words)
 
 
@@ -1297,7 +1325,11 @@ def helper_needs_refresh(tree: ast.AST, name: str) -> bool:
         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)
+            src = ast.unparse(node)
+            return (
+                "must be a JSON object" not in src
+                or "must be a JSON number" not in src
+            )
         if name == "_image" and isinstance(node, ast.FunctionDef) and node.name == "_image":
             return False
     return False

src/docgen/timestamps.py

@@ -32,10 +32,27 @@ def _json_kind(value: Any) -> str:
     return "null" if value is None else type(value).__name__
 
 
+def _json_number_kind(row: dict[str, Any], time_key: str) -> str | None:
+    """Return a kind label when *time_key* is not a JSON number; ``None`` when ok.
+
+    ``bool`` is a subclass of ``int``: ``start: true`` used to become ``1.0s``.
+    """
+    if time_key not in row:
+        return "missing"
+    value = row[time_key]
+    if value is None:
+        return "null"
+    if isinstance(value, bool) or not isinstance(value, (int, float)):
+        return _json_kind(value)
+    return None
+
+
 def _require_timing_object_list(
     path_name: str, stem: str, payload: dict[str, Any], key: str
 ) -> None:
-    """Require ``words`` / ``segments`` (when present and not null) to be object arrays."""
+    """Require ``words`` / ``segments`` (when present and not null) to be object arrays
+    whose ``start`` / ``end`` are JSON numbers.
+    """
     if key not in payload:
         return
     value = payload[key]
@@ -51,15 +68,23 @@ def _require_timing_object_list(
                 f"{path_name}[{stem!r}].{key}[{i}] must be a JSON object, "
                 f"not {_json_kind(item)}"
             )
+        for time_key in ("start", "end"):
+            kind = _json_number_kind(item, time_key)
+            if kind is not None:
+                raise TimestampError(
+                    f"{path_name}[{stem!r}].{key}[{i}].{time_key} must be a JSON "
+                    f"number, not {kind}"
+                )
 
 
 def load_bundle_timing(config: "Config") -> dict[str, Any]:
     """Load ``animations/timing.json``.
 
     A missing file is ``{}``. Corrupt JSON, a non-object root, a non-object
-    per-stem value, or a present ``words`` / ``segments`` field that is not an
-    array of objects raises :class:`TimestampError` so compile/validate cannot
-    treat garbage as empty ``words``.
+    per-stem value, a present ``words`` / ``segments`` field that is not an
+    array of objects, or a row whose ``start`` / ``end`` is not a JSON number
+    raises :class:`TimestampError` so compile/validate cannot treat garbage as
+    empty ``words`` or wait until ``0.0``.
     """
     path = config.animations_dir / "timing.json"
     if not path.is_file():
@@ -168,7 +193,8 @@ def extract_all(self, engine: str | None = None) -> None:
         Successful runs **merge** stems into the existing file (same as the
         wizard per-segment timestamps step) so extra keys not in
         ``segments.all`` are not wiped. Corrupt JSON, a non-object root, a
-        non-object stem, or a non-array ``words`` / ``segments`` field raises
+        non-object stem, a non-array ``words`` / ``segments`` field, or a row
+        whose ``start`` / ``end`` is not a JSON number raises
         :class:`TimestampError` and the file is not rewritten.
         """
         chosen = self.resolve_engine(engine)

tests/test_manim_scene_support.py

@@ -810,6 +810,21 @@ def test_load_timing_helpers_accept_object_rows(tmp_path: Path) -> None:
     assert ns["_load_timing_words"]("missing") == []
 
 
+def test_load_timing_helpers_reject_non_numeric_start_end(tmp_path: Path) -> None:
+    ns = _exec_timing_loaders(
+        tmp_path,
+        {"01-x": {"words": [{"word": "hi", "start": True, "end": 0.1}]}},
+    )
+    with pytest.raises(TypeError, match=r"\.words\[0\].start must be a JSON number, not bool"):
+        ns["_load_timing_words"]("01-x")
+    ns2 = _exec_timing_loaders(
+        tmp_path,
+        {"01-x": {"segments": [{"text": "hi", "start": 0.0}]}},
+    )
+    with pytest.raises(TypeError, match=r"\.segments\[0\].end must be a JSON number, not missing"):
+        ns2["_load_timing"]("01-x")
+
+
 def test_refresh_bootstrap_helpers_upgrades_stale_timing_loaders(tmp_path: Path) -> None:
     p = tmp_path / "scenes.py"
     p.write_text(
@@ -828,9 +843,26 @@ def test_refresh_bootstrap_helpers_upgrades_stale_timing_loaders(tmp_path: Path)
     assert set(changed) == {"_load_timing", "_load_timing_words"}
     text = p.read_text(encoding="utf-8")
     assert "must be a JSON object" in text
+    assert "must be a JSON number" in text
     assert "data.get(segment_key, {}).get('segments'" not in text
 
 
+def test_refresh_bootstrap_helpers_upgrades_object_only_timing_loaders(tmp_path: Path) -> None:
+    """#130 bodies typed stems/rows but still coerced missing start to 0.0."""
+    p = tmp_path / "scenes.py"
+    p.write_text(
+        "from manim import *\n"
+        "def _load_timing(segment_key):\n"
+        "    raise TypeError('timing.json[k] must be a JSON object')\n"
+        "def _load_timing_words(segment_key):\n"
+        "    raise TypeError('timing.json[k] must be a JSON object')\n",
+        encoding="utf-8",
+    )
+    changed = refresh_bootstrap_helpers(p)
+    assert set(changed) == {"_load_timing", "_load_timing_words"}
+    assert "must be a JSON number" in p.read_text(encoding="utf-8")
+
+
 
 def test_ensure_bootstrap_refreshes_stale_helpers(tmp_path: Path) -> None:
     p = tmp_path / "scenes.py"

tests/test_scene_asset_validate.py

@@ -227,6 +227,19 @@ def _load_timing_words(segment_key):
     assert any("_load_timing_words is stale" in i for i in issues)
 
 
+def test_helper_api_flags_object_only_timing_loaders() -> None:
+    stale = '''
+MANIM_FONT = "Liberation Sans"
+def _load_timing(segment_key):
+    raise TypeError("must be a JSON object")
+def _load_timing_words(segment_key):
+    raise TypeError("must be a JSON object")
+'''
+    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) == []
 
@@ -399,3 +412,23 @@ def test_non_array_timing_words_is_reported(tmp_path: Path) -> None:
     assert any("timing.json['01-x'].words must be a JSON array, not str" in i for i in issues)
 
 
+def test_non_numeric_timing_start_is_reported(tmp_path: Path) -> None:
+    cfg = _bundle(tmp_path)
+    specs = cfg.animations_dir / "specs"
+    specs.mkdir(parents=True, exist_ok=True)
+    (specs / "01-x.scene.yaml").write_text(
+        yaml.dump(_spec([_box("Alpha")])),
+        encoding="utf-8",
+    )
+    (cfg.animations_dir / "timing.json").write_text(
+        json.dumps({"01-x": {"words": [{"word": "hi", "start": True, "end": 0.1}]}})
+        + "\n",
+        encoding="utf-8",
+    )
+    issues = scene_asset_violations_for_segment(cfg, "01")
+    assert any(
+        "timing.json['01-x'].words[0].start must be a JSON number, not bool" in i
+        for i in issues
+    )
+
+

tests/test_timestamps_local.py

@@ -194,7 +194,12 @@ def test_extract_all_preserves_extra_timing_stems(self, cfg, monkeypatch) -> Non
         out = cfg.animations_dir / "timing.json"
         out.parent.mkdir(parents=True, exist_ok=True)
         out.write_text(
-            json.dumps({"legacy-stem": {"text": "keep-me", "words": [{"word": "x"}]}}),
+            json.dumps({
+                "legacy-stem": {
+                    "text": "keep-me",
+                    "words": [{"word": "x", "start": 0.0, "end": 0.1}],
+                }
+            }),
             encoding="utf-8",
         )
         TimestampExtractor(cfg).extract_all()
@@ -333,3 +338,49 @@ def test_extract_all_rejects_non_array_words_without_rewrite(self, cfg, monkeypa
             TimestampExtractor(cfg).extract_all()
         assert out.read_text(encoding="utf-8") == payload
 
+    def test_load_bundle_timing_rejects_non_numeric_start_end(self, cfg) -> None:
+        from docgen.timestamps import TimestampError, load_bundle_timing
+
+        out = cfg.animations_dir / "timing.json"
+        out.parent.mkdir(parents=True, exist_ok=True)
+        cases = (
+            (
+                {"01-x": {"words": [{"word": "hi", "end": 0.1}]}},
+                r"timing.json\['01-x'\].words\[0\].start must be a JSON number, not missing",
+            ),
+            (
+                {"01-x": {"words": [{"word": "hi", "start": None, "end": 0.1}]}},
+                r"timing.json\['01-x'\].words\[0\].start must be a JSON number, not null",
+            ),
+            (
+                {"01-x": {"words": [{"word": "hi", "start": True, "end": 0.1}]}},
+                r"timing.json\['01-x'\].words\[0\].start must be a JSON number, not bool",
+            ),
+            (
+                {"01-x": {"words": [{"word": "hi", "start": "0.0", "end": 0.1}]}},
+                r"timing.json\['01-x'\].words\[0\].start must be a JSON number, not str",
+            ),
+            (
+                {"01-x": {"segments": [{"text": "hi", "start": 0.0}]}},
+                r"timing.json\['01-x'\].segments\[0\].end must be a JSON number, not missing",
+            ),
+        )
+        for payload, match in cases:
+            out.write_text(json.dumps(payload), encoding="utf-8")
+            with pytest.raises(TimestampError, match=match):
+                load_bundle_timing(cfg)
+
+    def test_load_bundle_timing_accepts_numeric_start_end(self, cfg) -> None:
+        from docgen.timestamps import load_bundle_timing
+
+        out = cfg.animations_dir / "timing.json"
+        out.parent.mkdir(parents=True, exist_ok=True)
+        payload = {
+            "01-x": {
+                "words": [{"word": "hi", "start": 0, "end": 0.25}],
+                "segments": [{"text": "hi", "start": 0.0, "end": 1}],
+            }
+        }
+        out.write_text(json.dumps(payload), encoding="utf-8")
+        assert load_bundle_timing(cfg) == payload
+