[ryanmeowy]

Issue resolved by this Pull Request:
Resolves #4682

Checklist:

• Documentation has been updated, if necessary. (Not needed. This is a bug fix.)
• Examples have been added, if necessary. (Not needed.)
• Tests have been added, if necessary. An end-to-end check in tests/test_backend_html.py and one in tests/test_backend_mhtml.py; a unit test for the truncated-image path, carried over from fix(html): keep CMYK images and skip undecodable ones instead of failing #4681 by @usmanmateen, lives in tests/test_image_resource_loader.py.

Review evidence:

Behavior before. A picture in CMYK, LAB or F mode raised OSError: cannot write mode <mode> as PNG inside ImageRef.from_pil(). The shared ImageResourceLoader.create_image_ref() did not catch OSError, so the error escaped and aborted the whole conversion for the HTML, Markdown, AsciiDoc and JATS backends. The MHTML branch caught the error and dropped the picture. On the JATS backend the error also stopped the body walk, so the conversion lost all body content after the figure and still reported success.

Behavior after. The loader normalizes the picture first, as the Office backends already do. The except tuple also catches OSError. CMYK, LAB and F pictures are now kept and converted to RGB, through the embedded ICC profile when one is present. A picture that cannot be decoded no longer fails a conversion. The backend keeps the picture placeholder and emits a warning.

Checks.

Check	Command	Result
New tests	pytest tests/test_backend_html.py tests/test_backend_mhtml.py tests/test_image_resource_loader.py	148 passed, 1 skipped
Shared-loader consumer suites	pytest tests/test_backend_msword.py tests/test_backend_msword_lists.py tests/test_backend_msword_outline.py tests/test_backend_msword_spacer.py tests/test_backend_opendocument.py tests/test_office_image_modes.py tests/test_backend_image_native.py tests/test_api_image_request.py	386 passed, 1 skipped
Format and lint	ruff format --check . and ruff check .	All checks passed
Type check	ty check	606 diagnostics, same as main (line shifts only)

Not run: conversion of real print-scanned document corpora, and the remote-fetch path of the loader (the test URL allowlist blocks it).

[github-actions]

✅ DCO Check Passed

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

[mergify]

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)(?:\(.+\))?(!)?:

[github-actions]

AI triage (advisory)

Checked commit 5e46d36d59. The analysis read the diff only. It did not run code from this pull request.

This pull request prevents conversion failures when processing pictures with unsupported Pillow modes, such as CMYK, LAB, or F, or when processing truncated image data. It normalizes image modes to PNG-compatible formats and catches decoding exceptions in ImageResourceLoader.

CI safety: no concern found. A maintainer must still approve CI.

1 related pull request(s) in the same code

PR	State	Signal	Assessment
#4681	closed	also references issue #4682; referenced as #4681	related: This candidate is closed and not merged. The current pull request incorporates and extends the fixes and tests from this candidate.

_{The ai:* labels are advisory. They do not approve CI and they do not add tests:full. A new push runs this check again.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[ceberam]

@ryanmeowy The PR #4681 , which addresses issue #4680 , was open earlier on this same topic.
Can you explain how different your PR is from #4681 . In particular, does it resolve the issues described in #4682 and in #4680 ? Are there gaps that are not covered in your PR? Does it provide a better implementation in terms of code quality and pythonic patterns, test coverage and robustness, maintainability and complexity?

[ryanmeowy]

Thanks @ceberam, and thanks @usmanmateen — I hadn't seen #4681 when opening this one; we clearly hit the same root cause independently.

How it differs: the two hunks in image_resource_loader.py are identical (same resulting content). This PR additionally normalizes the embedded-MHTML branch in html_backend.py (_create_image_ref), which does not go through the loader — for images embedded in the MHTML archive itself, from_pil is called directly, so with only the loader change a CMYK image embedded in MHTML is still dropped with a warning instead of converted. That path is why #4682 exists as a separate report.

Does it resolve #4682 and #4680? Yes to both. #4680's repro (CMYK + truncated JPEG in HTML/Markdown) succeeds here. #4682 additionally documents the MHTML drop (fixed by the html_backend.py hunk) and the JATS silent truncation (an escaping exception aborts the rest of the document body — fixed here since the loader no longer raises; covered end-to-end).

Gaps: the La PNG mode still degrades to warn+drop (Pillow cannot write La to any raster format, so unreachable via real files); SVG resources untouched (never reach PIL); remote-fetch behavior unchanged.

Tests: #4681's are unit-level (direct loader calls); ours run end-to-end through DocumentConverter across HTML, Markdown, AsciiDoc, JATS and MHTML, which is what catches the MHTML wiring that loader unit tests cannot reach.

Happy to defer to #4681 since it was opened first — two ways: (a) I close this and move the MHTML fix + end-to-end tests into a follow-up PR on top of it, or (b) if you prefer a single PR, this one is a strict superset of #4681. Either works for me; I only care that the MHTML path doesn't stay broken.

[ceberam]

Thanks @ryanmeowy for your review.
Even though your issue and PR was opened later (with few minutes of difference), the completeness argument is here more relevant.
We closed #4681 and we will review this PR instead.

[ryanmeowy]

Thanks @usmanmateen — your cross-review made this easy for everyone: verifying the MHTML gap against your own branch is exactly the diligence that settles a duplicate. Carrying over test_create_image_ref_skips_undecodable_image as you offered, with attribution in the commit. And thanks @ceberam for the quick triage.

[ceberam]

Thanks @ryanmeowy for the PR. The fix is correct, the code is clean, and the tests are meaningful. Please address the suggestions below, plus a minor inconsistency note.

[ryanmeowy]

Hi @ceberam — sorry to ping you directly, and apologies if this is a bad time.

Just a gentle bump on #4683. I think I've covered everything from your review: I dropped the provenance comment, added the explicit img.load() in create_image_ref, and moved the tests into test_backend_html.py and test_backend_mhtml.py. CI is green and the review threads are resolved. No rush at all — whenever you have a moment to take another look, I'd appreciate it.

Small extra ask, only if it's easy: #4745's latest commit is stuck in action_required, so the CI workflows need a maintainer to approve them before they will run. If you happen to be passing by, could you hit "Approve and run" on that one?

Thanks, and sorry again for the nudge.

[ceberam]

Thanks @ryanmeowy for the heads-up. It all seems good but there are actually some conflicts after we merged some PRs recently. Could you please rebase to main and address them?

[ryanmeowy]

@ceberam Rebased onto main and resolved the conflicts — 83a84e9a. Thanks!

[PeterStaar-IBM]

@ryanmeowy Can you fix the merge conflict?

[ryanmeowy]

@PeterStaar-IBM Fixed — rebased onto main again (5e46d36d). The conflict was in tests/test_backend_html.py with #4912; both tests are kept. CI should pick it up shortly.