Skip to content

feat: add w:pPrChange translator for paragraph property tracked changes (SD-2417)#2705

Merged
caio-pizzol merged 7 commits intomainfrom
luccas/sd-2417-investigation-structural-tracked-changes-dropped-on-import
Apr 22, 2026
Merged

feat: add w:pPrChange translator for paragraph property tracked changes (SD-2417)#2705
caio-pizzol merged 7 commits intomainfrom
luccas/sd-2417-investigation-structural-tracked-changes-dropped-on-import

Conversation

@luccas-harbour
Copy link
Copy Markdown
Contributor

@luccas-harbour luccas-harbour commented Apr 2, 2026

Summary

  • Add a new w:pPrChange translator that encodes/decodes tracked changes on paragraph properties (author, date, id) along with the nested w:pPr containing the previous properties
  • Register the translator as a child of w:pPr and in the global handler index
  • Extract base pPr property translators into a shared module to avoid a circular dependency between pPr ↔ pPrChange

@linear
Copy link
Copy Markdown

linear Bot commented Apr 2, 2026

SD-2417 Investigation: Structural tracked changes dropped on import and export

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 2, 2026

Status: PASS

The w:pPrChange translator looks correct. All three attributes (w:id, w:author, w:date) match the spec exactly — correct names, correct types (w:id as integer, w:date as ST_DateTime string). The single child element w:pPr is the only valid child, and the translator handles that correctly.

One minor note: w:id is required by the spec (the document is non-conformant if omitted). The current decoder has an interesting behavior — if a change object has only { id, author, date } with no paragraphProperties, the decode returns undefined (as shown in the test at line ~145). That means a pPrChange with no prior paragraph properties (empty <w:pPr/>) won't round-trip cleanly. The spec explicitly allows an empty <w:pPr/> to mean "no prior properties existed." Whether that matters depends on whether this code path is exercised in practice, but it's worth noting for future work.

See https://ooxml.dev/spec?q=pPrChange for full spec details.

@luccas-harbour luccas-harbour force-pushed the luccas/sd-2417-investigation-structural-tracked-changes-dropped-on-import branch from 33e5e5c to 3e946ec Compare April 2, 2026 16:15
@luccas-harbour luccas-harbour self-assigned this Apr 2, 2026
@luccas-harbour luccas-harbour marked this pull request as ready for review April 2, 2026 16:16
@luccas-harbour
Copy link
Copy Markdown
Contributor Author

luccas-harbour commented Apr 2, 2026

The current decoder has an interesting behavior — if a change object has only { id, author, date } with no paragraphProperties, the decode returns undefined (as shown in the test at line ~145). That means a pPrChange with no prior paragraph properties (empty <w:pPr/>) won't round-trip cleanly. The spec explicitly allows an empty <w:pPr/> to mean "no prior properties existed." Whether that matters depends on whether this code path is exercised in practice, but it's worth noting for future work.

This issue was fixed.

caio-pizzol
caio-pizzol reviewed Apr 3, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@caio-pizzol caio-pizzol left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@luccas-harbour clean structure — sharing the base translators between pPr and pPrChange is a nice call.

one issue: when the original <w:pPr/> inside <w:pPrChange> is empty, it gets lost on round-trip. the decode guard you added works, but the encode side never feeds it — the key gets dropped for empty elements. tested with a Word file — the empty <w:pPr/> disappears after export.

pPrChange-empty-pPr.docx

also a minor DX thing and a pre-existing ordering note — details in inline comments.

tests look good. left a few comments.

Comment thread ...r-editor/src/editors/v1/core/super-converter/v3/handlers/w/pPrChange/pPrChange-translator.js Outdated
Comment thread packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/pPr/pPr-translator.js
Comment thread packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/utils.js
@luccas-harbour
fix(pPrChange): preserve empty w:pPr on round-trip
aeb4024 
An empty <w:pPr/> inside <w:pPrChange> means "the paragraph previously
had no properties" per the OOXML spec, but the encoder was dropping it:
pPrTranslator.encode() returns undefined for an empty pPr, so the
paragraphProperties key never landed in the encoded change. On decode,
the existing fallback couldn't fire because the key was absent, and the
element vanished on export.

Fall back to an empty object when the inner encode returns undefined,
so the key is always present when a <w:pPr> was in the source XML. The
decode path already reconstructs the empty <w:pPr/> from this shape.
@luccas-harbour
chore(super-converter): drop unused emitWhenAttributesOnly option
b511ab1
@luccas-harbour
fix(super-converter): order sectPr before pPrChange in w:pPr
6df83ac 
CT_PPr requires sectPr to precede pPrChange, but the exporter always
appended sectPr as the last child. Before the w:pPrChange translator
existed, no export contained both children, so the ordering bug was
latent. With pPrChange now round-tripping, paragraphs that carry both a
section break and a tracked property change were emitted in an order
Word rejects.

Insert sectPr before any existing w:pPrChange child; otherwise append
as before.
@luccas-harbour
Merge branch 'main' into luccas/sd-2417-investigation-structural-trac…
c25c3bf 
…ked-changes-dropped-on-import
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

caio-pizzol and others added 2 commits April 22, 2026 11:00
@caio-pizzol
Merge branch 'main' into luccas/sd-2417-investigation-structural-trac…
c6f09d3 
…ked-changes-dropped-on-import
test(pPrChange): cover mixed sectPr and pPr routing (SD-2417)
8c07f00 
- pPrChange-translator: add a round-trip that mixes sectPr with other
  paragraph properties so a regression dropping either half surfaces.
- pPr-translator: add two tests that exercise the pPrChange registration
  end-to-end through the real pPr translator, guarding against a silent
  SD-2417 regression if the registration is ever removed.
caio-pizzol
caio-pizzol approved these changes Apr 22, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@caio-pizzol caio-pizzol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm - round-1 items are all cleanly addressed. pushed 8c07f00 with the two test regression guards i flagged (mixed sectPr + other props, and pPrChange routing through wPPrTranslator); revert if you'd prefer your own take.

@caio-pizzol caio-pizzol merged commit 9adf23e into main Apr 22, 2026
57 of 77 checks passed
@caio-pizzol caio-pizzol deleted the luccas/sd-2417-investigation-structural-tracked-changes-dropped-on-import branch April 22, 2026 15:09
@superdoc-bot
Copy link
Copy Markdown
Contributor

superdoc-bot Bot commented Apr 22, 2026

🎉 This PR is included in vscode-ext v2.3.0-next.41

@superdoc-bot
Copy link
Copy Markdown
Contributor

superdoc-bot Bot commented Apr 22, 2026

🎉 This PR is included in @superdoc-dev/react v1.2.0-next.39

The release is available on GitHub release

@superdoc-bot
Copy link
Copy Markdown
Contributor

superdoc-bot Bot commented Apr 22, 2026

🎉 This PR is included in template-builder v1.6.0-next.4

The release is available on GitHub release

@superdoc-bot
Copy link
Copy Markdown
Contributor

superdoc-bot Bot commented Apr 22, 2026

🎉 This PR is included in esign v2.3.0-next.41

The release is available on GitHub release

@superdoc-bot
Copy link
Copy Markdown
Contributor

superdoc-bot Bot commented Apr 22, 2026

🎉 This PR is included in superdoc-cli v0.8.0-next.4

The release is available on GitHub release

@superdoc-bot
Copy link
Copy Markdown
Contributor

superdoc-bot Bot commented Apr 22, 2026

🎉 This PR is included in superdoc v1.28.0-next.4

The release is available on GitHub release

@superdoc-bot
Copy link
Copy Markdown
Contributor

superdoc-bot Bot commented Apr 22, 2026

🎉 This PR is included in superdoc-sdk v1.6.0-next.40

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

Reviewers

@caio-pizzol caio-pizzol caio-pizzol approved these changes

@harbournick harbournick Awaiting requested review from harbournick

@tupizz tupizz Awaiting requested review from tupizz

@artem-harbour artem-harbour Awaiting requested review from artem-harbour

@VladaHarbour VladaHarbour Awaiting requested review from VladaHarbour

@chittolinag chittolinag Awaiting requested review from chittolinag

Assignees

@luccas-harbour luccas-harbour

Labels

released on @next review: quick

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@luccas-harbour @codecov-commenter @caio-pizzol