improvements, more dry

codex-rs/tui/src/bottom_pane/chat_composer.rs

@@ -43,6 +43,7 @@ pub(crate) struct ChatComposer<'a> {
     ctrl_c_quit_hint: bool,
     use_shift_enter_hint: bool,
     dismissed_file_popup_token: Option<String>,
+    dismissed_slash_token: Option<String>,
     current_file_query: Option<String>,
     pending_pastes: Vec<(String, String)>,
 }
@@ -55,6 +56,95 @@ enum ActivePopup {
 }
 
 impl ChatComposer<'_> {
+    #[inline]
+    fn first_line(&self) -> &str {
+        self.textarea
+            .lines()
+            .first()
+            .map(|s| s.as_str())
+            .unwrap_or("")
+    }
+
+    #[inline]
+    fn slash_token_from_first_line(first_line: &str) -> Option<&str> {
+        if !first_line.starts_with('/') {
+            return None;
+        }
+        let stripped = first_line.strip_prefix('/').unwrap_or("");
+        let token = stripped.trim_start();
+        Some(token.split_whitespace().next().unwrap_or(""))
+    }
+
+    #[inline]
+    fn emit_unrecognized_slash_command(&mut self, cmd_token: &str) {
+        let attempted = if cmd_token.is_empty() {
+            "/".to_string()
+        } else {
+            format!("/{cmd_token}")
+        };
+        let msg = format!("{attempted} not a recognized command");
+        self.app_event_tx
+            .send(AppEvent::InsertHistory(vec![Line::from(msg)]));
+        self.dismissed_slash_token = Some(cmd_token.to_string());
+        self.active_popup = ActivePopup::None;
+    }
+
+    #[inline]
+    fn sync_popups(&mut self) {
+        self.sync_command_popup();
+        if matches!(self.active_popup, ActivePopup::Command(_)) {
+            self.dismissed_file_popup_token = None;
+        } else {
+            self.sync_file_search_popup();
+        }
+    }
+
+    #[inline]
+    fn compute_textarea_and_popup_rect(&self, area: Rect, desired_popup: u16) -> (Rect, Rect) {
+        let text_lines = self.textarea.lines().len().max(1) as u16;
+        let popup_height = desired_popup.min(area.height.saturating_sub(text_lines));
+        let textarea_rect = Rect {
+            x: area.x,
+            y: area.y,
+            width: area.width,
+            height: area.height.saturating_sub(popup_height),
+        };
+        let popup_rect = Rect {
+            x: area.x,
+            y: area.y + textarea_rect.height,
+            width: area.width,
+            height: popup_height,
+        };
+        (textarea_rect, popup_rect)
+    }
+
+    /// Convert a cursor column (in chars) to a byte offset within `line`.
+    #[inline]
+    fn cursor_byte_offset_for_col(line: &str, col_chars: usize) -> usize {
+        line.chars().take(col_chars).map(|c| c.len_utf8()).sum()
+    }
+
+    /// Determine token boundaries around a cursor position expressed as a byte offset.
+    /// A token is delimited by any Unicode whitespace on either side.
+    #[inline]
+    fn token_bounds(line: &str, cursor_byte_offset: usize) -> Option<(usize, usize)> {
+        let (before_cursor, after_cursor) = line.split_at(cursor_byte_offset.min(line.len()));
+
+        let start_idx = before_cursor
+            .char_indices()
+            .rfind(|(_, c)| c.is_whitespace())
+            .map(|(idx, c)| idx + c.len_utf8())
+            .unwrap_or(0);
+
+        let end_rel_idx = after_cursor
+            .char_indices()
+            .find(|(_, c)| c.is_whitespace())
+            .map(|(idx, _)| idx)
+            .unwrap_or(after_cursor.len());
+        let end_idx = cursor_byte_offset + end_rel_idx;
+
+        (start_idx < end_idx).then_some((start_idx, end_idx))
+    }
     pub fn new(
         has_input_focus: bool,
         app_event_tx: AppEventSender,
@@ -74,6 +164,7 @@ impl ChatComposer<'_> {
             ctrl_c_quit_hint: false,
             use_shift_enter_hint,
             dismissed_file_popup_token: None,
+            dismissed_slash_token: None,
             current_file_query: None,
             pending_pastes: Vec::new(),
         };
@@ -155,8 +246,7 @@ impl ChatComposer<'_> {
         } else {
             self.textarea.insert_str(&pasted);
         }
-        self.sync_command_popup();
-        self.sync_file_search_popup();
+        self.sync_popups();
         true
     }
 
@@ -190,19 +280,14 @@ impl ChatComposer<'_> {
             ActivePopup::None => self.handle_key_event_without_popup(key_event),
         };
 
-        // Update (or hide/show) popup after processing the key.
-        self.sync_command_popup();
-        if matches!(self.active_popup, ActivePopup::Command(_)) {
-            self.dismissed_file_popup_token = None;
-        } else {
-            self.sync_file_search_popup();
-        }
+        self.sync_popups();
 
         result
     }
 
     /// Handle key event when the slash-command popup is visible.
     fn handle_key_event_with_slash_popup(&mut self, key_event: KeyEvent) -> (InputResult, bool) {
+        let first_line_owned = self.first_line().to_string();
         let ActivePopup::Command(popup) = &mut self.active_popup else {
             unreachable!();
         };
@@ -216,16 +301,16 @@ impl ChatComposer<'_> {
                 popup.move_down();
                 (InputResult::None, true)
             }
+            Input { key: Key::Esc, .. } => {
+                if let Some(cmd_token) = Self::slash_token_from_first_line(&first_line_owned) {
+                    self.dismissed_slash_token = Some(cmd_token.to_string());
+                }
+                self.active_popup = ActivePopup::None;
+                (InputResult::None, true)
+            }
             Input { key: Key::Tab, .. } => {
                 if let Some(cmd) = popup.selected_command() {
-                    let first_line = self
-                        .textarea
-                        .lines()
-                        .first()
-                        .map(|s| s.as_str())
-                        .unwrap_or("");
-
-                    let starts_with_cmd = first_line
+                    let starts_with_cmd = first_line_owned
                         .trim_start()
                         .starts_with(&format!("/{}", cmd.command()));
 
@@ -244,19 +329,17 @@ impl ChatComposer<'_> {
                 ctrl: false,
             } => {
                 if let Some(cmd) = popup.selected_command() {
-                    // Send command to the app layer.
                     self.app_event_tx.send(AppEvent::DispatchCommand(*cmd));
-
-                    // Clear textarea so no residual text remains.
                     self.textarea.select_all();
                     self.textarea.cut();
-
-                    // Hide popup since the command has been dispatched.
                     self.active_popup = ActivePopup::None;
                     return (InputResult::None, true);
                 }
-                // Fallback to default newline handling if no command selected.
-                self.handle_key_event_without_popup(key_event)
+                if let Some(cmd_token) = Self::slash_token_from_first_line(&first_line_owned) {
+                    self.emit_unrecognized_slash_command(cmd_token);
+                    return (InputResult::None, true);
+                }
+                (InputResult::None, false)
             }
             input => self.handle_input_basic(input),
         }
@@ -317,37 +400,9 @@ impl ChatComposer<'_> {
     ///   one additional character, that token (without `@`) is returned.
     fn current_at_token(textarea: &tui_textarea::TextArea) -> Option<String> {
         let (row, col) = textarea.cursor();
-
-        // Guard against out-of-bounds rows.
         let line = textarea.lines().get(row)?.as_str();
-
-        // Calculate byte offset for cursor position
-        let cursor_byte_offset = line.chars().take(col).map(|c| c.len_utf8()).sum::<usize>();
-
-        // Split the line at the cursor position so we can search for word
-        // boundaries on both sides.
-        let before_cursor = &line[..cursor_byte_offset];
-        let after_cursor = &line[cursor_byte_offset..];
-
-        // Find start index (first character **after** the previous multi-byte whitespace).
-        let start_idx = before_cursor
-            .char_indices()
-            .rfind(|(_, c)| c.is_whitespace())
-            .map(|(idx, c)| idx + c.len_utf8())
-            .unwrap_or(0);
-
-        // Find end index (first multi-byte whitespace **after** the cursor position).
-        let end_rel_idx = after_cursor
-            .char_indices()
-            .find(|(_, c)| c.is_whitespace())
-            .map(|(idx, _)| idx)
-            .unwrap_or(after_cursor.len());
-        let end_idx = cursor_byte_offset + end_rel_idx;
-
-        if start_idx >= end_idx {
-            return None;
-        }
-
+        let cursor_byte_offset = Self::cursor_byte_offset_for_col(line, col);
+        let (start_idx, end_idx) = Self::token_bounds(line, cursor_byte_offset)?;
         let token = &line[start_idx..end_idx];
 
         if token.starts_with('@') && token.len() > 1 {
@@ -364,50 +419,24 @@ impl ChatComposer<'_> {
     /// `@tokens` exist in the line.
     fn insert_selected_path(&mut self, path: &str) {
         let (row, col) = self.textarea.cursor();
-
-        // Materialize the textarea lines so we can mutate them easily.
         let mut lines: Vec<String> = self.textarea.lines().to_vec();
 
         if let Some(line) = lines.get_mut(row) {
-            // Calculate byte offset for cursor position
-            let cursor_byte_offset = line.chars().take(col).map(|c| c.len_utf8()).sum::<usize>();
+            let cursor_byte_offset = Self::cursor_byte_offset_for_col(line, col);
+            if let Some((start_idx, end_idx)) = Self::token_bounds(line, cursor_byte_offset) {
+                let mut new_line =
+                    String::with_capacity(line.len() - (end_idx - start_idx) + path.len() + 1);
+                new_line.push_str(&line[..start_idx]);
+                new_line.push_str(path);
+                new_line.push(' ');
+                new_line.push_str(&line[end_idx..]);
+                *line = new_line;
 
-            let before_cursor = &line[..cursor_byte_offset];
-            let after_cursor = &line[cursor_byte_offset..];
-
-            // Determine token boundaries.
-            let start_idx = before_cursor
-                .char_indices()
-                .rfind(|(_, c)| c.is_whitespace())
-                .map(|(idx, c)| idx + c.len_utf8())
-                .unwrap_or(0);
-
-            let end_rel_idx = after_cursor
-                .char_indices()
-                .find(|(_, c)| c.is_whitespace())
-                .map(|(idx, _)| idx)
-                .unwrap_or(after_cursor.len());
-            let end_idx = cursor_byte_offset + end_rel_idx;
-
-            // Replace the slice `[start_idx, end_idx)` with the chosen path and a trailing space.
-            let mut new_line =
-                String::with_capacity(line.len() - (end_idx - start_idx) + path.len() + 1);
-            new_line.push_str(&line[..start_idx]);
-            new_line.push_str(path);
-            new_line.push(' ');
-            new_line.push_str(&line[end_idx..]);
-
-            *line = new_line;
-
-            // Re-populate the textarea.
-            let new_text = lines.join("\n");
-            self.textarea.select_all();
-            self.textarea.cut();
-            let _ = self.textarea.insert_str(new_text);
-
-            // Note: tui-textarea currently exposes only relative cursor
-            // movements. Leaving the cursor position unchanged is acceptable
-            // as subsequent typing will move the cursor naturally.
+                let new_text = lines.join("\n");
+                self.textarea.select_all();
+                self.textarea.cut();
+                let _ = self.textarea.insert_str(new_text);
+            }
         }
     }
 
@@ -579,29 +608,28 @@ impl ChatComposer<'_> {
     /// textarea. This must be called after every modification that can change
     /// the text so the popup is shown/updated/hidden as appropriate.
     fn sync_command_popup(&mut self) {
-        // Inspect only the first line to decide whether to show the popup. In
-        // the common case (no leading slash) we avoid copying the entire
-        // textarea contents.
-        let first_line = self
-            .textarea
-            .lines()
-            .first()
-            .map(|s| s.as_str())
-            .unwrap_or("");
-
+        let first_line = self.first_line().to_string();
         let input_starts_with_slash = first_line.starts_with('/');
+        if !input_starts_with_slash {
+            self.dismissed_slash_token = None;
+        }
+        let current_cmd_token: Option<&str> = Self::slash_token_from_first_line(&first_line);
         match &mut self.active_popup {
             ActivePopup::Command(popup) => {
                 if input_starts_with_slash {
-                    popup.on_composer_text_change(first_line.to_string());
+                    popup.on_composer_text_change(first_line.clone());
                 } else {
                     self.active_popup = ActivePopup::None;
+                    self.dismissed_slash_token = None;
                 }
             }
             _ => {
                 if input_starts_with_slash {
+                    if self.dismissed_slash_token.as_deref() == current_cmd_token {
+                        return;
+                    }
                     let mut command_popup = CommandPopup::new();
-                    command_popup.on_composer_text_change(first_line.to_string());
+                    command_popup.on_composer_text_change(first_line);
                     self.active_popup = ActivePopup::Command(command_popup);
                 }
             }
@@ -664,44 +692,16 @@ impl WidgetRef for &ChatComposer<'_> {
     fn render_ref(&self, area: Rect, buf: &mut Buffer) {
         match &self.active_popup {
             ActivePopup::Command(popup) => {
-                let popup_height = popup.calculate_required_height();
-
-                // Split the provided rect so that the popup is rendered at the
-                // **bottom** and the textarea occupies the remaining space above.
-                let popup_height = popup_height.min(area.height);
-                let textarea_rect = Rect {
-                    x: area.x,
-                    y: area.y,
-                    width: area.width,
-                    height: area.height.saturating_sub(popup_height),
-                };
-                let popup_rect = Rect {
-                    x: area.x,
-                    y: area.y + textarea_rect.height,
-                    width: area.width,
-                    height: popup_height,
-                };
-
+                let desired_popup = popup.calculate_required_height();
+                let (textarea_rect, popup_rect) =
+                    self.compute_textarea_and_popup_rect(area, desired_popup);
                 popup.render(popup_rect, buf);
                 self.textarea.render(textarea_rect, buf);
             }
             ActivePopup::File(popup) => {
-                let popup_height = popup.calculate_required_height();
-
-                let popup_height = popup_height.min(area.height);
-                let textarea_rect = Rect {
-                    x: area.x,
-                    y: area.y,
-                    width: area.width,
-                    height: area.height.saturating_sub(popup_height),
-                };
-                let popup_rect = Rect {
-                    x: area.x,
-                    y: area.y + textarea_rect.height,
-                    width: area.width,
-                    height: popup_height,
-                };
-
+                let desired_popup = popup.calculate_required_height();
+                let (textarea_rect, popup_rect) =
+                    self.compute_textarea_and_popup_rect(area, desired_popup);
                 popup.render(popup_rect, buf);
                 self.textarea.render(textarea_rect, buf);
             }
@@ -745,6 +745,7 @@ impl WidgetRef for &ChatComposer<'_> {
 
 #[cfg(test)]
 mod tests {
+
     use crate::bottom_pane::AppEventSender;
     use crate::bottom_pane::ChatComposer;
     use crate::bottom_pane::InputResult;
@@ -1020,6 +1021,97 @@ mod tests {
         }
     }
 
+    #[test]
+    fn esc_dismiss_slash_popup_reopen_on_token_change() {
+        use crate::bottom_pane::chat_composer::ActivePopup;
+        use crossterm::event::KeyCode;
+        use crossterm::event::KeyEvent;
+        use crossterm::event::KeyModifiers;
+
+        let (tx, _rx) = std::sync::mpsc::channel();
+        let sender = AppEventSender::new(tx);
+        let mut composer = ChatComposer::new(true, sender, false);
+
+        composer.handle_paste("/".to_string());
+        assert!(matches!(composer.active_popup, ActivePopup::Command(_)));
+
+        let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE));
+        assert!(matches!(composer.active_popup, ActivePopup::None));
+
+        composer.handle_paste("c".to_string());
+        assert!(matches!(composer.active_popup, ActivePopup::Command(_)));
+    }
+
+    #[test]
+    fn enter_with_unrecognized_slash_command_closes_popup_and_emits_error() {
+        use crate::app_event::AppEvent;
+        use crate::bottom_pane::chat_composer::ActivePopup;
+        use crossterm::event::KeyCode;
+        use crossterm::event::KeyEvent;
+        use crossterm::event::KeyModifiers;
+        use std::sync::mpsc::TryRecvError;
+
+        let (tx, rx) = std::sync::mpsc::channel();
+        let sender = AppEventSender::new(tx);
+        let mut composer = ChatComposer::new(true, sender, false);
+
+        composer.handle_paste("/ notacommand args".to_string());
+        assert!(matches!(composer.active_popup, ActivePopup::Command(_)));
+
+        let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
+
+        assert!(matches!(composer.active_popup, ActivePopup::None));
+
+        let mut saw_error = false;
+        loop {
+            match rx.try_recv() {
+                Ok(AppEvent::InsertHistory(lines)) => {
+                    let text = lines
+                        .into_iter()
+                        .map(|l| l.to_string())
+                        .collect::<Vec<_>>()
+                        .join("\n");
+                    if text.contains("/notacommand not a recognized command") {
+                        saw_error = true;
+                        break;
+                    }
+                }
+                Ok(_) => continue,
+                Err(TryRecvError::Empty) => break,
+                Err(TryRecvError::Disconnected) => break,
+            }
+        }
+
+        assert!(
+            saw_error,
+            "expected InsertHistory error for unrecognized command"
+        );
+    }
+
+    #[test]
+    fn esc_dismiss_then_delete_and_retype_slash_reopens_popup() {
+        use crate::bottom_pane::chat_composer::ActivePopup;
+        use crossterm::event::KeyCode;
+        use crossterm::event::KeyEvent;
+        use crossterm::event::KeyModifiers;
+
+        let (tx, _rx) = std::sync::mpsc::channel();
+        let sender = AppEventSender::new(tx);
+        let mut composer = ChatComposer::new(true, sender, false);
+
+        composer.handle_paste("/".to_string());
+        assert!(matches!(composer.active_popup, ActivePopup::Command(_)));
+
+        let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE));
+        assert!(matches!(composer.active_popup, ActivePopup::None));
+
+        let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE));
+        assert!(matches!(composer.active_popup, ActivePopup::None));
+
+        composer.handle_paste("/".to_string());
+        assert!(matches!(composer.active_popup, ActivePopup::Command(_)));
+    }
+
     #[test]
     fn test_multiple_pastes_submission() {
         use crossterm::event::KeyCode;
@@ -1030,18 +1122,15 @@ mod tests {
         let sender = AppEventSender::new(tx);
         let mut composer = ChatComposer::new(true, sender, false);
 
-        // Define test cases: (paste content, is_large)
         let test_cases = [
             ("x".repeat(LARGE_PASTE_CHAR_THRESHOLD + 3), true),
             (" and ".to_string(), false),
             ("y".repeat(LARGE_PASTE_CHAR_THRESHOLD + 7), true),
         ];
 
-        // Expected states after each paste
         let mut expected_text = String::new();
         let mut expected_pending_count = 0;
 
-        // Apply all pastes and build expected state
         let states: Vec<_> = test_cases
             .iter()
             .map(|(content, is_large)| {
@@ -1057,7 +1146,6 @@ mod tests {
             })
             .collect();
 
-        // Verify all intermediate states were correct
         assert_eq!(
             states,
             vec![
@@ -1083,7 +1171,6 @@ mod tests {
             ]
         );
 
-        // Submit and verify final expansion
         let (result, _) =
             composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
         if let InputResult::Submitted(text) = result {
@@ -1103,14 +1190,12 @@ mod tests {
         let sender = AppEventSender::new(tx);
         let mut composer = ChatComposer::new(true, sender, false);
 
-        // Define test cases: (content, is_large)
         let test_cases = [
             ("a".repeat(LARGE_PASTE_CHAR_THRESHOLD + 5), true),
             (" and ".to_string(), false),
             ("b".repeat(LARGE_PASTE_CHAR_THRESHOLD + 6), true),
         ];
 
-        // Apply all pastes
         let mut current_pos = 0;
         let states: Vec<_> = test_cases
             .iter()
@@ -1130,10 +1215,8 @@ mod tests {
             })
             .collect();
 
-        // Delete placeholders one by one and collect states
         let mut deletion_states = vec![];
 
-        // First deletion
         composer
             .textarea
             .move_cursor(tui_textarea::CursorMove::Jump(0, states[0].2 as u16));
@@ -1143,7 +1226,6 @@ mod tests {
             composer.pending_pastes.len(),
         ));
 
-        // Second deletion
         composer
             .textarea
             .move_cursor(tui_textarea::CursorMove::Jump(
@@ -1156,7 +1238,6 @@ mod tests {
             composer.pending_pastes.len(),
         ));
 
-        // Verify all states
         assert_eq!(
             deletion_states,
             vec![
@@ -1176,7 +1257,6 @@ mod tests {
         let sender = AppEventSender::new(tx);
         let mut composer = ChatComposer::new(true, sender, false);
 
-        // Define test cases: (cursor_position_from_end, expected_pending_count)
         let test_cases = [
             5, // Delete from middle - should clear tracking
             0, // Delete from end - should clear tracking

codex-rs/tui/src/bottom_pane/command_popup.rs

@@ -25,6 +25,8 @@ pub(crate) struct CommandPopup {
     command_filter: String,
     all_commands: Vec<(&'static str, SlashCommand)>,
     selected_idx: Option<usize>,
+    // Index of the first visible row in the filtered list.
+    scroll_top: usize,
 }
 
 impl CommandPopup {
@@ -33,6 +35,7 @@ impl CommandPopup {
             command_filter: String::new(),
             all_commands: built_in_slash_commands(),
             selected_idx: None,
+            scroll_top: 0,
         }
     }
 
@@ -66,26 +69,28 @@ impl CommandPopup {
             0 => None,
             _ => Some(self.selected_idx.unwrap_or(0).min(matches_len - 1)),
         };
+
+        self.adjust_scroll(matches_len);
     }
 
     /// Determine the preferred height of the popup. This is the number of
-    /// rows required to show **at most** `MAX_POPUP_ROWS` commands plus the
-    /// table/border overhead (one line at the top and one at the bottom).
+    /// rows required to show at most MAX_POPUP_ROWS commands.
     pub(crate) fn calculate_required_height(&self) -> u16 {
         self.filtered_commands().len().clamp(1, MAX_POPUP_ROWS) as u16
     }
 
     /// Return the list of commands that match the current filter. Matching is
-    /// performed using a *prefix* comparison on the command name.
+    /// performed using a case-insensitive prefix comparison on the command name.
     fn filtered_commands(&self) -> Vec<&SlashCommand> {
+        let filter = self.command_filter.as_str();
         self.all_commands
             .iter()
             .filter_map(|(_name, cmd)| {
-                if self.command_filter.is_empty()
-                    || cmd
-                        .command()
-                        .starts_with(&self.command_filter.to_ascii_lowercase())
-                {
+                if filter.is_empty() {
+                    return Some(cmd);
+                }
+                let name = cmd.command();
+                if name.len() >= filter.len() && name[..filter.len()].eq_ignore_ascii_case(filter) {
                     Some(cmd)
                 } else {
                     None
@@ -96,26 +101,30 @@ impl CommandPopup {
 
     /// Move the selection cursor one step up.
     pub(crate) fn move_up(&mut self) {
-        if let Some(len) = self.filtered_commands().len().checked_sub(1) {
-            if len == usize::MAX {
-                return;
-            }
+        let matches = self.filtered_commands();
+        let len = matches.len();
+        if len == 0 {
+            self.selected_idx = None;
+            self.scroll_top = 0;
+            return;
         }
 
-        if let Some(idx) = self.selected_idx {
-            if idx > 0 {
-                self.selected_idx = Some(idx - 1);
-            }
-        } else if !self.filtered_commands().is_empty() {
-            self.selected_idx = Some(0);
+        match self.selected_idx {
+            Some(idx) if idx > 0 => self.selected_idx = Some(idx - 1),
+            Some(_) => self.selected_idx = Some(len - 1), // wrap to last
+            None => self.selected_idx = Some(0),
         }
+
+        self.adjust_scroll(len);
     }
 
     /// Move the selection cursor one step down.
     pub(crate) fn move_down(&mut self) {
-        let matches_len = self.filtered_commands().len();
+        let matches = self.filtered_commands();
+        let matches_len = matches.len();
         if matches_len == 0 {
             self.selected_idx = None;
+            self.scroll_top = 0;
             return;
         }
 
@@ -123,11 +132,15 @@ impl CommandPopup {
             Some(idx) if idx + 1 < matches_len => {
                 self.selected_idx = Some(idx + 1);
             }
+            Some(_idx_last) => {
+                self.selected_idx = Some(0);
+            }
             None => {
                 self.selected_idx = Some(0);
             }
-            _ => {}
         }
+
+        self.adjust_scroll(matches_len);
     }
 
     /// Return currently selected command, if any.
@@ -135,6 +148,26 @@ impl CommandPopup {
         let matches = self.filtered_commands();
         self.selected_idx.and_then(|idx| matches.get(idx).copied())
     }
+
+    fn adjust_scroll(&mut self, matches_len: usize) {
+        if matches_len == 0 {
+            self.scroll_top = 0;
+            return;
+        }
+        let visible_rows = MAX_POPUP_ROWS.min(matches_len);
+        if let Some(sel) = self.selected_idx {
+            if sel < self.scroll_top {
+                self.scroll_top = sel;
+            } else {
+                let bottom = self.scroll_top + visible_rows - 1;
+                if sel > bottom {
+                    self.scroll_top = sel + 1 - visible_rows;
+                }
+            }
+        } else {
+            self.scroll_top = 0;
+        }
+    }
 }
 
 impl WidgetRef for CommandPopup {
@@ -142,10 +175,8 @@ impl WidgetRef for CommandPopup {
         let matches = self.filtered_commands();
 
         let mut rows: Vec<Row> = Vec::new();
-        let visible_matches: Vec<&SlashCommand> =
-            matches.into_iter().take(MAX_POPUP_ROWS).collect();
 
-        if visible_matches.is_empty() {
+        if matches.is_empty() {
             rows.push(Row::new(vec![
                 Cell::from(""),
                 Cell::from("No matching commands").add_modifier(Modifier::ITALIC),
@@ -153,10 +184,32 @@ impl WidgetRef for CommandPopup {
         } else {
             let default_style = Style::default();
             let command_style = Style::default().fg(Color::LightBlue);
-            for (idx, cmd) in visible_matches.iter().enumerate() {
+            let max_rows_from_area = area.height as usize;
+            let visible_rows = MAX_POPUP_ROWS
+                .min(matches.len())
+                .min(max_rows_from_area.max(1));
+
+            let mut start_idx = self.scroll_top.min(matches.len().saturating_sub(1));
+            if let Some(sel) = self.selected_idx {
+                if sel < start_idx {
+                    start_idx = sel;
+                } else if visible_rows > 0 {
+                    let bottom = start_idx + visible_rows - 1;
+                    if sel > bottom {
+                        start_idx = sel + 1 - visible_rows;
+                    }
+                }
+            }
+
+            for (global_idx, cmd) in matches
+                .iter()
+                .enumerate()
+                .skip(start_idx)
+                .take(visible_rows)
+            {
                 rows.push(Row::new(vec![
                     Cell::from(Line::from(vec![
-                        if Some(idx) == self.selected_idx {
+                        if Some(global_idx) == self.selected_idx {
                             Span::styled(
                                 "›",
                                 Style::default().bg(Color::DarkGray).fg(Color::LightCyan),
@@ -188,3 +241,67 @@ impl WidgetRef for CommandPopup {
         table.render(area, buf);
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use ratatui::buffer::Buffer;
+    use ratatui::layout::Rect;
+
+    #[test]
+    fn move_down_wraps_to_top() {
+        let mut popup = CommandPopup::new();
+        // Show all commands by simulating composer input starting with '/'.
+        popup.on_composer_text_change("/".to_string());
+        let len = popup.filtered_commands().len();
+        assert!(len > 0);
+
+        // Move to last item.
+        for _ in 0..len.saturating_sub(1) {
+            popup.move_down();
+        }
+        // Next move_down should wrap to index 0.
+        popup.move_down();
+        assert_eq!(popup.selected_idx, Some(0));
+    }
+
+    #[test]
+    fn move_up_wraps_to_bottom() {
+        let mut popup = CommandPopup::new();
+        popup.on_composer_text_change("/".to_string());
+        let len = popup.filtered_commands().len();
+        assert!(len > 0);
+
+        // Initial selection is 0; moving up should wrap to last.
+        popup.move_up();
+        assert_eq!(popup.selected_idx, Some(len - 1));
+    }
+
+    #[test]
+    fn respects_tiny_terminal_height_when_rendering() {
+        let mut popup = CommandPopup::new();
+        popup.on_composer_text_change("/".to_string());
+        assert!(popup.filtered_commands().len() >= 3);
+
+        let area = Rect::new(0, 0, 50, 2);
+        let mut buf = Buffer::empty(area);
+        popup.render(area, &mut buf);
+
+        let mut non_empty_rows = 0u16;
+        for y in 0..area.height {
+            let mut row_has_content = false;
+            for x in 0..area.width {
+                let c = buf[(x, y)].symbol();
+                if !c.trim().is_empty() {
+                    row_has_content = true;
+                    break;
+                }
+            }
+            if row_has_content {
+                non_empty_rows += 1;
+            }
+        }
+
+        assert_eq!(non_empty_rows, 2);
+    }
+}

codex-rs/tui/src/bottom_pane/file_search_popup.rs

@@ -108,7 +108,6 @@ impl FileSearchPopup {
             .map(|file_match| file_match.path.as_str())
     }
 
-    /// Preferred height (rows) including border.
     pub(crate) fn calculate_required_height(&self) -> u16 {
         // Row count depends on whether we already have matches. If no matches
         // yet (e.g. initial search or query with no results) reserve a single