Merge pull request #89 from jmjava/cursor/timing-json-parse-2ccd

Fail closed when timing.json is corrupt at compile and validate

Author: cursor[bot]

milestones/README.md

@@ -5,11 +5,12 @@ 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:** **[lint-empty-narration.md](lint-empty-narration.md)** —
-lint / validate must fail when narration has no spoken prose after markdown
-stripping.
+**Active:** **[timing-json-parse.md](timing-json-parse.md)** —
+corrupt `timing.json` must not be treated as empty words at compile/validate.
 
 **Shipped:**
+- **[lint-empty-narration.md](lint-empty-narration.md)** — lint / validate fail
+  when narration has no spoken prose after markdown stripping (#88).
 - **[segment-id-strings.md](segment-id-strings.md)** — unquoted YAML `01`
   must not become integer segment ids (#87).
 - **[visual-map-row-types.md](visual-map-row-types.md)** — `visual_map` rows

milestones/lint-empty-narration.md

@@ -1,7 +1,7 @@
 # Milestone: lint empty narration
 
-**Status:** Active  
-**PR:** pending  
+**Status:** Shipped  
+**PR:** #88  
 **Depends on:** `milestones/timestamps-empty-words.md` (PR #85)
 
 ## Problem
@@ -19,9 +19,9 @@ Same contract as TTS: empty spoken text is a lint failure.
 
 - [x] `lint_pre_tts` fails when `markdown_to_tts_plain` is empty.
 - [x] Tests: empty, whitespace-only, heading-only, CLI `docgen lint`.
-- [ ] `ruff check src/ tests/`
-- [ ] `pytest tests/`
-- [ ] `docgen benchmark` (no clock change)
+- [x] `ruff check src/ tests/`
+- [x] `pytest tests/`
+- [x] `docgen benchmark` (no clock change)
 
 ## Out of scope
 

milestones/timing-json-parse.md

@@ -0,0 +1,32 @@
+# Milestone: corrupt `timing.json` must not look like empty words
+
+**Status:** Active  
+**PR:** pending  
+**Depends on:** `milestones/wizard-narration-paths.md` (PR #81)
+
+## Problem
+
+Wizard timestamps already refuse to overwrite a corrupt `timing.json`. Compile
+and validate still swallowed `JSONDecodeError` and treated the file as missing
+words. Paced compile then failed with a “run timestamps” message; `pace: none`
+specs compiled as if there were no timing file.
+
+## Goal
+
+One loader: missing file → `{}`; garbage JSON or a non-object root → error.
+
+## Done when
+
+- [x] `load_bundle_timing` in `timestamps.py`
+- [x] scene-compile / `linted_class_block_from_spec` raise `SceneGenerationError`
+- [x] validate `timing_sync` / `story_end` / `av_sync` fail with the parse error
+- [x] `scene_asset_validate` and wizard timestamps reuse the loader
+- [x] Tests: corrupt JSON and list-root, including `pace: none`
+- [ ] `ruff check src/ tests/`
+- [ ] `pytest tests/`
+- [ ] `docgen benchmark` (no clock change)
+
+## Out of scope
+
+- Empty `segments.all` still leaves `timing.json` unchanged (#78)
+- Missing `timing.json` still means empty words (paced compile already fails)

src/docgen/manim_scene_support.py

@@ -595,13 +595,12 @@ def format_pacing_schedule_markdown(segments: list[dict], pace_indices: list[int
 
 def _load_timing_words_from_cfg(cfg: "Config", seg_name: str) -> list[dict]:
     """Return the ``words`` list from ``animations/timing.json`` for ``seg_name``."""
-    timing_path = cfg.animations_dir / "timing.json"
-    if not timing_path.is_file():
-        return []
+    from docgen.timestamps import TimestampError, load_bundle_timing
+
     try:
-        data = json.loads(timing_path.read_text(encoding="utf-8"))
-    except (OSError, json.JSONDecodeError):
-        return []
+        data = load_bundle_timing(cfg)
+    except TimestampError as exc:
+        raise SceneGenerationError(str(exc)) from exc
     block = data.get(seg_name)
     if not isinstance(block, dict):
         return []
@@ -1140,10 +1139,12 @@ def sync_audio_tail_waits_in_scenes(cfg: "Config") -> list[str]:
     timing_path = cfg.animations_dir / "timing.json"
     if not scenes_path.is_file() or not timing_path.is_file():
         return []
+    from docgen.timestamps import TimestampError, load_bundle_timing
+
     try:
-        timing = json.loads(timing_path.read_text(encoding="utf-8"))
-    except (OSError, json.JSONDecodeError):
-        return []
+        timing = load_bundle_timing(cfg)
+    except TimestampError as exc:
+        raise SceneGenerationError(str(exc)) from exc
 
     text = scenes_path.read_text(encoding="utf-8")
     changes: list[str] = []
@@ -1350,11 +1351,14 @@ def _load_narration(cfg: "Config", seg_id: str, seg_name: str) -> str:
 
 
 def _load_timing_segments(cfg: "Config", seg_name: str) -> list[dict]:
-    timing_path = cfg.animations_dir / "timing.json"
-    if not timing_path.exists():
-        return []
+    from docgen.timestamps import TimestampError, load_bundle_timing
+
     try:
-        data = json.loads(timing_path.read_text(encoding="utf-8"))
-    except (OSError, ValueError):
+        data = load_bundle_timing(cfg)
+    except TimestampError as exc:
+        raise SceneGenerationError(str(exc)) from exc
+    block = data.get(seg_name)
+    if not isinstance(block, dict):
         return []
-    return list(data.get(seg_name, {}).get("segments", []))
+    segs = block.get("segments")
+    return list(segs) if isinstance(segs, list) else []

src/docgen/scene_asset_validate.py

@@ -275,22 +275,14 @@ def scene_asset_violations_for_segment(cfg: "Config", seg_id: str) -> list[str]:
         return issues
 
     block: dict[str, Any] = {}
-    timing_path = cfg.animations_dir / "timing.json"
     stem = cfg.resolve_segment_name(seg_id)
-    if timing_path.is_file():
-        import json
+    if cfg.animations_dir.joinpath("timing.json").is_file():
+        from docgen.timestamps import TimestampError, load_bundle_timing
 
         try:
-            data = json.loads(timing_path.read_text(encoding="utf-8"))
-        except (OSError, json.JSONDecodeError) as exc:
-            issues.append(
-                f"timing.json is not valid JSON ({exc}) — run `docgen timestamps`"
-            )
-            data = None
-        if data is not None and not isinstance(data, dict):
-            issues.append(
-                f"timing.json root must be a JSON object, not {type(data).__name__}"
-            )
+            data = load_bundle_timing(cfg)
+        except TimestampError as exc:
+            issues.append(str(exc))
             data = None
         raw_block = data.get(stem) if isinstance(data, dict) else None
         if isinstance(raw_block, dict):

src/docgen/scene_spec_generate.py

@@ -323,16 +323,17 @@ def spec_to_yaml_text(spec: dict[str, Any]) -> str:
 
 
 def _load_timing_words(cfg: Config, timing_key: str) -> list[dict[str, Any]]:
-    """Return the ``words`` list from ``animations/timing.json`` for ``timing_key`` (best effort)."""
-    timing_path = cfg.animations_dir / "timing.json"
-    if not timing_path.exists():
-        return []
+    """Return the ``words`` list from ``animations/timing.json`` for ``timing_key``."""
+    from docgen.timestamps import TimestampError, load_bundle_timing
+
     try:
-        data = json.loads(timing_path.read_text(encoding="utf-8"))
-    except (OSError, ValueError):
+        data = load_bundle_timing(cfg)
+    except TimestampError as exc:
+        raise SceneGenerationError(str(exc)) from exc
+    block = data.get(timing_key)
+    if not isinstance(block, dict):
         return []
-    block = data.get(timing_key) or {}
-    words = block.get("words") if isinstance(block, dict) else None
+    words = block.get("words")
     return list(words) if isinstance(words, list) else []
 
 

src/docgen/timestamps.py

@@ -28,6 +28,32 @@ class TimestampError(RuntimeError):
     """Raised when timestamps cannot be extracted for required segments."""
 
 
+def load_bundle_timing(config: "Config") -> dict[str, Any]:
+    """Load ``animations/timing.json``.
+
+    A missing file is ``{}``. Corrupt JSON or a non-object root raises
+    :class:`TimestampError` so compile/validate cannot treat garbage as empty
+    ``words``.
+    """
+    path = config.animations_dir / "timing.json"
+    if not path.is_file():
+        return {}
+    try:
+        data = json.loads(path.read_text(encoding="utf-8"))
+    except json.JSONDecodeError as exc:
+        raise TimestampError(
+            f"{path.name} is not valid JSON ({exc}) — fix or delete it before "
+            "timestamps/compile"
+        ) from exc
+    except OSError as exc:
+        raise TimestampError(f"could not read {path}: {exc}") from exc
+    if not isinstance(data, dict):
+        raise TimestampError(
+            f"{path.name} root must be a JSON object, not {type(data).__name__}"
+        )
+    return data
+
+
 class TimestampExtractor:
     def __init__(self, config: Config) -> None:
         self.config = config

src/docgen/validate.py

@@ -732,7 +732,12 @@ def _check_timing_sync(self, seg_id: str) -> CheckResult:
             return CheckResult("timing_sync", True, ["Audio is an LFS pointer (skipped)"])
 
         is_manim = self.config.visual_map.get(seg_id, {}).get("type") == "manim"
-        block = self._load_timing_block(seg_id)
+        from docgen.timestamps import TimestampError
+
+        try:
+            block = self._load_timing_block(seg_id)
+        except TimestampError as exc:
+            return CheckResult("timing_sync", False, [str(exc)])
         if block is None:
             if is_manim:
                 return CheckResult(
@@ -811,7 +816,12 @@ def _check_story_end(self, seg_id: str) -> CheckResult:
                 "story_end", True, ["No animations/specs/*.scene.yaml (skipped)"]
             )
 
-        block = self._load_timing_block(seg_id)
+        from docgen.timestamps import TimestampError
+
+        try:
+            block = self._load_timing_block(seg_id)
+        except TimestampError as exc:
+            return CheckResult("story_end", False, [str(exc)])
         words = block.get("words") if isinstance(block, dict) else None
         words_ok = isinstance(words, list) and bool(words)
 
@@ -905,7 +915,12 @@ def _check_av_sync(self, seg_id: str, rec: Path) -> CheckResult:
         except Exception:
             return CheckResult("av_sync", True, ["tesseract binary not installed (skipped)"])
 
-        block = self._load_timing_block(seg_id)
+        from docgen.timestamps import TimestampError
+
+        try:
+            block = self._load_timing_block(seg_id)
+        except TimestampError as exc:
+            return CheckResult("av_sync", False, [str(exc)])
         if block is None:
             return CheckResult(
                 "av_sync", True, ["No timing.json entry (skipped) — run `docgen timestamps`"]
@@ -927,13 +942,9 @@ def _check_av_sync(self, seg_id: str, rec: Path) -> CheckResult:
 
     def _load_timing_block(self, seg_id: str) -> dict[str, Any] | None:
         """One segment's block from ``animations/timing.json`` (keyed by narration stem)."""
-        timing_path = self.config.animations_dir / "timing.json"
-        if not timing_path.is_file():
-            return None
-        try:
-            data = json.loads(timing_path.read_text(encoding="utf-8"))
-        except (OSError, json.JSONDecodeError):
-            return None
+        from docgen.timestamps import load_bundle_timing
+
+        data = load_bundle_timing(self.config)
         stem = self.config.resolve_segment_name(seg_id)
         block = data.get(stem)
         if not isinstance(block, dict):

src/docgen/wizard.py

@@ -861,20 +861,14 @@ def _run_segment_step(cfg: Any, step: str, segment_id: str) -> dict[str, Any]:
             block = (
                 ts.extract(mp3) if engine == "whisper" else ts.extract_local(mp3)
             )
-            out = cfg.animations_dir / "timing.json"
-            timing: dict = {}
-            if out.is_file():
-                try:
-                    timing = _json.loads(out.read_text(encoding="utf-8"))
-                except _json.JSONDecodeError as exc:
-                    raise RuntimeError(
-                        f"{out.name} is not valid JSON; fix or delete it before timestamps"
-                    ) from exc
-            if not isinstance(timing, dict):
-                raise RuntimeError(
-                    f"{out.name} root must be a JSON object, not {type(timing).__name__}"
-                )
+            from docgen.timestamps import TimestampError, load_bundle_timing
+
+            try:
+                timing = dict(load_bundle_timing(cfg))
+            except TimestampError as exc:
+                raise RuntimeError(str(exc)) from exc
             timing[mp3.stem] = block
+            out = cfg.animations_dir / "timing.json"
             out.parent.mkdir(parents=True, exist_ok=True)
             out.write_text(
                 _json.dumps(timing, indent=2, ensure_ascii=False) + "\n",

tests/test_scene_retime.py

@@ -229,6 +229,60 @@ def test_linted_class_block_allows_pace_none_without_words(tmp_path: Path) -> No
     assert merged["rows"][0]["boxes"][0].get("pace") == "none"
 
 
+def test_linted_class_block_fails_on_corrupt_timing_json(tmp_path: Path) -> None:
+    cfg = _cfg(tmp_path)
+    (tmp_path / "animations" / "timing.json").write_text("{not json", encoding="utf-8")
+    spec = {
+        "segment_id": "01",
+        "class_name": "DemoScene",
+        "title": {"text": "Demo", "font_size": 36, "color": "C_WHITE"},
+        "rows": [
+            {
+                "run_time": 1.0,
+                "boxes": [
+                    {
+                        "label": "Hello",
+                        "color": "C_GREEN",
+                        "width": 3.0,
+                        "height": 0.9,
+                        "font_size": 18,
+                        "pace": "none",
+                    }
+                ],
+            }
+        ],
+    }
+    with pytest.raises(SceneGenerationError, match="not valid JSON"):
+        linted_class_block_from_spec(cfg, spec, timing_key="01-demo")
+
+
+def test_linted_class_block_fails_on_list_root_timing_json(tmp_path: Path) -> None:
+    cfg = _cfg(tmp_path)
+    (tmp_path / "animations" / "timing.json").write_text("[]\n", encoding="utf-8")
+    spec = {
+        "segment_id": "01",
+        "class_name": "DemoScene",
+        "title": {"text": "Demo", "font_size": 36, "color": "C_WHITE"},
+        "rows": [
+            {
+                "run_time": 1.0,
+                "boxes": [
+                    {
+                        "label": "Hello",
+                        "color": "C_GREEN",
+                        "width": 3.0,
+                        "height": 0.9,
+                        "font_size": 18,
+                        "pace": "none",
+                    }
+                ],
+            }
+        ],
+    }
+    with pytest.raises(SceneGenerationError, match="JSON object"):
+        linted_class_block_from_spec(cfg, spec, timing_key="01-demo")
+
+
 def test_retime_compile_spec_rewrites_scenes_py(tmp_path: Path) -> None:
     cfg = _cfg(tmp_path)
     path = _write_spec(tmp_path, label="Hello")