Merge pull request #81 from jmjava/cursor/wizard-narration-paths-2ccd

Fail closed on wizard missing sources and corrupt timing.json

Author: cursor[bot]

milestones/README.md

@@ -5,11 +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:** **[validate-fail-closed.md](validate-fail-closed.md)** — validate
-and remaining CLI skips must not pass listed segments with no narration,
-untimed paced specs, or dropped context files.
+**Active:** **[wizard-narration-paths.md](wizard-narration-paths.md)** — wizard
+generate-narration must not drop missing sources; timestamps must not wipe
+corrupt `timing.json`.
 
 **Shipped:**
+- **[validate-fail-closed.md](validate-fail-closed.md)** — validate
+  narration/timing skips, empty Manim lists, missing context paths.
 - **[scene-compile-pace.md](scene-compile-pace.md)** — paced `scene-compile`
   requires timing words; `yaml-generate` keeps committed Manim rows.
 - **[timestamps-fail-closed.md](timestamps-fail-closed.md)** — timestamps,

milestones/wizard-narration-paths.md

@@ -0,0 +1,23 @@
+# Milestone — Wizard narration paths and timing.json
+
+**Goal:** Wizard generate-narration must not call the LLM with empty or
+dropped source files, and per-segment timestamps must not wipe
+`timing.json` when the file is corrupt.
+
+**Branch / PR:** `cursor/wizard-narration-paths-2ccd`
+
+Follows **[validate-fail-closed.md](validate-fail-closed.md)** (#80).
+
+## Shipped
+
+- [x] `/api/generate-narration` returns 400 when a listed `source_path`
+      is missing (same contract as hint-save)
+- [x] Generate mode requires at least one source file; revise may omit sources
+- [x] `generate_narration_via_llm` generate mode rejects empty `source_texts`
+- [x] Wizard timestamps step raises on invalid `timing.json` instead of
+      replacing it with `{}` (which dropped other stems)
+
+## Not this milestone
+
+- GET `/api/narration` empty 200 for a new segment (authoring UI)
+- Archived Playwright / Embabel / slides

src/docgen/wizard.py

@@ -319,6 +319,11 @@ def generate_narration_via_llm(
             "that satisfy the revision notes over a full rewrite."
         )
     else:
+        if not source_texts:
+            raise ValueError(
+                "generate mode requires source documentation — pick files in the wizard "
+                "or set narration_from_source.context.paths"
+            )
         user_parts = [
             f"Write a narration script focused on: {focus}.",
             "Do not mention segment numbers, file stems, ordinals, or any 'segment NN' phrasing.",
@@ -558,6 +563,12 @@ def api_generate_narration():
             except Exception:
                 topic_label = None
 
+        effective_mode = str(mode or "generate").strip().lower()
+        if effective_mode not in ("generate", "revise"):
+            effective_mode = "generate"
+        if effective_mode == "generate" and current_narration.strip() and revision_notes.strip():
+            effective_mode = "revise"
+
         root = cfg.repo_root if cfg else Path.cwd()
         wiz = cfg.wizard_config if cfg else {}
 
@@ -587,10 +598,19 @@ def api_generate_narration():
             fpath = (root / rel_s).resolve()
             if not _is_under_root(fpath, root):
                 return jsonify({"error": f"path escapes repo_root: {rel_s}"}), 400
-            if fpath.exists() and fpath.is_file():
-                source_texts.append(
-                    f"## File: {rel_s}\n{fpath.read_text(encoding='utf-8', errors='replace')}"
+            if not fpath.is_file():
+                return jsonify({"error": f"file not found: {rel_s}"}), 400
+            source_texts.append(
+                f"## File: {rel_s}\n{fpath.read_text(encoding='utf-8', errors='replace')}"
+            )
+
+        if effective_mode != "revise" and not source_texts:
+            return jsonify({
+                "error": (
+                    "generate mode requires source files — pick files in the wizard "
+                    "or set narration_from_source.context.paths / hint wiring"
                 )
+            }), 400
 
         try:
             narration = generate_narration_via_llm(
@@ -602,7 +622,7 @@ def api_generate_narration():
                 revision_notes=revision_notes,
                 topic_label=topic_label,
                 current_narration=current_narration,
-                mode=mode,
+                mode=effective_mode,
                 cfg=cfg,
             )
         except ValueError as exc:
@@ -615,12 +635,6 @@ def api_generate_narration():
         out = narration_dir / f"{segment_name}.md"
         out.write_text(narration + "\n", encoding="utf-8")
 
-        effective_mode = str(mode or "generate").strip().lower()
-        if effective_mode not in ("generate", "revise"):
-            effective_mode = "generate"
-        if effective_mode == "generate" and current_narration.strip() and revision_notes.strip():
-            effective_mode = "revise"
-
         return jsonify({
             "narration": narration,
             "path": str(out),
@@ -856,10 +870,14 @@ def _run_segment_step(cfg: Any, step: str, segment_id: str) -> dict[str, Any]:
             if out.is_file():
                 try:
                     timing = _json.loads(out.read_text(encoding="utf-8"))
-                except _json.JSONDecodeError:
-                    timing = {}
+                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):
-                timing = {}
+                raise RuntimeError(
+                    f"{out.name} root must be a JSON object, not {type(timing).__name__}"
+                )
             timing[mp3.stem] = block
             out.parent.mkdir(parents=True, exist_ok=True)
             out.write_text(

tests/test_asset_graph.py

@@ -198,6 +198,34 @@ def test_api_run_from_cascades_mocked(tmp_path: Path, monkeypatch: pytest.Monkey
     assert called == ["timestamps", "scene-retime", "manim", "compose", "validate"]
 
 
+def test_api_timestamps_rejects_corrupt_timing_json(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    cfg = _bundle(tmp_path)
+    (cfg.audio_dir / "01-demo.mp3").write_bytes(b"x")
+    (cfg.animations_dir / "timing.json").write_text("{not-json", encoding="utf-8")
+    app = create_app(cfg)
+    client = app.test_client()
+
+    from docgen import timestamps
+
+    monkeypatch.setattr(
+        timestamps.TimestampExtractor,
+        "resolve_engine",
+        lambda self, e: "local",
+    )
+    monkeypatch.setattr(
+        timestamps.TimestampExtractor,
+        "extract_local",
+        lambda self, mp3: {"words": [{"word": "hi", "start": 0, "end": 0.1}]},
+    )
+    res = client.post("/api/run/timestamps/01")
+    assert res.status_code == 500
+    err = res.get_json()["error"]
+    assert "not valid JSON" in err
+    assert (cfg.animations_dir / "timing.json").read_text(encoding="utf-8") == "{not-json"
+
+
 def test_revise_mode_includes_current_narration(monkeypatch: pytest.MonkeyPatch) -> None:
     captured: dict = {}
 

tests/test_wizard.py

@@ -1,5 +1,6 @@
 """Tests for docgen.wizard file scanning and tree building."""
 
+import pytest
 
 from docgen.wizard import scan_md_files, build_file_tree
 
@@ -114,3 +115,77 @@ def test_generate_narration_rejects_escaped_source_path(tmp_path):
     )
     assert resp.status_code == 400
     assert "invalid path" in resp.get_json()["error"]
+
+
+def test_generate_narration_rejects_missing_source_path(tmp_path):
+    from docgen.config import Config
+    from docgen.wizard import create_app
+
+    repo = tmp_path / "proj"
+    repo.mkdir()
+    yaml_path = repo / "docgen.yaml"
+    yaml_path.write_text(
+        "repo_root: .\nsegments:\n  default: ['01']\n  all: ['01']\n",
+        encoding="utf-8",
+    )
+    cfg = Config.from_yaml(yaml_path)
+    client = create_app(cfg).test_client()
+    resp = client.post(
+        "/api/generate-narration",
+        json={
+            "segment_id": "01",
+            "segment_name": "01-intro",
+            "source_paths": ["missing.md"],
+            "guidance": "x",
+        },
+    )
+    assert resp.status_code == 400
+    assert "file not found" in resp.get_json()["error"]
+
+
+def test_generate_narration_requires_sources_in_generate_mode(tmp_path, monkeypatch):
+    from docgen.config import Config
+    from docgen.wizard import create_app
+
+    repo = tmp_path / "proj"
+    repo.mkdir()
+    yaml_path = repo / "docgen.yaml"
+    yaml_path.write_text(
+        "repo_root: .\nsegments:\n  default: ['01']\n  all: ['01']\n",
+        encoding="utf-8",
+    )
+    cfg = Config.from_yaml(yaml_path)
+    called = {"n": 0}
+
+    def boom(**_kwargs):
+        called["n"] += 1
+        return "should not run"
+
+    monkeypatch.setattr("docgen.wizard.generate_narration_via_llm", boom)
+    client = create_app(cfg).test_client()
+    resp = client.post(
+        "/api/generate-narration",
+        json={
+            "segment_id": "01",
+            "segment_name": "01-intro",
+            "mode": "generate",
+            "source_paths": [],
+        },
+    )
+    assert resp.status_code == 400
+    assert "source" in resp.get_json()["error"]
+    assert called["n"] == 0
+
+
+def test_generate_narration_via_llm_generate_requires_sources() -> None:
+    from docgen.wizard import generate_narration_via_llm
+
+    with pytest.raises(ValueError, match="source documentation"):
+        generate_narration_via_llm(
+            source_texts=[],
+            guidance="",
+            system_prompt="x",
+            model="gpt-4o",
+            segment_name="01",
+            mode="generate",
+        )