Skip to content

Commit b2ea378

Browse files
cursoragentjmjava
andcommitted
Fail closed when wizard freshness sees corrupt timing.json
The asset graph treated garbage timing.json as a missing timestamps entry. Reuse load_bundle_timing so list/assets APIs return 500 instead of a fake "run timestamps" gap. Co-authored-by: jmjava <jmjava@gmail.com>
1 parent f11e99f commit b2ea378

6 files changed

Lines changed: 108 additions & 17 deletions

File tree

‎milestones/README.md‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@ repositories that install `docgen` and maintain their own demo bundle. The
55
library no longer ships an in-repo dogfood; consumers are the integration test
66
of record.
77

8-
**Active:** **[yaml-generate-mappings.md](yaml-generate-mappings.md)** —
9-
`yaml-generate` merge/discover must not replace a list/scalar `visual_map` or
10-
narration/manim block with `{}`.
8+
**Active:** **[asset-graph-timing.md](asset-graph-timing.md)** — wizard
9+
freshness must not treat corrupt `timing.json` as a missing timestamps entry.
1110

1211
**Shipped:**
12+
- **[yaml-generate-mappings.md](yaml-generate-mappings.md)** — `yaml-generate`
13+
merge/discover must not replace a list/scalar `visual_map` or narration/manim
14+
block with `{}` (#91).
1315
- **[context-path-lists.md](context-path-lists.md)** — `pages.segments` and
1416
narration/manim context path lists must be typed (#90).
1517
- **[timing-json-parse.md](timing-json-parse.md)** — corrupt `timing.json`

‎milestones/asset-graph-timing.md‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# Milestone: wizard freshness must not treat corrupt `timing.json` as missing
2+
3+
**Status:** Active
4+
**PR:** pending
5+
**Depends on:** `milestones/timing-json-parse.md` (PR #89),
6+
`milestones/wizard-narration-paths.md` (PR #81)
7+
8+
## Problem
9+
10+
Compile, validate, scene-asset preflight, and wizard **timestamps write** already
11+
fail closed on garbage `timing.json` (`load_bundle_timing` → `TimestampError`).
12+
13+
The wizard **asset freshness** graph still caught `JSONDecodeError` / a list root
14+
and returned “no timing.json entry — run timestamps”. Maintainers saw a missing
15+
step instead of a corrupt file, and `/api/segments` returned 200.
16+
17+
## Goal
18+
19+
Reuse `load_bundle_timing` in the freshness graph. Missing file stays “no entry”.
20+
Corrupt JSON / non-object root raises; wizard list/assets endpoints return 500
21+
and do not rewrite the file.
22+
23+
## Done when
24+
25+
- [x] `_timing_entry_exists` uses `load_bundle_timing` (no silent `False` on
26+
garbage JSON).
27+
- [x] `GET /api/segments` and `GET /api/segments/<id>/assets` map
28+
`TimestampError` to HTTP 500 with the parse message.
29+
- [x] Tests for corrupt JSON and a list root.
30+
- [ ] `ruff check src/ tests/`
31+
- [ ] `pytest tests/`
32+
- [ ] `docgen benchmark` (no clock change)
33+
34+
## Out of scope
35+
36+
- Missing `timing.json` still shows timestamps as missing.
37+
- CLI `timestamps extract_all` still rewrites the whole file for `segments.all`
38+
(recovery path; does not merge extra stems).
39+
- Wizard `.docgen-state.json` corrupt still resets to empty (UI session, not
40+
pipeline timing).

‎milestones/yaml-generate-mappings.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# Milestone: yaml-generate must not wipe non-mapping blocks
22

3-
**Status:** Active
4-
**PR:** pending
3+
**Status:** Shipped
4+
**PR:** #91
55
**Depends on:** `milestones/context-path-lists.md` (PR #90),
66
`milestones/config-mapping-keys.md` (PR #83)
77

‎src/docgen/asset_graph.py‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
from __future__ import annotations
1010

11-
import json
1211
from dataclasses import asdict, dataclass
1312
from pathlib import Path
1413
from typing import Any, TYPE_CHECKING
@@ -120,15 +119,15 @@ def _find_asset(directory: Path, seg_name: str, seg_id: str, ext: str) -> Path |
120119

121120

122121
def _timing_entry_exists(cfg: "Config", seg_name: str, audio: Path | None) -> bool:
123-
timing_path = cfg.animations_dir / "timing.json"
124-
if not timing_path.is_file():
125-
return False
126-
try:
127-
data = json.loads(timing_path.read_text(encoding="utf-8"))
128-
except (OSError, json.JSONDecodeError):
129-
return False
130-
if not isinstance(data, dict):
131-
return False
122+
"""True when ``timing.json`` has a stem for this segment.
123+
124+
A missing file is ``False`` (timestamps not run yet). Corrupt JSON or a
125+
non-object root raises :class:`~docgen.timestamps.TimestampError` so the
126+
wizard cannot treat garbage as “no entry”.
127+
"""
128+
from docgen.timestamps import load_bundle_timing
129+
130+
data = load_bundle_timing(cfg)
132131
if seg_name in data:
133132
return True
134133
if audio is not None and audio.stem in data:

‎src/docgen/wizard.py‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -685,8 +685,12 @@ def api_segments():
685685
merged_narration_from_source_settings(cfg, seg_id).context_paths
686686
)
687687
from docgen.asset_graph import segment_asset_report
688+
from docgen.timestamps import TimestampError
688689

689-
assets = segment_asset_report(cfg, seg_id)
690+
try:
691+
assets = segment_asset_report(cfg, seg_id)
692+
except TimestampError as exc:
693+
return jsonify({"error": str(exc)}), 500
690694
result.append({
691695
"id": seg_id,
692696
"name": seg_name,
@@ -711,8 +715,12 @@ def api_segment_assets(segment_id: str):
711715
if not cfg:
712716
return jsonify({"error": "no config"}), 400
713717
from docgen.asset_graph import segment_asset_report
718+
from docgen.timestamps import TimestampError
714719

715-
return jsonify(segment_asset_report(cfg, segment_id))
720+
try:
721+
return jsonify(segment_asset_report(cfg, segment_id))
722+
except TimestampError as exc:
723+
return jsonify({"error": str(exc)}), 500
716724

717725
@app.route("/api/segments/<segment_id>/focus")
718726
def api_get_focus(segment_id: str):

‎tests/test_asset_graph.py‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,48 @@ def test_api_segments_includes_assets(tmp_path: Path) -> None:
133133
assert assets.get_json()["segment_id"] == "01"
134134

135135

136+
def test_segment_statuses_corrupt_timing_json_raises(tmp_path: Path) -> None:
137+
from docgen.timestamps import TimestampError
138+
139+
cfg = _bundle(tmp_path)
140+
(cfg.audio_dir / "01-demo.mp3").write_bytes(b"fake")
141+
(cfg.animations_dir / "timing.json").write_text("{not-json", encoding="utf-8")
142+
with pytest.raises(TimestampError, match="not valid JSON"):
143+
segment_step_statuses(cfg, "01")
144+
145+
146+
def test_segment_statuses_non_object_timing_json_raises(tmp_path: Path) -> None:
147+
from docgen.timestamps import TimestampError
148+
149+
cfg = _bundle(tmp_path)
150+
(cfg.animations_dir / "timing.json").write_text("[1, 2]", encoding="utf-8")
151+
with pytest.raises(TimestampError, match="JSON object"):
152+
segment_step_statuses(cfg, "01")
153+
154+
155+
def test_api_segments_rejects_corrupt_timing_json(tmp_path: Path) -> None:
156+
cfg = _bundle(tmp_path)
157+
(cfg.animations_dir / "timing.json").write_text("{not-json", encoding="utf-8")
158+
app = create_app(cfg)
159+
client = app.test_client()
160+
res = client.get("/api/segments")
161+
assert res.status_code == 500
162+
err = res.get_json()["error"]
163+
assert "not valid JSON" in err
164+
165+
166+
def test_api_segment_assets_rejects_corrupt_timing_json(tmp_path: Path) -> None:
167+
cfg = _bundle(tmp_path)
168+
(cfg.animations_dir / "timing.json").write_text("{not-json", encoding="utf-8")
169+
app = create_app(cfg)
170+
client = app.test_client()
171+
res = client.get("/api/segments/01/assets")
172+
assert res.status_code == 500
173+
assert "not valid JSON" in res.get_json()["error"]
174+
assert (cfg.animations_dir / "timing.json").read_text(encoding="utf-8") == "{not-json"
175+
176+
177+
136178
def test_api_run_from_cascades_mocked(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
137179
cfg = _bundle(tmp_path)
138180
app = create_app(cfg)

0 commit comments

Comments
 (0)