diff --git a/AGENTS.md b/AGENTS.md index 680095d..d4321b3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -73,7 +73,7 @@ Commands registered on the **`docgen`** CLI include: - **`image-generate`** — render scene-spec **image elements** (`image:` + `prompt:` boxes) via OpenAI Images or xAI Imagine into the bundle (also runs for missing assets inside `generate-all`). - **`manim`** — render Manim scenes declared in config. - **`compose`** — mux narration audio with visual sources via ffmpeg. -- **`validate`** / **`validate --pre-push`** — drift, narration lint, Manim hints, **`timing_sync`**, **`story_end`** (last paced reveal vs audio end; hard fail), **`scene_assets`** (pre-render: stuck-board cadence, frame-budget overlaps, `MANIM_FONT` consistency, stale helpers / stale compiled class — hard fail; also a `generate-all` gate before Manim), **`av_sync`** (soft; prefers scene-spec labels as OCR anchors), **`subject_beat_coverage`** (declarative specs vs narration topic beats; hard fail when enabled), and related checks. +- **`validate`** / **`validate --pre-push`** — drift, narration lint, Manim hints, **`timing_sync`**, **`story_end`** (last paced reveal vs audio end; hard fail), **`scene_assets`** (pre-render: stuck-board cadence, frame-budget overlaps, `MANIM_FONT` consistency, stale helpers / stale compiled class — hard fail; also a `generate-all` gate before Manim), **`av_sync`** (hard fail on `--pre-push` / `generate-all`; prefers scene-spec labels as OCR anchors), **`subject_beat_coverage`** (declarative specs vs narration topic beats; hard fail when enabled), and related visual-sync checks (`ocr_scan`, `layout`, `freeze_ratio` — hard fail on `--pre-push` / `generate-all`). - **`lint`** — narration lint helper. - **`narration-generate`** — LLM-assisted narration from hints and repo context; optional **`--revise --revision-notes`** for in-place edits (same contract as the wizard Revise button). - **`scene-spec-generate`** — LLM emits declarative **`*.scene.yaml`**; enforces frame budget + **subject-beat coverage** (dwell OK; cover topic shifts; reject invented labels). diff --git a/README.md b/README.md index 36329f3..4b39eaf 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,7 @@ If you still need the legacy behaviour, pin a pre-removal commit Manim scene lint, **timing_sync** (stale `timing.json` vs regenerated mp3 — hard fail), **story_end** (paced visual story finishes long before narration — hard fail), and **av_sync** (OCR check that scene-spec label anchors appear on - screen near their spoken time — soft warning). + screen near their spoken time — hard fail on `--pre-push` / `generate-all`). - **GitHub Pages** — auto-generate `index.html`, deploy workflow, LFS rules, `.gitignore`. - **Wizard** — local web GUI to bootstrap narration scripts from existing project @@ -310,7 +310,7 @@ validation: enabled: true max_early_sec: 40.0 # idle after last paced box max_early_ratio: 0.45 # and idle / audio_end (both must exceed to fail) - av_sync: # OCR anchor check (soft warning in --pre-push) + av_sync: # OCR anchor check (hard fail in --pre-push / generate-all) enabled: true tolerance_sec: 3.0 prefer_scene_spec_labels: true # OCR anchors from paced box labels when specs exist diff --git a/src/docgen/validate.py b/src/docgen/validate.py index a14ce94..1715310 100644 --- a/src/docgen/validate.py +++ b/src/docgen/validate.py @@ -318,7 +318,10 @@ def run_pre_push(self) -> None: Missing recordings are reported as warnings, not failures — a project that hasn't generated videos yet should still be pushable. Quality - checks on *existing* recordings and narration lint are hard failures. + checks on *existing* recordings — including visual-sync (``av_sync``, + ``subject_beat_coverage``, ``ocr_scan``, ``layout``, ``freeze_ratio``) + — and narration lint are hard failures so ``generate-all`` cannot + print ``Pipeline complete`` after a desynced mux. """ reports = self.run_all() hard_fail = False @@ -326,17 +329,10 @@ def run_pre_push(self) -> None: if isinstance(r, dict): for c in r.get("checks", []): if not c.get("passed", True): + # Only "not generated yet" stays soft. Visual-sync FAILs + # used to warn here, which let generate-all finish. soft_checks = { "recording_exists", - "ocr_scan", - "freeze_ratio", - "layout", - # OCR keyword anchoring is heuristic; warn, don't block. - "av_sync", - # Subject-beat coverage is enforced hard at scene-spec-generate; - # on pre-push warn so shipping committed recordings is not blocked - # by a new heuristic gate mid-regeneration. - "subject_beat_coverage", } if c.get("name") in soft_checks: print(f"WARN [{r.get('segment')}] {c.get('name')}: {c.get('details')}") @@ -841,7 +837,7 @@ def _check_story_end(self, seg_id: str) -> CheckResult: Muxed recordings can still match mp3 length (compose freezes the last frame) while the diagram finished early. Uses scene-spec label→``wait_word`` starts - vs audio/transcript end. Hard fail in ``--pre-push`` (not soft like ``av_sync``). + vs audio/transcript end. Hard fail in ``--pre-push`` (same as ``av_sync``). """ se_cfg = self.config.story_end_config if not se_cfg.get("enabled", True): diff --git a/tests/conftest.py b/tests/conftest.py index 69f4ef5..ed32015 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -46,7 +46,7 @@ def clear_ai_env(monkeypatch: pytest.MonkeyPatch) -> None: "tests/test_validate.py::TestComposeGuard::test_compose_rejects_short_video", "tests/test_validate.py::TestComposeGuard::test_compose_allows_matching_durations", "tests/test_validate.py::TestComposeGuard::test_compose_nonstrict_warns", - "tests/test_validate.py::TestValidateSegmentIntegration::test_static_video_does_not_fail_pre_push", + "tests/test_validate.py::TestValidateSegmentIntegration::test_static_video_fails_pre_push", } ) diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index 17336a9..329b503 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -344,3 +344,62 @@ def compose_segments(self, _segments) -> int: assert "timestamps" not in calls assert "compose" not in calls + + +def test_pipeline_av_sync_fail_stops_complete(tmp_path, monkeypatch, capsys) -> None: + """One av_sync passed=False must stop generate-all before Pipeline complete.""" + from docgen.validate import Validator as RealValidator + + 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.validate as validate_module + + # Use real run_pre_push so soft_checks policy is under test, not FakeValidator. + monkeypatch.setattr(validate_module, "Validator", RealValidator) + + def fake_run_all(self, max_drift_override=None): + return [ + { + "segment": "01", + "checks": [ + { + "name": "av_sync", + "passed": False, + "details": ["desynced board"], + } + ], + } + ] + + monkeypatch.setattr(RealValidator, "run_all", fake_run_all) + + animations_dir = tmp_path / "animations" + animations_dir.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"], + ) + + with pytest.raises(SystemExit) as ei: + Pipeline(cfg).run(skip_tts=True, skip_manim=True, skip_scene_retime=True) + assert ei.value.code == 1 + + out = capsys.readouterr().out + assert "Pipeline complete" not in out + assert "FAIL" in out and "av_sync" in out + assert "WARN" not in out + assert "compose" in calls + assert "concat" not in calls + assert not any(str(c).startswith("pages:") for c in calls) diff --git a/tests/test_validate.py b/tests/test_validate.py index 893a125..e2fb80e 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -299,7 +299,7 @@ def test_compose_nonstrict_warns(self, config, cfg_dir, capsys): class TestValidateSegmentIntegration: def test_frozen_video_is_soft_warning(self, config, cfg_dir): - """Static video flags freeze_ratio but pre-push treats it as a warning.""" + """Static video flags freeze_ratio (pre-push treats that FAIL as hard).""" _make_video(cfg_dir / "recordings" / "01-test.mp4", 10, frames_fn=_static_gray) v = Validator(config) report = v.validate_segment("01") @@ -334,8 +334,8 @@ def test_half_black_fails_pre_push(self, config, cfg_dir): with pytest.raises(SystemExit): v.run_pre_push() - def test_static_video_does_not_fail_pre_push(self, config, cfg_dir): - """freeze_ratio is a soft check — static (non-black) video only warns.""" + def test_static_video_fails_pre_push(self, config, cfg_dir, capsys): + """freeze_ratio is a hard visual-sync check — static video fails pre-push.""" vid = cfg_dir / "recordings" / "01-test.mp4" vid_raw = cfg_dir / "recordings" / "01-test-raw.mp4" _make_video(vid_raw, 10, frames_fn=_static_gray) @@ -348,7 +348,12 @@ def test_static_video_does_not_fail_pre_push(self, config, cfg_dir): ) (cfg_dir / "narration" / "01-test.md").write_text("Narration text here.\n") v = Validator(config) - v.run_pre_push() # should NOT raise + with pytest.raises(SystemExit) as ei: + v.run_pre_push() + assert ei.value.code == 1 + out = capsys.readouterr().out + assert "FAIL" in out + assert "freeze_ratio" in out # ── Manim scene lint ─────────────────────────────────────────────────── @@ -679,6 +684,60 @@ def test_drift_pass_when_ffprobe_succeeds(self, config, tmp_path, monkeypatch): assert result.passed is True +@pytest.mark.parametrize( + "check_name", + ("av_sync", "subject_beat_coverage", "ocr_scan", "layout", "freeze_ratio"), +) +def test_run_pre_push_visual_sync_fail_is_hard(check_name: str, capsys) -> None: + """Visual-sync FAILs must be FAIL + SystemExit, not WARN (leftover #2).""" + v = Validator.__new__(Validator) + + def _reports(_max_drift_override=None): + return [ + { + "segment": "01", + "checks": [ + {"name": check_name, "passed": False, "details": ["desynced"]}, + ], + } + ] + + v.run_all = _reports # type: ignore[method-assign] + with pytest.raises(SystemExit) as ei: + v.run_pre_push() + assert ei.value.code == 1 + out = capsys.readouterr().out + assert f"FAIL [01] {check_name}" in out + assert "WARN" not in out + assert "[validate] All checks passed" not in out + + +def test_run_pre_push_missing_recording_stays_soft(capsys) -> None: + """recording_exists remains a warning so ungenerated bundles stay pushable.""" + v = Validator.__new__(Validator) + + def _reports(_max_drift_override=None): + return [ + { + "segment": "01", + "checks": [ + { + "name": "recording_exists", + "passed": False, + "details": ["No recording for 01"], + }, + ], + } + ] + + v.run_all = _reports # type: ignore[method-assign] + v.run_pre_push() + out = capsys.readouterr().out + assert "WARN [01] recording_exists" in out + assert "FAIL" not in out + assert "[validate] All checks passed" in out + + # ── Helper to create silent audio ───────────────────────────────────── def _make_silent_audio(path: Path, duration_sec: float = 10.0) -> Path: