Repository navigation
fix(docx): read the active branch of a body-level mc:AlternateContent - #4615
nikhilcrypto0 wants to merge 2 commits into
Conversation
The body walk skipped mc:AlternateContent, so every paragraph in the supported mc:Choice branch was lost. Walk one branch only: the first Choice whose Requires prefixes map to supported namespaces, else the Fallback. This keeps identical paragraphs inside the Choice and never reads both branches. Fixes docling-project#4611 Signed-off-by: nikhilcrypto0 <nikhilmittu1232@gmail.com>
|
✅ DCO Check Passed Thanks @nikhilcrypto0, all your commits are properly signed off. 🎉 |
Merge Protections🟢 Merge protection satisfied — ready to merge. Show 1 satisfied protection🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
dolfim-ibm
left a comment
There was a problem hiding this comment.
Body-level AlternateContent holding a text box now emits the text twice.
The text-box detection at msword_backend.py:1123-1180 runs on the AlternateContent element with descendant XPaths before the new elif, then _walk_linear(branch) re-detects the same w:txbxContent on the Choice's child w:p. Input: body-level AlternateContent, Choice = w:p with wps:txbx → TEXTBOX_TEXT, Fallback = w:p with v:textbox → TEXTBOX_TEXT.
- main:
['before', 'TEXTBOX_TEXT', 'after'] - this PR:
['before', 'TEXTBOX_TEXT', 'TEXTBOX_TEXT', 'after']
Dispatching AlternateContent before the textbox/image detection (and skipping the rest of the loop body) avoids this; please add that case to the tests. (Note: on main the same detection already reads text boxes from both branches; this change makes it visible.)
Minor: Requires="wpg" / w15 fall back to mc:Fallback since only _OOXML_NAMESPACES counts as supported. That is fine, but worth a one-line comment.
The body-level mc:AlternateContent branch ran after the text-box and image detection, which looks at descendants. That detection read the whole element, and then again on the chosen branch's own paragraph, so a text box in the Choice came out twice. Dispatch mc:AlternateContent first, before that detection, and skip the rest of the loop body for it. Add a test with a text box in both branches, and one for a block with no usable branch. Note in the helper that namespaces outside _OOXML_NAMESPACES, such as wpg or w15, yield to the Fallback. Signed-off-by: nikhilcrypto0 <nikhilmittu1232@gmail.com>
|
Thanks, you are right. Fixed in 9522f87. I reproduced your input first. On the previous commit it gave The empty item in that list is how a paragraph that anchors a text box already comes out outside Also in this push:
|
The DOCX body walk skipped a body-level
mc:AlternateContent, so every paragraph in its supportedmc:Choicebranch was lost. This change walks one branch only: the firstmc:ChoicewhoseRequiresprefixes map to namespaces the backend supports, otherwisemc:Fallback. Both branches are never read, so identical paragraphs inside the Choice are kept.Issue resolved by this Pull Request:
Resolves #4611
This only covers the body-level case. The duplicate-paragraph loss inside text boxes (#4604) is handled in #4606 and is not touched here.
Checklist:
Review evidence:
Before: a body-level
mc:AlternateContentproduced only the paragraphs before and after it. The Choice paragraphs, including two identical ones, were missing.After: the supported Choice paragraphs are present, identical paragraphs both stay, and the Fallback text is absent. If the Choice requires an unsupported namespace (for example
w16se), the Fallback is read instead.Checks run (from a clean
uv sync --frozen --group dev --all-extras):uv run pytest tests/test_backend_msword.py -k body_level_alternate_content: the two new tests fail onmainwithout the fix and pass with it.uv run pytest tests/test_backend_msword.py: 73 passed, 1 skipped.uv run ruff checkanduv run ruff format --checkon the two changed files: clean.Not run:
mypy, the full test suite, andprekhooks. No reference ground-truth data changed. The new tests build the DOCX in code, so no new test file was added.