diff --git a/src/api/diff.integration.test.ts b/src/api/diff.integration.test.ts index d55a4e84..c35673a6 100644 --- a/src/api/diff.integration.test.ts +++ b/src/api/diff.integration.test.ts @@ -329,6 +329,48 @@ function anchoredTableMeta(anchorUuid: string, text: string): ObjectMeta { }; } +/** + * #648: a table cell mixing one hidden (w:vanish) run and one visible run. + * `visibleText` matches ONLY the visible run — the object tier's AST + * (objectText, via parser/docx/body-objects.ts's collectText) already drops + * hidden runs (#641/ADR-092), so this is what a correctly round-tripped + * capture must store. Before the #648 fix, merge/extract.ts's visibleText + * had no vanish handling at all and would concatenate the hidden run's text + * in front of the visible one, diverging from this AST snapshot and reading + * an untouched round-tripped DOCX as modified. + */ +function anchoredMixedVisibilityTableMeta( + anchorUuid: string, + hiddenText: string, + visibleRunText: string +): ObjectMeta { + const anchoredCell: ObjectBlobNode = { + 'w:sdt': [ + { 'w:sdtPr': [{ 'w:tag': [], ':@': { '@_w:val': `specr-uuid-${anchorUuid}` } }] }, + { + 'w:sdtContent': [ + { + 'w:p': [ + { + 'w:r': [{ 'w:rPr': [{ 'w:vanish': [] }] }, { 'w:t': [{ '#text': hiddenText }] }], + }, + { 'w:r': [{ 'w:t': [{ '#text': visibleRunText }] }] }, + ], + }, + ], + }, + ], + } as ObjectBlobNode; + return { + kind: 'table', + floating: false, + generation: 'drawingml', + rows: 1, + columns: 1, + blob: [{ 'w:tbl': [{ 'w:tr': [{ 'w:tc': [anchoredCell] }] }] }], + }; +} + describe('body-level object round-trip — real wiring (#520 review finding)', () => { const OBJ_PART_ID = randomUUID(); const OBJ_OBJECT_ID = randomUUID(); @@ -627,3 +669,273 @@ describe('delete/modify conflict wire scenario (#465)', () => { } ); }); + +// #648: an object-interior table cell mixing a hidden and a visible run. +// Before the fix, merge/extract.ts's visibleText had no w:vanish handling at +// all, so it disagreed with the AST (objectText, which already drops hidden +// runs — #641/ADR-092): an untouched round-tripped DOCX with this shape would +// report as modified. Proved at the real wiring — DB → generateDocx → +// extractContentControls → getObjectStructuralSnapshots → computeDiff — not +// just at the unit level, since that is where the two code paths' divergence +// actually surfaces as a false diff entry. +describe('body-level object round-trip — hidden/visible run mix (#648)', () => { + const VAN_PART_ID = randomUUID(); + const VAN_OBJECT_ID = randomUUID(); + const VAN_TEXT_ID = randomUUID(); + const VAN_HIDDEN_TEXT = 'Hidden internal guidance. '; + const VAN_VISIBLE_TEXT = 'Visible spec cell text.'; + let vanSpecId: string; + + beforeAll(async () => { + vanSpecId = await createSpec({ + section: '09 91 26', + title: 'Object Vanish Diff Wiring Spec', + source: `d648diff_${randomUUID().slice(0, 8)}`, + }); + await insertTree( + { + id: vanSpecId, + section: '09 91 26', + title: 'Object Vanish Diff Wiring Spec', + parts: [ + { + id: VAN_PART_ID, + type: 'part', + text: 'GENERAL', + meta: {}, + children: [ + { + id: VAN_OBJECT_ID, + type: 'object', + text: '', + meta: { + object: anchoredMixedVisibilityTableMeta( + VAN_TEXT_ID, + VAN_HIDDEN_TEXT, + VAN_VISIBLE_TEXT + ), + }, + children: [ + { + id: VAN_TEXT_ID, + type: 'objectText', + // Matches the object tier's AST (visible run only) — + // never the hidden run's text (#641/ADR-092). + text: VAN_VISIBLE_TEXT, + meta: {}, + children: [], + }, + ], + }, + ], + }, + ], + }, + vanSpecId, + pool + ); + }); + + afterAll(async () => { + await pool.query('DELETE FROM specs WHERE id = $1', [vanSpecId]); + }); + + it( + 'an unmodified generated DOCX round-trips with an empty diff — the hidden run never ' + + 'leaks into the object-interior text merge extracts', + async () => { + const generateRes = await fetch(`${baseUrl}/specs/${vanSpecId}/generate`, { + method: 'POST', + }); + expect(generateRes.status).toBe(200); + const buffer = Buffer.from(await generateRes.arrayBuffer()); + + const form = new FormData(); + form.append( + 'file', + new Blob([new Uint8Array(buffer)], { type: DOCX_MIME }), + 'object-vanish.docx' + ); + const diffRes = await fetch(`${baseUrl}/specs/${vanSpecId}/diff`, { + method: 'POST', + body: form, + }); + const body = (await diffRes.json()) as ApiResponse; + + expect(diffRes.status).toBe(200); + expect(body.data).toEqual({ + added: [], + modified: [], + deleted: [], + deleteConflicts: [], + conflicts: [], + objectConflicts: [], + warnings: [], + }); + } + ); +}); + +/** + * #652: a textBox-kind body object, whose blob root is the HOST body + * paragraph (`w:p`) carrying the `w:r > w:drawing` run — the capture shape + * `parser/docx/body-objects.ts` documents in its module comment and + * `buildTextBoxObject` actually produces (`blob: [anchored.node]`, where + * `anchorInteriorParagraphs` preserves the root's own tag and only wraps + * INTERIOR paragraphs with SDT anchors). + * + * A table's blob root is the `w:tbl` itself, which is also exactly what + * `merge/extract.ts`'s `walkObjectBlocks` matches — so the table tier is + * symmetric and every existing table-based test passes. For a textBox the + * two sides used to hash different trees: base fingerprinted the host `w:p`, + * theirs fingerprinted the bare `w:drawing`, so `fingerprintsDiverge` was + * ALWAYS true and any untouched round trip false-reported an objectConflict. + */ +function anchoredTextBoxMeta(anchorUuid: string, text: string): ObjectMeta { + const anchoredInterior: ObjectBlobNode = { + 'w:sdt': [ + { 'w:sdtPr': [{ 'w:tag': [], ':@': { '@_w:val': `specr-uuid-${anchorUuid}` } }] }, + { 'w:sdtContent': [{ 'w:p': [{ 'w:r': [{ 'w:t': [{ '#text': text }] }] }] }] }, + ], + } as ObjectBlobNode; + return { + kind: 'textBox', + floating: false, + generation: 'drawingml', + blob: [ + { + 'w:p': [ + { + 'w:r': [ + { + 'w:drawing': [ + { + 'wp:inline': [ + { + 'a:graphic': [ + { + 'a:graphicData': [ + { + 'wps:wsp': [ + { 'wps:txbx': [{ 'w:txbxContent': [anchoredInterior] }] }, + ], + }, + ], + }, + ], + }, + ], + }, + ], + }, + ], + }, + ], + }, + ], + }; +} + +// #652: the textBox counterpart of the table-kind wiring test above. Proved +// at the real wiring — DB → generateDocx → extractContentControls → +// getObjectStructuralSnapshots → computeDiff — because the asymmetry is +// between two production call sites (diff.ts fingerprints the stored blob +// root; extract.ts fingerprints the matched OBJECT_BLOCK_TAGS node), so a +// unit test on either side alone cannot see it. +describe('body-level object round-trip — textBox fingerprint symmetry (#652)', () => { + const TB_PART_ID = randomUUID(); + const TB_OBJECT_ID = randomUUID(); + const TB_TEXT_ID = randomUUID(); + const TB_TEXT = 'Text box interior paragraph.'; + let tbSpecId: string; + + beforeAll(async () => { + tbSpecId = await createSpec({ + section: '09 91 26', + title: 'Object TextBox Diff Wiring Spec', + source: `d652diff_${randomUUID().slice(0, 8)}`, + }); + await insertTree( + { + id: tbSpecId, + section: '09 91 26', + title: 'Object TextBox Diff Wiring Spec', + parts: [ + { + id: TB_PART_ID, + type: 'part', + text: 'GENERAL', + meta: {}, + children: [ + { + id: TB_OBJECT_ID, + type: 'object', + text: '', + meta: { object: anchoredTextBoxMeta(TB_TEXT_ID, TB_TEXT) }, + children: [ + { + id: TB_TEXT_ID, + type: 'objectText', + text: TB_TEXT, + meta: {}, + children: [], + }, + ], + }, + ], + }, + ], + }, + tbSpecId, + pool + ); + }); + + afterAll(async () => { + // id-scoped teardown: only the spec row this describe block created. + await pool.query('DELETE FROM specs WHERE id = $1', [tbSpecId]); + }); + + it('an unmodified generated DOCX round-trips with ZERO objectConflicts', async () => { + const generateRes = await fetch(`${baseUrl}/specs/${tbSpecId}/generate`, { method: 'POST' }); + expect(generateRes.status).toBe(200); + const buffer = Buffer.from(await generateRes.arrayBuffer()); + + const form = new FormData(); + form.append( + 'file', + new Blob([new Uint8Array(buffer)], { type: DOCX_MIME }), + 'object-textbox.docx' + ); + const diffRes = await fetch(`${baseUrl}/specs/${tbSpecId}/diff`, { + method: 'POST', + body: form, + }); + const body = (await diffRes.json()) as ApiResponse; + + expect(diffRes.status).toBe(200); + // The assertion that fails without the #652 fix: base hashed the host + // w:p, theirs hashed the bare w:drawing, so objectConflicts held one + // entry. + // + // Non-vacuity is established by mutation, not by this shape: reverting + // fingerprintRoot's host-paragraph pick makes ONLY this test fail, and it + // fails carrying BOTH a base and a theirs fingerprint — which + // detectObjectConflicts emits only for a block findMatchingBlock actually + // matched by interior uuid, so the findInteriorUuids path is provably + // live here. (Asserting the whole diff rather than objectConflicts alone + // is broader coverage of the object round trip, but it is NOT by itself a + // guard on findInteriorUuids: theirsControlled is built by walkBlocks + // independently, so interior text would still round-trip cleanly if + // interior-uuid collection regressed. extract.test.ts pins that path.) + expect(body.data).toEqual({ + added: [], + modified: [], + deleted: [], + deleteConflicts: [], + conflicts: [], + objectConflicts: [], + warnings: [], + }); + }); +}); diff --git a/src/merge/extract.test.ts b/src/merge/extract.test.ts index 8eafbf24..0eef59f4 100644 --- a/src/merge/extract.test.ts +++ b/src/merge/extract.test.ts @@ -1,9 +1,13 @@ import { describe, it, expect } from 'vitest'; import JSZip from 'jszip'; +import { readdirSync, readFileSync } from 'node:fs'; +import { join } from 'node:path'; import { generateDocx } from '../generator/index.js'; import { extractContentControls } from './extract.js'; import { MergeError } from './error.js'; -import type { SpecTree } from '../ast/types.js'; +import type { SpecTree } from '../ast/index.js'; + +const MERGE_DIR = join(process.cwd(), 'src/merge'); const PART_ID = '00000000-0000-0000-0000-000000000002'; const ART_ID = '00000000-0000-0000-0000-000000000003'; @@ -71,6 +75,13 @@ const tableRow = (cellsXml: readonly string[]): string => `${cellsXml.map(tableCell).join('')}`; const table = (rowsXml: readonly string[]): string => `${rowsXml.join('')}`; +/** A run with w:vanish bare/enabled (ST_OnOff ON — hidden). */ +const hiddenRun = (text: string): string => + `${text}`; +/** A run with w:vanish explicitly val="0" (ST_OnOff OFF — visible, #648). */ +const visibleOffRun = (text: string): string => + `${text}`; + describe('extractContentControls', () => { it('roundtrip: recovers every SpecNode id → text from generateDocx output', async () => { const buffer = await generateDocx(TREE); @@ -353,3 +364,97 @@ describe('extractContentControls — object-block extraction (#520)', () => { expect(result.objectBlocks[0]?.interiorUuids).toEqual([U1, U2]); }); }); + +// #648: visibleText had no w:vanish handling at all, so it disagreed with the +// object-tier AST walk (which already drops hidden runs — #641/ADR-092): an +// untouched round-tripped DOCX with this shape could report as modified. The +// fix is deliberately scoped to OBJECT INTERIORS ONLY (table/drawing/pict) — +// the ordinary paragraph tier's KNOWN AMBIGUITY (document.test.ts near line +// 259: a mixed hidden/visible paragraph is treated as visible) must not +// change, and is pinned explicitly below as the regression this issue most +// risks. +describe('extractContentControls — object-interior vanish handling (#648)', () => { + it('object interior: a table-cell paragraph mixing hidden and visible runs extracts only the visible text', async () => { + const body = table([tableRow([sdt(U1, para(hiddenRun('secret ') + run('visible')))])]); + const result = await extractContentControls(await craftDocx(body)); + expect(result.controlled.get(U1)).toBe('visible'); + }); + + // KNOWN AMBIGUITY (unchanged by #648, mirrors document.test.ts near line 259): + // an ORDINARY paragraph mixing hidden and visible runs is NOT object-interior, + // so it still yields both concatenated — the visible text is real content, and + // only a fully-hidden paragraph would drop out. A blanket hidden-run skip here + // was proposed during PR 647's review and rejected on exactly this ground. + it('ordinary paragraph tier is UNCHANGED: a mixed hidden/visible paragraph still yields both, concatenated', async () => { + const body = sdt(U1, para(hiddenRun('secret ') + run('visible'))); + const result = await extractContentControls(await craftDocx(body)); + expect(result.controlled.get(U1)).toBe('secret visible'); + }); + + it('object interior: a run whose w:vanish carries val="0" is VISIBLE (ST_OnOff toggle, not presence-only)', async () => { + const body = table([tableRow([sdt(U1, para(visibleOffRun('kept ') + run('too')))])]); + const result = await extractContentControls(await craftDocx(body)); + expect(result.controlled.get(U1)).toBe('kept too'); + }); + + // collectDrawingAnchors (the w:drawing/w:pict text-box interior walk, gap-1 + // #520) forces objectInterior=true through a call site separate from the + // generic OBJECT_BLOCK_TAGS union path that reaches w:tbl. Without a test + // driving a hidden/visible mixed run through THIS specific path, a + // regression that broke vanish-skipping only for drawing/pict (e.g. + // collectDrawingAnchors's hard-coded `true` reverted to `false`, or never + // threaded through at all) would pass every other #648 test while silently + // reintroducing the false-modified bug for 2 of the 3 OBJECT_BLOCK_TAGS. + it('object interior (DrawingML text box): a mixed hidden/visible paragraph inside the box extracts only the visible text', async () => { + const body = sdt( + U1, + para(run('before ') + drawingTextBoxRun(sdt(U2, para(hiddenRun('secret ') + run('visible'))))) + ); + const result = await extractContentControls(await craftDocx(body)); + expect(result.controlled.get(U2)).toBe('visible'); + }); + + it('object interior (VML text box): a mixed hidden/visible paragraph inside the box extracts only the visible text', async () => { + const body = sdt( + U1, + para(run('before ') + vmlTextBoxRun(sdt(U2, para(hiddenRun('secret ') + run('visible'))))) + ); + const result = await extractContentControls(await craftDocx(body)); + expect(result.controlled.get(U2)).toBe('visible'); + }); + + // Structural guard (not a behavior test): #648's fix reaches vanish detection + // through the single, already-existing hasRunVanish predicate (body-objects.ts, + // #641/ADR-092) rather than growing a second, drifting vanish reader inside + // merge/. Pins the manual "grep -rn vanish src/merge/" audit from #648's + // verification as an automated regression guard, so a future change can't + // silently reintroduce a second predicate that disagrees with the first. + // + // Deliberately does NOT scan file contents for the literal string "w:vanish" + // — that coupled the guard to a doc comment's exact prose (#650) and made a + // purely cosmetic reword of extract.ts's comments fail CI with no functional + // change. Instead it asserts the two things that actually matter: no file + // declares its own vanish-named predicate, and extract.ts both imports and + // calls the shared one. + it('src/merge/*.ts declares no local vanish predicate, and extract.ts calls the imported hasRunVanish', () => { + const nonTestFiles = readdirSync(MERGE_DIR).filter( + (name) => name.endsWith('.ts') && !name.endsWith('.test.ts') + ); + + // No locally-declared vanish predicate anywhere in merge/ — hasRunVanish + // must remain the ONLY definition, never a second, independently-drifting one. + for (const name of nonTestFiles) { + const src = readFileSync(join(MERGE_DIR, name), 'utf8'); + const localVanishDeclarations = + src.match(/\b(?:function|const)\s+\w*[Vv]anish\w*\s*[:(=]/g) ?? []; + expect(localVanishDeclarations).toEqual([]); + } + + const extractSrc = readFileSync(join(MERGE_DIR, 'extract.ts'), 'utf8'); + expect(extractSrc).toMatch( + /import\s*\{\s*hasRunVanish\s*\}\s*from\s*'\.\.\/parser\/index\.js';/ + ); + // The import must actually be exercised, not just present. + expect(extractSrc).toMatch(/\bhasRunVanish\(/); + }); +}); diff --git a/src/merge/extract.ts b/src/merge/extract.ts index 81244c24..3abe692a 100644 --- a/src/merge/extract.ts +++ b/src/merge/extract.ts @@ -4,6 +4,27 @@ import { UUID_TAG_PREFIX } from '../ast/index.js'; import { MergeError } from './error.js'; import { fingerprintBlob } from './object-fingerprint.js'; import type { ExtractResult, ExtractedObjectBlock, TrackChangeRecord } from './types.js'; +import type { ObjectBlobNode } from '../ast/index.js'; +// #648/ADR-092: a narrow, deliberate exception to module-boundaries.md's +// "merge/ knows nothing about parsing" prose line — hasRunVanish is the +// SINGLE ST_OnOff-aware vanish predicate shared by object capture +// (body-objects.ts) and object-blob edit rewrite (object-blob-edit.ts); a +// second copy here would reintroduce the exact capture/rewrite drift +// ADR-092 closed. Reached through the parser's own barrel (parser/index.ts), +// per the repo's sibling-barrel-only import rule. +// +// The barrel does transitively load parser/pdf/index.ts's static pdfjs-dist / +// unpdf / tesseract.js imports (~+370ms, ~+290MB RSS on a cold graph), so this +// line was reviewed as a possible cold-start regression and deliberately kept: +// every production path that reaches merge/ ALREADY loads parser/index.js in +// the same file — src/api/diff.ts imports assertDocxSafe one line above its +// ../merge/index.js import, as does src/mcp/handlers.ts — and no merge-only +// worker, script, or CLI exists. Net production cost is therefore zero; the +// only module graph that grows is merge/extract.test.ts (31 tests, ~1.4s). +// Deep-importing ../parser/docx/body-objects.js instead would trade that zero +// for a strictly deeper violation of the same module-boundaries.md rule, and +// relocating hasRunVanish is what ADR-092 exists to prevent. +import { hasRunVanish } from '../parser/index.js'; // preserveOrder keeps w:sdt blocks and bare w:p siblings in document order — // required for orphan indexes (non-preserveOrder grouping destroys ordering). @@ -20,10 +41,13 @@ const xmlParser = new XMLParser({ }); // fast-xml-parser preserveOrder node: one element key → children array, -// '#text' → string, ':@' → attribute record. -interface OrderedNode { - readonly [key: string]: unknown; -} +// '#text' → string, ':@' → attribute record. This is the SAME runtime shape +// as ast/object-schemas.ts's ObjectBlobNode (both describe fast-xml-parser's +// preserveOrder output), so extract.ts aliases it directly rather than +// keeping a second, duplicate node-shape definition — that alignment is +// also what lets hasRunVanish (which is typed against ObjectBlobNode) accept +// an extract.ts node with zero cross-boundary cast (#648). +type OrderedNode = ObjectBlobNode; interface ParaContext { readonly uuid: string | undefined; @@ -32,6 +56,14 @@ interface ParaContext { * text-box run (visitRunNode's w:drawing/w:pict branch) capture interior * specr-uuid anchors without threading the whole ExtractAcc through. */ readonly controlled: Map; + /** true once the walk has descended into a body-level object block + * (w:tbl/w:drawing/w:pict — OBJECT_BLOCK_TAGS). Object-interior paragraphs + * must match the AST's objectText, which already drops hidden runs + * (issue #641/ADR-092); the ordinary paragraph tier's KNOWN AMBIGUITY + * (a mixed hidden/visible paragraph is treated as visible — see + * document.test.ts near line 259) is deliberately untouched outside an + * object interior. See visibleText's use of this flag (#648). */ + readonly objectInterior: boolean; } interface ExtractAcc { @@ -65,8 +97,12 @@ function childrenOf(node: OrderedNode, tag: string): readonly OrderedNode[] { function attrStr(node: OrderedNode, name: string): string | undefined { const attrs = node[':@']; - if (typeof attrs !== 'object' || attrs === null) return undefined; - const value = (attrs as Record)[name]; + // ObjectBlobNode's `:@` is `Readonly> | + // undefined` (an optional property, never nullable) — `typeof attrs !== + // 'object'` alone already excludes `undefined` (whose typeof is + // 'undefined'), so no separate null check or value cast is needed. + if (typeof attrs !== 'object') return undefined; + const value = attrs[name]; return typeof value === 'string' ? value : undefined; } @@ -149,7 +185,9 @@ function visitRunNode(node: OrderedNode, tag: string, ctx: ParaContext): string * are discarded — a drawing/shape body has no CSI tier to anchor an addition * against, mirroring the table-cell anchorless rule. Track-change records * ARE preserved: `ctx.records` is the same array reference as the top-level - * walk's. + * walk's. objectInterior is forced true (#648): this function is only ever + * reached from visitRunNode's w:drawing/w:pict dispatch, i.e. a text-box + * interior, which is object territory by construction. */ function collectDrawingAnchors(nodes: readonly OrderedNode[], ctx: ParaContext): void { const throwawayAcc: ExtractAcc = { @@ -157,15 +195,29 @@ function collectDrawingAnchors(nodes: readonly OrderedNode[], ctx: ParaContext): orphans: [], records: ctx.records, }; - walkBlocks(nodes, undefined, throwawayAcc, { index: 0, lastControlledUuid: undefined }, false); + walkBlocks( + nodes, + undefined, + throwawayAcc, + { index: 0, lastControlledUuid: undefined }, + false, + true + ); } -/** Visible (post virtual-accept) text of paragraph content nodes, in order. */ +/** Visible (post virtual-accept) text of paragraph content nodes, in order. + * Inside an object interior (ctx.objectInterior), a run whose w:vanish is + * enabled (hasRunVanish, ST_OnOff-aware) is skipped — matching the object + * tier's objectText, which already drops the same runs (#641/ADR-092). This + * check is intentionally scoped to objectInterior only: the ordinary + * paragraph tier's KNOWN AMBIGUITY (a mixed hidden/visible paragraph reads + * as visible) is out of scope for #648 and must not change. */ function visibleText(nodes: readonly OrderedNode[], ctx: ParaContext): string { let out = ''; for (const node of nodes) { const tag = tagOf(node); if (tag === undefined || tag === '#text' || PROPERTY_TAGS.has(tag)) continue; + if (ctx.objectInterior && hasRunVanish(node)) continue; out += visitRunNode(node, tag, ctx); } return out; @@ -204,12 +256,14 @@ function visitParagraph( uuid: string | undefined, acc: ExtractAcc, state: WalkState, - inTable: boolean + inTable: boolean, + objectInterior: boolean ): WalkState { const text = visibleText(childrenOf(node, 'w:p'), { uuid, records: acc.records, controlled: acc.controlled, + objectInterior, }); if (!text.trim()) return state; // whitespace-only spacer paragraphs ignored if (uuid !== undefined) { @@ -226,34 +280,52 @@ function visitParagraph( return { index: state.index + 1, lastControlledUuid: state.lastControlledUuid }; } -/** Walk block-level nodes in document order, tracking the enclosing sdt uuid - * and whether the walk has descended into a w:tbl (table cells never anchor). */ +const OBJECT_BLOCK_TAGS = new Set(['w:tbl', 'w:drawing', 'w:pict']); + +/** Walk block-level nodes in document order, tracking the enclosing sdt uuid, + * whether the walk has descended into a w:tbl (table cells never anchor), + * and whether it has descended into a body-level object block (#648 — + * OBJECT_BLOCK_TAGS: w:tbl/w:drawing/w:pict; a visibleText vanish-skip). */ function walkBlocks( nodes: readonly OrderedNode[], uuid: string | undefined, acc: ExtractAcc, state: WalkState, - inTable: boolean + inTable: boolean, + objectInterior: boolean ): WalkState { let s = state; for (const node of nodes) { const tag = tagOf(node); if (tag === undefined || tag === '#text' || PROPERTY_TAGS.has(tag)) continue; if (tag === 'w:sdt') { - s = walkBlocks(childrenOf(node, tag), readSdtUuid(node) ?? uuid, acc, s, inTable); + s = walkBlocks( + childrenOf(node, tag), + readSdtUuid(node) ?? uuid, + acc, + s, + inTable, + objectInterior + ); } else if (tag === 'w:p') { - s = visitParagraph(node, uuid, acc, s, inTable); + s = visitParagraph(node, uuid, acc, s, inTable, objectInterior); } else { // w:document, w:body, w:sdtContent, w:tbl, … — a w:tbl marks its whole - // subtree as table-descended so its cell paragraphs stay anchorless. - s = walkBlocks(childrenOf(node, tag), uuid, acc, s, inTable || tag === 'w:tbl'); + // subtree as table-descended so its cell paragraphs stay anchorless; + // any OBJECT_BLOCK_TAGS tag also marks its subtree as object-interior. + s = walkBlocks( + childrenOf(node, tag), + uuid, + acc, + s, + inTable || tag === 'w:tbl', + objectInterior || OBJECT_BLOCK_TAGS.has(tag) + ); } } return s; } -const OBJECT_BLOCK_TAGS = new Set(['w:tbl', 'w:drawing', 'w:pict']); - /** Every specr-uuid `w:sdt` anchor's uuid found anywhere in `nodes`, in document order. */ function findInteriorUuids(nodes: readonly OrderedNode[]): string[] { const uuids: string[] = []; @@ -280,7 +352,8 @@ function findInteriorUuids(nodes: readonly OrderedNode[]): string[] { function walkObjectBlocks( nodes: readonly OrderedNode[], inBlock: boolean, - blocks: ExtractedObjectBlock[] + blocks: ExtractedObjectBlock[], + hostParagraph: OrderedNode | undefined ): void { for (const node of nodes) { const tag = tagOf(node); @@ -289,16 +362,62 @@ function walkObjectBlocks( if (isObjectTag && !inBlock) { blocks.push({ interiorUuids: findInteriorUuids(childrenOf(node, tag)), - fingerprint: fingerprintBlob([node]), + fingerprint: fingerprintBlob([fingerprintRoot(node, tag, hostParagraph)]), }); } - walkObjectBlocks(childrenOf(node, tag), inBlock || isObjectTag, blocks); + walkObjectBlocks( + childrenOf(node, tag), + inBlock || isObjectTag, + blocks, + // A w:p becomes the host for any drawing run nested beneath it; deeper + // non-paragraph nodes inherit it unchanged. + tag === 'w:p' ? node : hostParagraph + ); } } +/** Tags whose captured blob root is the HOST body `w:p`, not the tag itself. */ +const PARAGRAPH_HOSTED_OBJECT_TAGS = new Set(['w:drawing', 'w:pict']); + +/** + * The node to fingerprint for a detected object block (#652) — it MUST be the + * same root that capture stored as `ObjectMeta.blob[0]`, because diff.ts's + * `detectObjectConflicts` fingerprints that stored blob directly and compares + * the two hashes. + * + * The two capture kinds do not agree on what their root is + * (parser/docx/body-objects.ts's module comment, "Two capture paths, one + * shape"): a table's blob root is the `w:tbl` itself, so the matched tag IS + * the root; a textBox/pict's blob root is the HOST body paragraph (`w:p`) + * carrying the `w:r > w:drawing`/`w:pict` run, so the matched tag is one + * wrapper layer BELOW the root. + * + * Fingerprinting the matched tag unconditionally is what made the table tier + * symmetric (and every table test pass) while making the textBox tier compare + * a bare `w:drawing(...)` shape against a stored `w:p(w:r(w:drawing(...)))` + * shape — hashes that can never match, so every untouched round trip of a + * text box reported a false `objectConflict`. + * + * Mirroring capture here (rather than changing capture to store the bare + * node) keeps `buildObjectBlocks`'s re-emit contract intact: the generator + * emits `blob[0]` as a body child, and a bare `w:drawing` is not a valid + * block-level body child — it must sit inside a run inside a paragraph. + */ +function fingerprintRoot( + node: OrderedNode, + tag: string, + hostParagraph: OrderedNode | undefined +): OrderedNode { + if (!PARAGRAPH_HOSTED_OBJECT_TAGS.has(tag)) return node; + // A drawing with no enclosing w:p is malformed OOXML; degrade to the bare + // node rather than throwing — extract.ts never rejects a document it can + // still partially read. + return hostParagraph ?? node; +} + function extractObjectBlocks(nodes: readonly OrderedNode[]): ExtractedObjectBlock[] { const blocks: ExtractedObjectBlock[] = []; - walkObjectBlocks(nodes, false, blocks); + walkObjectBlocks(nodes, false, blocks, undefined); return blocks; } @@ -334,7 +453,8 @@ export async function extractContentControls(docxBuffer: Buffer): Promise