clawdie/colibri
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 44

fix(tui): make the "All sessions" aggregated view reachable

Browse source 
Once any pane carried a session_id, rebuild_session_list() forced
session_filter = Some(first), so the operator could never get back to the
aggregated "All sessions" view — Tab only cycled individual sessions.
Documented as a known bug in GLASSPANE-TUI-ENHANCEMENTS.md.

Model the session cycle as [All, s1, s2, ...]: index 0 is a synthetic "All
sessions" entry (filter = None), any other index scopes to sessions[i-1].
Two helpers encode the mapping:
  - session_count() = sessions.len() + 1 (All is always present)
  - apply_session_filter() maps session_idx -> filter (0 => None)

Behavior changes:
  - On connect, the operator now lands on "All sessions" (was: the
    alphabetically-first session). The aggregated view is the more useful
    default and is always reachable via Tab/BackTab.
  - The position indicator now shows for any cycle > 1 item, so the
    "All (1 of 2)" hint appears even with a single session.

self.sessions still holds real session ids only (no sentinel string), so the
sorted/deduped invariant is unchanged.

Tests:
  - rebuild_session_list_dedupes_and_sorts: updated for the new default +
    offset mapping (index 1 => s1, index 2 => s2).
  - all_sessions_view_is_reachable_with_sessions_present: new regression test
    covering connect-defaults-to-All and the Tab cycle All -> s1 -> s2 -> All.
  - attention_bar_ignores_other_session_panes: comment corrected (rebuild no
    longer selects the first session).

19/19 TUI tests pass; fmt + clippy (-D warnings) clean.

(Sam & Claude)
This commit is contained in:
Sam & Claude 2026-06-25 22:38:32 +02:00
parent 1b37a9ea9f
commit 0b9ea33ce9
1 changed files with 98 additions and 26 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

124
crates/colibri-glasspane-tui/src/main.rs
View file

@ -178,6 +178,23 @@ impl App {
}
}
/// Total items in the session cycle: the synthetic "All sessions" entry
/// plus one per real session id. Always >= 1 — "All" is always present.
fn session_count(&self) -> usize {
self.sessions.len() + 1
}
/// Map the current `session_idx` to a filter. Index 0 is the synthetic
/// "All sessions" aggregated view (filter = None); any other index scopes
/// to the session id at `index - 1`. Call after every `session_idx` change.
fn apply_session_filter(&mut self) {
self.session_filter = if self.session_idx == 0 {
None
} else {
self.sessions.get(self.session_idx - 1).cloned()
};
}
fn rebuild_session_list(&mut self) {
let snap = match &self.snapshot {
Some(s) => s,
@ -195,17 +212,15 @@ impl App {
.collect();
ids.sort();
ids.dedup();
if ids.is_empty() {
self.sessions.clear();
self.sessions = ids;
// The cycle is [All, s1, s2, ...] = sessions.len() + 1. Keep the
// previous selection when it still maps; otherwise fall back to "All".
// Default on first connect is "All sessions" (the aggregated view).
let count = self.session_count();
if self.session_idx >= count {
self.session_idx = 0;
self.session_filter = None;
} else {
self.sessions = ids;
if self.session_idx >= self.sessions.len() {
self.session_idx = self.sessions.len().saturating_sub(1);
}
self.session_filter = self.sessions.get(self.session_idx).cloned();
}
self.apply_session_filter();
}
async fn refresh(&mut self) {
@ -312,12 +327,12 @@ impl App {
Some(sid) => format!("Session: {sid}"),
None => "All sessions".to_string(),
};
let session_span = if self.sessions.len() > 1 {
let session_span = if self.session_count() > 1 {
Span::styled(
format!(
"{session_label} ({} of {})",
self.session_idx + 1,
self.sessions.len()
self.session_count()
),
Style::default().add_modifier(Modifier::BOLD),
)
@ -638,30 +653,24 @@ async fn run(socket_path: PathBuf) -> io::Result<()> {
}
}
KeyCode::Tab | KeyCode::Char('\t') if !app.sessions.is_empty() => {
app.session_idx = (app.session_idx + 1) % app.sessions.len();
app.session_filter = app.sessions.get(app.session_idx).cloned();
let count = app.session_count();
app.session_idx = (app.session_idx + 1) % count;
app.apply_session_filter();
app.table_state.select(Some(0));
app.detail_pane = None;
app.set_status(format!(
"session {}/{}",
app.session_idx + 1,
app.sessions.len()
));
app.set_status(format!("session {}/{}", app.session_idx + 1, count));
}
KeyCode::BackTab if !app.sessions.is_empty() => {
let count = app.session_count();
app.session_idx = if app.session_idx == 0 {
app.sessions.len() - 1
count - 1
} else {
app.session_idx - 1
};
app.session_filter = app.sessions.get(app.session_idx).cloned();
app.apply_session_filter();
app.table_state.select(Some(0));
app.detail_pane = None;
app.set_status(format!(
"session {}/{}",
app.session_idx + 1,
app.sessions.len()
));
app.set_status(format!("session {}/{}", app.session_idx + 1, count));
}
KeyCode::Down | KeyCode::Char('j') => {
let count = app.filtered_panes().len();
@ -846,8 +855,71 @@ mod tests {
let mut app = App::new(PathBuf::from("/tmp/nonexistent.sock"));
app.snapshot = Some(snap);
app.rebuild_session_list();
// Real session ids are sorted + deduped (s1 appeared twice).
assert_eq!(app.sessions, vec!["s1", "s2"]);
// Default selection is "All sessions" (index 0) — the aggregated view
// stays reachable. Regression for the "All sessions unreachable" bug.
assert_eq!(app.session_idx, 0);
assert!(app.session_filter.is_none());
assert_eq!(app.session_count(), 3); // [All, s1, s2]
// Index 1 -> s1, index 2 -> s2.
app.session_idx = 1;
app.apply_session_filter();
assert_eq!(app.session_filter.as_deref(), Some("s1"));
app.session_idx = 2;
app.apply_session_filter();
assert_eq!(app.session_filter.as_deref(), Some("s2"));
}
#[test]
fn all_sessions_view_is_reachable_with_sessions_present() {
// Regression for the pre-existing bug documented in
// GLASSPANE-TUI-ENHANCEMENTS.md: once any pane had a session_id,
// rebuild_session_list() forced session_filter = Some(first), making
// the aggregated "All sessions" view unreachable. It must now default
// to All and stay selectable via Tab.
let snap = GlasspaneSnapshot::new(
"osa",
"2026-06-25T12:00:00Z",
vec![
colibri_glasspane::Pane {
id: "a".into(),
agent: "zot".into(),
state: AgentState::Working,
session_id: Some("s1".into()),
last_event_at: None,
cwd: None,
stalled: false,
},
colibri_glasspane::Pane {
id: "b".into(),
agent: "zot".into(),
state: AgentState::Working,
session_id: Some("s2".into()),
last_event_at: None,
cwd: None,
stalled: false,
},
],
);
let mut app = App::new(PathBuf::from("/tmp/nonexistent.sock"));
app.snapshot = Some(snap);
app.rebuild_session_list();
// After connect, the operator lands on the aggregated view.
assert_eq!(app.session_idx, 0);
assert!(app.session_filter.is_none());
assert_eq!(app.filtered_panes().len(), 2);
// Tab cycles All -> s1 -> s2 -> All.
let count = app.session_count();
app.session_idx = (app.session_idx + 1) % count;
app.apply_session_filter();
assert_eq!(app.session_filter.as_deref(), Some("s1"));
app.session_idx = (app.session_idx + 1) % count;
app.apply_session_filter();
assert_eq!(app.session_filter.as_deref(), Some("s2"));
app.session_idx = (app.session_idx + 1) % count;
app.apply_session_filter();
assert!(app.session_filter.is_none(), "wrap back to All sessions");
}
// ── render tests (TestBackend) ──
@ -1341,7 +1413,7 @@ mod tests {
let mut app = App::new(PathBuf::from("/tmp/nonexistent.sock"));
app.snapshot = Some(snap);
app.rebuild_session_list();
// Navigate to session s1 (rebuild selects first alphabetically = s1).
// rebuild now defaults to "All sessions"; force the view to session s1.
app.session_filter = Some("s1".into());
let text = render_text(&mut app, 80, 24);