From 4a42c9812b6d0fa794b90e1af683a9ad1f790923 Mon Sep 17 00:00:00 2001 From: John Menke Date: Fri, 11 Sep 2026 12:18:23 -0400 Subject: [PATCH] Fail closed when tesseract is missing for OCR, av_sync, and layout. Those checks used to return passed=True with a skip message, so a machine without tesseract could green-light visual-sync gates it never ran. Co-authored-by: Cursor --- AGENTS.md | 2 +- README.md | 1 + src/docgen/validate.py | 48 +++++++++++++++++------------- tests/test_validate_timing_sync.py | 45 ++++++++++++++++++++++++++-- 4 files changed, 71 insertions(+), 25 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ca2f276..aabf2dc 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`** (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`). +- **`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`). Missing tesseract fails `ocr_scan` / `av_sync` / `layout` (not skip-PASS). - **`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 35443f7..7662044 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,7 @@ If you still need the legacy behaviour, pin a pre-removal commit 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 — hard fail on `--pre-push` / `generate-all`). + Missing tesseract fails `ocr_scan` / `av_sync` / `layout` instead of skip-PASS. - **GitHub Pages** — auto-generate `index.html`, deploy workflow, LFS rules, `.gitignore`. - **Wizard** — local web GUI to bootstrap narration scripts from existing project diff --git a/src/docgen/validate.py b/src/docgen/validate.py index 1715310..9a0660d 100644 --- a/src/docgen/validate.py +++ b/src/docgen/validate.py @@ -1,8 +1,8 @@ """Unified validator combining all quality checks. Core checks (freeze_ratio, blank_frames) use only cv2 — always available. -OCR text scanning uses pytesseract — degrades gracefully if tesseract -binary is missing, but cv2 checks still run and still fail the build. +OCR / av_sync / layout need pytesseract plus the tesseract binary. Missing +either fails those checks (not skip-PASS). Other cv2 checks still run. """ from __future__ import annotations @@ -49,6 +49,17 @@ def to_dict(self) -> dict[str, Any]: } +def _tesseract_unavailable_detail() -> str | None: + """Return a fail-closed detail if pytesseract or the tesseract binary is missing.""" + try: + import pytesseract + + pytesseract.get_tesseract_version() + except Exception as exc: + return f"tesseract unavailable ({type(exc).__name__}: {exc})" + return None + + def _sample_frames(path: Path, interval_sec: float = 2.0) -> list[tuple[float, np.ndarray]]: """Read frames at *interval_sec* across the entire video. Returns (timestamp, frame) pairs.""" cap = cv2.VideoCapture(str(path)) @@ -445,7 +456,7 @@ def _check_blank_frames( return CheckResult("blank_frames", passed, details) - # ── OCR text scanning (pytesseract — degrades if binary missing) ── + # ── OCR text scanning (pytesseract — fail-closed if binary missing) ── def _check_ocr( self, path: Path, samples: list[tuple[float, np.ndarray]] @@ -453,15 +464,14 @@ def _check_ocr( """Run OCR on sampled frames to detect error text in recordings. Uses the SAME samples as freeze/blank checks so the entire video - is covered. Gracefully skips if tesseract binary is not installed. + is covered. Missing tesseract fails the check (not skip-PASS). """ import re - try: - import pytesseract - pytesseract.get_tesseract_version() - except Exception: - return CheckResult("ocr_scan", True, ["tesseract binary not installed (skipped)"]) + unavail = _tesseract_unavailable_detail() + if unavail: + return CheckResult("ocr_scan", False, [unavail]) + import pytesseract error_patterns = self.config.ocr_config.get("error_patterns", []) if not error_patterns or not samples: @@ -630,11 +640,9 @@ def _check_scene_assets(self, seg_id: str) -> CheckResult: def _check_layout(self, path: Path) -> CheckResult: """Run overlap/spacing/edge layout checks on a Manim video recording.""" - try: - import pytesseract - pytesseract.get_tesseract_version() - except Exception: - return CheckResult("layout", True, ["tesseract not installed — layout check skipped"]) + unavail = _tesseract_unavailable_detail() + if unavail: + return CheckResult("layout", False, [unavail]) try: from docgen.manim_layout import LayoutValidator @@ -952,8 +960,8 @@ def _check_av_sync(self, seg_id: str, rec: Path) -> CheckResult: """OCR anchor check: spoken keywords should be visible on screen near their spoken time. Heuristic (soft in --pre-push). Uses ``timing.json`` — no network calls. - Skips when tesseract is unavailable, timing data is missing, or the - segment's visual type is not in ``validation.av_sync.visual_types``. + Missing tesseract fails the check. Timing data or a visual type outside + ``validation.av_sync.visual_types`` still skip when those gates apply. """ sync_cfg = self.config.av_sync_config if not sync_cfg.get("enabled", True): @@ -964,11 +972,9 @@ def _check_av_sync(self, seg_id: str, rec: Path) -> CheckResult: if allowed and vt not in allowed: return CheckResult("av_sync", True, [f"visual type {vt!r} not checked (skipped)"]) - try: - import pytesseract - pytesseract.get_tesseract_version() - except Exception: - return CheckResult("av_sync", True, ["tesseract binary not installed (skipped)"]) + unavail = _tesseract_unavailable_detail() + if unavail: + return CheckResult("av_sync", False, [unavail]) from docgen.timestamps import TimestampError diff --git a/tests/test_validate_timing_sync.py b/tests/test_validate_timing_sync.py index 185adb1..04f38d6 100644 --- a/tests/test_validate_timing_sync.py +++ b/tests/test_validate_timing_sync.py @@ -491,13 +491,14 @@ def validate_video(self, _path: Path) -> None: assert any("conf is not int" in d for d in check.details) assert not any("(skipped)" in d for d in check.details) - def test_tesseract_missing_still_skips(self, cfg, monkeypatch) -> None: + def test_tesseract_missing_fails(self, cfg, monkeypatch) -> None: rec = cfg.recordings_dir / "01-x.mp4" rec.write_bytes(b"not a video") self._fake_tesseract(monkeypatch, installed=False) check = Validator(cfg)._check_layout(rec) - assert check.passed - assert any("tesseract not installed" in d for d in check.details) + assert check.passed is False + assert any("tesseract unavailable" in d for d in check.details) + assert not any("skipped" in d.lower() for d in check.details) def test_failed_report_still_fails(self, cfg, monkeypatch) -> None: from docgen.manim_layout import LayoutIssue, LayoutReport @@ -521,3 +522,41 @@ def validate_video(self, path: Path) -> LayoutReport: check = Validator(cfg)._check_layout(rec) assert not check.passed assert any("overlap" in d for d in check.details) + + +def _fake_tesseract_missing(monkeypatch, *, mode: str) -> None: + if mode == "no_module": + monkeypatch.setitem(sys.modules, "pytesseract", None) + return + + pt = types.ModuleType("pytesseract") + + def _missing() -> str: + raise RuntimeError("tesseract is not installed") + + pt.get_tesseract_version = _missing + monkeypatch.setitem(sys.modules, "pytesseract", pt) + + +class TestTesseractMissingFails: + """Missing tesseract must fail OCR / av_sync / layout, not skip-PASS.""" + + @pytest.mark.parametrize("check_name", ("ocr_scan", "layout", "av_sync")) + @pytest.mark.parametrize("mode", ("no_binary", "no_module")) + def test_tesseract_missing_does_not_skip_pass( + self, cfg, monkeypatch, check_name: str, mode: str + ) -> None: + rec = cfg.recordings_dir / "01-x.mp4" + rec.write_bytes(b"not a video") + _fake_tesseract_missing(monkeypatch, mode=mode) + v = Validator(cfg) + if check_name == "ocr_scan": + check = v._check_ocr(rec, []) + elif check_name == "layout": + check = v._check_layout(rec) + else: + check = v._check_av_sync("01", rec) + assert check.name == check_name + assert check.passed is False + assert any("tesseract unavailable" in d for d in check.details) + assert not any("skipped" in d.lower() for d in check.details)