From 14271258977563fc4d9a17926f252d8f4b302968 Mon Sep 17 00:00:00 2001 From: Umputun Date: Fri, 17 Apr 2026 12:10:43 -0500 Subject: [PATCH 1/2] fix(ui): suppress initial loading flash and drop stale file-list responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bubble Tea delivered the first WindowSizeMsg before filesLoadedMsg, so between ready=true and the async ChangedFiles returning, View() painted the populated two-pane layout with an empty tree and "no file selected" — a 100-500 ms flash on larger repos. Gate View() on a new filesLoaded flag: show "loading..." while !ready, then "loading files..." while ready && !filesLoaded. handleFilesLoaded flips the flag first thing, including on error, so the loading screen always exits. Also tag filesLoadedMsg with a seq captured at load time and drop mismatched messages in handleFilesLoaded. toggleUntracked (and any future reload site) bumps filesLoadSeq so an older in-flight ChangedFiles can't overwrite the tree after the user toggles state. --- app/ui/diffview_test.go | 1 + app/ui/loaders.go | 15 +++++++++++++-- app/ui/loaders_test.go | 38 ++++++++++++++++++++++++++++++++++++++ app/ui/model.go | 6 +++++- app/ui/model_test.go | 27 +++++++++++++++++++++++++++ app/ui/view.go | 7 +++++++ docs/ARCHITECTURE.md | 9 ++++++++- 7 files changed, 99 insertions(+), 4 deletions(-) diff --git a/app/ui/diffview_test.go b/app/ui/diffview_test.go index b7f5885..1510cd3 100644 --- a/app/ui/diffview_test.go +++ b/app/ui/diffview_test.go @@ -614,6 +614,7 @@ func TestModel_PlainStyles(t *testing.T) { m.layout.height = 40 m.layout.treeWidth = 36 m.ready = true + m.filesLoaded = true // plain styles should not panic and should render output := m.View() assert.NotEmpty(t, output) diff --git a/app/ui/loaders.go b/app/ui/loaders.go index 92dab5a..899dd83 100644 --- a/app/ui/loaders.go +++ b/app/ui/loaders.go @@ -15,12 +15,16 @@ import ( // loadFiles returns a command that fetches the list of changed files from the renderer. // it also appends staged-only and untracked files when applicable. +// the caller must bump m.filesLoadSeq before invoking loadFiles when issuing a new +// reload (e.g. toggleUntracked); the captured seq tags every emitted filesLoadedMsg +// so handleFilesLoaded can drop stale results from earlier in-flight loads. func (m Model) loadFiles() tea.Cmd { + seq := m.filesLoadSeq return func() tea.Msg { var warnings []string entries, err := m.diffRenderer.ChangedFiles(m.cfg.ref, m.cfg.staged) if err != nil { - return filesLoadedMsg{entries: entries, err: err} + return filesLoadedMsg{seq: seq, entries: entries, err: err} } // include staged-only files (new files added to index but not yet committed) // only when there are no unstaged entries; otherwise unstaged review should stay focused @@ -58,7 +62,7 @@ func (m Model) loadFiles() tea.Cmd { } } } - return filesLoadedMsg{entries: entries, warnings: warnings} + return filesLoadedMsg{seq: seq, entries: entries, warnings: warnings} } } @@ -99,6 +103,13 @@ func (m Model) loadSelectedIfChanged() (tea.Model, tea.Cmd) { // handleFilesLoaded processes the result of loadFiles, populating the file tree // and triggering the initial file diff load. func (m Model) handleFilesLoaded(msg filesLoadedMsg) (tea.Model, tea.Cmd) { + // drop stale responses from an earlier load; only the latest load request + // (matching m.filesLoadSeq) is accepted. Prevents an older in-flight load from + // overwriting the tree after toggleUntracked (or a rapid double-toggle). + if msg.seq != m.filesLoadSeq { + return m, nil + } + m.filesLoaded = true if msg.err != nil { m.layout.viewport.SetContent(fmt.Sprintf("error loading files: %v", msg.err)) return m, nil diff --git a/app/ui/loaders_test.go b/app/ui/loaders_test.go index a3cfd79..064ee80 100644 --- a/app/ui/loaders_test.go +++ b/app/ui/loaders_test.go @@ -32,12 +32,50 @@ func TestModel_FilesLoaded(t *testing.T) { func TestModel_FilesLoadedError(t *testing.T) { m := testModel(nil, nil) m.ready = true + m.filesLoaded = false result, cmd := m.Update(filesLoadedMsg{err: assert.AnError}) model := result.(Model) assert.Nil(t, cmd) assert.Equal(t, 0, model.tree.TotalFiles()) + // filesLoaded must flip even on error, otherwise View() stays on "loading files..." forever + assert.True(t, model.filesLoaded, "filesLoaded must be set before the error early-return so the loading screen exits") +} + +func TestModel_FilesLoaded_DropsStaleResponses(t *testing.T) { + // regression: a slow first load (seq=0) must not overwrite the tree after + // a newer load (seq=1) was issued — e.g. user toggled untracked immediately after startup. + m := testModel(nil, nil) + m.filesLoaded = false + m.filesLoadSeq = 1 // simulate a newer load already dispatched (e.g. toggleUntracked) + + // stale response (seq=0) arrives first — must be dropped + stale := []diff.FileEntry{{Path: "stale.go"}} + result, cmd := m.Update(filesLoadedMsg{seq: 0, entries: stale}) + model := result.(Model) + assert.Nil(t, cmd) + assert.False(t, model.filesLoaded, "stale response must not flip filesLoaded") + assert.Equal(t, 0, model.tree.TotalFiles(), "stale entries must not populate tree") + assert.Equal(t, "loading files...", model.View(), "View must still show loading while the current load is pending") + + // fresh response (seq=1) arrives — accepted + fresh := []diff.FileEntry{{Path: "fresh.go"}} + result, _ = m.Update(filesLoadedMsg{seq: 1, entries: fresh}) + model = result.(Model) + assert.True(t, model.filesLoaded) + assert.Equal(t, 1, model.tree.TotalFiles()) +} + +func TestModel_ToggleUntrackedBumpsFilesLoadSeq(t *testing.T) { + // regression: toggleUntracked must bump filesLoadSeq so any in-flight load + // from before the toggle is treated as stale by handleFilesLoaded. + m := testModel(nil, nil) + m.loadUntracked = func() ([]string, error) { return nil, nil } + before := m.filesLoadSeq + cmd := m.toggleUntracked() + require.NotNil(t, cmd) + assert.Greater(t, m.filesLoadSeq, before, "toggleUntracked must bump filesLoadSeq") } func TestModel_FilesLoadedMultipleFiles(t *testing.T) { diff --git a/app/ui/model.go b/app/ui/model.go index 2dedf62..96148af 100644 --- a/app/ui/model.go +++ b/app/ui/model.go @@ -316,7 +316,9 @@ type Model struct { search searchState // search lifecycle state annot annotationState // annotation input lifecycle state - ready bool // true after first WindowSizeMsg + ready bool // true after first WindowSizeMsg + filesLoaded bool // true after the first filesLoadedMsg is handled (keeps the loading view pinned until real data arrives) + filesLoadSeq uint64 // bumped before each new file-list load; stale filesLoadedMsg (seq mismatch) is dropped blamer Blamer // optional blame provider (nil when git unavailable) loadUntracked func() ([]string, error) // fetches untracked files; nil when unavailable @@ -349,6 +351,7 @@ type blameLoadedMsg struct { // filesLoadedMsg is sent when the changed file list is loaded. type filesLoadedMsg struct { + seq uint64 // matches m.filesLoadSeq at the time the load was issued; mismatched messages are dropped entries []diff.FileEntry err error warnings []string // non-fatal issues (staged/untracked fetch failures) @@ -790,6 +793,7 @@ func (m *Model) toggleWordDiff() { // toggleUntracked toggles visibility of untracked files in the tree. func (m *Model) toggleUntracked() tea.Cmd { m.modes.showUntracked = !m.modes.showUntracked + m.filesLoadSeq++ return m.loadFiles() } diff --git a/app/ui/model_test.go b/app/ui/model_test.go index b10d3fe..3015e91 100644 --- a/app/ui/model_test.go +++ b/app/ui/model_test.go @@ -159,6 +159,7 @@ func testModel(files []string, fileDiffs map[string][]diff.DiffLine) Model { m.layout.height = 40 m.layout.treeWidth = m.layout.width * m.cfg.treeWidthRatio / 10 m.ready = true + m.filesLoaded = true return m } @@ -264,6 +265,32 @@ func TestModel_Init(t *testing.T) { assert.NoError(t, flm.err) } +func TestModel_InitialLoadingState_NoEmptyFlash(t *testing.T) { + // regression: before filesLoadedMsg is handled, View() must not paint the two-pane + // layout with an empty tree and "no file selected". That made users see a 100-500 ms + // flash of a "no changes" screen on launch. See app/ui/view.go and handleFilesLoaded. + m := testModel([]string{"a.go", "b.go"}, nil) + // undo the testModel shortcut to simulate a real program start + m.ready = false + m.filesLoaded = false + + // before WindowSizeMsg: generic loading string + assert.Equal(t, "loading...", m.View()) + + // WindowSizeMsg arrives; filesLoadedMsg has not + result, _ := m.Update(tea.WindowSizeMsg{Width: 120, Height: 40}) + m = result.(Model) + assert.Equal(t, "loading files...", m.View()) + + // filesLoadedMsg arrives; loading state must end + result, _ = m.Update(filesLoadedMsg{entries: []diff.FileEntry{{Path: "a.go"}, {Path: "b.go"}}}) + m = result.(Model) + assert.True(t, m.filesLoaded) + got := m.View() + assert.NotEqual(t, "loading...", got) + assert.NotEqual(t, "loading files...", got) +} + func TestModel_EnterSwitchesToDiffPane(t *testing.T) { lines := []diff.DiffLine{{NewNum: 1, Content: "line1", ChangeType: diff.ChangeContext}} m := testModel([]string{"a.go", "b.go"}, map[string][]diff.DiffLine{"a.go": lines}) diff --git a/app/ui/view.go b/app/ui/view.go index 9cd011e..01df22c 100644 --- a/app/ui/view.go +++ b/app/ui/view.go @@ -18,6 +18,13 @@ func (m Model) View() string { if !m.ready { return "loading..." } + // ready but the first filesLoadedMsg hasn't landed yet — the file tree is still + // nil-populated and the diff pane has no file selected. Showing the empty two-pane + // layout here would flash a misleading "no changes" state for as long as ChangedFiles + // takes to return (can be 100-500ms on large repos). + if !m.filesLoaded { + return "loading files..." + } ph := m.paneHeight() diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index a3b14a5..b695df9 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -248,13 +248,20 @@ main() [main.go] ``` Model.Init() → loadFiles cmd → Renderer.ChangedFiles() → filesLoadedMsg - → tree.Rebuild(entries) + → tree.Rebuild(entries) + m.filesLoaded = true → auto-select first file → loadFileDiff cmd → Renderer.FileDiff() → fileLoadedMsg → highlight.HighlightLines() → highlightedLines → (optional) loadBlame cmd → blameLoadedMsg ``` +`View()` is gated on two flags so the user never sees an empty two-pane layout +during async initialisation: it returns the literal `"loading..."` while +`!m.ready` (before the first `WindowSizeMsg`), then `"loading files..."` while +`m.ready && !m.filesLoaded` (after resize, before `filesLoadedMsg`). `filesLoaded` +flips to true on every `filesLoadedMsg`, including the error path, so the loading +screen always exits. + ### Rendering Pipeline ``` From ee25d27da9418e9f37b75e787a24934243079e31 Mon Sep 17 00:00:00 2001 From: Umputun Date: Fri, 17 Apr 2026 12:24:28 -0500 Subject: [PATCH 2/2] docs(arch): clarify seq-check behavior in file loading diagram MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Copilot review feedback on PR #117. The previous wording said filesLoaded flips "on every filesLoadedMsg", which is inaccurate after the seq-check was added — stale messages are dropped and do not flip the flag. The diagram also coupled tree.Rebuild with the flag flip, but the flag is set before the error check while Rebuild only runs on non-error accepted messages. --- docs/ARCHITECTURE.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index b695df9..311b375 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -248,7 +248,8 @@ main() [main.go] ``` Model.Init() → loadFiles cmd → Renderer.ChangedFiles() → filesLoadedMsg - → tree.Rebuild(entries) + m.filesLoaded = true + → handleFilesLoaded drops stale msg (seq mismatch), else m.filesLoaded = true + → (on success) tree.Rebuild(entries) → auto-select first file → loadFileDiff cmd → Renderer.FileDiff() → fileLoadedMsg → highlight.HighlightLines() → highlightedLines @@ -258,9 +259,12 @@ Model.Init() → loadFiles cmd `View()` is gated on two flags so the user never sees an empty two-pane layout during async initialisation: it returns the literal `"loading..."` while `!m.ready` (before the first `WindowSizeMsg`), then `"loading files..."` while -`m.ready && !m.filesLoaded` (after resize, before `filesLoadedMsg`). `filesLoaded` -flips to true on every `filesLoadedMsg`, including the error path, so the loading -screen always exits. +`m.ready && !m.filesLoaded` (after resize, before an accepted `filesLoadedMsg`). +`filesLoaded` flips to true on every accepted `filesLoadedMsg` (success or +error), so the loading screen always exits once the current in-flight load +returns. Stale responses (`msg.seq != m.filesLoadSeq`, e.g. an older load still +in flight after `toggleUntracked` bumped the sequence) are dropped and do not +flip the flag or rebuild the tree. ### Rendering Pipeline