From 21671cc5907ec0e6c6a530b685145586af0d4fba Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 10 Jun 2026 11:25:21 -0400 Subject: [PATCH] =?UTF-8?q?optimistic=20Backspace/Delete=20=E2=80=94=20sin?= =?UTF-8?q?gle-codepoint=20deletes=20apply=20locally?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last round-tripping editing keys. Same latency profile Enter had: mid-burst they deferred behind unconfirmed inserts and everything typed after them flushed in a delayed lump. Daemon: single-delete CRDT hot path in apply_remote_crdt_op. The deletion's start byte converts through the post-import doc (the prefix is untouched); the end byte comes from walking the still pre-import rope over the deleted codepoint count (reads at most 4 bytes per codepoint, not the file). Compound updates keep the materialize+diff fallback. pmacs-gpu: - optimistic_crdt_delete mirrors the insert path; the shared gates (optimistic_edit_eligible) and tail (finish_optimistic_edit) are factored out. optimistic_delete_range predicts exactly one codepoint — matching buffer.delete-backward/-forward's no-region behavior — and declines on buffer edges, modifier variants (C-BS word delete), or a mid-codepoint cursor. Region deletes keep round-tripping into delete_region via the selection gate. - Cursor-floor semantics tightened for non-monotonic predictions: only the exact predicted byte (or another buffer) confirms; plus a 500ms timeout escape hatch — an unconfirmed floor (op dropped by validation, peer racing the window cursor) now releases instead of wedging deferred keys forever, falling back to round-trip input until the next CursorByte resynchronizes. - Unconfirmed-edit journal rebasing generalized from pure inserts to delete-shaped entries (old_end translates independently, clamped). Co-Authored-By: Claude Fable 5 --- pmacs-gpu/src/main.rs | 311 ++++++++++++++++++++++++++++++++++-------- src/buffer.rs | 121 +++++++++++++++- 2 files changed, 374 insertions(+), 58 deletions(-) diff --git a/pmacs-gpu/src/main.rs b/pmacs-gpu/src/main.rs index 1da86fa..ff5ba31 100644 --- a/pmacs-gpu/src/main.rs +++ b/pmacs-gpu/src/main.rs @@ -366,6 +366,13 @@ struct State { /// the floor is acknowledged would make its legitimate cursor /// result indistinguishable from an older in-flight frame. deferred_round_trip_keys: Vec<(ProtocolKey, Modifiers)>, + /// When the current `optimistic_cursor_floor` was armed. If the + /// daemon never confirms the prediction (op dropped by + /// validation, a peer racing our window cursor), an unbounded + /// floor would wedge deferred round-trip keys forever; after + /// [`FLOOR_CONFIRM_TIMEOUT`] the floor releases, `cursor_fresh` + /// drops, and the next `CursorByte` resynchronizes. + optimistic_floor_set_at: Option, /// Optimistic local edits not yet known to be reflected in /// incoming producer frames. Each entry pairs the version scalar /// of this replica's doc *after* the edit applied (computed by @@ -471,11 +478,11 @@ impl ApplicationHandler for App { && should_forward_key(pkey, pmods) && let Some(client) = self.attach_client.as_ref() { - if let Some(op) = self - .state - .as_mut() - .and_then(|state| state.optimistic_crdt_insert(pkey, pmods)) - { + if let Some(op) = self.state.as_mut().and_then(|state| { + state + .optimistic_crdt_insert(pkey, pmods) + .or_else(|| state.optimistic_crdt_delete(pkey, pmods)) + }) { if debug_input() { eprintln!( "pmacs-gpu send_crdt: key={pkey:?} buf={:?} bytes={}B", @@ -568,6 +575,7 @@ impl ApplicationHandler for App { { eprintln!("pmacs-gpu: send Viewport failed: {e}"); } + state.release_timed_out_floor(); let ready_keys = state.take_ready_round_trip_keys(); if let Some(client) = self.attach_client.as_ref() { for (key, mods) in ready_keys { @@ -608,6 +616,45 @@ struct CrdtOpSend { viewport: Option, } +/// How long an unconfirmed optimistic-cursor prediction may gate +/// `CursorByte` acceptance and defer round-trip keys before the +/// escape hatch releases it. Generous against a busy daemon tick; +/// tiny against a human noticing wedged keys. +const FLOOR_CONFIRM_TIMEOUT: std::time::Duration = std::time::Duration::from_millis(500); + +/// Byte range an optimistic Backspace/Delete removes at `cursor`, or +/// `None` when it can't be predicted locally: buffer edge (the +/// daemon's behavior is a no-op there anyway), a modifier variant +/// (C-BS word-delete and friends are separate bindings), or a stale +/// mid-codepoint cursor. The range is exactly one codepoint, matching +/// `buffer.delete-backward` / `buffer.delete-forward`'s no-region +/// behavior; region deletes are excluded upstream by the selection +/// gate (they round-trip into `delete_region`). +fn optimistic_delete_range( + text: &str, + cursor: usize, + key: ProtocolKey, + mods: Modifiers, +) -> Option<(usize, usize)> { + if !mods.is_empty() { + return None; + } + if cursor > text.len() || !text.is_char_boundary(cursor) { + return None; + } + match key { + ProtocolKey::Backspace => { + let (start, _) = text[..cursor].char_indices().next_back()?; + Some((start, cursor)) + } + ProtocolKey::Delete => { + let ch = text[cursor..].chars().next()?; + Some((cursor, cursor + ch.len_utf8())) + } + _ => None, + } +} + /// The literal text `key` inserts when handled optimistically, or /// `None` for keys that must round-trip through the daemon. /// @@ -813,6 +860,7 @@ impl State { cursor_fresh: false, optimistic_cursor_floor: None, deferred_round_trip_keys: Vec::new(), + optimistic_floor_set_at: None, unconfirmed_edits: Vec::new(), } } @@ -826,19 +874,22 @@ impl State { } } - fn optimistic_crdt_insert(&mut self, key: ProtocolKey, mods: Modifiers) -> Option { + /// Shared eligibility gates for the optimistic edit paths + /// (insert + delete). `None` ⇒ the key must round-trip: + /// - dispatcher busy (minibuffer/prefix flows own the keys), or + /// the cursor isn't authoritative; + /// - CUA region semantics: an own-window selection means typing + /// replaces and Backspace/Delete consume the region — those + /// semantics live in the daemon's region-aware commands, which + /// a raw `CrdtOp` bypasses. (Our own selection arrives as a + /// `Selection` decoration; peer selections live in + /// `peer_presences` and don't gate.) + /// - bookkeeping: no frontend id / cursor / matching buffer / + /// replica doc, or the peer id can't be set. + fn optimistic_edit_eligible(&self) -> Option<(OwnCursor, u64)> { if !self.dispatch_idle || !self.cursor_fresh { return None; } - // CUA type-over: with an active selection, typing replaces the - // region. Those semantics live in the daemon's region-aware - // insert commands (`buffer.self-insert` / `newline` / `tab`), - // which a raw CrdtOp insert would bypass — so an own-window - // selection sends the key round-trip instead. Our own - // selection arrives as a `Selection` decoration (peer - // selections live in `peer_presences` and don't gate). The - // daemon's replace clears the region, the next Decorations - // frame clears the wash, and typing resumes optimistically. if self .current_decorations .iter() @@ -846,17 +897,11 @@ impl State { { return None; } - let mut chbuf = [0u8; 4]; - let insert = optimistic_insert_text(key, mods, &mut chbuf)?; let frontend_id = self.local_frontend_id?; let own = self.own_cursor?; if self.current_buffer_id != Some(own.buffer_id) { return None; } - let cursor = usize::try_from(own.byte).ok()?; - if cursor > self.current_text.len() || !self.current_text.is_char_boundary(cursor) { - return None; - } let doc = self.loro_doc.as_ref()?; let peer_id = frontend_id.0; if doc.peer_id() != peer_id @@ -865,7 +910,18 @@ impl State { eprintln!("pmacs-gpu: failed to set optimistic Loro peer id: {e:?}"); return None; } + Some((own, peer_id)) + } + fn optimistic_crdt_insert(&mut self, key: ProtocolKey, mods: Modifiers) -> Option { + let mut chbuf = [0u8; 4]; + let insert = optimistic_insert_text(key, mods, &mut chbuf)?; + let (own, peer_id) = self.optimistic_edit_eligible()?; + let cursor = usize::try_from(own.byte).ok()?; + if cursor > self.current_text.len() || !self.current_text.is_char_boundary(cursor) { + return None; + } + let doc = self.loro_doc.as_ref()?; let delta_batches = self.loro_text_delta_batches.clone()?; clear_loro_text_delta_batches(&delta_batches); let before = doc.oplog_vv(); @@ -877,19 +933,79 @@ impl State { .export(ExportMode::updates(&before)) .expect("export local optimistic Loro update"); let drained = drain_loro_text_delta_batches(&delta_batches); + let predicted = OwnCursor { + buffer_id: own.buffer_id, + byte: own.byte.saturating_add(insert.len() as u64), + }; + Some(self.finish_optimistic_edit(&drained, predicted, peer_id, bytes)) + } + + /// Optimistic single-codepoint Backspace / Delete. Mirrors the + /// insert path: the daemon's `buffer.delete-backward/-forward` + /// no-region behavior is exactly "delete one codepoint", so the + /// local application cannot diverge; region deletes are excluded + /// by the selection gate (they round-trip into `delete_region`), + /// and modified variants (C-BS word delete, …) round-trip via + /// `optimistic_delete_range` returning `None`. The daemon applies + /// the op through its single-delete CRDT hot path. + fn optimistic_crdt_delete(&mut self, key: ProtocolKey, mods: Modifiers) -> Option { + if !matches!(key, ProtocolKey::Backspace | ProtocolKey::Delete) { + return None; + } + let (own, peer_id) = self.optimistic_edit_eligible()?; + let cursor = usize::try_from(own.byte).ok()?; + let (start, end) = optimistic_delete_range(&self.current_text, cursor, key, mods)?; + let doc = self.loro_doc.as_ref()?; + let delta_batches = self.loro_text_delta_batches.clone()?; + clear_loro_text_delta_batches(&delta_batches); + let before = doc.oplog_vv(); + if let Err(e) = doc + .get_text(LORO_TEXT_CONTAINER) + .delete_utf8(start, end - start) + { + eprintln!("pmacs-gpu: optimistic delete failed: {e:?}"); + return None; + } + let bytes = doc + .export(ExportMode::updates(&before)) + .expect("export local optimistic Loro update"); + let drained = drain_loro_text_delta_batches(&delta_batches); + let predicted = OwnCursor { + buffer_id: own.buffer_id, + byte: start as u64, + }; + Some(self.finish_optimistic_edit(&drained, predicted, peer_id, bytes)) + } + + /// Common tail of the optimistic edit paths: patch the local text + /// from the drained Loro deltas (journaling them for + /// incoming-frame translation), predict the cursor + arm the + /// confirmation floor, follow the caret, and package the wire op. + fn finish_optimistic_edit( + &mut self, + drained: &[Vec], + predicted: OwnCursor, + peer_id: u64, + bytes: Vec, + ) -> CrdtOpSend { if drained.is_empty() { - let text = doc.get_text(LORO_TEXT_CONTAINER).to_string(); - self.set_text(&text); + let text = self + .loro_doc + .as_ref() + .map(|doc| doc.get_text(LORO_TEXT_CONTAINER).to_string()); + if let Some(text) = text { + self.set_text(&text); + } // Cache rebuilt wholesale — there are no translated // anchors left for frame translation to protect. self.unconfirmed_edits.clear(); } else { - match self.apply_loro_text_delta_batches(&drained) { + match self.apply_loro_text_delta_batches(drained) { Ok(edits) => { // Journal this keystroke so producer frames the // daemon computed before integrating it can be // translated on arrival (see `unconfirmed_edits`). - // The scalar is read *after* the local insert, so + // The scalar is read *after* the local edit, so // any frame stamped at or beyond it includes us. let scalar = self.loro_doc.as_ref().map_or(0, loro_version_scalar); self.unconfirmed_edits @@ -911,27 +1027,25 @@ impl State { } } } - let predicted = OwnCursor { - buffer_id: own.buffer_id, - byte: own.byte.saturating_add(insert.len() as u64), - }; self.own_cursor = Some(predicted); self.optimistic_cursor_floor = Some(predicted); + self.optimistic_floor_set_at = Some(std::time::Instant::now()); // Follow the caret NOW rather than when the daemon's // `CursorByte` confirms — an optimistic Enter on the bottom - // visible line moves the caret to a line below the slice, and - // waiting a round trip to scroll reads as a hitch. + // visible line (or a Backspace pulling the caret above the + // top) moves it outside the slice, and waiting a round trip + // to scroll reads as a hitch. let viewport = if self.scroll_to_cursor() { self.reshape(); - self.viewport_send_if_changed(own.buffer_id) + self.viewport_send_if_changed(predicted.buffer_id) } else { None }; - Some(CrdtOpSend { - buffer_id: own.buffer_id, + CrdtOpSend { + buffer_id: predicted.buffer_id, op: CrdtOp { peer_id, bytes }, viewport, - }) + } } fn mark_cursor_stale_after_round_trip(&mut self) { @@ -971,6 +1085,27 @@ impl State { .retain(|(scalar, _)| *scalar > generation); } + fn optimistic_floor_timed_out(&self) -> bool { + self.optimistic_floor_set_at + .is_some_and(|armed| armed.elapsed() >= FLOOR_CONFIRM_TIMEOUT) + } + + /// Escape hatch: release a floor the daemon never confirmed so + /// deferred round-trip keys can't wedge forever. Dropping + /// `cursor_fresh` falls the GPU back to round-trip mode until the + /// next `CursorByte` resynchronizes the cursor. + fn release_timed_out_floor(&mut self) { + if self.optimistic_cursor_floor.is_some() && self.optimistic_floor_timed_out() { + eprintln!( + "pmacs-gpu: optimistic cursor unconfirmed after {FLOOR_CONFIRM_TIMEOUT:?}; \ + falling back to round-trip input" + ); + self.optimistic_cursor_floor = None; + self.optimistic_floor_set_at = None; + self.cursor_fresh = false; + } + } + fn defer_round_trip_key_if_needed(&mut self, key: ProtocolKey, mods: Modifiers) -> bool { if self.optimistic_cursor_floor.is_none() && self.deferred_round_trip_keys.is_empty() { return false; @@ -1087,6 +1222,7 @@ impl State { self.own_cursor = None; self.cursor_fresh = false; self.optimistic_cursor_floor = None; + self.optimistic_floor_set_at = None; self.deferred_round_trip_keys.clear(); self.unconfirmed_edits.clear(); // New buffer ⇒ back to the top, and force a viewport @@ -1167,16 +1303,20 @@ impl State { Ok(edits) => { // A daemon-originated edit shifts the text // under any still-unconfirmed optimistic - // inserts. Rebase the journal's anchors so + // edits. Rebase the journal's anchors so // frames that include this edit (but not - // ours) translate correctly. Journal - // entries are pure inserts (the optimistic - // path only inserts), so anchor == start. + // ours) translate correctly. Entries are + // inserts (start == old_end) or + // single-codepoint deletes; both rebase by + // position translation, clamped so a range + // can't invert. for incoming in &edits { for (_, pending) in &mut self.unconfirmed_edits { pending.start = translate_byte_position(pending.start, *incoming); - pending.old_end = pending.start; + pending.old_end = + translate_byte_position(pending.old_end, *incoming) + .max(pending.start); } } } @@ -1334,24 +1474,28 @@ impl State { self.current_buffer_id == Some(buffer_id) ); } - if let Some(floor) = self.optimistic_cursor_floor - && floor.buffer_id == buffer_id - && byte_pos < floor.byte - { - if debug_input() { - eprintln!( - "pmacs-gpu cursor: ignored stale optimistic rewind \ - buf={buffer_id:?} byte={byte_pos} floor={}", - floor.byte - ); + if let Some(floor) = self.optimistic_cursor_floor { + // With deletes in the optimistic set the predicted + // cursor is no longer monotonic, so only the EXACT + // predicted byte (or a cursor for another buffer) + // confirms; any other value is an in-flight frame + // from before our unconfirmed edits. The timeout + // hatch accepts daemon truth if confirmation never + // comes (op dropped, peer raced our cursor). + let confirmed = floor.buffer_id != buffer_id || byte_pos == floor.byte; + if confirmed || self.optimistic_floor_timed_out() { + self.optimistic_cursor_floor = None; + self.optimistic_floor_set_at = None; + } else { + if debug_input() { + eprintln!( + "pmacs-gpu cursor: ignored stale in-flight position \ + buf={buffer_id:?} byte={byte_pos} predicted={}", + floor.byte + ); + } + return None; } - return None; - } - if self - .optimistic_cursor_floor - .is_some_and(|floor| floor.buffer_id != buffer_id || byte_pos >= floor.byte) - { - self.optimistic_cursor_floor = None; } self.own_cursor = Some(OwnCursor { buffer_id, @@ -3695,6 +3839,59 @@ mod tests { ); } + #[test] + fn optimistic_delete_range_covers_single_codepoints_only() { + let none = Modifiers::NONE; + let text = "aé😀b"; + + // Backspace deletes the codepoint before the cursor, whatever + // its width: 'é' is 2 bytes, '😀' is 4. + assert_eq!( + optimistic_delete_range(text, 3, ProtocolKey::Backspace, none), + Some((1, 3)), + "backspace before the cursor crosses the full 'é'" + ); + assert_eq!( + optimistic_delete_range(text, 7, ProtocolKey::Backspace, none), + Some((3, 7)), + "backspace crosses the full '😀'" + ); + // Delete removes the codepoint at the cursor. + assert_eq!( + optimistic_delete_range(text, 1, ProtocolKey::Delete, none), + Some((1, 3)) + ); + assert_eq!( + optimistic_delete_range(text, 7, ProtocolKey::Delete, none), + Some((7, 8)) + ); + + // Buffer edges: nothing to delete ⇒ round-trip (daemon no-op). + assert_eq!( + optimistic_delete_range(text, 0, ProtocolKey::Backspace, none), + None + ); + assert_eq!( + optimistic_delete_range(text, text.len(), ProtocolKey::Delete, none), + None + ); + // Mid-codepoint (stale) cursor ⇒ round-trip, never a panic. + assert_eq!( + optimistic_delete_range(text, 2, ProtocolKey::Backspace, none), + None + ); + // Modified variants are separate bindings (C-BS word delete). + assert_eq!( + optimistic_delete_range(text, 3, ProtocolKey::Backspace, Modifiers::CTRL), + None + ); + // Non-delete keys are not this helper's business. + assert_eq!( + optimistic_delete_range(text, 3, ProtocolKey::Char('x'), none), + None + ); + } + #[test] fn incoming_frames_translate_through_unconfirmed_edits() { // A frame computed at daemon generation G arrives while one diff --git a/src/buffer.rs b/src/buffer.rs index d45740a..e01a683 100644 --- a/src/buffer.rs +++ b/src/buffer.rs @@ -765,7 +765,25 @@ impl Buffer { return result.map(Some); } - // Conservative fallback for deletes, compound updates, and + // Single-delete hot path (optimistic Backspace/Delete). The + // deletion's *start* converts through the post-import doc — + // the prefix is untouched, so the byte offset is identical + // pre- and post-import. The *end* byte cannot (those chars + // are gone from the doc); it comes from walking the still + // pre-import rope over the deleted codepoint count. + if let Some((unicode_pos, deleted_chars)) = single_remote_text_delete(&text_deltas) + && let Some(byte_start) = crdt.unicode_to_utf8_pos(unicode_pos) + && let Some(byte_end) = + rope_byte_end_after_chars(&self.rope, byte_start as Position, deleted_chars) + { + let byte_start = byte_start as Position; + let mut views = std::mem::take(&mut self.views); + let result = self.run_remote_rope_stages(&mut views, byte_start, byte_end, b""); + self.views = views; + return result.map(Some); + } + + // Conservative fallback for compound updates and // already-integrated ops. The rope is still the pre-import // projection, so it remains the source for the old bytes. let old_len = self.rope.len(); @@ -1490,6 +1508,72 @@ fn single_remote_text_insert(deltas: &[Vec]) -> Option<(usize, found } +/// Recognize the hot-path projection delta produced by one remote +/// deletion: an optional leading `Retain` followed by exactly one +/// `Delete`, nothing else. Returns `(unicode_start, deleted_chars)` +/// in Unicode scalar units. +#[cfg(feature = "crdt")] +fn single_remote_text_delete(deltas: &[Vec]) -> Option<(usize, usize)> { + let [delta] = deltas else { + return None; + }; + let mut cursor = 0usize; + let mut found = None; + for op in delta { + match op { + loro::TextDelta::Retain { retain, .. } => { + cursor = cursor.checked_add(*retain)?; + } + loro::TextDelta::Insert { insert, .. } if insert.is_empty() => {} + loro::TextDelta::Insert { .. } => return None, + loro::TextDelta::Delete { delete } if *delete == 0 => {} + loro::TextDelta::Delete { delete } => { + if found.is_some() { + return None; + } + found = Some((cursor, *delete)); + } + } + } + found +} + +/// Byte offset just past `chars` codepoints starting at `byte_start` +/// in `rope`. Reads at most `chars * 4` bytes (one UTF-8 max-width +/// each), so a Backspace-sized walk touches a handful of bytes, not +/// the file. `None` when the rope runs out (or a boundary is off) — +/// callers fall back to the materialize-and-diff path. +#[cfg(feature = "crdt")] +fn rope_byte_end_after_chars( + rope: &crate::rope::Rope, + byte_start: Position, + chars: usize, +) -> Option { + let len = rope.len(); + if byte_start > len || chars == 0 { + return None; + } + let take = (chars as Position).saturating_mul(4).min(len - byte_start); + let mut buf = vec![0u8; take as usize]; + rope.slice(byte_start, byte_start + take, &mut buf); + let s = match std::str::from_utf8(&buf) { + Ok(s) => s, + // The 4*chars window can cut a trailing codepoint that we + // don't need anyway; keep the valid prefix. + Err(e) => std::str::from_utf8(&buf[..e.valid_up_to()]).ok()?, + }; + let mut remaining = chars; + let mut offset = 0usize; + for ch in s.chars() { + if remaining == 0 { + break; + } + offset += ch.len_utf8(); + remaining -= 1; + } + (remaining == 0).then(|| byte_start + offset as Position) +} + /// T M10.4: derive a fine-grained `(range, inserted_len)` Edit /// description for the change from `old_rope` to `new_rope` via /// longest-common-prefix + longest-common-suffix trim. @@ -2952,6 +3036,41 @@ mod tests { assert_invariant(&buf); } + #[cfg(feature = "crdt")] + #[test] + fn apply_remote_crdt_op_single_delete_uses_pre_import_rope_for_end_byte() { + let donor = crate::crdt::CrdtState::new(2).expect("donor"); + donor.insert(0, "aéx").expect("seed"); + + let mut buf = Buffer::new_with_crdt(BufferId::next(), "*utf8-delete*", 1).expect("buf"); + let donor_snap = donor.export_snapshot().expect("snap"); + buf.crdt + .as_ref() + .expect("crdt") + .import_snapshot(&donor_snap) + .expect("init from snap"); + buf.rope = crate::rope::Rope::from_bytes("aéx".as_bytes()); + + // Delete the 2-byte 'é' (CrdtState::delete takes UTF-8 byte + // offsets; the wire delta reports it as 1 Unicode scalar). + let v_before = donor.version(); + donor.delete(1, "é".len()).expect("delete"); + let op_bytes = donor.export_updates_since(&v_before).expect("export"); + let edit = buf + .apply_remote_crdt_op(&op_bytes) + .expect("apply") + .expect("non-empty edit"); + + assert_eq!(rope_string(&buf), "ax"); + assert_eq!( + edit.range, + Range::new(1, 1 + "é".len() as u64), + "byte range covers the multibyte codepoint exactly" + ); + assert_eq!(edit.inserted_len, 0); + assert_invariant(&buf); + } + /// F25 (post-audit-round-4): a CRDT update that changes one /// codepoint into another with a shared leading UTF-8 byte /// must produce a char-boundary-aligned diff. Pre-fix, the