feat(mhtml): add MHTML input backend - #4184
Conversation
|
✅ DCO Check Passed Thanks @egorhowtocode, all your commits are properly signed off. 🎉 |
Merge Protections🟢 All 2 merge protections satisfied — ready to merge. Show 2 satisfied protections🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 Require two reviewer for test updatesWhen test data is updated, we require two reviewers
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
ceberam
left a comment
There was a problem hiding this comment.
Thanks @egorhowtocode for taking the initiative and suggesting a PR to address a quite old feature request.
You suggest adding a separate declarative backend that internally creates a second one, a subclass of HTMLDocumentBackend, and delegates convert() to it. This is a two-layer design.
Looking at how we have created declarative backends in Docling, the right model here would be to extend HTMLDocumentBackend directly and override supported_formats() to return {InputFormat.HTML, InputFormat.MHTML}, exactly how other backends accept multiple extensions. After all, MHTML is HTML: it is just HTML with embedded resources packed into a MIME envelope.
The most natural structure would be a single HTMLDocumentBackend that, when the format is MHTML, pre-processes the MIME envelope in __init__ (extracting the HTML bytes and building the resources dict), then proceeds with the existing convert() path but with its image resolver redirected. There would be no need for a second internal InputDocument, no second backend instantiation, and no private _MhtmlHTMLDocumentBackend class at all.
The current design introduces unnecessary indirection and complexity:
- Two backend classes where one would do.
- An
_MhtmlHTMLDocumentBackendwithset_archive_resources()that must be called immediately after construction (a fragile two-phase init pattern). - A new
InputDocumentandin_doc._backendaccess (inspecting a private attribute) insideconvert(), which is brittle. - A new
MHTMLFormatOptionindocument_converter.pythat is structurally identical toHTMLFormatOptionminus itsbackend_options_for_inputoverride.
There are other specific issues that I have seen in the PR (_load_image_data dead code, cid lookup redundancy, ruff formatting failure,...), but I would prefer that we first agree on the design (a single backend vs 2 backends) before entering into implementation details. What do you think?
3c3facb to
6243742
Compare
|
Thanks @ceberam for the detailed review. I agree that the previous two-layer design was unnecessarily complex, and I have reworked the PR around Docling’s existing declarative-backend model. The updated change is rebased onto the current
The tests cover Validation completed:
For context, I am an active Docling user and have encountered several unsupported formats relevant to my work, including MHTML. I opened this PR to help bring attention and a possible implementation to this feature request. I also want to be transparent that the implementation was produced through an LLM-driven workflow based on my requirements, examples, and validation criteria. I can test the resulting behavior, incorporate concrete review feedback, and address localized implementation problems. However, I do not have sufficiently deep familiarity with Docling’s internal architecture to lead a broader architectural redesign without maintainer guidance. For this revision, I iterated specifically against your architectural guidance, expanded tests, and a separate review of the resulting diff. If the revised implementation is generally sound but needs minor corrections or additional tests, I will be happy to address them. If it still has fundamental architectural or logical problems, I would also be completely comfortable with this PR being superseded by an implementation from a maintainer or another contributor. My main goal is to help get the missing feature resolved, rather than to retain ownership of the implementation. |
|
Thanks @egorhowtocode for the refactoring! The design is now much more robust. |
Signed-off-by: Egor Ivanov <ivanovegor1648@gmail.com>
…ration Signed-off-by: Cesar Berrospi Ramis <ceb@zurich.ibm.com>
Signed-off-by: Cesar Berrospi Ramis <ceb@zurich.ibm.com>
…limitation Signed-off-by: Cesar Berrospi Ramis <ceb@zurich.ibm.com>
Signed-off-by: Cesar Berrospi Ramis <ceb@zurich.ibm.com>
Signed-off-by: Cesar Berrospi Ramis <ceb@zurich.ibm.com>
Signed-off-by: Cesar Berrospi Ramis <ceb@zurich.ibm.com>
6243742 to
d9a3e7a
Compare
ceberam
left a comment
There was a problem hiding this comment.
@egorhowtocode we finally released docling-core and we can now properly handle the MIME types for MHTML.
I have added a commit to address this point and others minor wants that I had identified to strengthen this PR.
I think we are ready to merge this new feature 🚀
Resolves #659.
Adds a thin MHTML adapter based on Python's standard-library MIME parser.
The backend selects the root HTML part, resolves embedded raster images from
Content-Location and cid: MIME parts without network/filesystem access, then
delegates document semantics to HTMLDocumentBackend.
The design follows docling.rs's established MHTML architecture while adapting
to Python Docling's backend and safety conventions.
Tests cover .mhtml/.mht detection, root/start selection, nested MIME,
quoted-printable and base64 HTML, Unicode, HTML structure, Content-Location
and cid images, missing/unsupported images, text-only/unusable input,
path/stream input, origin preservation, image-size limits, and no outbound
network access.