fix(tui): yank cell/row no longer reports "No row selected"

src/buffer.rs

@@ -631,9 +631,20 @@ impl BufferAPI for Buffer {
 
     // --- Table Navigation ---
     fn get_selected_row(&self) -> Option<usize> {
-        // For backward compatibility, check if table_state has a selection
-        // This maintains the old API behavior where None means no selection
-        self.table_state.selected()
+        // The crosshair is the position the table actually renders, so derive the
+        // selection from it rather than from table_state. table_state is only written
+        // by row navigation and query execution, so it stays None after a file load
+        // (or after a filter that leaves a single row, where j/k never fires) - which
+        // used to make yank report "No row selected" on a perfectly visible cell.
+        //
+        // None now means "there is no data", and the row is clamped to the current
+        // view so a filter that shrinks the results can't leave us reading past the end.
+        let row_count = self.visible_row_count();
+        if row_count == 0 {
+            None
+        } else {
+            Some(self.view_state.crosshair_row.min(row_count - 1))
+        }
     }
 
     fn set_selected_row(&mut self, row: Option<usize>) {
@@ -1264,6 +1275,18 @@ impl BufferAPI for Buffer {
 }
 
 impl Buffer {
+    /// Number of rows currently visible, preferring the `DataView` (which knows about
+    /// filtering) and falling back to the raw `DataTable` for legacy buffers.
+    fn visible_row_count(&self) -> usize {
+        if let Some(ref dataview) = self.dataview {
+            dataview.row_count()
+        } else if let Some(ref datatable) = self.datatable {
+            datatable.row_count()
+        } else {
+            0
+        }
+    }
+
     /// Create a new empty buffer
     #[must_use]
     pub fn new(id: usize) -> Self {

src/ui/enhanced_tui.rs

@@ -4911,8 +4911,11 @@ impl EnhancedTuiApp {
 
         // Delegate state coordination to StateCoordinator
         use crate::ui::state::state_coordinator::StateCoordinator;
-        let _rows_after =
-            StateCoordinator::apply_text_filter_with_refs(&mut self.state_container, pattern);
+        let _rows_after = StateCoordinator::apply_text_filter_with_refs(
+            &mut self.state_container,
+            &self.viewport_manager,
+            pattern,
+        );
 
         // Update ViewportManager with the filtered DataView
         // Sync the dataview to both managers

src/ui/state/state_coordinator.rs

@@ -351,6 +351,7 @@ impl StateCoordinator {
     /// Returns the number of matching rows
     pub fn apply_text_filter_with_refs(
         state_container: &mut AppStateContainer,
+        viewport_manager: &RefCell<Option<ViewportManager>>,
         pattern: &str,
     ) -> usize {
         let case_insensitive = state_container.is_case_insensitive();
@@ -375,6 +376,35 @@ impl StateCoordinator {
             0
         };
 
+        // Reset navigation to the first match, the same way the fuzzy filter does.
+        // Without this the crosshair keeps pointing at wherever it was before the
+        // filter, which can now be past the end of the narrowed view.
+        if rows_after > 0 {
+            // Preserve horizontal scroll
+            let col_offset = state_container.get_scroll_offset().1;
+
+            state_container.set_selected_row(Some(0));
+            state_container.set_scroll_offset((0, col_offset));
+            state_container.set_table_selected_row(Some(0));
+
+            {
+                let mut nav = state_container.navigation_mut();
+                nav.selected_row = 0;
+                nav.scroll_offset.0 = 0;
+            }
+
+            if let Ok(mut vm_borrow) = viewport_manager.try_borrow_mut() {
+                if let Some(ref mut vm) = *vm_borrow {
+                    vm.set_crosshair_row(0);
+                    vm.set_scroll_offset(0, col_offset);
+                    debug!(
+                        "StateCoordinator: Reset viewport to first match (row 0) with {} total matches",
+                        rows_after
+                    );
+                }
+            }
+        }
+
         // Update status message
         let status = if pattern.is_empty() {
             "Filter cleared".to_string()

tests/main.rs

@@ -199,6 +199,9 @@ mod test_window_context;
 #[path = "test_yanked_query.rs"]
 mod test_yanked_query;
 
+#[path = "test_yank_single_row_repro.rs"]
+mod test_yank_single_row_repro;
+
 #[path = "tui_integration_test.rs"]
 mod tui_integration_test;
 

tests/test_buffer.rs

@@ -2,6 +2,24 @@ use serde_json::json;
 use sql_cli::api_client::{QueryInfo, QueryResponse};
 use sql_cli::buffer::{AppMode, Buffer, BufferAPI, SortOrder};
 
+/// Build a `QueryResponse` with `rows` rows, for tests that need data on screen.
+fn make_test_response(rows: usize) -> QueryResponse {
+    QueryResponse {
+        data: (0..rows)
+            .map(|i| json!({"id": i, "name": format!("row{i}")}))
+            .collect(),
+        count: rows,
+        query: QueryInfo {
+            select: vec!["id".to_string(), "name".to_string()],
+            where_clause: None,
+            order_by: None,
+        },
+        source: None,
+        table: None,
+        cached: None,
+    }
+}
+
 #[test]
 fn test_buffer_basic_operations() {
     let mut buffer = Buffer::new(1);
@@ -116,12 +134,25 @@ fn test_buffer_results() {
 fn test_buffer_navigation() {
     let mut buffer = Buffer::new(1);
 
-    // Test row selection
+    // The selection is derived from the visible data, so an empty buffer has none
+    assert_eq!(buffer.get_selected_row(), None);
+
+    // Test row selection against real data
+    buffer
+        .set_results_as_datatable(Some(make_test_response(10)))
+        .unwrap();
+
     buffer.set_selected_row(Some(5));
     assert_eq!(buffer.get_selected_row(), Some(5));
 
+    // Setting None resets the crosshair to the first row rather than clearing the
+    // selection - with data on screen there is always a selectable row
     buffer.set_selected_row(None);
-    assert_eq!(buffer.get_selected_row(), None);
+    assert_eq!(buffer.get_selected_row(), Some(0));
+
+    // Out-of-range positions are clamped to the last visible row
+    buffer.set_selected_row(Some(99));
+    assert_eq!(buffer.get_selected_row(), Some(9));
 
     // Test scroll offset
     buffer.set_scroll_offset((10, 20));

tests/test_buffer_state_refactor.rs

@@ -3,6 +3,26 @@
 /// using the new proxy-based architecture
 use sql_cli::app_state_container::AppStateContainer;
 use sql_cli::buffer::{Buffer, BufferAPI, BufferManager, SelectionMode};
+use sql_cli::data::datatable::{DataColumn, DataRow, DataTable, DataValue};
+use std::sync::Arc;
+
+/// A `DataTable` with `rows` rows and 8 columns, for tests that need data on screen.
+fn make_datatable(rows: usize) -> DataTable {
+    let mut table = DataTable::new("test");
+    for c in 0..8 {
+        table.add_column(DataColumn::new(format!("col{c}")));
+    }
+    for r in 0..rows {
+        table
+            .add_row(DataRow::new(
+                (0..8)
+                    .map(|c| DataValue::Integer((r * 8 + c) as i64))
+                    .collect(),
+            ))
+            .unwrap();
+    }
+    table
+}
 
 #[test]
 #[ignore = "Disabled: test is unreliable due to file system dependencies in CommandHistory::new()"]
@@ -176,6 +196,8 @@ fn test_proxy_with_no_buffer() {
 fn test_direct_buffer_viewstate_access() {
     // Test that we can also access ViewState directly from Buffer
     let mut buffer = Buffer::new(1);
+    // get_selected_row() derives from the visible data, so give the buffer some
+    buffer.set_datatable(Some(Arc::new(make_datatable(20))));
 
     // Modify ViewState directly
     buffer.view_state.crosshair_row = 15;

tests/test_yank_single_row_repro.rs

@@ -0,0 +1,135 @@
+//! Regression tests for yank reporting "No row selected" on a visible cell.
+//!
+//! The yank handlers resolve their row from `Buffer::get_selected_row()`. That used to
+//! be backed by ratatui's `TableState`, which is only written by row navigation and by
+//! query execution - so a freshly loaded file, or a filter that leaves a single row
+//! (where j/k can never fire), left it as `None` and `yv`/`yy` refused to copy anything.
+
+use sql_cli::app_state_container::AppStateContainer;
+use sql_cli::buffer::{Buffer, BufferAPI};
+use sql_cli::data::data_view::DataView;
+use sql_cli::data::datatable::{DataColumn, DataRow, DataTable, DataValue};
+use sql_cli::ui::state::state_coordinator::StateCoordinator;
+use sql_cli::ui::viewport_manager::ViewportManager;
+use std::cell::RefCell;
+use std::sync::Arc;
+
+fn make_table() -> DataTable {
+    let mut table = DataTable::new("versions");
+    table.add_column(DataColumn::new("project"));
+    table.add_column(DataColumn::new("version"));
+
+    for (p, v) in [("alpha", "1.0.0"), ("nucleus", "2.3.4"), ("beta", "9.9.9")] {
+        table
+            .add_row(DataRow::new(vec![
+                DataValue::String(p.to_string()),
+                DataValue::String(v.to_string()),
+            ]))
+            .unwrap();
+    }
+    table
+}
+
+/// Mirrors `EnhancedTuiApp::new_with_dataview` / `add_dataview_with_refs`:
+/// a buffer created straight from a loaded file, with no query executed.
+fn make_container() -> AppStateContainer {
+    let table = make_table();
+    let mut buffer = Buffer::new(1);
+    buffer.set_datatable(Some(Arc::new(table.clone())));
+    buffer.set_dataview(Some(DataView::new(Arc::new(table))));
+
+    // Default() rather than new(): it uses CommandHistory::default(), which doesn't
+    // touch the shared history file and so can't race other tests.
+    let mut container = AppStateContainer::default();
+    container.buffers_mut().add_buffer(buffer);
+    container.buffers_mut().switch_to(0);
+    container.update_data_size(3, 2);
+    container
+}
+
+fn viewport_for(container: &AppStateContainer) -> RefCell<Option<ViewportManager>> {
+    RefCell::new(Some(ViewportManager::new(Arc::new(
+        container.get_buffer_dataview().unwrap().clone(),
+    ))))
+}
+
+fn selected_row(container: &AppStateContainer) -> Option<usize> {
+    container.current_buffer().unwrap().get_selected_row()
+}
+
+#[test]
+fn freshly_loaded_file_has_a_selected_row() {
+    let container = make_container();
+    assert_eq!(
+        selected_row(&container),
+        Some(0),
+        "yank must work immediately after loading a file, without pressing j/k first"
+    );
+}
+
+#[test]
+fn empty_results_have_no_selected_row() {
+    let mut container = make_container();
+    let vm = viewport_for(&container);
+
+    container.set_fuzzy_filter_pattern("zzzznomatch".to_string());
+    let (count, _) = StateCoordinator::apply_fuzzy_filter_with_refs(&mut container, &vm);
+
+    assert_eq!(count, 0);
+    assert_eq!(
+        selected_row(&container),
+        None,
+        "with no visible rows there is genuinely nothing to yank"
+    );
+}
+
+#[test]
+fn fuzzy_filter_to_single_row_keeps_a_selected_row() {
+    let mut container = make_container();
+    let vm = viewport_for(&container);
+
+    container.set_fuzzy_filter_pattern("nucleus".to_string());
+    let (count, _) = StateCoordinator::apply_fuzzy_filter_with_refs(&mut container, &vm);
+
+    assert_eq!(count, 1);
+    assert_eq!(selected_row(&container), Some(0));
+}
+
+#[test]
+fn text_filter_to_single_row_keeps_a_selected_row() {
+    let mut container = make_container();
+    let vm = viewport_for(&container);
+
+    let count = StateCoordinator::apply_text_filter_with_refs(&mut container, &vm, "nucleus");
+
+    assert_eq!(count, 1, "text filter should narrow to the single match");
+    assert_eq!(
+        selected_row(&container),
+        Some(0),
+        "the 'f' filter must land on the first match like the fuzzy filter does"
+    );
+}
+
+#[test]
+fn selection_is_clamped_to_the_filtered_view() {
+    let mut container = make_container();
+    let vm = viewport_for(&container);
+
+    // User navigated down to the last row before filtering.
+    container.set_selected_row(Some(2));
+
+    let count = StateCoordinator::apply_text_filter_with_refs(&mut container, &vm, "nucleus");
+    assert_eq!(count, 1);
+
+    let row = selected_row(&container).expect("one visible row means one selectable row");
+
+    // This is the lookup YankManager::yank_cell performs; it must hit the real cell
+    // rather than running off the end of the narrowed view and copying "NULL".
+    let view = container.get_buffer_dataview().unwrap();
+    assert_eq!(view.row_count(), 1);
+    assert_eq!(
+        view.get_cell_value(row, 1),
+        Some("2.3.4".to_string()),
+        "yank must read the visible row, not a stale index"
+    );
+}