T M11.8 — diag-store stale-flag closes LSP-re-analysis-gap surface (#48)
Surfaced during session-5 manual validation as the final iteration of bet #1 from the framing pass: edits that shift byte positions left old diagnostic colors painted over post-edit text in both `pmacs-gpu` and (now-visible) the TUI. Persisted for the full LSP re-analysis window (100ms–5s). PR #47 fixed the StyleSpans side via generation-tracked full=true emission, but Decorations remained vulnerable: the producer's diff shipped old diagnostics from the diag store, whose entries were indexed at pre-edit byte positions until clangd republished. Fix: a per-URI `stale_uris` flag in `DiagnosticStore`. The LSP layer's `did_change_full` marks the URI stale right after sending the notification; the next `publishDiagnostics` absorb path's `set` clears it. The `semantic_render` producer reads `is_stale` and skips diagnostic emission entirely while stale. Effect: between an edit and clangd's next publish, the producer ships zero diagnostic decorations. Frontend's replace/merge clears old positions cleanly. Brief uncolored window (≤ LSP re-analysis latency) replaces the previous wrong-position-color persistence. The correct visual tradeoff: honest emptiness over deceptive staleness. Files changed: - `src/diag.rs` — `DiagnosticStore` gains `stale_uris: HashSet<String>`; new `mark_stale` / `is_stale` API; `set` and `clear` reset the flag on the assumption that absorption / explicit removal mean the LSP has caught up. - `src/lsp.rs` — `LspManager::did_change_full` calls `diag_store.lock().mark_stale(uri)` after `send_notification`. - `src/semantic_render.rs` — `scoped_decorations` reads `is_stale` alongside `for_uri`; when stale, suppresses the diagnostic loop (selection and other non-diagnostic kinds still emit). Tests (all crdt-gated where they reference semantic_render): - `diag::tests::stale_flag_default_false` - `diag::tests::mark_stale_sets_flag` (per-URI scoping) - `diag::tests::set_clears_stale_flag` - `diag::tests::empty_set_clears_stale_flag_too` - `diag::tests::clear_drops_stale_flag` - `semantic_render::tests::diagnostics_suppressed_while_diag_store_stale` — assert no diagnostic kinds emit while stale; assert they re-emit after a fresh `set` clears the flag. Gates: cargo fmt + clippy (workspace, with/without `crdt`) clean; lib 1482 (+6) with crdt; 1319 (+6) without; m4 83; m11_5 2. This is approach (A) from the session-5 ask: track per-URI freshness relative to buffer edits, suppress emission until LSP catches up. Approach (B) — clangd's didChange/publishDiagnostics version matching — would be more precise but requires plumbing version tracking through the LspManager's document state, which is a larger change deferred. The stale-flag captures the same semantic ("any edit since last publish ⇒ stale") at a cheaper cost. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
parent
36a53f8d5a
commit
886239480e
88
src/diag.rs
88
src/diag.rs
|
|
@ -167,9 +167,26 @@ impl Diagnostic {
|
||||||
/// Per-URI diagnostic set. Mutated by [`crate::lsp::LspManager`]
|
/// Per-URI diagnostic set. Mutated by [`crate::lsp::LspManager`]
|
||||||
/// when handling `textDocument/publishDiagnostics`; read by
|
/// when handling `textDocument/publishDiagnostics`; read by
|
||||||
/// [`DiagnosticView`] on every render.
|
/// [`DiagnosticView`] on every render.
|
||||||
|
///
|
||||||
|
/// **T M11.8 — `stale_uris` tracking.** Per-URI flag set by
|
||||||
|
/// [`Self::mark_stale`] whenever the LSP layer ships a
|
||||||
|
/// `textDocument/didChange` for that URI; cleared by [`Self::set`]
|
||||||
|
/// when fresh diagnostics arrive. The semantic-frontend producer
|
||||||
|
/// reads [`Self::is_stale`] and suppresses decorations during the
|
||||||
|
/// window between an edit and clangd's republish, so a `pmacs-gpu`
|
||||||
|
/// (or any `semantic_render`) frontend never paints diagnostic
|
||||||
|
/// colors at byte positions that have since shifted under the
|
||||||
|
/// document. Closes the LSP-re-analysis-gap surface that bet #1
|
||||||
|
/// in the session-4 framing pass predicted.
|
||||||
#[derive(Default)]
|
#[derive(Default)]
|
||||||
pub struct DiagnosticStore {
|
pub struct DiagnosticStore {
|
||||||
by_uri: HashMap<String, Vec<Diagnostic>>,
|
by_uri: HashMap<String, Vec<Diagnostic>>,
|
||||||
|
/// URIs whose stored diagnostics are known to be out of date
|
||||||
|
/// because a `textDocument/didChange` was issued after the last
|
||||||
|
/// `publishDiagnostics` was absorbed. `Self::set` clears entries
|
||||||
|
/// here on the assumption that a fresh `publishDiagnostics`
|
||||||
|
/// corresponds to the latest sent version.
|
||||||
|
stale_uris: std::collections::HashSet<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl DiagnosticStore {
|
impl DiagnosticStore {
|
||||||
|
|
@ -182,9 +199,14 @@ impl DiagnosticStore {
|
||||||
/// Replace the diagnostic set for `uri`. Sorts by start position
|
/// Replace the diagnostic set for `uri`. Sorts by start position
|
||||||
/// so [`Self::next_after`] / [`Self::previous_before`] don't
|
/// so [`Self::next_after`] / [`Self::previous_before`] don't
|
||||||
/// have to re-sort on each query.
|
/// have to re-sort on each query.
|
||||||
|
///
|
||||||
|
/// Also clears the URI's stale flag (T M11.8) — fresh
|
||||||
|
/// diagnostics imply the LSP has caught up to the current
|
||||||
|
/// document state.
|
||||||
pub fn set(&mut self, uri: impl Into<String>, mut diags: Vec<Diagnostic>) {
|
pub fn set(&mut self, uri: impl Into<String>, mut diags: Vec<Diagnostic>) {
|
||||||
diags.sort_by(Diagnostic::compare_by_position);
|
diags.sort_by(Diagnostic::compare_by_position);
|
||||||
let uri = uri.into();
|
let uri = uri.into();
|
||||||
|
self.stale_uris.remove(&uri);
|
||||||
if diags.is_empty() {
|
if diags.is_empty() {
|
||||||
self.by_uri.remove(&uri);
|
self.by_uri.remove(&uri);
|
||||||
} else {
|
} else {
|
||||||
|
|
@ -192,9 +214,28 @@ impl DiagnosticStore {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Drop the diagnostics for `uri`.
|
/// Drop the diagnostics for `uri`. Also clears the stale flag —
|
||||||
|
/// no entry to be stale about.
|
||||||
pub fn clear(&mut self, uri: &str) {
|
pub fn clear(&mut self, uri: &str) {
|
||||||
self.by_uri.remove(uri);
|
self.by_uri.remove(uri);
|
||||||
|
self.stale_uris.remove(uri);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Mark `uri`'s stored diagnostics as stale (T M11.8). Called
|
||||||
|
/// by the LSP layer on each `textDocument/didChange` so the
|
||||||
|
/// `semantic_render` producer can suppress emission during the
|
||||||
|
/// LSP-re-analysis gap. The next [`Self::set`] (or
|
||||||
|
/// [`Self::clear`]) clears the flag.
|
||||||
|
pub fn mark_stale(&mut self, uri: impl Into<String>) {
|
||||||
|
self.stale_uris.insert(uri.into());
|
||||||
|
}
|
||||||
|
|
||||||
|
/// `true` iff the URI's stored diagnostics are stale (the
|
||||||
|
/// document has been edited since the last `publishDiagnostics`
|
||||||
|
/// absorption). T M11.8.
|
||||||
|
#[must_use]
|
||||||
|
pub fn is_stale(&self, uri: &str) -> bool {
|
||||||
|
self.stale_uris.contains(uri)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// All diagnostics for `uri`, in start-position order.
|
/// All diagnostics for `uri`, in start-position order.
|
||||||
|
|
@ -685,6 +726,51 @@ mod tests {
|
||||||
assert_eq!(s.uris().count(), 0);
|
assert_eq!(s.uris().count(), 0);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ---- T M11.8 stale-flag ----
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn stale_flag_default_false() {
|
||||||
|
let s = DiagnosticStore::new();
|
||||||
|
assert!(!s.is_stale("file:///a"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn mark_stale_sets_flag() {
|
||||||
|
let mut s = DiagnosticStore::new();
|
||||||
|
s.mark_stale("file:///a");
|
||||||
|
assert!(s.is_stale("file:///a"));
|
||||||
|
assert!(!s.is_stale("file:///b"), "stale flag is per-URI");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn set_clears_stale_flag() {
|
||||||
|
let mut s = DiagnosticStore::new();
|
||||||
|
s.mark_stale("file:///a");
|
||||||
|
assert!(s.is_stale("file:///a"));
|
||||||
|
s.set("file:///a", vec![diag(0, DiagnosticSeverity::Error, "x")]);
|
||||||
|
assert!(!s.is_stale("file:///a"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn empty_set_clears_stale_flag_too() {
|
||||||
|
// Empty diags after an edit means "LSP has caught up and
|
||||||
|
// there are no issues" — clear stale to allow rendering an
|
||||||
|
// empty decoration set.
|
||||||
|
let mut s = DiagnosticStore::new();
|
||||||
|
s.mark_stale("file:///a");
|
||||||
|
s.set("file:///a", Vec::new());
|
||||||
|
assert!(!s.is_stale("file:///a"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn clear_drops_stale_flag() {
|
||||||
|
let mut s = DiagnosticStore::new();
|
||||||
|
s.set("file:///a", vec![diag(0, DiagnosticSeverity::Error, "x")]);
|
||||||
|
s.mark_stale("file:///a");
|
||||||
|
s.clear("file:///a");
|
||||||
|
assert!(!s.is_stale("file:///a"));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn severity_label_and_glyph_are_stable() {
|
fn severity_label_and_glyph_are_stable() {
|
||||||
for (s, lbl, gl) in [
|
for (s, lbl, gl) in [
|
||||||
|
|
|
||||||
|
|
@ -3044,6 +3044,15 @@ impl LspManager {
|
||||||
let uri = uri.into();
|
let uri = uri.into();
|
||||||
let text = text.into();
|
let text = text.into();
|
||||||
self.documents.insert((sid, uri.clone()), text.clone());
|
self.documents.insert((sid, uri.clone()), text.clone());
|
||||||
|
// T M11.8 — mark the diag-store entry stale so the
|
||||||
|
// semantic-frontend producer suppresses its emission until
|
||||||
|
// clangd's next `publishDiagnostics` re-establishes
|
||||||
|
// freshness via `set`. Closes the visible-stale-color
|
||||||
|
// window observed in session-5 manual validation.
|
||||||
|
self.diag_store
|
||||||
|
.lock()
|
||||||
|
.expect("diag store mutex poisoned")
|
||||||
|
.mark_stale(uri.clone());
|
||||||
let params = json!({
|
let params = json!({
|
||||||
"textDocument": {
|
"textDocument": {
|
||||||
"uri": uri,
|
"uri": uri,
|
||||||
|
|
|
||||||
|
|
@ -390,13 +390,21 @@ impl SemanticRenderState {
|
||||||
// Lua LSP glue opened the document under. The URI is derived
|
// Lua LSP glue opened the document under. The URI is derived
|
||||||
// from `vp.buffer_id` (see [`buffer_file_uri`]'s docs for why
|
// from `vp.buffer_id` (see [`buffer_file_uri`]'s docs for why
|
||||||
// not from `core.active_buffer_path()`).
|
// not from `core.active_buffer_path()`).
|
||||||
|
//
|
||||||
|
// T M11.8 — skip emission while the store is stale (the
|
||||||
|
// document has been edited since the last `publishDiagnostics`
|
||||||
|
// absorption). Without this, the producer ships diagnostics
|
||||||
|
// whose byte positions point at pre-edit text — the visible
|
||||||
|
// wrong-position color artifact session-5 validation surfaced.
|
||||||
|
// The LSP layer's `did_change_full` marks the URI stale; the
|
||||||
|
// next `publishDiagnostics` absorbs and clears the flag.
|
||||||
if let Some(uri) = buffer_file_uri(&core, vp.buffer_id) {
|
if let Some(uri) = buffer_file_uri(&core, vp.buffer_id) {
|
||||||
let diags = {
|
let (diags, is_stale) = {
|
||||||
let store = state.lsp_manager.borrow().diag_store();
|
let store = state.lsp_manager.borrow().diag_store();
|
||||||
let guard = store.lock().expect("diag store mutex poisoned");
|
let guard = store.lock().expect("diag store mutex poisoned");
|
||||||
guard.for_uri(&uri).to_vec()
|
(guard.for_uri(&uri).to_vec(), guard.is_stale(&uri))
|
||||||
};
|
};
|
||||||
if !diags.is_empty() {
|
if !is_stale && !diags.is_empty() {
|
||||||
let registry = core.registry.clone();
|
let registry = core.registry.clone();
|
||||||
let reg = registry.borrow();
|
let reg = registry.borrow();
|
||||||
if let Ok(buf) = reg.get(vp.buffer_id) {
|
if let Ok(buf) = reg.get(vp.buffer_id) {
|
||||||
|
|
@ -1124,6 +1132,76 @@ mod tests {
|
||||||
assert_eq!(decos[0].range, ByteRange { start: 4, end: 6 });
|
assert_eq!(decos[0].range, ByteRange { start: 4, end: 6 });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// T M11.8 regression: when the diag store's entry for the URI
|
||||||
|
/// is marked stale (an edit has been issued since the last
|
||||||
|
/// `publishDiagnostics`), the producer must suppress diagnostic
|
||||||
|
/// emission so the frontend doesn't paint colors at pre-edit
|
||||||
|
/// byte positions over post-edit text. Closes the bet-#1
|
||||||
|
/// surface that session-5 validation exposed.
|
||||||
|
#[test]
|
||||||
|
fn diagnostics_suppressed_while_diag_store_stale() {
|
||||||
|
let state = empty_state();
|
||||||
|
let buffer_id = active_buffer(&state);
|
||||||
|
seed_diagnostic(&state, buffer_id);
|
||||||
|
|
||||||
|
// Mark the diag store stale for the seeded URI. The producer
|
||||||
|
// should now emit zero diagnostic decorations.
|
||||||
|
let uri = crate::lsp::path_to_file_uri(std::path::Path::new("/tmp/m114.rs"));
|
||||||
|
state
|
||||||
|
.lsp_manager
|
||||||
|
.borrow()
|
||||||
|
.diag_store()
|
||||||
|
.lock()
|
||||||
|
.expect("diag store")
|
||||||
|
.mark_stale(uri.clone());
|
||||||
|
|
||||||
|
let mut s = local();
|
||||||
|
s.set_viewport(buffer_id, ByteRange { start: 0, end: 64 }, 0);
|
||||||
|
let (_full, decos) =
|
||||||
|
decorations_of(&s.render_frame(&state)).expect("a Decorations message");
|
||||||
|
assert!(
|
||||||
|
decos.iter().all(|d| !matches!(
|
||||||
|
d.kind,
|
||||||
|
DecorationKind::DiagnosticError
|
||||||
|
| DecorationKind::DiagnosticWarning
|
||||||
|
| DecorationKind::DiagnosticInfo
|
||||||
|
| DecorationKind::DiagnosticHint
|
||||||
|
)),
|
||||||
|
"stale diag store ⇒ no diagnostic decorations emitted; got {decos:?}"
|
||||||
|
);
|
||||||
|
|
||||||
|
// Once a fresh `set` clears the stale flag, the decoration
|
||||||
|
// re-appears on the next render frame.
|
||||||
|
state
|
||||||
|
.lsp_manager
|
||||||
|
.borrow()
|
||||||
|
.diag_store()
|
||||||
|
.lock()
|
||||||
|
.expect("diag store")
|
||||||
|
.set(
|
||||||
|
&uri,
|
||||||
|
vec![crate::diag::Diagnostic {
|
||||||
|
start_line: 1,
|
||||||
|
start_col: 0,
|
||||||
|
end_line: 1,
|
||||||
|
end_col: 2,
|
||||||
|
severity: crate::diag::DiagnosticSeverity::Warning,
|
||||||
|
message: "x".into(),
|
||||||
|
source: None,
|
||||||
|
code: None,
|
||||||
|
}],
|
||||||
|
);
|
||||||
|
|
||||||
|
let (_full, decos) =
|
||||||
|
decorations_of(&s.render_frame(&state)).expect("a Decorations message");
|
||||||
|
assert!(
|
||||||
|
decos
|
||||||
|
.iter()
|
||||||
|
.any(|d| d.kind == DecorationKind::DiagnosticWarning),
|
||||||
|
"fresh set ⇒ stale flag cleared ⇒ decoration re-emitted; got {decos:?}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
/// Regression: in a multi-frontend setup the editor's *active*
|
/// Regression: in a multi-frontend setup the editor's *active*
|
||||||
/// buffer (set by `core.active_buffer_id()`, derived from the
|
/// buffer (set by `core.active_buffer_id()`, derived from the
|
||||||
/// active frontend's view) can differ from the buffer a given
|
/// active frontend's view) can differ from the buffer a given
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue