Skip to content

fix(document-api): align position readback with write-side offset model (SD-3109)#3243

Merged
caio-pizzol merged 1 commit into
mainfrom
caio-pizzol/SD-3108-fix-content-control-wrap-and-readback-positions
May 12, 2026
Merged

fix(document-api): align position readback with write-side offset model (SD-3109)#3243
caio-pizzol merged 1 commit into
mainfrom
caio-pizzol/SD-3108-fix-content-control-wrap-and-readback-positions

Conversation

@caio-pizzol
Copy link
Copy Markdown
Contributor

Position readback (bookmarks.list, and a sibling helper in the permission-ranges adapter) was using raw PM arithmetic β€” pos - resolved.start(depth) β€” which counts the inline wrapper boundary tokens (run, etc.) that the write side's flattened text-offset model intentionally skips. So a bookmark inserted at flattened offset 19 read back at offset 22 in a doc with run-wrapped text. Fix routes the read path through pmPositionToTextOffset so write and read share one offset model.

  • Two regression tests, both integration-shaped: the SD-3109 one writes a bookmark via the public API and asserts the readback round-trips; the SD-3108 one is a guardrail set for content-control wrap by text offset (SD-3108 did not reproduce on main, so the tests stay as guardrails rather than a fix).
  • Verified at three layers: vitest (full document-api-adapters suite, 3373/3373), unit (24 bookmark-resolver tests, 33 bookmark-wrappers tests), and the dev app + Chrome DevTools (round-trip works for both a freshly-inserted block and a paragraph 410 chars deep in the Memorandum fixture).
  • The duplicated readback helper in permission-ranges-adapter.ts is a separate cleanup target β€” sharing one implementation between the two adapters would obsolete the cross-reference comment naturally. Leaving that for a later pass to keep this PR scoped to the readback fix.

Review: confirm the AIDEV-NOTE wording is the right anchor β€” it has to survive future "simplify" passes that might revert to the naive arithmetic.

…el (SD-3109)

The Document API write side accepts text offsets in a flattened model: text contributes its length, leaf atoms contribute 1, inline wrappers (run, etc.) contribute 0. Position readback was using raw PM arithmetic (`pos - resolved.start(depth)`), which counts the wrapper boundary tokens the flattened model skips. Result: `editor.doc.bookmarks.list()` returned offsets that drifted by the number of inline wrappers in the targeted block.

- Route both readback helpers through `pmPositionToTextOffset` so read and write share one offset model.
- Same fix applied in `permission-ranges-adapter.ts`, which carried a parallel copy of the buggy helper.
- Regression test for the round-trip invariant (write at offset N, list returns offset N).
- Includes regression coverage for SD-3108 (content-control wrap by text offset). That bug did not reproduce on main; the tests stay as guardrails.
@caio-pizzol caio-pizzol requested a review from a team as a code owner May 12, 2026 10:08
@linear
Copy link
Copy Markdown

linear Bot commented May 12, 2026

SD-3109

SD-3108

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

βœ… All modified and coverable lines are covered by tests.

πŸ“’ Thoughts on this report? Let us know!

@caio-pizzol caio-pizzol merged commit c5d04d8 into main May 12, 2026
67 checks passed
@caio-pizzol caio-pizzol deleted the caio-pizzol/SD-3108-fix-content-control-wrap-and-readback-positions branch May 12, 2026 12:47
@superdoc-bot
Copy link
Copy Markdown
Contributor

superdoc-bot Bot commented May 12, 2026

πŸŽ‰ This PR is included in @superdoc-dev/react v1.2.0-next.118

The release is available on GitHub release

@superdoc-bot
Copy link
Copy Markdown
Contributor

superdoc-bot Bot commented May 12, 2026

πŸŽ‰ This PR is included in vscode-ext v2.3.0-next.120

@superdoc-bot
Copy link
Copy Markdown
Contributor

superdoc-bot Bot commented May 12, 2026

πŸŽ‰ This PR is included in @superdoc-dev/mcp v0.3.0-next.75

The release is available on GitHub release

@superdoc-bot
Copy link
Copy Markdown
Contributor

superdoc-bot Bot commented May 12, 2026

πŸŽ‰ This PR is included in superdoc v1.30.0-next.74

The release is available on GitHub release

@superdoc-bot
Copy link
Copy Markdown
Contributor

superdoc-bot Bot commented May 12, 2026

πŸŽ‰ This PR is included in superdoc-sdk v1.8.0-next.74

@superdoc-bot
Copy link
Copy Markdown
Contributor

superdoc-bot Bot commented May 12, 2026

πŸŽ‰ This PR is included in superdoc-cli v0.8.0-next.92

The release is available on GitHub release

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants