diff --git a/crates/cli/src/app.rs b/crates/cli/src/app.rs index 71cdc0f4..1e1df9ff 100644 --- a/crates/cli/src/app.rs +++ b/crates/cli/src/app.rs @@ -602,6 +602,12 @@ pub(crate) fn list_archive_indent_cells(nesting_depth: usize) -> u16 { list_session_indent_cells(nesting_depth) } +/// Blank rows that stand above a project header in full mode. Deliberately +/// more than the single row separating two sibling session cards: at one row +/// a project reads as another item in the stream, and the thing it needs to +/// say is "a new section starts here". +pub(crate) const PROJECT_MARGIN_ROWS: usize = 2; + impl ListItem { pub fn matches(&self, sel: &Selection) -> bool { match (self, sel) { @@ -11667,14 +11673,53 @@ impl App { } /// A list item's display height under the current view mode: full-mode - /// session cards take three rows, everything else (group headers, archived - /// disclosure rows, every compact-mode row) takes one. - pub(crate) fn list_item_display_height(&self, item: &ListItem) -> usize { - match item { - ListItem::Service { .. } => 1, - ListItem::Session { .. } if self.list_mode == SessionListViewMode::Full => 3, - _ => 1, + /// session cards take three rows, full-mode rows that head a subtree + /// (project headers, services) take two, and everything else (archived + /// disclosure rows, every compact-mode row) takes one. A project header + /// also carries its leading margin (see + /// [`Self::list_project_margin_rows`]), so the rows above it belong to + /// the item they set apart. + pub(crate) fn list_item_display_height(&self, items: &[ListItem], idx: usize) -> usize { + if self.list_mode != SessionListViewMode::Full { + return 1; + } + match &items[idx] { + ListItem::Session { .. } => 3, + // A project header or a service heads a subtree, so full mode + // gives it the same closing rail/breathing row a session card + // gets. Archived-disclosure rows head nothing and stay flat. + ListItem::GroupHeader { .. } => 2 + self.list_project_margin_rows(items, idx), + ListItem::Service { .. } => 2, + ListItem::ArchivedRow { .. } => 1, + } + } + + /// Blank rows full mode inserts above a project header so it reads as a + /// section break rather than one more row in the stream. + /// + /// The gap is NORMALIZED, not additive: rows already trailing the item + /// above count toward it, so every project sits under the same amount of + /// air no matter what precedes it. A session card, a project header and a + /// service each end in one blank/rail row and so need one more; an + /// archived-disclosure row ends in nothing and needs the full gap. A + /// project opening the list needs none — there is nothing above to be set + /// apart from. + pub(crate) fn list_project_margin_rows(&self, items: &[ListItem], idx: usize) -> usize { + if self.list_mode != SessionListViewMode::Full + || !matches!(items[idx], ListItem::GroupHeader { .. }) + { + return 0; } + let Some(prev) = idx.checked_sub(1).map(|p| &items[p]) else { + return 0; + }; + let trailing_blank = match prev { + ListItem::Session { .. } + | ListItem::GroupHeader { .. } + | ListItem::Service { .. } => 1, + ListItem::ArchivedRow { .. } => 0, + }; + PROJECT_MARGIN_ROWS.saturating_sub(trailing_blank) } /// Largest useful `list_scroll_offset`: the smallest offset that still @@ -11683,8 +11728,8 @@ impl App { /// this reduces to `len − visible_rows`. pub(crate) fn list_max_scroll(&self, items: &[ListItem], visible_rows: usize) -> usize { let mut rows = 0usize; - for (idx, item) in items.iter().enumerate().rev() { - rows += self.list_item_display_height(item); + for idx in (0..items.len()).rev() { + rows += self.list_item_display_height(items, idx); if rows > visible_rows { return idx + 1; } @@ -11718,8 +11763,8 @@ impl App { // viewport pins to itself rather than scrolling past. let mut rows = 0usize; let mut offset = target_idx; - for (idx, item) in items[..=target_idx].iter().enumerate().rev() { - rows += self.list_item_display_height(item); + for idx in (0..=target_idx).rev() { + rows += self.list_item_display_height(items, idx); if rows > visible_rows { break; } @@ -44425,6 +44470,234 @@ mod tests { assert_eq!(app.selection, Selection::Group("project".into())); } + /// Full mode gives a project header the rail/breathing row a session card + /// gets, so the header reads as the root its members hang from instead of + /// sitting flush against the first one. Compact mode keeps every row flat. + #[tokio::test] + async fn full_mode_project_header_caps_its_member_rails() { + let (mut app, _dir, _server) = empty_app().await; + let mut member = summary_with_kind(construct_protocol::SessionKind::User); + member.id = "member".into(); + member.title = Some("member one".into()); + member.group_id = Some("p1".into()); + app.sessions = vec![member]; + app.groups = vec![GroupSummary { + id: "p1".into(), + name: "Project One".into(), + created_at: chrono::Utc::now(), + position: 0, + collapsed: false, + }]; + app.matrix_rain_hidden = true; + + let items = app.list_items(); + let header = &items[0]; + assert!(matches!(header, ListItem::GroupHeader { .. })); + + let backend = ratatui::backend::TestBackend::new(60, 20); + let mut term = ratatui::Terminal::new(backend).expect("terminal"); + let row_text = |term: &ratatui::Terminal, y: u16| { + (0..term.backend().buffer().area.width) + .map(|x| { + term.backend() + .buffer() + .cell((x, y)) + .map(|cell| cell.symbol()) + .unwrap_or(" ") + }) + .collect::() + }; + + app.list_mode = SessionListViewMode::Compact; + assert_eq!(app.list_item_display_height(&items, 0), 1); + term.draw(|frame| crate::ui::render(frame, &mut app)) + .expect("render compact"); + let top = app.layout.list_items_area.expect("list rows").y; + assert!(row_text(&term, top).contains("Project One")); + // Compact packs rows: the member follows the header immediately. + assert!(row_text(&term, top + 1).contains("member one")); + + app.list_mode = SessionListViewMode::Full; + // Opening the list, this project has nothing above it to be set apart + // from, so it carries no margin and stands at its bare two rows. + assert_eq!(app.list_project_margin_rows(&items, 0), 0); + assert_eq!(app.list_item_display_height(&items, 0), 2); + // Services head a depth-1 subtree the same way and earn the same row. + let service_only = vec![ListItem::Service { + summary: service_summary_for_test("svc"), + session_count: 1, + sessions_expanded: true, + }]; + assert_eq!(app.list_item_display_height(&service_only, 0), 2); + term.draw(|frame| crate::ui::render(frame, &mut app)) + .expect("render full"); + let top = app.layout.list_items_area.expect("list rows").y; + assert!(row_text(&term, top).contains("Project One")); + // The header's own row carries the stem down to the first member, + // which now starts one row later than it did in compact mode. + let rail = row_text(&term, top + 1); + assert!( + rail.contains('│') && !rail.contains("member one"), + "expected a rail-only row under the header, got {rail:?}" + ); + assert!(row_text(&term, top + 2).contains("member one")); + } + + /// The gap above a project is NORMALIZED, not additive: whatever the item + /// above it happens to leave behind, a project always opens on two rows of + /// air. An archived disclosure leaves none and a session card leaves one, + /// so the same visual break costs a different number of rows depending on + /// the neighbor — which is the whole point of measuring it here instead of + /// stapling a constant onto every header. + #[tokio::test] + async fn project_margin_normalizes_the_gap_above_a_header() { + let (mut app, _dir, _server) = empty_app().await; + let header = |id: &str| ListItem::GroupHeader { + group: GroupSummary { + id: id.into(), + name: format!("Project {id}"), + created_at: chrono::Utc::now(), + position: 0, + collapsed: false, + }, + member_count: 1, + attention_rollup: false, + }; + let session = || ListItem::Session { + summary: summary_with_kind(construct_protocol::SessionKind::User), + nesting_depth: 0, + has_children: false, + children_expanded: false, + attention_rollup: false, + }; + let archived = || ListItem::ArchivedRow { + section: ArchiveSection::Ungrouped, + count: 2, + expanded: false, + nesting_depth: 1, + }; + let service = || ListItem::Service { + summary: service_summary_for_test("svc"), + session_count: 1, + sessions_expanded: true, + }; + + let items = vec![ + header("a"), // 0: opens the list + session(), // 1 + header("b"), // 2: after a session card + archived(), // 3 + header("c"), // 4: after an archived disclosure + service(), // 5 + header("d"), // 6: after a service header + header("e"), // 7: after another project + ]; + + app.list_mode = SessionListViewMode::Full; + // A project that opens the list has nothing to be set apart from. + assert_eq!(app.list_project_margin_rows(&items, 0), 0); + assert_eq!(app.list_item_display_height(&items, 0), 2); + // Cards, service headers, and project headers all end in one blank + // row, so one more row tops the gap up to two. + for idx in [2usize, 6, 7] { + assert_eq!( + app.list_project_margin_rows(&items, idx), + 1, + "item {idx} follows a row that already breathes once" + ); + assert_eq!(app.list_item_display_height(&items, idx), 3); + } + // An archived disclosure ends flush against what follows, so the + // project below it has to supply both rows itself. + assert_eq!(app.list_project_margin_rows(&items, 4), 2); + assert_eq!(app.list_item_display_height(&items, 4), 4); + // Nothing but a project earns the margin. + for idx in [1usize, 3, 5] { + assert_eq!(app.list_project_margin_rows(&items, idx), 0); + } + + // Compact mode is packed by design: every item is one row, margin + // included. + app.list_mode = SessionListViewMode::Compact; + for idx in 0..items.len() { + assert_eq!(app.list_project_margin_rows(&items, idx), 0); + assert_eq!(app.list_item_display_height(&items, idx), 1); + } + } + + /// The margin is measured as part of the header's height but rendered as + /// its own widget row. Two things have to hold at once: the blank rows + /// really appear above the title, and the header's gutter affordances stay + /// pinned to its title row rather than sliding up into the air above it. + #[tokio::test] + async fn project_margin_renders_above_the_header_without_moving_its_first_line() { + let (mut app, _dir, _server) = empty_app().await; + let mut loose = summary_with_kind(construct_protocol::SessionKind::User); + loose.id = "loose".into(); + loose.title = Some("loose session".into()); + let mut member = summary_with_kind(construct_protocol::SessionKind::User); + member.id = "member".into(); + member.title = Some("member one".into()); + member.group_id = Some("p1".into()); + app.sessions = vec![loose, member]; + app.groups = vec![GroupSummary { + id: "p1".into(), + name: "Project One".into(), + created_at: chrono::Utc::now(), + position: 0, + collapsed: false, + }]; + app.matrix_rain_hidden = true; + app.list_mode = SessionListViewMode::Full; + + let items = app.list_items(); + let header_idx = items + .iter() + .position(|i| matches!(i, ListItem::GroupHeader { .. })) + .expect("a project header"); + assert_eq!(header_idx, 1, "the ungrouped session renders above it"); + assert_eq!(app.list_project_margin_rows(&items, header_idx), 1); + + let backend = ratatui::backend::TestBackend::new(60, 20); + let mut term = ratatui::Terminal::new(backend).expect("terminal"); + term.draw(|frame| crate::ui::render(frame, &mut app)) + .expect("render full"); + // Read only the list's own columns — the pane beside it is busy. + let area = app.layout.list_items_area.expect("list rows"); + let row_text = |y: u16| { + (area.x..area.x + area.width) + .map(|x| { + term.backend() + .buffer() + .cell((x, y)) + .map(|cell| cell.symbol()) + .unwrap_or(" ") + }) + .collect::() + }; + let top = area.y; + + assert!(row_text(top).contains("loose session")); + // Rows 2 and 3 are the card's own trailing row plus the project's + // margin: two rows of air, however they were paid for. + assert!( + row_text(top + 2).trim().is_empty() && row_text(top + 3).trim().is_empty(), + "expected two blank rows above the project, got {:?} / {:?}", + row_text(top + 2), + row_text(top + 3) + ); + assert!(row_text(top + 4).contains("Project One")); + assert!(row_text(top + 6).contains("member one")); + + // The margin belongs to the project — clicking that air selects it — + // but only its title row answers as the header's first line. + let hits = &app.layout.list_visible_rows; + assert_eq!(hits[3].item_index, header_idx); + assert!(!hits[3].first_line, "the gap is not the header's first line"); + assert_eq!(hits[4].item_index, header_idx); + assert!(hits[4].first_line, "the title row is"); + } + #[tokio::test] async fn archived_section_resolves_only_its_archived_members() { use construct_client::Client; diff --git a/crates/cli/src/ui.rs b/crates/cli/src/ui.rs index 9cc91d13..d57a9c56 100644 --- a/crates/cli/src/ui.rs +++ b/crates/cli/src/ui.rs @@ -2368,6 +2368,31 @@ fn list_tree_continuation_prefix( prefix } +/// Full mode's closing row for a subtree-heading row (a project header or +/// a service). Sessions end their card with a breathing row that keeps the +/// rails continuous; a header owns a depth-1 subtree the same way a parent +/// session does, so it earns the same row — otherwise it sits flush against +/// its first member while every card around it floats, and its members' +/// rails begin in mid-air with nothing above them to hang from. +/// +/// Only one rail level can matter: headers always sit at depth 0 and their +/// members at depth 1. When the header is collapsed or empty the next row +/// isn't a descendant, so this is a plain breathing row. +fn list_header_continuation_line( + theme: &Theme, + items: &[AppListItem], + item_index: usize, +) -> Line<'static> { + Line::from(Span::styled( + list_tree_continuation_prefix(items, item_index, 0, LIST_HEADER_RAIL_W), + session_list_secondary_style(theme), + )) +} + +/// Cells the header's continuation row spends: exactly the one depth step a +/// depth-0 row can own. +const LIST_HEADER_RAIL_W: usize = 2; + /// Ceiling on the detail line's model column, so one verbose model id /// (`codex-oauth:gpt-5.6-sol`) can't push every other column off a /// narrow sidebar. @@ -3205,13 +3230,17 @@ fn render_sessions(f: &mut Frame, area: Rect, app: &mut App) { } else { Style::default().fg(app.theme.text) }; - vec![Line::from(vec![ + let mut lines = vec![Line::from(vec![ Span::styled(disclosure, Style::default().fg(app.theme.group)), Span::styled("⛓︎ ", Style::default().fg(app.theme.accent)), Span::styled(name, name_style), Span::raw(" ".repeat(gap)), Span::styled(suffix, session_list_secondary_style(&app.theme)), - ])] + ])]; + if full_mode { + lines.push(list_header_continuation_line(&app.theme, &app_items, i)); + } + lines } AppListItem::Session { summary: s, @@ -3346,7 +3375,7 @@ fn render_sessions(f: &mut Frame, area: Rect, app: &mut App) { attention_rollup, } => { let glyph = if group.collapsed { "▶" } else { "▼" }; - vec![Line::from(vec![ + let mut lines = vec![Line::from(vec![ Span::styled(format!("{glyph} "), Style::default().fg(app.theme.group)), Span::styled(group.name.clone(), group_name_style(&app.theme)), Span::styled( @@ -3358,7 +3387,11 @@ fn render_sessions(f: &mut Frame, area: Rect, app: &mut App) { format!("({member_count})"), session_list_secondary_style(&app.theme), ), - ])] + ])]; + if full_mode { + lines.push(list_header_continuation_line(&app.theme, &app_items, i)); + } + lines } AppListItem::ArchivedRow { section, @@ -3458,15 +3491,20 @@ fn render_sessions(f: &mut Frame, area: Rect, app: &mut App) { let mut visible_end = visible_start; let mut used_rows = 0usize; app.layout.list_visible_rows.clear(); - for (i, item) in app_items.iter().enumerate().skip(visible_start) { - let h = app.list_item_display_height(item); + for i in visible_start..app_items.len() { + let h = app.list_item_display_height(&app_items, i); if visible_end > visible_start && used_rows + h > list_items_area.height as usize { break; } + // A project's leading margin belongs to the project it sets apart, so + // clicking that air selects it. The gutter affordances still live on + // the header's own first line, which is `margin` rows down — not on + // the first row of the gap. + let margin = app.list_project_margin_rows(&app_items, i); for line_no in 0..h { app.layout.list_visible_rows.push(crate::app::ListRowHit { item_index: i, - first_line: line_no == 0, + first_line: line_no == margin, }); } used_rows += h; @@ -3475,22 +3513,29 @@ fn render_sessions(f: &mut Frame, area: Rect, app: &mut App) { app.layout .list_visible_rows .truncate(list_items_area.height as usize); - let selected_visible_idx = selected_idx.and_then(|idx| { - if idx >= visible_start && idx < visible_end { - Some(idx - visible_start) - } else { - None - } - }); - let items: Vec = item_lines[visible_start..visible_end] - .iter() - .map(|lines| ListItem::new(lines.clone())) - .collect(); + // A project's leading margin is rendered as its own widget item rather + // than as extra lines on the header, so the selection highlight stops at + // the header's own rows instead of painting the gap above it as a block. + // The rows are still counted with the header by + // `list_item_display_height`, so this splits only what the widget draws, + // never the geometry the hit map and scrolling agree on. + let mut items: Vec = Vec::with_capacity(visible_end - visible_start); + let mut selected_widget_idx: Option = None; + for i in visible_start..visible_end { + let margin = app.list_project_margin_rows(&app_items, i); + if margin > 0 { + items.push(ListItem::new(vec![Line::raw(""); margin])); + } + if selected_idx == Some(i) { + selected_widget_idx = Some(items.len()); + } + items.push(ListItem::new(item_lines[i].clone())); + } let mut state = ListState::default(); state.select(if matches!(app.selection, Selection::None) { None } else { - selected_visible_idx + selected_widget_idx }); f.render_widget(block, area); let list = List::new(items).highlight_style(highlight_style); @@ -3523,9 +3568,8 @@ fn render_sessions(f: &mut Frame, area: Rect, app: &mut App) { if show_list_scrollbar && max_scroll > 0 && list_items_area.width > 0 { let track_h = list_items_area.height as usize; if track_h > 0 { - let total_rows: usize = app_items - .iter() - .map(|item| app.list_item_display_height(item)) + let total_rows: usize = (0..app_items.len()) + .map(|i| app.list_item_display_height(&app_items, i)) .sum(); let thumb_h = (track_h * track_h / total_rows.max(1)).clamp(1, track_h); let max_top = track_h - thumb_h; @@ -3572,8 +3616,12 @@ fn render_sessions(f: &mut Frame, area: Rect, app: &mut App) { // the harness width already computed there, mirroring the same tradeoff // `hovered_diamond` already makes for its own hit zone. let mut hit_y = list_items_area.y; - for item in app_items[visible_start..visible_end].iter() { - if let AppListItem::Session { summary, .. } = item { + for i in visible_start..visible_end { + // The label sits on the item's own first line, which a leading margin + // pushes down. Sessions never carry one, but the offset is applied + // here rather than assumed away. + let margin = app.list_project_margin_rows(&app_items, i) as u16; + if let AppListItem::Session { summary, .. } = &app_items[i] { let harness_w = harness_label(summary).chars().count() as u16; let x_end = list_items_area.x + list_items_area.width; app.layout @@ -3582,10 +3630,10 @@ fn render_sessions(f: &mut Frame, area: Rect, app: &mut App) { session_id: summary.id.clone(), x_start: x_end.saturating_sub(harness_w), x_end, - y: hit_y, + y: hit_y.saturating_add(margin), }); } - hit_y = hit_y.saturating_add(app.list_item_display_height(item) as u16); + hit_y = hit_y.saturating_add(app.list_item_display_height(&app_items, i) as u16); } clear_pane_side_borders(f, area, app); if let (Some(rect), Some((id, mut rows))) = (lineage_rect, lineage) { diff --git a/specs/0106-session-list-view-modes.md b/specs/0106-session-list-view-modes.md index a1b49285..1295e3b0 100644 --- a/specs/0106-session-list-view-modes.md +++ b/specs/0106-session-list-view-modes.md @@ -1,7 +1,7 @@ # 0106-session-list-view-modes Status: accepted -Date: 2026-08-01 +Date: 2026-08-02 Area: tui Scope: The sidebar session list offers a compact one-line view and a vertically spaced full card view per session, toggled from the pane's border and persisted per user. @@ -54,7 +54,32 @@ Rules both modes must preserve: wrapping, least important first: tokens, then activity, then identity, keeping the context gauge longest — and identically on every row. Full mode never forces the sidebar wider and never horizontally scrolls. -- Group headers and archived-disclosure rows stay one line in both modes. +- A row that heads a subtree — a project header, a service — carries no + detail line, but in full mode it gains the same closing rail/breathing row + a session card ends with, drawing the stem down into its first member. Its + members hang off rails; without that row those rails begin in mid-air, and + the header alone would sit flush against its first member while every card + around it floats. When the header is collapsed or has no members the row is + blank, still separating it from what follows. Archived-disclosure rows head + nothing and stay one line in both modes; no header gains a detail line, in + either mode. +- In full mode a project header also opens on a fixed margin of blank rows, + wider than the single row separating two sibling cards: at one row a project + reads as another item in the stream, and what it has to say is "a new + section starts here". The margin is NORMALIZED, not additive — it counts + whatever blank row the item above already leaves behind (a card and a header + leave one; an archived-disclosure row leaves none) and supplies only the + difference, so every project in the list opens on the same amount of air + regardless of its neighbor. A project at the top of the list has nothing to + be set apart from and carries no margin. Compact mode packs its rows with + nothing between them and gains no margin, and services — which head a + subtree but not a section — do not take one. +- That margin belongs to the project it sets apart: it counts toward the + header's measured height, so clicking the gap selects the project and + scrolling accounts for it, but it renders as its own row rather than as + part of the header, so selecting a project never highlights the air above + it, and the header's first line — where its gutter affordances live — + remains its title row. - Session nesting uses the same two-cell indentation step in both modes. Each generation gets its own depth, including fork-of-fork, subagent-of-subagent, and mixed trees; the hierarchy never changes the user's depth-first session @@ -100,7 +125,12 @@ lineage section set for a full/compact pair. (hover zones, drag targets, new gutter affordances) must go through the row-to-item mapping and declare which line of a card it lives on. - Scroll limits and scrollbar geometry are measured in display rows, so - mixed-height items (one-line headers among three-row cards) stay correct. + mixed-height items (one-line disclosure rows and headers whose height + depends on the item above them, among three-row cards) stay correct. +- An item's height can depend on its neighbor, so nothing may assume a kind + of item has a fixed height, and any new spacing rule has to be expressed in + the same measurement the hit map and scroll geometry read — spacing painted + only at render time would desynchronize them. - Adding new per-session data to the detail line means placing it in the existing drop-priority order, not appending unconditionally.