Skip to content

fix: skip image enrichment without pages - #3875

Merged
dolfim-ibm merged 1 commit into
docling-project:mainfrom
DebadityaHait:fix/simplepipeline-chart-enrichment
Jul 29, 2026
Merged

dolfim-ibm merged 1 commit into
docling-project:mainfrom
DebadityaHait:fix/simplepipeline-chart-enrichment

Conversation

@DebadityaHait

Copy link
Copy Markdown
Contributor

Issue resolved by this Pull Request:
Resolves #3867

Description

Skip image enrichment for native charts that have neither an embedded image nor page images, preventing SimplePipeline from indexing an empty page list. The chart remains in the converted document.

Checklist:

  • Documentation has been updated, if necessary.
  • Examples have been added, if necessary.
  • Tests have been added, if necessary.

Tests:

  • uv run pytest tests/test_backend_pptx.py -q
  • uv run ruff format --check docling/models/base_model.py tests/test_backend_pptx.py
  • uv run ruff check docling/models/base_model.py tests/test_backend_pptx.py

Signed-off-by: debaditya <debaditya2005hait@gmail.com>
@github-actions

Copy link
Copy Markdown
Contributor

✅ DCO Check Passed

Thanks @DebadityaHait, all your commits are properly signed off. 🎉

@mergify

mergify Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Merge Protections

🟢 Merge protection satisfied — ready to merge.

Show 1 satisfied protection

🟢 Enforce conventional commit

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

  • title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.+\))?(!)?:

@dolfim-ibm dolfim-ibm left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this PR is solving the reported issue. PPTX still has pages but no page image.

@DebadityaHait can you please verify this?

Copy link
Copy Markdown
Contributor Author

Verified. PPTX produces one logical page in result.document.pages, but SimplePipeline does not populate the separate ConversionResult.pages list used by prepare_element. On the chart fixture, document_pages=1 while conversion_result_pages=0, and the chart has no embedded image. The pre-fix regression fails at conv_res.pages[0]; with this guard the conversion succeeds and retains the chart. I’m happy to make that distinction more explicit in the test if you prefer.

@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@dolfim-ibm dolfim-ibm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@dolfim-ibm
dolfim-ibm merged commit 00acb59 into docling-project:main Jul 29, 2026
26 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.

IndexError in prepare_element for PictureItem with non-empty prov but no embedded image (charts in PPTX)

2 participants