Close pmacs-gpu phase A audit

This commit is contained in:
Levi Neuwirth 2026-05-28 12:49:23 -04:00
parent e47536ccae
commit a67cb8a6f1
6 changed files with 325 additions and 21 deletions

View File

@ -0,0 +1,95 @@
# pmacs-gpu Phase A audit
Date: 2026-05-28
Phase A's framing criterion was not "the GUI is done." It was:
exercise the producer-arc wire shapes against a real consumer, absorb
small shape findings, and score the predicted finding categories.
This audit covers the producer/consumer arc through Sessions 5-8:
`StyleSpans`, `Decorations`, `InlineAdornments`,
`FileStyleSummary`, `CursorByte`, and the pmacs-gpu consumer paths
that render or cache them.
## Evidence
| Session | Surface | Evidence |
|---|---|---|
| 5 | `Decorations` consumption | pmacs-gpu paints diagnostic severities as foreground overrides. Session validation surfaced stale diagnostic styling and byte-position replacement mistakes; the producer now resolves URI by `vp.buffer_id`, suppresses stale diagnostics, and full-resyncs style/deco families on CRDT generation transitions. |
| 6 | `InlineAdornments` consumption | pmacs-gpu inserts LSP inlay hints as zero-width virtual text. `m4_29_real_rust_analyzer_inlay_hints_via_auto_attach` covers rust-analyzer's strict range behavior; the GUI was manually validated with a Rust inlay-hints test file. |
| 7 | `FileStyleSummary` consumption | pmacs-gpu renders the right-side minimap from whole-file dominant line style and local rope structure. Manual validation pushed it from one-color-per-line bars to code-shaped strokes. |
| 8 | Temporal probe | Added a 1000-edit synthetic semantic-render probe that exercises CRDT generation transitions, stale inlay suppression, and fresh inlay rehydration. Also extended `didChange` stale marking to inlay hints. |
The Session 8 automated probe is producer-level and deterministic: it
does not spawn rust-analyzer for all 1000 edits. Real-server coverage
for inlay production remains the PATH-gated rust-analyzer acceptance
test; the stress loop validates the temporal contract that LSP-derived
zero-width adornments must not render while their store is stale.
## Findings
| ID | Finding | Category | Classification | Resolution |
|---|---|---|---|---|
| A1 | Incremental `StyleSpans` / `Decorations` could leave cached byte ranges positioned against pre-edit text after CRDT edits. | Headless-test-blind-spot | Small | Track buffer generation and force `full=true` style/deco resync on generation transitions. |
| A2 | Diagnostics could render against stale post-edit text before the next `publishDiagnostics`. | Headless-test-blind-spot | Small | `DiagnosticStore` stale flag; `did_change_full` marks stale; `publishDiagnostics` clears via `set`; semantic producer suppresses stale diagnostics. |
| A3 | Multi-frontend semantic projection could resolve LSP stores through the editor's active buffer rather than the projected buffer. | Headless-test-blind-spot | Small | URI lookup now routes through `vp.buffer_id` for diagnostics, inlay hints, and LSP semantic tokens. |
| A4 | rust-analyzer rejected over-wide inlay-hint ranges, leaving `InlineAdornments` empty in normal Rust files. | Real-server-strictness | Small | Auto-pull inlay hints over the exact document end; guarded by `m4_29_real_rust_analyzer_inlay_hints_via_auto_attach`. |
| A5 | Inlay hints were not marked stale on `didChange`, so zero-width virtual text could stay anchored to pre-edit positions during sustained typing. | Temporal-interaction | Small | `InlayHintStore` now has stale flags; `did_change_full` marks stale; semantic render emits one empty `InlineAdornments` replacement to clear cached virtual text until fresh hints arrive. |
| A6 | `FileStyleSummary` trailing empty line needed an explicit convention. | Convention-vs-contract | Small | Producer test codifies "final newline creates a trailing empty summary line"; minimap consumes that shape directly. |
| A7 | Initial minimap rendering was visually valid but too coarse to be useful. | Consumer-projection-granularity | Small | pmacs-gpu derives indentation/length strokes from its local rope while using `FileStyleSummary` for color. |
| A8 | Selection/search/current-line backgrounds accumulate in consumer state but cannot render via glyph foreground attributes. | Quad-pipeline-needed | Structural | Deferred to the post-Phase-A wgpu quad pipeline. |
## Predicted vs actual
| Predicted bet | Result | Count | Notes |
|---|---:|---:|---|
| `StyleSpans` / `Decorations` dirty-segment edges at viewport boundaries | Surfaced | 3 | A1, A2, A3. The category was right and under-counted: stale state came from generation, freshness, and buffer identity. |
| `InlineAdornments` suppression flicker / edit-then-revert behavior | Surfaced | 1 | A5. Whole-set suppression is acceptable only when the backing store has freshness state. |
| `FileStyleSummary` trailing-empty-line behavior may need wire decision | Surfaced | 1 | A6. This is now a producer convention, not an open contract question. |
| `CursorByte` per-tick cadence may be wrong for a 60fps consumer | Did not surface as a Phase A fix | 0 | Current shape remains state-notification cadence. Cursor-derived backgrounds are deferred to the quad pipeline, so no new wire frequency was justified in Phase A. |
| `PresenceUpdate` peer color stability may need renderer-side identity discipline | Not exercised | 0 | pmacs-gpu does not yet consume peer cursors. This remains a future multi-frontend GUI surface, not a Phase A blocker. |
Unpredicted categories that surfaced:
| Category | Count | Findings |
|---|---:|---|
| Real-server-strictness | 1 | A4: rust-analyzer exact inlay range requirement. |
| Consumer-projection-granularity | 1 | A7: minimap needed local rope shape, not just per-line color. |
| Quad-pipeline-needed | 1 | A8: foreground-only glyph attributes cannot render background decoration kinds. |
Score summary:
- Predicted categories surfaced: 3 of 5.
- Predicted categories not fixed in Phase A: 2 of 5 (`CursorByte`
cadence, `PresenceUpdate` identity).
- Unpredicted categories surfaced: 3.
- Small findings absorbed in Phase A: 7.
- Structural findings deferred: 1.
## Phase A close
Phase A can close for the shipped producer-arc families:
- `StyleSpans`: consumed by pmacs-gpu; generation/full-resync and stale
LSP token suppression are tested.
- `Decorations`: diagnostic foreground rendering works; stale
diagnostics are suppressed. Background kinds are stored but deferred
to the quad pipeline.
- `InlineAdornments`: inlay hints render as virtual text; real
rust-analyzer range behavior and edit-time stale clearing are tested.
- `FileStyleSummary`: minimap consumes the whole-file summary and uses
local rope shape for useful granularity.
- `CursorByte`: still delivered to semantic/CRDT sessions; no Phase A
contract change.
Not closed by Phase A:
- Background rectangles for `Selection`, `SearchMatch`,
`SearchMatchActive`, and `CurrentLine`.
- pmacs-gpu `PresenceUpdate` consumption.
- Producers for `BlockAdornments`, `FoldState`, and `ResourceOffer`.
- Soft-wrap intent beyond the current local frontend implementation.
The next natural step is the wgpu quad pipeline, because it unlocks the
background-bearing decoration kinds already present in the wire and in
pmacs-gpu's cached decoration state.

View File

@ -667,8 +667,11 @@ impl State {
// InlineAdornments use whole-set suppression rather // InlineAdornments use whole-set suppression rather
// than dirty segments, so the same ownership rule // than dirty segments, so the same ownership rule
// applies here: keep the last set until the producer // applies here: keep the last set until the producer
// sends a replacement. Session 8's temporal probe is // sends a replacement. Session 8 closed the stale
// where visible edit-flicker/staleness gets scored. // inlay case producer-side: `didChange` marks the
// inlay store stale, and the producer sends one empty
// replacement to clear cached virtual text until a
// fresh `textDocument/inlayHint` response arrives.
let text = doc.get_text(LORO_TEXT_CONTAINER).to_string(); let text = doc.get_text(LORO_TEXT_CONTAINER).to_string();
self.set_text(&text); self.set_text(&text);
None None

View File

@ -17,7 +17,7 @@
//! Lua reads [`InlayHintStore`] and surfaces hints; a render layer can //! Lua reads [`InlayHintStore`] and surfaces hints; a render layer can
//! subscribe to the same store when it lands. //! subscribe to the same store when it lands.
use std::collections::HashMap; use std::collections::{HashMap, HashSet};
use std::sync::{Arc, Mutex}; use std::sync::{Arc, Mutex};
use serde_json::Value; use serde_json::Value;
@ -153,6 +153,7 @@ fn parse_hint(v: &Value) -> Option<InlayHint> {
#[derive(Default)] #[derive(Default)]
pub struct InlayHintStore { pub struct InlayHintStore {
by_key: HashMap<InlayHintKey, InlayHintResponse>, by_key: HashMap<InlayHintKey, InlayHintResponse>,
stale_uris: HashSet<String>,
} }
/// Key into [`InlayHintStore`]. /// Key into [`InlayHintStore`].
@ -184,12 +185,31 @@ impl InlayHintStore {
/// Replace the response at `key`. /// Replace the response at `key`.
pub fn set(&mut self, key: InlayHintKey, response: InlayHintResponse) { pub fn set(&mut self, key: InlayHintKey, response: InlayHintResponse) {
self.stale_uris.remove(&key.uri);
self.by_key.insert(key, response); 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 hint data for it.
pub fn clear(&mut self, key: &InlayHintKey) { pub fn clear(&mut self, key: &InlayHintKey) {
self.by_key.remove(key); self.by_key.remove(key);
if !self.by_key.keys().any(|k| k.uri == key.uri) {
self.stale_uris.remove(&key.uri);
}
}
/// Mark all inlay-hint entries for `uri` stale. Called when a
/// `textDocument/didChange` is sent so renderers do not paint
/// zero-width adornments at byte anchors from pre-edit text.
pub fn mark_stale(&mut self, uri: impl Into<String>) {
self.stale_uris.insert(uri.into());
}
/// `true` iff `uri` has inlay-hint 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`. /// Look up the entry at `key`.
@ -314,6 +334,40 @@ mod tests {
assert!(s.get(&key).is_none()); assert!(s.get(&key).is_none());
} }
#[test]
fn stale_flag_clears_on_set_and_final_clear() {
let mut s = InlayHintStore::new();
let key = InlayHintKey::new("1", "file:///a");
let response = InlayHintResponse {
hints: vec![InlayHint {
line: 0,
col: 0,
label: "h".into(),
kind: None,
padding_left: false,
padding_right: false,
tooltip: None,
}],
};
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 inlay hints clear stale flag"
);
s.mark_stale("file:///a");
s.clear(&key);
assert!(
!s.is_stale("file:///a"),
"clearing final hint entry clears stale flag"
);
}
#[test] #[test]
fn for_uri_filters_by_uri_and_picks_lowest_server() { fn for_uri_filters_by_uri_and_picks_lowest_server() {
let mk = |label: &str| InlayHintResponse { let mk = |label: &str| InlayHintResponse {

View File

@ -3044,11 +3044,12 @@ 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 // T M11.8 / Session 8 — mark cached LSP-derived render
// semantic-frontend producer suppresses its emission until // families stale so semantic frontends suppress byte ranges
// clangd's next `publishDiagnostics` re-establishes // anchored to pre-edit text until the server refreshes them.
// freshness via `set`. Closes the visible-stale-color // Diagnostics clear on `publishDiagnostics`, semantic tokens
// window observed in session-5 manual validation. // on `textDocument/semanticTokens`, and inlay hints on
// `textDocument/inlayHint`.
self.diag_store self.diag_store
.lock() .lock()
.expect("diag store mutex poisoned") .expect("diag store mutex poisoned")
@ -3057,6 +3058,10 @@ impl LspManager {
.lock() .lock()
.expect("semantic token store mutex poisoned") .expect("semantic token store mutex poisoned")
.mark_stale(uri.clone()); .mark_stale(uri.clone());
self.inlay_hint_store
.lock()
.expect("inlay hint store mutex poisoned")
.mark_stale(uri.clone());
let params = json!({ let params = json!({
"textDocument": { "textDocument": {
"uri": uri, "uri": uri,

View File

@ -453,7 +453,10 @@ impl SemanticRenderState {
/// Every hint is `AtOffset` (inlay hints are inline by definition) /// Every hint is `AtOffset` (inlay hints are inline by definition)
/// carrying `Text` with the (padding-applied) label and the default /// carrying `Text` with the (padding-applied) label and the default
/// style — the instance has no inlay-specific theme face yet, and a /// style — the instance has no inlay-specific theme face yet, and a
/// fabricated one would be dishonest. /// fabricated one would be dishonest. Stale store entries are
/// suppressed the same way stale diagnostics / semantic tokens are:
/// a zero-width hint anchored to pre-edit text is still a byte range
/// bug, even though it has no source-byte width of its own.
fn scoped_inline_adornments(state: &EditorState, vp: &DeclaredViewport) -> Vec<InlineAdornment> { fn scoped_inline_adornments(state: &EditorState, vp: &DeclaredViewport) -> Vec<InlineAdornment> {
let core = state.core.borrow(); let core = state.core.borrow();
let Some(uri) = buffer_file_uri(&core, vp.buffer_id) else { let Some(uri) = buffer_file_uri(&core, vp.buffer_id) else {
@ -462,6 +465,9 @@ fn scoped_inline_adornments(state: &EditorState, vp: &DeclaredViewport) -> Vec<I
let hints = { let hints = {
let store = state.lsp_manager.borrow().inlay_hint_store(); let store = state.lsp_manager.borrow().inlay_hint_store();
let guard = store.lock().expect("inlay-hint store mutex poisoned"); let guard = store.lock().expect("inlay-hint store mutex poisoned");
if guard.is_stale(&uri) {
return Vec::new();
}
match guard.for_uri(&uri) { match guard.for_uri(&uri) {
Some(resp) => resp.hints.clone(), Some(resp) => resp.hints.clone(),
None => return Vec::new(), None => return Vec::new(),
@ -1741,11 +1747,23 @@ mod tests {
// --- Step 3: InlineAdornments from the LSP inlay-hint store --- // --- Step 3: InlineAdornments from the LSP inlay-hint store ---
/// Seed buffer text + path and an inlay-hint store entry. Keyed by fn inlay_uri() -> String {
/// `(server, uri)`; `for_uri` picks the lowest server, so a fixed crate::lsp::path_to_file_uri(std::path::Path::new("/tmp/h.rs"))
/// `"1"` is fine. No LSP client needed — the inlay path never }
/// consults `semantic_style_context` (cols are already byte
/// offsets, Step 0). /// Seed the inlay-hint store. Keyed by `(server, uri)`; `for_uri`
/// picks the lowest server, so a fixed `"1"` is fine. No LSP
/// client needed — the inlay path never consults
/// `semantic_style_context` (cols are already byte offsets, Step 0).
fn set_inlay_store(state: &EditorState, uri: &str, hints: Vec<crate::inlay_hint::InlayHint>) {
let store = state.lsp_manager.borrow().inlay_hint_store();
store.lock().expect("inlay store").set(
crate::inlay_hint::InlayHintKey::new("1", uri),
crate::inlay_hint::InlayHintResponse { hints },
);
}
/// Seed buffer text + path and an inlay-hint store entry.
fn seed_inlay( fn seed_inlay(
state: &EditorState, state: &EditorState,
buffer_id: BufferId, buffer_id: BufferId,
@ -1765,12 +1783,7 @@ mod tests {
.expect("seed buffer text"); .expect("seed buffer text");
core.set_buffer_path(buffer_id, Some(std::path::PathBuf::from("/tmp/h.rs"))); core.set_buffer_path(buffer_id, Some(std::path::PathBuf::from("/tmp/h.rs")));
} }
let uri = crate::lsp::path_to_file_uri(std::path::Path::new("/tmp/h.rs")); set_inlay_store(state, &inlay_uri(), hints);
let store = state.lsp_manager.borrow().inlay_hint_store();
store.lock().expect("inlay store").set(
crate::inlay_hint::InlayHintKey::new("1", uri),
crate::inlay_hint::InlayHintResponse { hints },
);
} }
fn hint(line: u32, col: u32, label: &str) -> crate::inlay_hint::InlayHint { fn hint(line: u32, col: u32, label: &str) -> crate::inlay_hint::InlayHint {
@ -1845,6 +1858,133 @@ mod tests {
} }
} }
#[test]
fn inline_adornments_emit_empty_clear_while_inlay_store_stale() {
let state = empty_state();
let mut s = local();
let bid = active_buffer(&state);
seed_inlay(&state, bid, vec![hint(0, 5, ": i32")]);
s.set_viewport(bid, ByteRange { start: 0, end: 64 }, 0);
assert!(
adornments_of(&s.render_frame(&state)).is_some(),
"first frame ships the adornments"
);
let uri = inlay_uri();
let store = state.lsp_manager.borrow().inlay_hint_store();
store.lock().expect("inlay store").mark_stale(uri.clone());
let clear =
adornments_of(&s.render_frame(&state)).expect("stale transition clears adornments");
assert!(
clear.is_empty(),
"stale hints must clear the frontend's cached virtual text"
);
assert!(
adornments_of(&s.render_frame(&state)).is_none(),
"unchanged stale-empty state is suppressed after the clear"
);
set_inlay_store(&state, &uri, vec![hint(0, 5, ": i32")]);
let refreshed = adornments_of(&s.render_frame(&state)).expect("fresh hints re-emit");
assert_eq!(refreshed.len(), 1);
assert_eq!(refreshed[0].at, 5);
}
#[cfg(feature = "crdt")]
#[test]
fn session8_temporal_probe_sustained_edits_clear_stale_inlays_until_refresh() {
let state = empty_state();
let mut s = local();
let bid = active_buffer(&state);
let path = std::path::PathBuf::from("/tmp/session8-inlay.txt");
let uri = crate::lsp::path_to_file_uri(&path);
{
let mut core = state.core.borrow_mut();
let mut reg = core.registry.borrow_mut();
let buf = reg.get_mut(bid).expect("active buffer");
buf.apply_edit(crate::buffer::EditOp::Insert {
pos: 0,
bytes: b"let x = f();\n",
})
.expect("seed buffer text");
buf.upgrade_to_crdt(1).expect("upgrade to crdt");
drop(reg);
core.set_buffer_path(bid, Some(path));
}
set_inlay_store(&state, &uri, vec![hint(0, 5, ": i32")]);
s.set_viewport(
bid,
ByteRange {
start: 0,
end: 4096,
},
0,
);
assert!(
adornments_of(&s.render_frame(&state)).is_some(),
"baseline emits the fresh inlay hint"
);
let mut clear_frames = 0;
let mut full_style_frames = 0;
let mut full_deco_frames = 0;
for _ in 0..1000 {
{
let core = state.core.borrow();
let mut reg = core.registry.borrow_mut();
let buf = reg.get_mut(bid).expect("active buffer");
buf.apply_edit(crate::buffer::EditOp::Insert {
pos: 0,
bytes: b"x",
})
.expect("typing edit");
}
let store = state.lsp_manager.borrow().inlay_hint_store();
store.lock().expect("inlay store").mark_stale(uri.clone());
let msgs = s.render_frame(&state);
if let Some((full, _)) = style_segments(&msgs)
&& full
{
full_style_frames += 1;
}
if let Some((full, _)) = decorations_of(&msgs)
&& full
{
full_deco_frames += 1;
}
if let Some(items) = adornments_of(&msgs) {
assert!(
items.is_empty(),
"stale inlay hints must not render during sustained typing"
);
clear_frames += 1;
}
}
assert_eq!(
clear_frames, 1,
"first stale frame clears cached hints; later stale frames stay silent"
);
assert_eq!(
full_style_frames, 1000,
"each CRDT generation transition forces a StyleSpans full resync"
);
assert_eq!(
full_deco_frames, 1000,
"each CRDT generation transition forces a Decorations full resync"
);
set_inlay_store(&state, &uri, vec![hint(0, 1005, ": i32")]);
let refreshed = adornments_of(&s.render_frame(&state)).expect("fresh hints re-emit");
assert_eq!(refreshed.len(), 1);
assert_eq!(refreshed[0].at, 1005);
}
// --- M1: FileStyleSummary (minimap producer, Open Q#2) --- // --- M1: FileStyleSummary (minimap producer, Open Q#2) ---
fn summary_of(msgs: &[InstanceMessage]) -> Option<(u64, Vec<Style>)> { fn summary_of(msgs: &[InstanceMessage]) -> Option<(u64, Vec<Style>)> {

View File

@ -1387,6 +1387,13 @@ fn m4_5_did_change_notifications_go_out_after_edits() {
"didChange must mark semantic tokens stale so stale TUI LSP styles are suppressed" "didChange must mark semantic tokens stale so stale TUI LSP styles are suppressed"
); );
} }
{
let store = mgr.borrow().inlay_hint_store();
assert!(
store.lock().expect("inlay hint store").is_stale(uri),
"didChange must mark inlay hints stale so stale semantic frontend virtual text is suppressed"
);
}
// The fake LSP replies with a `pmacs/echo` notification per // The fake LSP replies with a `pmacs/echo` notification per
// didOpen/didChange (5 total). // didOpen/didChange (5 total).
let evs = drain_lsp_until(&sup, &mgr, sid, Duration::from_secs(5), |evs| { let evs = drain_lsp_until(&sup, &mgr, sid, Duration::from_secs(5), |evs| {