From 8db0485839803fd142f87f8aab0d257b6ec961bc Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 21 May 2026 19:31:56 +0000 Subject: [PATCH] =?UTF-8?q?T=20M11.9=20=E2=80=94=20handle=5Fremote=5Fcrdt?= =?UTF-8?q?=5Fop=20fires=20buffer.after-edit=20(closes=20session-5=20root?= =?UTF-8?q?=20cause)=20(#49)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Surfaced when PR #48's diag-store stale-flag turned out to have no observable effect on session-5 validation: the wrong-position-color artifact persisted even though the stale-flag suppression chain was in place. ## Root cause (the *actual* one) The M10.10 optimistic-apply layer routes plain-char keystrokes (EOL-eligible, no Ctrl-modifier, etc.) as `FrontendEvent::CrdtOp` rather than `FrontendEvent::Key`. The daemon dispatches CrdtOps via `handle_remote_crdt_op`, which applies the buffer edit and queues the op for broadcast — but **does not fire `buffer.after-edit`**. `buffer.after-edit` was only fired by `dispatch_key` (editor.rs:506) after a Key-path edit. The CrdtOp path bypassed it entirely. The downstream LSP hook in `builtin/runtime/lsp.lua:379` calls `pmacs.lsp.did_change` on every `buffer.after-edit`. With CrdtOp edits not firing the hook, `did_change_full` (and therefore `textDocument/didChange`) was never sent to clangd for the bulk of typing activity. clangd's view of the document silently froze at whatever state the last Key-path edit (find-file, keystrokes through the minibuffer, modifier-combinations) had left it in. Downstream symptoms, all silent: - **Diagnostics frozen at pre-edit byte positions** — the session-5 visible artifact. - **LSP semantic tokens stale** (for grammar-less languages where semantic_render uses LSP not tree-sitter — i.e. C++). - **Inlay hints stale**. - **Hover/go-to-definition/rename can return wrong-position results** if a CrdtOp edit moved positions since the last Key edit. PR #47 (full=true on generation transition) and PR #48 (diag-store stale-flag) were correctness fixes on the producer side, but they depended on `did_change` actually firing to trigger their effects. With did_change silenced, both were dormant for any CrdtOp edit. ## Fix In `handle_remote_crdt_op`, when `edit_opt` is `Some` (the import produced a text delta), after notifying views: 1. Set `active_frontend = source` so the hook's `pmacs.window.buffer()` resolves to the right buffer (matches the pattern `dispatch_key` uses). 2. Fire `buffer.after-edit` via `editor.lua_host.run_hook(...)`. This makes the Lua observer chain (LSP `did_change` and any future consumers) see CrdtOp-path edits identically to Key-path edits. ## Regression test `daemon::tests::handle_remote_crdt_op_fires_after_edit_hook` (crdt-gated): 1. Upgrade the active buffer to CRDT-backed 2. Install a Lua `buffer.after-edit` hook that bumps a global 3. Build a peer LoroDoc from the buffer snapshot, edit on the peer, export the op 4. Call `handle_remote_crdt_op` with the op 5. Assert the global counter is `1` Pre-fix, the counter stays at `0`. ## Gates | Gate | Result | |---|---| | `cargo fmt --check` | clean | | `clippy --features crdt --workspace -D warnings` | clean | | `clippy --workspace -D warnings` (no crdt) | clean | | `cargo test --features crdt --lib` | 1483 (+1) | | `cargo test --lib` (no crdt) | 1319 (test crdt-gated) | | `m4_acceptance --features crdt` | 83 | | `m11_5_semantic_acceptance --features crdt` | 2 | 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 --- src/daemon.rs | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) diff --git a/src/daemon.rs b/src/daemon.rs index 7f7b063..1f928a0 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -1777,6 +1777,29 @@ fn handle_remote_crdt_op( } core.notify_buffer_edit(buffer_id, edit); + // T M11.9 — temporarily switch active_frontend to source so + // the `buffer.after-edit` hook's Lua observers (notably the + // LSP `did_change` glue in `builtin/runtime/lsp.lua`) read + // the right buffer via `pmacs.window.buffer()`. Matches the + // pattern `dispatch_key` uses (it assigns `active_frontend` + // before running its hook). + core.active_frontend = source; + drop(core); + + // T M11.9 — fire `buffer.after-edit` for replicated edits. + // Without this, LSP `textDocument/didChange` is never sent + // for keystrokes the M10.10 optimistic-apply layer routed as + // `FrontendEvent::CrdtOp` (the bulk of plain-char typing), + // so clangd's view of the document drifts behind reality. + // Diagnostics, semantic tokens, and inlay hints all silently + // freeze at the byte positions they last had when an edit + // happened to fall back to the Key path. Closes the actual + // root cause of the session-5 wrong-position-color + // artifact; the diag-store stale-flag from T M11.8 finally + // gets reached. + editor + .lua_host + .run_hook("buffer.after-edit", mlua::MultiValue::new()); } // Effect 4: queue for broadcast. The source frontend's mirror @@ -1981,4 +2004,100 @@ mod tests { // Test environments are UTF-8, so this should be non-empty. assert!(!id.working_directory.is_empty()); } + + /// T M11.9 regression: `handle_remote_crdt_op` fires the + /// `buffer.after-edit` Lua hook. Without this, the M10.10 + /// optimistic-apply path's `FrontendEvent::CrdtOp` route + /// bypasses every Lua observer of buffer mutations — most + /// importantly the LSP `did_change` notification, which means + /// clangd never re-analyzes documents edited via the optimistic + /// path. The session-5 wrong-position-color artifact was the + /// downstream symptom: diagnostics frozen at pre-edit byte + /// positions because clangd had never been told about the edit. + #[cfg(feature = "crdt")] + #[test] + fn handle_remote_crdt_op_fires_after_edit_hook() { + use crate::editor::EditorState; + use crate::protocol::FrontendId; + + let mut editor = EditorState::new(); + + // Install an after-edit hook that bumps a global counter + // we can read back from Lua. + editor + .lua_host + .eval( + Some("test"), + r#" + _G.PMACS_TEST_AFTER_EDIT_FIRED = 0 + pmacs.hook.add("buffer.after-edit", function() + _G.PMACS_TEST_AFTER_EDIT_FIRED = (_G.PMACS_TEST_AFTER_EDIT_FIRED or 0) + 1 + end) + "#, + ) + .expect("install after-edit hook"); + + // Upgrade the active buffer to CRDT-backed so + // `handle_remote_crdt_op` finds a `CrdtState` to apply + // against (the non-CRDT path is not exercised here). + let buffer_id = editor.core.borrow().active_window().buffer_id; + { + let core = editor.core.borrow(); + let mut reg = core.registry.borrow_mut(); + reg.get_mut(buffer_id) + .expect("active buffer") + .upgrade_to_crdt(2) + .expect("upgrade to crdt"); + } + + // Build a peer CRDT doc from the buffer's snapshot, perform + // an edit on the peer, export the op bytes. This is the + // shape `optimistic::apply_local_insert` produces in the + // attach loop's optimistic-apply branch. + let snapshot_bytes = { + let core = editor.core.borrow(); + let reg = core.registry.borrow(); + let buf = reg.get(buffer_id).expect("buffer"); + buf.crdt_state() + .expect("crdt-backed") + .export_snapshot() + .expect("export snapshot") + }; + let peer = loro::LoroDoc::new(); + peer.set_peer_id(99).expect("set peer id"); + peer.import(&snapshot_bytes).expect("import snapshot"); + let v_before = peer.oplog_vv(); + peer.get_text("body").insert(0, "x").expect("peer insert"); + let op_bytes = peer + .export(loro::ExportMode::updates(&v_before)) + .expect("export op"); + + // Apply the op via `handle_remote_crdt_op`. + super::handle_remote_crdt_op( + &mut editor, + FrontendId(99), + buffer_id, + crate::rope::CrdtOp { + peer_id: 99, + bytes: op_bytes, + }, + ); + + // The hook should have fired exactly once. + let count_val = editor + .lua_host + .eval( + Some("test-readback"), + "return _G.PMACS_TEST_AFTER_EDIT_FIRED", + ) + .expect("read counter"); + let count = match count_val { + mlua::Value::Integer(n) => n, + other => panic!("expected counter integer, got {other:?}"), + }; + assert_eq!( + count, 1, + "buffer.after-edit must fire when handle_remote_crdt_op produces a text Edit" + ); + } }