From ccdf352258ad959cf88a1bba0fc2fbd5816a74d6 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sun, 26 Jul 2026 10:38:21 -0400 Subject: [PATCH] fix(panel): close Stage 2A review round 2 (2 P1, 1 P2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **P1-1 — the `Invalidated` arm published the panel context on the document wire.** Real bug, and the live half of the routing defect: the semantic peer has ONE statusline slot, so emitting an authoritative-empty payload for every context replaced the document's with the panel's. Now filtered by document-window identity exactly like the `Ready` arm; a panel's own clear belongs to `PanelFrame` in 2B. Pinned by `invalidated_statusline_clears_only_the_document_not_the_panel`, which reproduces the reported shape — two targets instead of one — when the filter is removed. Honest note on the `Ready` arm: its identity selector is **defensive**, not independently falsifiable today, because the document context is captured first so "first context for my frontend" happens to pick it. Rather than leave that as a silent dependency, `the_semantic_fan_out_captures_the_document_first` pins the order and says why it matters. **P1-2 — round-1 finding 3 was not closed; four of my pins were vacuous.** All four confirmed and fixed: - The statusline consumer test discarded `render_frame`'s output. It now observes the WIRE payload from a v18 peer with a registered provider, and asserts non-emptiness so it cannot pass by emitting nothing. - The terminal test compared two NON-terminal buffers, so both routings answered `false`. The document window now holds a REAL terminal, so the routes disagree; reverting `semantic_terminal_key` fails it. - The decorations test used different buffers and an empty selection — again the same answer either way. The panel now displays the declared buffer with a non-empty selection while the document has none. - #1/#3/#21 had no discriminating pin at all. Their only production caller is `dispatcher_loop`, which no test can drive, so this extracts three named seams the loop calls — `document_buffer_to_follow`, `document_cursor_byte`, `peer_displays_buffer_as_document` — and pins each. Also newly pinned: #2 the lazy CRDT upgrade (the census's sharpest case), #7 `Viewport` aligning WITHOUT taking focus, and #9 a focused terminal panel not suppressing the document viewport. **Every one of the nine pins was falsified by revert.** Two needed a second attempt after the first bite came back green. **P2-3 — stale docs.** `StatuslineEvaluationTarget::Semantic`'s documentation described evaluating only the focused window; it now describes the document-plus-side fan-out, the capture order, the identity-selection requirement, and that `active` reports actual focus. The ledger's Stage 2A entry is corrected to five commits, 2,014 CRDT tests, and 16 acceptance tests. Two clippy findings the refactor introduced were fixed: `document_buffer_to_follow` is `crdt`-gated to match its only caller, and the `CursorByte` guard collapses into one `if`. Gates: fmt clean; workspace clippy clean; 1,832 default + 2,014 CRDT library; Stage 2A 16; Stage 1 46; statusline 8; m11_5 2; GPU initial target 14; terminal config 12; folding Stage 2 48; vterm 1/2 10 / 6; M4 121; required GPU 202; `git diff --check` clean. Co-Authored-By: Claude Opus 5 (1M context) --- docs/active-work.md | 43 +-- src/daemon.rs | 343 ++++++++++++++++++++--- src/semantic_render.rs | 26 +- src/statusline.rs | 21 +- tests/bottom_panel_stage2a_acceptance.rs | 259 +++++++++++++++-- 5 files changed, 602 insertions(+), 90 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index 98727e5..36a99de 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -492,27 +492,36 @@ implemented and in review.** worktree `../pmacs-bp-stage2a`, **canonical `main` @ `cf54270` integrated** (review round 1, finding 4 — the terminal-config lane #173 also changes `src/editor.rs`, so gates were rerun on the merge - result, not the old combination). Two commits: the classified census routing, then the - painter extraction + acceptance. **No protocol change; no behavior + result, not the old combination). Five commits: the classified census + routing, the painter extraction + acceptance, the lane record, then + the round-1 and round-2 review fixes. **No protocol change; no behavior change for any frontend today** — with `panel_capable = false` for semantic sessions, `primary_document_window` returns `view.active` in every existing configuration, so this is seam adoption that becomes load-bearing in 2B. -- Verification on this branch: `cargo fmt --check` clean; strict - workspace Clippy clean; **1,832 default + 2,009 CRDT** library tests; - new `bottom_panel_stage2a_acceptance` 10/10; bottom-panel Stage 1 - 46; statusline segments 7 default / 8 CRDT; m11_5 semantic 2 CRDT; - GPU initial target 14 CRDT; vterm Stage 1/2 10 / 6; folding Stage 2 - 48; M4 121; required GPU 202; `git diff --check` clean. -- **Both key routings were falsified by revert.** Rerouting - `dispatch_idle_for` (#14, Focus) through `primary_document_window` - fails `focus_class_dispatch_idle_still_tracks_the_focused_window`; - reverting the statusline lookup (#12, Projection) to `view.active` - fails the document-context test. Worth recording: the *structural* - test `focus_and_projection_disagree_in_the_same_state` did **not** - catch the first bite — it compares the two authorities directly, so - only a consumer-level assertion catches a misrouted consumer. Keep - both kinds. +- Verification on the merge result: `cargo fmt --check` clean; strict + workspace Clippy clean; **1,832 default + 2,014 CRDT** library tests; + `bottom_panel_stage2a_acceptance` **16**; bottom-panel Stage 1 46; + statusline segments 8 CRDT; m11_5 semantic 2 CRDT; GPU initial target + 14 CRDT; terminal config 12 CRDT; vterm Stage 1/2 10 / 6; folding + Stage 2 48; M4 121; required GPU 202; `git diff --check` clean. +- **Every routed producer is now pinned at a seam its production caller + uses, and each pin was falsified by revert**: #1 follow, #2 lazy CRDT + upgrade, #3 `CursorByte`, #5 decorations, #7 `Viewport` (aligns + without focusing), #8 `Pointer` (aligns and focuses), #9 the + terminal-context gate, #12 statusline, #21 the publication filter, + plus the focus-class negatives. #1/#3/#21 required extracting three + named helpers, because their only production caller is + `dispatcher_loop`, which no test can drive. +- **Three lessons about the TESTS, not the code, all from review:** + (a) a *structural* test comparing the two authorities directly does + **not** catch a misrouted consumer — only consumer-level assertions + do; (b) a daemon-path test must `register_session` or the event is + dropped at the uninstalled-session check before reaching the code + under test; (c) a discriminating fixture must make the two routings + DISAGREE — comparing two non-terminal buffers, or two windows with no + selection, yields the same answer either way and proves nothing. + Round 2 found four of my own pins vacuous by exactly these shapes. - **Review round 1 closed: 4 P1 + 2 P2, all real.** The P1s were a stale-`Pointer` focus steal (the failed-alignment arm returned the window, so #8's activation focused it before `dispatch_pointer` diff --git a/src/daemon.rs b/src/daemon.rs index 606839d..63f6b87 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -1145,14 +1145,7 @@ fn dispatcher_loop( .session_state(*fid) .is_some_and(|s| s.negotiated_capabilities.semantic_render) { - // Bottom-panel §1.3 #1 — Projection. The buffer this - // frontend DISPLAYS AS ITS DOCUMENT, not the one it - // happens to focus: focusing a panel must re-send no - // snapshot and must never swap the replica's mirror. - let active_now = { - let core = editor.core.borrow(); - core.primary_document_buffer(*fid) - }; + let active_now = document_buffer_to_follow(editor, *fid); if let Some(active_now) = active_now && last_active_buffer_sent.get(fid) != Some(&active_now) { @@ -1433,25 +1426,15 @@ fn dispatcher_loop( && session_registry .session_state(*fid) .is_some_and(|s| s.negotiated_capabilities.crdt_replica) + && let Some((buffer_id, byte_pos)) = document_cursor_byte(editor, *fid) { - // Bottom-panel §1.3 #3 — Projection. `CursorByte` is - // the replica's authoritative DOCUMENT cursor; a - // focused panel must not retarget it at the panel - // buffer (Q#BP14's "active buffer is a - // document-surface term, not an input-focus term"). - let core = editor.core.borrow(); - if let Some(window) = core - .primary_document_window(*fid) - .and_then(|win_id| core.windows.get(&win_id)) - { - let cursor_byte_msg = InstanceMessage::CursorByte { - buffer_id: window.buffer_id, - byte_pos: window.cursor, - }; - if let Err(e) = write_message(stream, &cursor_byte_msg) { - eprintln!("pmacs: write CursorByte for {fid:?} failed: {e}"); - write_failed = true; - } + let cursor_byte_msg = InstanceMessage::CursorByte { + buffer_id, + byte_pos, + }; + if let Err(e) = write_message(stream, &cursor_byte_msg) { + eprintln!("pmacs: write CursorByte for {fid:?} failed: {e}"); + write_failed = true; } } } @@ -2524,13 +2507,7 @@ fn publish_buffer_snapshot_to_replicas( continue; } if session.negotiated_capabilities.semantic_render { - // Bottom-panel §1.3 #21 — Projection. "Displays this - // buffer" means the peer's DOCUMENT surface: testing the - // focused window would both miss a buffer visible in the - // document (panel focused elsewhere) and replace the peer's - // mirror for one visible only in a panel. - let displays_buffer = - editor.core.borrow().primary_document_buffer(*peer_id) == Some(buffer_id); + let displays_buffer = peer_displays_buffer_as_document(editor, *peer_id, buffer_id); if !displays_buffer { continue; } @@ -2969,6 +2946,53 @@ fn handle_remote_crdt_op( /// whole switch. This is the input/display alignment fix for B1: the /// frontend's *declared* buffer becomes the buffer its keys edit and /// its `CursorByte` reports. +/// The buffer a semantic frontend DISPLAYS AS ITS DOCUMENT — the +/// buffer-follow / `BufferSnapshot` re-send target (bottom-panel §1.3 +/// #1, Projection). +/// +/// Not the focused buffer: focusing a panel must re-send no snapshot and +/// must never swap the replica's document mirror. Named as its own +/// function so the rule is pinnable — its only caller is +/// `dispatcher_loop`, which no test can drive. +#[cfg(feature = "crdt")] +fn document_buffer_to_follow( + editor: &EditorState, + fid: FrontendId, +) -> Option { + editor.core.borrow().primary_document_buffer(fid) +} + +/// The `(buffer, byte)` a semantic replica's authoritative `CursorByte` +/// describes (bottom-panel §1.3 #3, Projection). +/// +/// Q#BP14's vocabulary split: "active buffer" in the replica is a +/// DOCUMENT-SURFACE term, not an input-focus term, so a focused panel +/// must not retarget the document caret at the panel's buffer. +fn document_cursor_byte( + editor: &EditorState, + fid: FrontendId, +) -> Option<(crate::buffer::BufferId, u64)> { + let core = editor.core.borrow(); + let win_id = core.primary_document_window(fid)?; + let window = core.windows.get(&win_id)?; + Some((window.buffer_id, window.cursor)) +} + +/// Whether `peer_id` displays `buffer_id` on its DOCUMENT surface — the +/// `BufferSnapshot` publication recipient filter (bottom-panel §1.3 #21, +/// Projection). +/// +/// Testing the focused window instead would both miss a buffer visible +/// in the document while a panel holds focus, and replace the peer's +/// document mirror for a buffer visible only in a panel. +fn peer_displays_buffer_as_document( + editor: &EditorState, + peer_id: FrontendId, + buffer_id: crate::buffer::BufferId, +) -> bool { + editor.core.borrow().primary_document_buffer(peer_id) == Some(buffer_id) +} + /// Align a semantic frontend's **primary document window** to the /// buffer it declared (bottom-panel §1.3 #7, Q#BP14). /// @@ -4770,4 +4794,257 @@ mod tests { "non-vacuity: the document window is a real, distinct focus target" ); } + + /// Bottom-panel §1.3 #1/#3/#21 — the three Projection producers whose + /// only production caller is `dispatcher_loop`, pinned at the named + /// seams that loop calls. Round 2 finding: reverting any of them to + /// `active_window_for` previously left every test green. + #[cfg(feature = "crdt")] + #[test] + fn tick_producers_describe_the_document_while_a_panel_is_focused() { + let (editor, fid, document, panel) = panel_focused_semantic_fixture(); + let (doc_buf, panel_buf, doc_cursor) = { + let core = editor.core.borrow(); + ( + core.windows[&document].buffer_id, + core.windows[&panel].buffer_id, + core.windows[&document].cursor, + ) + }; + assert_ne!(doc_buf, panel_buf, "fixture: distinct buffers"); + + // #1 buffer-follow / BufferSnapshot re-send target. + assert_eq!( + document_buffer_to_follow(&editor, fid), + Some(doc_buf), + "#1: the follow target must be the DOCUMENT buffer, not the focused panel's" + ); + + // #3 CursorByte. + assert_eq!( + document_cursor_byte(&editor, fid), + Some((doc_buf, doc_cursor)), + "#3: CursorByte must describe the DOCUMENT surface" + ); + + // #21 publication recipient filter, both directions. + assert!( + peer_displays_buffer_as_document(&editor, fid, doc_buf), + "#21: a buffer visible in the document must still receive publications while a panel holds focus" + ); + assert!( + !peer_displays_buffer_as_document(&editor, fid, panel_buf), + "#21: a buffer visible only in a panel must NOT replace the document mirror" + ); + } + + /// Bottom-panel §1.3 #2 — the sharpest census case: the lazy CRDT + /// upgrade BROADCASTS a snapshot, so keying it on focus would let + /// focusing a fresh generated panel buffer swap every peer's mirror. + #[cfg(feature = "crdt")] + #[test] + fn lazy_crdt_upgrade_never_targets_a_focused_panel_buffer() { + let (editor, fid, document, panel) = panel_focused_semantic_fixture(); + let (doc_buf, panel_buf) = { + let core = editor.core.borrow(); + ( + core.windows[&document].buffer_id, + core.windows[&panel].buffer_id, + ) + }; + + let upgraded = ensure_active_buffer_crdt_backed(&editor, fid); + assert_eq!( + upgraded, + Some(doc_buf), + "#2: the upgrade must target the DOCUMENT buffer" + ); + assert_ne!( + upgraded, + Some(panel_buf), + "#2: focusing a panel must never trigger its buffer's upgrade+broadcast" + ); + } + + /// Bottom-panel §1.3 #7 vs #8 — `Viewport` aligns WITHOUT moving + /// focus; only `Pointer` activates. Driven through the real + /// dispatcher seam. + #[cfg(feature = "crdt")] + #[test] + fn viewport_aligns_the_document_without_taking_focus_from_the_panel() { + let (mut editor, fid, document, panel) = panel_focused_semantic_fixture(); + let other = { + let mut core = editor.core.borrow_mut(); + core.registry.borrow_mut().create("*other*") + }; + + dispatch_one_semantic_event( + &mut editor, + fid, + FrontendEvent::Viewport { + frontend_id: fid, + buffer_id: other, + visible: pmacs_protocol::ByteRange { start: 0, end: 0 }, + generation: 0, + }, + ); + + assert_eq!( + editor.core.borrow().views[&fid].active, + panel, + "#7: a document Viewport must NOT move focus out of the panel" + ); + assert_eq!( + editor.core.borrow().windows[&document].buffer_id, + other, + "#7: it must still have ALIGNED the document window to the declared buffer" + ); + } + + /// Shared fixture: a semantic frontend with a document window and a + /// FOCUSED bottom panel. `panel_capable` is set explicitly because + /// Stage 1 ships `false` for semantic sessions and 2B flips it for a + /// v21-negotiated peer. + #[cfg(feature = "crdt")] + fn panel_focused_semantic_fixture() -> ( + crate::editor::EditorState, + FrontendId, + crate::window::WindowId, + crate::window::WindowId, + ) { + use crate::window::{FrontendView, Layout, LayoutNode, Orientation, Window, WindowParams}; + + let editor = crate::editor::EditorState::new(); + let fid = FrontendId(91); + let (document, panel) = { + let mut core = editor.core.borrow_mut(); + let doc_buf = core.active_window().buffer_id; + let panel_buf = core.registry.borrow_mut().create("*panel*"); + let document = crate::window::WindowId::next(); + let panel = crate::window::WindowId::next(); + let (doc_view, panel_view) = { + let reg = core.registry.borrow(); + ( + crate::text_view::TextView::new(reg.get(doc_buf).expect("doc")), + crate::text_view::TextView::new(reg.get(panel_buf).expect("panel")), + ) + }; + core.windows + .insert(document, Window::new(document, doc_buf, doc_view)); + let mut panel_window = Window::new(panel, panel_buf, panel_view); + let mut params = WindowParams::default(); + params.side = Some(crate::window::Side::Bottom); + params.fixed_rows = Some(4); + panel_window.params = params; + core.windows.insert(panel, panel_window); + core.register_frontend_view( + fid, + FrontendView { + layout: Layout { + root: LayoutNode::Split { + orientation: Orientation::Horizontal, + children: vec![LayoutNode::Leaf(document), LayoutNode::Leaf(panel)], + weights: vec![1, 1], + }, + }, + active: panel, + fold_projection: false, + panel_capable: true, + frame_geometry: None, + panel_hidden: false, + }, + ); + (document, panel) + }; + editor.sync_frame_geometry(fid, CellSize::new(24, 80)); + (editor, fid, document, panel) + } + + /// Drive ONE authenticated semantic event through the real + /// dispatcher. The session must be registered or the event is + /// dropped at the uninstalled-session check before reaching any + /// handler. + #[cfg(feature = "crdt")] + fn dispatch_one_semantic_event( + editor: &mut crate::editor::EditorState, + fid: FrontendId, + event: FrontendEvent, + ) { + let mut render_states = HashMap::new(); + let mut semantic_states = HashMap::new(); + semantic_states.insert(fid, crate::semantic_render::SemanticRenderState::new(fid)); + let mut streams = HashMap::new(); + let mut term_sizes = HashMap::new(); + term_sizes.insert(fid, CellSize::new(24, 80)); + let mut last_idle = HashMap::new(); + let mut last_active = HashMap::new(); + let mut bells = HashMap::new(); + let mut registry = SessionRegistry::new(); + registry.register_session( + fid, + crate::presence::SessionState { + negotiated_protocol_version: pmacs_protocol::PROTOCOL_VERSION, + negotiated_capabilities: crate::protocol::NegotiatedCapabilities { + semantic_render: true, + crdt_replica: true, + ..Default::default() + }, + color_slot: 0, + }, + ); + handle_dispatcher_event( + DispatcherEvent::FrontendEvent { source: fid, event }, + editor, + &mut render_states, + &mut semantic_states, + &mut streams, + &mut term_sizes, + &mut last_idle, + &mut last_active, + &mut bells, + &mut registry, + ); + } + + /// Bottom-panel §1.3 #9 — Projection. The `Viewport` terminal-context + /// gate asks "is this frontend's DOCUMENT surface a terminal", so a + /// focused TERMINAL PANEL must not suppress the still-visible + /// document's viewport. + #[cfg(feature = "crdt")] + #[test] + fn a_focused_terminal_panel_does_not_suppress_the_document_viewport() { + use crate::terminal::TerminalSpec; + + let (mut editor, fid, document, panel) = panel_focused_semantic_fixture(); + let other = editor.core.borrow().registry.borrow_mut().create("*other*"); + + // A REAL terminal in the focused panel. + let mut spec = TerminalSpec::new("/bin/sh"); + spec.rows = 10; + spec.cols = 40; + let term_buf = editor.open_terminal(spec).expect("a real terminal"); + editor + .core + .borrow_mut() + .install_buffer_in_window(panel, term_buf) + .expect("terminal into the panel"); + editor.core.borrow_mut().focus_window(fid, panel); + + dispatch_one_semantic_event( + &mut editor, + fid, + FrontendEvent::Viewport { + frontend_id: fid, + buffer_id: other, + visible: pmacs_protocol::ByteRange { start: 0, end: 0 }, + generation: 0, + }, + ); + + assert_eq!( + editor.core.borrow().windows[&document].buffer_id, + other, + "#9: a focused TERMINAL panel must not suppress the document viewport — the document window should still have aligned to the declared buffer" + ); + } } diff --git a/src/semantic_render.rs b/src/semantic_render.rs index b63efcf..af08741 100644 --- a/src/semantic_render.rs +++ b/src/semantic_render.rs @@ -985,10 +985,18 @@ impl SemanticRenderState { StatuslineEvaluationOutcome::Invalidated { authoritative_empty, } => { - for context in authoritative_empty - .into_iter() - .filter(|context| context.frontend_id == frontend_id) - { + // Bottom-panel A2A-2: the clear must be filtered by + // DOCUMENT WINDOW exactly like the Ready arm. The + // semantic peer has ONE statusline slot, so publishing + // the panel context's clear here replaces the document's + // payload with the panel's — the same misrouting the + // Ready arm was fixed for, on the clear path. + // + // A panel's own clear belongs to the future panel + // painter (`PanelFrame`, Stage 2B), not to this wire. + for context in authoritative_empty.into_iter().filter(|context| { + context.frontend_id == frontend_id && Some(context.window_id) == document_window + }) { self.emit_statusline_payload(context.buffer_id, Vec::new(), Vec::new(), out); } } @@ -2949,11 +2957,15 @@ mod tests { "stale evaluation retains the prior baseline until snapshot reset" ); + // Bottom-panel A2A-2: the clear is filtered by DOCUMENT window + // identity, so the context under test must BE the document + // window — passing `None` here would assert nothing. + let document_window = crate::window::WindowId::next(); let invalidated = || StatuslineEvaluation { outcome: StatuslineEvaluationOutcome::Invalidated { authoritative_empty: vec![crate::statusline::StatuslineContext { frontend_id: FrontendId::LOCAL, - window_id: crate::window::WindowId::next(), + window_id: document_window, buffer_id, active: true, }], @@ -2961,13 +2973,13 @@ mod tests { new_failures: Vec::new(), }; let mut replacement = Vec::new(); - semantic.emit_statusline_segments(invalidated(), None, &mut replacement); + semantic.emit_statusline_segments(invalidated(), Some(document_window), &mut replacement); assert_eq!( statusline_of(&replacement), Some((buffer_id, Vec::new(), Vec::new())) ); let mut unchanged = Vec::new(); - semantic.emit_statusline_segments(invalidated(), None, &mut unchanged); + semantic.emit_statusline_segments(invalidated(), Some(document_window), &mut unchanged); assert!( unchanged.is_empty(), "the empty invalidation became baseline" diff --git a/src/statusline.rs b/src/statusline.rs index 7bf345f..3eb4ef6 100644 --- a/src/statusline.rs +++ b/src/statusline.rs @@ -215,10 +215,25 @@ pub enum StatuslineEvaluationTarget { /// Frontend whose entire visible layout is evaluated. frontend_id: FrontendId, }, - /// Only the frontend's active window, iff it still displays the declared - /// semantic viewport buffer. + /// The frontend's **primary document window**, iff it still displays + /// the declared semantic viewport buffer, **plus its visible side + /// window** when one exists (bottom-panel Q#BP8 / A2A-2). + /// + /// Two contexts, not one: the document result feeds the semantic + /// `StatuslineSegments` wire, while the side result paints in the + /// panel's own mode line. Unprojected document splits run no + /// callbacks, and a derived-hidden side (Q#BP2b) is omitted because + /// it has no mode line to paint this frame. + /// + /// The document context is captured **first**; consumers must still + /// select by window identity rather than position, since only one of + /// the two may reach the single semantic statusline slot. + /// + /// `active` on each context reports **actual focus**, so a document + /// provider truthfully observes `active = false` while a panel owns + /// focus (Q#BP14, parent acceptance 42). Semantic { - /// Frontend whose focused daemon window is evaluated. + /// Frontend whose document (and visible side) window is evaluated. frontend_id: FrontendId, /// Buffer declared by the semantic viewport. declared_buffer: BufferId, diff --git a/tests/bottom_panel_stage2a_acceptance.rs b/tests/bottom_panel_stage2a_acceptance.rs index 3b6c183..1063e9f 100644 --- a/tests/bottom_panel_stage2a_acceptance.rs +++ b/tests/bottom_panel_stage2a_acceptance.rs @@ -32,6 +32,14 @@ fn exec(s: &EditorState, src: &str) { s.lua_host.lua().load(src.to_string()).exec().unwrap(); } +fn side_window_of(core: &pmacs::editor_core::EditorCore, fid: FrontendId) -> Option { + core.views[&fid].layout.iter_ids().into_iter().find(|id| { + core.windows + .get(id) + .is_some_and(|w| w.params.side.is_some()) + }) +} + fn side_window(s: &EditorState) -> Option { let core = s.core.borrow(); core.views[&FrontendId::LOCAL] @@ -540,48 +548,239 @@ fn consumer_line_numbers_follow_the_document_not_the_focused_panel() { } #[test] -fn consumer_statusline_segments_name_the_document_window() { - use pmacs::protocol::ByteRange; +fn consumer_statusline_segments_carry_the_document_payload_not_the_panel() { + use pmacs::protocol::{ByteRange, InstanceMessage}; use pmacs::semantic_render::SemanticRenderState; - // §1.3 #12 at the producer: the wire segments must be selected by - // the DOCUMENT window even though the fan-out now also evaluates the - // visible side window (A2A-2). + // §1.3 #12 / A2A-2 at the WIRE. Round 2 finding: the previous + // version discarded `render_frame`'s output and only reasserted + // `primary_document_window`, so restoring the producer's + // "first context for my frontend" selector left it green. + // + // The peer must negotiate v18 or no `StatuslineSegments` is emitted + // at all and the assertion would be vacuous a second way. let s = editor(); - let (fid, doc_win, _panel_win, doc_buf) = semantic_frontend_with_focused_panel(&s); + let (fid, _doc_win, _panel_win, doc_buf) = semantic_frontend_with_focused_panel(&s); + let panel_buf = { + let core = s.core.borrow(); + let panel = side_window_of(&core, fid).expect("panel"); + core.windows[&panel].buffer_id + }; - let mut sem = SemanticRenderState::new(fid); + // One provider so a payload exists to misroute. + exec( + &s, + "pmacs.statusline.register({ name = \"probe\", side = \"left\", + face = \"ui.modeline\", fn = function(ctx) return \"X\" end })", + ); + + let mut sem = SemanticRenderState::for_peer(fid, 18); sem.set_viewport(doc_buf, ByteRange { start: 0, end: 0 }, 0); - // Not asserting on message presence (a peer that never negotiated - // v18 emits none); asserting the routing input the producer uses. - let _ = sem.render_frame(&s); + let msgs = sem.render_frame(&s); - assert_eq!( - s.core.borrow().primary_document_window(fid), - Some(doc_win), - "the producer's document-window selector must name the document" + let targets: Vec<_> = msgs + .iter() + .filter_map(|m| match m { + InstanceMessage::StatuslineSegments { buffer_id, .. } => Some(*buffer_id), + _ => None, + }) + .collect(); + + assert!( + !targets.is_empty(), + "non-vacuity: a v18 peer with a registered provider must emit StatuslineSegments" + ); + assert!( + targets.iter().all(|b| *b == doc_buf), + "every StatuslineSegments must target the DOCUMENT buffer; got {targets:?} (document {doc_buf:?}, panel {panel_buf:?})" + ); + assert!( + !targets.contains(&panel_buf), + "the panel's context must never reach the document statusline wire" ); } #[test] -fn consumer_terminal_declaration_cannot_be_claimed_by_a_focused_panel() { - // §1.3 #6/#10/#11 through the real guard: with the panel focused, - // a declaration naming the PANEL's buffer must be refused, because - // the full-window terminal surface is the document window. - let s = editor(); - let (fid, _doc_win, panel_win, doc_buf) = semantic_frontend_with_focused_panel(&s); - let panel_buf = s.core.borrow().windows[&panel_win].buffer_id; +fn consumer_terminal_declaration_resolves_the_document_not_the_focused_panel() { + use pmacs::terminal::TerminalSpec; + + // §1.3 #6/#10/#11 through the real guard. Round 2 finding: the + // previous version compared two NON-terminal buffers, so both the + // old and new routings returned `false` and it could not + // discriminate. Make the DOCUMENT window hold a real terminal: the + // document routing then answers `true` while the old `view.active` + // routing (which names the focused panel) answers `false`. + let mut s = editor(); + let (fid, doc_win, _panel_win, _doc_buf) = semantic_frontend_with_focused_panel(&s); + + let mut spec = TerminalSpec::new("/bin/sh"); + spec.rows = 10; + spec.cols = 40; + let term_buf = s.open_terminal(spec).expect("a real terminal session"); + + // Install the terminal in the DOCUMENT window; the panel keeps its + // own non-terminal buffer and keeps focus. + { + let mut core = s.core.borrow_mut(); + core.install_buffer_in_window(doc_win, term_buf) + .expect("install the terminal in the document window"); + } + let panel_buf = { + let core = s.core.borrow(); + let panel = side_window_of(&core, fid).expect("panel"); + core.windows[&panel].buffer_id + }; assert!( - !s.semantic_terminal_declaration_is_active(fid, panel_buf), - "a focused panel's buffer must not become the document terminal declaration" + s.semantic_terminal_declaration_is_active(fid, term_buf), + "the DOCUMENT window's terminal must be declarable while the panel owns focus" ); - // Non-vacuity: the document buffer is not a terminal either, so pin - // that the guard resolves the DOCUMENT window by asserting the - // window identity the resolver used. - assert_eq!( - s.core.borrow().primary_document_buffer(fid), - Some(doc_buf), - "the terminal resolver's window must be the document window" + assert!( + !s.semantic_terminal_declaration_is_active(fid, panel_buf), + "the focused panel's own buffer must never claim the document declaration" + ); +} + +#[test] +fn invalidated_statusline_clears_only_the_document_not_the_panel() { + use pmacs::protocol::{ByteRange, InstanceMessage}; + use pmacs::semantic_render::SemanticRenderState; + + // Round 2 finding 1. The `Invalidated` arm emits an + // authoritative-empty payload for EVERY context of the frontend. + // Once A2A-2's fan-out yields document + panel, that publishes two + // clears on a wire with ONE statusline slot, so the panel's payload + // replaces the document's. This is the live, observable half of the + // routing bug — the `Ready` arm happens to be safe today only + // because the document context is captured first. + let s = editor(); + let (fid, _doc_win, _panel_win, doc_buf) = semantic_frontend_with_focused_panel(&s); + let panel_buf = { + let core = s.core.borrow(); + let panel = side_window_of(&core, fid).expect("panel"); + core.windows[&panel].buffer_id + }; + assert_ne!(doc_buf, panel_buf, "fixture: the two buffers must differ"); + + // A provider that unregisters itself mid-evaluation is the canonical + // registry-mutation invalidation. + exec( + &s, + r"_G.SL_SELF = pmacs.statusline.register { + name='self-remove', side='left', priority=100, + fn=function() pmacs.statusline.unregister(SL_SELF); return 'STALE' end, + }", + ); + + let mut sem = SemanticRenderState::for_peer(fid, 18); + sem.set_viewport(doc_buf, ByteRange { start: 0, end: 0 }, 0); + let msgs = sem.render_frame(&s); + + let targets: Vec<_> = msgs + .iter() + .filter_map(|m| match m { + InstanceMessage::StatuslineSegments { buffer_id, .. } => Some(*buffer_id), + _ => None, + }) + .collect(); + + assert!( + !targets.contains(&panel_buf), + "an invalidated evaluation must not clear the PANEL's context on the \ + document statusline wire; got {targets:?} (document {doc_buf:?}, \ + panel {panel_buf:?})" + ); +} + +#[test] +fn the_semantic_fan_out_captures_the_document_first() { + use pmacs::statusline::{ + StatuslineEvaluationOutcome, StatuslineEvaluationTarget, evaluate_statusline, + }; + + // The `Ready` arm selects by window identity, so capture order is not + // load-bearing for correctness — but it IS load-bearing for the + // falsifiability of that selector, so pin it explicitly rather than + // leaving a silent dependency. If a future change reorders the + // fan-out, this fails and whoever reads it learns why it mattered. + let s = editor(); + let (fid, doc_win, _panel_win, doc_buf) = semantic_frontend_with_focused_panel(&s); + + let evaluation = evaluate_statusline( + s.lua_host.lua(), + &s.core, + &s.statusline_registry, + StatuslineEvaluationTarget::Semantic { + frontend_id: fid, + declared_buffer: doc_buf, + }, + ); + + match evaluation.outcome { + StatuslineEvaluationOutcome::Ready(windows) => { + assert_eq!(windows.len(), 2, "document + visible side window"); + assert_eq!( + windows[0].context.window_id, doc_win, + "the DOCUMENT context must be captured first" + ); + } + other => panic!("expected Ready, got {other:?}"), + } +} + +#[test] +fn consumer_decorations_follow_the_document_selection_not_the_panel() { + use pmacs::protocol::{ByteRange, InstanceMessage}; + use pmacs::semantic_render::SemanticRenderState; + + // §1.3 #5 — Projection. A selection made inside a FOCUSED PANEL must + // not paint selection decorations into the document's viewport. + // + // To DISCRIMINATE, the panel must display the SAME buffer the + // viewport declares and hold a NON-EMPTY selection while the + // document holds none. With different buffers (the first attempt) + // both routings emit nothing and the test proves nothing. + let s = editor(); + let (fid, doc_win, panel_win, doc_buf) = semantic_frontend_with_focused_panel(&s); + + exec(&s, "PROBE = pmacs.buffer.list()[1]"); + { + let mut core = s.core.borrow_mut(); + // Put real text in the document buffer so a span exists. + { + let reg = core.registry.borrow(); + let _ = reg.get(doc_buf).expect("doc"); + } + // The panel shows the document's buffer and selects a range. + core.install_buffer_in_window(panel_win, doc_buf) + .expect("panel shows the document buffer"); + let panel = core.windows.get_mut(&panel_win).expect("panel"); + panel.selection = Some(pmacs::window::Selection { anchor: 0 }); + panel.cursor = 4; + // The document window selects nothing. + let doc = core.windows.get_mut(&doc_win).expect("doc"); + doc.selection = None; + doc.cursor = 0; + } + + let mut sem = SemanticRenderState::for_peer(fid, 18); + sem.set_viewport(doc_buf, ByteRange { start: 0, end: 8 }, 0); + let msgs = sem.render_frame(&s); + + let selection_decorations: usize = msgs + .iter() + .filter_map(|m| match m { + InstanceMessage::Decorations { segments, .. } => Some( + segments + .iter() + .map(|seg| seg.decorations.len()) + .sum::(), + ), + _ => None, + }) + .sum(); + assert_eq!( + selection_decorations, 0, + "a selection living in the focused PANEL must not decorate the document viewport" ); }