add --type comment doesn't sync commentsExtended.xml/commentsIds.xml/commentsExtensible.xml unless done/parentId is passed #429

Activity

1. argszero commented on Sep 23, 2026

   

   argszero

   on Sep 23, 2026

   Contributor

   More actions

   Confirmed on 1.0.152 (macOS arm64, ffa8a0af, officecli.dll sha256 8879524487282769…) — so this is not limited to 1.0.146/Windows.

   Measured, counting entries per part in the zip after each step:

   create; add /body --type paragraph
   add /body/p[1] --type comment --prop text=first --prop author=Alice
   set /comments/comment[1] --prop done=true
     -> comments.xml=1  commentsExtended.xml=1  commentsIds.xml=absent  commentsExtensible.xml=absent
   
   add /body/p[1] --type comment --prop text=second --prop author=Bob    # no done, no parentId
     -> comments.xml=2  commentsExtended.xml=1   <-- one entry short
   

   So the second add leaves the part that already exists one entry behind comments.xml, exactly as reported. Each part is individually schema-valid, which is why validate is silent — the inconsistency is cross-part, not per-part.

   The workaround also reproduces, and localising it points straight at the cause:

   add ... --prop text=second --prop author=Bob --prop done=false
     -> comments.xml=2  commentsExtended.xml=2      (entry: <w15:commentEx w15:paraId="00100004" w15:done="false" />)
   

   Cause

   Handlers/Word/WordHandler.Add.Misc.cs upserts the w15:commentEx only inside two conditional branches — one for parentId, one for done/resolved. Pass neither and no entry is written at all. The comment sitting directly above those branches states the invariant the code then doesn't implement:

   // Word writes a commentEx for every comment; ensure the parent's
   // thread-root entry exists, then link this reply to it.

   UpsertCommentEx (WordHandler.CommentsExt.cs:50) is already find-or-create and already defaults new entries to Done=false "to mirror Word's w15:done="0" output", and EnsureCommentsExRoot already handles creating the part — so the mechanism is all there; the add path just doesn't call it unless a prop asks.

   What I'd change

   Make the upsert unconditional for the newly added comment, after the existing branches:

   if (string.IsNullOrEmpty(commentBody.ParagraphId?.Value)) AssignParaId(commentBody);
   UpsertCommentEx(commentBody.ParagraphId!.Value!, null, null);

   Because the upsert is find-or-create, the parentId and done branches keep working unchanged (the done branch's value still wins — it is applied after). This makes the entry appear whether the part already exists (the case in your repro, and the one that matters for files Word produced) or not.

   Not in that change

   commentsIds.xml and commentsExtensible.xml have no writer in the add path at all — the only code that touches them is the dump/raw round-trip in WordBatchEmitter.AuxParts.cs, which passes them through verbatim. Writing them means plumbing a part that has no props and never gets created, including a durable-id for w16cid:commentId, which is a design decision rather than a missing call. I've scoped that as a separate change rather than bundling it, per the repo's Rule 1. Your note that Word's own repair reconciles commentsIds.xml but drops commentsExtensible.xml is useful there — it suggests the two shouldn't be treated as symmetric.

   I'll open a PR for the commentsExtended.xml half with a before/after on the counts above.
2. goworm commented on Sep 24, 2026

   

   goworm

   on Sep 24, 2026

   Contributor

   More actions

   Reproduced exactly as described (on 1.0.152 too), and fixed on main for the next release.

   Rule now: every comment sidecar part that already exists in the document is kept in step — add --type comment appends the comment's entry to commentsExtended.xml (w15:commentEx, done=false), commentsIds.xml (w16cid:commentId with a fresh 8-hex durableId) and commentsExtensible.xml (w16cex:commentExtensible with the same durableId and a whole-second dateUtc, as Word writes) — with or without done/parentId. Removing a comment (directly, or via the orphan cleanup when its host paragraph goes) drops its entry from all three. A part that does not exist is never created, so a document that never had them stays minimal.

   Verified with a Word-shaped fixture (all four parts present): 2/2/2/2 → add → 3/3/3/3 → remove → 2/2/2/2, and desktop Word opens the result. Thanks for the precise repro, and for the note about Word's own repair dropping commentsExtensible — that made the case for keeping the third part in step rather than leaving it to recovery.