feat: improve offline editing and page navigation#22
Conversation
Greptile SummaryThis PR upgrades offline collaborative editing from REST-body replacement to CRDT-native Yjs state persistence, adds per-account last-selected-space memory, shows an author byline in the editor, and adds a TestFlight badge to the README.
Confidence Score: 4/5The CRDT offline editing path is safe to merge; the one ordering edge case in the synchronizer is unlikely to trigger with Hocuspocus's typical event delivery order. The CRDT save-and-replay pipeline is well-exercised by new unit and integration tests covering coalescing, conflict resolution, round-trip serialization, and end-to-end replay. The docmostly/Features/Editor/NativeEditorOfflineCRDTSynchronizer.swift — the event ordering assumption in Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant VM as NativeRichEditorViewModel
participant AS as AppState
participant Q as OfflineMutationQueue
participant DB as CachedCRDTDocument
participant Sync as NativeEditorOfflineCRDTSynchronizer
participant Server as Collaboration Server
Note over VM,Server: Online save path
VM->>AS: updateCollaborativePageTitle(crdtStateUpdate:)
AS->>DB: persistCRDTStateUpdate(pageID:update:)
AS->>Server: REST page update
Server-->>AS: updated page
Note over VM,Server: Offline save path
VM->>AS: updateCollaborativePageTitle(crdtStateUpdate:)
AS->>Q: supersedePendingCRDTPageUpdate(stateUpdate:)
Q-->>AS: .enqueued / .superseded
AS->>DB: persistCRDTStateUpdate(pageID:update:)
Note over AS,Server: Queue replay (back online)
AS->>Q: next .updatePageCRDT mutation
AS->>AS: makeDocumentEngine then applyRemoteUpdate(queuedState)
AS->>AS: flushPendingLocalChanges then preparedState
AS->>DB: persistCRDTStateUpdate(preparedState)
AS->>Server: GET /api/auth/collab-token
Server-->>AS: token
AS->>Sync: synchronize(engine, url, token)
Sync->>Server: WebSocket Yjs sync
Server-->>Sync: authenticated + syncStatus(true)
Sync-->>AS: done
AS->>DB: persistCRDTStateUpdate(mergedState)
AS->>Server: GET /api/pages/info title reconcile
Server-->>AS: serverPage
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant VM as NativeRichEditorViewModel
participant AS as AppState
participant Q as OfflineMutationQueue
participant DB as CachedCRDTDocument
participant Sync as NativeEditorOfflineCRDTSynchronizer
participant Server as Collaboration Server
Note over VM,Server: Online save path
VM->>AS: updateCollaborativePageTitle(crdtStateUpdate:)
AS->>DB: persistCRDTStateUpdate(pageID:update:)
AS->>Server: REST page update
Server-->>AS: updated page
Note over VM,Server: Offline save path
VM->>AS: updateCollaborativePageTitle(crdtStateUpdate:)
AS->>Q: supersedePendingCRDTPageUpdate(stateUpdate:)
Q-->>AS: .enqueued / .superseded
AS->>DB: persistCRDTStateUpdate(pageID:update:)
Note over AS,Server: Queue replay (back online)
AS->>Q: next .updatePageCRDT mutation
AS->>AS: makeDocumentEngine then applyRemoteUpdate(queuedState)
AS->>AS: flushPendingLocalChanges then preparedState
AS->>DB: persistCRDTStateUpdate(preparedState)
AS->>Server: GET /api/auth/collab-token
Server-->>AS: token
AS->>Sync: synchronize(engine, url, token)
Sync->>Server: WebSocket Yjs sync
Server-->>Sync: authenticated + syncStatus(true)
Sync-->>AS: done
AS->>DB: persistCRDTStateUpdate(mergedState)
AS->>Server: GET /api/pages/info title reconcile
Server-->>AS: serverPage
Reviews (1): Last reviewed commit: "feat: polish page navigation and discove..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| .padding(.horizontal, 10) | ||
| .padding(.vertical, 6) |
There was a problem hiding this comment.
.padding(.horizontal, 10) and .padding(.vertical, 6) are literal point values. The project style guide requires avoiding hardcoded padding and spacing in favor of system-default spacing so the layout can adapt to Dynamic Type and platform conventions. Consider removing both explicit padding calls or using .padding(.horizontal) / .padding(.vertical) with no argument to accept the default inset.
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| } | ||
|
|
||
| private func waitForSynchronization( | ||
| events: AsyncThrowingStream<NativeEditorCollaborationEvent, any Error> | ||
| ) async throws { | ||
| try await withThrowingTaskGroup(of: Void.self) { group in | ||
| group.addTask { | ||
| var hasWriteAccess = false | ||
| for try await event in events { | ||
| switch event { | ||
| case .authenticated(let scope): | ||
| guard scope.allowsLocalDocumentUpdates else { | ||
| throw NativeEditorOfflineCRDTSyncError.writeAccessRequired | ||
| } | ||
| hasWriteAccess = true | ||
| case .syncStatus(true) where hasWriteAccess: | ||
| return | ||
| case .awareness, .stateless, .syncStatus: | ||
| continue | ||
| } | ||
| } |
There was a problem hiding this comment.
syncStatus(true) before authenticated silently drops the sync completion
hasWriteAccess starts as false. If the stream delivers .syncStatus(true) before the .authenticated event — possible if the server signals sync completion mid-handshake — the where hasWriteAccess guard fails and the event is swallowed by the case .awareness, .stateless, .syncStatus: continue arm. A subsequent .authenticated sets the flag, but no further syncStatus(true) will arrive, so the loop drains until the stream closes and throws disconnectedBeforeSync. The replay then propagates that error and the mutation stays in the offline queue, even though the actual sync succeeded. The 30-second timeout also fires, blocking the reconciliation loop unnecessarily.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a24795903
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| throw OfflinePageUpdateReplayConflict(pageID: pageId) | ||
| } | ||
|
|
||
| _ = try await saveLocalEditableDraft(pageId: pageId, title: persistedTitle, document: document) |
There was a problem hiding this comment.
When an offline CRDT edit reconnects after another collaborator has changed the document, synchronize(...) can merge remote updates into the engine before this cache write, but this line persists the original queued ProseMirror snapshot rather than the engine's merged document. Because the replay record is removed immediately afterward, offline editable loads can keep showing the stale pre-merge body until the page is fetched online again; persist a fresh snapshot from the engine after synchronization instead of document.
Useful? React with 👍 / 👎.
| return .updatePageCRDT( | ||
| pageId: pageId, | ||
| title: title, | ||
| document: patchedDocument, | ||
| stateUpdate: stateUpdate, | ||
| baseTitle: baseTitle |
There was a problem hiding this comment.
When an offline inline comment is created before a queued CRDT page update, replaceQueuedInlineCommentID reaches this branch after the comment receives its server ID, but only the ProseMirror projection is patched while the Yjs stateUpdate that replay actually applies and synchronizes is carried forward unchanged. In that scenario the server document can still end up with the temporary offline-comment-* mark even though the local projection now shows the server comment ID, so the CRDT state needs the same replacement or the replay should be regenerated from the patched document.
Useful? React with 👍 / 👎.
| case .syncStatus(true) where hasWriteAccess: | ||
| return |
There was a problem hiding this comment.
For an offline replay with buffered local updates, this returns as soon as the initial collaboration sync reports true, but the local update sender that drains syncDriver.localUpdates() runs in a separate task and is never awaited before synchronize disconnects the WebSocket. If the sync status arrives before that sender has sent the queued Yjs update, the replay is removed even though the offline edit never reached the server; wait for the local update stream/send completion (or an acknowledgement after sending) before treating the replay as synchronized.
Useful? React with 👍 / 👎.
1 participant
Summary
Validation
Xcode builds and tests were not run locally, per repository instructions.