Fail closed when concat ffmpeg leaves a truncated output

Concat already raised ConcatError on timeout or non-zero ffmpeg, but
left recordings/<target>.mp4 in place. Unlink the incomplete file so
pages and validate cannot treat a hung concat as a finished demo.

Co-authored-by: jmjava <jmjava@gmail.com>

Author: cursoragent

milestones/README.md

@@ -5,11 +5,14 @@ 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:** **[generation-zero-values.md](generation-zero-values.md)** —
-`temperature: 0` / `max_whisper_segment_text_chars: 0` must not be
-replaced by ``or`` defaults.
+**Active:** **[concat-ffmpeg-timeout.md](concat-ffmpeg-timeout.md)** —
+concat must not leave a truncated full-demo mp4 after ffmpeg
+timeout or failure.
 
 **Shipped:**
+- **[generation-zero-values.md](generation-zero-values.md)** —
+  `temperature: 0` / `max_whisper_segment_text_chars: 0` must not be
+  replaced by ``or`` defaults (#122).
 - **[compose-ffmpeg-timeout.md](compose-ffmpeg-timeout.md)** —
   compose must not treat a timed-out ffmpeg mux as success (#121).
 - **[visual-beats-numeric.md](visual-beats-numeric.md)** —

milestones/concat-ffmpeg-timeout.md

@@ -0,0 +1,34 @@
+# Milestone: concat must not leave a truncated ffmpeg output
+
+**Status:** Active  
+**PR:** (pending)  
+**Depends on:** `milestones/compose-ffmpeg-timeout.md` (PR #121),
+`milestones/generation-zero-values.md` (PR #122)
+
+## Problem
+
+Compose now raises and unlinks a partial mux on ffmpeg timeout (#121).
+Concat already raised ``ConcatError`` on timeout / non-zero ffmpeg, but
+it left ``recordings/<target>.mp4`` in place. ``ffmpeg -y`` writes as
+it goes, so a hung or failed concat could leave a truncated full-demo
+file that ``pages`` / validate treat as a finished recording.
+
+## Goal
+
+On concat ffmpeg timeout, ``CalledProcessError``, or missing ffmpeg,
+unlink the incomplete output (same contract as compose). Keep raising
+``ConcatError``. Empty concat maps stay a no-op.
+
+## Done when
+
+- [ ] Timeout / failed concat removes the incomplete target mp4.
+- [ ] Tests cover timeout and CalledProcessError with a pre-existing
+      partial file.
+- [ ] `ruff check src/ tests/`
+- [ ] `pytest tests/`
+- [ ] `docgen benchmark` (no clock change; meets baseline)
+
+## Out of scope
+
+- Changing the 300s concat ffmpeg timeout.
+- Empty ``concat:`` maps (still a no-op).

milestones/generation-zero-values.md

@@ -1,6 +1,6 @@
 # Milestone: honor explicit generation zeros (temperature / max_chars)
 
-**Status:** Active  
+**Status:** Shipped  
 **PR:** [#122](https://github.com/jmjava/documentation-generator/pull/122)  
 **Depends on:** `milestones/generation-numeric-tunables.md` (PR #117),
 `milestones/compose-ffmpeg-timeout.md` (PR #121)

src/docgen/concat.py

@@ -89,11 +89,25 @@ def _build_one(self, out_name: str, seg_ids: list[str]) -> None:
                 cwd=str(recordings_dir),
             )
         except FileNotFoundError as exc:
+            _unlink_incomplete(out)
             raise ConcatError("[concat] ffmpeg not found in PATH") from exc
         except subprocess.CalledProcessError as exc:
+            _unlink_incomplete(out)
             detail = (exc.stderr or exc.stdout or "")[:400]
             raise ConcatError(f"[concat] ffmpeg failed: {detail}") from exc
-        except subprocess.TimeoutExpired as exc:
-            raise ConcatError("[concat] ffmpeg timed out") from exc
+        except subprocess.TimeoutExpired as ext:
+            existed = out.exists()
+            _unlink_incomplete(out)
+            extra = f" (removed incomplete {out.name})" if existed else ""
+            raise ConcatError(f"[concat] ffmpeg timed out{extra}") from ext
         finally:
             concat_list.unlink(missing_ok=True)
+
+
+def _unlink_incomplete(path: Path) -> None:
+    """Remove a truncated concat output so later stages cannot treat it as finished."""
+    if path.exists():
+        try:
+            path.unlink()
+        except OSError:
+            pass

tests/test_concat.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import subprocess
 from pathlib import Path
 
 import pytest
@@ -71,3 +72,45 @@ def test_concat_builder_rejects_integer_segment_id() -> None:
     )
     with pytest.raises(ConcatError, match="quoted string segment id"):
         ConcatBuilder(cfg).build()  # type: ignore[arg-type]
+
+
+def _seed_recordings(tmp_path: Path) -> None:
+    rec = tmp_path / "recordings"
+    rec.mkdir()
+    (rec / "01-a.mp4").write_bytes(b"seg-a")
+    (rec / "02-b.mp4").write_bytes(b"seg-b")
+
+
+def test_concat_ffmpeg_timeout_removes_incomplete_output(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    cfg = _cfg(tmp_path, {"full": ["01", "02"]})
+    _seed_recordings(tmp_path)
+    out = tmp_path / "recordings" / "full.mp4"
+    out.write_bytes(b"partial-concat")
+
+    def fake_run(cmd, **_kwargs):
+        raise subprocess.TimeoutExpired(cmd=cmd, timeout=300)
+
+    monkeypatch.setattr(subprocess, "run", fake_run)
+    with pytest.raises(ConcatError, match="removed incomplete full.mp4"):
+        ConcatBuilder(cfg).build(name="full")
+    assert not out.exists()
+    assert not list((tmp_path / "recordings").glob(".concat-*.txt"))
+
+
+def test_concat_ffmpeg_failure_removes_incomplete_output(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    cfg = _cfg(tmp_path, {"full": ["01", "02"]})
+    _seed_recordings(tmp_path)
+    out = tmp_path / "recordings" / "full.mp4"
+    out.write_bytes(b"partial-concat")
+
+    def fake_run(cmd, **_kwargs):
+        raise subprocess.CalledProcessError(1, cmd, stderr="mux error")
+
+    monkeypatch.setattr(subprocess, "run", fake_run)
+    with pytest.raises(ConcatError, match="ffmpeg failed"):
+        ConcatBuilder(cfg).build(name="full")
+    assert not out.exists()