From 2fe6738d684e93c5ffa077ebb9ce3de256484404 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 15 Jul 2026 10:49:10 +0100 Subject: [PATCH] fix(themes): PR #120 round 3 -- GPU snapshot symmetry, count freeze Finding 1: the round-2 reset contract was asymmetric. The producer resets search/menu/status baselines on every snapshot send, but the GPU's BufferSnapshot arm only cleared spans, decorations, adornments, summary, and the completion popup -- a menu or search open at switch time survived the snapshot with no close message ever coming (the new buffer's first CLOSED state is suppressed daemon-side), leaving a stale popup that also held daemon_intercepts_keys true and swallowed pointer events indefinitely. The arm now clears search_prompt, menu, and status_facts; the minibuffer is deliberately exempt on both sides (one global core instance, matching the producer's surviving last_minibuffer baseline). GPU test opens search + menu + status via the real wire arms, applies a snapshot, and asserts all three clear, the intercept gate releases, and the popup pixels vanish -- hand-bitten by disabling the three clears (fix and test share main.rs). Finding 2: the round-2 reset broke the diagnostic-count freeze. last_status was both the peer emission baseline and the stale-store freeze source, so a snapshot between didChange and fresh diagnostics re-shipped StatusFacts with zeroed counts. The freeze source now lives apart: frozen_diag_counts advances on every fresh count, is read when the store is stale, and survives on_buffer_snapshot_sent -- which keeps killing the emission baseline to force the re-send. Acceptance renders (1,1), marks the store stale, applies the reset, and asserts the re-sent StatusFacts still carries (1,1); runtime bite vs pre-fix semantic_render.rs fails exactly as predicted (Some((0,0)) vs Some((1,1))). Framing revision 7; acceptance items 31-32; the protocol doc's snapshot-reset paragraph now lists the full frontend drop set and names the count freeze as daemon knowledge, not peer state. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01VoiEyuPjoBhvwACf8HAnLB --- docs/semantic-frontend-protocol.md | 20 ++++--- docs/theme-faces-framing.md | 53 ++++++++++++++++-- pmacs-gpu/src/main.rs | 90 ++++++++++++++++++++++++++++++ src/semantic_render.rs | 44 ++++++++++++--- tests/theme_faces_acceptance.rs | 76 ++++++++++++++++++++++++- 5 files changed, 259 insertions(+), 24 deletions(-) diff --git a/docs/semantic-frontend-protocol.md b/docs/semantic-frontend-protocol.md index 225c933..870bc7d 100644 --- a/docs/semantic-frontend-protocol.md +++ b/docs/semantic-frontend-protocol.md @@ -119,14 +119,18 @@ variants, and one frontend→instance `Viewport` variant. **`BufferSnapshot` resets buffer-scoped interpretation state.** A frontend receiving a snapshot drops everything it holds for the named buffer — spans, decorations, adornments, minimap summary, -completion popup — and rebuilds from the frames that follow; the -instance mirrors this by invalidating its per-buffer emission -baselines whenever it writes a snapshot, so the frontend's -post-snapshot viewport declaration receives authoritative re-sends -even when nothing changed daemon-side (the unchanged-generation -A → B → A revisit). Bufferless facts (`ThemeFacts`, the minibuffer -prompt) and per-frontend state (the gutter mode) survive snapshots -on both sides. +completion popup, search and menu prompts (which also gate the +frontend's key/pointer interception), and status facts — and +rebuilds from the frames that follow; the instance mirrors this by +invalidating its per-buffer emission baselines whenever it writes a +snapshot, so the frontend's post-snapshot viewport declaration +receives authoritative re-sends even when nothing changed +daemon-side (the unchanged-generation A → B → A revisit). Bufferless +facts (`ThemeFacts`, the minibuffer prompt) and per-frontend state +(the gutter mode) survive snapshots on both sides, and the +instance's stale-store diagnostic-count freeze is daemon knowledge, +not peer state — the re-sent `StatusFacts` after a snapshot carries +the frozen counts, never zeros. ## Capability and version mechanics diff --git a/docs/theme-faces-framing.md b/docs/theme-faces-framing.md index 9216ff4..598f3bd 100644 --- a/docs/theme-faces-framing.md +++ b/docs/theme-faces-framing.md @@ -1,7 +1,29 @@ # Theme faces — framing (Arc 4 stage 1, themes) -**Revision 6 — 2026-07-15. Status: implemented on branch -`theme-faces` (PR #120); revision 6 folds PR round 2.** +**Revision 7 — 2026-07-15. Status: implemented on branch +`theme-faces` (PR #120); revision 7 folds PR round 3.** + +Revision 7 (PR #120 round 3, findings 1–2): the reset contract is +now symmetric on the GPU. The round-2 producer reset covered +search/menu/status baselines, but the GPU's `BufferSnapshot` arm +only cleared spans, decorations, adornments, summary, and the +completion popup — a menu (or search) open at switch time survived +the snapshot with no close message ever coming (the new buffer's +first CLOSED state is suppressed daemon-side), leaving a stale +popup that also gated key and pointer interception +(`daemon_intercepts_keys`) indefinitely. The arm now clears +`search_prompt`, `menu`, and `status_facts`; the minibuffer is +deliberately exempt (one global core instance, matching the +producer's surviving `last_minibuffer` baseline) (finding 1). And +the round-2 reset broke the diagnostic-count freeze: `last_status` +was both the peer emission baseline and the stale-store freeze +source, so a snapshot between didChange and fresh diagnostics +re-shipped `StatusFacts` with zeroed counts. The freeze source now +lives apart — `frozen_diag_counts`, daemon-side knowledge about the +buffer, advanced on every fresh count and read when the store is +stale — and survives `on_buffer_snapshot_sent`, which keeps killing +the emission baseline to force the re-send (finding 2). Acceptance +items 31–32. Revision 6 (PR #120 round 2, findings 1–2): the producer gains the **snapshot/baseline reset contract** @@ -627,10 +649,19 @@ pub struct ThemeFace { needs no call. NOT reset: `last_face_epoch` / `last_theme_faces` (this channel is bufferless — the frontend keeps its face table across snapshots), the global minibuffer baseline, the - per-frontend gutter mode, the revision-keyed diag line cache, and - other buffers' baselines (each buffer's own snapshot precedes its - revisit). Resetting on a failed write is harmless — the failure - mode is one redundant re-send, never staleness. + per-frontend gutter mode, the revision-keyed diag line cache, + `frozen_diag_counts` (round 3 finding 2: the stale-store freeze + source is daemon-side knowledge about the buffer, split from the + `last_status` peer baseline precisely so the reset cannot zero + mid-edit counts), and other buffers' baselines (each buffer's own + snapshot precedes its revisit). Resetting on a failed write is + harmless — the failure mode is one redundant re-send, never + staleness. The GPU's `BufferSnapshot` arm mirrors the contract + (round 3 finding 1): it clears its buffer-scoped facts — + search/menu popups (which gate key and pointer interception) and + the status band — alongside spans, decorations, adornments, + summary, and the completion popup; the minibuffer survives on + both sides. - `docs/semantic-frontend-protocol.md` gains the channel's contract section. @@ -951,3 +982,13 @@ Keybinding-driven tests dispatch keys, never `pmacs.command.invoke`. round trip (same text both buffers, isolating the minimap), the stale minimap is gone, and applying the re-shipped summary at the same generation restores the first visit's pixels exactly. +31. **Snapshots clear GPU search/menu/status and release the + intercept gate** (PR round 3 finding 1, `PMACS_REQUIRE_GPU=1`): + with an open search prompt, menu popup, and status facts, + applying a `BufferSnapshot` clears all three, flips + `daemon_intercepts_keys` back to local key handling, and removes + the popup pixels — hand-bitten by disabling the three clears. +32. **The reset preserves the diagnostic-count freeze** (PR round 3 + finding 2): render nonzero counts, mark the store stale + (didChange), apply the snapshot reset — the re-sent + `StatusFacts` carries the frozen nonzero counts, never (0, 0). diff --git a/pmacs-gpu/src/main.rs b/pmacs-gpu/src/main.rs index 1b34fe8..79567e4 100644 --- a/pmacs-gpu/src/main.rs +++ b/pmacs-gpu/src/main.rs @@ -2465,6 +2465,22 @@ impl State { // keep hijacking Esc/RET/TAB. The daemon-side session // was already invalidated by the switch. self.completion = None; + // PR #120 round 3 finding 1 — the remaining + // buffer-scoped facts, same reasoning: search and menu + // popups anchor in the prior buffer AND gate key and + // pointer interception (`daemon_intercepts_keys`, the + // pointer arms), and the new buffer's first CLOSED + // state is suppressed daemon-side, so no close message + // ever comes — a retained popup would hijack input + // forever. The status band's name/counts describe the + // buffer we just left; the producer's reset contract + // re-ships the new buffer's facts on its first frame. + // The minibuffer deliberately survives: it is one + // global core instance, matching the producer's + // surviving `last_minibuffer` baseline. + self.search_prompt = None; + self.menu = None; + self.status_facts = None; self.cursor_fresh = false; self.optimistic_cursor_floor = None; self.optimistic_floor_set_at = None; @@ -8777,6 +8793,80 @@ mod tests { ); } + #[test] + fn headless_snapshot_clears_search_menu_status_and_the_intercept_gate() { + // PR #120 round 3 finding 1: search/menu popups anchor in the + // prior buffer AND gate key/pointer interception, and the new + // buffer's first CLOSED state is suppressed daemon-side, so + // no close message ever comes — a `BufferSnapshot` must clear + // them (and the stale status band) or the popup hijacks input + // forever. The minibuffer is global and deliberately exempt. + let text = "alpha\nbeta\n"; + let Some(mut state) = headless_or_skip(400, 300, text) else { + return; + }; + let bid_a = BufferId::next(); + state.current_buffer_id = Some(bid_a); + let _ = state.apply_attach_message(InstanceMessage::DispatchIdle { idle: true }); + assert!( + !state.daemon_intercepts_keys(), + "idle with nothing open: keys apply locally" + ); + + let _ = state.apply_attach_message(InstanceMessage::SearchPrompt { + buffer_id: bid_a, + query: Some("al".into()), + active: Some(0), + total: 1, + regex: false, + invalid: false, + }); + let _ = state.apply_attach_message(InstanceMessage::MenuPrompt { + buffer_id: bid_a, + rows: vec![MenuPromptRow { + label: "Cut".into(), + separator: false, + }], + active: Some(0), + }); + let _ = state.apply_attach_message(InstanceMessage::StatusFacts { + buffer_id: bid_a, + name: "old.rs".into(), + modified: false, + diag_errors: 3, + diag_warnings: 1, + message: None, + }); + assert!( + state.daemon_intercepts_keys(), + "an open search/menu round-trips every key" + ); + let with_popups = state.render_offscreen(); + + let doc = loro::LoroDoc::new(); + doc.get_text(LORO_TEXT_CONTAINER) + .insert(0, text) + .expect("insert snapshot text"); + let _ = state.apply_attach_message(InstanceMessage::BufferSnapshot { + buffer_id: BufferId::next(), + crdt_snapshot: doc.export(loro::ExportMode::Snapshot).expect("export"), + }); + + assert!( + state.search_prompt.is_none() && state.menu.is_none() && state.status_facts.is_none(), + "buffer-scoped search/menu/status facts die with the snapshot" + ); + assert!( + !state.daemon_intercepts_keys(), + "the intercept gate releases — keys apply locally again" + ); + assert_ne!( + with_popups, + state.render_offscreen(), + "the popup pixels are gone" + ); + } + #[test] fn own_wash_faces_color_local_rects_peers_keep_the_constant() { // Acceptance 21 + 23: decode the emitted decoration vertex diff --git a/src/semantic_render.rs b/src/semantic_render.rs index 55d170a..a098222 100644 --- a/src/semantic_render.rs +++ b/src/semantic_render.rs @@ -177,6 +177,16 @@ pub struct SemanticRenderState { /// emitted as `StatusFacts` (Q#S1; `message` since v15) — /// cached-compare suppression. last_status: HashMap)>, + /// Last *fresh* whole-file diagnostic counts per buffer — the + /// freeze source while the diag store is stale (mid-edit counts + /// merely lag; flickering to zero per keystroke would be worse). + /// Split from `last_status` (PR #120 round 3 finding 2): that map + /// is a PEER emission baseline and dies with a `BufferSnapshot` + /// (`on_buffer_snapshot_sent`), while this is daemon-side + /// knowledge about the buffer and must survive the reset — or a + /// buffer switch between didChange and fresh diagnostics would + /// re-ship `StatusFacts` with zeroed counts. + frozen_diag_counts: HashMap, /// Last-emitted line-number gutter mode (UX gutter arc, protocol v14) — /// cached-compare suppression. Seeded to `Some(Off)` (the frontend's /// default) so an off gutter never emits. Per-frontend (one value), @@ -352,6 +362,7 @@ impl SemanticRenderState { last_completion_popup: HashMap::new(), last_summary: HashMap::new(), last_status: HashMap::new(), + frozen_diag_counts: HashMap::new(), // Seed to the frontend's default (gutter off): a plain default // window never emits `LineNumbers`, so the common case adds no // traffic and the first frame is unchanged. Only an actual @@ -403,10 +414,14 @@ impl SemanticRenderState { /// table across snapshots), `last_minibuffer` (one global core /// instance, not buffer-scoped), `last_line_numbers` /// (per-frontend gutter mode, kept by the frontend across the - /// switch), and `diag_line_cache` (a revision-keyed compute - /// cache, not a peer-state baseline). Baselines for OTHER buffers - /// also survive — the snapshot names one buffer, and any buffer - /// the frontend navigates to receives its own snapshot first. + /// switch), `diag_line_cache` (a revision-keyed compute cache, + /// not a peer-state baseline), and `frozen_diag_counts` (the + /// stale-store freeze source — daemon-side knowledge about the + /// buffer, not about the peer; deleting it would zero the counts + /// on a mid-edit revisit, round 3 finding 2). Baselines for OTHER + /// buffers also survive — the snapshot names one buffer, and any + /// buffer the frontend navigates to receives its own snapshot + /// first. pub fn on_buffer_snapshot_sent(&mut self, buffer_id: BufferId) { self.last_sent.remove(&buffer_id); self.last_style_gate.remove(&buffer_id); @@ -919,11 +934,24 @@ impl SemanticRenderState { } }) }; - let cached = self.last_status.get(&buffer_id); - let (diag_errors, diag_warnings) = - counts.unwrap_or_else(|| cached.map_or((0, 0), |c| (c.2, c.3))); + // Fresh counts advance the freeze source; a stale store reads + // it back. Never `last_status` — that is the peer emission + // baseline and dies with a `BufferSnapshot`, while the frozen + // counts are daemon-side knowledge that must survive the + // reset (round 3 finding 2). + let (diag_errors, diag_warnings) = match counts { + Some(fresh) => { + self.frozen_diag_counts.insert(buffer_id, fresh); + fresh + } + None => self + .frozen_diag_counts + .get(&buffer_id) + .copied() + .unwrap_or((0, 0)), + }; let facts = (name, modified, diag_errors, diag_warnings, message); - if cached == Some(&facts) { + if self.last_status.get(&buffer_id) == Some(&facts) { return None; } let msg = InstanceMessage::StatusFacts { diff --git a/tests/theme_faces_acceptance.rs b/tests/theme_faces_acceptance.rs index 88ab9bc..4aa70d4 100644 --- a/tests/theme_faces_acceptance.rs +++ b/tests/theme_faces_acceptance.rs @@ -1,6 +1,6 @@ // theme_faces_acceptance.rs --- Themes Arc 4 stage 1 acceptance -// (docs/theme-faces-framing.md, acceptance items 1–19, 24–26, and -// 28–29; the GPU routes — 20–23, 27, and 30 — live in pmacs-gpu's +// (docs/theme-faces-framing.md, acceptance items 1–19, 24–26, 28–29, +// and 32; the GPU routes — 20–23, 27, and 30–31 — live in pmacs-gpu's // headless suite). //! Named UI faces (`ui` / `ui.*` theme entries) + the `ThemeFacts` @@ -1074,6 +1074,78 @@ fn daemon_reships_the_summary_after_a_real_buffer_round_trip() { ); } +// --------------------------------------------------------------------------- +// 32 — the snapshot reset must not break the diagnostic-count freeze +// (PR #120 round 3 finding 2) +// --------------------------------------------------------------------------- + +#[test] +fn snapshot_reset_keeps_frozen_diag_counts_while_the_store_is_stale() { + // `last_status` was both the peer emission baseline AND the + // stale-store freeze source, so the round-2 reset zeroed the + // counts: a buffer switch between didChange and fresh + // diagnostics re-shipped StatusFacts with (0, 0). The freeze + // source now lives apart (`frozen_diag_counts`) and survives + // `on_buffer_snapshot_sent`. + use pmacs::diag::DiagnosticSeverity; + + let mut state = editor(); + type_str(&mut state, "boom\nfine\n"); + let uri = attach_diags( + &state, + vec![ + diag(DiagnosticSeverity::Error), + diag(DiagnosticSeverity::Warning), + ], + ); + + let status_counts = |msgs: &[InstanceMessage]| { + msgs.iter().find_map(|m| match m { + InstanceMessage::StatusFacts { + diag_errors, + diag_warnings, + .. + } => Some((*diag_errors, *diag_warnings)), + _ => None, + }) + }; + + let a = active_buffer(&state); + let mut sem = semantic(&state); + assert_eq!( + status_counts(&sem.render_frame(&state)), + Some((1, 1)), + "fresh counts ship on the first frame" + ); + + // didChange: the store goes stale — counts freeze, never zero. + { + let store = state.lsp_manager.borrow().diag_store(); + store + .lock() + .expect("diag store lock") + .mark_stale(uri.clone()); + } + + // The daemon writes snapshot(A) mid-staleness (a revisit, or the + // F29 upgrade broadcast). The reset kills the emission baseline — + // forcing the re-send — but must not discard the freeze source. + sem.on_buffer_snapshot_sent(a); + sem.set_viewport( + a, + ByteRange { + start: 0, + end: 1 << 20, + }, + 0, + ); + assert_eq!( + status_counts(&sem.render_frame(&state)), + Some((1, 1)), + "the re-sent StatusFacts preserves the frozen counts" + ); +} + // --------------------------------------------------------------------------- // 12 + 16 + 19 — ThemeFacts emission discipline; late join; set wipes // ---------------------------------------------------------------------------