From 5c5caa94820357924b57222df0ed32f23133061d Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Tue, 7 Jul 2026 20:46:50 -0400 Subject: [PATCH] fix(panels): follow active buffer on semantic frontends; re-attach overlays on switch Two PR #94 validation findings. 1. (High, blocking) GPU stuck after leaving a panel: the GPU only swaps its displayed buffer on BufferSnapshot, and the daemon only sent one on the first CRDT upgrade (F29's ensure returns None for an already-backed buffer). A panel's q / RET switched the daemon's active buffer back to the already-known source and sent nothing -- the GPU kept rendering the panel while input targeted the source: a typing-into-a-buffer-you-can't-see hazard. Fix: the per-tick loop now FOLLOWS each replica frontend's own active buffer -- when it differs from the last snapshot sent to that frontend, ship that buffer's snapshot to that frontend only (the F29 broadcast records itself so the upgrade tick doesn't double-send). First-tick send also repairs the attach-time last-snapshot-wins ambiguity. Snapshot export extracted and shared with the F29 broadcast; per-fid state cleaned on both detach paths. 2. (High, wider than reported) 'LSP doesn't activate on navigate': switch_active_buffer clears the window's overlays, and the runtime dedup tables (highlighted_buffers, styled_buffers, diag_viewed_buffers) blocked re-attachment -- so EVERY buffer switch (plain C-x b included, long-latent) permanently stripped syntax color, LSP semantic style, and diagnostic underlines; verified: overlay kinds [syntax-highlight, lsp-style, diagnostic] -> [] after one away-and-back. Fix: a new additive buffer.after-switch hook, fired by the window.switch_buffer binding and find_or_open's existing-buffer branch; syntax.lua and lsp.lua subscribe and re-push their views (the just-cleared window makes that exactly-once per switch; fresh loads keep firing after-load). Regression: tests/overlay_reattach_acceptance.rs (double round-trip counts exactly one highlight overlay; panel q restores styling). The daemon follow path is validated live (daemon + GPU) -- its unit seam is the shared export helper. Co-Authored-By: Claude Fable 5 --- builtin/hooks/default.lua | 10 +++ builtin/runtime/lsp.lua | 20 +++++ builtin/runtime/syntax.lua | 19 +++++ src/daemon.rs | 107 +++++++++++++++++++---- src/lua_bindings/mod.rs | 16 +++- tests/overlay_reattach_acceptance.rs | 123 +++++++++++++++++++++++++++ 6 files changed, 277 insertions(+), 18 deletions(-) create mode 100644 tests/overlay_reattach_acceptance.rs diff --git a/builtin/hooks/default.lua b/builtin/hooks/default.lua index 5e48993..98d89e4 100644 --- a/builtin/hooks/default.lua +++ b/builtin/hooks/default.lua @@ -44,6 +44,16 @@ define { kind = "all-must-succeed", } +define { + name = "buffer.after-switch", + description = "Fired after the active window switches to a different, " .. + "already-open buffer (C-x b, panel visits, find_or_open of " .. + "an open file). Switching clears the window's overlays; " .. + "syntax/LSP subscribers re-attach theirs here. Fresh loads " .. + "fire buffer.after-load instead.", + kind = "all-must-succeed", +} + define { name = "buffer.after-save", description = "Fired after a successful save. LSP did_save and " .. diff --git a/builtin/runtime/lsp.lua b/builtin/runtime/lsp.lua index 2a11595..d2b171b 100644 --- a/builtin/runtime/lsp.lua +++ b/builtin/runtime/lsp.lua @@ -569,6 +569,26 @@ pmacs.hook.add("buffer.after-load", function() pcall(attach_buffer, pmacs.window.buffer()) end) +pmacs.hook.add("buffer.after-switch", function() + -- Arc 1b: switching buffers clears the window's overlays, and + -- `attach_buffer` early-returns for a live attachment without + -- touching views — so a switch back to an attached buffer must + -- re-push the LSP style + diagnostic views itself. The just- + -- cleared window makes this exactly-once per switch; the dedup + -- tables keep gating the after-load path only. Without this, + -- navigating between attached buffers looked like "the LSP + -- deactivated" (no semantic color, no underlines). + local buf = pmacs.window.buffer() + if not buf then return end + local key = tostring(buf) + local rec = attachments[key] + if not rec then return end + local ok_s, attached_s = pcall(pmacs.lsp._attach_style, buf) + if ok_s and attached_s then styled_buffers[key] = true end + local ok_d, attached_d = pcall(pmacs.diag._attach_view, buf, rec.uri) + if ok_d and attached_d then diag_viewed_buffers[key] = true end +end) + pmacs.hook.add("buffer.after-edit", function() local buf = pmacs.window.buffer() if not buf then return end diff --git a/builtin/runtime/syntax.lua b/builtin/runtime/syntax.lua index f28e012..5f7fcb8 100644 --- a/builtin/runtime/syntax.lua +++ b/builtin/runtime/syntax.lua @@ -83,6 +83,25 @@ pmacs.hook.add("buffer.after-load", function() end end) +pmacs.hook.add("buffer.after-switch", function() + -- Arc 1b: switching buffers clears the window's overlays + -- (`switch_active_buffer` resets window view state), so the + -- highlight view must be re-pushed for the now-active buffer. + -- Dropping the `highlighted_buffers` entry first lets + -- `attach_for_active_buffer` re-attach; the just-cleared window + -- makes that exactly-once per switch. Without this, C-x b / + -- panel navigation permanently stripped syntax color. + local ok, err = pcall(function() + local buf = pmacs.window.buffer() + if not buf then return end + highlighted_buffers[tostring(buf)] = nil + attach_for_active_buffer() + end) + if not ok and pmacs.error then + pmacs.error("syntax.after-switch: " .. tostring(err)) + end +end) + local function reparse_active_buffer_after_edit() local buf = pmacs.window.buffer() if not buf then return end diff --git a/src/daemon.rs b/src/daemon.rs index c5616b2..d58013f 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -842,6 +842,16 @@ fn dispatcher_loop( // attach emits an initial `DispatchIdle` so the frontend starts // from a known idle state (its default is pessimistic-`false`). let mut last_dispatch_idle_sent: HashMap = HashMap::new(); + // Arc 1b — the buffer each replica frontend last received a + // `BufferSnapshot` for via the active-buffer-follow path. Absence + // means "never sent": the first tick after attach ships the + // frontend its own active buffer, which also repairs the + // attach-time last-snapshot-wins ambiguity (the initial + // `send_buffer_snapshots` sweep sends every buffer; the display + // follows whichever arrived last, not necessarily the active one). + // Declared for both flavors (the follow path is crdt-gated; the + // detach cleanup isn't). + let mut last_active_buffer_sent: HashMap = HashMap::new(); let mut session_registry = SessionRegistry::new(); // T M10.11 Q8 — jitter PRNG, seeded once so the // convergence-under-jitter scenario is deterministically @@ -966,6 +976,31 @@ fn dispatcher_loop( &session_registry, &mut streams, ); + // The broadcast just delivered this buffer to this + // frontend too; record it so the follow check below + // doesn't send a duplicate on the same tick. + last_active_buffer_sent.insert(*fid, upgraded); + } + + // Arc 1b — follow this frontend's active buffer. The + // F29 push above only fires on the *upgrade* tick; + // switching to an already-CRDT-backed buffer (a + // panel's `q`, `find_or_open` of an open file, plain + // `C-x b`) previously sent nothing, so a semantic + // frontend kept rendering the old buffer while + // daemon-side input targeted the new one — a + // typing-into-a-buffer-you-can't-see hazard. Ship the + // now-active buffer's snapshot to THIS frontend only + // (its own view changed; nobody else's did). + let active_now = { + let core = editor.core.borrow(); + core.active_window_for(*fid).map(|w| w.buffer_id) + }; + if let Some(active_now) = active_now + && last_active_buffer_sent.get(fid) != Some(&active_now) + { + send_buffer_snapshot_to_frontend(editor, active_now, *fid, &mut streams); + last_active_buffer_sent.insert(*fid, active_now); } } #[cfg(not(feature = "crdt"))] @@ -1181,6 +1216,7 @@ fn dispatcher_loop( semantic_states.remove(fid); term_sizes.remove(fid); last_dispatch_idle_sent.remove(fid); + last_active_buffer_sent.remove(fid); session_registry.unregister_session(*fid); editor.core.borrow_mut().unregister_frontend_view(*fid); } @@ -1221,6 +1257,7 @@ fn dispatcher_loop( &mut streams, &mut term_sizes, &mut last_dispatch_idle_sent, + &mut last_active_buffer_sent, &mut session_registry, ); // Drain a burst of immediately-available events to @@ -1236,6 +1273,7 @@ fn dispatcher_loop( &mut streams, &mut term_sizes, &mut last_dispatch_idle_sent, + &mut last_active_buffer_sent, &mut session_registry, ); } @@ -1341,6 +1379,7 @@ fn handle_dispatcher_event( streams: &mut HashMap, term_sizes: &mut HashMap, last_dispatch_idle_sent: &mut HashMap, + last_active_buffer_sent: &mut HashMap, session_registry: &mut SessionRegistry, ) { match event { @@ -1515,6 +1554,7 @@ fn handle_dispatcher_event( streams.remove(&frontend_id); term_sizes.remove(&frontend_id); last_dispatch_idle_sent.remove(&frontend_id); + last_active_buffer_sent.remove(&frontend_id); session_registry.unregister_session(frontend_id); editor .core @@ -1691,6 +1731,55 @@ fn ensure_active_buffer_crdt_backed( /// send is small (snapshot bytes for the upgrade-instant state, /// which is the empty / freshly-loaded buffer content the replica /// already has) and only fires on the actual upgrade tick. +/// Export `buffer_id`'s CRDT snapshot bytes, or `None` (logged) when +/// the buffer is missing, not CRDT-backed, or the export fails. +#[cfg(feature = "crdt")] +fn export_buffer_snapshot( + editor: &EditorState, + buffer_id: crate::buffer::BufferId, +) -> Option> { + let core = editor.core.borrow(); + let registry = core.registry.borrow(); + let buf = registry.get(buffer_id).ok()?; + let crdt = buf.crdt_state()?; + match crdt.export_snapshot() { + Ok(bytes) => Some(bytes), + Err(e) => { + eprintln!("pmacs: export_snapshot for {buffer_id:?} failed: {e:?}"); + None + } + } +} + +/// Arc 1b — send `buffer_id`'s snapshot to ONE frontend. The +/// active-buffer-follow path (see the per-tick loop) uses this when a +/// semantic frontend's own active buffer changes to an +/// already-CRDT-backed buffer: the F29 broadcast only fires on the +/// upgrade tick, so without this a frontend that switched *back* to a +/// known buffer (a panel's `q`, `find_or_open` of an open file) kept +/// displaying the old buffer while daemon-side input targeted the new +/// one. +#[cfg(feature = "crdt")] +fn send_buffer_snapshot_to_frontend( + editor: &EditorState, + buffer_id: crate::buffer::BufferId, + fid: FrontendId, + streams: &mut HashMap, +) { + let Some(snapshot_bytes) = export_buffer_snapshot(editor, buffer_id) else { + return; + }; + let msg = InstanceMessage::BufferSnapshot { + buffer_id, + crdt_snapshot: snapshot_bytes, + }; + if let Some(stream) = streams.get_mut(&fid) + && let Err(e) = write_message(stream, &msg) + { + eprintln!("pmacs: send BufferSnapshot for {buffer_id:?} to {fid:?} failed: {e}"); + } +} + #[cfg(feature = "crdt")] fn broadcast_buffer_snapshot_to_replicas( editor: &EditorState, @@ -1698,22 +1787,8 @@ fn broadcast_buffer_snapshot_to_replicas( session_registry: &SessionRegistry, streams: &mut HashMap, ) { - let snapshot_bytes = { - let core = editor.core.borrow(); - let registry = core.registry.borrow(); - let Ok(buf) = registry.get(buffer_id) else { - return; - }; - let Some(crdt) = buf.crdt_state() else { - return; - }; - match crdt.export_snapshot() { - Ok(bytes) => bytes, - Err(e) => { - eprintln!("pmacs: F29 export_snapshot for {buffer_id:?} failed: {e:?}"); - return; - } - } + let Some(snapshot_bytes) = export_buffer_snapshot(editor, buffer_id) else { + return; }; let msg = InstanceMessage::BufferSnapshot { buffer_id, diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 0cd09de..1f9f4a0 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -2402,6 +2402,11 @@ fn install_buffer_module(lua: &Lua, registry: &SharedRegistry) -> mlua::Result mlua::Result { let cc = core.clone(); win.set( "switch_buffer", - lua.create_function(move |_, id: BufferIdLua| -> mlua::Result<()> { + lua.create_function(move |lua, id: BufferIdLua| -> mlua::Result<()> { cc.borrow_mut() .switch_active_buffer(id.0) - .map_err(mlua::Error::external) + .map_err(mlua::Error::external)?; + // Arc 1b: switching clears the window's overlays; + // subscribers (syntax highlight, LSP style/diag views) + // re-attach theirs here — without this, C-x b / panel + // navigation permanently stripped styling from the + // session. + run_hook_if_defined(lua, "buffer.after-switch", mlua::MultiValue::new()); + Ok(()) })?, )?; } diff --git a/tests/overlay_reattach_acceptance.rs b/tests/overlay_reattach_acceptance.rs new file mode 100644 index 0000000..48eafbb --- /dev/null +++ b/tests/overlay_reattach_acceptance.rs @@ -0,0 +1,123 @@ +//! Overlay re-attach on buffer switch (Arc 1b, PR #94 validation +//! finding 2): `switch_active_buffer` clears the window's overlays, +//! and the runtime's dedup tables blocked re-attachment — so plain +//! `C-x b`, buffer-list visits, and panel navigation permanently +//! stripped syntax/LSP styling ("the LSP doesn't activate if I +//! navigate to a reference"). The `buffer.after-switch` hook now +//! re-pushes the views. +//! +//! Hermetic: only the tree-sitter `syntax-highlight` overlay is +//! asserted (always available for `.rs`); the LSP style/diag views +//! ride the same hook but need a live server. + +use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyEventState, KeyModifiers}; +use pmacs::editor::EditorState; + +fn open_probe_file(_s: &EditorState) -> String { + let dir = std::env::temp_dir().join(format!("pmacs-ovl-{}", std::process::id())); + std::fs::create_dir_all(&dir).expect("mk tempdir"); + let file = dir.join("probe.rs"); + std::fs::write(&file, "fn main() {}\n").expect("write probe file"); + file.display().to_string() +} + +/// `(overlay kinds on the active window, active buffer name)`. +fn kinds(s: &EditorState) -> (Vec, String) { + s.lua_host + .lua() + .load( + r" + local d = pmacs.describe.buffer(pmacs.window.buffer()) + return pmacs.window._overlay_kinds(), d.name + ", + ) + .eval() + .expect("probe overlay kinds") +} + +fn count_of(kinds: &[String], kind: &str) -> usize { + kinds.iter().filter(|k| *k == kind).count() +} + +#[test] +fn switch_away_and_back_reattaches_syntax_overlay_exactly_once() { + let s = EditorState::new(); + let path = open_probe_file(&s); + s.lua_host + .lua() + .load(format!( + r#" + _G.TARGET = pmacs.buffer.find_or_open("{path}") + for _, id in ipairs(pmacs.buffer.list()) do + if pmacs.describe.buffer(id).name == "*scratch*" then _G.SCRATCH = id end + end + "# + )) + .exec() + .expect("open probe + find scratch"); + let (before, _) = kinds(&s); + assert_eq!( + count_of(&before, "syntax-highlight"), + 1, + "fresh open attaches the highlight overlay once (got {before:?})" + ); + + // Away and back — twice, so stacking would show as a count > 1. + s.lua_host + .lua() + .load( + r" + pmacs.window.switch_buffer(_G.SCRATCH) + pmacs.window.switch_buffer(_G.TARGET) + pmacs.window.switch_buffer(_G.SCRATCH) + pmacs.window.switch_buffer(_G.TARGET) + ", + ) + .exec() + .expect("switch away and back twice"); + let (after, name) = kinds(&s); + assert!(name.ends_with("probe.rs"), "back on the probe file"); + assert_eq!( + count_of(&after, "syntax-highlight"), + 1, + "the switch re-attaches exactly one highlight overlay (got {after:?})" + ); +} + +#[test] +fn panel_quit_restores_overlays_on_the_source_buffer() { + let s = EditorState::new(); + let path = open_probe_file(&s); + s.lua_host + .lua() + .load(format!( + r#" + pmacs.buffer.find_or_open("{path}") + pmacs.listview.open {{ + name = "*ovl-panel*", + header = "h", + rows = {{ {{ text = "row", item = 1 }} }}, + }} + "# + )) + .exec() + .expect("open probe + panel"); + // q leaves the panel back to the source file. + let mut s = s; + s.dispatch_key( + pmacs::protocol::FrontendId::LOCAL, + KeyEvent { + code: KeyCode::Char('q'), + modifiers: KeyModifiers::NONE, + kind: KeyEventKind::Press, + state: KeyEventState::empty(), + }, + ); + let (after, name) = kinds(&s); + assert!(name.ends_with("probe.rs"), "q restored the source buffer"); + assert_eq!( + count_of(&after, "syntax-highlight"), + 1, + "leaving a panel restores the source buffer's styling (got {after:?})" + ); +}