Docs: Add design note: local caret anchoring (issue #237)#257
Merged
hackerwins merged 6 commits intoMay 31, 2026
Conversation
📝 WalkthroughWalkthroughThis PR adds design docs and frontend changes to anchor and restore local caret/selection across remote edits using Yorkie Tree–anchored representations, plus editor IME composition lifecycle hooks and YorkieDocStore conversion/resolution helpers for deterministic fallback handling. ChangesLocal Caret Anchoring
Sequence Diagram(s)sequenceDiagram
participant Frontend as Frontend UI
participant DocsView as DocsView
participant YorkieStore as YorkieDocStore
participant Editor as Editor/TextEditor
Frontend->>YorkieStore: updateCursorPos(cursor, selection)
Frontend->>YorkieStore: setCompositionStart(pos) (on compositionStart)
DocsView->>YorkieStore: resolveAnchoredLocalCursor()
YorkieStore->>DocsView: {cursor, selection, compositionStart}
DocsView->>Editor: restoreLocalCursor(cursor, selection) (handle composing)
Editor->>YorkieStore: publishResolvedLocalCursor(resolved)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/design/docs/docs-local-caret-anchoring.md (1)
244-250: ⚡ Quick winAdd two-client integration coverage for non-collapsed selection drift.
The integration section validates caret anchoring, but Goal lines 48-49 also require anchored selection endpoints. Add at least one two-client scenario where User B has a non-collapsed selection and User A inserts/deletes upstream, asserting both
anchorandfocusresolve correctly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/docs/docs-local-caret-anchoring.md` around lines 244 - 250, Add a two-client integration test that covers non-collapsed selection drift: simulate two clients opening the same document (as in the existing caret anchoring tests), have User B create a non-collapsed selection (both anchor and focus set in the same paragraph), then have User A perform upstream edits (inserts and a deletion) and assert that both selection.anchor and selection.focus are correctly re-resolved on User B after synchronization; update the integration test harness used by the caret anchoring tests (the same scenario setup code) and assert both endpoints for a plain paragraph and one table-cell/header/footer paragraph variant to satisfy Goal lines 48–49.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/docs/docs-local-caret-anchoring.md`:
- Around line 150-153: The design doc currently leaves caret tie-breaking
underspecified; make the tie-breaking policy normative by explicitly stating
that we adopt Yorkie Tree position semantics for same-boundary conflicts (i.e.,
specify whether the local caret resolves before or after concurrently-inserted
remote text) and define the exact fallback order for key conflicts
(same-boundary affinity then the documented fallback). Update the sections that
defer to implementation (the passages around "tie-breaking", "same-boundary
affinity", and "fallback order") to state the single canonical rule and add a
short illustrative example and a deterministic test expectation that asserts
caret placement under concurrent inserts; reference the phrases "tie-breaking",
"same-boundary affinity", and the document's existing Yorkie Tree mention to
locate the edits.
---
Nitpick comments:
In `@docs/design/docs/docs-local-caret-anchoring.md`:
- Around line 244-250: Add a two-client integration test that covers
non-collapsed selection drift: simulate two clients opening the same document
(as in the existing caret anchoring tests), have User B create a non-collapsed
selection (both anchor and focus set in the same paragraph), then have User A
perform upstream edits (inserts and a deletion) and assert that both
selection.anchor and selection.focus are correctly re-resolved on User B after
synchronization; update the integration test harness used by the caret anchoring
tests (the same scenario setup code) and assert both endpoints for a plain
paragraph and one table-cell/header/footer paragraph variant to satisfy Goal
lines 48–49.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d5cda7e9-4ca8-449b-8344-0b6f7d2acd5d
📒 Files selected for processing (2)
docs/design/README.mddocs/design/docs/docs-local-caret-anchoring.md
semimikoh
force-pushed
the
docs/local-caret-anchoring
branch
from
May 17, 2026 13:39
01d0c9f to
ee2a3e8
Compare
Collaborator
|
Thanks for the careful note, @semimikoh. Direction is right and the Doc updates1. Pin the Yorkie API. Open Question #1 is answered by the official Note that headers, body, and footers all live in the same State as "verify-then-pin during implementation": same-boundary 2. Make affinity normative (L149–L153). Replace "implementation Nice-to-have
Implementation in this PRPlease carry the implementation here rather than opening a new PR — the
|
hackerwins
marked this pull request as draft
May 18, 2026 15:59
semimikoh
force-pushed
the
docs/local-caret-anchoring
branch
from
May 26, 2026 04:52
e268fd1 to
a42db6b
Compare
Contributor
Author
|
Thanks @hackerwins — implementation is now in this PR alongside the design updates you asked for. Design doc (
Implementation:
Logistics:
One small question on logistics: do you want the PR title updated to reflect that this is no longer design-only (e.g. 🤖 Generated with Claude Code |
hackerwins
marked this pull request as ready for review
May 27, 2026 16:12
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
hackerwins
force-pushed
the
main
branch
2 times, most recently
from
May 30, 2026 17:23
9c3e5b8 to
7805ddb
Compare
The local caret in the docs editor is stored as an absolute
{ blockId, offset } and is not transformed when a remote peer
edits the same block. When User A inserts text upstream of User
B's caret, B's offset stays put and ends up pointing at the
wrong character (issue wafflebase#237).
This note records the proposed direction before implementation,
per the maintainer's request: anchor the local caret and
selection to Yorkie Tree positions in the frontend collaboration
layer, and resolve back to DocPosition only at render time. It
spells out the edge-case behavior across body, headers, footers,
tables, and selection paths so the implementation PR can land
without scope debate.
Local caret and text-selection endpoints are stored as absolute
{ blockId, offset } today, so a remote peer editing upstream text in the
same block leaves the caret pointing at the wrong character (wafflebase#237).
Convert each endpoint to a Yorkie Tree position via
tree.indexRangeToPosRange when the cursor is captured and resolve back
to DocPosition with tree.posRangeToIndexRange on every remote change so
the caret follows its logical text boundary.
Yorkie-specific anchor state stays in the frontend collaboration layer;
packages/docs continues to receive only resolved DocPosition values.
When an anchor's block is removed by a concurrent edit, the resolver
walks a deterministic fallback ladder — same block, end of previous
region block, start of next region block, first editable region/body
block — so two clients in the same race converge.
Add focused unit tests for body, header, footer, and table-cell anchors
across insert/delete/split/merge edge cases, plus two-client integration
tests for caret/selection drift in body, table-cell, header, and footer
paragraphs.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The text editor stores composition.startPosition as an absolute
{ blockId, offset } captured on compositionstart. When a remote peer
inserts text upstream during a Korean/Japanese/Chinese composition, the
stored offset no longer matches the logical boundary, so the next
composing-text replacement or the final commit lands at the wrong
character.
Wire the same anchor mechanism used for the live cursor: the text
editor emits onCompositionStart / onCompositionEnd events, the
collaboration layer captures a Yorkie Tree anchor for the start
position, and on every remote change the anchor is resolved and pushed
back via EditorAPI.updateCompositionStartPosition so the next composing
operation targets the drift-corrected offset.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Pin the design doc to the concrete Yorkie Tree APIs the implementation uses (indexRangeToPosRange / posRangeToIndexRange), make the same-boundary affinity normative, spell out the deterministic fallback ladder, and call out split / merge as a documented limitation since the current splitBlock / mergeBlock paths use delete+insert rather than Yorkie's native splitLevel and cannot preserve TreePos through deleted content. Trim the Open Questions list to the two that remain genuinely open (shared anchored representation for peer presence, fallback ordering preference) and capture the implementation notes in active task files. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Two follow-ups surfaced after rebasing onto main: 1. main's frontend test runner moved from node:test to vitest. The anchor unit tests added in this branch still called the node:assert API (assert.deepEqual / assert.equal / assert.ok), which crashed with "assert is not defined" under vitest. Rewrite the new "local caret anchoring" tests to vitest's expect() so they line up with every other test in the file. 2. isDebug() in YorkieDocStore short-circuits on `typeof localStorage === 'undefined'`, but Node environments that define `localStorage` as a partial stub (no `getItem`) bypass that guard and throw `TypeError: localStorage.getItem is not a function` the moment any code path touches insertText / deleteText / splitBlock / mergeBlock. Tighten the guard so isDebug() also bails out when `getItem` is not a function. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The previous resolveAnchoredLocalCursor() did two things behind one name: resolved the locally anchored caret / selection back to DocPosition AND wrote them into Yorkie presence as a side effect. That made the function unsafe to call from tests or any code path that only wants to read the resolved positions — every call broadcast a presence update. Make the resolve path a pure read and introduce an explicit publishResolvedLocalCursor() that callers (currently the onRemoteChange handler in DocsView) opt into. Behavior is unchanged: the handler still pushes the resolved cursor into the editor and then broadcasts it. Tests now exercise resolve without triggering presence writes. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
hackerwins
force-pushed
the
docs/local-caret-anchoring
branch
from
May 31, 2026 04:35
a42db6b to
cd5e7c3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/docs/docs-local-caret-anchoring.md`:
- Around line 342-343: The sentence "Should fallback after a deleted block
prefer visual proximity, document order, or region-local order?" conflicts with
the earlier normative fallback ladder and should not be presented as an
alternative current policy; reword this question to clearly mark it as a future
design consideration or open research item (for example, prefix with "Future
work:" or "To be revisited:") and explicitly state it does not override the
existing region-local deterministic fallback described earlier so there is no
ambiguity between the question and the canonical fallback ladder.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3bcc43fe-bd48-48ef-91b2-a6b9c5f0941e
📒 Files selected for processing (8)
docs/design/README.mddocs/design/docs/docs-local-caret-anchoring.mddocs/tasks/active/20260519-docs-local-caret-anchoring-lessons.mddocs/tasks/active/20260519-docs-local-caret-anchoring-todo.mdpackages/docs/src/view/editor.tspackages/docs/src/view/text-editor.tspackages/frontend/src/app/docs/docs-view.tsxpackages/frontend/src/app/docs/yorkie-doc-store.ts
💤 Files with no reviewable changes (4)
- packages/frontend/src/app/docs/docs-view.tsx
- packages/docs/src/view/text-editor.ts
- packages/docs/src/view/editor.ts
- packages/frontend/src/app/docs/yorkie-doc-store.ts
✅ Files skipped from review due to trivial changes (3)
- docs/tasks/active/20260519-docs-local-caret-anchoring-todo.md
- docs/tasks/active/20260519-docs-local-caret-anchoring-lessons.md
- docs/design/README.md
Comment on lines
+342
to
+343
| - Should fallback after a deleted block prefer visual proximity, document order, | ||
| or region-local order? |
There was a problem hiding this comment.
Avoid re-opening fallback policy already defined as canonical.
This open question conflicts with the earlier normative fallback ladder (region-local deterministic order). Please reword it to avoid ambiguity (e.g., frame it as a future redesign possibility rather than current behavior).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/docs/docs-local-caret-anchoring.md` around lines 342 - 343, The
sentence "Should fallback after a deleted block prefer visual proximity,
document order, or region-local order?" conflicts with the earlier normative
fallback ladder and should not be presented as an alternative current policy;
reword this question to clearly mark it as a future design consideration or open
research item (for example, prefix with "Future work:" or "To be revisited:")
and explicitly state it does not override the existing region-local
deterministic fallback described earlier so there is no ambiguity between the
question and the canonical fallback ladder.
hackerwins
self-requested a review
May 31, 2026 05:50
hackerwins
approved these changes
May 31, 2026
hackerwins
left a comment
hackerwins
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for your contribution.
hackerwins
added a commit
that referenced
this pull request
May 31, 2026
PRs #257/#301/#303/#304/#305/#306/#307/#309/#315/#316/#317/#321/#322 shipped between v0.4.2 and the v0.4.3 cut, leaving their paired todo files in active with the process checkboxes still open. Flip the remaining boxes for the work that landed, then move the paired todo/lessons into docs/tasks/archive/2026/05/ via pnpm tasks:archive. Active count drops from 17 to 6; archive count grows by 21 files. Patch release covering 29 commits since v0.4.2. Headline theme is Slides editing & UX polish toward Google Slides / PowerPoint parity: smart guides (equal-spacing / distance / size), Shift drag modifiers, text inside shapes, a Format options right panel, tier-1 universal toolbar controls, text autofit (shrink + grow), group-selection visuals, and several smaller text-box / selection fixes. PPTX import gained paragraph-level bullet styles, body anchor (vertical alignment), and stretched blipFill cover-crop. Docs added a pending-inline-style at collapsed caret, a caret that tracks the resolved text color, bullet indent on Tab / Shift+Tab, font / size / line-spacing / clear formatting toolbar controls, an inline-style-attribute clear that actually lands in the CRDT, and a polished toolbar trigger / color swatch pass shared across docs / sheets / slides. Sheets gained .xlsx import. Infrastructure: Docker images now build on native arm64 runners, fixing the release hang seen on v0.4.2. No Prisma migration, no new backend env vars — the devops bump is a routine image-tag change. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
semimikoh
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The local caret in the docs editor is stored as an absolute
{ blockId, offset }. When a remote peer edits the same block, therender cache is invalidated but the local offset is not transformed, so
the caret ends up pointing at the wrong character (issue #237).
This PR lands both the design note and its implementation: local
cursor, non-collapsed selection endpoints, and the IME composition
start position are anchored to Yorkie Tree positions in the frontend
collaboration layer and resolved back to
DocPositiononly at rendertime.
Summary
docs/design/docs/docs-local-caret-anchoring.mdpins theYorkie API (
indexRangeToPosRange/posRangeToIndexRange), makesthe same-boundary affinity rule normative, documents the deterministic
fallback ladder, and records
splitBlock/mergeBlockas a knownlimitation (current
delete + insertimplementation cannot preserveTreePosthrough deleted content; nativesplitLevelis afollow-up).
YorkieDocStorecapturesAnchoredDocPosition/AnchoredDocRangeon cursor change, resolves both on every remote document change, and
walks a five-step fallback (same block → previous region block end →
next region block start → first editable region block → first
editable body block) when the anchor's block was removed.
TextEditoremitsonCompositionStart/onCompositionEnd; thecollaboration layer captures a composition anchor and pushes the
drift-corrected start position back through
EditorAPI.updateCompositionStartPositionso IME commits never landat a stale absolute offset.
packages/frontend;packages/docsstill receives only resolvedDocPositionvalues.Why
The maintainer confirmed in #237 that the absolute-offset drift is a
bug and pointed at anchoring through Yorkie Tree positions. The review
on this PR also asked for two doc updates that are now reflected in the
note: pin the concrete API surface and make the affinity rule
normative. The implementation lands here rather than in a follow-up
because the maintainer asked to keep design and implementation in the
same PR.
Linked Issues
Fixes #237.
Author checklist
Verification
Local
pnpm verify:fastruns green on each of the three commits inthis branch.
Risk Assessment
before this PR, a remote insert before the local caret left the caret
pointing at the wrong character; after, the caret follows the
intended logical boundary. The change is scoped to docs and does not
touch sheets/slides.
API additions (
resolveAnchoredLocalCursor,setCompositionStart)and
EditorAPIadditions (restoreLocalCursor, composition hooks)are net-new and not depended on by callers outside this PR.
Notes for Reviewers
behavior change is "the caret no longer drifts under concurrent
remote inserts" and is exercised by the two-client integration tests
rather than by a screenshot.
splitBlock/mergeBlockusedelete + insertagainst the Yorkie Tree rather than native
splitLevel=2. Anchorswhose target text is removed by a remote split or merge therefore go
through the deterministic fallback ladder instead of preserving the
logical offset through CRDT semantics. Switching to native
split/merge is tracked in Known Follow-Ups and would also need to
restore the undo/redo regression previously documented under
splitLevel=2.design note and the implementation (anchor data flow, fallback
ladder, IME wiring, tests, doc updates). I reviewed every line and
own the technical choices — happy to defend or revise any of them.
Summary by CodeRabbit
Release Notes
New Features
Documentation