From 1be7a30468f1fab73432b363dd8e44c8d2234d93 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Tue, 21 Jul 2026 12:43:22 -0400 Subject: [PATCH] fix(statusline): harden narrow-band review edges Document and pin the GPU built-in-only narrow-band clipping policy, including the intentional ability of a wide right group to hide the left identity. Guard the fixed UI face ordering used by binary search, preserve flattened provider tracebacks in *errors*, and rename the phase-one unavailable reason to cover missing layout contexts. Refresh the implementation verification record after the full gate suite. Co-Authored-By: Claude --- docs/agent-handoff.md | 6 +-- docs/statusline-segments-framing.md | 52 ++++++++++++++++-------- pmacs-gpu/src/main.rs | 60 ++++++++++++++++++++++++++- src/lua_bindings/mod.rs | 2 +- src/semantic_render.rs | 8 ++++ src/statusline.rs | 63 ++++++++++++++++++++++++++--- 6 files changed, 163 insertions(+), 28 deletions(-) diff --git a/docs/agent-handoff.md b/docs/agent-handoff.md index cb69dae..8252577 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -118,9 +118,9 @@ next machine reads it the way you just did. `StatuslineSegments`; authoritative-empty/snapshot symmetry; and atomic GPU validation, face resolution, shaping, clipping, and cache invalidation. Acceptance 1-27 is implemented. Final gates: - Clippy clean; 1,617 default + 1,791 CRDT library tests; 7 default + - 8 CRDT feature acceptance; 114 M4; 108 required GPU; one-invocation - workspace sweep 2,715 passed across 78 suites (19 ignored, + Clippy clean; 1,619 default + 1,793 CRDT library tests; 7 default + + 8 CRDT feature acceptance; 114 M4; 109 required GPU; one-invocation + workspace sweep 2,718 passed across 78 suites (19 ignored, `basedpyright` filtered); `git diff --check` clean. This branch is awaiting review and **must not be described as merged**. - Roadmap: `docs/roadmap-2026-07.md` (ranked arcs). Position: diff --git a/docs/statusline-segments-framing.md b/docs/statusline-segments-framing.md index 13142bd..612f89a 100644 --- a/docs/statusline-segments-framing.md +++ b/docs/statusline-segments-framing.md @@ -8,6 +8,10 @@ and is fully gated; awaiting review, not merged.** Revision 3: closes review findings on authoritative-empty baseline retention and the TUI's protected-suffix clipping boundary. +The implementation review corrected the record for the GPU's +built-in-only narrow-band case: stage 3 deliberately changes the legacy +clipping edge and now pins that behavior with a headless regression test. + Revision 2: closes review findings on invalidation, terminal-control-safe grapheme painting, separator ownership, detached-frontend latches, and the unknown-LSP label. Revision 1 was the initial post-#124 architecture scout. @@ -40,16 +44,17 @@ framed ownership boundary: `BufferSnapshot`. - The GPU consumes v18 atomically, resolves exact dynamic faces, clips provider runs without wrapping or displacing the protected suffix, - and preserves its prior valid state on malformed input. + deliberately right-pins over-wide built-in-only readouts, and + preserves its prior valid state on malformed input. - `builtin/runtime/lsp.lua` registers the first pure right-side provider from its private attachment map; the Rust tracker exposes bounded `init`/`ready`/`degraded`/`crashed`/`stopped`/unknown labels. The final gate run was sequential and clean: `cargo fmt --check`; -workspace/all-target Clippy with `-D warnings`; 1,617 default and 1,791 +workspace/all-target Clippy with `-D warnings`; 1,619 default and 1,793 CRDT library tests; 7 default and 8 CRDT stage-3 acceptance tests; 114 -M4 acceptance tests (3 ignored, `basedpyright` filtered); 108 required -GPU tests; and the one-invocation workspace sweep (2,715 passed across +M4 acceptance tests (3 ignored, `basedpyright` filtered); 109 required +GPU tests; and the one-invocation workspace sweep (2,718 passed across 78 suites, 19 ignored, `basedpyright` filtered). `git diff --check` was clean. No flaky rerun was needed. @@ -437,7 +442,7 @@ or no enabled providers is an O(1) fast path. ### Q#SL4 - Composition, order, separators, and narrow-window policy -Current built-ins remain protected: +Current built-in positions remain anchored: - **Left:** the frontend's current active/modified/buffer-identity group, with its existing edge padding, then custom left segments. @@ -462,20 +467,25 @@ Priority means **survival priority when horizontal space is tight**: - Left custom providers are ordered by `(priority descending, registration id ascending)`. Higher-priority items sit closest to the - protected buffer identity. Overflow clips the low-priority tail. + leading-edge buffer identity. Overflow clips the low-priority tail. - Right custom providers are displayed by `(priority ascending, registration id ascending)`, placing higher-priority items closest to the protected diagnostic/cursor/scroll suffix. The complete right run is right-aligned; overflow clips its low-priority left edge. - The protected built-in suffix is never discarded merely because a custom provider is long. If the built-in suffix itself cannot fit, - each frontend retains today's behavior: the TUI drops the right group - wholesale, while the GPU keeps its right edge fixed and clips its left - edge. Custom-prefix clipping preserves the complete built-in suffix - only when that suffix fits by itself. + the TUI retains its legacy wholesale drop. The GPU deliberately + changes its legacy narrow-band policy: before stage 3 it pinned the + built-in group's left edge and clipped the right tail; stage 3 pins + the right edge and clips the left so the readout tail survives. + Custom-prefix clipping preserves the complete built-in suffix only + when that suffix fits by itself. - The left group gets the space before the right group's measured - origin and clips at the collision boundary. It never overwrites the - right group. + origin and clips at the collision boundary, without the legacy GPU's + extra 10-pixel gap. It never overwrites the right group. This anchors + buffer identity at the leading edge but does not guarantee its + survival: an over-wide right group may consume all available left + space. This asymmetric visual ordering is intentional: priority determines what survives, not a generic ascending sort that would protect opposite @@ -743,9 +753,14 @@ same belt as `StatusFacts`. - Right placement uses the full shaped width without clamping its origin to `TEXT_LEFT`: the run's right edge stays at the right pad, while a negative/left-of-surface origin clips low-priority custom - prefixes and preserves the built-in tail. The left TextArea clips at - the right group's actual origin. Existing geometry bounds still keep - all glyphs inside the band. + prefixes and preserves the built-in tail. This intentionally changes + the legacy built-in-only narrow case, which anchored the readout at + `TEXT_LEFT` and clipped its right tail. +- The left TextArea clips at the right group's actual origin rather + than retaining the legacy extra `STATUS_TEXT_PAD` gap. The right + group therefore owns collision priority and may fully obscure the + left buffer identity in an extremely narrow band. Existing geometry + bounds still keep all glyphs inside the band. - `ThemeFacts` continues to invalidate both caches. FontFacts already re-metrics/re-shapes both status buffers; the new rich runs ride that path without a new font transaction. @@ -845,8 +860,11 @@ TUI, producer, and daemon/wire behavior; protocol pins stay in `RenderState`/semantic frame paths, not direct helper-only formatting. 1. **Default preservation:** with no visible provider output, scratch - TUI cells and GPU pixels are byte-identical to the pre-stage - modeline/status band. The global TUI echo row is unchanged. + TUI cells and ordinary non-overlapping GPU modeline/status-band + pixels are byte-identical to the pre-stage rendering. The deliberate + GPU narrow-band exception pins an over-wide built-in readout's right + edge and clips its left edge; a built-in-only headless fixture pins + that behavior. The global TUI echo row is unchanged. 2. **Lua strict contract:** valid registration returns a handle and appears in `providers`; bad/unknown side, empty name, non-integer or out-of-range priority, non-function `fn`, non-modeline face diff --git a/pmacs-gpu/src/main.rs b/pmacs-gpu/src/main.rs index a2da949..4da619e 100644 --- a/pmacs-gpu/src/main.rs +++ b/pmacs-gpu/src/main.rs @@ -4156,7 +4156,7 @@ impl State { /// Compose the left group. Minibuffer, isearch, and transient /// messages suppress custom left segments; ordinary buffer identity - /// stays protected at the leading edge. + /// starts at the leading edge but may be fully clipped by the right group. fn compose_status_left_runs(&self) -> Vec<(String, Color)> { if let Some(minibuffer) = self.minibuffer.as_ref() { return vec![( @@ -10007,6 +10007,64 @@ mod tests { ); } + #[test] + fn built_in_only_overwide_readout_clips_left_and_keeps_its_right_tail_pinned() { + let (narrow_width, wide_width, height) = (96, 500, 260); + let Some(mut narrow) = headless_or_skip(narrow_width, height, "text") else { + return; + }; + let Some(mut wide) = headless_or_skip(wide_width, height, "text") else { + return; + }; + for state in [&mut narrow, &mut wide] { + let buffer_id = BufferId::next(); + state.current_buffer_id = Some(buffer_id); + state.status_facts = Some(status_facts(buffer_id, None)); + state.own_cursor = Some(OwnCursor { buffer_id, byte: 0 }); + } + + let narrow_frame = narrow.render_offscreen(); + let wide_frame = wide.render_offscreen(); + assert!( + narrow.statusline_segments.is_none() && wide.statusline_segments.is_none(), + "fixture must exercise the built-in-only legacy surface" + ); + let narrow_status_width = narrow + .status_buffer + .layout_runs() + .map(|run| run.line_w) + .fold(0.0_f32, f32::max); + let wide_status_width = wide + .status_buffer + .layout_runs() + .map(|run| run.line_w) + .fold(0.0_f32, f32::max); + assert!( + (narrow_status_width - wide_status_width).abs() < 0.01, + "surface width must not reshape the no-wrap readout" + ); + assert!( + narrow_width as f32 - STATUS_TEXT_PAD - narrow_status_width < 0.0, + "fixture must force the built-in readout past the left edge" + ); + assert!( + wide_width as f32 - STATUS_TEXT_PAD - wide_status_width > 0.0, + "comparison surface must fit the complete built-in readout" + ); + + let band_top = text_area_bottom(height, narrow.fm).floor() as u32; + let pinned_tail_width = 80; + for y in band_top..height { + for offset in 0..pinned_tail_width { + assert_eq!( + px_at(&narrow_frame, narrow_width, narrow_width - 1 - offset, y), + px_at(&wide_frame, wide_width, wide_width - 1 - offset, y), + "built-in readout tail moved at right-edge offset {offset}, y={y}" + ); + } + } + } + #[test] fn overwide_status_runs_never_wrap_and_keep_the_suffix_pinned() { let (width, height) = (800, 300); diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index b55f37a..24caee1 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -5601,7 +5601,7 @@ fn log_hook_error(lua: &Lua, hook_name: &str, err: &crate::hook::HookCallbackErr /// this function owns only the repository-standard durable sink and window /// invalidation. pub(crate) fn log_statusline_provider_error(lua: &Lua, failure: &StatuslineProviderFailure) { - let message = crate::statusline::sanitize_provider_text(&failure.message); + let message = crate::statusline::sanitize_provider_error_text(&failure.message); let line = format!( "[statusline:{}] provider registered at {} failed for {:?}/{:?}/{:?}/active={}: {}\n", failure.provider_name, diff --git a/src/semantic_render.rs b/src/semantic_render.rs index b361da8..14ff5a7 100644 --- a/src/semantic_render.rs +++ b/src/semantic_render.rs @@ -2381,6 +2381,14 @@ mod tests { state.core.borrow().active_window().buffer_id } + #[test] + fn fixed_ui_face_inventory_is_strictly_sorted() { + assert!( + UI_FACES.windows(2).all(|pair| pair[0] < pair[1]), + "theme_facts_msg uses binary_search; duplicates or unsorted insertions misclassify faces" + ); + } + #[test] fn line_numbers_emitted_on_toggle_then_suppressed() { // UX gutter (protocol v13): the daemon ships the per-window gutter diff --git a/src/statusline.rs b/src/statusline.rs index 035335a..d11c885 100644 --- a/src/statusline.rs +++ b/src/statusline.rs @@ -155,8 +155,8 @@ pub struct StatuslineWindowSegments { /// Why phase 1 intentionally produced no message. #[derive(Copy, Clone, Debug, Eq, PartialEq)] pub enum StatuslineNoMessageReason { - /// The frontend/view no longer exists. - FrontendUnavailable, + /// The target frontend, view, or one of its layout windows no longer exists. + ContextUnavailable, /// A layout window points at a buffer that has been removed. BufferUnavailable, /// The semantic frontend's daemon window does not match its declared @@ -612,13 +612,13 @@ fn capture_target_contexts( let view = core .views .get(&frontend_id) - .ok_or(StatuslineNoMessageReason::FrontendUnavailable)?; + .ok_or(StatuslineNoMessageReason::ContextUnavailable)?; let mut contexts = Vec::new(); for window_id in view.layout.iter_ids() { let window = core .windows .get(&window_id) - .ok_or(StatuslineNoMessageReason::FrontendUnavailable)?; + .ok_or(StatuslineNoMessageReason::ContextUnavailable)?; if buffers.get(window.buffer_id).is_err() { return Err(StatuslineNoMessageReason::BufferUnavailable); } @@ -638,11 +638,11 @@ fn capture_target_contexts( let view = core .views .get(&frontend_id) - .ok_or(StatuslineNoMessageReason::FrontendUnavailable)?; + .ok_or(StatuslineNoMessageReason::ContextUnavailable)?; let window = core .windows .get(&view.active) - .ok_or(StatuslineNoMessageReason::FrontendUnavailable)?; + .ok_or(StatuslineNoMessageReason::ContextUnavailable)?; if buffers.get(window.buffer_id).is_err() { return Err(StatuslineNoMessageReason::BufferUnavailable); } @@ -748,6 +748,15 @@ pub fn sanitize_provider_text(text: &str) -> String { output } +/// Flatten a provider failure into one durable `*errors*` entry while +/// retaining multi-line traceback content. +#[must_use] +pub(crate) fn sanitize_provider_error_text(text: &str) -> String { + text.chars() + .map(|ch| if ch.is_control() { ' ' } else { ch }) + .collect() +} + #[cfg(test)] mod tests { use super::*; @@ -763,6 +772,48 @@ mod tests { (host, core, statusline) } + #[test] + fn provider_output_and_error_log_have_distinct_newline_policies() { + let multiline = "boom\nstack\ttrace\r\0"; + assert_eq!(sanitize_provider_text(multiline), "boom"); + assert_eq!( + sanitize_provider_error_text(multiline), + "boom stack trace " + ); + + let (host, core, registry) = harness(); + host.lua() + .load( + r" + pmacs.statusline.register { + name='trace', side='left', + fn=function() error('boom\nstack detail') end, + } + ", + ) + .exec() + .unwrap(); + let evaluation = evaluate_statusline( + host.lua(), + &core, + ®istry, + StatuslineEvaluationTarget::Grid { + frontend_id: FrontendId::LOCAL, + }, + ); + assert_eq!(evaluation.new_failures.len(), 1); + let errors = host.errors_buffer_text(); + assert!( + errors.contains("boom stack detail") && errors.contains("stack traceback"), + "flattened provider traceback must retain every line: {errors:?}" + ); + assert_eq!( + errors.lines().count(), + 1, + "one failure run must remain one durable error entry" + ); + } + #[test] fn registry_epochs_track_layout_and_distinct_enabled_faces() { let lua = Lua::new();