Skip to content

fix(pptx): ignore empty llm captions - #1886

Merged
afourney merged 4 commits into
microsoft:mainfrom
Lubrsy706:fix/pptx-none-llm-description
Sep 3, 2026
Merged

fix(pptx): ignore empty llm captions#1886
afourney merged 4 commits into
microsoft:mainfrom
Lubrsy706:fix/pptx-none-llm-description

Conversation

@Lubrsy706

Copy link
Copy Markdown
Contributor

Summary

  • Skip empty/None LLM image captions when building PPTX image alt text
  • Preserve embedded deck alt text and shape-name fallback behavior
  • Add regression coverage for an LLM caption provider returning None

Fixes #1534

Tests

  • PYTHONPATH=packages/markitdown/src /Users/bytedance/MySpace/open-source/markitdown-excel-currency/.venv/bin/python -m pytest packages/markitdown/tests/test_module_misc.py::test_pptx_converter_treats_none_llm_caption_as_empty
  • PYTHONPATH=packages/markitdown/src /Users/bytedance/MySpace/open-source/markitdown-excel-currency/.venv/bin/python -m compileall -q packages/markitdown/src/markitdown/converters/_pptx_converter.py packages/markitdown/tests/test_module_misc.py
  • git diff --check

@Lubrsy706

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Whitespace-only captions must preserve the shape-name fallback, and the regression assertion should verify the intended output.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Prevents PPTX conversion failures when LLM image captioning returns None.

Changes:

  • Filters missing LLM captions when composing image alt text.
  • Adds regression coverage for None captions.
File summaries
File Description
packages/markitdown/tests/test_module_misc.py Adds regression coverage, but the assertion does not verify preservation of embedded alt text and uses the deprecated result alias.
packages/markitdown/src/markitdown/converters/_pptx_converter.py Handles None captions, but whitespace-only captions can bypass the shape-name fallback.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/markitdown/src/markitdown/converters/_pptx_converter.py Outdated
Comment thread packages/markitdown/tests/test_module_misc.py Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@afourney
afourney merged commit 7003346 into microsoft:main Sep 3, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A bug when llm_description got None response from LLM in PptxConverter

4 participants