Skip to content

Add stable full-surface semantic DOCX changes - #494

Open
JSv4 wants to merge 3 commits into
agent/issue-456-package-manifestfrom
agent/issue-457-semantic-diff
Open

Add stable full-surface semantic DOCX changes#494
JSv4 wants to merge 3 commits into
agent/issue-456-package-manifestfrom
agent/issue-457-semantic-diff

Conversation

@JSv4

@JSv4 JSv4 commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Closes #457

Stacked on #493 / issue #456. Review the semantic-diff commits in this PR; once #493 lands, this branch can be retargeted to main without changing the #457 patch.

Summary

  • adds a stable, versioned docxodus.semantic-changes v1 schema with deterministic ordering and canonical JSON
  • reports text/structure, formatting/styles, lists/numbering, tables, sections/page setup, stories/notes/comments, fields/links/bookmarks/controls, media/relationships, revisions/annotations, and opaque package changes
  • distinguishes insert/delete/move/modify and supplies owning part, paths, side-specific anchors/scopes, and typed before/after values
  • exposes the same semantic evidence through .NET, session, WASM/npm, Python, and MCP surfaces without changing legacy diff/redline formats
  • publishes and tests the formal JSON Schema plus documented performance/output budgets

Architecture and safety

  • keeps the existing IR/edit-script implementation as the modeled alignment authority
  • supplements only the surfaces IR does not model using [Verify] Capture baseline and output OOXML package manifests #456's manifest relationships, digests, locations, XML normalizer, and content-type resolution
  • performs mandatory manifest preflight before SDK/IR parsing, even when package supplementation is disabled
  • enforces declared and actual per-entry, aggregate, XML, and compression-ratio limits
  • shares one bounded package pass and retains detached parsed XML only—no second ZIP reader, archive handles, or duplicate raw payload copies
  • suppresses relationship-ID churn and XML serialization-only changes while preserving opaque XML whitespace/comments/processing instructions

Verification

  • SemanticDiffTests: 30 passed
  • PackageManifestTests: 30 passed
  • McpServerDispatcherTests: 85 passed
  • Python semantic host tests: 6 passed
  • npm TypeScript typecheck: passed
  • .NET build: 0 errors
  • git diff --check: passed

Measured guards:

  • 1,000 paragraphs: 132.4 ms, 1 change, 589 canonical bytes
  • 200 tables / 400 rows: 205.0 ms, 8 changes, 6,663 canonical bytes

@JSv4
JSv4 force-pushed the agent/issue-457-semantic-diff branch from fbddcfa to dc9e896 Compare August 15, 2026 23:40
@JSv4
JSv4 marked this pull request as ready for review August 15, 2026 23:47
@JSv4
JSv4 force-pushed the agent/issue-457-semantic-diff branch from dc9e896 to 64576d8 Compare August 17, 2026 09:02

JSv4 commented Aug 18, 2026

Copy link
Copy Markdown
Owner Author

Claude Code review — 3 finding(s)

normal: Docxodus/Verification/SemanticDiff.cs:1596-1607

SemanticDiff crashes with ArgumentOutOfRangeException on any DOCX containing OOXML integer attributes (image wp:extent/@cx,@cy; w:gridCol/@w; page/margin twips; etc.) whose values fit in long but exceed SemanticValue.MaxSafeInteger (2^53−1). A single crafted <wp:extent cx="9007199254740992" cy="1"/> turns the public verification surface into an uncaught crash across every transport (WASM/npm, Python, MCP docxodus_get_content format:"semantic_changes"). Fix in one place — clamp to bounds, return Absent, or add a lossless integer-string kind — at SemanticValue.Integer or the call sites in SemanticDiff.cs (image dims at 1598-1599, gridCol widths at 1834, and the twips projections at 1695-1786).

nit: Docxodus.Tests/Verification/SemanticChangeSetContractTests.cs:1-5

New file Docxodus.Tests/Verification/SemanticChangeSetContractTests.cs is missing the #nullable enable directive at the top, unlike all six sibling new files added in this PR. CLAUDE.md's Coding Standards explicitly require "New files: add #nullable enable at the top." — insert the directive after the license header for consistency.

nit: Docxodus/Verification/SemanticDiff.cs:1829-1839

TableGridValue/ElementValue/AttributeValue in SemanticDiff.cs use unbounded Descendants()/DescendantsAndSelf() sweeps rooted at the outer w:tbl/w:tc, so nested-table w:gridCol, w:tblStyle, w:tblW, and w:tcW leak into the outer container's emitted before/after values. When the outer table has a legitimate TblGridDigest/TblPrDigest/ShellDigest change, the v1 audit schema records anchor-accurate values that are in fact composed from nested tables, undermining the schema's stated contract. Fix by scoping to direct children — e.g. element.Element(W + "tblPr")?.Element(W + "tblStyle"), .Element(W + "tblPr")?.Element(W + "tblW"), .Element(W + "tcPr")?.Element(W + "tcW"), and table.Source.Element?.Element(W + "tblGrid")?.Elements(W + "gridCol") — mirroring IrReader's own tblGrid scoping.


Generated by Claude Code

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant