feat: emit word/commentsIds.xml with durable comment ids (w16cid)#3472
feat: emit word/commentsIds.xml with durable comment ids (w16cid)#3472alexvcasillas wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds support for the DOCX ChangescommentsIds.xml support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3472 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 311 311
Lines 3237 3265 +28
Branches 732 739 +7
=========================================
+ Hits 3237 3265 +28 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/file/file.ts (1)
25-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a project path alias for the new import.
The added relative import should follow the repository alias convention for TypeScript imports.
♻️ Proposed fix
-import { CommentsExtended, CommentsIds } from "./paragraph/run/comments-extended"; +import { CommentsExtended, CommentsIds } from "`@file/paragraph/run/comments-extended`";As per coding guidelines, "
**/*.ts: Use path aliases@file/,@export/, and@util/for imports".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/file/file.ts` at line 25, The import statement for CommentsExtended and CommentsIds uses a relative path import pattern instead of the project path alias convention. Replace the relative import path "./paragraph/run/comments-extended" with the corresponding path alias "`@file/paragraph/run/comments-extended`" to follow the repository's TypeScript import guidelines that require using path aliases like `@file/` for imports in this file.Source: Coding guidelines
src/file/file.spec.ts (1)
447-481: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert
CommentsIdsXML shape here, not only object presence.These tests currently pass even if
CommentsIdsserializes the wrong structure. Add at least one assertion on the formatted XML tree (root/tag/attrs) for the positive durableId case.As per coding guidelines, "
**/*.spec.ts: Test the XML output structure, not just that code runs".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/file/file.spec.ts` around lines 447 - 481, The test cases for CommentsIds creation are only validating that the property exists or is undefined, but not asserting the actual XML structure being serialized. For at least one of the positive durableId test cases (either the single comment or threaded comments test), add an assertion that validates the formatted XML output structure by checking the root element, tags, and attributes rather than just verifying the CommentsIds property is not undefined. This will ensure the XML serialization produces the expected structure, not just that the code runs.Source: Coding guidelines
src/export/packer/next-compiler.spec.ts (1)
169-219: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a mixed durable/non-durable integration test for fallback behavior.
Please add a case where at least one comment has
durableIdand another omits it, then assertword/commentsIds.xmlmaps the omitted one to its generatedparaId. That option combination is part of the feature contract and is not covered here yet.As per coding guidelines, "
**/*.spec.ts: Cover edge cases and option combinations in test files".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/export/packer/next-compiler.spec.ts` around lines 169 - 219, Add a new test case using the `it()` block pattern to cover the mixed durable/non-durable comment scenario. Create a File with comments where at least one comment includes a durableId and another omits it, then compile the file using compiler.compile(). Assert that word/commentsIds.xml is generated and contains proper mappings for both types of comments: verify that comments with durableId are mapped with the correct durableId attribute, and verify that comments without durableId are mapped to their generated paraId values. This ensures the fallback behavior when durableId is omitted is properly tested and documented.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/export/packer/next-compiler.spec.ts`:
- Around line 169-219: Add a new test case using the `it()` block pattern to
cover the mixed durable/non-durable comment scenario. Create a File with
comments where at least one comment includes a durableId and another omits it,
then compile the file using compiler.compile(). Assert that word/commentsIds.xml
is generated and contains proper mappings for both types of comments: verify
that comments with durableId are mapped with the correct durableId attribute,
and verify that comments without durableId are mapped to their generated paraId
values. This ensures the fallback behavior when durableId is omitted is properly
tested and documented.
In `@src/file/file.spec.ts`:
- Around line 447-481: The test cases for CommentsIds creation are only
validating that the property exists or is undefined, but not asserting the
actual XML structure being serialized. For at least one of the positive
durableId test cases (either the single comment or threaded comments test), add
an assertion that validates the formatted XML output structure by checking the
root element, tags, and attributes rather than just verifying the CommentsIds
property is not undefined. This will ensure the XML serialization produces the
expected structure, not just that the code runs.
In `@src/file/file.ts`:
- Line 25: The import statement for CommentsExtended and CommentsIds uses a
relative path import pattern instead of the project path alias convention.
Replace the relative import path "./paragraph/run/comments-extended" with the
corresponding path alias "`@file/paragraph/run/comments-extended`" to follow the
repository's TypeScript import guidelines that require using path aliases like
`@file/` for imports in this file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0a3e4fe6-8cf5-418d-877a-b9f9e5ec3498
📒 Files selected for processing (10)
src/export/packer/next-compiler.spec.tssrc/export/packer/next-compiler.tssrc/file/content-types/content-types.tssrc/file/file.spec.tssrc/file/file.tssrc/file/paragraph/run/comment-run.spec.tssrc/file/paragraph/run/comment-run.tssrc/file/paragraph/run/comments-extended.spec.tssrc/file/paragraph/run/comments-extended.tssrc/file/relationships/relationship/relationship.ts
|
Addressed the nitpicks in c74aa32:
Full suite passes ( |
1 participant
Summary
Adds support for the
word/commentsIds.xmlOOXML part, which maps each comment's volatile paragraph id (w14:paraId) to a stablew16cid:durableIdthat Word preserves across edits (unlike the volatilew:idincomments.xml).This mirrors the existing
commentsExtended.xmlfeature end to end. A new optionaldurableIdfield on a comment opts a document into the part: when any comment carries adurableId, the library now generates and registerscommentsIds.xml.Why
Word, and the apps and services that interoperate with it (for example collaborative editors that round-trip DOCX), rely on
w16cid:durableIdto track comment identity stably across save and edit cycles. Without it, comment ids can be reassigned and external systems lose the ability to reconcile a comment with its original.docxalready emitscomments.xmlandcommentsExtended.xml, but had no way to write the durable-id part.What changed
Public API
ICommentOptionsgains an optionalreadonly durableId?: string. When it is omitted on a comment that lives in acommentsIds-bearing document, the comment's generatedparaIdis reused as itsdurableId(matching how Word emits the part).CommentsIdscomponent (sibling ofCommentsExtended).ICommentIdDatatype (sibling ofICommentThreadData).File#CommentsIdsgetter (sibling ofFile#CommentsExtended).OOXML parts produced
w16cid:commentsIdswith namespaces:xmlns:w16cid="http://schemas.microsoft.com/office/word/2016/wordml/cid"xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"mc:Ignorable="w16cid"w16cid:commentIdelements withw16cid:paraIdandw16cid:durableId.application/vnd.openxmlformats-officedocument.wordprocessingml.commentsIds+xmlfor/word/commentsIds.xml.http://schemas.microsoft.com/office/2016/09/relationships/commentsIdspointing atcommentsIds.xml.Generation logic
Commentsnow assigns deterministicparaIds when threading is active or when any comment carries adurableId(previously threading only). It exposes a newCommentIdsDatagetter (sibling ofThreadData), built only when adurableIdis present.Filewires up theCommentsIdspart, relationship, and content-type override wheneverComments#CommentIdsDatais set (sibling of thecommentsExtendedwiring).word/commentsIds.xml(sibling of the existingcommentsExtended.xmlblock).Files touched
src/file/paragraph/run/comment-run.ts:durableIdoption,ICommentIdData, generator and gettersrc/file/paragraph/run/comments-extended.ts:CommentsIdscomponentsrc/file/file.ts: part wiring, relationship, content-type, gettersrc/file/content-types/content-types.ts:addCommentsIds()src/file/relationships/relationship/relationship.ts: new relationship typesrc/export/packer/next-compiler.ts: serialize part toword/commentsIds.xmlTests
Added to the existing specs (vitest), covering threaded and single (non-threaded) cases:
comment-run.spec.ts:CommentIdsDatamapping, paraId fallback, threaded plus durable, paraId injectioncomments-extended.spec.ts:CommentsIdsXML serializationfile.spec.ts:File#CommentsIdspresence and absencenext-compiler.spec.ts:commentsIds.xmlemitted, content-type and relationship registeredFull suite passes (
npx vitest run): 196 files, 1032 tests.npm run build, lint, and prettier are clean.Notes
durableId. The part is only created when at least one comment opts in.durableIdis a string so callers can supply Word-style 8-character hex ids (for example"12AB34CD") verbatim, without numeric coercion.Summary by CodeRabbit
word/commentsIds.xmlpart so comment IDs remain consistent across edits, including for threaded comments.commentsIds.xml, durable-id mapping, and related XML parts and relationships.