Fix stale fetch race condition and empty-table search restore

Author: simonfaltum

libs/tableview/paginated.go

@@ -19,9 +19,10 @@ const (
 
 // rowsFetchedMsg carries newly fetched rows from the iterator.
 type rowsFetchedMsg struct {
-	rows      [][]string
-	exhausted bool
-	err       error
+	rows       [][]string
+	exhausted  bool
+	err        error
+	generation int
 }
 
 type paginatedModel struct {
@@ -38,20 +39,22 @@ type paginatedModel struct {
 	err       error
 
 	// Fetch state
-	rowIter        RowIterator
-	makeFetchCmd   func(m paginatedModel) tea.Cmd // closure capturing ctx
-	makeSearchIter func(query string) RowIterator // closure capturing ctx
+	rowIter         RowIterator
+	makeFetchCmd    func(m paginatedModel) tea.Cmd // closure capturing ctx
+	makeSearchIter  func(query string) RowIterator // closure capturing ctx
+	fetchGeneration int
 
 	// Display
 	cursor int
 	widths []int
 
 	// Search
-	searching    bool
-	searchInput  string
-	savedRows    [][]string
-	savedIter    RowIterator
-	savedExhaust bool
+	searching      bool
+	searchInput    string
+	hasSearchState bool
+	savedRows      [][]string
+	savedIter      RowIterator
+	savedExhaust   bool
 
 	// Limits
 	maxItems     int
@@ -64,6 +67,7 @@ func newFetchCmdFunc(ctx context.Context) func(paginatedModel) tea.Cmd {
 		iter := m.rowIter
 		currentLen := len(m.rows)
 		maxItems := m.maxItems
+		generation := m.fetchGeneration
 
 		return func() tea.Msg {
 			var rows [][]string
@@ -73,7 +77,7 @@ func newFetchCmdFunc(ctx context.Context) func(paginatedModel) tea.Cmd {
 			if maxItems > 0 {
 				remaining := maxItems - currentLen
 				if remaining <= 0 {
-					return rowsFetchedMsg{exhausted: true}
+					return rowsFetchedMsg{exhausted: true, generation: generation}
 				}
 				limit = min(limit, remaining)
 			}
@@ -85,7 +89,7 @@ func newFetchCmdFunc(ctx context.Context) func(paginatedModel) tea.Cmd {
 				}
 				row, err := iter.Next(ctx)
 				if err != nil {
-					return rowsFetchedMsg{err: err}
+					return rowsFetchedMsg{err: err, generation: generation}
 				}
 				rows = append(rows, row)
 			}
@@ -94,7 +98,7 @@ func newFetchCmdFunc(ctx context.Context) func(paginatedModel) tea.Cmd {
 				exhausted = true
 			}
 
-			return rowsFetchedMsg{rows: rows, exhausted: exhausted}
+			return rowsFetchedMsg{rows: rows, exhausted: exhausted, generation: generation}
 		}
 	}
 }
@@ -160,6 +164,9 @@ func (m paginatedModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		return m, nil
 
 	case rowsFetchedMsg:
+		if msg.generation != m.fetchGeneration {
+			return m, nil
+		}
 		m.loading = false
 		if msg.err != nil {
 			m.err = msg.err
@@ -345,25 +352,31 @@ func (m paginatedModel) updateSearch(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
 		query := m.searchInput
 		if query == "" {
 			// Restore original state
-			if m.savedRows != nil {
+			if m.hasSearchState {
+				m.fetchGeneration++
 				m.rows = m.savedRows
 				m.rowIter = m.savedIter
 				m.exhausted = m.savedExhaust
+				m.loading = false
+				m.hasSearchState = false
 				m.savedRows = nil
 				m.savedIter = nil
+				m.savedExhaust = false
 				m.cursor = 0
 				m.viewport.SetContent(m.renderContent())
 				m.viewport.GotoTop()
 			}
 			return m, nil
 		}
 		// Save current state
-		if m.savedRows == nil {
+		if !m.hasSearchState {
+			m.hasSearchState = true
 			m.savedRows = m.rows
 			m.savedIter = m.rowIter
 			m.savedExhaust = m.exhausted
 		}
 		// Create new iterator with search
+		m.fetchGeneration++
 		m.rows = nil
 		m.exhausted = false
 		m.loading = false

libs/tableview/paginated_test.go

@@ -273,7 +273,8 @@ func TestPaginatedSearchEnterAndRestore(t *testing.T) {
 	assert.False(t, pm.searching)
 	assert.True(t, searchCalled)
 	assert.NotNil(t, cmd)
-	assert.NotNil(t, pm.savedRows)
+	assert.True(t, pm.hasSearchState)
+	assert.Equal(t, 1, pm.fetchGeneration)
 
 	// Restore by submitting empty search
 	pm.searching = true
@@ -283,7 +284,60 @@ func TestPaginatedSearchEnterAndRestore(t *testing.T) {
 	pm = result.(paginatedModel)
 
 	assert.Equal(t, [][]string{{"original"}}, pm.rows)
+	assert.False(t, pm.hasSearchState)
 	assert.Nil(t, pm.savedRows)
+	assert.Equal(t, 2, pm.fetchGeneration)
+}
+
+func TestPaginatedSearchRestoreEmptyOriginalTable(t *testing.T) {
+	cfg := &TableConfig{
+		Columns: []ColumnDef{
+			{Header: "Name"},
+		},
+		Search: &SearchConfig{
+			Placeholder: "search...",
+			NewIterator: func(_ context.Context, query string) RowIterator {
+				return &stringRowIterator{rows: [][]string{{"found:" + query}}}
+			},
+		},
+	}
+
+	ctx := t.Context()
+	originalIter := &stringRowIterator{}
+	m := paginatedModel{
+		cfg:            cfg,
+		headers:        []string{"Name"},
+		rowIter:        originalIter,
+		makeFetchCmd:   newFetchCmdFunc(ctx),
+		makeSearchIter: newSearchIterFunc(ctx, cfg.Search),
+		exhausted:      true,
+		ready:          true,
+	}
+	m.viewport.Width = 80
+	m.viewport.Height = 20
+
+	m.searching = true
+	m.searchInput = "test"
+
+	result, cmd := m.updateSearch(tea.KeyMsg{Type: tea.KeyEnter})
+	pm := result.(paginatedModel)
+
+	assert.NotNil(t, cmd)
+	assert.True(t, pm.hasSearchState)
+	assert.Nil(t, pm.savedRows)
+	assert.Equal(t, 1, pm.fetchGeneration)
+
+	pm.searching = true
+	pm.searchInput = ""
+	pm.rows = [][]string{{"found:test"}}
+	result, _ = pm.updateSearch(tea.KeyMsg{Type: tea.KeyEnter})
+	pm = result.(paginatedModel)
+
+	assert.Nil(t, pm.rows)
+	assert.Equal(t, originalIter, pm.rowIter)
+	assert.True(t, pm.exhausted)
+	assert.False(t, pm.hasSearchState)
+	assert.Equal(t, 2, pm.fetchGeneration)
 }
 
 func TestPaginatedSearchEscCancels(t *testing.T) {
@@ -389,6 +443,28 @@ func TestMaybeFetchSetsLoadingAndPreventsDoubleFetch(t *testing.T) {
 	assert.Nil(t, cmd2, "should not trigger second fetch while loading")
 }
 
+func TestPaginatedIgnoresStaleFetchMessages(t *testing.T) {
+	m := newTestModel(t, nil, 0)
+	m.ready = true
+	m.viewport.Width = 80
+	m.viewport.Height = 20
+	m.rows = [][]string{{"search", "1"}}
+	m.widths = []int{6, 1}
+	m.loading = true
+	m.fetchGeneration = 1
+
+	result, _ := m.Update(rowsFetchedMsg{
+		rows:       [][]string{{"stale", "2"}},
+		exhausted:  true,
+		generation: 0,
+	})
+	pm := result.(paginatedModel)
+
+	assert.Equal(t, [][]string{{"search", "1"}}, pm.rows)
+	assert.False(t, pm.exhausted)
+	assert.True(t, pm.loading)
+}
+
 func TestFetchCmdWithIterator(t *testing.T) {
 	rows := make([][]string, 60)
 	for i := range rows {
@@ -406,6 +482,7 @@ func TestFetchCmdWithIterator(t *testing.T) {
 	require.True(t, ok)
 
 	assert.NoError(t, fetched.err)
+	assert.Equal(t, 0, fetched.generation)
 	assert.Len(t, fetched.rows, fetchBatchSize)
 	assert.False(t, fetched.exhausted, "iterator should have more rows")
 }
@@ -422,6 +499,7 @@ func TestFetchCmdExhaustsSmallIterator(t *testing.T) {
 	require.True(t, ok)
 
 	assert.NoError(t, fetched.err)
+	assert.Equal(t, 0, fetched.generation)
 	assert.Len(t, fetched.rows, 2)
 	assert.True(t, fetched.exhausted, "small iterator should be exhausted")
 }