diff --git a/CHANGELOG.md b/CHANGELOG.md index 4daee38..86a059f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed +- The interactive file tree now caches its flatten/filter/sort result between + redraws, so holding a navigation key or scrolling a large image's tree stays + responsive instead of recomputing the whole tree on every frame. The cache + refreshes whenever the selected layer, filter, sort, or collapse state + changes. Output is unchanged; only the redundant per-frame work is removed. + ## [v1.6.0] - 2026-07-28 Eight built-in colour themes, transparent-background mode, and TUI visual diff --git a/tui/model.go b/tui/model.go index b0c0ddd..90f2b05 100644 --- a/tui/model.go +++ b/tui/model.go @@ -166,6 +166,40 @@ type fileSavedMsg struct { var spinnerFrames = []string{"⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"} +// treeCache memoizes the flatten→filter→sort output of displayTreeFor between +// frames. A single keystroke in split+filter+sort mode drives displayTreeFor up +// to seven times per round-trip (cursor bounds, clamp, status bar, both render +// passes); without a cache each call re-walks the whole FileNode tree. +// +// The cache lives behind a pointer on the model because displayTreeFor has a +// value receiver and the model is copied by value on every Update — a value +// field would be written to a throwaway copy and never survive. All copies of +// the model share one *treeCache; staleness is caught by comparing the stored +// key against the current inputs, not by assuming a copy carried fresh data. +// +// Both panes (focusTree and focusTreeAgg) get an independent slot so split-mode +// rendering, which asks for both trees in the same frame, does not thrash a +// single slot. The generation counter that keys collapse state lives on the +// model (collapsedGen), because toggleCollapsed mutates a map in place and +// returns the same reference — neither map identity nor contents can be +// compared cheaply, so the counter is bumped on every collapse mutation +// instead (see the toggleCollapsed call sites / clear*Collapsed). +type treeCacheSlot struct { + valid bool + layerCursor int + filterQuery string + diffOnly bool + sortMode sortMode + collapsedGen uint64 + analysisGen uint64 + files []*image.FileNode +} + +type treeCache struct { + top treeCacheSlot // focusTree + bot treeCacheSlot // focusTreeAgg +} + type model struct { width int height int @@ -239,6 +273,20 @@ type model struct { theme Theme transparentBg bool + // collapsedGen is bumped whenever a collapse map is mutated; it is the + // invalidation key for the displayTreeFor cache (see treeCache). + collapsedGen uint64 + // analysisGen is bumped whenever m.analysis is replaced. It keys the + // displayTreeFor cache against the analysis identity: a valid slot whose + // layerCursor/filters are unchanged would otherwise return the previous + // analysis's FileNode slice after a re-analysis. Today analysis is set + // exactly once, so this only ever reaches 1 — it is future-proofing, not a + // live fix, and costs one comparison per lookup. + analysisGen uint64 + // treeCache memoizes displayTreeFor across frames. Behind a pointer so the + // value-receiver method can write through it and all model copies share it. + treeCache *treeCache + fetchCtx context.Context fetchCancel context.CancelFunc } @@ -283,6 +331,7 @@ func NewModel(cfg Config) model { noCache: cfg.NoCache, theme: themeFor(cfg.Theme), transparentBg: cfg.TransparentBg, + treeCache: &treeCache{}, fetchCtx: ctx, fetchCancel: cancel, } @@ -389,6 +438,7 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } m.state = stateReady m.analysis = msg.analysis + m.analysisGen++ m.efficiency = image.EfficiencyFromAnalysis(msg.analysis) if src, ok := m.resolver.(image.ExtractorSource); ok { m.extractor = src.NewExtractor() @@ -1067,6 +1117,7 @@ func (m model) tryOpenSelectedFile() (tea.Model, tea.Cmd) { } else { m.treeCollapsed = toggleCollapsed(m.treeCollapsed, f.Path) } + m.collapsedGen++ mp := &m mp.clampCursors() return *mp, nil @@ -1204,7 +1255,43 @@ func (m model) collapsedFor(f focus) map[string]bool { // displayTreeFor flattens, filters, and sorts the tree visible in the given // pane. The same composition rules (collapse → diff-only → filter → sort) // apply to both panes. +// +// The result is memoized per pane (see treeCache). A cache miss recomputes and +// stores; a hit returns the stored slice untouched. When m.treeCache is nil +// (bare model{} literals in tests) it computes through without caching, so the +// observable output is identical with or without the cache. func (m model) displayTreeFor(f focus) []*image.FileNode { + if m.treeCache == nil { + return m.computeDisplayTreeFor(f) + } + slot := &m.treeCache.top + if f == focusTreeAgg { + slot = &m.treeCache.bot + } + if slot.valid && + slot.layerCursor == m.layerCursor && + slot.filterQuery == m.filterQuery && + slot.diffOnly == m.diffOnly && + slot.sortMode == m.sortMode && + slot.collapsedGen == m.collapsedGen && + slot.analysisGen == m.analysisGen { + return slot.files + } + files := m.computeDisplayTreeFor(f) + *slot = treeCacheSlot{ + valid: true, + layerCursor: m.layerCursor, + filterQuery: m.filterQuery, + diffOnly: m.diffOnly, + sortMode: m.sortMode, + collapsedGen: m.collapsedGen, + analysisGen: m.analysisGen, + files: files, + } + return files +} + +func (m model) computeDisplayTreeFor(f focus) []*image.FileNode { root := m.rootFor(f) var files []*image.FileNode if m.useTreeCollapse() { @@ -1243,10 +1330,12 @@ func (m model) activeTreeFocus() focus { func (m *model) clearTreeCollapsed() { m.treeCollapsed = nil + m.collapsedGen++ } func (m *model) clearAggCollapsed() { m.aggCollapsed = nil + m.collapsedGen++ } func (m *model) resetTreeForLayerChange() { diff --git a/tui/model_test.go b/tui/model_test.go index 83069bf..edf00d6 100644 --- a/tui/model_test.go +++ b/tui/model_test.go @@ -2560,3 +2560,147 @@ func TestPageOnEmptyTreeIsNoop(t *testing.T) { m.moveByPage(1, false) assert.Equal(t, 0, m.treeCursor, "paging an empty tree must not move or panic") } + +// --- displayTreeFor cache invalidation --------------------------------------- +// +// These tests exercise the memoization added to displayTreeFor. Each drives one +// input-mutation path and asserts the returned slice reflects the new state. +// They fail if an invalidation key is missing from the cache — the regression +// class the cache introduces. + +func TestDisplayTreeCacheReflectsDiffOnlyToggle(t *testing.T) { + m := setupModelWithDiffs() + before := len(m.displayTreeFor(focusTree)) // warms the cache + + m.diffOnly = true + filtered := m.displayTreeFor(focusTree) + + assert.Less(t, len(filtered), before, "diff-only must drop unchanged files even after the cache is warm") + for _, f := range filtered { + assert.NotEqual(t, image.Unchanged, f.DiffType) + } +} + +func TestDisplayTreeCacheReflectsFilterQuery(t *testing.T) { + m := setupModelWithDiffs() + all := m.displayTreeFor(focusTree) // warms the cache + require.NotEmpty(t, all) + + m.filterQuery = "nginx" + filtered := m.displayTreeFor(focusTree) + + assert.NotEmpty(t, filtered) + assert.Less(t, len(filtered), len(all), "a filter query must narrow the cached slice") + for _, f := range filtered { + assert.Contains(t, strings.ToLower(f.Path), "nginx") + } +} + +func TestDisplayTreeCacheReflectsSortMode(t *testing.T) { + m := setupModelWithDiffs() + unsorted := m.displayTreeFor(focusTree) // warms the cache + require.NotEmpty(t, unsorted) + + m.sortMode = sortDesc + sorted := m.displayTreeFor(focusTree) + + // Sorting by size descending must yield non-increasing effective sizes. + for i := 1; i < len(sorted); i++ { + assert.GreaterOrEqual(t, nodeEffectiveSize(sorted[i-1]), nodeEffectiveSize(sorted[i]), + "sortDesc must return files by descending effective size, not a stale unsorted slice") + } +} + +func TestDisplayTreeCacheReflectsLayerChange(t *testing.T) { + m := setupModelWithDiffs() + m.layerCursor = 0 + base := m.displayTreeFor(focusTree) // warms the cache for layer 0 + + m.layerCursor = 1 + next := m.displayTreeFor(focusTree) + + // The two layers have different tree shapes; a stale cache would return the + // layer-0 slice for layer 1. + assert.NotEqual(t, base, next, "changing the selected layer must recompute the tree") +} + +func TestDisplayTreeCacheReflectsCollapseToggle(t *testing.T) { + m := setupModel() + m.focus = focusTree + expanded := m.displayTreeFor(focusTree) // warms the cache + require.NotEmpty(t, expanded) + + // Collapse the first directory in the visible list. + var dirPath string + for _, f := range expanded { + if f.IsDir { + dirPath = f.Path + break + } + } + require.NotEmpty(t, dirPath, "fixture must contain at least one directory") + + m.treeCollapsed = toggleCollapsed(m.treeCollapsed, dirPath) + m.collapsedGen++ + collapsed := m.displayTreeFor(focusTree) + + assert.Less(t, len(collapsed), len(expanded), + "collapsing a directory must hide its descendants even after the cache is warm") +} + +func TestDisplayTreeCachePanesAreIndependent(t *testing.T) { + m := setupModelWithDiffs() + m.aggregated = true + + // Rendering split mode asks for both panes in one frame; the two cache + // slots must not clobber each other. + top := m.displayTreeFor(focusTree) + bot := m.displayTreeFor(focusTreeAgg) + topAgain := m.displayTreeFor(focusTree) + botAgain := m.displayTreeFor(focusTreeAgg) + + require.NotEmpty(t, top) + require.NotEmpty(t, bot) + assert.Equal(t, &top[0], &topAgain[0], "the top pane slot must survive a bot-pane lookup in the same frame") + assert.Equal(t, &bot[0], &botAgain[0], "the bot pane slot must survive a top-pane lookup in the same frame") + assert.NotEqual(t, &top[0], &bot[0], "the two panes must not share one cache slot") +} + +func TestDisplayTreeCacheHitReturnsIdenticalSlice(t *testing.T) { + m := setupModelWithDiffs() + first := m.displayTreeFor(focusTree) + second := m.displayTreeFor(focusTree) + + // A warm hit with unchanged inputs should return the very same backing + // slice, not recompute a fresh one. + require.NotEmpty(t, first) + assert.Equal(t, &first[0], &second[0], "an unchanged repeat lookup should return the cached slice") +} + +func TestDisplayTreeCacheReflectsReanalysis(t *testing.T) { + m := setupModel() + m.focus = focusTree + m.layerCursor = 0 + before := m.displayTreeFor(focusTree) // warms the cache for the first analysis + require.NotEmpty(t, before) + + // A second analysis replaces m.analysis while layerCursor and every filter + // stay put. Without the analysisGen key the warm slot would return the old + // analysis's nodes; the gen bump in the analysisMsg handler must force a + // recompute against the new tree. + other := testAnalysis() + root := other.StackedTrees[0].Root + root.AddChild(&image.FileNode{Name: "REANALYZED", Path: "/REANALYZED", Size: 1}) + m = send(m, analysisMsg{analysis: other}) + + after := m.displayTreeFor(focusTree) + var found bool + for _, f := range after { + if f.Path == "/REANALYZED" { + found = true + break + } + } + assert.True(t, found, "a replaced analysis must invalidate the warm tree cache") +} +