Repository navigation
Conversation
HTMLTextSerializer added the <p> or <hN> tag first and applied the item's formatting and hyperlink afterwards, so a bold paragraph was exported as <strong><p>...</p></strong>. A paragraph or a heading inside a phrasing element such as <strong> or <em> is invalid HTML. Bold-only rich table cells go through the same path and gave <th><strong><p>...</p></strong></th>. Apply the formatting and the hyperlink before adding the block tag in the heading and paragraph branches, as the list item branch already does. Formulas, code and text in an inline group are unchanged. The HTML ground truth of polymers, rich_table and inline_and_formatting and the chunks of test_chunk_rich_table_custom_serializer change only in the order of these tags. Signed-off-by: Yashvardhan Thanvi <[email protected]>
|
✅ DCO Check Passed Thanks @Thanatos9404, 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✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ceberam
left a comment
There was a problem hiding this comment.
Thanks @Thanatos9404 for the fix and for the thorough description. The change does what it claims for paragraphs, headings and rich table cells.
There are some minor comments that would be good to address before we merge the PR.
Format the item's own text in one place, right after it is escaped, for every text item except code, formulas and list items, instead of repeating the post_process call in the heading and paragraph branches. List items keep their own call, as on main. The HTML output is the same as in the previous commit. Drop test_html_ground_truth_has_no_inline_element_around_a_block: it checks the files in tests/data, not the serializer. Signed-off-by: Yashvardhan Thanvi <[email protected]>
ceberam
left a comment
There was a problem hiding this comment.
Thanks for addressing all the comments 🚀
docling-core 2.103.0 puts formatting tags inside the <p> element instead of around it (docling-project/docling-core#879). Signed-off-by: Daniel Nguyen <[email protected]>
* feat(calendar): add an iCalendar parser and renderer Read RFC 5545 content lines into a component tree with the standard library only, make dates, durations and recurrence rules readable, and add events, tasks, journal entries and free/busy times to a DoclingDocument. The renderer takes a parent and a heading level, so another backend can reuse it. Signed-off-by: Daniel Nguyen <[email protected]> * feat(calendar): add the iCalendar backend and format detection Add InputFormat.CALENDAR for .ics, .ical, .icalendar and .ifb files, CalendarDocumentBackend with its format and backend options, and a content check on the first line, so a calendar without a known extension is recognized. Folded lines are joined on the bytes before decoding, because a fold may split a UTF-8 character. Signed-off-by: Daniel Nguyen <[email protected]> * test(calendar): add calendar fixtures, reference data and tests Synthetic Google, Outlook and Apple style calendars with reference outputs, plus tests for folding, line endings, encodings, recovery from a missing END, all-day and duration handling, recurrence rules, alarms and format detection. Signed-off-by: Daniel Nguyen <[email protected]> * docs: list iCalendar files among the supported formats Signed-off-by: Daniel Nguyen <[email protected]> * build(deps): require docling-core 2.103.0 for the calendar MIME types docling-core 2.103.0 accepts text/calendar in DocumentOrigin without a system MIME table. Raise the minimum in the three pins and lock 2.103.1; only the docling-core entries of uv.lock change. Signed-off-by: Daniel Nguyen <[email protected]> * test(calendar): check conversion without a host MIME table Signed-off-by: Daniel Nguyen <[email protected]> * fix(json): read local images explicitly when enable_local_fetch is set docling-core 2.103.0 no longer reads local image files unless the caller opts in, so a JSON input converted with enable_local_fetch=True kept its file references but failed on the first image export. Load them with DoclingDocument.load_local_images(), the opt-in docling-core provides. Signed-off-by: Daniel Nguyen <[email protected]> * test(docx): update the HTML reference for docling-core 2.103 docling-core 2.103.0 puts formatting tags inside the <p> element instead of around it (docling-project/docling-core#879). Signed-off-by: Daniel Nguyen <[email protected]> * refactor(calendar): use built-in union syntax and add docstrings Use `X | None` and `A | B` instead of typing.Optional and typing.Union in the new calendar modules, annotate CalendarDocumentBackend.unload, and add Google-style docstrings to the public classes and functions. Signed-off-by: Daniel Nguyen <[email protected]> * docs: shorten the iCalendar format row and detail it in the overview Signed-off-by: Daniel Nguyen <[email protected]> --------- Signed-off-by: Daniel Nguyen <[email protected]>
HTMLTextSerializeradds the<p>or<hN>tag of a text item first and applies the item's formatting and hyperlink afterwards, so a bold paragraph is exported as<strong><p>Bold paragraph</p></strong>.<strong>,<em>,<u>,<sub>and<sup>only accept phrasing content, so a paragraph or a heading inside them is invalid HTML.Resolves #874
Root cause
In
HTMLTextSerializer.serialize, the heading branch and the regular text branch wrap the text in<hN>or<p>, and only after that the shared "Apply formatting and hyperlinks to the parent's own text+tag" step callsdoc_serializer.post_process. The list item branch already callspost_processbefore it adds<li>, so list items were not affected. A bold-only table cell is aRichTableCellthat is serialized through the same text path, which gave<th><strong><p>Bold cell</p></strong></th>.Change
<p>or<hN>tag. List items already did this in their own branch (after review, eb6487f).<p><a href="..."><strong>...</strong></a></p>, the same order as for list items.Behavior on a docx like the one in the issue (one bold paragraph, and a table whose first cell is bold), converted with docling 2.138.0:
<strong><p>Bold paragraph</p></strong>and<th><strong><p>Bold cell</p></strong></th><p><strong>Bold paragraph</strong></p>and<th><p><strong>Bold cell</strong></p></th>Ground truth
Regenerated with
DOCLING_GEN_TEST_DATA=1. Each file changes in one line, and only in the order of the tags (same text, same tags):tests/data/doc/polymers.gt.html:<strong><p>Common migration testing methods</p></strong>becomes<p><strong>Common migration testing methods</strong></p>.tests/data/doc/rich_table.gt.html: the italic rich cell,<em><p>text in italic</p></em>becomes<p><em>text in italic</em></p>.tests/data/doc/inline_and_formatting.gt.html:<em><h1>Whole heading is italic</h1></em>becomes<h1><em>Whole heading is italic</em></h1>.tests/data/chunker/0c_out_chunks.json(test_chunk_rich_table_custom_serializer, which uses the HTML serializer): the same italic rich cell.Downstream, docling's
tests/data/docx/groundtruth/docx_rich_cells.docx.htmlhas the old tag order and will need regenerating when docling picks up this change.Tests
New tests in
tests/test_serialization.py:test_html_text_formatting_inside_paragraph: bold, italic, underline, strikethrough, subscript, superscript, a hyperlink, and bold with italic and a hyperlink.test_html_heading_formatting_inside_heading_tag: an italic title and a bold section header with a hyperlink.test_html_rich_table_cell_formatting_inside_paragraph: bold-only rich cells in a header row (<th>) and in a body row (<td>).All 10 test cases fail on
mainand pass with this change.Results (Windows 11, Python 3.10.20,
uv sync --frozen --all-extras --all-packages):uv run pytest tests/test_serialization.py tests/test_hierarchical_chunker.py tests/test_hybrid_chunker.py tests/test_plain_text_serialization.py tests/test_latex_serialization.py: 255 passed, 1 xfailed.uv run pytest tests: 701 passed, 287 failed (on eb6487f). The same 287 tests fail onmainon this machine (DocLang Schematron validation through Java,/tmppaths and Windows path separators); none of them is an HTML serializer test.uv run pytest packages/dclq/tests: 10 passed.uv run pre-commit run <hook> --files <changed files>forruff-check,ruff-formatandmypy: passed. Thedocshook leavesdocs/DoclingDocument.jsonunchanged apart from line endings, anduv lock --checkpasses.