From 30f8375acfd4f41dbb7079b82b83bd93e6dae535 Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Mon, 24 Aug 2026 22:27:15 +0700 Subject: [PATCH 1/4] feat(tui): show persisted guidance on rejected policy chunks The reviewer's rejection note is stored in PolicyChunk.rejection_reason and already reaches the TUI in GetDraftPolicyResponse.chunks, but openshell-tui never read the field. The note was dropped at the last step, so a reviewer had no way to recall why a chunk had been rejected. Render it in two places, following the truncate-in-list / full-in-popup convention in the TUI development guide: a truncated, dimmed suffix on the list row, and a "Guidance:" line in the detail popup. Gate the accessor on status == "rejected" rather than on the field alone. Approving a chunk passes None for the reason and the gateway writes the field only when Some, so a chunk that was rejected and later approved still carries the old note. Reading the field unconditionally would surface a stale rejection on an approved rule. Part of #1098, Definition of Done item "Rejected chunks show persisted guidance". The other items on that issue are untouched. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- crates/openshell-tui/src/ui/sandbox_draft.rs | 88 ++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/crates/openshell-tui/src/ui/sandbox_draft.rs b/crates/openshell-tui/src/ui/sandbox_draft.rs index cda60d7742..2ef364703c 100644 --- a/crates/openshell-tui/src/ui/sandbox_draft.rs +++ b/crates/openshell-tui/src/ui/sandbox_draft.rs @@ -150,6 +150,12 @@ pub fn draw(frame: &mut Frame<'_>, app: &mut App, area: Rect) { if chunk.hit_count > 1 { spans.push(Span::styled(format!(" {}x", chunk.hit_count), t.accent)); } + if let Some(reason) = rejection_guidance(chunk) { + spans.push(Span::styled( + format!(" \"{}\"", truncate_str(reason, 32)), + t.muted, + )); + } let mut line = Line::from(spans); if is_selected { @@ -221,6 +227,14 @@ pub fn draw_detail_popup( ])); } + // Reviewer's persisted rejection guidance. + if let Some(reason) = rejection_guidance(chunk) { + lines.push(Line::from(vec![ + Span::styled("Guidance: ", t.muted), + Span::styled(reason, t.status_err), + ])); + } + // Binary (denormalized from the denial). if !chunk.binary.is_empty() { lines.push(Line::from(vec![ @@ -460,6 +474,19 @@ fn truncate_str(s: &str, max_len: usize) -> String { } } +/// The reviewer's persisted note for a rejected chunk. +/// +/// Gated on status rather than on the field alone: the gateway's +/// `update_draft_chunk_status` leaves `rejection_reason` untouched when a chunk +/// is later approved, so an approved chunk can still carry a stale note. +fn rejection_guidance(chunk: &PolicyChunk) -> Option<&str> { + if chunk.status != "rejected" { + return None; + } + let reason = chunk.rejection_reason.trim(); + (!reason.is_empty()).then_some(reason) +} + #[derive(Clone, Copy)] enum ApprovalAnnotationKind { AutoApproved, @@ -701,3 +728,64 @@ fn format_short_time(epoch_ms: i64) -> String { let seconds = time_of_day % 60; format!("{hours:02}:{minutes:02}:{seconds:02}") } + +#[cfg(test)] +mod tests { + use super::*; + + fn make_chunk(status: &str, rejection_reason: &str) -> PolicyChunk { + PolicyChunk { + status: status.to_string(), + rejection_reason: rejection_reason.to_string(), + ..Default::default() + } + } + + #[test] + fn rejected_chunk_exposes_its_reason() { + let chunk = make_chunk("rejected", "too broad: allows any port"); + assert_eq!( + rejection_guidance(&chunk), + Some("too broad: allows any port") + ); + } + + #[test] + fn approved_chunk_hides_stale_reason() { + // Approving passes None for the reason, which leaves the stored value in + // place, so a chunk rejected and later approved still carries the note. + let chunk = make_chunk("approved", "too broad: allows any port"); + assert_eq!(rejection_guidance(&chunk), None); + } + + #[test] + fn pending_chunk_has_no_guidance() { + assert_eq!(rejection_guidance(&make_chunk("pending", "")), None); + } + + #[test] + fn blank_reason_is_dropped() { + assert_eq!(rejection_guidance(&make_chunk("rejected", "")), None); + assert_eq!(rejection_guidance(&make_chunk("rejected", " \n")), None); + } + + #[test] + fn reason_is_trimmed() { + let chunk = make_chunk("rejected", " needs a narrower host "); + assert_eq!(rejection_guidance(&chunk), Some("needs a narrower host")); + } + + #[test] + fn long_reason_truncates_for_the_list_row() { + let reason = "rejected because the endpoint list is far too permissive"; + let shown = truncate_str(reason, 32); + assert_eq!(shown.chars().count(), 32); + assert!(shown.ends_with("...")); + assert!(shown.starts_with("rejected because")); + } + + #[test] + fn short_reason_is_not_truncated() { + assert_eq!(truncate_str("too broad", 32), "too broad"); + } +} From f18b11b5989eea1bb0df55f5c680f8c39fe682ed Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Tue, 25 Aug 2026 06:24:38 +0700 Subject: [PATCH 2/4] fix(tui): scroll the draft detail popup so long guidance stays reachable A rejection reason has no server-side length cap, so a paragraph-length one overflowed the fixed 22-row detail popup: the tail was clipped and the later fields and both action hints were pushed off screen. The same overflow already affected the unbounded rationale and security_notes fields, so fix the popup rather than special-case the guidance line. Split the popup's inner area into a scrolling body and a pinned hint row, following the pattern in ui/create_provider.rs, and drive it with j/k, the arrow keys, PageUp, PageDown, g and G. The approve and close controls now stay on screen at every scroll position, and the bottom border carries the scroll position. Wrap free-form values explicitly instead of relying on Paragraph's Wrap, so the rendered row count is exactly lines.len() and the scroll clamp cannot under-run the content. Paragraph::line_count would answer the same question but sits behind ratatui's unstable-rendered-line-info feature. Add deterministic TestBackend coverage at 80x24 with a 2,000-character reason, covering the head and tail, the pinned hints, over-scroll clamping, and the pre-existing long-rationale case. Document the reviewer-facing guidance in the policy advisor page. Part of #1098. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- crates/openshell-tui/src/app.rs | 50 +++ crates/openshell-tui/src/ui/mod.rs | 9 +- crates/openshell-tui/src/ui/sandbox_draft.rs | 402 ++++++++++++++++--- docs/sandboxes/policy-advisor.mdx | 2 + 4 files changed, 414 insertions(+), 49 deletions(-) diff --git a/crates/openshell-tui/src/app.rs b/crates/openshell-tui/src/app.rs index 1619dab9fa..a5e14bcbab 100644 --- a/crates/openshell-tui/src/app.rs +++ b/crates/openshell-tui/src/app.rs @@ -711,6 +711,13 @@ pub struct App { pub draft_viewport_height: usize, /// When true, the detail popup is shown for the selected draft chunk. pub draft_detail_open: bool, + /// Scroll offset, in rendered rows, of the draft detail popup body. + pub draft_detail_scroll: usize, + /// Total rows of detail-popup content (set by the draw pass). + pub draft_detail_rows: usize, + /// Visible rows in the detail-popup body, excluding the pinned hint row + /// (set by the draw pass). + pub draft_detail_body_height: usize, /// Per-sandbox count of pending draft recommendations (parallel to `sandbox_names`). pub sandbox_draft_counts: Vec, @@ -1032,6 +1039,9 @@ impl App { draft_scroll: 0, draft_viewport_height: 0, draft_detail_open: false, + draft_detail_scroll: 0, + draft_detail_rows: 0, + draft_detail_body_height: 0, sandbox_draft_counts: Vec::new(), pending_draft_approve: false, pending_draft_reject: false, @@ -1648,6 +1658,7 @@ impl App { KeyCode::Esc => { self.cancel_log_stream(); self.draft_detail_open = false; + self.draft_detail_scroll = 0; self.sandbox_policy_tab = SandboxPolicyTab::Policy; self.screen = Screen::Dashboard; self.focus = Focus::Sandboxes; @@ -1856,6 +1867,25 @@ impl App { } } + /// Largest useful scroll offset for the draft detail popup. + fn draft_detail_max_scroll(&self) -> usize { + self.draft_detail_rows + .saturating_sub(self.draft_detail_body_height) + } + + /// One screenful of the draft detail popup body. + fn draft_detail_page(&self) -> isize { + isize::try_from(self.draft_detail_body_height.max(1)).unwrap_or(1) + } + + /// Move the draft detail popup by `delta` rows, clamped to the content. + fn scroll_draft_detail(&mut self, delta: isize) { + let max = isize::try_from(self.draft_detail_max_scroll()).unwrap_or(isize::MAX); + let current = isize::try_from(self.draft_detail_scroll).unwrap_or(0); + let next = current.saturating_add(delta).clamp(0, max); + self.draft_detail_scroll = usize::try_from(next).unwrap_or(0); + } + fn handle_draft_key(&mut self, key: KeyEvent) { // Approve-all confirmation modal intercepts all keys when open. if self.approve_all_confirm_open { @@ -1880,6 +1910,7 @@ impl App { match key.code { KeyCode::Esc | KeyCode::Enter => { self.draft_detail_open = false; + self.draft_detail_scroll = 0; } // Allow approve/reject toggle from within the popup. KeyCode::Char('a') => { @@ -1893,6 +1924,7 @@ impl App { if st == "pending" || st == "rejected" { self.pending_draft_approve = true; self.draft_detail_open = false; + self.draft_detail_scroll = 0; } } } @@ -1908,10 +1940,27 @@ impl App { if st == "pending" || st == "approved" { self.pending_draft_reject = true; self.draft_detail_open = false; + self.draft_detail_scroll = 0; } } } } + // Scroll the detail body; long rejection guidance and rationales + // can exceed the fixed popup height. + KeyCode::Down | KeyCode::Char('j') => self.scroll_draft_detail(1), + KeyCode::Up | KeyCode::Char('k') => self.scroll_draft_detail(-1), + KeyCode::PageDown => { + let page = self.draft_detail_page(); + self.scroll_draft_detail(page); + } + KeyCode::PageUp => { + let page = self.draft_detail_page(); + self.scroll_draft_detail(-page); + } + KeyCode::Home | KeyCode::Char('g') => self.draft_detail_scroll = 0, + KeyCode::End | KeyCode::Char('G') => { + self.draft_detail_scroll = self.draft_detail_max_scroll(); + } _ => {} } return; @@ -1937,6 +1986,7 @@ impl App { } KeyCode::Enter if !self.draft_chunks.is_empty() => { self.draft_detail_open = true; + self.draft_detail_scroll = 0; } KeyCode::Char('j') | KeyCode::Down => { if total == 0 { diff --git a/crates/openshell-tui/src/ui/mod.rs b/crates/openshell-tui/src/ui/mod.rs index 8df6dd3470..3078ff793f 100644 --- a/crates/openshell-tui/src/ui/mod.rs +++ b/crates/openshell-tui/src/ui/mod.rs @@ -103,8 +103,13 @@ fn draw_sandbox_screen(frame: &mut Frame<'_>, app: &mut App, area: Rect) { // Draft detail popup renders over the full frame. if app.focus == Focus::SandboxDraft && app.draft_detail_open { let abs = app.draft_scroll + app.draft_selected; - if let Some(chunk) = app.draft_chunks.get(abs) { - sandbox_draft::draw_detail_popup(frame, chunk, frame.size(), &app.theme); + let scroll = app.draft_detail_scroll; + let metrics = app.draft_chunks.get(abs).map(|chunk| { + sandbox_draft::draw_detail_popup(frame, chunk, frame.size(), &app.theme, scroll) + }); + if let Some(metrics) = metrics { + app.draft_detail_rows = metrics.total_rows; + app.draft_detail_body_height = metrics.body_height; } } diff --git a/crates/openshell-tui/src/ui/sandbox_draft.rs b/crates/openshell-tui/src/ui/sandbox_draft.rs index 2ef364703c..0bd74f77fd 100644 --- a/crates/openshell-tui/src/ui/sandbox_draft.rs +++ b/crates/openshell-tui/src/ui/sandbox_draft.rs @@ -6,10 +6,10 @@ use crate::app::App; use openshell_core::proto::{L7Allow, L7DenyRule, L7QueryMatcher, NetworkEndpoint, PolicyChunk}; use ratatui::Frame; -use ratatui::layout::Rect; -use ratatui::style::Modifier; +use ratatui::layout::{Constraint, Direction, Layout, Rect}; +use ratatui::style::{Modifier, Style}; use ratatui::text::{Line, Span}; -use ratatui::widgets::{Block, Borders, Clear, Padding, Paragraph, Wrap}; +use ratatui::widgets::{Block, Borders, Clear, Padding, Paragraph}; use super::centered_rect; @@ -178,12 +178,22 @@ pub fn draw(frame: &mut Frame<'_>, app: &mut App, area: Rect) { // Detail popup (Enter key) // --------------------------------------------------------------------------- +/// What `draw_detail_popup` actually laid out, so the app can clamp its scroll +/// offset to content that exists. +pub struct DetailMetrics { + /// Total rows of content, after wrapping. + pub total_rows: usize, + /// Rows visible in the scrollable body, excluding the pinned hint row. + pub body_height: usize, +} + pub fn draw_detail_popup( frame: &mut Frame<'_>, chunk: &PolicyChunk, area: Rect, theme: &crate::theme::Theme, -) { + scroll: usize, +) -> DetailMetrics { let t = theme; let popup_width = (area.width * 4 / 5).min(area.width.saturating_sub(4)); let popup_height = 22u16.min(area.height.saturating_sub(4)); @@ -204,6 +214,11 @@ pub fn draw_detail_popup( .border_style(t.accent) .padding(Padding::new(1, 1, 0, 0)); + // Text columns inside the borders and the one-column padding on each side. + // Content is wrapped to this width here rather than by `Wrap`, so the row + // count below is exactly what renders and the scroll clamp stays honest. + let text_width = usize::from(popup_width).saturating_sub(4).max(1); + let mut lines: Vec> = vec![ Line::from(vec![ Span::styled("Status: ", t.muted), @@ -221,26 +236,40 @@ pub fn draw_detail_popup( ApprovalAnnotationKind::RequiresReview => t.status_warn.add_modifier(Modifier::BOLD), ApprovalAnnotationKind::Reviewed => t.muted, }; - lines.push(Line::from(vec![ - Span::styled("Review: ", t.muted), - Span::styled(annotation.detail_label, annotation_style), - ])); + push_wrapped( + &mut lines, + "Review: ", + t.muted, + &annotation.detail_label, + annotation_style, + text_width, + ); } - // Reviewer's persisted rejection guidance. + // Reviewer's persisted rejection guidance. The reason is free-form and has + // no server-side length cap, so it wraps and the popup scrolls instead of + // clipping the tail. if let Some(reason) = rejection_guidance(chunk) { - lines.push(Line::from(vec![ - Span::styled("Guidance: ", t.muted), - Span::styled(reason, t.status_err), - ])); + push_wrapped( + &mut lines, + "Guidance: ", + t.muted, + reason, + t.status_err, + text_width, + ); } // Binary (denormalized from the denial). if !chunk.binary.is_empty() { - lines.push(Line::from(vec![ - Span::styled("Binary: ", t.muted), - Span::styled(&chunk.binary, t.text), - ])); + push_wrapped( + &mut lines, + "Binary: ", + t.muted, + &chunk.binary, + t.text, + text_width, + ); } // Hit count (accumulated real denial count) and first/last seen. @@ -269,17 +298,17 @@ pub fn draw_detail_popup( lines.push(Line::from("")); lines.push(Line::from(Span::styled("Endpoints:", t.muted))); for ep in &rule.endpoints { - lines.push(Line::from(vec![ - Span::raw(" "), - Span::styled("-> ", t.muted), - Span::styled(format_endpoint_summary(ep), t.accent), - ])); + push_wrapped( + &mut lines, + " -> ", + t.muted, + &format_endpoint_summary(ep), + t.accent, + text_width, + ); for detail in format_endpoint_details(ep) { - lines.push(Line::from(vec![ - Span::raw(" "), - Span::styled(detail, t.text), - ])); + push_wrapped(&mut lines, " ", t.text, &detail, t.text, text_width); } } @@ -288,10 +317,7 @@ pub fn draw_detail_popup( lines.push(Line::from("")); lines.push(Line::from(Span::styled("Binaries:", t.muted))); for b in &rule.binaries { - lines.push(Line::from(vec![ - Span::raw(" "), - Span::styled(&b.path, t.text), - ])); + push_wrapped(&mut lines, " ", t.text, &b.path, t.text, text_width); } } } @@ -299,23 +325,61 @@ pub fn draw_detail_popup( // Rationale. if !chunk.rationale.is_empty() { lines.push(Line::from("")); - lines.push(Line::from(vec![ - Span::styled("Rationale: ", t.muted), - Span::styled(&chunk.rationale, t.text), - ])); + push_wrapped( + &mut lines, + "Rationale: ", + t.muted, + &chunk.rationale, + t.text, + text_width, + ); } // Security notes. if !chunk.security_notes.is_empty() { lines.push(Line::from("")); - lines.push(Line::from(vec![Span::styled( - format!("! {}", chunk.security_notes), - t.status_warn.add_modifier(Modifier::BOLD), - )])); + let warn = t.status_warn.add_modifier(Modifier::BOLD); + push_wrapped( + &mut lines, + "! ", + warn, + &chunk.security_notes, + warn, + text_width, + ); } - // Action hints — state-aware toggle keys. - lines.push(Line::from("")); + // Split the inner area into a scrollable body and a pinned hint row, so the + // action and close controls stay on screen however long the content is. + let inner = block.inner(popup_area); + let parts = Layout::default() + .direction(Direction::Vertical) + .constraints([Constraint::Min(0), Constraint::Length(1)]) + .split(inner); + let body_height = usize::from(parts[0].height); + let total_rows = lines.len(); + let max_scroll = total_rows.saturating_sub(body_height); + let scroll = scroll.min(max_scroll); + + let block = if max_scroll > 0 { + block.title_bottom( + Line::from(Span::styled( + format!(" {}/{} ", scroll + 1, total_rows), + t.muted, + )) + .right_aligned(), + ) + } else { + block + }; + + frame.render_widget(block, popup_area); + frame.render_widget( + Paragraph::new(lines).scroll((u16::try_from(scroll).unwrap_or(u16::MAX), 0)), + parts[0], + ); + + // Action hints — state-aware toggle keys, pinned below the scrolling body. let mut hint_spans: Vec> = Vec::new(); match chunk.status.as_str() { "pending" => { @@ -340,18 +404,22 @@ pub fn draw_detail_popup( } _ => {} } + if max_scroll > 0 { + hint_spans.extend([ + Span::styled("[j/k]", t.key_hint), + Span::styled(" Scroll ", t.text), + ]); + } hint_spans.extend([ Span::styled("[Esc]", t.muted), Span::styled(" Close", t.muted), ]); - lines.push(Line::from(hint_spans)); + frame.render_widget(Paragraph::new(Line::from(hint_spans)), parts[1]); - frame.render_widget( - Paragraph::new(lines) - .block(block) - .wrap(Wrap { trim: false }), - popup_area, - ); + DetailMetrics { + total_rows, + body_height, + } } // --------------------------------------------------------------------------- @@ -474,6 +542,78 @@ fn truncate_str(s: &str, max_len: usize) -> String { } } +/// Word-wrap `text` into rows of at most `width` columns. +/// +/// A word longer than `width` is hard-broken rather than allowed to overflow. +/// Always returns at least one row so callers can index the first row safely. +fn wrap_value(text: &str, width: usize) -> Vec { + if width == 0 { + return vec![text.to_string()]; + } + let mut rows: Vec = Vec::new(); + let mut cur = String::new(); + let mut cur_len = 0usize; + for word in text.split_whitespace() { + let word_len = word.chars().count(); + if word_len > width { + if cur_len > 0 { + rows.push(std::mem::take(&mut cur)); + cur_len = 0; + } + for ch in word.chars() { + if cur_len == width { + rows.push(std::mem::take(&mut cur)); + cur_len = 0; + } + cur.push(ch); + cur_len += 1; + } + continue; + } + let needed = if cur_len == 0 { + word_len + } else { + cur_len + 1 + word_len + }; + if needed > width { + rows.push(std::mem::take(&mut cur)); + cur_len = 0; + } + if cur_len > 0 { + cur.push(' '); + cur_len += 1; + } + cur.push_str(word); + cur_len += word_len; + } + if cur_len > 0 || rows.is_empty() { + rows.push(cur); + } + rows +} + +/// Push `text` wrapped to `width`, with `prefix` on the first row and a matching +/// indent on continuation rows so the label column stays aligned. +fn push_wrapped( + lines: &mut Vec>, + prefix: &str, + prefix_style: Style, + text: &str, + text_style: Style, + width: usize, +) { + let indent = prefix.chars().count(); + let available = width.saturating_sub(indent).max(1); + for (i, row) in wrap_value(text, available).into_iter().enumerate() { + let head = if i == 0 { + Span::styled(prefix.to_string(), prefix_style) + } else { + Span::raw(" ".repeat(indent)) + }; + lines.push(Line::from(vec![head, Span::styled(row, text_style)])); + } +} + /// The reviewer's persisted note for a rejected chunk. /// /// Gated on status rather than on the field alone: the gateway's @@ -732,6 +872,9 @@ fn format_short_time(epoch_ms: i64) -> String { #[cfg(test)] mod tests { use super::*; + use crate::theme::Theme; + use ratatui::Terminal; + use ratatui::backend::TestBackend; fn make_chunk(status: &str, rejection_reason: &str) -> PolicyChunk { PolicyChunk { @@ -788,4 +931,169 @@ mod tests { fn short_reason_is_not_truncated() { assert_eq!(truncate_str("too broad", 32), "too broad"); } + + // --- wrapping --------------------------------------------------------- + + #[test] + fn wrap_value_breaks_on_word_boundaries() { + assert_eq!( + wrap_value("alpha beta gamma", 11), + vec!["alpha beta", "gamma"] + ); + } + + #[test] + fn wrap_value_hard_breaks_a_word_longer_than_the_width() { + assert_eq!(wrap_value("abcdefghij", 4), vec!["abcd", "efgh", "ij"]); + } + + #[test] + fn wrap_value_always_returns_at_least_one_row() { + assert_eq!(wrap_value("", 10), vec![String::new()]); + } + + #[test] + fn wrap_value_never_exceeds_the_width() { + let text = "reject this rule because the endpoint list is far too permissive"; + for row in wrap_value(text, 17) { + assert!(row.chars().count() <= 17, "row too wide: {row:?}"); + } + } + + // --- rendering -------------------------------------------------------- + + fn render(chunk: &PolicyChunk, width: u16, height: u16, scroll: usize) -> (String, usize) { + let mut terminal = Terminal::new(TestBackend::new(width, height)).unwrap(); + let mut max_scroll = 0usize; + terminal + .draw(|frame| { + let metrics = draw_detail_popup( + frame, + chunk, + Rect::new(0, 0, width, height), + &Theme::dark(), + scroll, + ); + max_scroll = metrics.total_rows.saturating_sub(metrics.body_height); + }) + .unwrap(); + let buffer = terminal.backend().buffer(); + let text: String = buffer + .content() + .iter() + .map(ratatui::buffer::Cell::symbol) + .collect(); + (text, max_scroll) + } + + /// A rejection reason has no server-side length cap, so the popup has to keep + /// a paragraph-length one reachable rather than clipping its tail. + fn long_reason() -> String { + let mut reason = String::from("PREFIX_MARKER "); + while reason.len() < 1980 { + reason.push_str("this rule is far too broad and must be narrowed; "); + } + reason.push_str(" SUFFIX_MARKER"); + reason + } + + #[test] + fn long_guidance_head_and_tail_are_both_reachable() { + let chunk = PolicyChunk { + status: "rejected".to_string(), + rejection_reason: long_reason(), + rule_name: "allow-github".to_string(), + ..Default::default() + }; + + let (top, max_scroll) = render(&chunk, 80, 24, 0); + assert!(max_scroll > 0, "content should overflow an 80x24 popup"); + assert!( + top.contains("PREFIX_MARKER"), + "head not visible at scroll 0" + ); + + let (bottom, _) = render(&chunk, 80, 24, max_scroll); + assert!( + bottom.contains("SUFFIX_MARKER"), + "tail not reachable at max scroll" + ); + } + + #[test] + fn action_hints_stay_visible_at_every_scroll_position() { + let chunk = PolicyChunk { + status: "rejected".to_string(), + rejection_reason: long_reason(), + rule_name: "allow-github".to_string(), + ..Default::default() + }; + + let (top, max_scroll) = render(&chunk, 80, 24, 0); + let (bottom, _) = render(&chunk, 80, 24, max_scroll); + for (label, screen) in [("top", &top), ("bottom", &bottom)] { + assert!(screen.contains("Close"), "close hint missing at {label}"); + assert!( + screen.contains("Approve"), + "approve hint missing at {label}" + ); + assert!(screen.contains("Scroll"), "scroll hint missing at {label}"); + } + } + + #[test] + fn scrolling_past_the_end_is_clamped_to_the_last_page() { + let chunk = PolicyChunk { + status: "rejected".to_string(), + rejection_reason: long_reason(), + rule_name: "allow-github".to_string(), + ..Default::default() + }; + let (_, max_scroll) = render(&chunk, 80, 24, 0); + let (clamped, _) = render(&chunk, 80, 24, max_scroll + 500); + let (last, _) = render(&chunk, 80, 24, max_scroll); + assert_eq!( + clamped, last, + "over-scrolling should clamp to the last page" + ); + } + + /// The same overflow already affected `rationale` before this change, so the + /// scrollable body has to fix that case too. + #[test] + fn long_rationale_tail_is_reachable_on_a_pending_chunk() { + let mut rationale = String::from("PREFIX_MARKER "); + while rationale.len() < 1980 { + rationale.push_str("the agent needs broad network access; "); + } + rationale.push_str(" SUFFIX_MARKER"); + let chunk = PolicyChunk { + status: "pending".to_string(), + rule_name: "allow-github".to_string(), + rationale, + ..Default::default() + }; + + let (_, max_scroll) = render(&chunk, 80, 24, 0); + let (bottom, _) = render(&chunk, 80, 24, max_scroll); + assert!(bottom.contains("SUFFIX_MARKER")); + assert!(bottom.contains("Close")); + } + + #[test] + fn short_content_does_not_scroll() { + let chunk = PolicyChunk { + status: "rejected".to_string(), + rejection_reason: "too broad".to_string(), + rule_name: "allow-github".to_string(), + ..Default::default() + }; + let (screen, max_scroll) = render(&chunk, 80, 24, 0); + assert_eq!(max_scroll, 0, "short content should not be scrollable"); + assert!(screen.contains("too broad")); + assert!( + !screen.contains("Scroll"), + "scroll hint should be hidden when everything fits" + ); + } } diff --git a/docs/sandboxes/policy-advisor.mdx b/docs/sandboxes/policy-advisor.mdx index bc09468cdb..b4d607cb33 100644 --- a/docs/sandboxes/policy-advisor.mdx +++ b/docs/sandboxes/policy-advisor.mdx @@ -231,6 +231,8 @@ openshell rule reject \ The rejection reason is returned to the agent through `policy.local`. The agent can use it to draft a narrower proposal. +Reviewers see the same guidance in the terminal UI. Run `openshell term`, open the sandbox's draft inbox, and select a rejected chunk to open its detail popup. The stored reason appears on a `Guidance:` line, and the list row shows a shortened copy of it. Rejection reasons have no length limit, so long guidance wraps and the popup body scrolls with `j`/`k`, `PageUp`/`PageDown`, and `g`/`G`; the approve and close controls stay pinned below the body, and the bottom border shows the scroll position. + ## Agent API `policy.local` is available only inside the sandbox and uses plain HTTP: From 03c67b18b405fd4c8d8204973efcc1474457f8a8 Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Tue, 25 Aug 2026 06:38:00 +0700 Subject: [PATCH 3/4] fix(tui): wrap the denial row and hint footer on a narrow popup Making the scroll clamp exact meant dropping Paragraph's Wrap, which also removed wrapping from the lines that do not go through push_wrapped. At 70 columns the denial row clipped mid-value and lost the last-seen timestamp, and at 60 a pending chunk with scrollable content pushed [Esc] Close past the right edge of the single hint row. The key still worked, but that is the same controls-not-visible failure one axis over. Keep the denial row on one line while it fits and wrap it onto the label indent when it does not, so the common 80-column layout is unchanged. Pack the footer hints into as many rows as they need without splitting a hint, and derive the footer height from that: adding a hint row shrinks the body and can itself change whether the content scrolls, so the two settle together. The earlier tests were all 80x24, which is why they missed this. Cover the denial timestamps and the approve, reject and close hints at 60, 70 and 80 columns, plus the packing helper directly. Part of #1098. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- crates/openshell-tui/src/ui/sandbox_draft.rs | 252 ++++++++++++++----- 1 file changed, 193 insertions(+), 59 deletions(-) diff --git a/crates/openshell-tui/src/ui/sandbox_draft.rs b/crates/openshell-tui/src/ui/sandbox_draft.rs index 0bd74f77fd..a2eefdbd92 100644 --- a/crates/openshell-tui/src/ui/sandbox_draft.rs +++ b/crates/openshell-tui/src/ui/sandbox_draft.rs @@ -272,26 +272,47 @@ pub fn draw_detail_popup( ); } - // Hit count (accumulated real denial count) and first/last seen. - lines.push(Line::from(vec![ - Span::styled("Denied: ", t.muted), - Span::styled( - format!( - "{} connection{}", - chunk.hit_count, - if chunk.hit_count == 1 { "" } else { "s" } - ), + // Hit count (accumulated real denial count) and first/last seen. Kept on one + // row while it fits, so a narrow terminal wraps it instead of losing the tail. + let denied_label = "Denied: "; + let denied_count = format!( + "{} connection{}", + chunk.hit_count, + if chunk.hit_count == 1 { "" } else { "s" } + ); + let denied_seen = format!( + "(first {} / last {})", + format_short_time(chunk.first_seen_ms), + format_short_time(chunk.last_seen_ms), + ); + let denied_width = denied_label.chars().count() + + denied_count.chars().count() + + 2 + + denied_seen.chars().count(); + if denied_width <= text_width { + lines.push(Line::from(vec![ + Span::styled(denied_label, t.muted), + Span::styled(denied_count, t.accent), + Span::styled(format!(" {denied_seen}"), t.muted), + ])); + } else { + push_wrapped( + &mut lines, + denied_label, + t.muted, + &denied_count, t.accent, - ), - Span::styled( - format!( - " (first {} / last {})", - format_short_time(chunk.first_seen_ms), - format_short_time(chunk.last_seen_ms), - ), + text_width, + ); + push_wrapped( + &mut lines, + &" ".repeat(denied_label.chars().count()), t.muted, - ), - ])); + &denied_seen, + t.muted, + text_width, + ); + } // Endpoints. if let Some(ref rule) = chunk.proposed_rule { @@ -349,15 +370,36 @@ pub fn draw_detail_popup( ); } - // Split the inner area into a scrollable body and a pinned hint row, so the - // action and close controls stay on screen however long the content is. + // Split the inner area into a scrollable body and pinned hint rows, so the + // action and close controls stay on screen however long the content is. The + // hints need a second row on a narrow terminal and that shrinks the body, so + // settle the two together. let inner = block.inner(popup_area); + let inner_height = usize::from(inner.height); + let total_rows = lines.len(); + let max_footer = inner_height.saturating_sub(1).max(1); + + let mut footer_rows = 1usize; + let mut hint_rows = Vec::new(); + for _ in 0..2 { + let body = inner_height.saturating_sub(footer_rows); + let scrollable = total_rows.saturating_sub(body) > 0; + hint_rows = pack_hints(&hint_units(chunk, t, scrollable), text_width); + let needed = hint_rows.len().clamp(1, max_footer); + if needed == footer_rows { + break; + } + footer_rows = needed; + } + let parts = Layout::default() .direction(Direction::Vertical) - .constraints([Constraint::Min(0), Constraint::Length(1)]) + .constraints([ + Constraint::Min(0), + Constraint::Length(u16::try_from(footer_rows).unwrap_or(1)), + ]) .split(inner); let body_height = usize::from(parts[0].height); - let total_rows = lines.len(); let max_scroll = total_rows.saturating_sub(body_height); let scroll = scroll.min(max_scroll); @@ -378,43 +420,7 @@ pub fn draw_detail_popup( Paragraph::new(lines).scroll((u16::try_from(scroll).unwrap_or(u16::MAX), 0)), parts[0], ); - - // Action hints — state-aware toggle keys, pinned below the scrolling body. - let mut hint_spans: Vec> = Vec::new(); - match chunk.status.as_str() { - "pending" => { - hint_spans.extend([ - Span::styled("[a]", t.key_hint), - Span::styled(" Approve ", t.text), - Span::styled("[x]", t.key_hint), - Span::styled(" Reject ", t.text), - ]); - } - "approved" => { - hint_spans.extend([ - Span::styled("[x]", t.key_hint), - Span::styled(" Revoke ", t.text), - ]); - } - "rejected" => { - hint_spans.extend([ - Span::styled("[a]", t.key_hint), - Span::styled(" Approve ", t.text), - ]); - } - _ => {} - } - if max_scroll > 0 { - hint_spans.extend([ - Span::styled("[j/k]", t.key_hint), - Span::styled(" Scroll ", t.text), - ]); - } - hint_spans.extend([ - Span::styled("[Esc]", t.muted), - Span::styled(" Close", t.muted), - ]); - frame.render_widget(Paragraph::new(Line::from(hint_spans)), parts[1]); + frame.render_widget(Paragraph::new(hint_rows), parts[1]); DetailMetrics { total_rows, @@ -614,6 +620,64 @@ fn push_wrapped( } } +/// One footer hint: the spans that render it, and its display width. +type HintUnit = (Vec>, usize); + +/// State-aware hints for the detail popup footer. +fn hint_units(chunk: &PolicyChunk, t: &crate::theme::Theme, scrollable: bool) -> Vec { + let mut units: Vec = Vec::new(); + { + let mut add = |key: &str, label: &str, key_style: Style, label_style: Style| { + units.push(( + vec![ + Span::styled(key.to_string(), key_style), + Span::styled(label.to_string(), label_style), + ], + key.chars().count() + label.chars().count(), + )); + }; + match chunk.status.as_str() { + "pending" => { + add("[a]", " Approve ", t.key_hint, t.text); + add("[x]", " Reject ", t.key_hint, t.text); + } + "approved" => add("[x]", " Revoke ", t.key_hint, t.text), + "rejected" => add("[a]", " Approve ", t.key_hint, t.text), + _ => {} + } + if scrollable { + add("[j/k]", " Scroll ", t.key_hint, t.text); + } + add("[Esc]", " Close", t.muted, t.muted); + } + units +} + +/// Pack hint units into rows no wider than `width`, never splitting a unit. +/// +/// A narrow terminal would otherwise clip the trailing hints, and `[Esc] Close` +/// is the last one. +fn pack_hints(units: &[HintUnit], width: usize) -> Vec> { + let mut rows: Vec> = Vec::new(); + let mut current: Vec> = Vec::new(); + let mut current_width = 0usize; + for (spans, unit_width) in units { + if current_width + unit_width > width && !current.is_empty() { + rows.push(Line::from(std::mem::take(&mut current))); + current_width = 0; + } + current.extend(spans.iter().cloned()); + current_width += unit_width; + } + if !current.is_empty() { + rows.push(Line::from(current)); + } + if rows.is_empty() { + rows.push(Line::from(String::new())); + } + rows +} + /// The reviewer's persisted note for a rejected chunk. /// /// Gated on status rather than on the field alone: the gateway's @@ -1096,4 +1160,74 @@ mod tests { "scroll hint should be hidden when everything fits" ); } + + fn denied_chunk(status: &str, reason: &str) -> PolicyChunk { + PolicyChunk { + status: status.to_string(), + rejection_reason: reason.to_string(), + rule_name: "allow-github".to_string(), + confidence: 0.82, + hit_count: 3, + first_seen_ms: 1_700_000_000_000, + last_seen_ms: 1_700_000_100_000, + ..Default::default() + } + } + + /// Dropping `Wrap` means anything not routed through `push_wrapped` clips + /// horizontally, so the denial timestamps have to wrap on a narrow popup. + #[test] + fn denied_timestamps_survive_a_narrow_popup() { + for width in [60u16, 70u16, 80u16] { + let chunk = denied_chunk("rejected", "scope this to docs/ paths only"); + let (screen, _) = render(&chunk, width, 24, 0); + assert!( + screen.contains("22:13:20"), + "first-seen lost at width {width}" + ); + assert!( + screen.contains("22:15:00"), + "last-seen lost at width {width}" + ); + } + } + + /// The close hint is the last one, so a narrow footer would drop it first. + #[test] + fn close_hint_survives_a_narrow_popup_with_scrollable_content() { + for width in [60u16, 70u16, 80u16] { + let chunk = denied_chunk("pending", ""); + let chunk = PolicyChunk { + rationale: long_reason(), + ..chunk + }; + let (screen, max_scroll) = render(&chunk, width, 24, 0); + assert!(max_scroll > 0, "content should overflow at width {width}"); + assert!(screen.contains("Close"), "close hint lost at width {width}"); + assert!( + screen.contains("Approve"), + "approve hint lost at width {width}" + ); + assert!( + screen.contains("Reject"), + "reject hint lost at width {width}" + ); + } + } + + #[test] + fn hints_pack_onto_one_row_when_they_fit() { + let theme = Theme::dark(); + let chunk = denied_chunk("pending", ""); + let rows = pack_hints(&hint_units(&chunk, &theme, true), 200); + assert_eq!(rows.len(), 1); + } + + #[test] + fn hints_spill_onto_a_second_row_when_they_do_not_fit() { + let theme = Theme::dark(); + let chunk = denied_chunk("pending", ""); + let rows = pack_hints(&hint_units(&chunk, &theme, true), 30); + assert!(rows.len() > 1, "hints should wrap at 30 columns"); + } } From b47d01e00d6972aba3e636edbad107af55564ce7 Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Tue, 25 Aug 2026 07:05:46 +0700 Subject: [PATCH 4/4] fix(tui): measure popup wrapping in display columns, not chars wrap_value packed rows by chars().count(), and a CJK glyph is one char but two terminal columns. A double-width rejection reason therefore produced rows about twice the popup's width: the right half of every row was clipped, and unlike the vertical case there is no horizontal scroll to recover it. Measure width with Span::width(), the same measurement ratatui applies when it lays cells out, so the wrap agrees with the renderer by construction and no new dependency is needed. This covers word packing, the hard-break path for a word wider than the line, the label indent, the footer hint packing, and the denial row's fits-on-one-line check. The list row's shortened copy had the same defect through truncate_str, which counts chars. Leave truncate_str alone for its two existing callers and add truncate_display for the guidance row. Cover it with an 80x24 render regression that interleaves markers through the CJK text: a trailing marker lands on its own short row in both the broken and fixed layouts, so it would not detect this. The two unit tests measure with an independent column oracle rather than the helper under test, which would make them tautological. Part of #1098. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- crates/openshell-tui/src/ui/sandbox_draft.rs | 189 ++++++++++++++++--- 1 file changed, 161 insertions(+), 28 deletions(-) diff --git a/crates/openshell-tui/src/ui/sandbox_draft.rs b/crates/openshell-tui/src/ui/sandbox_draft.rs index a2eefdbd92..470463b7d4 100644 --- a/crates/openshell-tui/src/ui/sandbox_draft.rs +++ b/crates/openshell-tui/src/ui/sandbox_draft.rs @@ -152,7 +152,7 @@ pub fn draw(frame: &mut Frame<'_>, app: &mut App, area: Rect) { } if let Some(reason) = rejection_guidance(chunk) { spans.push(Span::styled( - format!(" \"{}\"", truncate_str(reason, 32)), + format!(" \"{}\"", truncate_display(reason, 32)), t.muted, )); } @@ -285,10 +285,10 @@ pub fn draw_detail_popup( format_short_time(chunk.first_seen_ms), format_short_time(chunk.last_seen_ms), ); - let denied_width = denied_label.chars().count() - + denied_count.chars().count() + let denied_width = display_width(denied_label) + + display_width(&denied_count) + 2 - + denied_seen.chars().count(); + + display_width(&denied_seen); if denied_width <= text_width { lines.push(Line::from(vec![ Span::styled(denied_label, t.muted), @@ -306,7 +306,7 @@ pub fn draw_detail_popup( ); push_wrapped( &mut lines, - &" ".repeat(denied_label.chars().count()), + &" ".repeat(display_width(denied_label)), t.muted, &denied_seen, t.muted, @@ -548,9 +548,19 @@ fn truncate_str(s: &str, max_len: usize) -> String { } } -/// Word-wrap `text` into rows of at most `width` columns. +/// Terminal display width of `text` in columns. /// -/// A word longer than `width` is hard-broken rather than allowed to overflow. +/// Uses the same measurement ratatui applies when it lays cells out, so a +/// double-width glyph such as CJK counts as the two columns it will occupy. +/// Counting `chars` here instead would let CJK text render past the popup edge +/// with no way to scroll to the missing tail. +fn display_width(text: &str) -> usize { + Span::raw(text).width() +} + +/// Word-wrap `text` into rows of at most `width` display columns. +/// +/// A word wider than `width` is hard-broken rather than allowed to overflow. /// Always returns at least one row so callers can index the first row safely. fn wrap_value(text: &str, width: usize) -> Vec { if width == 0 { @@ -558,46 +568,75 @@ fn wrap_value(text: &str, width: usize) -> Vec { } let mut rows: Vec = Vec::new(); let mut cur = String::new(); - let mut cur_len = 0usize; + let mut cur_width = 0usize; + let mut buf = [0u8; 4]; for word in text.split_whitespace() { - let word_len = word.chars().count(); - if word_len > width { - if cur_len > 0 { + let word_width = display_width(word); + if word_width > width { + if cur_width > 0 { rows.push(std::mem::take(&mut cur)); - cur_len = 0; + cur_width = 0; } for ch in word.chars() { - if cur_len == width { + let ch_width = display_width(ch.encode_utf8(&mut buf)); + if cur_width + ch_width > width && cur_width > 0 { rows.push(std::mem::take(&mut cur)); - cur_len = 0; + cur_width = 0; } cur.push(ch); - cur_len += 1; + cur_width += ch_width; } continue; } - let needed = if cur_len == 0 { - word_len + let needed = if cur_width == 0 { + word_width } else { - cur_len + 1 + word_len + cur_width + 1 + word_width }; if needed > width { rows.push(std::mem::take(&mut cur)); - cur_len = 0; + cur_width = 0; } - if cur_len > 0 { + if cur_width > 0 { cur.push(' '); - cur_len += 1; + cur_width += 1; } cur.push_str(word); - cur_len += word_len; + cur_width += word_width; } - if cur_len > 0 || rows.is_empty() { + if cur_width > 0 || rows.is_empty() { rows.push(cur); } rows } +/// Truncate `text` to `max_columns` display columns, appending `...` when cut. +/// +/// `truncate_str` counts chars, which is right for its existing callers but +/// would let a CJK value take twice its budget on the list row. +fn truncate_display(text: &str, max_columns: usize) -> String { + if display_width(text) <= max_columns { + return text.to_string(); + } + if max_columns <= 3 { + return ".".repeat(max_columns); + } + let budget = max_columns - 3; + let mut out = String::new(); + let mut width = 0usize; + let mut buf = [0u8; 4]; + for ch in text.chars() { + let ch_width = display_width(ch.encode_utf8(&mut buf)); + if width + ch_width > budget { + break; + } + out.push(ch); + width += ch_width; + } + out.push_str("..."); + out +} + /// Push `text` wrapped to `width`, with `prefix` on the first row and a matching /// indent on continuation rows so the label column stays aligned. fn push_wrapped( @@ -608,7 +647,7 @@ fn push_wrapped( text_style: Style, width: usize, ) { - let indent = prefix.chars().count(); + let indent = display_width(prefix); let available = width.saturating_sub(indent).max(1); for (i, row) in wrap_value(text, available).into_iter().enumerate() { let head = if i == 0 { @@ -633,7 +672,7 @@ fn hint_units(chunk: &PolicyChunk, t: &crate::theme::Theme, scrollable: bool) -> Span::styled(key.to_string(), key_style), Span::styled(label.to_string(), label_style), ], - key.chars().count() + label.chars().count(), + display_width(key) + display_width(label), )); }; match chunk.status.as_str() { @@ -940,6 +979,25 @@ mod tests { use ratatui::Terminal; use ratatui::backend::TestBackend; + /// Column width computed independently of the production `display_width`. + /// + /// Asserting with the helper under test would be tautological: if it + /// regressed to counting chars, the assertion would regress with it. + fn expected_columns(text: &str) -> usize { + text.chars() + .map(|c| { + let wide = ('\u{1100}'..='\u{115f}').contains(&c) + || ('\u{2e80}'..='\u{a4cf}').contains(&c) + || ('\u{ac00}'..='\u{d7a3}').contains(&c) + || ('\u{f900}'..='\u{faff}').contains(&c) + || ('\u{fe30}'..='\u{fe6f}').contains(&c) + || ('\u{ff00}'..='\u{ff60}').contains(&c) + || ('\u{ffe0}'..='\u{ffe6}').contains(&c); + usize::from(wide) + 1 + }) + .sum() + } + fn make_chunk(status: &str, rejection_reason: &str) -> PolicyChunk { PolicyChunk { status: status.to_string(), @@ -985,8 +1043,8 @@ mod tests { #[test] fn long_reason_truncates_for_the_list_row() { let reason = "rejected because the endpoint list is far too permissive"; - let shown = truncate_str(reason, 32); - assert_eq!(shown.chars().count(), 32); + let shown = truncate_display(reason, 32); + assert_eq!(display_width(&shown), 32); assert!(shown.ends_with("...")); assert!(shown.starts_with("rejected because")); } @@ -1020,10 +1078,47 @@ mod tests { fn wrap_value_never_exceeds_the_width() { let text = "reject this rule because the endpoint list is far too permissive"; for row in wrap_value(text, 17) { - assert!(row.chars().count() <= 17, "row too wide: {row:?}"); + assert!(display_width(&row) <= 17, "row too wide: {row:?}"); } } + #[test] + fn wrap_value_measures_display_columns_not_chars() { + // CJK glyphs occupy two columns each, and with no spaces the whole + // string takes the hard-break path. + let cjk = "这条规则的范围太广".repeat(20); + for row in wrap_value(&cjk, 40) { + assert!( + expected_columns(&row) <= 40, + "row is {} columns: {row:?}", + expected_columns(&row) + ); + } + + // Mixed script exercises the word-packing path instead. + let mixed = "scope 这条规则 to docs 路径 only ".repeat(20); + for row in wrap_value(&mixed, 33) { + assert!( + expected_columns(&row) <= 33, + "row is {} columns: {row:?}", + expected_columns(&row) + ); + } + } + + #[test] + fn truncate_display_counts_columns_not_chars() { + let cjk = "这条规则的范围太广".repeat(10); + let shown = truncate_display(&cjk, 32); + assert!( + expected_columns(&shown) <= 32, + "truncated value is {} columns", + expected_columns(&shown) + ); + assert!(shown.ends_with("...")); + assert_eq!(truncate_display("too broad", 32), "too broad"); + } + // --- rendering -------------------------------------------------------- fn render(chunk: &PolicyChunk, width: u16, height: u16, scroll: usize) -> (String, usize) { @@ -1144,6 +1239,44 @@ mod tests { assert!(bottom.contains("Close")); } + /// Double-width guidance must stay reachable too. Counting chars rather than + /// columns made every wrapped row about twice as wide as the popup, so the + /// right half of each row was clipped with nowhere to scroll. Markers are + /// interleaved through the text rather than appended, because a trailing + /// marker lands on its own short row either way and would not detect this. + #[test] + fn cjk_guidance_is_fully_reachable_at_80x24() { + use std::fmt::Write as _; + + const MARKERS: usize = 16; + let mut reason = String::new(); + for i in 0..MARKERS { + let _ = write!(reason, "M{i:02}"); + reason.push_str(&"这条规则范围太广".repeat(4)); + } + let chunk = PolicyChunk { + status: "rejected".to_string(), + rejection_reason: reason, + rule_name: "allow-github".to_string(), + ..Default::default() + }; + + let (_, max_scroll) = render(&chunk, 80, 24, 0); + assert!(max_scroll > 0, "content should overflow an 80x24 popup"); + + let mut seen = String::new(); + for scroll in 0..=max_scroll { + seen.push_str(&render(&chunk, 80, 24, scroll).0); + } + for i in 0..MARKERS { + let marker = format!("M{i:02}"); + assert!( + seen.contains(&marker), + "guidance marker {marker} was clipped and is unreachable at every scroll offset" + ); + } + } + #[test] fn short_content_does_not_scroll() { let chunk = PolicyChunk {