Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions milestones/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions milestones/lint-empty-narration.md
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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

Expand Down
32 changes: 32 additions & 0 deletions milestones/timing-json-parse.md
Original file line number Diff line number Diff line change
@@ -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)
34 changes: 19 additions & 15 deletions src/docgen/manim_scene_support.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 []
Expand Down Expand Up @@ -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] = []
Expand Down Expand Up @@ -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 []
18 changes: 5 additions & 13 deletions src/docgen/scene_asset_validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
17 changes: 9 additions & 8 deletions src/docgen/scene_spec_generate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 []


Expand Down
26 changes: 26 additions & 0 deletions src/docgen/timestamps.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 21 additions & 10 deletions src/docgen/validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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`"]
Expand All @@ -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):
Expand Down
20 changes: 7 additions & 13 deletions src/docgen/wizard.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
54 changes: 54 additions & 0 deletions tests/test_scene_retime.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
7 changes: 7 additions & 0 deletions tests/test_validate_timing_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,13 @@ def test_missing_timing_entry_fails_for_manim(self, cfg, monkeypatch) -> None:
assert not check.passed
assert any("docgen timestamps" in d for d in check.details)

def test_corrupt_timing_json_fails_for_manim(self, cfg, monkeypatch) -> None:
(cfg.animations_dir / "timing.json").write_text("{not json", encoding="utf-8")
_patch_audio_duration(monkeypatch, 10.0)
check = Validator(cfg)._check_timing_sync("01")
assert not check.passed
assert any("not valid JSON" in d for d in check.details)

def test_missing_timing_entry_skips_for_non_manim(self, tmp_path, monkeypatch) -> None:
cfg = _bundle(tmp_path, visual_type="still")
(cfg.audio_dir / "01-x.mp3").write_bytes(b"fake mp3 bytes")
Expand Down