Skip to content

Align default document initialization ids#370

Merged
appflowy merged 4 commits into
mainfrom
fix/deterministic-default-document-init
Jun 26, 2026
Merged

Align default document initialization ids#370
appflowy merged 4 commits into
mainfrom
fix/deterministic-default-document-init

Conversation

@appflowy

@appflowy appflowy commented May 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • derive Web default document page and initial paragraph ids from UUID document ids to match collab server
  • use a stable document-derived Yjs init client for default document initialization, then restore the real client id
  • clean up paragraph children/text entries by their stored ids
  • add Yjs regression coverage for independently initialized default documents merging text edits

Tests

  • pnpm exec jest src/application/slate-yjs/tests/yjs-utils.test.ts --no-coverage --runInBand

Companion server PR: https://github.com/AppFlowy-IO/AppFlowy-Cloud-Premium/pull/681

Summary by Sourcery

Align default document initialization with server-generated IDs and deterministic client identification for collaborative editing.

New Features:

  • Derive deterministic initial paragraph, children, and text IDs from the document ID when initializing default documents.
  • Introduce a deterministic initial Yjs client ID based on the document ID while preserving the actual client ID after initialization.

Bug Fixes:

  • Ensure non-UUID document IDs are rejected for page ID derivation to avoid inconsistent identifier generation.
  • Clean up document-scoped paragraph children and text entries using their stored IDs when deleting blocks to prevent stale metadata.

Enhancements:

  • Add deterministic ID helpers derived from document IDs to mirror server-side behavior.
  • Stabilize Yjs document initialization by temporarily using a document-scoped client ID for initial structure creation.

Tests:

  • Extend Yjs utility tests to cover deterministic ID generation, UUID validation failures, document-scoped paragraph IDs, correct cleanup, and regression for merging edits from independently initialized default documents.

@sourcery-ai

sourcery-ai Bot commented May 26, 2026

Copy link
Copy Markdown

Reviewer's Guide

Align default document initialization IDs and Yjs client IDs with the Rust/collab server by deriving them deterministically from the document UUID, and adjust initialization/deletion logic and tests to ensure stable paragraph/children/text IDs and correct CRDT merging behavior.

Sequence diagram for deterministic default document initialization

sequenceDiagram
  participant Caller
  participant YDoc
  participant YSharedRoot as SharedRoot
  participant Utils as YjsUtils

  Caller->>Utils: initializeDocumentStructure(doc, documentId, includeInitialParagraph)
  alt documentId is provided
    Utils->>Utils: defaultDocumentInitClientId(documentId)
    Utils->>YDoc: set clientID to defaultDocumentInitClientId(documentId)
  end

  opt includeInitialParagraph
    Utils->>Utils: nanoidFromDocumentId(documentId, block)
    Utils->>Utils: nanoidFromDocumentId(documentId, children)
    Utils->>Utils: nanoidFromDocumentId(documentId, text)
    Utils->>YSharedRoot: sharedRoot.set(document)
  end

  alt documentId is provided
    Utils->>YDoc: restore original clientID
  end
Loading

File-Level Changes

Change Details Files
Make page ID and other derived IDs strictly UUID-based and introduce deterministic ID derivation helpers shared by page, client, and paragraph entities.
  • Change pageIdFromDocumentId to require a valid UUID string and remove fallback generation from arbitrary strings, matching the Rust implementation.
  • Add idFromDocumentId and nanoidFromDocumentId helpers that derive deterministic IDs from a document UUID and a role using uuidv5 and a Rust-compatible nanoid alphabet.
  • Export defaultDocumentInitClientId that deterministically derives a numeric Yjs client ID from a document UUID and role, clamped to be at least 1.
src/application/slate-yjs/utils/yjs.ts
Initialize default document structures using deterministic, document-scoped IDs and a temporary document-derived Yjs client ID, then restore the original client ID.
  • Extend initializeDocumentStructure to accept an initClientId derived from documentId and, when documentId is provided, temporarily set doc.clientID to that value while writing the initial document state, restoring the original client ID afterward.
  • Change initial paragraph block creation so that its block ID, children ID, and text ID are deterministic, document-scoped nanoids when a documentId is provided, while preserving the previous random behavior when documentId is absent.
  • Ensure children_map and text_map entries are keyed by the derived paragraph children/text IDs rather than the paragraph block ID itself, and still create page-level children and text entries.
src/application/slate-yjs/utils/yjs.ts
Ensure block deletion cleans up document-scoped children and text entries based on stored IDs rather than assuming they equal the block ID.
  • Update deleteBlock to read block_children and block_external_id from the block and use those to delete corresponding entries from childrenMap and textMap, instead of deleting by blockId.
  • Keep existing behavior of removing the block from blocks map, updating parent children arrays, and cascading deletion of empty column blocks.
src/application/slate-yjs/utils/yjs.ts
Add and adjust tests to validate deterministic ID behavior, stricter UUID handling, and correct Yjs merge semantics for independently initialized default documents.
  • Tighten pageIdFromDocumentId tests to assert a specific derived page UUID and to verify that non-UUID document IDs now throw a clear error.
  • Add a test for deterministic defaultDocumentInitClientId given a known document UUID.
  • Add initializeDocumentStructure tests that verify deterministic paragraph/children/text IDs when a documentId is provided, that two independently initialized docs with the same documentId merge concurrent text edits correctly, and that deleteBlock cleans up derived children/text IDs and pageChildren as expected.
src/application/slate-yjs/__tests__/yjs-utils.test.ts

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • Directly mutating doc.clientID in initializeDocumentStructure could be fragile if Yjs treats this as internal/readonly state; consider using a separate initialization doc or a Yjs-supported way to emulate a stable init client rather than reassigning the client ID on the existing instance.
  • Now that pageIdFromDocumentId and idFromDocumentId throw on invalid UUIDs, it may be worth reviewing/guarding call sites so that unexpected runtime errors are avoided and invalid IDs are rejected closer to their source.
  • In deleteBlock, the new cleanup logic assumes block_children and block_external_id are always present and valid; if older data or malformed blocks are ever encountered, you may want to null-check these IDs before using them to delete from childrenMap/textMap to avoid runtime errors.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Directly mutating `doc.clientID` in `initializeDocumentStructure` could be fragile if Yjs treats this as internal/readonly state; consider using a separate initialization doc or a Yjs-supported way to emulate a stable init client rather than reassigning the client ID on the existing instance.
- Now that `pageIdFromDocumentId` and `idFromDocumentId` throw on invalid UUIDs, it may be worth reviewing/guarding call sites so that unexpected runtime errors are avoided and invalid IDs are rejected closer to their source.
- In `deleteBlock`, the new cleanup logic assumes `block_children` and `block_external_id` are always present and valid; if older data or malformed blocks are ever encountered, you may want to null-check these IDs before using them to delete from `childrenMap`/`textMap` to avoid runtime errors.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@appflowy appflowy closed this Jun 4, 2026
@appflowy appflowy reopened this Jun 26, 2026
@appflowy appflowy merged commit bf47f63 into main Jun 26, 2026
9 of 13 checks passed
@appflowy appflowy deleted the fix/deterministic-default-document-init branch June 26, 2026 07:10
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