From 359824156e4bbdae674bf87ff4ce891b407966ab Mon Sep 17 00:00:00 2001 From: Steven Obiajulu Date: Fri, 26 Jun 2026 01:39:51 -0400 Subject: [PATCH 1/2] fix(docx-core): preserve merge-source commentsIds durable-ID rows The comment ancillary post-pass merged comments.xml, commentsExtended.xml, and people.xml from the merge-source side but skipped commentsIds.xml. Merged-in comments therefore lost their Word durable-ID metadata ([MS-DOCX] w16cid:commentId rows) until Word regenerated them. Add mergeCommentsIds, mirroring mergeCommentsExtended: it carries forward the source rows keyed by w16cid:paraId (the same paraIds threaded through commentsExtended.xml) for the expanded comment set, appending into an existing result part or bootstrapping the part with OPC metadata when the base archive lacks it. Wired into mergeCommentAncillaryParts so it runs in both reconstruction modes. Ref: #471 --- .../src/baselines/atomizer/pipeline.ts | 88 +++++++++- .../rebuild-auxiliary-merge.test.ts | 158 ++++++++++++++++++ 2 files changed, 245 insertions(+), 1 deletion(-) diff --git a/packages/docx-core/src/baselines/atomizer/pipeline.ts b/packages/docx-core/src/baselines/atomizer/pipeline.ts index 3d9eed22..f2874f0a 100644 --- a/packages/docx-core/src/baselines/atomizer/pipeline.ts +++ b/packages/docx-core/src/baselines/atomizer/pipeline.ts @@ -1235,8 +1235,9 @@ async function mergeCommentAncillaryParts( // (and any roots not yet present in the result, defensively). await mergeMissingCommentDefinitions(resultArchive, commentById, includedCommentIds); - // Merge commentsExtended and people for the expanded set. + // Merge commentsExtended, commentsIds, and people for the expanded set. await mergeCommentsExtended(sourceArchive, resultArchive, includedParaIds); + await mergeCommentsIds(sourceArchive, resultArchive, includedParaIds); await mergePeople(sourceArchive, resultArchive, includedAuthors); } @@ -1358,6 +1359,91 @@ const COMMENTS_EXTENDED_DESCRIPTOR: AuxiliaryPartDescriptor = { idBearingTags: [], // keyed by w15:paraId, not w:id }; +const COMMENTS_IDS_DESCRIPTOR: AuxiliaryPartDescriptor = { + label: 'commentsIds', + partPath: 'word/commentsIds.xml', + referenceTag: '', + entryTag: 'w16cid:commentId', + rootTag: 'w16cid:commentsIds', + contentType: 'application/vnd.openxmlformats-officedocument.wordprocessingml.commentsIds+xml', + relationshipType: 'http://schemas.microsoft.com/office/2016/09/relationships/commentsIds', + idBearingTags: [], // keyed by w16cid:paraId, not w:id +}; + +/** + * Merge the merge-source side's commentsIds.xml durable-ID rows for the + * expanded comment set. commentsIds.xml ([MS-DOCX]) keys each + * by the comment paragraph's w16cid:paraId — the same + * paraIds threaded through commentsExtended.xml — so merged-in comments retain + * their Word durable IDs instead of forcing Word to regenerate them (issue + * #471). Mirrors mergeCommentsExtended's append/bootstrap shape. + */ +async function mergeCommentsIds( + sourceArchive: DocxArchive, + resultArchive: DocxArchive, + mergedParaIds: Set, +): Promise { + if (mergedParaIds.size === 0) return; + + const sourceXml = await sourceArchive.getFile('word/commentsIds.xml'); + if (!sourceXml) return; + + const sourceDoc = parseXml(sourceXml); + const sourceEntries = sourceDoc.getElementsByTagName('w16cid:commentId'); + + // Collect rows whose paraId matches a merged comment's paragraph. + const entriesToMerge: Element[] = []; + for (let i = 0; i < sourceEntries.length; i++) { + const el = sourceEntries[i] as Element; + const paraId = el.getAttribute('w16cid:paraId'); + if (paraId && mergedParaIds.has(paraId)) { + entriesToMerge.push(el); + } + } + + if (entriesToMerge.length === 0) return; + + const resultXml = await resultArchive.getFile('word/commentsIds.xml'); + + if (resultXml) { + const resultDoc = parseXml(resultXml); + const rootEl = resultDoc.documentElement; + + const existingParaIds = new Set(); + const existing = rootEl.getElementsByTagName('w16cid:commentId'); + for (let i = 0; i < existing.length; i++) { + const pid = (existing[i] as Element).getAttribute('w16cid:paraId'); + if (pid) existingParaIds.add(pid); + } + + for (const el of entriesToMerge) { + const pid = el.getAttribute('w16cid:paraId'); + if (pid && !existingParaIds.has(pid)) { + rootEl.appendChild(resultDoc.importNode(el, true)); + } + } + + resultArchive.setFile('word/commentsIds.xml', serializer.serializeToString(resultDoc)); + return; + } + + // Bootstrap: result lacks commentsIds.xml but the merged comments carry + // durable IDs. Clone the source's root (preserves namespaces), drop + // non-matching rows, then add OPC metadata. + const newDoc = parseXml(sourceXml); + const newRoot = newDoc.documentElement; + const allEntries = newRoot.getElementsByTagName('w16cid:commentId'); + const toRemove: Element[] = []; + for (let i = 0; i < allEntries.length; i++) { + const el = allEntries[i] as Element; + const paraId = el.getAttribute('w16cid:paraId'); + if (!paraId || !mergedParaIds.has(paraId)) toRemove.push(el); + } + for (const el of toRemove) newRoot.removeChild(el); + resultArchive.setFile('word/commentsIds.xml', serializer.serializeToString(newDoc)); + await ensureOpcMetadata(resultArchive, COMMENTS_IDS_DESCRIPTOR); +} + const PEOPLE_DESCRIPTOR: AuxiliaryPartDescriptor = { label: 'people', partPath: 'word/people.xml', diff --git a/packages/docx-core/src/integration/rebuild-auxiliary-merge.test.ts b/packages/docx-core/src/integration/rebuild-auxiliary-merge.test.ts index f7e987be..871c48dd 100644 --- a/packages/docx-core/src/integration/rebuild-auxiliary-merge.test.ts +++ b/packages/docx-core/src/integration/rebuild-auxiliary-merge.test.ts @@ -18,6 +18,58 @@ import { compareDocuments } from '../index.js'; import { buildSyntheticDocx, buildDocxFromParts, getResultParts } from './synthetic-docx-fixture.js'; import { buildDocxFromBodyXml } from '../testing/ooxml-fixtures.js'; import { parseXml } from '../primitives/xml.js'; +import type JSZip from 'jszip'; + +/** + * Re-zip a DOCX buffer after mutating its parts. Used to splice in Word + * extension parts (e.g. commentsIds.xml) the synthetic fixture does not emit. + */ +async function spliceZip(buffer: Buffer, mutate: (zip: JSZip) => Promise): Promise { + const JSZip = (await import('jszip')).default; + const zip = await JSZip.loadAsync(buffer); + await mutate(zip); + return (await zip.generateAsync({ type: 'nodebuffer' })) as Buffer; +} + +/** + * Splice a Word commentsIds.xml part (with its content-type override and + * relationship) into a DOCX buffer, carrying one durable-ID + * row per [paraId, durableId] pair. [MS-DOCX] keys these rows by the comment + * paragraph's w16cid:paraId. + */ +async function addCommentsIdsPart( + buffer: Buffer, + rows: Array<{ paraId: string; durableId: string }>, +): Promise { + return spliceZip(buffer, async (zip) => { + const entries = rows + .map((r) => ``) + .join(''); + zip.file( + 'word/commentsIds.xml', + `` + + `` + + entries + + `` + ); + const contentTypes = await zip.file('[Content_Types].xml')!.async('string'); + zip.file( + '[Content_Types].xml', + contentTypes.replace( + '', + `` + ) + ); + const rels = await zip.file('word/_rels/document.xml.rels')!.async('string'); + zip.file( + 'word/_rels/document.xml.rels', + rels.replace( + '', + `` + ) + ); + }); +} /** * Find every element with the given local name and report its immediate-parent @@ -222,6 +274,112 @@ describe('Rebuild Auxiliary Part Merging (issue #94)', () => { }); }); + // --------------------------------------------------------------------------- + // Issue #471: merge-source commentsIds.xml durable-ID rows dropped + // + // commentsIds.xml ([MS-DOCX]) keys each durable-ID row by + // the comment paragraph's w16cid:paraId. Before the fix, neither + // reconstruction mode merged these rows, so merged-in comments lost their + // Word durable IDs (cosmetic — Word regenerates them — but the source's + // durable-ID metadata was silently dropped). + // --------------------------------------------------------------------------- + describe('Comment with commentsIds.xml added on revised side (issue #471)', () => { + test('rebuild bootstraps commentsIds.xml with the merge-source durable ID', async ({ given, when, then }: AllureBddContext) => { + let original: Buffer, revised: Buffer; + await given('original has no comments and revised adds one with a commentsIds durable ID', async () => { + original = await buildSyntheticDocx({ + paragraphs: ['First paragraph', 'Commented paragraph', 'Third paragraph'], + }); + const revisedBase = await buildSyntheticDocx({ + paragraphs: ['First paragraph', 'Commented paragraph', 'Third paragraph'], + commentOnParagraph: 1, + commentText: 'Has durable id', + commentAuthor: 'Alice', + commentAncillaryParts: true, + }); + revised = await addCommentsIdsPart(revisedBase, [ + { paraId: '00000001', durableId: '11111111' }, + ]); + }); + + let result: Awaited>; + await when('documents are compared in rebuild mode', async () => { + result = await compareDocuments(original, revised, { + engine: 'atomizer', + reconstructionMode: 'rebuild', + }); + }); + + await then('rebuild creates commentsIds.xml carrying the durable ID with OPC metadata', async () => { + expect(result.reconstructionModeUsed).toBe('rebuild'); + + const parts = await getResultParts(result.document); + + expect(parts.commentsIdsXml).not.toBeNull(); + expect(parts.commentsIdsXml!).toContain('w16cid:paraId="00000001"'); + expect(parts.commentsIdsXml!).toContain('w16cid:durableId="11111111"'); + + expect(parts.contentTypesXml!).toContain('commentsIds.xml'); + expect(parts.contentTypesXml!).toContain( + 'application/vnd.openxmlformats-officedocument.wordprocessingml.commentsIds+xml' + ); + expect(parts.relsXml!).toContain('commentsIds.xml'); + expect(parts.relsXml!).toContain( + 'http://schemas.microsoft.com/office/2016/09/relationships/commentsIds' + ); + }); + }); + + test('rebuild appends the reply durable ID into an existing commentsIds.xml', async ({ given, when, then }: AllureBddContext) => { + let original: Buffer, revised: Buffer; + await given('original has a root durable ID; revised adds a reply with its own durable ID', async () => { + const originalBase = await buildSyntheticDocx({ + paragraphs: ['First paragraph', 'Commented paragraph', 'Third paragraph'], + commentOnParagraph: 1, + commentText: 'Root question', + commentAuthor: 'Alice', + commentAncillaryParts: true, + }); + original = await addCommentsIdsPart(originalBase, [ + { paraId: '00000001', durableId: '11111111' }, + ]); + const revisedBase = await buildSyntheticDocx({ + paragraphs: ['First paragraph', 'Commented paragraph', 'Third paragraph'], + commentOnParagraph: 1, + commentText: 'Root question', + commentAuthor: 'Alice', + commentAncillaryParts: true, + replyText: 'Reply text', + replyAuthor: 'Bob', + }); + revised = await addCommentsIdsPart(revisedBase, [ + { paraId: '00000001', durableId: '11111111' }, + { paraId: '00000002', durableId: '22222222' }, + ]); + }); + + let result: Awaited>; + await when('documents are compared in rebuild mode', async () => { + result = await compareDocuments(original, revised, { + engine: 'atomizer', + reconstructionMode: 'rebuild', + }); + }); + + await then('both root and reply durable IDs survive in commentsIds.xml', async () => { + expect(result.reconstructionModeUsed).toBe('rebuild'); + + const parts = await getResultParts(result.document); + + expect(parts.commentsIdsXml).not.toBeNull(); + expect(parts.commentsIdsXml!).toContain('w16cid:paraId="00000001"'); + expect(parts.commentsIdsXml!).toContain('w16cid:durableId="11111111"'); + expect(parts.commentsIdsXml!).toContain('w16cid:paraId="00000002"'); + expect(parts.commentsIdsXml!).toContain('w16cid:durableId="22222222"'); + }); + }); + }); + // --------------------------------------------------------------------------- // Issue #108: Reply comments dropped in rebuild mode // From 4e64c512e1c6f840191448703f7af765a6eed438 Mon Sep 17 00:00:00 2001 From: Steven Obiajulu Date: Tue, 30 Jun 2026 12:50:19 -0400 Subject: [PATCH 2/2] test(docx-core): count-guard duplicate w16cid:commentId rows in merge append Ref: #471 --- .../src/integration/rebuild-auxiliary-merge.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/docx-core/src/integration/rebuild-auxiliary-merge.test.ts b/packages/docx-core/src/integration/rebuild-auxiliary-merge.test.ts index 871c48dd..d4a47c31 100644 --- a/packages/docx-core/src/integration/rebuild-auxiliary-merge.test.ts +++ b/packages/docx-core/src/integration/rebuild-auxiliary-merge.test.ts @@ -376,6 +376,14 @@ describe('Rebuild Auxiliary Part Merging (issue #94)', () => { expect(parts.commentsIdsXml!).toContain('w16cid:durableId="11111111"'); expect(parts.commentsIdsXml!).toContain('w16cid:paraId="00000002"'); expect(parts.commentsIdsXml!).toContain('w16cid:durableId="22222222"'); + + // Count-guard: appending the reply durable-ID row must not duplicate + // the pre-existing root row. Each paraId keys EXACTLY ONE + // durable-ID row. + const idRowsForPara = (paraId: string) => + (parts.commentsIdsXml!.match(new RegExp(`w16cid:paraId="${paraId}"`, 'g')) ?? []).length; + expect(idRowsForPara('00000001')).toBe(1); + expect(idRowsForPara('00000002')).toBe(1); }); }); });