fix(read_extract): dedupe DOCX text-box text in _extract_docx - #67853
fix(read_extract): dedupe DOCX text-box text in _extract_docx#67853JiataiWang wants to merge 2 commits into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused regression test and for tracing the duplicate to recursive paragraph traversal. The bug remains on current main at tools/read_extract.py:118-126.
Problems
- The new ownership filter at
tools/read_extract.py:146skipsBOXTEXTwhile the outer paragraph is emitted first (:138), then emits the nested paragraph later. For this PR's fixture (tests/tools/test_read_extract.py:170-174), that changes source order fromBefore → BOXTEXT → AftertoBefore After → BOXTEXT. tests/tools/test_read_extract.py:177-180only checks uniqueness and presence, so it does not catch that reordering.
Suggested changes
- Preserve the fixture's source/document order while eliminating the duplicate, and assert the resulting full output or relative fragment order in the regression test.
Automated hermes-sweeper review.
A Word text box stores its content as <w:p> paragraphs inside
<w:txbxContent>, so paragraphs can nest. root.iter("w:p") yields those
nested paragraphs too, and the ancestor paragraph's iter() also descends
into them — so text-box text was extracted twice (once inline via the
ancestor, once when the nested paragraph was visited on its own).
Attribute each text node to its nearest enclosing <w:p> via a
child->parent map and skip text owned by a nested paragraph. Each <w:p>
now contributes exactly one line and each run of text appears once; no
change for documents without nested paragraphs.
Adds a failing-first regression test.
Fixes NousResearch#67851
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review feedback: the nearest-paragraph attribution emitted a text box's text as its own line *after* the enclosing paragraph (Before After → BOXTEXT), losing XML order. Replace it with a single document-order walk that treats each <w:p> as a line boundary — a nested paragraph flushes the line it interrupts, emits its own line(s), then the outer paragraph resumes — so text stays in reading order (Before → BOXTEXT → After) and is still emitted exactly once. Empty paragraphs remain blank lines. Test now asserts relative order, not just uniqueness. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
23e0195 to
5b5ade3
Compare
|
Rebased onto current |
SummaryTwo open PRs touch OOXML extraction, but only #67853 addresses issue #67851's nested-paragraph duplication: its document-order walk emits text-box content once without reordering it, whereas #61881 adds archive resource limits and leaves paragraph traversal unchanged. Related pull requests
Suggested consolidationKeep #67853 open with a salvage path: retain its focused document-order traversal and uniqueness/order regression test as the best existing fix for #67851 while routing it through normal backlog review. Keep #61881 separate for its archive-hardening scope; it is not a duplicate of #67853. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I67851(["issue #67851 (open)"])
P67853["PR #67853 (open)"]
P67853 -->|best fix| I67851
class I67851 open
class P67853 open
class P67853 best
class P67853 target
click I67851 "https://github.com/NousResearch/hermes-agent/issues/67851"
click P67853 "https://github.com/NousResearch/hermes-agent/pull/67853"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 2 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 31 kB of PR diffs, 5 kB of issue/PR text, 5 kB of discussion (9 comments), 3 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
Fixes #67851.
Problem
read_fileon a.docxcontaining a text box (or any block-level content control) emitted the text-box contents twice.Root cause
A Word text box stores its content as
<w:p>paragraphs inside<w:txbxContent>, so paragraphs can nest._extract_docxdid:ElementTree.iter()walks the whole subtree, so the outer paragraph pulled the box's<w:t>in inline and the nested<w:p>was visited independently — emitting the same text twice.Fix
ElementTree has no parent pointers, so build a
child → parentmap and attribute each text node to its nearest enclosing<w:p>. Text owned by a nested paragraph is skipped in the ancestor and emitted only when that paragraph is visited on its own. Each<w:p>now contributes exactly one line and each run of text appears once.Documents without nested paragraphs are unaffected (the nearest
<w:p>of every text node is its own paragraph, exactly as before).Testing
test_text_box_not_duplicatedfails onmain(AssertionError: 2 != 1) and passes with this change.python -m pytest tests/tools/test_read_extract.py→ 19 passed.🤖 Generated with Claude Code