diff --git a/docs/semantic-frontend-protocol.md b/docs/semantic-frontend-protocol.md index 33d65e4..225c933 100644 --- a/docs/semantic-frontend-protocol.md +++ b/docs/semantic-frontend-protocol.md @@ -116,6 +116,18 @@ layout-local frontend consumes). Peer cursors reuse the existing is: one capability bit, ~five instance→frontend interpretation 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. + ## Capability and version mechanics Identical pattern to `crdt_replica`: diff --git a/docs/theme-faces-framing.md b/docs/theme-faces-framing.md index 7674b55..9216ff4 100644 --- a/docs/theme-faces-framing.md +++ b/docs/theme-faces-framing.md @@ -1,7 +1,35 @@ # Theme faces — framing (Arc 4 stage 1, themes) -**Revision 5 — 2026-07-14. Status: implemented on branch -`theme-faces` (PR #120); revision 5 folds PR round 1.** +**Revision 6 — 2026-07-15. Status: implemented on branch +`theme-faces` (PR #120); revision 6 folds PR round 2.** + +Revision 6 (PR #120 round 2, findings 1–2): the producer gains the +**snapshot/baseline reset contract** +(`SemanticRenderState::on_buffer_snapshot_sent`). A `BufferSnapshot` +resets the receiving frontend's buffer-scoped render state wholesale +— spans, decorations, adornments, minimap summary, completion popup — +but the returned viewport only replaced the producer's declared +viewport, leaving every per-buffer emission baseline intact. On an +unchanged A → B → A round trip, `last_summary[A]`'s key still +matched, so the daemon emitted nothing and the frontend never +regained A's themed minimap (or A's `StatusFacts` — the band kept +showing B's name) until an edit, diagnostic republish, or theme +mutation happened to move the key. The daemon now invalidates every +buffer-scoped baseline for the snapshot's buffer wherever it writes +one — the active-buffer-follow path and the F29 upgrade broadcast; +the attach bootstrap needs no call because its session state is +constructed fresh — general contract, not a minimap special case +(finding 1). Deliberately surviving the reset: the bufferless +`ThemeFacts` pair (the frontend keeps its face table across +snapshots), the global minibuffer baseline, the per-frontend gutter +mode, and the revision-keyed diag line cache (a compute cache, not a +peer-state baseline); other buffers' baselines also survive, since +any buffer the frontend navigates to receives its own snapshot +first. Acceptance items 28–30 cover the producer round trip (themed +summary and `StatusFacts` return at one generation), the real-daemon +wiring over the wire, and the GPU minimap's pixel-identical return. +The acceptance-suite manifest header now lists the true item split +(finding 2). Revision 5 (PR #120 round 1, findings 1–4): accepting a `FileStyleSummary` on the GPU now drops the cached minimap vertices — @@ -585,6 +613,24 @@ pub struct ThemeFace { debug-name arm (`main.rs:5606-5628`); `ThemeFace` re-exported from `pmacs-protocol/src/lib.rs`; pin tests at `protocol.rs:1710` and the ladder test (accept 16, reject 17); postcard round-trip pin. +- **Snapshot/baseline reset contract** (round 2 finding 1): a + `BufferSnapshot` resets the receiving frontend's buffer-scoped + render state wholesale, so wherever the daemon writes one it calls + `SemanticRenderState::on_buffer_snapshot_sent(buffer_id)`, killing + every buffer-scoped emission baseline for that buffer — spans + + style gate, decorations, adornments, summary, status, search/menu + prompts, completion popup. Without this, an unchanged-generation + A → B → A round trip suppressed every re-send and the frontend + never regained A's themed minimap or `StatusFacts`. Call sites: + the active-buffer-follow path and the F29 upgrade broadcast; the + attach bootstrap constructs its session state fresh, so its sweep + 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. - `docs/semantic-frontend-protocol.md` gains the channel's contract section. @@ -886,3 +932,22 @@ Keybinding-driven tests dispatch keys, never `pmacs.command.invoke`. (PR round 1 finding 1, `PMACS_REQUIRE_GPU=1`): two summaries at one generation with different mark colors render different frames — fails without the cache invalidation on accept. +28. **Snapshot round trip re-ships buffer-scoped state** (PR round 2 + finding 1): with a themed diag mark on A, driving the producer + through the daemon's exact A → B → A sequence + (`on_buffer_snapshot_sent` + viewport re-declaration at each + switch) re-ships A's `FileStyleSummary` — identical themed + payload, UNCHANGED generation — and A's `StatusFacts`; an + unchanged tick before the trip stays suppressed. A Rust unit + pins the reset's scope: one buffer's baselines die, other + buffers' and the bufferless `ThemeFacts` pair survive. +29. **The daemon wiring, end to end** (PR round 2 finding 1, CRDT): + a real daemon session navigating A → B → A via dispatched keys + (the active-buffer-follow path writes the snapshots) re-ships + `FileStyleSummary` for A at its unchanged generation after the + revisit's viewport re-declaration. +30. **The GPU minimap returns** (PR round 2 finding 1, + `PMACS_REQUIRE_GPU=1`): after a real `BufferSnapshot` A → B → A + 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. diff --git a/pmacs-gpu/src/main.rs b/pmacs-gpu/src/main.rs index cd274ce..1b34fe8 100644 --- a/pmacs-gpu/src/main.rs +++ b/pmacs-gpu/src/main.rs @@ -8710,6 +8710,73 @@ mod tests { ); } + #[test] + fn headless_snapshot_round_trip_summary_restores_the_minimap() { + // PR #120 round 2 finding 1 (frontend half): a `BufferSnapshot` + // drops `current_summary` with the rest of the buffer-scoped + // state, so after an A → B → A round trip no stale minimap + // may survive — and when the daemon's baseline reset re-ships + // the summary at the unchanged generation, the first visit's + // pixels must return exactly. + let text = "alpha\nbeta\ngamma\ndelta\n"; + let (w, h) = (400u32, 300u32); + let Some(mut state) = headless_or_skip(w, h, text) else { + return; + }; + // Both buffers carry the same text so the frames differ by + // the minimap alone. + let snapshot = || -> Vec { + let doc = loro::LoroDoc::new(); + doc.get_text(LORO_TEXT_CONTAINER) + .insert(0, text) + .expect("insert snapshot text"); + doc.export(loro::ExportMode::Snapshot) + .expect("export snapshot") + }; + let bid_a = BufferId::next(); + let bid_b = BufferId::next(); + let visit = |state: &mut State, bid: BufferId| { + let _ = state.apply_attach_message(InstanceMessage::BufferSnapshot { + buffer_id: bid, + crdt_snapshot: snapshot(), + }); + state.view_range = (0, text.len() as u64); + }; + let lines: Vec = (0..4) + .map(|_| CellStyle { + fg: CellColor::Rgb(200, 40, 40), + ..CellStyle::default() + }) + .collect(); + + visit(&mut state, bid_a); + let _ = state.apply_attach_message(InstanceMessage::FileStyleSummary { + buffer_id: bid_a, + generation: 7, + lines: lines.clone(), + }); + let first_visit = state.render_offscreen(); + + visit(&mut state, bid_b); + visit(&mut state, bid_a); + assert_ne!( + first_visit, + state.render_offscreen(), + "the round trip dropped the summary — no stale minimap" + ); + + let _ = state.apply_attach_message(InstanceMessage::FileStyleSummary { + buffer_id: bid_a, + generation: 7, + lines, + }); + assert_eq!( + first_visit, + state.render_offscreen(), + "the re-shipped summary restores the first visit's minimap" + ); + } + #[test] fn own_wash_faces_color_local_rects_peers_keep_the_constant() { // Acceptance 21 + 23: decode the emitted decoration vertex diff --git a/src/daemon.rs b/src/daemon.rs index b84d3fb..8d3d901 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -984,6 +984,7 @@ fn dispatcher_loop( upgraded, &session_registry, &mut streams, + &mut semantic_states, ); // The broadcast just delivered this buffer to this // frontend too; record it so the follow check below @@ -1022,6 +1023,14 @@ fn dispatcher_loop( && last_active_buffer_sent.get(fid) != Some(&active_now) { send_buffer_snapshot_to_frontend(editor, active_now, *fid, &mut streams); + // PR #120 round 2 — the snapshot just wiped + // this frontend's buffer-scoped render state; + // the emission baselines must die with it or + // an unchanged-generation revisit (A → B → A) + // suppresses every re-send. + if let Some(sem) = semantic_states.get_mut(fid) { + sem.on_buffer_snapshot_sent(active_now); + } last_active_buffer_sent.insert(*fid, active_now); } } @@ -1863,6 +1872,7 @@ fn broadcast_buffer_snapshot_to_replicas( buffer_id: crate::buffer::BufferId, session_registry: &SessionRegistry, streams: &mut HashMap, + semantic_states: &mut HashMap, ) { let Some(snapshot_bytes) = export_buffer_snapshot(editor, buffer_id) else { return; @@ -1881,6 +1891,12 @@ fn broadcast_buffer_snapshot_to_replicas( if let Err(e) = write_message(stream, &msg) { eprintln!("pmacs: F29 send BufferSnapshot for {buffer_id:?} to {fid:?} failed: {e}"); } + // PR #120 round 2 — same reset contract as the follow path: + // the snapshot wiped this replica's buffer-scoped render + // state, so its emission baselines for the buffer die too. + if let Some(sem) = semantic_states.get_mut(fid) { + sem.on_buffer_snapshot_sent(buffer_id); + } } } diff --git a/src/semantic_render.rs b/src/semantic_render.rs index 69da64a..55d170a 100644 --- a/src/semantic_render.rs +++ b/src/semantic_render.rs @@ -381,6 +381,44 @@ impl SemanticRenderState { }); } + /// Snapshot/baseline reset contract (PR #120 round 2 finding 1). + /// + /// A `BufferSnapshot` resets the receiving frontend's + /// buffer-scoped render state wholesale — spans, decorations, + /// adornments, minimap summary, completion popup (see the GPU's + /// `BufferSnapshot` arm) — so every buffer-scoped emission + /// baseline this producer holds for that buffer must die with the + /// send. Otherwise an unchanged-key revisit (the A → B → A round + /// trip at one CRDT generation) suppresses every re-send and the + /// frontend never regains the state until an edit, diagnostic + /// republish, or theme mutation happens to move the key. + /// + /// Called by the daemon wherever it writes a `BufferSnapshot` to + /// this session's stream. Resetting when the write later fails is + /// harmless — the failure mode is one redundant re-send, never + /// staleness. + /// + /// Deliberately NOT reset: `last_face_epoch` / `last_theme_faces` + /// (`ThemeFacts` is bufferless — the frontend keeps its face + /// 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. + pub fn on_buffer_snapshot_sent(&mut self, buffer_id: BufferId) { + self.last_sent.remove(&buffer_id); + self.last_style_gate.remove(&buffer_id); + self.last_decorations.remove(&buffer_id); + self.last_adornments.remove(&buffer_id); + self.last_summary.remove(&buffer_id); + self.last_status.remove(&buffer_id); + self.last_search_prompt.remove(&buffer_id); + self.last_menu_prompt.remove(&buffer_id); + self.last_completion_popup.remove(&buffer_id); + } + /// Project one frame. /// /// Returns up to three messages — [`InstanceMessage::StyleSpans`] @@ -2248,6 +2286,63 @@ mod tests { ); } + #[test] + fn snapshot_reset_drops_one_buffers_baselines_and_keeps_the_rest() { + // PR #120 round 2 finding 1 — the reset contract's scope: a + // `BufferSnapshot` for buffer A kills A's buffer-scoped + // emission baselines (here: summary and status, the two the + // A → B → A round trip visibly strands) while OTHER buffers' + // baselines and the bufferless ThemeFacts pair survive. + let state = empty_state(); + let mut s = local(); + let a = active_buffer(&state); + let b = { + let core = state.core.borrow(); + let mut reg = core.registry.borrow_mut(); + reg.create("other") + }; + s.set_viewport(a, ByteRange { start: 0, end: 64 }, 0); + let first = s.render_frame(&state); + assert!( + first + .iter() + .any(|m| matches!(m, InstanceMessage::FileStyleSummary { .. })), + "first frame ships A's summary" + ); + s.set_viewport(b, ByteRange { start: 0, end: 64 }, 0); + let _ = s.render_frame(&state); + assert!(s.last_summary.contains_key(&a)); + assert!(s.last_summary.contains_key(&b)); + assert!(s.last_status.contains_key(&a)); + let facts_baseline = s.last_theme_faces.clone(); + assert!(facts_baseline.is_some(), "first frame shipped ThemeFacts"); + + s.on_buffer_snapshot_sent(a); + + assert!( + !s.last_summary.contains_key(&a) && !s.last_status.contains_key(&a), + "A's baselines die with A's snapshot" + ); + assert!( + s.last_summary.contains_key(&b), + "B's baselines survive A's snapshot" + ); + assert_eq!( + s.last_theme_faces, facts_baseline, + "ThemeFacts is bufferless — the face table survives snapshots" + ); + + // And the behavioral consequence: revisiting A at the SAME + // generation re-ships the summary the frontend just dropped. + s.set_viewport(a, ByteRange { start: 0, end: 64 }, 0); + let back = s.render_frame(&state); + assert!( + back.iter() + .any(|m| matches!(m, InstanceMessage::FileStyleSummary { .. })), + "the unchanged-generation revisit re-ships A's summary" + ); + } + #[test] fn style_gate_differs_when_syntax_epoch_bumps() { // Q#TH6: the gate is a pure function of the theme too — a diff --git a/tests/theme_faces_acceptance.rs b/tests/theme_faces_acceptance.rs index 482257e..88ab9bc 100644 --- a/tests/theme_faces_acceptance.rs +++ b/tests/theme_faces_acceptance.rs @@ -1,6 +1,7 @@ // theme_faces_acceptance.rs --- Themes Arc 4 stage 1 acceptance -// (docs/theme-faces-framing.md, acceptance items 1–19 and 24; the GPU -// routes 20–23 live in pmacs-gpu's headless suite). +// (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 +// headless suite). //! Named UI faces (`ui` / `ui.*` theme entries) + the `ThemeFacts` //! wire channel (protocol v16). @@ -232,6 +233,65 @@ fn attach_diags(state: &EditorState, diags: Vec) -> Str uri } +// --- Daemon wire helpers (item 29; CRDT suites only) ------------------------- + +#[cfg(feature = "crdt")] +fn wire_viewport( + stream: &mut std::os::unix::net::UnixStream, + fid: FrontendId, + buffer_id: pmacs::buffer::BufferId, +) { + pmacs::transport::write_message( + stream, + &pmacs::protocol::FrontendEvent::Viewport { + frontend_id: fid, + buffer_id, + visible: ByteRange { + start: 0, + end: 4096, + }, + generation: 0, + }, + ) + .expect("write Viewport"); +} + +#[cfg(feature = "crdt")] +fn wire_key( + stream: &mut std::os::unix::net::UnixStream, + fid: FrontendId, + key: pmacs::protocol::Key, +) { + pmacs::transport::write_message( + stream, + &pmacs::protocol::FrontendEvent::Key(pmacs::protocol::KeyEvent { + frontend_id: fid, + key, + mods: pmacs::protocol::Modifiers::NONE, + timestamp_ns: 0, + }), + ) + .expect("write Key"); +} + +/// Read wire messages until `pick` returns, or panic at the deadline. +#[cfg(feature = "crdt")] +fn wire_wait_for( + stream: &mut std::os::unix::net::UnixStream, + what: &str, + mut pick: impl FnMut(InstanceMessage) -> Option, +) -> T { + let deadline = Instant::now() + Duration::from_secs(5); + while Instant::now() < deadline { + if let Ok(msg) = pmacs::transport::read_message::(stream) + && let Some(t) = pick(msg) + { + return t; + } + } + panic!("timeout waiting for {what}"); +} + // --------------------------------------------------------------------------- // 1 — unset faces: byte-identical chrome; default_style never leaks // --------------------------------------------------------------------------- @@ -827,6 +887,193 @@ fn v15_peers_get_no_face_derived_summary_marks() { assert!(theme_facts_of(&frame).is_some()); } +// --------------------------------------------------------------------------- +// 28 + 29 — snapshot/baseline reset: the A → B → A round trip +// (PR #120 round 2 finding 1) +// --------------------------------------------------------------------------- + +#[test] +fn snapshot_round_trip_restores_the_themed_summary_at_one_generation() { + // A `BufferSnapshot` wipes the frontend's buffer-scoped render + // state, so the producer's emission baselines must die with the + // send (`on_buffer_snapshot_sent`). Pre-fix, revisiting A at an + // unchanged generation matched `last_summary[A]` and the + // frontend never regained the themed minimap — or A's + // StatusFacts — until an edit, republish, or theme mutation + // happened to move the key. + use pmacs::diag::DiagnosticSeverity; + + let mut state = editor(); + type_str(&mut state, "boom\nfine\n"); + attach_diags(&state, vec![diag(DiagnosticSeverity::Error)]); + exec( + &state, + r#"pmacs.theme.merge { ["ui.diag.error"] = { fg = 45 } }"#, + ); + exec(&state, "_G.__A = pmacs.window.buffer()"); + + let summary_msg = |msgs: &[InstanceMessage]| { + msgs.iter().find_map(|m| match m { + InstanceMessage::FileStyleSummary { + generation, lines, .. + } => Some((*generation, lines.clone())), + _ => None, + }) + }; + let has_status = |msgs: &[InstanceMessage]| { + msgs.iter() + .any(|m| matches!(m, InstanceMessage::StatusFacts { .. })) + }; + let viewport = ByteRange { + start: 0, + end: 1 << 20, + }; + + let a = active_buffer(&state); + let mut sem = semantic(&state); + let first = sem.render_frame(&state); + let (gen_a, themed) = summary_msg(&first).expect("first frame ships the summary"); + assert_eq!( + themed[0].underline_color, + Color::Indexed(45), + "the mark is themed" + ); + assert!(has_status(&first), "first frame ships StatusFacts"); + let quiet = sem.render_frame(&state); + assert!( + summary_msg(&quiet).is_none() && !has_status(&quiet), + "an unchanged tick is suppressed" + ); + + // The daemon switches this session to B: it writes snapshot(B) + // (resetting B's baselines) and the frontend re-declares. + exec( + &state, + "local id = pmacs.instance.show(); pmacs.window.switch_buffer(id)", + ); + let b = active_buffer(&state); + assert_ne!(a, b, "the instance buffer is a distinct buffer"); + sem.on_buffer_snapshot_sent(b); + sem.set_viewport(b, viewport, 0); + let _ = sem.render_frame(&state); + + // ... and back to A. Zero edits: same CRDT generation. + exec(&state, "pmacs.window.switch_buffer(_G.__A)"); + assert_eq!(active_buffer(&state), a); + sem.on_buffer_snapshot_sent(a); + sem.set_viewport(a, viewport, 0); + let back = sem.render_frame(&state); + let (gen_back, lines_back) = summary_msg(&back).expect("the revisit re-ships the summary"); + assert_eq!( + gen_back, gen_a, + "at the SAME generation — no edit forced it" + ); + assert_eq!(lines_back, themed, "the identical themed payload returns"); + assert!(has_status(&back), "StatusFacts returns with it"); +} + +#[cfg(feature = "crdt")] +#[test] +fn daemon_reships_the_summary_after_a_real_buffer_round_trip() { + // Item 29 — the daemon wiring for finding 1: a REAL daemon whose + // session navigates A → B → A (via dispatched keys, so the + // active-buffer-follow path writes the snapshots) must re-ship + // `FileStyleSummary` for A at its unchanged generation. Pre-fix, + // the producer baselines survived the snapshot and the revisit + // was summary-silent forever. + use common::daemon::{TestDaemon, build_default_caps}; + use pmacs::protocol::{AttachRequest, FrontendCapabilities, Hello, Key}; + use pmacs::transport::{read_message, write_message}; + + let daemon = TestDaemon::spawn_with_config( + r#" + pmacs.command.define { + name = "test.go-instance", + description = "themes round 2: switch to the instance buffer", + fn = function() + _G.__orig = pmacs.window.buffer() + local id = pmacs.instance.show() + pmacs.window.switch_buffer(id) + end, + } + pmacs.command.define { + name = "test.go-back", + description = "themes round 2: switch back to the original buffer", + fn = function() pmacs.window.switch_buffer(_G.__orig) end, + } + pmacs.keymap.bind { scope = "global", sequence = "", command = "test.go-instance" } + pmacs.keymap.bind { scope = "global", sequence = "", command = "test.go-back" } + "#, + ); + + let mut stream = daemon.connect(); + stream + .set_read_timeout(Some(Duration::from_millis(250))) + .unwrap(); + let hello: Hello = read_message(&mut stream).expect("read Hello"); + let fid = hello.assigned_frontend_id; + write_message( + &mut stream, + &AttachRequest { + protocol_version: 16, + frontend_capabilities: FrontendCapabilities { + multi_frontend: true, + crdt_replica: true, + semantic_render: true, + ..build_default_caps() + }, + initial_size: CellSize::new(24, 80), + }, + ) + .expect("write AttachRequest"); + + // Learn buffer A from the bootstrap snapshot, declare, and note + // the first summary's generation. + let a = wire_wait_for(&mut stream, "bootstrap BufferSnapshot", |m| match m { + InstanceMessage::BufferSnapshot { buffer_id, .. } => Some(buffer_id), + _ => None, + }); + wire_viewport(&mut stream, fid, a); + let gen_a = wire_wait_for(&mut stream, "first FileStyleSummary(A)", |m| match m { + InstanceMessage::FileStyleSummary { + buffer_id, + generation, + .. + } if buffer_id == a => Some(generation), + _ => None, + }); + + // A → B: the follow path writes snapshot(B); re-declare for B. + wire_key(&mut stream, fid, Key::F(6)); + let b = wire_wait_for(&mut stream, "BufferSnapshot(B)", |m| match m { + InstanceMessage::BufferSnapshot { buffer_id, .. } if buffer_id != a => Some(buffer_id), + _ => None, + }); + wire_viewport(&mut stream, fid, b); + + // B → A: the follow path writes snapshot(A) — the reset under + // test — and the revisit must re-ship A's summary, unchanged + // generation included. + wire_key(&mut stream, fid, Key::F(7)); + wire_wait_for(&mut stream, "BufferSnapshot(A) on revisit", |m| match m { + InstanceMessage::BufferSnapshot { buffer_id, .. } if buffer_id == a => Some(()), + _ => None, + }); + wire_viewport(&mut stream, fid, a); + let gen_back = wire_wait_for(&mut stream, "re-shipped FileStyleSummary(A)", |m| match m { + InstanceMessage::FileStyleSummary { + buffer_id, + generation, + .. + } if buffer_id == a => Some(generation), + _ => None, + }); + assert_eq!( + gen_back, gen_a, + "the revisit summary arrives at A's unchanged generation" + ); +} + // --------------------------------------------------------------------------- // 12 + 16 + 19 — ThemeFacts emission discipline; late join; set wipes // ---------------------------------------------------------------------------