Fail closed when image-generate returns empty bytes

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:** **[hint-front-matter-yaml.md](hint-front-matter-yaml.md)** —
-Invalid `hints/*.md` YAML front matter must fail `yaml-generate`, not skip
-hint `visual_map` wiring.
+**Active:** **[image-empty-bytes.md](image-empty-bytes.md)** —
+`image-generate` must not write empty PNG bytes as success.
 
 **Shipped:**
+- **[hint-front-matter-yaml.md](hint-front-matter-yaml.md)** —
+  Invalid `hints/*.md` YAML front matter must fail `yaml-generate`, not skip
+  hint `visual_map` wiring (#102).
 - **[av-sync-anchor-keywords.md](av-sync-anchor-keywords.md)** —
   `validation.av_sync.anchor_keywords` must be a mapping of keyword rows
   (#101).

milestones/hint-front-matter-yaml.md

@@ -1,7 +1,7 @@
 # Milestone: invalid hint front matter must not skip wiring
 
-**Status:** Active  
-**PR:** pending  
+**Status:** Shipped  
+**PR:** #102  
 **Depends on:** `milestones/av-sync-anchor-keywords.md` (PR #101),
 `milestones/yaml-generate-mappings.md` (PR #91)
 

milestones/image-empty-bytes.md

@@ -0,0 +1,35 @@
+# Milestone: image-generate must not write empty assets
+
+**Status:** Active  
+**PR:** pending  
+**Depends on:** `milestones/hint-front-matter-yaml.md` (PR #102),
+`milestones/tts-empty-audio.md` (PR #99)
+
+## Problem
+
+`generate_images_for_spec` wrote whatever `image_fn` / the Images API
+returned, including **0-byte** files, and reported `generated`. Pipeline
+Manim then loaded an empty PNG. TTS already fails closed on empty audio
+(#99); images did not.
+
+`generate_image_bytes` also returned empty `b64_json` / URL bodies.
+
+## Goal
+
+Empty provider bytes raise `ImageGenerationError` **before** writing.
+`--force` must not clobber a committed asset with empty bytes.
+
+## Done when
+
+- [x] Empty `image_fn` / API bytes raise and do not write a new file.
+- [x] `--force` with empty bytes leaves an existing asset unchanged.
+- [x] Empty b64 / URL download raises in `generate_image_bytes`.
+- [x] Tests for the spec write path.
+- [x] `ruff check src/ tests/`
+- [x] `pytest tests/`
+- [x] `docgen benchmark` (no clock change)
+
+## Out of scope
+
+- `image_generation.model` / `size` string typing is separate.
+- Existing non-empty assets are still skipped unless `--force`.

src/docgen/image_generate.py

@@ -103,15 +103,25 @@ def generate_image_bytes(
     data = response.data[0] if response.data else None
     b64 = getattr(data, "b64_json", None) if data is not None else None
     if b64:
-        return base64.b64decode(b64)
+        raw = base64.b64decode(b64)
+        if not raw:
+            raise ImageGenerationError(
+                f"Image model {resolved!r} returned empty b64_json bytes"
+            )
+        return raw
     url = getattr(data, "url", None) if data is not None else None
     if url:
         try:
-            return fetch_url_bytes(str(url))
+            raw = fetch_url_bytes(str(url))
         except Exception as exc:
             raise ImageGenerationError(
                 f"Image model {resolved!r} returned a URL but download failed: {exc}."
             ) from exc
+        if not raw:
+            raise ImageGenerationError(
+                f"Image model {resolved!r} URL download was empty"
+            )
+        return raw
     raise ImageGenerationError(
         f"Image response for model {resolved!r} had neither b64_json nor url; "
         "cannot write the asset."
@@ -178,6 +188,10 @@ def generate_images_for_spec(
             )
         )
         data = fn(prompt)
+        if not data:
+            raise ImageGenerationError(
+                f"{spec_path}: image element {rel!r} — provider returned empty bytes"
+            )
         out.parent.mkdir(parents=True, exist_ok=True)
         out.write_bytes(data)
         results.append(ImageAssetResult(rel, out, "generated", prompt))

tests/test_image_generate.py

@@ -103,6 +103,23 @@ def test_bundle_scan_generates_only_missing(cfg: Config) -> None:
     assert existing.read_bytes() == b"committed"
 
 
+def test_empty_provider_bytes_fails_without_writing(cfg: Config) -> None:
+    spec = _write_spec(cfg.animations_dir / "specs" / "01-x.scene.yaml")
+    with pytest.raises(ImageGenerationError, match="empty bytes"):
+        generate_images_for_spec(cfg, spec, image_fn=lambda p: b"")
+    assert not (cfg.base_dir / "images" / "arch.png").exists()
+
+
+def test_empty_provider_bytes_does_not_clobber_existing(cfg: Config) -> None:
+    spec = _write_spec(cfg.animations_dir / "specs" / "01-x.scene.yaml")
+    out = cfg.base_dir / "images" / "arch.png"
+    out.parent.mkdir(parents=True)
+    out.write_bytes(b"committed")
+    with pytest.raises(ImageGenerationError, match="empty bytes"):
+        generate_images_for_spec(cfg, spec, force=True, image_fn=lambda p: b"")
+    assert out.read_bytes() == b"committed"
+
+
 def test_no_specs_dir_is_noop(cfg: Config) -> None:
     assert spec_files_for_bundle(cfg) == []
     assert generate_missing_images_for_bundle(cfg, image_fn=lambda p: _PNG_BYTES) == []