Handle editable PPTX fallback failures#8
Conversation
Implements a modular image-to-editable-PPTX reconstruction path with OCR, VLM-first analysis, image-edit/background/assets, manifest composition, validation, export integration, and real verification runners. Adds UI/config support for optional VLM and OCR models, plus stage docs and tests. Current live full-chain validation remains gated by VLM provider quota; reports now expose retry attempts and fallback status instead of silently accepting raster output.
There was a problem hiding this comment.
Code Review
This pull request introduces an experimental high-fidelity editable PPTX export mode (generative_editable_pptx) that reconstructs slide images into PowerPoint decks with editable text boxes, native shapes, and positioned bitmap assets. Key feedback from the review highlights a critical bug in _slide_text_metadata where string inputs can cause callable methods to be serialized as text, and a file-locking issue on Windows when resizing background images in-place. Additionally, the reviewer recommends raising specific fallback errors instead of raw exceptions during export fallback execution to prevent generic 500 errors, and suggests using defensive getattr calls in the profile resolver.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def _slide_text_metadata(slide_prompt) -> list[dict]: | ||
| metadata = [] | ||
| title = str(getattr(slide_prompt, "title", "") or "").strip() |
There was a problem hiding this comment.
If slide_prompt is a string object, getattr(slide_prompt, "title", "") will return the built-in title method of the str class rather than an empty string. This will result in a callable object being converted to a string (e.g., "<built-in method title of str object at ...>"), causing downstream failures or garbage text in the generated metadata. Add an explicit check to return early if slide_prompt is a string.
| def _slide_text_metadata(slide_prompt) -> list[dict]: | |
| metadata = [] | |
| title = str(getattr(slide_prompt, "title", "") or "").strip() | |
| def _slide_text_metadata(slide_prompt) -> list[dict]: | |
| if isinstance(slide_prompt, str): | |
| return [] | |
| metadata = [] | |
| title = getattr(slide_prompt, "title", "") | |
| if callable(title): | |
| title = "" | |
| title = str(title or "").strip() |
| def _export_text_editable_background_fallback(*, artifact_root: Path, job_id: str, output_path: str) -> str: | ||
| job_dir = artifact_root / job_id | ||
| deck_path = job_dir / "deck.json" | ||
| deck = read_deck_manifest(deck_path) |
There was a problem hiding this comment.
If the pipeline fails before creating the deck.json manifest (for example, due to an early provider timeout or configuration error), calling _export_text_editable_background_fallback will raise a raw FileNotFoundError. This bypasses the custom HTTP exception mapping and results in a generic 500 Internal Server Error instead of the correct 502 Bad Gateway or 504 Gateway Timeout. Check for the existence of deck_path and raise a GenerativeEditableFallbackError if it is missing.
def _export_text_editable_background_fallback(*, artifact_root: Path, job_id: str, output_path: str) -> str:
job_dir = artifact_root / job_id
deck_path = job_dir / "deck.json"
if not deck_path.is_file():
raise GenerativeEditableFallbackError(
"text_clean_background fallback failed: deck manifest is missing because the pipeline failed before creating artifacts"
)
deck = read_deck_manifest(deck_path)| with Image.open(output_path) as output: | ||
| if output.size == source_size: | ||
| return {} | ||
| normalized = output.convert("RGB").resize(source_size) | ||
| normalized.save(output_path) |
There was a problem hiding this comment.
Overwriting the file at output_path while it is still open for reading inside the with Image.open(output_path) as output block can raise a PermissionError or OSError on Windows and other platforms due to file locking. Read the image and close the file handle before saving the resized image back to the same path.
| with Image.open(output_path) as output: | |
| if output.size == source_size: | |
| return {} | |
| normalized = output.convert("RGB").resize(source_size) | |
| normalized.save(output_path) | |
| needs_resize = False | |
| with Image.open(output_path) as output: | |
| if output.size != source_size: | |
| needs_resize = True | |
| normalized = output.convert("RGB").resize(source_size) | |
| if needs_resize: | |
| normalized.save(output_path) | |
| return {"normalized_to_source_size": list(source_size)} | |
| return {} |
| background = page.text_clean_background or page.base_clean_background or page.chosen_background | ||
| if not background: | ||
| raise RuntimeError("text_clean_background fallback requires a cleaned background artifact") |
There was a problem hiding this comment.
Raising a raw RuntimeError when the cleaned background artifact is missing will cause the request to fail with a 500 Internal Server Error instead of propagating the correct fallback error status. Raise a GenerativeEditableFallbackError instead.
background = page.text_clean_background or page.base_clean_background or page.chosen_background
if not background:
raise GenerativeEditableFallbackError(
"text_clean_background fallback requires a cleaned background artifact"
)| or getattr(profiles, "prompt_model", None) | ||
| or getattr(profiles, "VLM", None) | ||
| ) | ||
| required = (text_profile, profiles.image_model) |
There was a problem hiding this comment.
For safer defensive programming, use getattr to access image_model on the profiles object. This prevents potential AttributeError exceptions if profiles is not a fully-formed Pydantic model or is a mock object during testing.
image_profile = getattr(profiles, "image_model", None)
required = (text_profile, image_profile)
Summary
run_real_generative_editable_pptx.py --mode vlm_first --fallback-policy raster_pptxto produce fallback artifacts instead of failing before report generation.Root Cause
The VLM-first export path only converted validation failures into fallback output. Provider failures bypassed
finalize_validated_export, so explicitraster_pptxfallback requests still failed without producing a PPTX. The real-test runner had the same gap in its defaultvlm_firstpath.Validation
/Users/lzj/.cache/codex-runtimes/codex-primary-runtime/dependencies/python/bin/python3 -m pytest tests/test_generative_editable_export_route.py tests/test_real_generative_editable_runner.py -q->74 passed in 44.61s/Users/lzj/.cache/codex-runtimes/codex-primary-runtime/dependencies/python/bin/python3 -m pytest tests/test_generative_editable_export_contract.py -q->12 passed in 16.66sgit diff --check-> passed