fix(docx): clamp Word Heading 7-9 to h6 - #2555
stanleys12 wants to merge 2 commits into
Conversation
Mammoth's default style map covers Heading 1-6, so paragraphs styled Heading 7, 8 or 9 fall through as plain text and lose their heading structure. Markdown has no level past 6, so add style map entries that clamp those levels to h6, in both the style id and style name forms the default map uses.
|
@microsoft-github-policy-service agree |
PRABHU KIRAN VANDRANKI (VANDRANKI)
left a comment
There was a problem hiding this comment.
Community review, so this does not clear the merge gate.
I read the full diff and DocxConverter.convert in _docx_converter.py on main. I did not run the PR test file or the markitdown suite. I did test the style-map behavior directly in mammoth.
What I ran: a minimal .docx built in memory with paragraphs styled Heading1 to Heading9 and no styles.xml, converted with mammoth.convert_to_html:
- No style map: levels 1 to 6 give h1 to h6, and levels 7, 8, 9 give plain
<p>. This reproduces the bug on main. - With
p.Heading7/8/9 => h6:fresh: levels 7 to 9 give<h6>. This matches the PR's expected output. - With
p.Heading7 => p:freshplaced before the clamp rules: Level 7 stays<p>and 8 and 9 become<h6>. So earlier rules win, which is what makes the PR's ordering claim true: the clamp is appended aftercaller_style_mapandembedded_style_mapin the join, so a caller or the document's own embedded map can still override it. That matches the third test.
Not verified by me: the p[style-name='Heading 7'] form for localized style ids. The test for it builds a styles.xml, but I did not run that case. I also did not run markitdown-ocr.
Notes:
- Clamping to h6 is a reasonable choice since Markdown has no level 7. It does flatten the hierarchy: 6, 7, 8 and 9 all look the same downstream. That is a trade-off, and the PR text already says so. Worth a line in the docs or a code comment that this is lossy by design.
- The change is only 12 lines in the converter, and the test file is larger than the fix because it builds docx files by hand. That is fine, but if there is an existing docx fixture helper in the tests directory, reusing it would shrink the test.
_DEEP_HEADING_STYLE_MAPusesh6:fresh, the same:freshmodifier the default mammoth map uses, so adjacent headings will not merge. Good.
I think this is correct and low risk.
|
Thanks PRABHU KIRAN VANDRANKI (@VANDRANKI), appreciate you actually running it through mammoth. Agreed on the lossy part, added a note on that to the comment above the style map, including that a caller's or the document's own map still wins. I looked for a shared docx fixture helper to shrink the test but didn't find one in the tests dir, so I left the builder as is. |
|
Small correction to what I said: test_docx_images.py does have its own private |
Word defines Heading styles 1 through 9, but mammoth's default style map stops at Heading 6 and
DocxConverteradds nothing beyond it, so paragraphs styled Heading 7, 8 or 9 come through as plain text and lose their heading structure entirely.A document with one paragraph per heading level converts like this on
main:Nothing warns about it, and downstream the last three levels read as body text rather than as sections. Deep heading levels are common in legal and technical-spec documents, which is the kind of long structured input this library is usually pointed at.
Fix
Markdown has no heading level past 6, so clamp 7-9 to
h6instead of dropping them: a new_DEEP_HEADING_STYLE_MAPis joined into the style map alongside_UNDERLINE_STYLE_MAP. It uses both selector forms that mammoth's default map already uses for levels 1-6:Both are needed. mammoth matches style ids exactly, so the
p.HeadingNform is what catches a document with no stylesheet part; it matches style names case-insensitively, so thestyle-nameform is what catches the non-English style ids a localized Word writes (Titre7,Überschrift7) via<w:name w:val="heading 7"/>. The entries go after the caller-supplied and embedded maps in the existing join, so a caller passingstyle_map=still overrides the clamp, the same way it overrides theu => udefault.After:
Documents that do not use Heading 7-9 are unaffected.
Tests
New file
packages/markitdown/tests/test_docx_headings.py, over nine-heading documents built in memory:style_mapstill outranks the clampAgainst unmodified
main:With the fix:
The third test passes before and after; it is there so the ordering inside the style map join stays overridable.
Verification
packages/markitdown:python -m pytest tests -q-> 998 passed, 14 skippedpackages/markitdown-ocr:python -m pytest -q-> 108 passed (DocxConverterWithOCRreuses the core pipeline, so the clamp reaches it too)black --check packages/with black 23.7.0 -> 107 files unchanged;git diff --checkcleanRun on macOS / CPython 3.13; the CI matrix was not reproduced locally.
Fixes #2530. This is the heading part of #2531, which was closed because it was bundled with unrelated whitespace changes; rebuilt here on its own.
AI assistance (Claude) was used to draft this change; the full
packages/markitdowntest suite (998 passed) andblack --checkwere run locally.