Skip to content

fix(docx-core): resolve tracked move paragraph marks safely - #989

Merged
stevenobiajulu merged 8 commits into
mainfrom
985-reject-move-paragraph-marks-20260917
Sep 18, 2026
Merged

stevenobiajulu merged 8 commits into
mainfrom
985-reject-move-paragraph-marks-20260917

Conversation

@stevenobiajulu

@stevenobiajulu stevenobiajulu commented Sep 17, 2026

Copy link
Copy Markdown
Member

Summary

Resolve selected moveFrom/moveTo paragraph marks through the existing native mark-based break policy, matching complete-range Accept/Reject projections without retaining empty moved containers. Preserve local bookmarks around untracked or foreign surviving content; the AST path receives the same independently justified guard.

Why / scope

Native PhaseA treated only del/ins marks as break revisions while comparison projection also handled moves. A reduced parity discrepancy was confirmed using complete named ranges, content wrappers and schema-valid packages, including terminal and cell cases. A fresh reviewer found the first bookmark guard discarded anchors around surviving text; independent red/green controls repair that in both engines.

One concern, no public APIs, arbitrary content-based paragraph deletion, hosted API work or protection changes. Existing inherited cross-paragraph bookmark and selective orphan-range-end behavior is not represented as fixed. Foreign endpoint preservation is asserted; a fully valid selective move-group resolution contract is not newly claimed. LibreOffice terminal-container limitation remains separate in #973. Word Accept/Reject UNVERIFIED.

OpenSpec: focused bug repair, no new capability or architecture shift. Relevant OOXML source/test claims use edition5 citations. Complete packages reuse shared synthetic fixtures; inline markup is the scenario under test. No web/UI visual changes.

Verification at a99b143

  • npm run build
  • npm run lint:workspaces
  • npm run test:run (comparison575 passed/11 skipped, core1318 passed/1 skipped; render verifiers passed)
  • npm run check:spec-coverage
  • npm run check:conformance-citations
  • npm run check:conformance-doc
  • Required serialized public corpus21/21,23-row differential manifest unchanged (467.26s).
  • Eighteen focused complete-range/mixed-content/selective-author controls.
  • Reviewer regenerated12 guard-exercising schema controls validate; original complete-range schema controls9/9.
  • Fresh built public NVCA compare/native/AST and actual LibreOffice Accept/Reject each match all33 paragraphs; tracked/native output schemas3/3.
  • Fresh dynamic Claude Opus5 review APPROVE at this exact head, with prior failing cases executed against prior and current source.

Fresh main integration verification at b5f326a

Normal merge (no rewritten reviewed history) with mainca618780/PR988 retains both parents' changes.

  • All six complete root pre-submit gates passed at this exact integration head.
  • Required one-worker corpus21/21 passed (450.36s);23-row manifest unchanged.
  • Fresh built NVCA native/AST and actual LibreOffice Accept/Reject match33paragraphs each, independently loaded original/revised controls;3/3schema controls validate.
  • Fresh dynamic Claude Opus5 integration review APPROVE;166focused tests and28combined property/move source/projection schemas pass.

Residual section-on-move, selective range-end and spanning-bookmark characterizations are explicitly retained in open941: #941 (comment) . Convergence with the existing ins/del section-break policy is not general reader/Word section fidelity. Inherited foreign paragraph-mark property history is now separately tracked in991 and is not claimed repaired here.

All local gates passed before each push. Base freshness is rechecked before arming automerge. Exact-merge public smoke and production-docs merge-SHA verification remain required after landing.

Fixes: #985
Ref: #941, #973, #982, #986

Native paragraph resolution handled ordinary insertion/deletion marks but ignored equivalent moved-to/moved-from marks, leaving empty moved endpoint containers after projection. Resolve the matching move marks without changing content-only move paragraph ownership, and prevent bookmarks local to discarded endpoints from being rescued onto unrelated surviving content. Complete named-range fixtures, terminal and table-cell controls, selective foreign histories, and an actual compareDocuments case establish native/AST parity. Full bounded root pre-submit passes.

Fixes: #985
Independent dynamic Opus 5 review reproduced a regression where rejecting a moved-to paragraph mark deleted a local bookmark pair even though its untracked content survived and merged forward. Harvest local endpoint pairs only when all substantive content is selected insertion/move-to markup. Apply the same guard to the inherited AST gap rather than using parity with that gap as an oracle. Two red-to-green surviving-content controls and a real two-author selective-resolution case raise the focused move suite to eighteen passing tests.

Ref: #985
@vercel

vercel Bot commented Sep 17, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
site Ready Ready Preview Sep 17, 2026 11:45pm UTC

Request Review

@usejunior-llm-gate

usejunior-llm-gate Bot commented Sep 17, 2026

Copy link
Copy Markdown

LLM gate (advisory)

All evaluated rules passed - 8 pass, 0 warn, 0 error, 8 skipped, 16 total

Findings

None.

All 16 rules (8 evaluated, 8 skipped)
Rule Verdict Detail
read_file response metadata parity SKIPPED paths not touched by this PR
Live DOM namespace-safe OOXML writes SKIPPED paths not touched by this PR
Complex-field revisions preserve complete accept/reject state machines PASS The PR only touches native paragraph-mark move resolution (moveTo/moveFrom) and does not modify field atomization, validateFieldStructure, w:fldChar, w:instrText, w:delInstrText, or collapsed-field comparison.
Field validation per story, not global SKIPPED paths not touched by this PR
Revision IDs seeded from all revision-bearing side parts SKIPPED paths not touched by this PR
Accept/reject sweep side parts and caches PASS The PR touches accept_changes (packages/docx-core/src/primitives/accept_changes.ts) and reject_changes (packages/docx-core/src/primitives/reject_changes.ts); DocxDocument (packages/docx-core/src/primitives/document.ts:430-514) processes every story in REVISION_STORY_PART_PATHS, writing back only modified parts, refreshing this.footnotesXml, and pruning orphaned footnotes while preserving separator entries.
DocumentViewNode.heading stays canonical SKIPPED paths not touched by this PR
AI-author parity across entry points SKIPPED paths not touched by this PR
Property-change wrapper discipline SKIPPED paths not touched by this PR
SUPPORT.md Table A drift vs. implementation PASS The precondition is not met because the PR only modifies the accept/reject consumer utilities in packages/docx-core/src/primitives/accept_changes.ts and packages/docx-core/src/primitives/reject_changes.ts rather than OOXML revision emission behavior, and does not touch SUPPORT.md.
Table A / Table B boundary on side-part revisions SKIPPED paths not touched by this PR
Canonical-emission surface completeness PASS The PR only modifies accept/reject tracked-change resolution logic in packages/docx-core/src/primitives/accept_changes.ts and reject_changes.ts, which resolves existing markup and does not add or change any write-time tracked-edit/emission surface.
Unit-test quality (avoid tautological / change-detector tests) PASS The tests in packages/docx-compare/src/tagged/nativeMoveParagraphParity.test.ts:1 and packages/docx-core/src/primitives/native_move_paragraph_marks.test.ts:1 use independent, first-principles assertions of text content and XML element structure without any mocking of the SUT.
Re-derived facts vs canonical sources PASS The PR modifies both the native primitives in packages/docx-core/src/primitives/accept_changes.ts:385, reject_changes.ts:478 and their sibling AST-based counterparts in packages/docx-compare/src/tagged/trackChangesAcceptorAst.ts:728, while adding a parity test in packages/docx-compare/src/tagged/nativeMoveParagraphParity.test.ts:28 to pin the two in agreement.
.openspec tag ↔ test-assertion drift PASS The PR does not add, move, or change any .openspec tags on any tests, so the precondition for this checklist question is not met.
Library stays general (no downstream-domain leakage) PASS The PR modifies acceptChanges and rejectChanges to support native move paragraph marks (moveFrom and moveTo), adhering strictly to general OOXML/Word terminology with no downstream domain leakage.

@stevenobiajulu

Copy link
Copy Markdown
Member Author

Advisory adjudication for head b5f326a73b8e00305a402e3eb263580630afd3f3:

The canonical-emission completeness warning concerns a revision consumer change, not a new write-time emission surface. SUPPORT.md Table A enumerates tracked-edit emitters; this PR does not add one. The same advisory explicitly reports Table A drift PASS. Adding canonical-emission tests or emitter support rows would misclassify this change.

Consumption is verified by the new complete-range native/AST paragraph parity controls and the built-code real public NVCA Accept/Reject smoke. All six local gates and the required 21-test corpus replay pass on this head, and fresh dynamic Opus review approves it. The warning is adjudicated as not applicable to an emission rollout, not silently counted as PASS. Residual consumer limitations are recorded separately in #941 and #991; Word projections remain UNVERIFIED.

@codecov

codecov Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Comparison-package parity controls execute native code but their coverage excludes docx-core sources, leaving the CI report blind to the new move-mark and selected-bookmark branches. Add direct native tests with independent expected paragraph and bookmark projections, including middle/terminal endpoints, fully selected insertion content, empty/untracked/mixed survivors and foreign move history. Preserve all runtime behavior and revision-selection boundaries; do not make coverage green by changing exclusions or production code.

Ref: #985, #941
Independent dynamic review showed the loop headers were measured but wrapper-nested bookmark harvesting could be reverted with all tests green. Add source and destination cases with live counterparts outside selected wrappers, an unpaired spanning start, and accept-side empty/untracked/foreign/mixed survivors. The strengthened suite passes sixteen controls; reverted harvesting fails two cases and removing local pairing fails one, independently executed rather than inferred. Correct destination conformance citations and explicitly document the inherited authorless range-end test selection. Runtime and filter policy remain unchanged.

Ref: #985, #941
Preserve reviewed history while incorporating main's independently bounded public ILPA annotation cases. Repeat final integration gates and review before pushing this focused runtime repair; do not rewrite or bypass protected history.

Ref: #985
The independent integration review validated resolved output but found that the new native consumer fixtures omitted the schema-required range-start date. Add a fixed valid UTC date to the actual endpoint helper so these regressions start from schema-valid OOXML rather than accidentally malformed input. All sixteen control inputs and resolved outputs now pass the repository MCE-aware schema gate, and the test helper keeps moveFrom content as w:t. Preserve selectors, IDs, text, survivor guards and every mutation discriminator; no production code or timeout changes.

Ref: #985, #941
@stevenobiajulu

Copy link
Copy Markdown
Member Author

Final local release evidence for head ccff879aa769e5d46b296a3b1f089c1b8c954812, normally integrated with main 24b749524c62f8bf1c9852736482801a129c4b5c / shipped test-only CI blocker #995:

  • All six mandatory gates: PASS; 3,463 tests passed, 47 platform/opt-in skips. Required public corpus: all 21 tests passed, all 23 reviewed rows matched, no absent-corpus skip. Complete log: /private/tmp/985-schema-date-presubmit-final-20260917.log; process exited 0.
  • Independent dynamic Claude Opus 5: APPROVE on exact final head. Actual 16 native + 18 complete move-parity controls passed. Prior fresh mutations discriminate wrapper-nested bookmark harvesting, local pairing and native/AST survivor guards; the seven new direct controls are real regression coverage, not only coverage percentages.
  • Corrected schema-required range-start date in the shared test helper; assertions/runtime unchanged. Actual-helper-derived 16 inputs + 16 outputs validate 32/32 via the repository MCE-aware checker. Reviewer independently regenerated them, checked all 16 body strings against actual tests, and proved a negative control without dates fails exactly the 16 inputs.
  • Fresh built-code public SHA-pinned NVCA management-rights letter: native/AST Accept and Reject plus actual isolated LibreOffice projections match independent revised/original identity controls, all four reader states contain the expected 33 paragraphs. Tracked/accepted/rejected outputs validate 3/3. Log: /private/tmp/985-final-date-public-reader-20260917.log; process exited 0.
  • Limits remain explicit: terminal LibreOffice move behavior Characterize and resolve LibreOffice terminal move paragraph containers #973 is NOT repaired; spanning bookmark ownership and authorless selective range-end limitations remain open fix(docx-compare): emit schema-valid paragraph move markup #941; foreign property history fix(docx-core): preserve foreign paragraph-mark revisions during property rejection #991 remains open; section handling follows existing ins/del policy, not a general section-fidelity claim. Word projections remain UNVERIFIED. Public/synthetic only, no private documents or hosted API work.

Fresh CI/Codecov and actual advisory verdicts still required. Automerge is not armed, and this is not post-merge proof. Exact-merge own clean build/full gates/public real-document smoke, local visual evidence where meaningful, and production exact-SHA verification will follow merge.

Keep the native move fix current with the independently shipped terminal-mark cleanup without rewriting reviewed history. Re-run release gates against the combined source before shipping.

Ref: #985
@stevenobiajulu

Copy link
Copy Markdown
Member Author

Latest-main integration and final-head release verification

Exact head 9b1e1e443e26988044a8f3dd804abc56f410b0d6, normal merge of shipped main 9d82bab9 into reviewed ccff879a; no history rewrite. All six local gates passed: 3,496 tests passed / 47 documented platform or opt-in skips, plus required public corpus 21/21, all 23 reviewed manifest rows unchanged (366.49 seconds). Complete log /private/tmp/985-integration-993-presubmit-20260917.log, actual process exit0.

Independent dynamic Claude Opus5 at this exact head: APPROVE. Actual20 core +51 parity/cleanup controls pass, 32 newly regenerated actual-helper schema packages pass, and source-vs-both-parent interaction probes found no new regression. Complete collected 19,287-byte analysis .peer-review/985-integration-993-opus5-20260917.review.txt, actual model claude-opus-5 session 19d53e18-82f5-4e44-bde3-ecd325531ec9, exit0, tracked worktree clean. Extra scratch interaction probes are not all schema-certified or Word measurements.

Fresh built public API smoke: source SHA-verified against this head's NVCA Management Rights Letter manifest. Native and AST Accept/Reject match the corresponding original/revised text; actual LibreOffice dispatches on unresolved tracked input, compared against independently imported original/revised identity controls, each match 33/33 paragraph/heading texts. Tracked, native Accepted and native Rejected packages schema-validate 3/3. Local public-only log .peer-review/985-public-integration-993-reader.log; no document/render upload.

Limits remain explicit: #973 terminal-reader move structure is not fixed; #941 spanning/unpaired annotation cases and #991 foreign co-located revision-history handling remain separate. Native preserves untouched pre-existing empty property containers while AST may remove them; no universal byte-for-byte property parity claim. All Word projections remain UNVERIFIED. Normal push only; automerge remains unarmed until this head's CI and actual advisory verdicts finish. Exact-merge clean-build smoke/local meaningful renders/production exact-SHA proof still follow after merging. No hosted API or protection mutation.

@stevenobiajulu
stevenobiajulu merged commit 3949e60 into main Sep 18, 2026
27 checks passed
@stevenobiajulu
stevenobiajulu deleted the 985-reject-move-paragraph-marks-20260917 branch September 18, 2026 00:03
@stevenobiajulu

Copy link
Copy Markdown
Member Author

Post-merge smoke passed

Merged and freshly built in an isolated detached worktree: 3949e608db46fbef97868342fa41f47cf3fc1cd6 (PR #989 / #985), normal squash after every CI check and the actual advisory verdict passed. Exact merge tree verified identical to independently reviewed head 9b1e1e44. Primary checkout untouched.

Own locked dependencies and all six mandatory local gates passed: build, workspace lint, full suite 3,496 passed / 47 documented platform or opt-in skips, spec coverage, conformance citations and conformance document. Actual smoke process exit0 collected.

Real-world fixture: public NVCA Management Rights Letter, SHA-verified against this exact merge's manifest. Fresh built public compare API emits a genuine tracked paragraph move. Native and AST Accept/Reject match the corresponding source text. Actual LibreOffice dispatches on the unresolved tracked comparison, against independently imported original/revised identity controls, each match 33/33 paragraph/heading entries. Tracked/native Accepted/native Rejected packages pass repository MCE-aware schema validation 3/3. Required corpus 21/21 and all23 reviewed rows also passed at the tree-identical premerge head and in CI.

Visual evidence: fresh original/revised/Accept/Reject PDFs and physical page-two PNGs generated from this exact merge. Accept PNG is byte-identical to revised, Reject PNG byte-identical to original. Both projections inspected in-session and all four images opened locally. The moved paragraph's revised order and original restoration are visible; this is target-page evidence, not all-page/font/Word certification. Local-only artifacts: /private/tmp/automerge-smoke-989-20260917/.peer-review/985-public-renders/. No document or image uploaded. Nonfatal macOS render task-policy warning remains in the log; all four conversions and raster checks completed successfully.

Production docs/redirect deployment READY, actual dpl_B2ZBUz85BteAbt1yNyJ319WopUNE, metadata githubCommitSha=3949e608db46fbef97868342fa41f47cf3fc1cd6, githubCommitRef=main, verified read-only. No hosted API build/deployment or branch-protection change.

Limits remain: #973 terminal LibreOffice move structure, #941 spanning/selective range annotations and #991 foreign paragraph-mark history are not repaired. Word Accept/Reject remains UNVERIFIED, and open-cleanly is not a projection oracle. Worktrees retained because other follow-ups remain open; no cleanup or close-out.

Local log: /private/tmp/automerge-smoke-989-20260917.log; production receipt .peer-review/989-production-receipt.json in that worktree.

First 50 log lines
3949e608db46fbef97868342fa41f47cf3fc1cd6
npm warn deprecated node-domexception@1.0.0: Use your platform's native DOMException instead

added 617 packages, and audited 630 packages in 2s

155 packages are looking for funding
  run `npm fund` for details

7 vulnerabilities (6 moderate, 1 high)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.
npm warn install-scripts 2 packages have install scripts not yet covered by allowScripts:
npm warn install-scripts   esbuild@0.28.1 (postinstall: node install.js)
npm warn install-scripts   fsevents@2.3.3 (install: (install scripts present))
npm warn install-scripts
npm warn install-scripts Run `npm install-scripts ls` to review, or `npm install-scripts approve <pkg>` to allow.

> safe-docx-suite@0.20.1 build
> npm run build --workspaces --if-present


> @usejunior/docx-compare@0.20.1 build
> npm run build -w @usejunior/docx-core && npm run clean:dist && tsc -p tsconfig.build.json


> @usejunior/docx-core@0.20.1 build
> npm run clean:dist && tsc -p tsconfig.build.json


> @usejunior/docx-core@0.20.1 clean:dist
> node -e "const { rmSync } = require('node:fs'); rmSync('dist', { recursive: true, force: true });"


> @usejunior/docx-compare@0.20.1 clean:dist
> node -e "const { rmSync } = require('node:fs'); rmSync('dist', { recursive: true, force: true });"


> @usejunior/docx-core@0.20.1 build
> npm run clean:dist && tsc -p tsconfig.build.json


> @usejunior/docx-core@0.20.1 clean:dist
> node -e "const { rmSync } = require('node:fs'); rmSync('dist', { recursive: true, force: true });"

Last 50 log lines
PASS add-table-context-to-document-view: 4 scenarios covered by 4 story mappings
PASS add-text-export: 5 scenarios covered by 5 story mappings
PASS add-typescript-mcp-server: 21 scenarios covered by 21 story mappings
PASS document-paragraph-id-stability-and-fingerprint: 9 scenarios covered by 9 story mappings
PASS expose-comment-range-metadata: 4 scenarios covered by 4 story mappings
PASS forbid-untracked-ai-mutations: 8 scenarios covered by 8 story mappings
PASS guard-row-level-revision-resolution: 5 scenarios covered by 5 story mappings
PASS remove-comparison-from-default-save: 3 scenarios covered by 3 story mappings
PASS replace-plan-tools-with-batch-edit: 12 scenarios covered by 12 story mappings
PASS update-safe-docx-save-defaults-and-stable-node-ids: 12 scenarios covered by 12 story mappings
PASS update-safe-docx-sessionless-entry-and-session-controls: 11 scenarios covered by 12 story mappings (+1 bonus tests beyond spec)

> @usejunior/docx-core@0.20.1 check:spec-coverage-generation
> node scripts/validate_generation_openspec_coverage.mjs --strict

PASS docx-generation (canonical): 34 scenarios covered (+32 bonus tests beyond spec)
PASS add-custom-theme: 1 scenarios covered
PASS add-docx-generation: 34 scenarios covered
PASS add-generation-ancillary-parts: 1 scenarios covered (+2 bonus tests beyond spec)
PASS add-generation-baseline-settings: 1 scenarios covered
PASS add-generation-compare-roundtrip: 5 scenarios covered
PASS add-numbering-level-justification: 1 scenarios covered
PASS add-paragraph-borders: 2 scenarios covered
PASS add-run-highlight: 1 scenarios covered
PASS add-signature-and-keeplines: 1 scenarios covered
PASS remove-agreement-domain-recipes: 0 scenarios covered

> safe-docx-suite@0.20.1 check:conformance-citations
> node scripts/check_conformance_citations.mjs

check_conformance_citations: OK (121 registry entries, 51 XSDs)

> safe-docx-suite@0.20.1 check:conformance-doc
> node scripts/check_conformance_doc.mjs

generate_conformance_doc: wrote spec-compliance/CONFORMANCE.md
check_conformance_doc: OK
{"builtRoot":"/private/tmp/automerge-smoke-989-20260917","smoke":"985","publicFixtureOnly":true}
Public source SHA pin verified against exact-merge manifest.
{"fixture":"public NVCA management rights letter","nativeAccept":true,"nativeReject":true,"astAccept":true,"astReject":true,"readerAccept":true,"readerReject":true,"paragraphs":[33,33,33,33]}
3 of 3 document.xml instances validate against the Transitional WML schema
2026-09-17 19:15:33.357 soffice[69789:119107767] ApplePersistenceIgnoreState: Existing state will not be touched. New state will be written to /var/folders/0q/zgzp789j36b9gscddw1lyxp80000gn/T/org.libreoffice.script.savedState
2026-09-17 19:15:33.895 soffice[69789:119108524] ApplePersistenceIgnoreState: Existing state will not be touched. New state will be written to /var/folders/0q/zgzp789j36b9gscddw1lyxp80000gn/T/org.libreoffice.script.savedState
2026-09-17 19:15:34.657 soffice[69789:119108524] Task policy set failed: 4 ((os/kern) invalid argument)
convert /private/tmp/automerge-smoke-989-20260917/.peer-review/985-public-original.docx as a Writer document -> /private/tmp/automerge-smoke-989-20260917/.peer-review/985-public-renders/985-public-original.pdf using filter : writer_pdf_Export
convert /private/tmp/automerge-smoke-989-20260917/.peer-review/985-public-revised.docx as a Writer document -> /private/tmp/automerge-smoke-989-20260917/.peer-review/985-public-renders/985-public-revised.pdf using filter : writer_pdf_Export
convert /private/tmp/automerge-smoke-989-20260917/.peer-review/985-public-accept.docx as a Writer document -> /private/tmp/automerge-smoke-989-20260917/.peer-review/985-public-renders/985-public-accept.pdf using filter : writer_pdf_Export
convert /private/tmp/automerge-smoke-989-20260917/.peer-review/985-public-reject.docx as a Writer document -> /private/tmp/automerge-smoke-989-20260917/.peer-review/985-public-renders/985-public-reject.pdf using filter : writer_pdf_Export

{"builtRoot":"/private/tmp/automerge-smoke-989-20260917","issue":"985","dir":"/private/tmp/automerge-smoke-989-20260917/.peer-review/985-public-renders","images":["985-public-accept-2.png","985-public-original-2.png","985-public-reject-2.png","985-public-revised-2.png"],"publicOnly":true,"uploaded":false,"profile":"/private/tmp/safe-docx-public-render-cRw0bU"}

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(docx-core): characterize moveTo paragraph-mark Reject parity

1 participant