diff --git a/milestones/README.md b/milestones/README.md index 909d8de..01ab8d1 100644 --- a/milestones/README.md +++ b/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). diff --git a/milestones/hint-front-matter-yaml.md b/milestones/hint-front-matter-yaml.md index dc64dcb..e2c3679 100644 --- a/milestones/hint-front-matter-yaml.md +++ b/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) diff --git a/milestones/image-empty-bytes.md b/milestones/image-empty-bytes.md new file mode 100644 index 0000000..08779c4 --- /dev/null +++ b/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`. diff --git a/src/docgen/image_generate.py b/src/docgen/image_generate.py index 023b3a7..b41722b 100644 --- a/src/docgen/image_generate.py +++ b/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)) diff --git a/tests/test_image_generate.py b/tests/test_image_generate.py index d46f667..ea0df85 100644 --- a/tests/test_image_generate.py +++ b/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) == []