Skip to content

fix(docx): ignore XML comments and processing instructions - #4627

Closed
taran-dev4u wants to merge 1 commit into
docling-project:mainfrom
taran-dev4u:fix/docx-skip-xml-comments
Closed

taran-dev4u wants to merge 1 commit into
docling-project:mainfrom
taran-dev4u:fix/docx-skip-xml-comments

Conversation

@taran-dev4u

Copy link
Copy Markdown
Contributor

Closes #4583.

In MsWordDocumentBackend, XML comments and processing instructions (such as docx4j's <!-- Created by docx4j --> banner or comments in paragraphs and table cells) caused etree.QName to raise ValueError because non-element lxml nodes do not have string tags. When present inside table cells, equations and rich-cell checks also raised TypeError or AttributeError, causing tables to be dropped as broken.

This adds isinstance(node.tag, str) guards across element iteration paths in the backend so comments and processing instructions are skipped cleanly, and adds a regression unit test covering document body, paragraph, run, table row, and cell locations.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

✅ DCO Check Passed

Thanks @taran-dev4u, all your commits are properly signed off. 🎉

@mergify

mergify Bot commented Oct 7, 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)(?:\(.+\))?(!)?:

@taran-dev4u
taran-dev4u marked this pull request as ready for review October 7, 2026 02:30
@codecov

codecov Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 57.14286% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
docling/backend/msword_backend.py 57.14% 6 Missing and 6 partials ⚠️

📢 Thoughts on this report? Let us know!

Signed-off-by: Taran Mamidala <mamidalataran6333@gmail.com>
@taran-dev4u
taran-dev4u force-pushed the fix/docx-skip-xml-comments branch from 5ea931c to eb6131f Compare October 7, 2026 22:13
@github-actions github-actions Bot added ai:ci-safe AI triage found no CI-safety concern (advisory) ai:possible-duplicate AI triage: another PR may fix the same problem bug Something isn't working docx issue related to docx backend labels Oct 7, 2026
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

AI triage (advisory)

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

This PR adds type checks to skip XML comments and processing instructions during DOCX element iteration, and adds a regression test.

Topic labels added: bug, docx.

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

Possible duplicates

PR State Signal Assessment
#4584 open also references issue #4583 duplicate: Both PRs fix issue #4583 by handling XML comments and processing instructions in DOCX files.
2 related pull request(s) in the same code
PR State Signal Assessment
#4649 open changes the same code related: Both PRs modify docx element traversal, but PR 4649 adds support for body-level mc:AlternateContent choice branches.
#4622 open changes the same code related: Both PRs modify msword_backend.py, but PR 4622 preserves w:sym symbols and maps Symbol font characters.

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

@taran-dev4u

Copy link
Copy Markdown
Contributor Author

Closing in favor of #4584 which routes XML comments to the NOTES layer per maintainer feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai:ci-safe AI triage found no CI-safety concern (advisory) ai:possible-duplicate AI triage: another PR may fix the same problem bug Something isn't working docx issue related to docx backend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DOCX conversion fails or drops table text when document.xml contains an XML comment

1 participant