From 6583d6245450353fa388aa7f6febeaaed920f0f4 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 1 Aug 2026 11:27:26 +0000 Subject: [PATCH] fix(scene): fail-closed beat sync, required pacing, pipeline retime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address #56 semantic sync failures: 1. Label→wait_word matching is fail-closed (no fuzzy containment, no leftover LLM wait_word indices when the label is unspoken). 2. When timing.json has words, every story box must resolve wait_word or explicitly set pace: none — compile/generate fail otherwise. 3. generate-all / rebuild-after-audio retime-compile existing animations/specs/*.scene.yaml after timestamps (optional --regen-scene-specs for OpenAI). 4. scene-compile --all --retime for offline wait_word rewrite. Co-authored-by: John Menke --- AGENTS.md | 7 +- README.md | 8 +- src/docgen/cli.py | 121 +++++++++++++++---- src/docgen/pipeline.py | 90 ++++++++++++++- src/docgen/scene_retime.py | 105 +++++++++++++++++ src/docgen/scene_spec.py | 87 +++++++++----- src/docgen/scene_spec_generate.py | 16 ++- tests/test_pipeline.py | 63 +++++++++- tests/test_scene_retime.py | 185 ++++++++++++++++++++++++++++++ tests/test_scene_spec.py | 94 +++++++++++++++ 10 files changed, 716 insertions(+), 60 deletions(-) create mode 100644 src/docgen/scene_retime.py create mode 100644 tests/test_scene_retime.py diff --git a/AGENTS.md b/AGENTS.md index 9572cd5..c6316e4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -56,12 +56,13 @@ Commands registered on the **`docgen`** CLI include: - **`clean-bundle`** — remove regenerable outputs per policy. - **`concat`** — stitch segment videos. - **`pages`** — emit static HTML for demo assets. -- **`generate-all`** — orchestrated pipeline for a bundle. -- **`rebuild-after-audio`** — rerun steps that depend on fresh audio/timing. +- **`generate-all`** — orchestrated pipeline: TTS → timestamps → **scene retime** (compile existing `*.scene.yaml` against fresh timing) → images → Manim → compose → validate → concat → pages. Optional `--regen-scene-specs` for LLM scene YAML first. +- **`rebuild-after-audio`** — same as generate-all with TTS skipped (still retimes scenes after timestamps). ## Implications for changes here -- **Manim / `scenes.py` (marker blocks):** Fix generators under `src/docgen/**` (`manim_scene_support.py`, `scene_spec.py`, `scene_spec_generate.py`, `validate`, `yaml_generate`, tests). **Do not** patch generated classes inside a consumer's **`animations/scenes.py`** between **`BEGIN/END GENERATED SCENE`** markers; re-run **`scene-spec-generate`** / **`scene-compile`** and **`manim`** instead. +- **Manim / `scenes.py` (marker blocks):** Fix generators under `src/docgen/**` (`manim_scene_support.py`, `scene_spec.py`, `scene_spec_generate.py`, `validate`, `yaml_generate`, tests). **Do not** patch generated classes inside a consumer's **`animations/scenes.py`** between **`BEGIN/END GENERATED SCENE`** markers; re-run **`scene-spec-generate`** / **`scene-compile --retime`** and **`manim`** instead. Preferred consumer order: narration → TTS → timestamps → scene-spec/compile → Manim → compose. +- **Beat sync (fail-closed):** when `timing.json` has words, every story box label must match a spoken phrase (`wait_word`); unmatched labels and leftover LLM indices are rejected. Opt out with ``pace: none``. Fuzzy containment matching is not used. - **Subject-beat coverage:** implemented in `scene_spec.layout_density_violations` / `cluster_subject_beats`; enforced by **`scene-spec-generate`** and **`validate`** (`validation.subject_beat_coverage.enabled`, default true). Not a blind label count. - Prefer **stable CLI / library contracts** and **documented exit codes** so CI can depend on them. - **`narration_from_source`:** hints in config + **`docgen narration-generate`** — owner-supplied context paths, not opaque bulk edits to outputs. diff --git a/README.md b/README.md index 5b97d6e..6eaab4d 100644 --- a/README.md +++ b/README.md @@ -91,7 +91,7 @@ CI installs `ffmpeg` and `tesseract` via apt — see `.github/workflows/ci.yml`. ```bash cd your-project/docs/demos docgen wizard # optional: bootstrap narration from project docs -docgen generate-all # TTS → timestamps → Manim → compose → validate → concat +docgen generate-all # TTS → timestamps → scene retime → Manim → compose → validate docgen validate --pre-push ``` @@ -110,12 +110,12 @@ docgen validate --pre-push | `docgen lint [--segment 01]` | Narration lint only | | `docgen concat [--config full-demo]` | Concatenate full demo files | | `docgen pages [--force]` | Generate `index.html`, `pages.yml`, `.gitattributes`, `.gitignore` | -| `docgen generate-all [--skip-tts] [--skip-manim] [--retry-manim]` | Full pipeline | -| `docgen rebuild-after-audio` | Recompose + validate + concat (skips TTS) | +| `docgen generate-all [--skip-tts] [--skip-manim] [--retry-manim] [--regen-scene-specs]` | Full pipeline: TTS → timestamps → **scene retime** (existing specs) → Manim → compose → validate. `--regen-scene-specs` also runs OpenAI scene-spec-generate | +| `docgen rebuild-after-audio [--regen-scene-specs]` | Timestamps → scene retime → Manim → compose → validate (skips TTS) | | `docgen clean-bundle [-y] [--delete-config] [--keep-narration]` | Remove regenerable outputs under the bundle | | `docgen narration-generate --segment 01 [--extra-path REL] [--hint TEXT] [--dry-run] [--force]` | Generate narration `.md` from repo sources + owner hints (OpenAI); see `narration_from_source` in YAML | | `docgen yaml-generate [--merge-defaults] [--llm] [--dry-run] [--list-gaps]` | Merge defaults into `docgen.yaml`; optional OpenAI refresh of `tts.instructions` / `wizard.system_prompt` (rewrites the file — review in Git) | -| `docgen scene-compile SPEC.scene.yaml [--dry-run]` | Compile a declarative scene spec (YAML) into a `_TimedScene` class and inject it into `animations/scenes.py` — deterministic layout (rows of `_box`); applies auto-pagination + Whisper `wait_word` | +| `docgen scene-compile [SPEC.scene.yaml \| --all] [--retime] [--dry-run]` | Compile declarative scene YAML into `animations/scenes.py`. **`--all --retime`** re-derives `wait_word` from current `timing.json` with no OpenAI; unmatched labels fail closed (or set `pace: none`) | | `docgen scene-spec-generate [--segment 01 \| --all] [--compile] [--print-only] [--output PATH] [--hint …] [--model …]` | Call OpenAI to emit YAML only (same schema as `scene-compile`); rejects frame-budget overflow and **subject-beat coverage** failures (hold board on same topic; cover topic shifts; no invented labels — not a blind count); auto-paginate + word-alignment; optionally writes `animations/specs/.scene.yaml` and `--compile`s into `scenes.py` | ## Configuration diff --git a/src/docgen/cli.py b/src/docgen/cli.py index 4ebfeac..f812b64 100644 --- a/src/docgen/cli.py +++ b/src/docgen/cli.py @@ -428,47 +428,93 @@ def narration_generate( @main.command("scene-compile") @click.argument( "spec_path", + required=False, + default=None, type=click.Path(path_type=Path, exists=True, dir_okay=False), ) +@click.option( + "--all", + "all_specs", + is_flag=True, + help="Compile every animations/specs/*.scene.yaml (mutually exclusive with SPEC_PATH).", +) +@click.option( + "--retime", + is_flag=True, + help=( + "Re-derive wait_word indices from current timing.json (no OpenAI) and fail if " + "labels do not match spoken words. Implied by --all when timing exists; safe to " + "pass explicitly after `docgen timestamps`." + ), +) @click.option( "--dry-run", is_flag=True, help="Print generated Python only; do not write animations/scenes.py.", ) @click.pass_context -def scene_compile(ctx: click.Context, spec_path: Path, dry_run: bool) -> None: +def scene_compile( + ctx: click.Context, + spec_path: Path | None, + all_specs: bool, + retime: bool, + dry_run: bool, +) -> None: """Compile a declarative ``*.scene.yaml`` into ``animations/scenes.py``. Deterministic layout (rows of ``_box`` mobjects) — use for reliable diagrams or after an LLM emits **only** YAML. Schema: :mod:`docgen.scene_spec`. ``timing_key`` defaults from ``segment_names`` in docgen.yaml when omitted. + + After TTS/timestamps, prefer ``docgen scene-compile --all --retime`` (or + ``generate-all``, which retimes existing specs automatically) so beat sync + uses fresh ``timing.json`` without calling OpenAI. """ if ctx.obj.get("config") is None: raise click.ClickException("No docgen.yaml found (use --config PATH).") + if all_specs and spec_path is not None: + raise click.ClickException("Pass SPEC_PATH or --all, not both.") + if not all_specs and spec_path is None: + raise click.ClickException("Pass SPEC_PATH or --all.") + from docgen.manim_scene_support import SceneGenerationError - from docgen.scene_spec import load_scene_spec - from docgen.scene_spec_generate import inject_class_block_into_scenes_py, linted_class_block_from_spec + from docgen.scene_retime import list_scene_spec_paths, retime_compile_spec + from docgen.scene_spec import SceneSpecError cfg = ctx.obj["config"] - raw = load_scene_spec(spec_path) - try: - class_block, merged = linted_class_block_from_spec(cfg, dict(raw)) - except SceneGenerationError as exc: - raise click.ClickException(str(exc)) from exc + # --retime is the same compile path (label sync + pacing gate); the flag + # documents intent and is the recommended post-timestamps invocation. + _ = retime - if dry_run: - click.echo(class_block, nl=False) - return + paths = list_scene_spec_paths(cfg) if all_specs else [spec_path] + if not paths: + raise click.ClickException("No animations/specs/*.scene.yaml files found.") - sid = str(merged["segment_id"]).strip() - class_name = str(merged["class_name"]).strip() - scenes_path = inject_class_block_into_scenes_py( - cfg, seg_id=sid, class_name=class_name, class_block=class_block - ) - click.echo( - f"[scene-compile] wrote {class_name} to {scenes_path} " - f"(segment {sid} → timing_key {merged['timing_key']!r})" - ) + failures: list[str] = [] + for path in paths: + assert path is not None + try: + result = retime_compile_spec(cfg, path, dry_run=dry_run) + except (SceneGenerationError, SceneSpecError) as exc: + if all_specs: + click.echo(f"[scene-compile] FAIL {path.name}: {exc}", err=True) + failures.append(path.name) + continue + raise click.ClickException(str(exc)) from exc + if dry_run: + click.echo(result["class_block"], nl=False) + if all_specs: + click.echo(f"\n--- end {path.name} ---\n") + continue + click.echo( + f"[scene-compile] wrote {result['class_name']} to {result['scenes_path']} " + f"(segment {result['segment_id']} → timing_key {result['timing_key']!r}" + f"{', retime' if retime or all_specs else ''})" + ) + if failures: + raise click.ClickException( + f"scene-compile --all: {len(failures)} failed: " + ", ".join(failures) + ) @main.command("scene-spec-generate") @@ -1009,14 +1055,34 @@ def pages(ctx: click.Context, force: bool) -> None: is_flag=True, help="If compose hits FREEZE GUARD, clear Manim cache and retry Manim + compose once.", ) +@click.option( + "--regen-scene-specs", + is_flag=True, + help=( + "After timestamps, run OpenAI scene-spec-generate for every manim segment " + "(expensive). Default only retime-compiles existing animations/specs/*.scene.yaml." + ), +) +@click.option( + "--skip-scene-retime", + is_flag=True, + help="Skip the post-timestamps scene retime / scene-spec stage.", +) @click.pass_context def generate_all( ctx: click.Context, skip_tts: bool, skip_manim: bool, retry_manim: bool, + regen_scene_specs: bool, + skip_scene_retime: bool, ) -> None: - """Run full pipeline: TTS -> Manim -> compose -> validate -> concat -> pages.""" + """Run full pipeline: TTS → timestamps → scene retime → Manim → compose → validate. + + Order matters for beat sync: timestamps must land before scene compile so + ``wait_word`` indices match the current mp3. Existing declarative specs are + retime-compiled offline; pass ``--regen-scene-specs`` to call OpenAI first. + """ from docgen.pipeline import Pipeline cfg = ctx.obj["config"] @@ -1025,16 +1091,23 @@ def generate_all( skip_tts=skip_tts, skip_manim=skip_manim, retry_manim_on_freeze=retry_manim, + regen_scene_specs=regen_scene_specs, + skip_scene_retime=skip_scene_retime, ) @main.command("rebuild-after-audio") +@click.option( + "--regen-scene-specs", + is_flag=True, + help="Also regenerate scene specs via OpenAI before retime-compile.", +) @click.pass_context -def rebuild_after_audio(ctx: click.Context) -> None: - """Rebuild everything after new audio: Manim -> compose -> validate -> concat.""" +def rebuild_after_audio(ctx: click.Context, regen_scene_specs: bool) -> None: + """Rebuild after new audio: timestamps → scene retime → Manim → compose → validate.""" from docgen.pipeline import Pipeline cfg = ctx.obj["config"] pipeline = Pipeline(cfg) - pipeline.run(skip_tts=True) + pipeline.run(skip_tts=True, regen_scene_specs=regen_scene_specs) diff --git a/src/docgen/pipeline.py b/src/docgen/pipeline.py index 9fa4924..68833b8 100644 --- a/src/docgen/pipeline.py +++ b/src/docgen/pipeline.py @@ -1,8 +1,12 @@ -"""Pipeline orchestrator: tts -> manim -> compose -> validate -> concat -> pages. +"""Pipeline orchestrator: tts -> timestamps -> scene retime -> manim -> compose -> validate. The Manim stage renders only scenes referenced by ``visual_map`` for active ``segments.all`` entries (see :meth:`docgen.config.Config.pipeline_manim_scene_names`). Segments whose visuals are pre-recorded (``recordings/*.mp4``) do not run through Manim capture here. + +After timestamps, existing ``animations/specs/*.scene.yaml`` files are **retime-compiled** +against fresh ``timing.json`` (no OpenAI) so ``wait_word`` indices stay aligned. Optional +``regen_scene_specs`` runs LLM ``scene-spec-generate`` for manim segments before that compile. """ from __future__ import annotations @@ -23,6 +27,8 @@ def run( skip_tts: bool = False, skip_manim: bool = False, retry_manim_on_freeze: bool = False, + regen_scene_specs: bool = False, + skip_scene_retime: bool = False, ) -> None: if not skip_tts: print("\n=== Stage: TTS ===") @@ -33,6 +39,9 @@ def run( from docgen.timestamps import TimestampExtractor TimestampExtractor(self.config).extract_all() + if not skip_manim and not skip_scene_retime: + self._run_scene_stages(regen_scene_specs=regen_scene_specs) + if not skip_manim: from docgen.image_generate import generate_missing_images_for_bundle image_msgs = generate_missing_images_for_bundle(self.config) @@ -84,6 +93,85 @@ def run( print("\n=== Pipeline complete ===") + def _manim_segment_ids(self) -> list[str]: + ids: list[str] = [] + for seg_id in self.config.segments_all: + vm = self.config.visual_map.get(seg_id) + if isinstance(vm, dict) and str(vm.get("type", "")).strip().lower() == "manim": + ids.append(str(seg_id)) + elif isinstance(vm, dict) and not str(vm.get("type", "")).strip(): + # Untyped but has a scene class — treat as manim for regen. + if vm.get("scene") or vm.get("class"): + ids.append(str(seg_id)) + return ids + + def _run_scene_stages(self, *, regen_scene_specs: bool) -> None: + if regen_scene_specs: + manim_ids = self._manim_segment_ids() + if not manim_ids: + print("\n=== Stage: Scene-spec generate (skipped — no manim segments) ===") + return + print("\n=== Stage: Scene-spec generate (LLM) ===") + from docgen.manim_scene_support import SceneGenerationError + from docgen.scene_spec_generate import ( + generate_scene_spec, + inject_class_block_into_scenes_py, + linted_class_block_from_spec, + ) + + failures: list[str] = [] + for sid in manim_ids: + print(f"[scene-spec-generate] segment {sid}") + try: + res = generate_scene_spec( + self.config, sid, extra_paths=[], extra_hints=[] + ) + specs_dir = self.config.animations_dir / "specs" + specs_dir.mkdir(parents=True, exist_ok=True) + wpath = specs_dir / f"{res.seg_name}.scene.yaml" + wpath.write_text(res.yaml_text, encoding="utf-8") + class_block, merged = linted_class_block_from_spec( + self.config, res.spec, timing_key=res.seg_name + ) + inject_class_block_into_scenes_py( + self.config, + seg_id=merged["segment_id"], + class_name=merged["class_name"], + class_block=class_block, + ) + print(f"[scene-spec-generate] wrote {wpath.name} → {merged['class_name']}") + except (SceneGenerationError, OSError, ValueError) as exc: + print(f"[scene-spec-generate] FAIL {sid}: {exc}") + failures.append(sid) + if failures: + raise RuntimeError( + "scene-spec-generate failed for: " + ", ".join(failures) + ) + return + + # Default: offline retime of existing declarative specs against fresh timing. + from docgen.scene_retime import list_scene_spec_paths, retime_compile_all + + paths = list_scene_spec_paths(self.config) + if not paths: + print("\n=== Stage: Scene retime (skipped — no animations/specs/*.scene.yaml) ===") + return + + print("\n=== Stage: Scene retime (compile specs against timing.json) ===") + results, errors = retime_compile_all(self.config) + for res in results: + print( + f"[scene-retime] {res['path'].name} → {res['class_name']} " + f"(timing_key {res.get('timing_key')!r})" + ) + for err in errors: + print(f"[scene-retime] FAIL {err}") + if errors: + raise RuntimeError( + f"scene retime failed for {len(errors)} spec(s); " + "fix unmatched labels (spoken phrases) or set pace: none, then re-run" + ) + @staticmethod def _should_retry_manim( exc: Exception, skip_manim: bool, retry_manim_on_freeze: bool diff --git a/src/docgen/scene_retime.py b/src/docgen/scene_retime.py new file mode 100644 index 0000000..70e6536 --- /dev/null +++ b/src/docgen/scene_retime.py @@ -0,0 +1,105 @@ +"""Offline retime: recompile ``*.scene.yaml`` against current ``timing.json`` (no OpenAI).""" + +from __future__ import annotations + +from pathlib import Path +from typing import TYPE_CHECKING, Any + +if TYPE_CHECKING: + from docgen.config import Config + + +def list_scene_spec_paths(cfg: "Config", *, segment_id: str | None = None) -> list[Path]: + """Return ``animations/specs/*.scene.yaml`` paths (optionally one segment).""" + specs_dir = cfg.animations_dir / "specs" + if not specs_dir.is_dir(): + return [] + if segment_id is not None: + sid = str(segment_id).strip() + if sid.isdigit(): + sid = sid.zfill(2) + stem = cfg.resolve_segment_name(sid) + candidates = [ + specs_dir / f"{stem}.scene.yaml", + specs_dir / f"{sid}.scene.yaml", + ] + return [p for p in candidates if p.is_file()] + return sorted(specs_dir.glob("*.scene.yaml")) + + +def retime_compile_spec( + cfg: "Config", + spec_path: Path, + *, + dry_run: bool = False, +) -> dict[str, Any]: + """Load one spec, re-derive ``wait_word`` from timing, compile into ``scenes.py``. + + Raises ``SceneGenerationError`` / ``SceneSpecError`` on schema or pacing failure. + """ + from docgen.scene_spec import load_scene_spec + from docgen.scene_spec_generate import ( + inject_class_block_into_scenes_py, + linted_class_block_from_spec, + ) + + raw = load_scene_spec(spec_path) + class_block, merged = linted_class_block_from_spec(cfg, dict(raw)) + sid = str(merged["segment_id"]).strip() + class_name = str(merged["class_name"]).strip() + if dry_run: + return { + "path": spec_path, + "segment_id": sid, + "class_name": class_name, + "timing_key": merged.get("timing_key"), + "class_block": class_block, + "wrote": False, + } + scenes_path = inject_class_block_into_scenes_py( + cfg, seg_id=sid, class_name=class_name, class_block=class_block + ) + return { + "path": spec_path, + "segment_id": sid, + "class_name": class_name, + "timing_key": merged.get("timing_key"), + "scenes_path": scenes_path, + "wrote": True, + } + + +def retime_compile_all( + cfg: "Config", + *, + dry_run: bool = False, + segment_ids: list[str] | None = None, +) -> tuple[list[dict[str, Any]], list[str]]: + """Retime-compile every (or selected) scene spec. Returns (results, error messages).""" + from docgen.manim_scene_support import SceneGenerationError + from docgen.scene_spec import SceneSpecError + + paths: list[Path] = [] + if segment_ids: + for sid in segment_ids: + paths.extend(list_scene_spec_paths(cfg, segment_id=str(sid))) + # Dedupe while preserving order + seen: set[Path] = set() + uniq: list[Path] = [] + for p in paths: + rp = p.resolve() + if rp not in seen: + seen.add(rp) + uniq.append(p) + paths = uniq + else: + paths = list_scene_spec_paths(cfg) + + results: list[dict[str, Any]] = [] + errors: list[str] = [] + for path in paths: + try: + results.append(retime_compile_spec(cfg, path, dry_run=dry_run)) + except (SceneGenerationError, SceneSpecError, OSError, ValueError) as exc: + errors.append(f"{path.name}: {exc}") + return results, errors diff --git a/src/docgen/scene_spec.py b/src/docgen/scene_spec.py index 6b1ccae..837d1dd 100644 --- a/src/docgen/scene_spec.py +++ b/src/docgen/scene_spec.py @@ -373,16 +373,6 @@ def _tokens_match(label_token: str, word_token: str) -> bool: return _stem(label_token) == _stem(word_token) -def _soft_token_match(a: str, b: str) -> bool: - """Cheap fuzzy match for long tokens (prefix / containment after length check).""" - if abs(len(a) - len(b)) > 3: - return False - if len(a) < 5 or len(b) < 5: - return False - shorter, longer = (a, b) if len(a) <= len(b) else (b, a) - return longer.startswith(shorter) or shorter in longer - - def segment_index_for_whisper_time( segments: list[dict[str, Any]], wall_time: float ) -> int: @@ -447,10 +437,10 @@ def sync_row_labels_to_whisper_words( ) -> dict[str, Any]: """Set ``wait_word`` on each **box** from its ``label`` → first spoken match (in order). - Uses the same label/word token rules as before. Each matched box waits at **word** - ``start``, not segment boundary. Row-level ``wait_word`` / ``wait_segment`` are cleared - when ``overwrite=True`` (compile path); legacy row ``wait_word`` is seeded onto the first - box only if that box has no label match. + Matching is **fail-closed**: exact/stem token equality only (plus hyphen splits). + No fuzzy containment and no leftover LLM ``wait_word`` when the label is absent + from the transcript. Each matched box waits at word ``start``. Row-level + ``wait_word`` / ``wait_segment`` are cleared when ``overwrite=True``. """ if not isinstance(words, list) or not words: return spec @@ -490,14 +480,6 @@ def _find_label(label: str, from_idx: int) -> tuple[int, int] | None: if ok: return (i + m - 1, word_stream[i][2]) i += 1 - # Soft fallback: accept a single long label token that equals a spoken - # word after stripping a short edit distance (hyphen/TTS orthography). - if m == 1 and len(tokens[0]) >= 5: - target = tokens[0] - for j in range(from_idx, n): - w = word_stream[j][0] - if _tokens_match(target, w) or _soft_token_match(target, w): - return (j, word_stream[j][2]) return None def _process_rows(rows: list[Any]) -> None: @@ -520,11 +502,11 @@ def _process_rows(rows: list[Any]) -> None: cursor = found[0] + 1 continue - legacy_rw = row.pop("wait_word", None) if overwrite else None if overwrite: + row.pop("wait_word", None) row.pop("wait_segment", None) - for bi, box in enumerate(boxes): + for box in boxes: if not isinstance(box, dict): continue box.pop("wait_at", None) @@ -545,9 +527,9 @@ def _process_rows(rows: list[Any]) -> None: box["wait_word"] = int(first_word_i) box.pop("wait_segment", None) cursor = last_stream_i + 1 - elif overwrite and bi == 0 and legacy_rw is not None: - box["wait_word"] = int(legacy_rw) elif overwrite: + # Fail-closed: never keep a leftover LLM / legacy index for an + # unmatched spoken label — callers must reject or set pace: none. box.pop("wait_word", None) box.pop("wait_segment", None) @@ -567,6 +549,49 @@ def _process_rows(rows: list[Any]) -> None: return new_spec +def _pace_none(obj: dict[str, Any]) -> bool: + return str(obj.get("pace", "")).strip().lower() == "none" + + +def pacing_violations(spec: dict[str, Any], *, words_present: bool) -> list[str]: + """Return issues when timing words exist but story boxes lack ``wait_word``. + + Unpaced cascading ``timed_play`` finishes the board early and freezes through + the rest of the narration. Opt out per box/row with ``pace: none``. + Unlabeled image elements are exempt (no spoken anchor). + """ + if not words_present: + return [] + issues: list[str] = [] + pages = _spec_pages_rows(spec) + for pi, rows in enumerate(pages): + for ri, row in enumerate(rows): + if not isinstance(row, dict): + continue + row_opt_out = _pace_none(row) + boxes = row.get("boxes") + if not isinstance(boxes, list): + continue + prefix = f"pages[{pi}].rows[{ri}]" if spec.get("pages") is not None else f"rows[{ri}]" + for bi, box in enumerate(boxes): + if not isinstance(box, dict): + continue + if row_opt_out or _pace_none(box): + continue + if _is_image_element(box) and not str(box.get("label", "")).strip(): + continue + label = str(box.get("label", "")).strip() + if not label: + continue + if box.get("wait_word") is None: + issues.append( + f"{prefix}.boxes[{bi}]: label {label!r} has no wait_word match in " + "timing.json words — use a spoken phrase from the narration, or set " + "pace: none to opt out of beat sync" + ) + return issues + + def upgrade_wait_segments_to_wait_words( spec: dict[str, Any], words: list[dict[str, Any]], @@ -1007,6 +1032,14 @@ def _validate_image_element(box: dict[str, Any], *, bp: str) -> None: raise SceneSpecError(f"{bp}: label must be a string if set (used as timing anchor only)") +def _validate_pace_field(obj: dict[str, Any], *, path: str) -> None: + if "pace" not in obj or obj.get("pace") is None: + return + val = str(obj.get("pace")).strip().lower() + if val != "none": + raise SceneSpecError(f"{path}: pace must be 'none' if set (opt out of beat sync)") + + def _validate_row_list(rows: list[Any], *, path_label: str, prefix: str) -> None: for i, row in enumerate(rows): rp = f"{path_label}: {prefix}[{i}]" @@ -1022,6 +1055,7 @@ def _validate_row_list(rows: list[Any], *, path_label: str, prefix: str) -> None rt = row["run_time"] if not isinstance(rt, (int, float)) or rt <= 0: raise SceneSpecError(f"{rp}: run_time must be a positive number") + _validate_pace_field(row, path=rp) ws = row.get("wait_segment") if ws is not None and (not isinstance(ws, int) or ws < 0): raise SceneSpecError(f"{rp}: wait_segment must be a non-negative int or null") @@ -1043,6 +1077,7 @@ def _validate_row_list(rows: list[Any], *, path_label: str, prefix: str) -> None bp = f"{rp}: boxes[{j}]" if not isinstance(box, dict): raise SceneSpecError(f"{bp}: box must be a mapping") + _validate_pace_field(box, path=bp) if box.get("wait_segment") is not None: raise SceneSpecError( f"{bp}: wait_segment on a box is not supported — use ``wait_word`` on the box, " diff --git a/src/docgen/scene_spec_generate.py b/src/docgen/scene_spec_generate.py index bd21138..3e51018 100644 --- a/src/docgen/scene_spec_generate.py +++ b/src/docgen/scene_spec_generate.py @@ -40,6 +40,7 @@ layout_stack_budget, narration_sentences, spec_rows_reference_whisper_waits, + pacing_violations, sync_row_labels_to_whisper_words, upgrade_wait_segments_to_wait_words, validate_scene_spec, @@ -91,9 +92,12 @@ Optional per-box (**Whisper ``words`` only**); omit if unsure — compile fills from each box ``label`` → first transcript match: - wait_word: non-negative int — index into ``timing.json`` → ``words``; that box waits until that token's **start**, then fades in (**one box at a time** within each row). +- pace: optional ``none`` — opt out of beat sync for that box (rare; decorative only). When timing + words exist, every other labeled box **must** match a spoken phrase or compile fails. Optional per-row (legacy; first box only — prefer per-box above): - wait_word: non-negative int — if set, and boxes omit ``wait_word``, only the **first** box in the row uses this index. +- pace: optional ``none`` — opt out for every box in the row. Optional top-level: - layout: optional first_row_title_buff, row_gap, column_gap (positive numbers); @@ -326,7 +330,8 @@ def linted_class_block_from_spec( if words: # LLM-authored wait_word values are often wrong (duplicates / guesses). Compile # always re-derives indices from each box label + transcript order so multi-box - # rows reveal one box at a time. + # rows reveal one box at a time. Fail-closed: unmatched labels clear wait_word + # and are rejected below (no leftover LLM indices, no fuzzy false positives). merged = sync_row_labels_to_whisper_words(merged, words, overwrite=True) if spec_rows_reference_whisper_waits(merged) and not words: @@ -335,6 +340,15 @@ def linted_class_block_from_spec( "before compiling scenes that use wait_word or wait_segment." ) + pace_issues = pacing_violations(merged, words_present=bool(words)) + if pace_issues: + shown = "\n ".join(pace_issues[:12]) + more = f"\n (+{len(pace_issues) - 12} more)" if len(pace_issues) > 12 else "" + raise SceneGenerationError( + f"scene pacing failed for timing_key {tk!r} — every story box needs a " + f"spoken label matched in timing.json words (or pace: none):\n {shown}{more}" + ) + try: class_block = compile_scene_class(merged) except SceneSpecError as exc: diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index 978910c..d368287 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -51,11 +51,12 @@ def generate_all(self, force=False) -> None: calls.append(f"pages:{force}") import docgen.concat as concat_module + import docgen.compose as compose_module + import docgen.image_generate as image_module import docgen.manim_runner as manim_module import docgen.pages as pages_module import docgen.timestamps as timestamps_module import docgen.validate as validate_module - import docgen.compose as compose_module monkeypatch.setattr(timestamps_module, "TimestampExtractor", FakeTimestampExtractor) monkeypatch.setattr(manim_module, "ManimRunner", FakeManimRunner) @@ -63,6 +64,11 @@ def generate_all(self, force=False) -> None: monkeypatch.setattr(concat_module, "ConcatBuilder", FakeConcatBuilder) monkeypatch.setattr(pages_module, "PagesGenerator", FakePagesGenerator) monkeypatch.setattr(compose_module, "Composer", composer_cls) + monkeypatch.setattr( + image_module, + "generate_missing_images_for_bundle", + lambda _cfg: calls.append("images") or [], + ) def test_retry_manim_after_freeze_guard(tmp_path, monkeypatch) -> None: @@ -91,6 +97,7 @@ def compose_segments(self, _segments) -> int: cfg = SimpleNamespace( animations_dir=animations_dir, segments_all=["01"], + visual_map={"01": {"type": "manim", "scene": "Scene01"}}, pipeline_manim_scene_names=lambda: ["Scene01"], ) @@ -101,6 +108,59 @@ def compose_segments(self, _segments) -> int: assert not media_dir.exists(), "Retry path should clear Manim cache directory" +def test_pipeline_retimes_existing_specs_after_timestamps(tmp_path, monkeypatch) -> None: + calls: list[str] = [] + + class OkComposer: + def __init__(self, _config) -> None: + pass + + def compose_segments(self, _segments) -> int: + calls.append("compose") + return 1 + + _patch_pipeline_stages(monkeypatch, OkComposer, calls) + + import docgen.scene_retime as retime_module + + def fake_retime_all(cfg, **_kwargs): + calls.append("retime") + return ( + [ + { + "path": cfg.animations_dir / "specs" / "01.scene.yaml", + "class_name": "Scene01", + "timing_key": "01", + "wrote": True, + } + ], + [], + ) + + monkeypatch.setattr(retime_module, "retime_compile_all", fake_retime_all) + monkeypatch.setattr( + retime_module, + "list_scene_spec_paths", + lambda cfg, segment_id=None: [cfg.animations_dir / "specs" / "01.scene.yaml"], + ) + + animations_dir = tmp_path / "animations" + (animations_dir / "specs").mkdir(parents=True) + + cfg = SimpleNamespace( + animations_dir=animations_dir, + segments_all=["01"], + visual_map={"01": {"type": "manim", "scene": "Scene01"}}, + pipeline_manim_scene_names=lambda: ["Scene01"], + ) + + Pipeline(cfg).run(skip_tts=True) + + assert calls.index("timestamps") < calls.index("retime") + assert calls.index("retime") < calls.index("manim") + assert "compose" in calls + + def test_no_retry_when_flag_disabled(tmp_path, monkeypatch) -> None: calls: list[str] = [] @@ -121,6 +181,7 @@ def compose_segments(self, _segments) -> int: cfg = SimpleNamespace( animations_dir=animations_dir, segments_all=["01"], + visual_map={"01": {"type": "manim", "scene": "Scene01"}}, pipeline_manim_scene_names=lambda: ["Scene01"], ) diff --git a/tests/test_scene_retime.py b/tests/test_scene_retime.py new file mode 100644 index 0000000..2f91760 --- /dev/null +++ b/tests/test_scene_retime.py @@ -0,0 +1,185 @@ +"""Tests for offline scene retime + fail-closed pacing in linted_class_block.""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest +import yaml + +from docgen.config import Config +from docgen.manim_scene_support import BOOTSTRAP_HEADER, SceneGenerationError +from docgen.scene_retime import list_scene_spec_paths, retime_compile_all, retime_compile_spec +from docgen.scene_spec_generate import linted_class_block_from_spec + + +def _cfg(tmp_path: Path) -> Config: + raw = { + "dirs": { + "narration": "narration", + "animations": "animations", + "audio": "audio", + "recordings": "recordings", + }, + "segments": {"all": ["01"], "default": ["01"]}, + "segment_names": {"01": "01-demo"}, + "visual_map": {"01": {"type": "manim", "scene": "DemoScene", "source": "x.mp4"}}, + } + p = tmp_path / "docgen.yaml" + p.write_text(yaml.dump(raw), encoding="utf-8") + (tmp_path / "narration").mkdir() + (tmp_path / "narration" / "01-demo.md").write_text( + "Hello world from the demo.\n", encoding="utf-8" + ) + anim = tmp_path / "animations" + anim.mkdir() + (anim / "scenes.py").write_text(BOOTSTRAP_HEADER, encoding="utf-8") + return Config.from_yaml(p) + + +def _write_spec(tmp_path: Path, *, label: str = "Hello") -> Path: + specs = tmp_path / "animations" / "specs" + specs.mkdir(parents=True, exist_ok=True) + path = specs / "01-demo.scene.yaml" + path.write_text( + yaml.dump( + { + "segment_id": "01", + "class_name": "DemoScene", + "title": {"text": "Demo", "font_size": 36, "color": "C_WHITE"}, + "rows": [ + { + "run_time": 1.0, + "boxes": [ + { + "label": label, + "color": "C_GREEN", + "width": 3.0, + "height": 0.9, + "font_size": 18, + } + ], + } + ], + } + ), + encoding="utf-8", + ) + return path + + +def _write_timing(tmp_path: Path, words: list[dict]) -> None: + path = tmp_path / "animations" / "timing.json" + path.write_text( + json.dumps( + { + "01-demo": { + "text": " ".join(w["word"] for w in words), + "segments": [{"start": 0.0, "end": 2.0, "text": "hi"}], + "words": words, + } + } + ), + encoding="utf-8", + ) + + +def test_linted_class_block_fails_closed_on_unmatched_label(tmp_path: Path) -> None: + cfg = _cfg(tmp_path) + _write_timing( + tmp_path, + [ + {"word": "hello", "start": 0.0, "end": 0.3}, + {"word": "world", "start": 0.4, "end": 0.7}, + ], + ) + spec = { + "segment_id": "01", + "class_name": "DemoScene", + "title": {"text": "Demo", "font_size": 36, "color": "C_WHITE"}, + "rows": [ + { + "run_time": 1.0, + "wait_word": 99, + "boxes": [ + { + "label": "Originator", + "color": "C_GREEN", + "width": 3.0, + "height": 0.9, + "font_size": 18, + } + ], + } + ], + } + with pytest.raises(SceneGenerationError, match="pacing failed|Originator"): + linted_class_block_from_spec(cfg, spec, timing_key="01-demo") + + +def test_linted_class_block_succeeds_when_label_spoken(tmp_path: Path) -> None: + cfg = _cfg(tmp_path) + _write_timing( + tmp_path, + [ + {"word": "Hello", "start": 0.0, "end": 0.3}, + {"word": "world", "start": 0.4, "end": 0.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, + } + ], + } + ], + } + block, merged = linted_class_block_from_spec(cfg, spec, timing_key="01-demo") + assert "wait_until_word(timing_words, 0)" in block + assert merged["rows"][0]["boxes"][0]["wait_word"] == 0 + + +def test_retime_compile_spec_rewrites_scenes_py(tmp_path: Path) -> None: + cfg = _cfg(tmp_path) + path = _write_spec(tmp_path, label="Hello") + _write_timing( + tmp_path, + [ + {"word": "noise", "start": 0.0, "end": 0.2}, + {"word": "Hello", "start": 1.0, "end": 1.3}, + ], + ) + result = retime_compile_spec(cfg, path) + assert result["wrote"] is True + assert result["class_name"] == "DemoScene" + text = (tmp_path / "animations" / "scenes.py").read_text(encoding="utf-8") + assert "wait_until_word(timing_words, 1)" in text + + +def test_retime_compile_all_reports_failures(tmp_path: Path) -> None: + cfg = _cfg(tmp_path) + _write_spec(tmp_path, label="MissingLabel") + _write_timing(tmp_path, [{"word": "hello", "start": 0.0, "end": 0.2}]) + results, errors = retime_compile_all(cfg) + assert results == [] + assert errors and "MissingLabel" in errors[0] + + +def test_list_scene_spec_paths(tmp_path: Path) -> None: + cfg = _cfg(tmp_path) + assert list_scene_spec_paths(cfg) == [] + path = _write_spec(tmp_path) + assert list_scene_spec_paths(cfg) == [path] + assert list_scene_spec_paths(cfg, segment_id="01") == [path] diff --git a/tests/test_scene_spec.py b/tests/test_scene_spec.py index 349f765..09734f4 100644 --- a/tests/test_scene_spec.py +++ b/tests/test_scene_spec.py @@ -20,6 +20,7 @@ load_scene_spec, narration_sentence_count, narration_sentences, + pacing_violations, segment_index_for_whisper_time, sync_row_labels_to_whisper_words, validate_scene_spec, @@ -817,6 +818,99 @@ def test_sync_row_labels_overwrite_true_clears_unmatched_wait_word() -> None: assert out["rows"][0]["boxes"][0].get("wait_word") is None +def test_sync_row_labels_never_keeps_legacy_row_wait_word_for_unmatched() -> None: + """Issue #56: leftover LLM wait_word must not bind Originator → unrelated token.""" + spec = { + "segment_id": "1", + "class_name": "X", + "title": {"text": "T", "font_size": 36, "color": "C_WHITE"}, + "rows": [ + { + "run_time": 1.0, + "wait_word": 146, + "boxes": [ + { + "label": "Originator", + "color": "C_GREEN", + "width": 3.0, + "height": 1.0, + "font_size": 18, + } + ], + } + ], + } + words = [ + {"word": "the", "start": 0.0, "end": 0.2}, + {"word": "operator", "start": 64.0, "end": 64.4}, + {"word": "path", "start": 65.0, "end": 65.3}, + ] + out = sync_row_labels_to_whisper_words(spec, words, overwrite=True) + assert out["rows"][0].get("wait_word") is None + assert out["rows"][0]["boxes"][0].get("wait_word") is None + # Soft/fuzzy containment must not map Originator → operator either. + assert pacing_violations(out, words_present=True) + + +def test_pacing_violations_allow_pace_none_opt_out() -> None: + spec = { + "segment_id": "1", + "class_name": "X", + "title": {"text": "T", "font_size": 36, "color": "C_WHITE"}, + "rows": [ + { + "run_time": 1.0, + "boxes": [ + { + "label": "Spoken", + "color": "C_GREEN", + "width": 3.0, + "height": 1.0, + "font_size": 18, + "wait_word": 0, + }, + { + "label": "Decor", + "color": "C_BLUE", + "width": 3.0, + "height": 1.0, + "font_size": 18, + "pace": "none", + }, + ], + } + ], + } + assert pacing_violations(spec, words_present=True) == [] + assert pacing_violations(spec, words_present=False) == [] + + +def test_validate_pace_none_rejects_unknown_values() -> None: + with pytest.raises(SceneSpecError, match="pace"): + validate_scene_spec( + { + "segment_id": "1", + "class_name": "X", + "title": {"text": "T", "font_size": 36, "color": "C_WHITE"}, + "rows": [ + { + "run_time": 1.0, + "boxes": [ + { + "label": "A", + "color": "C_GREEN", + "width": 2.0, + "height": 1.0, + "font_size": 18, + "pace": "auto", + } + ], + } + ], + } + ) + + def test_sync_row_labels_hyphenated_label_matches_spoken_parts() -> None: """``yaml-generate`` aligns to spoken ``yaml`` + ``generate``.""" spec = {