Skip to content

feat(merge): detect attribute-value-only object geometry/formatting changes (structural-attribute allowlist) #524

Description

@thewrz

Context

Follow-up from #520 (body-object structural-conflict detection, #300 WS3b). #520's ObjectStructureFingerprint (src/merge/object-fingerprint.ts) hashes tag topology only — #text nodes and attribute values are dropped by design (ADR-072 §21). So element-shape geometry/formatting changes are detected (adding/removing a border element w:tcBorders, shading w:shd, an w:rPr/w:b wrapper, a row, a column), but attribute-value-only changes at a fixed tag shape are not — e.g. changing an existing w:tblW/w:gridCol width, or a w:tcMar margin, with no element added or removed.

Raised by CodeRabbit during the #520 review:

Why this is its own issue, not a #520 patch

The naive fix (hash all :@ attributes) regresses: the two sides compared are a stored base blob vs a freshly Word-saved DOCX, and Word regenerates volatile revision attributes (w:rsidR, w:rsidRPr, w:rsidTr, paraId, textId) on every save. Hashing them indiscriminately would diverge the fingerprint on every object the editor merely opened — a false-positive conflict storm worse than the current miss. Correct coverage needs a curated allowlist, which is real design + review scope.

Proposed approach

Add a structural-attribute allowlist to structuralShape/fingerprintBlob: include canonical geometry/formatting scalar attribute values (widths w:w, drawing extents cx/cy, grid-span w:gridSpan, vertical-merge w:vMerge, margins w:tcMar, border/shading values) while excluding volatile revision attributes (w:rsid*, paraId, textId).

Acceptance criteria

  • A change that alters only an attribute value (e.g. a column width) diverges the fingerprint.
  • Opening + resaving a DOCX in Word with no structural edit does not diverge the fingerprint (no rsid false-positives) — pin this with a regression test.
  • Regression tests for geometry-only and formatting-only attribute changes.
  • ADR-072 §21 updated to describe the allowlist boundary.

Notes

Detection is DETECTION-ONLY over an opaque blob, so the current miss is not data loss — the stored blob is untouched; it just doesn't raise a "resolve by hand" flag. This is a detection-coverage enhancement, not a data-integrity fix.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    • Status
      Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions