Merge pull request #53 from levineuwirth/task-25-stale-styling

Fix stale TUI styling after edits

builtin/runtime/syntax.lua

@@ -17,6 +17,11 @@
 -- when `pmacs.parse._dispatch` is called below; drained by the tick
 -- step.
 local pending_parse_jobs = {}
+local parse_job_buffer_keys = {}
+local inflight_parse_by_buffer = {}
+local parse_buffer_by_key = {}
+local parse_lang_by_buffer = {}
+local reparse_requested_by_buffer = {}
 
 local raw_dispatch = pmacs.parse._dispatch
 
@@ -24,8 +29,18 @@ local raw_dispatch = pmacs.parse._dispatch
 -- pending set. Calls into `_dispatch` from outside this file (e.g.
 -- a hand-rolled user script) get the same tracking for free.
 function pmacs.parse._dispatch(buf, lang)
+  local key = tostring(buf)
+  parse_buffer_by_key[key] = buf
+  parse_lang_by_buffer[key] = lang
+  local inflight = inflight_parse_by_buffer[key]
+  if inflight then
+    reparse_requested_by_buffer[key] = true
+    return inflight
+  end
   local job_id = raw_dispatch(buf, lang)
   pending_parse_jobs[job_id] = true
+  parse_job_buffer_keys[job_id] = key
+  inflight_parse_by_buffer[key] = job_id
   return job_id
 end
 
@@ -68,6 +83,40 @@ pmacs.hook.add("buffer.after-load", function()
   end
 end)
 
+local function reparse_active_buffer_after_edit()
+  local buf = pmacs.window.buffer()
+  if not buf then return end
+  if not pmacs.parse._has_view(buf) then return end
+  local pending = pmacs.parse._pending_edits(buf)
+  if not pending or pending == 0 then return end
+  local path = buf:name()
+  if not path then return end
+  local lang = pmacs.parse.language_for_path(path)
+  if not lang then return end
+  pmacs.parse._dispatch(buf, lang)
+end
+
+pmacs.hook.add("buffer.after-edit", function()
+  -- `ParseView:on_edit` records incremental edits synchronously, but
+  -- highlight overlays only see new spans after a fresh parse settles.
+  local ok, err = pcall(reparse_active_buffer_after_edit)
+  if not ok and pmacs.error then
+    pmacs.error("syntax.after-edit: " .. tostring(err))
+  end
+end)
+
+local function dispatch_follow_up_if_dirty(key)
+  local buf = parse_buffer_by_key[key]
+  local lang = parse_lang_by_buffer[key]
+  if not buf or not lang then return end
+  local pending = pmacs.parse._pending_edits(buf)
+  local requested = reparse_requested_by_buffer[key]
+  reparse_requested_by_buffer[key] = nil
+  if requested or (pending and pending > 0) then
+    pmacs.parse._dispatch(buf, lang)
+  end
+end
+
 -- After-tick step: any parse job that has settled gets its bundle
 -- installed into the buffer's view (and its pending entry drained).
 -- Extension hook on top of the async runtime's tick rather than a
@@ -78,8 +127,14 @@ pmacs._async.tick = function(...)
   local ret = prior_tick(...)
   for job_id in pairs(pending_parse_jobs) do
     if pmacs._async._is_complete(job_id) then
+      local key = parse_job_buffer_keys[job_id]
       pmacs.parse._install_settled(job_id)
       pending_parse_jobs[job_id] = nil
+      parse_job_buffer_keys[job_id] = nil
+      if key and inflight_parse_by_buffer[key] == job_id then
+        inflight_parse_by_buffer[key] = nil
+        dispatch_follow_up_if_dirty(key)
+      end
     end
   end
   return ret

src/diag.rs

@@ -421,6 +421,9 @@ impl View for DiagnosticView {
         // doesn't today, but the discipline is cheap).
         let diags: Vec<Diagnostic> = {
             let guard = self.store.lock().expect("diag store mutex poisoned");
+            if guard.is_stale(&self.uri) {
+                return;
+            }
             guard.for_uri(&self.uri).to_vec()
         };
         if diags.is_empty() {
@@ -796,4 +799,50 @@ mod tests {
         let view = DiagnosticView::new("file:///a", store);
         assert_eq!(view.kind(), "diagnostic");
     }
+
+    #[test]
+    fn diagnostic_view_suppresses_stale_store_entries() {
+        use crate::cell::{Cell, CellSize, UnderlineStyle};
+
+        let store = make_shared_store();
+        {
+            let mut guard = store.lock().expect("diag store");
+            guard.set(
+                "file:///a",
+                vec![diag(0, DiagnosticSeverity::Error, "stale")],
+            );
+            guard.mark_stale("file:///a");
+        }
+
+        let mut buf = Buffer::new(crate::buffer::BufferId::next(), "test.c");
+        buf.apply_edit(crate::buffer::EditOp::Insert {
+            pos: 0,
+            bytes: b"hello\n",
+        })
+        .expect("seed buffer");
+
+        let mut view = DiagnosticView::new("file:///a", store);
+        let mut backing = vec![Cell::default(); 10];
+        let mut grid = CellGrid {
+            cells: &mut backing,
+            stride: 10,
+            size: CellSize::new(1, 10),
+        };
+        view.render(
+            &buf,
+            Viewport {
+                buffer_start: 0,
+                buffer_end: buf.len(),
+                cell_origin: CellCoord::new(0, 0),
+                cell_size: CellSize::new(1, 10),
+            },
+            &mut grid,
+        );
+
+        assert_eq!(
+            grid.get(CellCoord::new(0, 0)).style.underline,
+            UnderlineStyle::None,
+            "stale diagnostics must not underline shifted TUI bytes"
+        );
+    }
 }

src/highlight.rs

@@ -531,6 +531,9 @@ impl View for LspStyleView {
             };
             let store = mgr.semantic_token_store();
             let guard = store.lock().expect("semantic-token store mutex poisoned");
+            if guard.is_stale(&uri) {
+                return;
+            }
             let Some((_, resp)) = guard.for_uri(&uri) else {
                 return;
             };
@@ -878,6 +881,94 @@ mod tests {
         );
     }
 
+    #[test]
+    fn lsp_style_view_suppresses_stale_semantic_tokens() {
+        use crate::cell::{Cell, CellSize};
+        use crate::editor::EditorState;
+        use crate::lsp::PositionEncoding;
+        use crate::semantic_tokens::{SemanticToken, SemanticTokenKey, SemanticTokensResponse};
+
+        let state = EditorState::new();
+        let buffer_id = state.core.borrow().active_window().buffer_id;
+        {
+            let mut core = state.core.borrow_mut();
+            core.registry
+                .clone()
+                .borrow_mut()
+                .get_mut(buffer_id)
+                .expect("active buffer")
+                .apply_edit(crate::buffer::EditOp::Insert {
+                    pos: 0,
+                    bytes: b"foo\n",
+                })
+                .expect("seed");
+            core.set_buffer_path(buffer_id, Some(std::path::PathBuf::from("/tmp/x.c")));
+        }
+        state.syntax_registry.theme().lock().expect("theme").insert(
+            "function",
+            Style {
+                bold: true,
+                ..Style::default()
+            },
+        );
+        let sid = state
+            .lsp_manager
+            .borrow_mut()
+            .insert_initialized_test_client(
+                serde_json::json!({
+                    "semanticTokensProvider": {
+                        "legend": { "tokenTypes": ["function"], "tokenModifiers": [] }
+                    }
+                }),
+                PositionEncoding::Utf16,
+            );
+        let active_path = state.core.borrow().active_buffer_path().expect("path set");
+        let uri = crate::lsp::path_to_file_uri(&active_path);
+        {
+            let mgr = state.lsp_manager.borrow();
+            let store = mgr.semantic_token_store();
+            let mut guard = store.lock().expect("store");
+            guard.set(
+                SemanticTokenKey::new(sid.raw().to_string(), uri.clone()),
+                SemanticTokensResponse {
+                    tokens: vec![SemanticToken {
+                        line: 0,
+                        start: 0,
+                        length: 3,
+                        token_type: 0,
+                        token_modifiers: 0,
+                    }],
+                    result_id: None,
+                    raw: Vec::new(),
+                },
+            );
+            guard.mark_stale(uri);
+        }
+
+        let mut view = LspStyleView::new(state.lsp_manager.clone(), state.syntax_registry.theme());
+        let mut backing: Vec<Cell> = vec![Cell::default(); 20];
+        let mut grid = CellGrid {
+            cells: &mut backing,
+            stride: 20,
+            size: CellSize::new(1, 20),
+        };
+        let viewport = Viewport {
+            buffer_start: 0,
+            buffer_end: u64::MAX,
+            cell_origin: CellCoord::new(0, 0),
+            cell_size: CellSize::new(1, 20),
+        };
+        let registry = state.core.borrow().registry.clone();
+        let reg = registry.borrow();
+        let buf = reg.get(buffer_id).expect("buffer");
+        view.render(buf, viewport, &mut grid);
+
+        assert!(
+            !grid.get(CellCoord::new(0, 0)).style.bold,
+            "stale semantic tokens must not paint over current syntax/text"
+        );
+    }
+
     #[test]
     #[allow(clippy::too_many_lines)] // Scripted fixture; the headline
     // test next door is similarly

src/lsp.rs

@@ -3053,6 +3053,10 @@ impl LspManager {
             .lock()
             .expect("diag store mutex poisoned")
             .mark_stale(uri.clone());
+        self.semantic_token_store
+            .lock()
+            .expect("semantic token store mutex poisoned")
+            .mark_stale(uri.clone());
         let params = json!({
             "textDocument": {
                 "uri": uri,

src/semantic_render.rs

@@ -770,6 +770,9 @@ fn lsp_scoped_style_spans(state: &EditorState, vp: &DeclaredViewport) -> Vec<Sty
     let tokens = {
         let store = mgr.semantic_token_store();
         let guard = store.lock().expect("semantic-token store mutex poisoned");
+        if guard.is_stale(&uri) {
+            return Vec::new();
+        }
         match guard.for_uri(&uri) {
             Some((_, resp)) => resp.tokens.clone(),
             None => return Vec::new(),

src/semantic_tokens.rs

@@ -25,7 +25,7 @@
 //! priority vs. tree-sitter) is a separate rendering milestone; like
 //! the other LSP features, nothing here paints — Lua reads the store.
 
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::sync::{Arc, Mutex};
 
 use serde_json::Value;
@@ -219,6 +219,10 @@ impl SemanticTokensLegend {
 #[derive(Default)]
 pub struct SemanticTokenStore {
     by_key: HashMap<SemanticTokenKey, SemanticTokensResponse>,
+    /// URIs whose stored semantic tokens are known to be stale
+    /// because the document changed after the last full/delta token
+    /// response was absorbed.
+    stale_uris: HashSet<String>,
 }
 
 /// Key into [`SemanticTokenStore`].
@@ -250,12 +254,31 @@ impl SemanticTokenStore {
 
     /// Replace the response at `key`.
     pub fn set(&mut self, key: SemanticTokenKey, response: SemanticTokensResponse) {
+        self.stale_uris.remove(&key.uri);
         self.by_key.insert(key, response);
     }
 
-    /// Drop the entry at `key`.
+    /// Drop the entry at `key`. Also clears the stale flag for that
+    /// URI when no other server has token data for it.
     pub fn clear(&mut self, key: &SemanticTokenKey) {
         self.by_key.remove(key);
+        if !self.by_key.keys().any(|k| k.uri == key.uri) {
+            self.stale_uris.remove(&key.uri);
+        }
+    }
+
+    /// Mark all semantic-token entries for `uri` stale. Called when
+    /// a `textDocument/didChange` is sent so renderers do not paint
+    /// byte ranges from a pre-edit token set.
+    pub fn mark_stale(&mut self, uri: impl Into<String>) {
+        self.stale_uris.insert(uri.into());
+    }
+
+    /// `true` iff `uri` has semantic-token data that should not be
+    /// rendered against the current buffer text.
+    #[must_use]
+    pub fn is_stale(&self, uri: &str) -> bool {
+        self.stale_uris.contains(uri)
     }
 
     /// Look up the entry at `key`.
@@ -425,6 +448,40 @@ mod tests {
         assert!(s.get(&key).is_none());
     }
 
+    #[test]
+    fn stale_flag_clears_on_set_and_final_clear() {
+        let mut s = SemanticTokenStore::new();
+        let key = SemanticTokenKey::new("1", "file:///a");
+        let response = SemanticTokensResponse {
+            tokens: vec![SemanticToken {
+                line: 0,
+                start: 0,
+                length: 1,
+                token_type: 0,
+                token_modifiers: 0,
+            }],
+            result_id: None,
+            raw: vec![0, 0, 1, 0, 0],
+        };
+
+        s.set(key.clone(), response.clone());
+        s.mark_stale("file:///a");
+        assert!(s.is_stale("file:///a"));
+
+        s.set(key.clone(), response);
+        assert!(
+            !s.is_stale("file:///a"),
+            "fresh semantic tokens clear stale flag"
+        );
+
+        s.mark_stale("file:///a");
+        s.clear(&key);
+        assert!(
+            !s.is_stale("file:///a"),
+            "clearing final token entry clears stale flag"
+        );
+    }
+
     #[test]
     fn for_uri_filters_by_uri_and_picks_lowest_server() {
         let mk = |tok_type: u32| SemanticTokensResponse {

tests/m4_acceptance.rs

@@ -87,8 +87,10 @@ use std::fmt::Write as _;
 use std::sync::Arc;
 use std::time::{Duration, Instant};
 
+use crossterm::event::{KeyCode, KeyEvent, KeyModifiers};
 use pmacs::async_runtime::{AsyncRuntime, JobOutcome, JobResult};
 use pmacs::buffer::{Buffer, BufferId, EditOp};
+use pmacs::protocol::FrontendId;
 use pmacs::syntax::{self, ParseRequest, ParseView};
 
 /// Synthetic Rust source: `n` lines of plausible-but-trivial code.
@@ -330,6 +332,20 @@ fn current_tree_language(state: &pmacs::editor::EditorState) -> Option<String> {
     lua.load(chunk).eval::<Option<String>>().ok().flatten()
 }
 
+/// Returns the source snapshot attached to the active buffer's most
+/// recently installed parse tree.
+fn current_tree_text(state: &pmacs::editor::EditorState) -> Option<String> {
+    let lua = state.lua_host.lua();
+    let chunk = r"
+        local buf = pmacs.window.buffer()
+        if not buf then return nil end
+        local tree = pmacs.parse.tree(buf)
+        if not tree then return nil end
+        return tree:text()
+    ";
+    lua.load(chunk).eval::<Option<String>>().ok().flatten()
+}
+
 /// Opening a `.rs` file produces a parse tree whose root is a
 /// `source_file` and whose language label is `rust`. The dispatch
 /// runs through the `buffer.after-load` hook installed by
@@ -625,6 +641,75 @@ fn m4_3_highlight_updates_within_one_frame_after_parse() {
     );
 }
 
+/// Regression for task #25: an edit delivered through the normal
+/// key-dispatch path must trigger the syntax runtime to dispatch a
+/// fresh parse. Without the `buffer.after-edit` hook in
+/// `builtin/runtime/syntax.lua`, `ParseView:on_edit` accumulates a
+/// pending edit but `pmacs.parse.tree(buf)` remains the old source
+/// indefinitely, which leaves syntax colors pinned to stale byte
+/// offsets in both frontends.
+#[test]
+fn m4_3_key_edit_reparses_active_buffer_without_manual_dispatch() {
+    let dir = tempfile::tempdir().expect("tempdir");
+    let path = dir.path().join("after_edit.rs");
+    std::fs::write(&path, b"fn main() {}\n").expect("write");
+
+    let mut state = open_and_wait_for_parse(path);
+    assert_eq!(current_tree_text(&state).as_deref(), Some("fn main() {}\n"));
+
+    state.dispatch_key(
+        FrontendId::LOCAL,
+        KeyEvent::new(KeyCode::Char('x'), KeyModifiers::NONE),
+    );
+
+    let pending_after_key: Option<usize> = state
+        .lua_host
+        .lua()
+        .load(
+            r"
+            local buf = pmacs.window.buffer()
+            return pmacs.parse._pending_edits(buf)
+        ",
+        )
+        .eval()
+        .expect("pending edit count");
+    assert_eq!(
+        pending_after_key,
+        Some(0),
+        "after-edit syntax hook should dispatch and drain pending parse edits"
+    );
+
+    state.dispatch_key(
+        FrontendId::LOCAL,
+        KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE),
+    );
+
+    let pending_after_second_key: Option<usize> = state
+        .lua_host
+        .lua()
+        .load(
+            r"
+            local buf = pmacs.window.buffer()
+            return pmacs.parse._pending_edits(buf)
+        ",
+        )
+        .eval()
+        .expect("pending edit count after queued edit");
+    assert_eq!(
+        pending_after_second_key,
+        Some(1),
+        "edits made while a parse is in flight should wait for a follow-up parse"
+    );
+
+    pump_async(&mut state, |s| {
+        current_tree_text(s).as_deref() == Some("xyfn main() {}\n")
+    });
+    assert_eq!(
+        current_tree_text(&state).as_deref(),
+        Some("xyfn main() {}\n")
+    );
+}
+
 /// A Lua-set theme observably changes the rendered cells: replacing
 /// the default theme with one that maps `keyword` to a distinctive
 /// foreground color produces cells in that color where keywords
@@ -1295,6 +1380,13 @@ fn m4_5_did_change_notifications_go_out_after_edits() {
             .did_change_full(sid, uri, v, text)
             .expect("didChange");
     }
+    {
+        let store = mgr.borrow().semantic_token_store();
+        assert!(
+            store.lock().expect("semantic token store").is_stale(uri),
+            "didChange must mark semantic tokens stale so stale TUI LSP styles are suppressed"
+        );
+    }
     // The fake LSP replies with a `pmacs/echo` notification per
     // didOpen/didChange (5 total).
     let evs = drain_lsp_until(&sup, &mgr, sid, Duration::from_secs(5), |evs| {