From a403ca152b6268eacf2804c2d857ead16af37ef3 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Tue, 24 Feb 2026 10:35:13 +0200 Subject: tui: address review feedback for dashboard and export --- internal/statsengine/snapshot.go | 15 ++++++++++++ internal/tui/common/keys.go | 6 ++++- internal/tui/dashboard/files.go | 6 +++-- internal/tui/dashboard/model.go | 42 ++++++++++++++++++++++++++++----- internal/tui/dashboard/model_test.go | 40 +++++++++++++++++++++++++++++++ internal/tui/dashboard/overview.go | 17 ++++++++++--- internal/tui/dashboard/overview_test.go | 1 + internal/tui/dashboard/processes.go | 5 ++-- internal/tui/dashboard/syscalls.go | 5 ++-- internal/tui/dashboard/tabs.go | 5 ++-- internal/tui/export/model.go | 3 +-- internal/tui/pidpicker/model.go | 14 +++++------ internal/tui/tui.go | 38 ++++++++++++++++++++++------- 13 files changed, 161 insertions(+), 36 deletions(-) diff --git a/internal/statsengine/snapshot.go b/internal/statsengine/snapshot.go index 898b7f1..6583514 100644 --- a/internal/statsengine/snapshot.go +++ b/internal/statsengine/snapshot.go @@ -174,16 +174,31 @@ func (s Snapshot) Syscalls() []SyscallSnapshot { return slices.Clone(s.syscalls) } +// SyscallsCount returns number of syscall rows without cloning backing slices. +func (s Snapshot) SyscallsCount() int { + return len(s.syscalls) +} + // Files returns a defensive copy of per-file snapshot rows. func (s Snapshot) Files() []FileSnapshot { return slices.Clone(s.files) } +// FilesCount returns number of file rows without cloning backing slices. +func (s Snapshot) FilesCount() int { + return len(s.files) +} + // Processes returns a defensive copy of per-process snapshot rows. func (s Snapshot) Processes() []ProcessSnapshot { return slices.Clone(s.processes) } +// ProcessesCount returns number of process rows without cloning backing slices. +func (s Snapshot) ProcessesCount() int { + return len(s.processes) +} + // Buckets returns a defensive copy of histogram buckets. func (h HistogramSnapshot) Buckets() []HistogramBucketSnapshot { return slices.Clone(h.buckets) diff --git a/internal/tui/common/keys.go b/internal/tui/common/keys.go index 0dae542..a7ded71 100644 --- a/internal/tui/common/keys.go +++ b/internal/tui/common/keys.go @@ -52,7 +52,11 @@ func (k KeyMap) DashboardShortHelp() []key.Binding { func (k KeyMap) DashboardFullHelp() [][]key.Binding { return [][]key.Binding{ {k.One, k.Two, k.Three, k.Four, k.Five, k.Six}, - {k.Tab, k.ShiftTab, k.Export, k.Help, k.Quit}, + {k.Tab, k.ShiftTab, k.Export, k.Refresh, k.Help, k.Quit}, + { + key.NewBinding(key.WithKeys("j/k"), key.WithHelp("j/k", "scroll")), + key.NewBinding(key.WithKeys("up/down"), key.WithHelp("up/down", "scroll")), + }, } } diff --git a/internal/tui/dashboard/files.go b/internal/tui/dashboard/files.go index c52e887..9887f45 100644 --- a/internal/tui/dashboard/files.go +++ b/internal/tui/dashboard/files.go @@ -1,6 +1,7 @@ package dashboard import ( + "fmt" "ior/internal/statsengine" "strconv" @@ -37,8 +38,9 @@ func renderFilesWithOffset(snap *statsengine.Snapshot, width, height, offset int ) tbl.SetHeight(syscallTableHeight(height)) tbl.SetWidth(tableWidth(width)) - tbl.SetCursor(clampOffset(offset, len(rows))) - return tbl.View() + cursor := clampOffset(offset, len(rows)) + tbl.SetCursor(cursor) + return tbl.View() + fmt.Sprintf("\nRow %d/%d", cursor+1, len(rows)) } func fileRows(files []statsengine.FileSnapshot) []table.Row { diff --git a/internal/tui/dashboard/model.go b/internal/tui/dashboard/model.go index 9c47f4b..8eb7619 100644 --- a/internal/tui/dashboard/model.go +++ b/internal/tui/dashboard/model.go @@ -75,6 +75,9 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { ) case messages.StatsTickMsg: m.latest = msg.Snap + m.syscallsOffset = clampOffset(m.syscallsOffset, m.maxSyscallsRows()) + m.filesOffset = clampOffset(m.filesOffset, m.maxFilesRows()) + m.processesOffset = clampOffset(m.processesOffset, m.maxProcessesRows()) return m, nil case tea.KeyMsg: return m.handleKey(msg) @@ -86,7 +89,9 @@ func (m Model) handleKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { if m.activeTab == TabSyscalls { switch msg.String() { case "down", "j": - m.syscallsOffset++ + if m.syscallsOffset < m.maxSyscallsRows()-1 { + m.syscallsOffset++ + } return m, nil case "up", "k": if m.syscallsOffset > 0 { @@ -98,7 +103,9 @@ func (m Model) handleKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { if m.activeTab == TabProcesses { switch msg.String() { case "down", "j": - m.processesOffset++ + if m.processesOffset < m.maxProcessesRows()-1 { + m.processesOffset++ + } return m, nil case "up", "k": if m.processesOffset > 0 { @@ -110,7 +117,9 @@ func (m Model) handleKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { if m.activeTab == TabFiles { switch msg.String() { case "down", "j": - m.filesOffset++ + if m.filesOffset < m.maxFilesRows()-1 { + m.filesOffset++ + } return m, nil case "up", "k": if m.filesOffset > 0 { @@ -137,10 +146,34 @@ func (m Model) handleKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { m.activeTab = TabLatency case key.Matches(msg, m.keys.Six): m.activeTab = TabGaps + case key.Matches(msg, m.keys.Refresh): + snap := m.snapshot() + return m, func() tea.Msg { return messages.StatsTickMsg{Snap: snap} } } return m, nil } +func (m Model) maxSyscallsRows() int { + if m.latest == nil { + return 0 + } + return m.latest.SyscallsCount() +} + +func (m Model) maxFilesRows() int { + if m.latest == nil { + return 0 + } + return m.latest.FilesCount() +} + +func (m Model) maxProcessesRows() int { + if m.latest == nil { + return 0 + } + return m.latest.ProcessesCount() +} + func (m Model) snapshot() *statsengine.Snapshot { if m.engine == nil { return nil @@ -171,9 +204,6 @@ func tickCmd(d time.Duration) tea.Cmd { } func renderActiveTab(tab Tab, snap *statsengine.Snapshot, width, height, syscallsOffset, filesOffset, processesOffset int) string { - _ = width - _ = height - if snap == nil { return common.PanelStyle.Render(tab.String() + ": waiting for stats...") } diff --git a/internal/tui/dashboard/model_test.go b/internal/tui/dashboard/model_test.go index 11cfc2b..f1e6f35 100644 --- a/internal/tui/dashboard/model_test.go +++ b/internal/tui/dashboard/model_test.go @@ -46,6 +46,8 @@ func TestKeySwitchingChangesActiveTab(t *testing.T) { func TestSyscallsTabScrollsWithJK(t *testing.T) { m := NewModelWithConfig(nil, 250, common.DefaultKeyMap()) m.activeTab = TabSyscalls + snap := statsengine.NewSnapshot(nil, nil, nil, []statsengine.SyscallSnapshot{{Name: "read", Count: 1}, {Name: "write", Count: 1}}, nil, nil, statsengine.HistogramSnapshot{}, statsengine.HistogramSnapshot{}) + m.latest = &snap next, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'j'}}) model := next.(Model) @@ -63,6 +65,8 @@ func TestSyscallsTabScrollsWithJK(t *testing.T) { func TestProcessesTabScrollsWithJK(t *testing.T) { m := NewModelWithConfig(nil, 250, common.DefaultKeyMap()) m.activeTab = TabProcesses + snap := statsengine.NewSnapshot(nil, nil, nil, nil, nil, []statsengine.ProcessSnapshot{{PID: 1}, {PID: 2}}, statsengine.HistogramSnapshot{}, statsengine.HistogramSnapshot{}) + m.latest = &snap next, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'j'}}) model := next.(Model) @@ -80,6 +84,8 @@ func TestProcessesTabScrollsWithJK(t *testing.T) { func TestFilesTabScrollsWithJK(t *testing.T) { m := NewModelWithConfig(nil, 250, common.DefaultKeyMap()) m.activeTab = TabFiles + snap := statsengine.NewSnapshot(nil, nil, nil, nil, []statsengine.FileSnapshot{{Path: "/a"}, {Path: "/b"}}, nil, statsengine.HistogramSnapshot{}, statsengine.HistogramSnapshot{}) + m.latest = &snap next, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'j'}}) model := next.(Model) @@ -94,6 +100,40 @@ func TestFilesTabScrollsWithJK(t *testing.T) { } } +func TestScrollOffsetDoesNotGrowUnbounded(t *testing.T) { + m := NewModelWithConfig(nil, 250, common.DefaultKeyMap()) + m.activeTab = TabSyscalls + snap := statsengine.NewSnapshot(nil, nil, nil, []statsengine.SyscallSnapshot{{Name: "read", Count: 1}, {Name: "write", Count: 1}}, nil, nil, statsengine.HistogramSnapshot{}, statsengine.HistogramSnapshot{}) + m.latest = &snap + + for i := 0; i < 50; i++ { + next, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'j'}}) + m = next.(Model) + } + if m.syscallsOffset != 1 { + t.Fatalf("expected bounded offset 1, got %d", m.syscallsOffset) + } +} + +func TestRefreshKeyEmitsRefreshTick(t *testing.T) { + snap := &statsengine.Snapshot{TotalSyscalls: 13} + engine := &fakeSnapshotSource{snap: snap} + m := NewModelWithConfig(engine, 250, common.DefaultKeyMap()) + next, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'r'}}) + _ = next + if cmd == nil { + t.Fatalf("expected refresh command") + } + msg := cmd() + stats, ok := msg.(messages.StatsTickMsg) + if !ok { + t.Fatalf("expected StatsTickMsg from refresh key command, got %T", msg) + } + if stats.Snap != snap { + t.Fatalf("expected refreshed snapshot from engine") + } +} + func TestRefreshTickEmitsStatsTickMsg(t *testing.T) { snap := &statsengine.Snapshot{TotalSyscalls: 9} engine := &fakeSnapshotSource{snap: snap} diff --git a/internal/tui/dashboard/overview.go b/internal/tui/dashboard/overview.go index 8b6b13c..6d705da 100644 --- a/internal/tui/dashboard/overview.go +++ b/internal/tui/dashboard/overview.go @@ -6,6 +6,8 @@ import ( common "ior/internal/tui/common" "strings" "time" + + "github.com/charmbracelet/lipgloss" ) func renderOverview(snap *statsengine.Snapshot, width, height int) string { @@ -19,7 +21,7 @@ func renderOverview(snap *statsengine.Snapshot, width, height int) string { box2 := renderBytesBox(snap, boxWidth) box3 := renderErrorBox(snap, boxWidth) - row := strings.Join([]string{box1, box2, box3}, "\n") + row := lipgloss.JoinHorizontal(lipgloss.Top, box1, box2, box3) trends := fmt.Sprintf( "Trends: latency %s gap %s throughput %s", trendWithArrow(snap.LatencyTrend), @@ -28,6 +30,7 @@ func renderOverview(snap *statsengine.Snapshot, width, height int) string { ) latencySpark := "Latency: " + renderSparkline(snap.LatencySeriesNs(), sparklineWidth(width)) + gapSpark := "Gap: " + renderSparkline(snap.GapSeriesNs(), sparklineWidth(width)) throughputSpark := "Throughput: " + renderSparkline(snap.ThroughputSeriesB(), sparklineWidth(width)) topSyscalls := "Top syscalls: " + summarizeTopSyscalls(snap) topFiles := "Top files: " + summarizeTopFiles(snap) @@ -40,6 +43,7 @@ func renderOverview(snap *statsengine.Snapshot, width, height int) string { row, common.HighlightStyle.Render(trends), common.PanelStyle.Render(latencySpark), + common.PanelStyle.Render(gapSpark), common.PanelStyle.Render(throughputSpark), common.PanelStyle.Render(topSyscalls), common.PanelStyle.Render(topFiles), @@ -52,11 +56,16 @@ func renderOverview(snap *statsengine.Snapshot, width, height int) string { } func renderSyscallBox(snap *statsengine.Snapshot, width int) string { + generatedAt := "n/a" + if !snap.GeneratedAt.IsZero() { + generatedAt = snap.GeneratedAt.Format("15:04:05") + } content := fmt.Sprintf( - "Elapsed: %s\nSyscalls: %d\nRate: %.1f/s", + "Elapsed: %s\nSyscalls: %d\nRate: %.1f/s\nSnapshot: %s", formatElapsed(snap.Elapsed), snap.TotalSyscalls, snap.SyscallRatePerSec, + generatedAt, ) return common.PanelStyle.Width(width).Render(content) } @@ -77,10 +86,12 @@ func renderErrorBox(snap *statsengine.Snapshot, width int) string { errPercent = float64(snap.TotalErrors) / float64(snap.TotalSyscalls) * 100 } content := fmt.Sprintf( - "Errors: %d\nError rate: %.2f%%\nLatency mean: %.0fns", + "Errors: %d\nError rate: %.2f%%\nError/s: %.2f\nLatency mean: %.0fns\nGap mean: %.0fns", snap.TotalErrors, errPercent, + snap.ErrorRatePerSec, snap.LatencyMeanNs, + snap.GapMeanNs, ) return common.PanelStyle.Width(width).Render(content) } diff --git a/internal/tui/dashboard/overview_test.go b/internal/tui/dashboard/overview_test.go index e44b015..cee6cf2 100644 --- a/internal/tui/dashboard/overview_test.go +++ b/internal/tui/dashboard/overview_test.go @@ -31,6 +31,7 @@ func TestRenderOverviewIncludesCoreMetrics(t *testing.T) { "Errors:", "Trends:", "Latency:", + "Gap:", "Throughput:", "Top syscalls:", "Top files:", diff --git a/internal/tui/dashboard/processes.go b/internal/tui/dashboard/processes.go index d229c10..03a38f1 100644 --- a/internal/tui/dashboard/processes.go +++ b/internal/tui/dashboard/processes.go @@ -40,9 +40,10 @@ func renderProcessesWithOffset(snap *statsengine.Snapshot, width, height, offset ) tbl.SetHeight(syscallTableHeight(height)) tbl.SetWidth(tableWidth(width)) - tbl.SetCursor(clampOffset(offset, len(rows))) + cursor := clampOffset(offset, len(rows)) + tbl.SetCursor(cursor) - out := tbl.View() + out := tbl.View() + fmt.Sprintf("\nRow %d/%d", cursor+1, len(rows)) if flags.Get().PidFilter > 0 { out += "\n" + "Note: this tab is most useful with All PIDs." } diff --git a/internal/tui/dashboard/syscalls.go b/internal/tui/dashboard/syscalls.go index f25781e..e40c2e7 100644 --- a/internal/tui/dashboard/syscalls.go +++ b/internal/tui/dashboard/syscalls.go @@ -45,8 +45,9 @@ func renderSyscallsWithOffset(snap *statsengine.Snapshot, width, height, offset ) tbl.SetHeight(syscallTableHeight(height)) tbl.SetWidth(tableWidth(width)) - tbl.SetCursor(clampOffset(offset, len(rows))) - return tbl.View() + cursor := clampOffset(offset, len(rows)) + tbl.SetCursor(cursor) + return tbl.View() + fmt.Sprintf("\nRow %d/%d", cursor+1, len(rows)) } func syscallRows(syscalls []statsengine.SyscallSnapshot) []table.Row { diff --git a/internal/tui/dashboard/tabs.go b/internal/tui/dashboard/tabs.go index 9965d1f..799e9f1 100644 --- a/internal/tui/dashboard/tabs.go +++ b/internal/tui/dashboard/tabs.go @@ -1,6 +1,7 @@ package dashboard import ( + "fmt" common "ior/internal/tui/common" "strings" @@ -77,8 +78,8 @@ func tabIndex(tab Tab) int { func renderTabBar(active Tab, width int) string { parts := make([]string, 0, len(allTabs)) - for _, tab := range allTabs { - label := tab.String() + for i, tab := range allTabs { + label := fmt.Sprintf("%d:%s", i+1, tab.String()) if tab == active { parts = append(parts, common.TabActiveStyle.Render(label)) } else { diff --git a/internal/tui/export/model.go b/internal/tui/export/model.go index 1e875e5..57612db 100644 --- a/internal/tui/export/model.go +++ b/internal/tui/export/model.go @@ -13,8 +13,7 @@ import ( type Option int const ( - OptionFlamegraph Option = iota - OptionCSV + OptionCSV Option = iota OptionCancel ) diff --git a/internal/tui/pidpicker/model.go b/internal/tui/pidpicker/model.go index 34da674..37af257 100644 --- a/internal/tui/pidpicker/model.go +++ b/internal/tui/pidpicker/model.go @@ -2,6 +2,7 @@ package pidpicker import ( "fmt" + common "ior/internal/tui/common" "ior/internal/tui/messages" "strings" @@ -34,14 +35,11 @@ func (k KeyMap) PickerShortHelp() []key.Binding { } var ( - screenStyle = lipgloss.NewStyle() - headerStyle = lipgloss.NewStyle().Bold(true).Foreground(lipgloss.Color("75")) - helpBarStyle = lipgloss.NewStyle(). - Foreground(lipgloss.Color("246")). - BorderTop(true). - BorderForeground(lipgloss.Color("238")) - highlightStyle = lipgloss.NewStyle().Bold(true).Foreground(lipgloss.Color("222")) - errorStyle = lipgloss.NewStyle().Bold(true).Foreground(lipgloss.Color("203")) + screenStyle = common.ScreenStyle + headerStyle = common.HeaderStyle + helpBarStyle = common.HelpBarStyle + highlightStyle = common.HighlightStyle + errorStyle = common.ErrorStyle ) type processesLoadedMsg struct { diff --git a/internal/tui/tui.go b/internal/tui/tui.go index 5bc7bf9..d988fa1 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -299,12 +299,6 @@ func runExportCmd(option tuiexport.Option, snap *statsengine.Snapshot) tea.Cmd { return tuiexport.FailedMsg{Err: err} } return tuiexport.CompletedMsg{Path: path} - case tuiexport.OptionFlamegraph: - path, err := exportFlamegraph() - if err != nil { - return tuiexport.FailedMsg{Err: err} - } - return tuiexport.CompletedMsg{Path: path} default: return tuiexport.FailedMsg{Err: errors.New("unknown export option")} } @@ -335,6 +329,8 @@ func exportSnapshotCSV(snap *statsengine.Snapshot) (string, error) { {"section", "name", "value1", "value2", "value3"}, {"summary", "totals", fmt.Sprint(snapValue(snap, func(s *statsengine.Snapshot) uint64 { return s.TotalSyscalls })), fmt.Sprint(snapValue(snap, func(s *statsengine.Snapshot) uint64 { return s.TotalErrors })), fmt.Sprint(snapValue(snap, func(s *statsengine.Snapshot) uint64 { return s.TotalBytes }))}, {"summary", "rates_per_sec", fmt.Sprintf("%.2f", snapValueF(snap, func(s *statsengine.Snapshot) float64 { return s.SyscallRatePerSec })), fmt.Sprintf("%.2f", snapValueF(snap, func(s *statsengine.Snapshot) float64 { return s.ReadBytesPerSec })), fmt.Sprintf("%.2f", snapValueF(snap, func(s *statsengine.Snapshot) float64 { return s.WriteBytesPerSec }))}, + {"summary", "latency_gap_mean_ns", fmt.Sprintf("%.2f", snapValueF(snap, func(s *statsengine.Snapshot) float64 { return s.LatencyMeanNs })), fmt.Sprintf("%.2f", snapValueF(snap, func(s *statsengine.Snapshot) float64 { return s.GapMeanNs })), ""}, + {"summary", "trend", trendSummary(snap, func(s *statsengine.Snapshot) statsengine.Trend { return s.LatencyTrend }), trendSummary(snap, func(s *statsengine.Snapshot) statsengine.Trend { return s.GapTrend }), trendSummary(snap, func(s *statsengine.Snapshot) statsengine.Trend { return s.ThroughputTrend })}, } for _, row := range rows { if err := w.Write(row); err != nil { @@ -347,16 +343,38 @@ func exportSnapshotCSV(snap *statsengine.Snapshot) (string, error) { if err := w.Write([]string{"syscall", s.Name, fmt.Sprint(s.Count), fmt.Sprintf("%.2f", s.RatePerSec), fmt.Sprint(s.Bytes)}); err != nil { return "", err } + if err := w.Write([]string{"syscall_latency_ns", s.Name, fmt.Sprintf("%.2f", s.LatencyMeanNs), fmt.Sprint(s.LatencyMinNs), fmt.Sprint(s.LatencyMaxNs)}); err != nil { + return "", err + } + if err := w.Write([]string{"syscall_percentiles_ns", s.Name, fmt.Sprint(s.LatencyP50Ns), fmt.Sprint(s.LatencyP95Ns), fmt.Sprint(s.LatencyP99Ns)}); err != nil { + return "", err + } } for _, r := range snap.Files() { if err := w.Write([]string{"file", r.Path, fmt.Sprint(r.Accesses), fmt.Sprint(r.BytesRead), fmt.Sprint(r.BytesWritten)}); err != nil { return "", err } + if err := w.Write([]string{"file_latency_ns", r.Path, fmt.Sprintf("%.2f", r.AvgLatencyNs), fmt.Sprint(r.MaxLatencyNs), ""}); err != nil { + return "", err + } } for _, p := range snap.Processes() { if err := w.Write([]string{"process", fmt.Sprint(p.PID), fmt.Sprint(p.Syscalls), fmt.Sprintf("%.2f", p.RatePerSec), fmt.Sprint(p.Bytes)}); err != nil { return "", err } + if err := w.Write([]string{"process_latency_ns", fmt.Sprint(p.PID), fmt.Sprintf("%.2f", p.AvgLatencyNs), "", ""}); err != nil { + return "", err + } + } + for _, b := range snap.LatencyHistogram.Buckets() { + if err := w.Write([]string{"latency_hist", b.Label, fmt.Sprint(b.Count), fmt.Sprint(b.LowerNs), fmt.Sprint(b.UpperNs)}); err != nil { + return "", err + } + } + for _, b := range snap.GapHistogram.Buckets() { + if err := w.Write([]string{"gap_hist", b.Label, fmt.Sprint(b.Count), fmt.Sprint(b.LowerNs), fmt.Sprint(b.UpperNs)}); err != nil { + return "", err + } } } @@ -381,8 +399,12 @@ func snapValueF(snap *statsengine.Snapshot, get func(*statsengine.Snapshot) floa return get(snap) } -func exportFlamegraph() (string, error) { - return "", errors.New("flamegraph export is not yet available in TUI mode") +func trendSummary(snap *statsengine.Snapshot, get func(*statsengine.Snapshot) statsengine.Trend) string { + if snap == nil { + return "stable:0.00" + } + trend := get(snap) + return fmt.Sprintf("%s:%.2f", trend.Direction, trend.DeltaPercent) } func renderHelpOverlay(width, height int, groups [][]key.Binding) string { -- cgit v1.2.3