Skip to content

fix(tui/mcp): keep menu selection in view #366

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zemaj merged 3 commits into from
Oct 29, 2025
Merged

fix(tui/mcp): keep menu selection in view #366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9f6481a
fix(rollout): sort sessions by mtime first
zemaj Oct 28, 2025
4c11475
fix(tui/mcp): keep selected row visible
zemaj Oct 28, 2025
8ea20c7
ci: retrigger preview build after #364
zemaj Oct 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions code-rs/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions code-rs/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ env-flags = "0.1.1"
env_logger = "0.11.5"
escargot = "0.5"
eventsource-stream = "0.2.3"
filetime = "0.2"
futures = "0.3"
icu_decimal = "2.0.0"
icu_locale_core = "2.0.0"
Expand Down
2 changes: 2 additions & 0 deletions code-rs/core/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ windows-sys = { version = "0.61.2", features = [
] }

[dev-dependencies]
filetime = { workspace = true }
maplit = { workspace = true }
once_cell = { workspace = true }
pretty_assertions = { workspace = true }
tokio-test = { workspace = true }
wiremock = { workspace = true }
Expand Down
61 changes: 46 additions & 15 deletions code-rs/core/src/rollout/list.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use time::OffsetDateTime;
use time::PrimitiveDateTime;
use time::format_description::FormatItem;
use time::macros::format_description;
use time::format_description::well_known::Rfc3339;
use uuid::Uuid;

use super::SESSIONS_SUBDIR;
Expand Down Expand Up @@ -47,6 +48,8 @@ pub struct ConversationItem {
pub created_at: Option<String>,
/// RFC3339 timestamp string for the most recent response in the tail, if available.
pub updated_at: Option<String>,
/// RFC3339 timestamp string for the file's last modification time, if available.
pub modified_at: Option<String>,
}

#[derive(Default)]
Expand Down Expand Up @@ -147,7 +150,7 @@ async fn traverse_directories_for_paths(
anchor: Option<Cursor>,
allowed_sources: &[SessionSource],
) -> io::Result<ConversationsPage> {
let mut items: Vec<ConversationItem> = Vec::with_capacity(page_size);
let mut candidates: Vec<(OffsetDateTime, OffsetDateTime, Uuid, ConversationItem)> = Vec::new();
let mut scanned_files = 0usize;
let mut anchor_passed = anchor.is_none();
let (anchor_ts, anchor_id) = match anchor {
Expand All @@ -171,7 +174,7 @@ async fn traverse_directories_for_paths(
if scanned_files >= MAX_SCAN_FILES {
break 'outer;
}
let mut day_files = collect_files(day_path, |name_str, path| {
let day_files = collect_files(day_path, |name_str, path| {
if !name_str.starts_with("rollout-") || !name_str.ends_with(".jsonl") {
return None;
}
Expand All @@ -180,11 +183,22 @@ async fn traverse_directories_for_paths(
.map(|(ts, id)| (ts, id, name_str.to_string(), path.to_path_buf()))
})
.await?;
// Stable ordering within the same second: (timestamp desc, uuid desc)
day_files.sort_by_key(|(ts, sid, _name_str, _path)| (Reverse(*ts), Reverse(*sid)));
let mut day_entries = Vec::with_capacity(day_files.len());
for (ts, sid, _name_str, path) in day_files.into_iter() {
let modified = tokio::fs::metadata(&path)
.await
.ok()
.and_then(|meta| meta.modified().ok())
.map(OffsetDateTime::from)
.unwrap_or(OffsetDateTime::UNIX_EPOCH);
day_entries.push((modified, ts, sid, path));
}
day_entries.sort_by_key(|(modified, ts, sid, _)| {
(Reverse(*modified), Reverse(*ts), Reverse(*sid))
});
for (modified, ts, sid, path) in day_entries.into_iter() {
scanned_files += 1;
if scanned_files >= MAX_SCAN_FILES && items.len() >= page_size {
if scanned_files >= MAX_SCAN_FILES {
break 'outer;
}
if !anchor_passed {
Expand All @@ -194,9 +208,6 @@ async fn traverse_directories_for_paths(
continue;
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Align cursor with mtime-based ordering

Traversal now ranks conversations by filesystem modification time, but the pagination cursor and skip logic still compare only the timestamp encoded in the filename. The loop marks anchor_passed once it sees a record with ts < anchor_ts; any files with a newer timestamp but an older mtime are skipped before this happens and will never be yielded on subsequent pages. For example, with three files (mtime 20, ts Aug), (mtime 15, ts Jul), (mtime 10, ts Sep), requesting the Aug entry first means the Sep session can never appear on later pages. Pagination will drop or reorder sessions unless the cursor also tracks the mtime (or the anchor filter is applied after sorting by mtime).

Useful? React with 👍 / 👎.

}
}
if items.len() == page_size {
break 'outer;
}
let summary = read_head_and_tail(&path, HEAD_RECORD_LIMIT, TAIL_RECORD_LIMIT)
.await
.unwrap_or_default();
Expand All @@ -216,19 +227,39 @@ async fn traverse_directories_for_paths(
..
} = summary;
updated_at = updated_at.or_else(|| created_at.clone());
items.push(ConversationItem {
path,
head,
tail,
created_at,
updated_at,
});
let modified_at = if modified == OffsetDateTime::UNIX_EPOCH {
updated_at.clone().or_else(|| created_at.clone())
} else {
modified.format(&Rfc3339).ok()
};
candidates.push((
modified,
ts,
sid,
ConversationItem {
path,
head,
tail,
created_at,
updated_at,
modified_at,
},
));
}
}
}
}
}

candidates.sort_by_key(|(modified, ts, sid, _)| {
(Reverse(*modified), Reverse(*ts), Reverse(*sid))
});

let mut items: Vec<ConversationItem> = Vec::with_capacity(page_size.min(candidates.len()));
for (_, _, _, item) in candidates.into_iter().take(page_size) {
items.push(item);
}

let next = build_next_cursor(&items);
Ok(ConversationsPage {
items,
Expand Down
68 changes: 67 additions & 1 deletion code-rs/core/src/rollout/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ use std::io::BufWriter;
use std::io::Write;
use std::path::{Path, PathBuf};

use filetime::{set_file_mtime, FileTime};
use tempfile::TempDir;
use time::OffsetDateTime;
use time::PrimitiveDateTime;
use time::format_description::FormatItem;
use time::format_description::well_known::Rfc3339;
use time::macros::format_description;
use uuid::Uuid;

Expand All @@ -19,6 +21,7 @@ use crate::rollout::list::ConversationsPage;
use crate::rollout::list::Cursor;
use crate::rollout::list::get_conversation;
use crate::rollout::list::get_conversations;
use std::time::{Duration, SystemTime};
use code_protocol::models::{ContentItem, ResponseItem};
use code_protocol::ConversationId;
use code_protocol::protocol::{
Expand Down Expand Up @@ -426,7 +429,7 @@ async fn test_pagination_cursor() {
];
let expected_cursor1: Cursor =
serde_json::from_str(&format!("\"2025-03-04T09-00-00|{u4}\"")).unwrap();
assert_page_summary(&page1, &expected_page1_items, Some(expected_cursor1.clone()), 3);
assert_page_summary(&page1, &expected_page1_items, Some(expected_cursor1.clone()), 5);

let page2 = get_conversations(
home,
Expand Down Expand Up @@ -524,6 +527,69 @@ async fn test_get_conversation_contents() {
}
}

#[tokio::test]
async fn test_list_conversations_prefers_recent_mtime() {
let temp = TempDir::new().unwrap();
let home = temp.path();

let older_uuid = Uuid::from_u128(0x11);
let newer_uuid = Uuid::from_u128(0x22);

write_session_file(
home,
"2025-08-01T10-00-00",
older_uuid,
1,
Some(SessionSource::VSCode),
)
.unwrap();
write_session_file(
home,
"2025-09-01T10-00-00",
newer_uuid,
1,
Some(SessionSource::VSCode),
)
.unwrap();

let older_path = home
.join("sessions")
.join("2025")
.join("08")
.join("01")
.join(format!("rollout-2025-08-01T10-00-00-{older_uuid}.jsonl"));

let future_time = SystemTime::now() + Duration::from_secs(300);
set_file_mtime(&older_path, FileTime::from_system_time(future_time)).unwrap();

let page = get_conversations(home, 1, None, INTERACTIVE_SESSION_SOURCES)
.await
.unwrap();

let first = page.items.first().expect("expected at least one session");
assert_eq!(first.path, older_path);

let modified_at = first
.modified_at
.as_deref()
.expect("expected modified_at");
let updated_at = first
.updated_at
.as_deref()
.expect("expected updated_at");

let modified_dt = OffsetDateTime::parse(modified_at, &Rfc3339).expect("parse modified_at");
let format: &[FormatItem] =
format_description!("[year]-[month]-[day]T[hour]-[minute]-[second]");
let updated_dt = PrimitiveDateTime::parse(updated_at, format)
.expect("parse updated_at")
.assume_utc();
assert!(
modified_dt > updated_dt,
"modified_at should reflect newer filesystem mtime"
);
}

#[tokio::test]
async fn test_stable_ordering_same_second_pagination() {
let temp = TempDir::new().unwrap();
Expand Down
31 changes: 30 additions & 1 deletion code-rs/tui/src/bottom_pane/mcp_settings_view.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ impl<'a> BottomPaneView<'a> for McpSettingsView {
block.render(area, buf);

let mut lines: Vec<Line<'static>> = Vec::new();
let mut selected_line_index: usize = 0;

if self.rows.is_empty() {
lines.push(Line::from(vec![Span::styled("No MCP servers configured.", Style::default().fg(crate::colors::text_dim()))]));
lines.push(Line::from(""));
Expand All @@ -123,6 +125,7 @@ impl<'a> BottomPaneView<'a> for McpSettingsView {
let check = if row.enabled { "[on ]" } else { "[off]" };
let name = format!("{} {}", check, row.name);
let name_style = if sel { Style::default().bg(crate::colors::selection()).add_modifier(Modifier::BOLD) } else { Style::default() };
let arrow_line_index = lines.len();
lines.push(Line::from(vec![
Span::styled(if sel { "› " } else { " " }, Style::default()),
Span::styled(name, name_style),
Expand All @@ -133,18 +136,29 @@ impl<'a> BottomPaneView<'a> for McpSettingsView {
Span::styled(" ", Style::default()),
Span::styled(row.summary.clone(), sum_style),
]));
if sel {
selected_line_index = arrow_line_index;
}
}

// Add New…
let add_sel = self.selected == self.rows.len();
let add_style = if add_sel { Style::default().bg(crate::colors::selection()).add_modifier(Modifier::BOLD) } else { Style::default() };
lines.push(Line::from(""));
let add_line_index = lines.len();
lines.push(Line::from(vec![Span::styled(if add_sel { "› " } else { " " }, Style::default()), Span::styled("Add new server…", add_style)]));
if add_sel {
selected_line_index = add_line_index;
}

// Close
let close_sel = self.selected == self.rows.len().saturating_add(1);
let close_style = if close_sel { Style::default().bg(crate::colors::selection()).add_modifier(Modifier::BOLD) } else { Style::default() };
let close_line_index = lines.len();
lines.push(Line::from(vec![Span::styled(if close_sel { "› " } else { " " }, Style::default()), Span::styled("Close", close_style)]));
if close_sel {
selected_line_index = close_line_index;
}

lines.push(Line::from(""));
lines.push(Line::from(vec![
Expand All @@ -156,9 +170,24 @@ impl<'a> BottomPaneView<'a> for McpSettingsView {
Span::styled(" Close", Style::default().fg(crate::colors::text_dim())),
]));

let total_lines = lines.len();
let viewport_height = inner.height as usize;
let selected_line_index = selected_line_index.min(total_lines.saturating_sub(1));
let mut scroll_top = 0usize;
if viewport_height > 0 && total_lines > viewport_height {
let half = viewport_height / 2;
let mut candidate = selected_line_index.saturating_sub(half);
let max_scroll = total_lines - viewport_height;
if candidate > max_scroll {
candidate = max_scroll;
}
scroll_top = candidate;
}

let paragraph = Paragraph::new(lines)
.alignment(Alignment::Left)
.style(Style::default().bg(crate::colors::background()).fg(crate::colors::text()));
.style(Style::default().bg(crate::colors::background()).fg(crate::colors::text()))
.scroll((scroll_top as u16, 0));
paragraph.render(Rect { x: inner.x.saturating_add(1), y: inner.y, width: inner.width.saturating_sub(2), height: inner.height }, buf);
}
}
Loading