Skip to content

Commit c5a097c

Browse files
fix: address review findings for prefetch and column width
- Short-circuit countVisibleJobsAfter once threshold reached (perf) - Extract maybePrefetch helper to deduplicate prefetch logic - Clamp flex columns to 1 char when no remaining space (narrow terms) - Add clarifying comment about spacing dual accounting in colWidths Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent d2ebfc2 commit c5a097c

File tree

3 files changed

+31
-13
lines changed

3 files changed

+31
-13
lines changed

cmd/roborev/tui/handlers.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -308,10 +308,8 @@ func (m model) handleDownKey() (tea.Model, tea.Cmd) {
308308
if nextIdx >= 0 {
309309
m.selectedIdx = nextIdx
310310
m.updateSelectedJobID()
311-
// Prefetch more jobs when approaching the end of loaded data
312-
if m.canPaginate() && m.countVisibleJobsAfter(nextIdx) < queuePrefetchBuffer {
313-
m.loadingMore = true
314-
return m, m.fetchMoreJobs()
311+
if cmd := m.maybePrefetch(nextIdx); cmd != nil {
312+
return m, cmd
315313
}
316314
} else if m.canPaginate() {
317315
m.loadingMore = true
@@ -344,10 +342,8 @@ func (m model) handleNextKey() (tea.Model, tea.Cmd) {
344342
if nextIdx >= 0 {
345343
m.selectedIdx = nextIdx
346344
m.updateSelectedJobID()
347-
// Prefetch more jobs when approaching the end of loaded data
348-
if m.canPaginate() && m.countVisibleJobsAfter(nextIdx) < queuePrefetchBuffer {
349-
m.loadingMore = true
350-
return m, m.fetchMoreJobs()
345+
if cmd := m.maybePrefetch(nextIdx); cmd != nil {
346+
return m, cmd
351347
}
352348
} else if m.canPaginate() {
353349
m.loadingMore = true

cmd/roborev/tui/nav.go

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package tui
22

3-
import "github.com/roborev-dev/roborev/internal/storage"
3+
import (
4+
tea "github.com/charmbracelet/bubbletea"
5+
"github.com/roborev-dev/roborev/internal/storage"
6+
)
47

58
// updateSelectedJobID updates the tracked job ID after navigation
69
func (m *model) updateSelectedJobID() {
@@ -192,17 +195,32 @@ func (m model) findPrevVisibleJob(currentIdx int) int {
192195
return -1
193196
}
194197

195-
// countVisibleJobsAfter returns how many visible jobs exist after currentIdx.
198+
// countVisibleJobsAfter returns the number of visible jobs after currentIdx,
199+
// short-circuiting once the count reaches queuePrefetchBuffer since callers
200+
// only need to know whether the count is below that threshold.
196201
func (m model) countVisibleJobsAfter(currentIdx int) int {
197202
count := 0
198203
for i := currentIdx + 1; i < len(m.jobs); i++ {
199204
if m.isJobVisible(m.jobs[i]) {
200205
count++
206+
if count >= queuePrefetchBuffer {
207+
return count
208+
}
201209
}
202210
}
203211
return count
204212
}
205213

214+
// maybePrefetch triggers a page fetch if the cursor is near the end of loaded
215+
// data. Returns a tea.Cmd if a fetch was started, nil otherwise.
216+
func (m *model) maybePrefetch(idx int) tea.Cmd {
217+
if m.canPaginate() && m.countVisibleJobsAfter(idx) < queuePrefetchBuffer {
218+
m.loadingMore = true
219+
return m.fetchMoreJobs()
220+
}
221+
return nil
222+
}
223+
206224
// findFirstVisibleJob returns the index of the first visible job.
207225
func (m model) findFirstVisibleJob() int {
208226
for i := range m.jobs {

cmd/roborev/tui/render_queue.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,10 @@ func (m model) renderQueueView() string {
322322
}
323323

324324
remaining := m.width - totalFixed
325-
// Distribute remaining space among flex columns
325+
// Distribute remaining space among flex columns.
326+
// colWidths stores content-only width; StyleFunc adds spacing via
327+
// s.Width(w + spacing(col, logicalCol)) so the total column width
328+
// on screen = content width + inter-column spacing.
326329
colWidths := make(map[int]int, len(visCols))
327330
for c, fw := range fixedWidth {
328331
colWidths[c] = fw
@@ -352,10 +355,11 @@ func (m model) renderQueueView() string {
352355
}
353356
}
354357
} else if totalFlex > 0 {
355-
// No remaining space: give flex columns minimal width
358+
// No remaining space: give flex columns 1 char each to avoid
359+
// overflow at very narrow terminal widths.
356360
for _, c := range flexCols {
357361
if !m.hiddenColumns[c] {
358-
colWidths[c] = max(contentWidth[c], 1)
362+
colWidths[c] = 1
359363
}
360364
}
361365
}

0 commit comments

Comments
 (0)