Detect orphaned docx comments - #1734

Closes #1733. This adds validation for comments.xml entries that have no corresponding comment marker in document.xml.

Thanks for reviewing this contribution. The validator now reports comment IDs present in comments.xml but missing from document.xml markers, while preserving existing marker/reference checks. I also verified the orphaned-comment case with a focused runtime test.

Thanks for this — it closes a real gap (comments present in comments.xml with no anchor in document.xml currently pass silently).

One correctness concern: comments_root.xpath(".//w:comment") selects all descendant w:comment elements, including nested replies. In threaded comments (Word 2019+ / MS-OI29500), replies are child w:comment elements of the parent w:comment, and they intentionally have no commentRangeStart/commentRangeEnd/commentReference in document.xml — only the parent comment is anchored. With .//, every threaded reply would be reported as comment id=N is not anchored anywhere in document.xml (false positive).

Verified with lxml (two-state check):

• comments.xml: <w:comment w:id="0">…<w:comment w:id="1">reply</w:comment></w:comment>, document.xml markers = {"0"}
• .//w:comment → comment_ids = {0, 1} → orphaned = {1} ✗ false positive
• ./w:comment (direct children only) → comment_ids = {0} → orphaned = {} ✓

Suggested fix: use comments_root.xpath("./w:comment", namespaces=namespaces) so only top-level comments are compared against anchors. Please apply to all three copies (docx, pptx, xlsx skill trees) — the existing reverse check (invalid_refs) is unaffected since it subtracts from marker_ids.

@98zc5g5jyw-arch
Thanks for taking this on — the fix addresses the case I reported. For reference, the repro is the fast agent output in #1733: validate.py --original printed "All validations PASSED!" on a file whose reviewer comment was present in comments.xml and anchored to nothing.

On the threaded-reply concern above: I don't think replies are nested w:comment elements. Per the schema shipped in this repo (scripts/office/schemas/ISO-IEC29500-4_2016/wml.xsd), CT_Comment's content model is EG_BlockLevelElts, which expands to altChunk | customXml | sdt | p | tbl | proofErr | permStart | permEnd | ins | del | moveFrom | moveTo | bookmarkStart/End | commentRangeStart/End | ... — no comment. And w:comment is declared only as a child of CT_Comments. So a <w:comment> inside a <w:comment> isn't schema-valid, and .//w:comment and ./w:comment select the same set on any conformant file. Threading is stored out of band, which is also what this repo's own scripts/comment.py writes (lines 279-290).

I checked it against a Word-authored file rather than leaving it at the schema — Word 16.0, one comment plus one Reply on the same phrase:

comments.xml:  2 x <w:comment>, zero nested   (id=0, id=1)
document.xml:  commentRangeStart ids [0, 1]
               commentRangeEnd   ids [0, 1]
               commentReference  ids [0, 1]
commentsExtended.xml:
               w15:paraId="57B5BAC5"
               w15:paraId="1E34214A" w15:paraIdParent="57B5BAC5"

Both comments are flat siblings, both carry all three markers, and the thread relationship exists only as paraIdParent in commentsExtended.xml. So a reply is anchored like any other comment and the reverse check produces no false positive on threads — with either XPath.

Two smaller notes:

• .//w:comment is pre-existing code in the comment_ids set, not something this PR introduced. I'd still switch it to ./w:comment since it matches the flat structure and costs nothing, but on this evidence it isn't load-bearing.
• marker_ids = range_starts | range_ends | references treats a comment as anchored if any one of the three is present, which is the right call and makes the check tolerant of unusual but conformant anchoring.

The three copies of this validator (docx, pptx and xlsx skill trees) are byte-identical here and this PR patches all three, so that part is already covered.

For what it's worth, the checker I used to find the original issue does the same reverse check with direct children only and produced no false positives across 220 labelled producer pairs from five producers — though that corpus contains no threaded comments, which is why I made the file above.

Fixture-tested this against the #1733 files and a few synthetic cases (head 0410f59 vs main).

The orphan check is correct. agreement.docx has comments {1,2} but markers only {2}. This branch reports:

comments.xml: comment id="1" is not anchored anywhere in document.xml

Main still silent-passes the same file.

The nested-reply false positive is real — but only on schema-invalid XML. Nested <w:comment w:id="1"> inside id=0, markers only for 0:

• .//w:comment → {0,1} → reports id=1 as unanchored
• ./w:comment → {0} → no orphan

./w:comment does not regress the real orphan, healthy comments, or a Word-like flat-sibling thread (both ids anchored).

Nested w:comment is not valid OOXML: CT_Comment is EG_BlockLevelElts, not a child comment. Injecting one already fails XSD. This repo’s comment.py and Word both store replies as flat siblings plus commentsExtended.xml paraIdParent, and those replies do get document.xml markers. On any conformant file the two XPaths select the same set.

I’d still switch all three copies to ./w:comment — it matches the schema, matches the review, and avoids a misleading second error on a file that already fails XSD. Worth adding fixtures for the agreement orphan, a synthetic orphan, the nested hypothetical, and a flat-sibling thread.

Residual, out of scope: markers are collected only from document.xml, so a header-only comment looks unanchored with either XPath.

Confirmed the residual you flagged, and it applies to my checker too — worth stating since I cited its false-positive record above.

Built a document where the only anchor for comment id=1 lives in word/header1.xml (range start, range end and the reference run all inside the header paragraph), with nothing left in document.xml:

[ERROR] CMT005  comment id=1 is orphaned - present in comments.xml but anchored to nothing

Same cause: marker collection reads document.xml only. Word's UI doesn't offer comments in headers, but the schema permits the range markup there and programmatic producers can write it, which is exactly the population these checks exist for. Fixing it on my side and adding a labelled pair for it.

So for this PR I'd agree with your scoping: ./w:comment now, header/footer story coverage separately — it needs the same treatment in every story (headers, footers, footnotes, endnotes), not a one-line change.

Follow-up on the header-only residual: fixed on my side, with the labelled pair.

ooxml-integrity now reads comment ranges and references in every story the main document relates (headers, footers, footnotes, endnotes), not only in document.xml. A range still has to start and end in the same story, and markers in a part the document doesn't relate don't count (Dmitry-Kov/ooxml-integrity#25).

The labelled pairs: comment 1 anchored only in the default header, and only in footnote 1, each with an untouched control and a copy that loses the anchors. The previous checker reported CMT005 on both controls; the fixed one reports nothing there and still reports CMT005 on both damaged copies. The files are synthetic, and I haven't checked how Word displays them. The fix is on main, not yet in a PyPI release.

Nothing changes for this PR: ./w:comment here, story coverage separately. If that follow-up happens, the pairs are MIT-licensed and could serve as fixtures.