Fail closed when ffprobe duration probes exit nonzero

milestones/README.md

@@ -5,10 +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:** **[timing-start-end.md](timing-start-end.md)** —
-`timing.json` word/segment `start` / `end` must be JSON numbers.
+**Active:** **[ffprobe-returncode.md](ffprobe-returncode.md)** —
+ffprobe duration probes must honor exit status; image compose needs `-t`.
 
 **Shipped:**
+- **[timing-start-end.md](timing-start-end.md)** —
+  `timing.json` word/segment `start` / `end` must be JSON numbers
+  (#134).
 - **[wizard-path-ref.md](wizard-path-ref.md)** —
   wizard `open-bundle` `path` and `tool/update` `ref` must be JSON strings
   (#133).

milestones/ffprobe-returncode.md

@@ -0,0 +1,39 @@
+# Milestone: ffprobe duration probes must honor returncode
+
+**Status:** Active  
+**PR:** [#135](https://github.com/jmjava/documentation-generator/pull/135)  
+**Depends on:** `milestones/compose-ffmpeg-timeout.md` (PR #121),
+`milestones/timing-start-end.md` (PR #134)
+
+## Problem
+
+Duration probes in compose / TTS / validate / local timestamps ran ffprobe
+without checking ``returncode``, then ``float(stdout)``. A failed probe
+that still printed a number (or leftover stdout) looked like a real
+duration.
+
+``_compose_image`` treated a missing duration as ``-t ""`` and stripped
+it, so a looping still was muxed **without** a finite ``-t``.
+
+Manim/simple compose already SKIPs when both probes fail (CLI / pipeline
+still fail if composed < mapped). That SKIP stays.
+
+## Goal
+
+``returncode != 0`` is a failed probe (``None``, or ``AlignmentError``
+for local timestamps). Image compose raises ``ComposeError`` instead of
+dropping ``-t``.
+
+## Done when
+
+- [x] Compose / TTS / validate probes ignore stdout when ffprobe fails
+- [x] Image compose does not run ffmpeg without a duration
+- [x] Local ``probe_duration`` raises on nonzero ffprobe
+- [x] `ruff check src/ tests/`
+- [x] `pytest tests/` (770 passed, 1 skipped)
+- [x] `docgen benchmark` (no clock change; meets baseline)
+
+## Out of scope
+
+- Manim/simple compose SKIP when probes fail (CLI still fails if short)
+- Validate ``timing_sync`` still skips as passed when duration is unknown

milestones/timing-start-end.md

@@ -1,6 +1,6 @@
 # Milestone: timing.json word/segment start and end must be numbers
 
-**Status:** Active  
+**Status:** Shipped  
 **PR:** [#134](https://github.com/jmjava/documentation-generator/pull/134)  
 **Depends on:** `milestones/timing-inner-lists.md` (PR #127),
 `milestones/bootstrap-timing-helpers.md` (PR #130),

src/docgen/align.py

@@ -94,10 +94,19 @@ def probe_duration(audio_path: Path) -> float:
              "-of", "csv=p=0", str(audio_path)],
             capture_output=True, text=True, timeout=30,
         )
-        return float(out.stdout.strip())
     except FileNotFoundError as exc:
         raise AlignmentError("ffprobe not found in PATH (required for local timing)") from exc
-    except (ValueError, subprocess.TimeoutExpired) as exc:
+    except subprocess.TimeoutExpired as exc:
+        raise AlignmentError(f"cannot probe duration of {audio_path}: {exc}") from exc
+    if out.returncode != 0:
+        detail = (out.stderr or out.stdout or "").strip()[:200]
+        extra = f": {detail}" if detail else ""
+        raise AlignmentError(
+            f"ffprobe failed on {audio_path} (exit {out.returncode}){extra}"
+        )
+    try:
+        return float(out.stdout.strip())
+    except ValueError as exc:
         raise AlignmentError(f"cannot probe duration of {audio_path}: {exc}") from exc
 
 

src/docgen/compose.py

@@ -259,6 +259,11 @@ def _compose_image(self, seg_id: str, relpath: str) -> bool:
         out.parent.mkdir(parents=True, exist_ok=True)
 
         audio_dur = self._probe_duration(audio)
+        if audio_dur is None or audio_dur <= 0:
+            raise ComposeError(
+                f"cannot probe audio duration for {audio.name} — "
+                "looping image mux needs a finite -t (check ffprobe / the mp3)"
+            )
         cmd = [
             "ffmpeg", "-y",
             "-loop", "1", "-framerate", "30", "-i", str(img),
@@ -267,13 +272,12 @@ def _compose_image(self, seg_id: str, relpath: str) -> bool:
                    "pad=1280:720:(ow-iw)/2:(oh-ih)/2,format=yuv420p",
             "-c:v", "libx264", "-preset", "fast", "-crf", "23",
             "-c:a", "aac", "-b:a", "128k",
-            "-t", f"{audio_dur:.3f}" if audio_dur else "",
+            "-t", f"{audio_dur:.3f}",
             "-movflags", "+faststart",
             str(out),
         ]
-        cmd = [c for c in cmd if c]
         self._run_ffmpeg(cmd)
-        print(f"    ok image {img.name} + audio={audio_dur:.1f}s" if audio_dur else "    ok image")
+        print(f"    ok image {img.name} + audio={audio_dur:.1f}s")
         return True
 
     def _find_audio(self, seg_id: str) -> Path | None:
@@ -317,8 +321,13 @@ def _probe_duration(path: Path) -> float | None:
                  "-of", "csv=p=0", str(path)],
                 capture_output=True, text=True, timeout=30,
             )
+        except (subprocess.TimeoutExpired, FileNotFoundError):
+            return None
+        if out.returncode != 0:
+            return None
+        try:
             return float(out.stdout.strip())
-        except (ValueError, subprocess.TimeoutExpired, FileNotFoundError):
+        except ValueError:
             return None
 
     def _run_ffmpeg(self, cmd: list[str]) -> None:

src/docgen/tts.py

@@ -23,8 +23,13 @@ def _probe_duration(path: Path) -> float | None:
              "-of", "csv=p=0", str(path)],
             capture_output=True, text=True, timeout=30,
         )
+    except (subprocess.TimeoutExpired, FileNotFoundError):
+        return None
+    if out.returncode != 0:
+        return None
+    try:
         return float(out.stdout.strip())
-    except (ValueError, subprocess.TimeoutExpired, FileNotFoundError):
+    except ValueError:
         return None
 
 

src/docgen/validate.py

@@ -987,8 +987,13 @@ def _probe_media_duration(path: Path) -> float | None:
                  "-of", "csv=p=0", str(path)],
                 capture_output=True, text=True, timeout=30,
             )
+        except (subprocess.TimeoutExpired, FileNotFoundError):
+            return None
+        if out.returncode != 0:
+            return None
+        try:
             return float(out.stdout.strip())
-        except (ValueError, subprocess.TimeoutExpired, FileNotFoundError):
+        except ValueError:
             return None
 
     # ── Helpers ────────────────────────────────────────────────────────

tests/test_align.py

@@ -130,3 +130,18 @@ def test_build_local_timing_uses_reconcile_for_near_miss_counts(self) -> None:
         assert timing["segments"][0]["start"] == pytest.approx(0.0)
         assert timing["segments"][0]["end"] == pytest.approx(3.5)
         assert timing["segments"][1]["start"] == pytest.approx(4.0)
+
+
+def test_probe_duration_rejects_nonzero_ffprobe_exit(monkeypatch) -> None:
+    from pathlib import Path
+
+    from docgen.align import AlignmentError, probe_duration
+
+    class _Proc:
+        returncode = 1
+        stdout = "9.0\n"
+        stderr = "ffprobe: Invalid data"
+
+    monkeypatch.setattr("docgen.align.subprocess.run", lambda *_a, **_k: _Proc())
+    with pytest.raises(AlignmentError, match="ffprobe failed"):
+        probe_duration(Path("/tmp/x.mp3"))

tests/test_compose.py

@@ -311,3 +311,48 @@ def fake_run(cmd, **_kwargs):
     with pytest.raises(ComposeError, match="ffmpeg timed out after 2s"):
         composer._run_ffmpeg(["ffmpeg", "-y", str(missing)])
     assert not missing.exists()
+
+
+def test_compose_image_raises_when_audio_duration_unknown(tmp_path: Path, monkeypatch) -> None:
+    cfg = {
+        "dirs": {"animations": "animations", "audio": "audio", "recordings": "recordings"},
+        "segments": {"default": ["01"], "all": ["01"]},
+        "segment_names": {"01": "01-demo"},
+        "visual_map": {"01": {"type": "image", "source": "images/slide.png"}},
+    }
+    c = _write_cfg(tmp_path, cfg)
+    (tmp_path / "audio").mkdir(parents=True, exist_ok=True)
+    (tmp_path / "images").mkdir(parents=True, exist_ok=True)
+    (tmp_path / "audio" / "01-demo.mp3").write_bytes(b"fake-mp3")
+    (tmp_path / "images" / "slide.png").write_bytes(b"fake-png")
+    (tmp_path / "recordings").mkdir(parents=True, exist_ok=True)
+
+    composer = Composer(c)
+    monkeypatch.setattr(composer, "_probe_duration", lambda _p: None)
+    ran = {"n": 0}
+
+    def boom(_cmd):
+        ran["n"] += 1
+
+    monkeypatch.setattr(composer, "_run_ffmpeg", boom)
+    with pytest.raises(ComposeError, match="cannot probe audio duration"):
+        composer._compose_image("01", "images/slide.png")
+    assert ran["n"] == 0
+
+
+def test_probe_duration_ignores_stdout_when_ffprobe_fails(tmp_path: Path, monkeypatch) -> None:
+    cfg = {
+        "dirs": {"animations": "animations", "audio": "audio", "recordings": "recordings"},
+        "segments": {"default": ["01"], "all": ["01"]},
+        "visual_map": {"01": {"type": "manim", "source": "Scene01.mp4"}},
+    }
+    c = _write_cfg(tmp_path, cfg)
+    composer = Composer(c)
+
+    class _Proc:
+        returncode = 1
+        stdout = "12.345\n"
+        stderr = "error"
+
+    monkeypatch.setattr(subprocess, "run", lambda *_a, **_k: _Proc())
+    assert composer._probe_duration(tmp_path / "missing.mp3") is None

tests/test_tts.py

@@ -162,6 +162,13 @@ def test_probe_duration_returns_none_for_missing_file(tmp_path):
 
 @patch("docgen.tts.subprocess.run")
 def test_probe_duration_returns_float(mock_run):
-    mock_run.return_value = type("R", (), {"stdout": "12.345\n"})()
+    mock_run.return_value = type("R", (), {"stdout": "12.345\n", "returncode": 0})()
     result = _probe_duration(__import__("pathlib").Path("/tmp/test.mp3"))
     assert result == 12.345
+
+
+@patch("docgen.tts.subprocess.run")
+def test_probe_duration_returns_none_when_ffprobe_fails(mock_run):
+    mock_run.return_value = type("R", (), {"stdout": "12.345\n", "returncode": 1})()
+    result = _probe_duration(__import__("pathlib").Path("/tmp/test.mp3"))
+    assert result is None