Fail closed when compose ffmpeg times out with a partial mux

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:** **[visual-beats-numeric.md](visual-beats-numeric.md)** —
-`visual_beats` / `default_visual_beats` must be YAML numbers (bool
-`true` used to become one beat; invalid types auto-estimated).
+**Active:** **[compose-ffmpeg-timeout.md](compose-ffmpeg-timeout.md)** —
+compose must not treat a timed-out ffmpeg mux as success because a
+partial output file exists.
 
 **Shipped:**
+- **[visual-beats-numeric.md](visual-beats-numeric.md)** —
+  `visual_beats` / `default_visual_beats` must be YAML numbers (#120).
 - **[validation-enable-bools.md](validation-enable-bools.md)** —
   validation / manim enable flags must be YAML booleans (#119).
 - **[validation-numeric-tunables.md](validation-numeric-tunables.md)** —

milestones/compose-ffmpeg-timeout.md

@@ -0,0 +1,38 @@
+# Milestone: compose must not accept a timed-out ffmpeg mux
+
+**Status:** Active  
+**PR:** [#121](https://github.com/jmjava/documentation-generator/pull/121)  
+**Depends on:** `milestones/pipeline-fail-closed.md`,
+`milestones/visual-beats-numeric.md` (PR #120)
+
+## Problem
+
+``Composer._run_ffmpeg`` treated ``subprocess.TimeoutExpired`` as
+**success** when the output path already existed and had a non-zero
+size. A hung mux could leave a truncated ``recordings/<stem>.mp4``;
+compose printed a warning, returned, and counted the segment as
+composed. Concat already raises ``ConcatError`` on ffmpeg timeout.
+
+## Goal
+
+Fail closed. A timed-out ffmpeg compose run must raise
+``ComposeError`` even when a partial output file exists. Remove the
+incomplete file so later stages cannot treat it as a finished
+recording.
+
+## Done when
+
+- [x] Timeout with a partial output raises ``ComposeError``.
+- [x] Incomplete output is removed.
+- [x] Tests cover timeout with and without an existing output file.
+- [x] `ruff check src/ tests/`
+- [x] `pytest tests/` (703 passed, 1 skipped)
+- [x] `docgen benchmark` (no clock change; meets baseline)
+
+## Out of scope
+
+- Changing ``compose.ffmpeg_timeout_sec`` defaults.
+- Duration-probe SKIP when ffprobe is missing (pipeline still fails
+  when mapped segments are not composed).
+- Manim render timeouts (already treated as a failed scene, not a
+  successful mp4).

milestones/visual-beats-numeric.md

@@ -1,6 +1,6 @@
 # Milestone: visual_beats must be YAML numbers (no silent auto-fallback)
 
-**Status:** Active  
+**Status:** Shipped  
 **PR:** [#120](https://github.com/jmjava/documentation-generator/pull/120)  
 **Depends on:** `milestones/generation-numeric-tunables.md` (PR #117),
 `milestones/validation-enable-bools.md` (PR #119)

src/docgen/compose.py

@@ -331,14 +331,17 @@ def _run_ffmpeg(self, cmd: list[str]) -> None:
         except subprocess.CalledProcessError as exc:
             detail = (exc.stderr or exc.stdout or "")[:400]
             raise ComposeError(f"ffmpeg failed: {detail}")
-        except subprocess.TimeoutExpired:
-            if out_path and out_path.exists() and out_path.stat().st_size > 0:
-                print(
-                    f"    WARNING: ffmpeg timed out after {timeout_sec}s, "
-                    f"but output exists at {out_path}."
-                )
-                return
-            raise ComposeError(f"ffmpeg timed out after {timeout_sec}s")
+        except subprocess.TimeoutExpired as exc:
+            removed = ""
+            if out_path is not None and out_path.exists():
+                try:
+                    out_path.unlink()
+                    removed = f" (removed incomplete {out_path.name})"
+                except OSError:
+                    removed = f" (incomplete {out_path.name} still present)"
+            raise ComposeError(
+                f"ffmpeg timed out after {timeout_sec}s{removed}"
+            ) from exc
 
     def _manim_video_dirs(self) -> list[Path]:
         root = self.config.animations_dir / "media" / "videos"

tests/test_compose.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import os
+import subprocess
 import time
 from pathlib import Path
 
@@ -265,3 +266,48 @@ def test_cli_compose_empty_segments_is_click_error(tmp_path: Path) -> None:
     result = runner.invoke(main, ["--config", str(c.yaml_path), "compose"])
     assert result.exit_code != 0
     assert "no segments to compose" in (result.output + result.stderr).lower()
+
+
+def test_run_ffmpeg_timeout_with_partial_output_raises(tmp_path: Path, monkeypatch) -> None:
+    """A timed-out mux must not count as success just because a partial file exists."""
+    cfg = {
+        "dirs": {"animations": "animations", "audio": "audio", "recordings": "recordings"},
+        "segments": {"default": ["01"], "all": ["01"]},
+        "segment_names": {"01": "01-demo"},
+        "visual_map": {"01": {"type": "manim", "source": "Scene01.mp4"}},
+    }
+    c = _write_cfg(tmp_path, cfg)
+    out = tmp_path / "recordings" / "01-demo.mp4"
+    out.parent.mkdir(parents=True, exist_ok=True)
+    out.write_bytes(b"partial-mux")
+
+    def fake_run(cmd, **_kwargs):
+        raise subprocess.TimeoutExpired(cmd=cmd, timeout=1)
+
+    monkeypatch.setattr(subprocess, "run", fake_run)
+    composer = Composer(c)
+    composer.ffmpeg_timeout_sec = 1
+    with pytest.raises(ComposeError, match="removed incomplete 01-demo.mp4"):
+        composer._run_ffmpeg(["ffmpeg", "-y", str(out)])
+    assert not out.exists()
+
+
+def test_run_ffmpeg_timeout_without_output_raises(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)
+    missing = tmp_path / "recordings" / "missing.mp4"
+    missing.parent.mkdir(parents=True, exist_ok=True)
+
+    def fake_run(cmd, **_kwargs):
+        raise subprocess.TimeoutExpired(cmd=cmd, timeout=2)
+
+    monkeypatch.setattr(subprocess, "run", fake_run)
+    composer = Composer(c)
+    composer.ffmpeg_timeout_sec = 2
+    with pytest.raises(ComposeError, match="ffmpeg timed out after 2s"):
+        composer._run_ffmpeg(["ffmpeg", "-y", str(missing)])
+    assert not missing.exists()