diff --git a/internal/ui/chat/agent.go b/internal/ui/chat/agent.go index 882971290d7af78e21add7663f993731c618a967..abc65e7b47c684cd1a4d8a12b0a96efaa924329e 100644 --- a/internal/ui/chat/agent.go +++ b/internal/ui/chat/agent.go @@ -77,9 +77,23 @@ func (a *AgentToolMessageItem) NestedTools() []ToolMessageItem { } // SetNestedTools sets the nested tools. +// +// SetNestedTools always bumps the version. The previous design +// deduped when the slice's length and element pointers were +// unchanged, but the live update path in internal/ui/model/ui.go +// mutates existing children in place (SetToolCall / SetResult on the +// same pointers) and then calls SetNestedTools with the same slice. +// Pointer-equality dedupe in that case skips the parent Bump even +// though the parent's rendered output (which embeds the children +// inline) has changed, leaving a stale parent entry in the list +// cache. Always bumping is cheap (one uint64 increment) and called +// at most once per agent event; in the rare case the slice is +// truly unchanged the worst case is one extra parent re-render +// while every child cache hit stays warm. func (a *AgentToolMessageItem) SetNestedTools(tools []ToolMessageItem) { a.nestedTools = tools a.clearCache() + a.Bump() } // AddNestedTool adds a nested tool. @@ -90,6 +104,7 @@ func (a *AgentToolMessageItem) AddNestedTool(tool ToolMessageItem) { } a.nestedTools = append(a.nestedTools, tool) a.clearCache() + a.Bump() } // AgentToolRenderContext renders agent tool messages. @@ -201,10 +216,12 @@ func (a *AgenticFetchToolMessageItem) NestedTools() []ToolMessageItem { return a.nestedTools } -// SetNestedTools sets the nested tools. +// SetNestedTools sets the nested tools. Always bumps the version; +// see [AgentToolMessageItem.SetNestedTools] for the rationale. func (a *AgenticFetchToolMessageItem) SetNestedTools(tools []ToolMessageItem) { a.nestedTools = tools a.clearCache() + a.Bump() } // AddNestedTool adds a nested tool. @@ -215,6 +232,7 @@ func (a *AgenticFetchToolMessageItem) AddNestedTool(tool ToolMessageItem) { } a.nestedTools = append(a.nestedTools, tool) a.clearCache() + a.Bump() } // AgenticFetchToolRenderContext renders agentic fetch tool messages. diff --git a/internal/ui/chat/applyhighlight_callback_test.go b/internal/ui/chat/applyhighlight_callback_test.go new file mode 100644 index 0000000000000000000000000000000000000000..f008c0e2c74a91b277670e4e6a9f9ee6c3a0cff4 --- /dev/null +++ b/internal/ui/chat/applyhighlight_callback_test.go @@ -0,0 +1,207 @@ +package chat + +import ( + "testing" + + "github.com/charmbracelet/crush/internal/message" + "github.com/charmbracelet/crush/internal/ui/list" + "github.com/charmbracelet/crush/internal/ui/styles" + "github.com/stretchr/testify/require" +) + +// renderCountingItem wraps a real chat item and counts Render calls +// to expose the list-level cache behaviour to tests. The wrapper +// forwards the list.Item methods exercised by this test — Render, +// Version, Finished — plus the list.Highlightable surface +// (SetHighlight / Highlight) used by the callback-driven scenario. +// Focus is not exercised here, so list.Focusable is not forwarded; +// add SetFocused/IsFocused if a future test needs to drive focus +// through the wrapper. +type renderCountingItem struct { + inner MessageItem + renderHits int + highlightCb func(start [4]int) +} + +func newRenderCountingItem(inner MessageItem) *renderCountingItem { + return &renderCountingItem{inner: inner} +} + +func (r *renderCountingItem) Render(width int) string { + r.renderHits++ + return r.inner.Render(width) +} + +func (r *renderCountingItem) Version() uint64 { + return r.inner.(versionedItem).Version() +} + +func (r *renderCountingItem) Finished() bool { + return r.inner.Finished() +} + +// SetHighlight forwards to the embedded item; the underlying +// highlightableMessageItem dedupes equivalent ranges and bumps the +// shared version on observable change. +func (r *renderCountingItem) SetHighlight(startLine, startCol, endLine, endCol int) { + if h, ok := r.inner.(list.Highlightable); ok { + h.SetHighlight(startLine, startCol, endLine, endCol) + if r.highlightCb != nil { + r.highlightCb([4]int{startLine, startCol, endLine, endCol}) + } + } +} + +func (r *renderCountingItem) Highlight() (int, int, int, int) { + if h, ok := r.inner.(list.Highlightable); ok { + return h.Highlight() + } + return -1, -1, -1, -1 +} + +// TestList_CallbackDrivenHighlightUnfreezeAndReFreeze covers F6 +// §4.5.1 along the live applyHighlightRange path. Instead of +// driving BeginSelectionDrag directly, the test registers a render +// callback that mutates the chat items' highlight ranges (just like +// Chat.applyHighlightRange does in production) and verifies the +// resulting cache behaviour: +// +// - Items inside the active range pick up a SetHighlight call, +// their version bumps, the F6 cache invalidates, and the list +// re-renders them on the next draw. The post-render entry is +// frozen again because the items are Finished() — but their +// stored output now reflects the highlight. +// - Subsequent draws while the range is unchanged are cache hits: +// the callback's SetHighlight call dedupes (same range), the +// version is stable, and the list serves the previous output +// verbatim without calling Render. +// - When the range moves OFF an item, the callback clears the +// highlight, the version bumps, and the item re-renders. After +// that single re-render the entry re-freezes; further draws are +// cache hits. +func TestList_CallbackDrivenHighlightUnfreezeAndReFreeze(t *testing.T) { + t.Parallel() + + sty := styles.CharmtonePantera() + + // Build three finished assistant messages so all three are + // candidates for freezing. Real items (per Round 2 spec) — the + // surrounding renderCountingItem wrapper just lets the test see + // per-item Render calls. + mk := func(id, body string) *renderCountingItem { + msg := &message.Message{ + ID: id, + Role: message.Assistant, + Parts: []message.ContentPart{ + message.ReasoningContent{ + Thinking: "thinking", + StartedAt: testStartedAt, + FinishedAt: testFinishedAt, + }, + message.TextContent{Text: body}, + message.Finish{Reason: message.FinishReasonEndTurn, Time: testFinishTime}, + }, + } + inner := NewAssistantMessageItem(&sty, msg) + require.True(t, inner.Finished(), "test fixture must be Finished()") + return newRenderCountingItem(inner) + } + + a := mk("a", "alpha") + b := mk("b", "bravo") + c := mk("c", "charlie") + + l := list.NewList(a, b, c) + l.SetSize(80, 30) + + // activeRange holds the inclusive [start, end] item indexes the + // callback should highlight. -1 means no active selection. + activeRange := [2]int{-1, -1} + + cb := func(idx, _ int, item list.Item) list.Item { + hi, ok := item.(list.Highlightable) + if !ok { + return item + } + if activeRange[0] >= 0 && idx >= activeRange[0] && idx <= activeRange[1] { + // Inside the range: highlight the entire item. + hi.SetHighlight(0, 0, -1, -1) + } else { + // Outside the range: clear highlight. + hi.SetHighlight(-1, -1, -1, -1) + } + return item + } + l.RegisterRenderCallback(cb) + + // First render populates the cache. Each item renders exactly + // once even though the callback runs for all three. + _ = l.Render() + require.Equal(t, 1, a.renderHits, "first render: a renders once") + require.Equal(t, 1, b.renderHits, "first render: b renders once") + require.Equal(t, 1, c.renderHits, "first render: c renders once") + + // Subsequent renders without an active range are cache hits. + // The callback's SetHighlight call dedupes (already cleared), + // no version bump, frozen entries served verbatim. + for range 3 { + _ = l.Render() + } + require.Equal(t, 1, a.renderHits, "frozen item must not re-render across stable draws") + require.Equal(t, 1, b.renderHits, "frozen item must not re-render across stable draws") + require.Equal(t, 1, c.renderHits, "frozen item must not re-render across stable draws") + + // Activate a selection range over items a and b. The callback + // will SetHighlight on both during the next render, bumping + // their versions. The cache hit fails (version mismatch) and + // each in-range item re-renders exactly once. + activeRange = [2]int{0, 1} + _ = l.Render() + require.Equal(t, 2, a.renderHits, "in-range item must re-render after SetHighlight") + require.Equal(t, 2, b.renderHits, "in-range item must re-render after SetHighlight") + require.Equal(t, 1, c.renderHits, "out-of-range item stays frozen") + + // Verify the highlight actually landed on the in-range items. + sLine, _, eLine, _ := a.Highlight() + require.Equal(t, 0, sLine) + require.Equal(t, -1, eLine) + sLine, _, eLine, _ = c.Highlight() + require.Equal(t, -1, sLine, "out-of-range item must not be highlighted") + require.Equal(t, -1, eLine) + + // While the range stays the same, subsequent renders are cache + // hits. The callback dedupes (same range), no version bump, + // the post-render entry served verbatim. Note: items are + // re-frozen because they're still Finished() and not in the + // list's freezeSuppressed set. + for range 3 { + _ = l.Render() + } + require.Equal(t, 2, a.renderHits, "in-range item re-freezes after the highlight render") + require.Equal(t, 2, b.renderHits, "in-range item re-freezes after the highlight render") + require.Equal(t, 1, c.renderHits, "out-of-range item stays frozen") + + // Move the range off the items entirely. The callback clears + // each in-range item's highlight back to (-1,-1,-1,-1), which + // bumps their versions and triggers exactly one re-render + // each. After that, the entries re-freeze. + activeRange = [2]int{-1, -1} + _ = l.Render() + require.Equal(t, 3, a.renderHits, "exiting-range item must re-render once when highlight clears") + require.Equal(t, 3, b.renderHits, "exiting-range item must re-render once when highlight clears") + require.Equal(t, 1, c.renderHits, "never-highlighted item stays frozen") + + // Confirm the highlight has been fully cleared. + sLine, _, eLine, _ = a.Highlight() + require.Equal(t, -1, sLine) + require.Equal(t, -1, eLine) + + // And subsequent renders are cache hits again — the items + // re-froze. + for range 3 { + _ = l.Render() + } + require.Equal(t, 3, a.renderHits, "re-frozen item must not re-render across stable draws") + require.Equal(t, 3, b.renderHits, "re-frozen item must not re-render across stable draws") + require.Equal(t, 1, c.renderHits, "never-highlighted item stays frozen") +} diff --git a/internal/ui/chat/assistant.go b/internal/ui/chat/assistant.go index ac2c0c88819ebe4334b76cdf9b6774ccf75adca7..e9f00020294abe2d707b645a111edf6181d424f4 100644 --- a/internal/ui/chat/assistant.go +++ b/internal/ui/chat/assistant.go @@ -11,6 +11,7 @@ import ( "github.com/charmbracelet/crush/internal/message" "github.com/charmbracelet/crush/internal/ui/anim" "github.com/charmbracelet/crush/internal/ui/common" + "github.com/charmbracelet/crush/internal/ui/list" "github.com/charmbracelet/crush/internal/ui/styles" "github.com/charmbracelet/x/ansi" ) @@ -119,6 +120,7 @@ func fnvFields(fields ...[]byte) uint64 { // // This item includes thinking, and the content but does not include the tool calls. type AssistantMessageItem struct { + *list.Versioned *highlightableMessageItem *cachedMessageItem *focusableMessageItem @@ -141,10 +143,12 @@ var _ Expandable = (*AssistantMessageItem)(nil) // NewAssistantMessageItem creates a new AssistantMessageItem. func NewAssistantMessageItem(sty *styles.Styles, message *message.Message) MessageItem { + v := list.NewVersioned() a := &AssistantMessageItem{ - highlightableMessageItem: defaultHighlighter(sty), + Versioned: v, + highlightableMessageItem: defaultHighlighter(sty, v), cachedMessageItem: &cachedMessageItem{}, - focusableMessageItem: &focusableMessageItem{}, + focusableMessageItem: newFocusableMessageItem(v), message: message, sty: sty, } @@ -173,6 +177,13 @@ func (a *AssistantMessageItem) Animate(msg anim.StepMsg) tea.Cmd { if !a.isSpinning() { return nil } + // Bump the F6 list-cache version so the next draw re-renders + // this item: a spinner tick mutates anim's internal frame + // counter, which changes the rendered output but is invisible + // to the per-section content hashes. Without the bump the + // list cache would serve the previously rendered frame + // indefinitely and the spinner would appear frozen. + a.Bump() return a.anim.Animate(msg) } @@ -523,6 +534,12 @@ func (a *AssistantMessageItem) isSpinning() bool { func (a *AssistantMessageItem) SetMessage(msg *message.Message) tea.Cmd { wasSpinning := a.isSpinning() a.message = msg + // Bump the F6 version even if the underlying *message.Message + // pointer is identical: callers may have mutated the message in + // place (delta append) and we cannot tell from here. The + // per-section caches dedupe identical content via FNV-64 hashes, + // so a redundant bump only costs one list-cache repopulation. + a.Bump() // The prefix cache is keyed by a fingerprint that includes every // section's source hash, so an unchanged section keeps its prefix // cache valid while a changed section forces a miss naturally. @@ -534,6 +551,16 @@ func (a *AssistantMessageItem) SetMessage(msg *message.Message) tea.Cmd { return nil } +// Finished implements list.Item. The assistant message is freezable +// once the message reports IsFinished() and is no longer spinning +// (no animation tick remains pending). Streaming tail animation is +// caught by isSpinning, so freezing only kicks in once the turn is +// fully terminal. The list cache invalidates the entry on the next +// version bump if anything (focus, highlight, expansion) changes. +func (a *AssistantMessageItem) Finished() bool { + return a.message.IsFinished() && !a.isSpinning() +} + // clearCache drops every cached render for this item, including the // per-section caches. Shadows the embedded cachedMessageItem.clearCache // so ClearItemCaches (style change) wipes the section caches too. @@ -572,6 +599,7 @@ func (a *AssistantMessageItem) ToggleExpanded() bool { case thinkingFullExpanded: a.thinkingViewMode = thinkingCollapsed } + a.Bump() return a.thinkingViewMode != thinkingCollapsed } diff --git a/internal/ui/chat/messages.go b/internal/ui/chat/messages.go index 3c1c5a13b7ba13a5e6990b4cc546c8813c0d5252..1e83ec6f6797a866f3587ad6f349bb4c760b1eb3 100644 --- a/internal/ui/chat/messages.go +++ b/internal/ui/chat/messages.go @@ -75,6 +75,12 @@ type SendMsg struct { } type highlightableMessageItem struct { + // version is the parent item's version counter. SetHighlight + // bumps it on every observable change so the F6 list memo and + // any frozen entry get invalidated when a selection drag enters + // or leaves the item. + version *list.Versioned + startLine int startCol int endLine int @@ -103,13 +109,20 @@ func (h *highlightableMessageItem) SetHighlight(startLine int, startCol int, end // Adjust columns for the style's left inset (border + padding) since we // highlight the content only. offset := MessageLeftPaddingTotal + newStartCol := max(0, startCol-offset) + newEndCol := endCol + if endCol >= 0 { + newEndCol = max(0, endCol-offset) + } + if h.startLine == startLine && h.startCol == newStartCol && h.endLine == endLine && h.endCol == newEndCol { + return + } h.startLine = startLine - h.startCol = max(0, startCol-offset) + h.startCol = newStartCol h.endLine = endLine - if endCol >= 0 { - h.endCol = max(0, endCol-offset) - } else { - h.endCol = endCol + h.endCol = newEndCol + if h.version != nil { + h.version.Bump() } } @@ -118,8 +131,9 @@ func (h *highlightableMessageItem) Highlight() (startLine int, startCol int, end return h.startLine, h.startCol, h.endLine, h.endCol } -func defaultHighlighter(sty *styles.Styles) *highlightableMessageItem { +func defaultHighlighter(sty *styles.Styles, v *list.Versioned) *highlightableMessageItem { return &highlightableMessageItem{ + version: v, startLine: -1, startCol: -1, endLine: -1, @@ -135,12 +149,17 @@ type cacheClearable interface { } // ClearItemCaches drops any cached rendered output on each item so the -// next render uses the current styles. +// next render uses the current styles. It also bumps each item's +// version so the F6 list-level memo invalidates frozen entries on +// the next render. func ClearItemCaches(items []MessageItem) { for _, item := range items { if cc, ok := item.(cacheClearable); ok { cc.clearCache() } + if v, ok := item.(interface{ Bump() }); ok { + v.Bump() + } } } @@ -216,12 +235,28 @@ func (c *cachedMessageItem) clearCache() { // focusableMessageItem is a base struct for message items that can be focused. type focusableMessageItem struct { + // version is the parent item's version counter. SetFocused + // bumps it whenever focus actually flips so the F6 list memo + // invalidates the per-line focus prefix. + version *list.Versioned focused bool } +// newFocusableMessageItem returns a focusableMessageItem wired to the +// shared version counter. +func newFocusableMessageItem(v *list.Versioned) *focusableMessageItem { + return &focusableMessageItem{version: v} +} + // SetFocused implements MessageItem. func (f *focusableMessageItem) SetFocused(focused bool) { + if f.focused == focused { + return + } f.focused = focused + if f.version != nil { + f.version.Bump() + } } // AssistantInfoID returns a stable ID for assistant info items. @@ -231,6 +266,7 @@ func AssistantInfoID(messageID string) string { // AssistantInfoItem renders model info and response time after assistant completes. type AssistantInfoItem struct { + *list.Versioned *cachedMessageItem id string @@ -243,6 +279,7 @@ type AssistantInfoItem struct { // NewAssistantInfoItem creates a new AssistantInfoItem. func NewAssistantInfoItem(sty *styles.Styles, message *message.Message, cfg *config.Config, lastUserMessageTime time.Time) MessageItem { return &AssistantInfoItem{ + Versioned: list.NewVersioned(), cachedMessageItem: &cachedMessageItem{}, id: AssistantInfoID(message.ID), message: message, @@ -252,6 +289,13 @@ func NewAssistantInfoItem(sty *styles.Styles, message *message.Message, cfg *con } } +// Finished implements list.Item. Assistant info blocks render a fixed +// model/duration footer once the assistant turn finishes; the data +// is immutable after construction so the entry is safe to freeze. +func (a *AssistantInfoItem) Finished() bool { + return true +} + // ID implements MessageItem. func (a *AssistantInfoItem) ID() string { return a.id diff --git a/internal/ui/chat/tools.go b/internal/ui/chat/tools.go index 560fd2fc5312852b5cc91a228b4587f4accb0d2a..5855cbe2698e1defe4a5c8800ccbb6c71356c926 100644 --- a/internal/ui/chat/tools.go +++ b/internal/ui/chat/tools.go @@ -19,6 +19,7 @@ import ( "github.com/charmbracelet/crush/internal/stringext" "github.com/charmbracelet/crush/internal/ui/anim" "github.com/charmbracelet/crush/internal/ui/common" + "github.com/charmbracelet/crush/internal/ui/list" "github.com/charmbracelet/crush/internal/ui/styles" "github.com/charmbracelet/x/ansi" ) @@ -135,6 +136,7 @@ func (f ToolRendererFunc) RenderTool(sty *styles.Styles, width int, opts *ToolRe // baseToolMessageItem represents a tool call message that can be displayed in the UI. type baseToolMessageItem struct { + *list.Versioned *highlightableMessageItem *cachedMessageItem *focusableMessageItem @@ -176,10 +178,12 @@ func newBaseToolMessageItem( status = ToolStatusCanceled } + v := list.NewVersioned() t := &baseToolMessageItem{ - highlightableMessageItem: defaultHighlighter(sty), + Versioned: v, + highlightableMessageItem: defaultHighlighter(sty, v), cachedMessageItem: &cachedMessageItem{}, - focusableMessageItem: &focusableMessageItem{}, + focusableMessageItem: newFocusableMessageItem(v), sty: sty, toolRenderer: toolRenderer, toolCall: toolCall, @@ -270,8 +274,12 @@ func NewToolMessageItem( // SetCompact implements the Compactable interface. func (t *baseToolMessageItem) SetCompact(compact bool) { + if t.isCompact == compact { + return + } t.isCompact = compact t.clearCache() + t.Bump() } // ID returns the unique identifier for this tool message item. @@ -378,12 +386,14 @@ func (t *baseToolMessageItem) ToolCall() message.ToolCall { func (t *baseToolMessageItem) SetToolCall(tc message.ToolCall) { t.toolCall = tc t.clearCache() + t.Bump() } // SetResult sets the tool result associated with this message item. func (t *baseToolMessageItem) SetResult(res *message.ToolResult) { t.result = res t.clearCache() + t.Bump() } // MessageID returns the ID of the message containing this tool call. @@ -392,14 +402,20 @@ func (t *baseToolMessageItem) MessageID() string { } // SetMessageID sets the ID of the message containing this tool call. +// MessageID is metadata only and does not affect the rendered output, +// so we deliberately do not bump the version here. func (t *baseToolMessageItem) SetMessageID(id string) { t.messageID = id } // SetStatus sets the tool status. func (t *baseToolMessageItem) SetStatus(status ToolStatus) { + if t.status == status { + return + } t.status = status t.clearCache() + t.Bump() } // Status returns the current tool status. @@ -439,9 +455,25 @@ func (t *baseToolMessageItem) SetSpinningFunc(fn SpinningFunc) { func (t *baseToolMessageItem) ToggleExpanded() bool { t.expandedContent = !t.expandedContent t.clearCache() + t.Bump() return t.expandedContent } +// Finished implements list.Item. A tool call is freezable once the +// tool call itself is marked finished AND a result has been recorded +// (or it has been canceled). Tools that override the spinning logic +// via spinningFunc would short-circuit live ticks; we still gate +// freezing on isSpinning to keep the contract conservative. +func (t *baseToolMessageItem) Finished() bool { + if t.isSpinning() { + return false + } + if t.status == ToolStatusCanceled { + return true + } + return t.toolCall.Finished && t.result != nil +} + // HandleMouseClick implements MouseClickable. func (t *baseToolMessageItem) HandleMouseClick(btn ansi.MouseButton, x, y int) bool { return btn == ansi.MouseLeft diff --git a/internal/ui/chat/user.go b/internal/ui/chat/user.go index 8194955a6fa99e20a7d949651d6d1400d8f3fb72..0f11c00a68f7aec9013ce2d80b73e0ab4df9c1d9 100644 --- a/internal/ui/chat/user.go +++ b/internal/ui/chat/user.go @@ -8,11 +8,13 @@ import ( "github.com/charmbracelet/crush/internal/message" "github.com/charmbracelet/crush/internal/ui/attachments" "github.com/charmbracelet/crush/internal/ui/common" + "github.com/charmbracelet/crush/internal/ui/list" "github.com/charmbracelet/crush/internal/ui/styles" ) // UserMessageItem represents a user message in the chat UI. type UserMessageItem struct { + *list.Versioned *highlightableMessageItem *cachedMessageItem *focusableMessageItem @@ -24,16 +26,24 @@ type UserMessageItem struct { // NewUserMessageItem creates a new UserMessageItem. func NewUserMessageItem(sty *styles.Styles, message *message.Message, attachments *attachments.Renderer) MessageItem { + v := list.NewVersioned() return &UserMessageItem{ - highlightableMessageItem: defaultHighlighter(sty), + Versioned: v, + highlightableMessageItem: defaultHighlighter(sty, v), cachedMessageItem: &cachedMessageItem{}, - focusableMessageItem: &focusableMessageItem{}, + focusableMessageItem: newFocusableMessageItem(v), attachments: attachments, message: message, sty: sty, } } +// Finished implements list.Item. User messages are immutable once +// submitted, so the entry is always safe to freeze. +func (m *UserMessageItem) Finished() bool { + return true +} + // RawRender implements [MessageItem]. func (m *UserMessageItem) RawRender(width int) string { cappedWidth := cappedMessageWidth(width) diff --git a/internal/ui/chat/version_bump_test.go b/internal/ui/chat/version_bump_test.go new file mode 100644 index 0000000000000000000000000000000000000000..847f0053bb38324e7019e35294c7e9a81a9ba7a4 --- /dev/null +++ b/internal/ui/chat/version_bump_test.go @@ -0,0 +1,425 @@ +package chat + +import ( + "testing" + "time" + + "github.com/charmbracelet/crush/internal/config" + "github.com/charmbracelet/crush/internal/message" + "github.com/charmbracelet/crush/internal/ui/anim" + "github.com/charmbracelet/crush/internal/ui/attachments" + "github.com/charmbracelet/crush/internal/ui/list" + "github.com/charmbracelet/crush/internal/ui/styles" + "github.com/stretchr/testify/require" +) + +// versionedItem is the cross-cutting interface every chat item type +// must satisfy under F6: every documented mutator must bump the +// shared version counter so the list-level memo invalidates. +type versionedItem interface { + list.Item + Version() uint64 +} + +// requireBump asserts that the supplied mutator advances the item's +// Version(). The mutator runs once; an absent bump is a regression +// (a finished item would keep serving stale frozen output to the +// list cache). +func requireBump(t *testing.T, name string, item versionedItem, mutate func()) { + t.Helper() + before := item.Version() + mutate() + after := item.Version() + require.Greaterf(t, after, before, "%s must bump Version() (before=%d, after=%d)", name, before, after) +} + +// TestAssistantMessageItem_MutatorsBumpVersion enumerates every +// documented mutator on AssistantMessageItem and asserts each one +// advances Version(). +func TestAssistantMessageItem_MutatorsBumpVersion(t *testing.T) { + t.Parallel() + + sty := styles.CharmtonePantera() + build := func(thinking, content string) *message.Message { + parts := []message.ContentPart{ + message.ReasoningContent{ + Thinking: thinking, + StartedAt: testStartedAt, + FinishedAt: testFinishedAt, + }, + } + if content != "" { + parts = append(parts, message.TextContent{Text: content}) + } + return &message.Message{ID: "a-mut", Role: message.Assistant, Parts: parts} + } + + item := NewAssistantMessageItem(&sty, build("thinking", "content")).(*AssistantMessageItem) + + requireBump(t, "SetMessage", item, func() { + item.SetMessage(build("thinking", "more content")) + }) + requireBump(t, "SetFocused", item, func() { + item.SetFocused(true) + }) + requireBump(t, "SetHighlight", item, func() { + item.SetHighlight(0, 0, 0, 5) + }) + // ToggleExpanded only mutates state when there is non-empty + // thinking text — which the build helper provides. + requireBump(t, "ToggleExpanded", item, func() { + item.ToggleExpanded() + }) +} + +// TestUserMessageItem_MutatorsBumpVersion enumerates UserMessageItem +// mutators. +func TestUserMessageItem_MutatorsBumpVersion(t *testing.T) { + t.Parallel() + + sty := styles.CharmtonePantera() + r := attachments.NewRenderer( + sty.Attachments.Normal, + sty.Attachments.Deleting, + sty.Attachments.Image, + sty.Attachments.Text, + ) + msg := &message.Message{ + ID: "u-mut", + Role: message.User, + Parts: []message.ContentPart{ + message.TextContent{Text: "Hello"}, + }, + } + item := NewUserMessageItem(&sty, msg, r).(*UserMessageItem) + + requireBump(t, "SetFocused", item, func() { + item.SetFocused(true) + }) + requireBump(t, "SetHighlight", item, func() { + item.SetHighlight(0, 0, 0, 3) + }) +} + +// TestAssistantInfoItem_VersionedAndFinished sanity-checks the +// AssistantInfoItem wiring. The item carries only immutable data +// after construction; we still assert Version() is callable and +// Finished() returns true. +func TestAssistantInfoItem_VersionedAndFinished(t *testing.T) { + t.Parallel() + + sty := styles.CharmtonePantera() + cfg := &config.Config{} + msg := &message.Message{ + ID: "info", + Role: message.Assistant, + Parts: []message.ContentPart{message.Finish{Reason: message.FinishReasonEndTurn, Time: time.Now().Unix()}}, + } + item := NewAssistantInfoItem(&sty, msg, cfg, time.Unix(0, 0)).(*AssistantInfoItem) + + require.True(t, item.Finished(), "AssistantInfoItem must be Finished()") + // Version() is callable and starts at zero. + require.Equal(t, uint64(0), item.Version()) +} + +// TestBaseToolMessageItem_MutatorsBumpVersion enumerates the base +// tool item mutators. Specific tool types layer on top of this +// base; the base bumps cover the shared mutator surface. +func TestBaseToolMessageItem_MutatorsBumpVersion(t *testing.T) { + t.Parallel() + + sty := styles.CharmtonePantera() + tc := message.ToolCall{ID: "tc1", Name: "bash", Input: "{}", Finished: false} + item := NewToolMessageItem(&sty, "msg", tc, nil, false) + + v := item.(versionedItem) + + requireBump(t, "SetFocused", v, func() { + if f, ok := item.(list.Focusable); ok { + f.SetFocused(true) + } + }) + requireBump(t, "SetHighlight", v, func() { + if h, ok := item.(list.Highlightable); ok { + h.SetHighlight(0, 0, 0, 3) + } + }) + requireBump(t, "SetToolCall", v, func() { + tc2 := tc + tc2.Input = `{"command":"echo"}` + item.SetToolCall(tc2) + }) + requireBump(t, "SetResult", v, func() { + item.SetResult(&message.ToolResult{ToolCallID: "tc1", Content: "ok"}) + }) + requireBump(t, "SetStatus", v, func() { + item.SetStatus(ToolStatusSuccess) + }) + requireBump(t, "ToggleExpanded", v, func() { + if e, ok := item.(Expandable); ok { + e.ToggleExpanded() + } + }) + requireBump(t, "SetCompact", v, func() { + if c, ok := item.(Compactable); ok { + c.SetCompact(true) + } + }) +} + +// TestAssistantMessageItem_AnimateBumpsVersion covers the spinner +// regression: while the assistant message is spinning, every +// anim.StepMsg fed through Animate must bump Version() so the +// list-level cache invalidates and the next draw re-renders the +// advanced spinner frame. Without this bump the cached entry's +// version stays put and the spinner appears frozen. +func TestAssistantMessageItem_AnimateBumpsVersion(t *testing.T) { + t.Parallel() + + sty := styles.CharmtonePantera() + streaming := &message.Message{ + ID: "spin", + Role: message.Assistant, + Parts: []message.ContentPart{ + message.ReasoningContent{Thinking: "thinking..."}, + }, + } + item := NewAssistantMessageItem(&sty, streaming).(*AssistantMessageItem) + + requireBump(t, "Animate", item, func() { + item.Animate(anim.StepMsg{}) + }) + + // A non-spinning item must not bump on Animate: the bump only + // makes sense while the spinner is live, and a stray bump on a + // finished item would needlessly invalidate frozen entries. + finished := &message.Message{ + ID: "spin", + Role: message.Assistant, + Parts: []message.ContentPart{ + message.TextContent{Text: "done"}, + message.Finish{Reason: message.FinishReasonEndTurn, Time: testFinishTime}, + }, + } + item.SetMessage(finished) + require.True(t, item.Finished(), "item must report Finished() once the message finishes") + before := item.Version() + item.Animate(anim.StepMsg{}) + require.Equal(t, before, item.Version(), "Animate must not bump Version() on a non-spinning item") +} + +// TestAssistantMessageItem_FinishedTransition covers §4.5.1: a +// streaming assistant message reports Finished() == false; once the +// message reports IsFinished() and stops spinning, Finished() must +// return true. +func TestAssistantMessageItem_FinishedTransition(t *testing.T) { + t.Parallel() + + sty := styles.CharmtonePantera() + + // Streaming: no finish part, no content yet — isSpinning == true. + streaming := &message.Message{ + ID: "stream", + Role: message.Assistant, + Parts: []message.ContentPart{ + message.ReasoningContent{Thinking: "thinking..."}, + }, + } + item := NewAssistantMessageItem(&sty, streaming).(*AssistantMessageItem) + require.False(t, item.Finished(), "streaming assistant message must not be Finished()") + + // Finished with content. + finished := &message.Message{ + ID: "stream", + Role: message.Assistant, + Parts: []message.ContentPart{ + message.ReasoningContent{Thinking: "thinking", StartedAt: testStartedAt, FinishedAt: testFinishedAt}, + message.TextContent{Text: "the answer"}, + message.Finish{Reason: message.FinishReasonEndTurn, Time: testFinishTime}, + }, + } + item.SetMessage(finished) + require.True(t, item.Finished(), "finished assistant message must be Finished()") +} + +// TestUserMessageItem_FinishedAlwaysTrue locks in the freezable +// contract: user messages are never spinning. +func TestUserMessageItem_FinishedAlwaysTrue(t *testing.T) { + t.Parallel() + + sty := styles.CharmtonePantera() + r := attachments.NewRenderer( + sty.Attachments.Normal, + sty.Attachments.Deleting, + sty.Attachments.Image, + sty.Attachments.Text, + ) + msg := &message.Message{ + ID: "u-fin", + Role: message.User, + Parts: []message.ContentPart{message.TextContent{Text: "hi"}}, + } + item := NewUserMessageItem(&sty, msg, r).(*UserMessageItem) + require.True(t, item.Finished()) +} + +// TestAgentToolMessageItem_NestedToolMutatorsBumpVersion covers B1: +// the nested-tool mutators on AgentToolMessageItem must bump +// Version() so the list cache invalidates frozen entries when a +// nested tool is added or the slice changes. SetNestedTools always +// bumps unconditionally — the live update path in +// internal/ui/model/ui.go mutates existing children in place and +// then re-passes the same slice, so a pointer-equality dedupe would +// hide observable child-render changes. AddNestedTool also always +// observably mutates state and always bumps. +func TestAgentToolMessageItem_NestedToolMutatorsBumpVersion(t *testing.T) { + t.Parallel() + + sty := styles.CharmtonePantera() + parent := message.ToolCall{ID: "agent-parent", Name: "agent", Input: `{}`, Finished: false} + item := NewAgentToolMessageItem(&sty, parent, nil, false) + + mkChild := func(id string) ToolMessageItem { + tc := message.ToolCall{ID: id, Name: "bash", Input: `{}`, Finished: false} + return NewToolMessageItem(&sty, "msg", tc, nil, false) + } + + // AddNestedTool always bumps. + requireBump(t, "AddNestedTool", item, func() { + item.AddNestedTool(mkChild("c1")) + }) + + // SetNestedTools always bumps, even with a pointer-equal slice. + current := append([]ToolMessageItem(nil), item.NestedTools()...) + requireBump(t, "SetNestedTools[pointer-equal]", item, func() { + item.SetNestedTools(current) + }) + + // SetNestedTools with a different slice (extra element) bumps. + requireBump(t, "SetNestedTools[different]", item, func() { + item.SetNestedTools(append(current, mkChild("c2"))) + }) + + // SetNestedTools to an empty slice from a non-empty state bumps. + requireBump(t, "SetNestedTools[empty]", item, func() { + item.SetNestedTools(nil) + }) +} + +// TestAgenticFetchToolMessageItem_NestedToolMutatorsBumpVersion is +// the agentic-fetch counterpart to the agent-tool nested mutator +// bump test above. +func TestAgenticFetchToolMessageItem_NestedToolMutatorsBumpVersion(t *testing.T) { + t.Parallel() + + sty := styles.CharmtonePantera() + parent := message.ToolCall{ID: "fetch-parent", Name: "agentic_fetch", Input: `{}`, Finished: false} + item := NewAgenticFetchToolMessageItem(&sty, parent, nil, false) + + mkChild := func(id string) ToolMessageItem { + tc := message.ToolCall{ID: id, Name: "fetch", Input: `{}`, Finished: false} + return NewToolMessageItem(&sty, "msg", tc, nil, false) + } + + requireBump(t, "AddNestedTool", item, func() { + item.AddNestedTool(mkChild("c1")) + }) + + current := append([]ToolMessageItem(nil), item.NestedTools()...) + requireBump(t, "SetNestedTools[pointer-equal]", item, func() { + item.SetNestedTools(current) + }) + + requireBump(t, "SetNestedTools[different]", item, func() { + item.SetNestedTools(append(current, mkChild("c2"))) + }) + + requireBump(t, "SetNestedTools[empty]", item, func() { + item.SetNestedTools(nil) + }) +} + +// TestAgentToolMessageItem_NestedChildInPlaceMutationBumpsParent is +// the T5 regression test: it mirrors the live update flow at +// internal/ui/model/ui.go:1242-1281 where nested tool calls are +// updated in place (SetToolCall / SetResult on the same child +// pointers) and then the same slice is handed back to the parent +// via SetNestedTools. The parent must still bump its version so +// the list cache invalidates the parent's pre-rendered string and +// the freshly-rendered child output becomes visible. +func TestAgentToolMessageItem_NestedChildInPlaceMutationBumpsParent(t *testing.T) { + t.Parallel() + + sty := styles.CharmtonePantera() + parent := message.ToolCall{ID: "agent-parent", Name: "agent", Input: `{}`, Finished: false} + item := NewAgentToolMessageItem(&sty, parent, nil, false) + + childTC := message.ToolCall{ID: "c1", Name: "bash", Input: `{}`, Finished: false} + child := NewToolMessageItem(&sty, "msg", childTC, nil, false) + item.AddNestedTool(child) + + v0 := item.Version() + childVersionBefore := child.(versionedItem).Version() + + // In-place mutate the existing child, exactly like the live + // flow in ui.go:1271-1278 does. + child.SetResult(&message.ToolResult{ToolCallID: "c1", Content: "ok"}) + require.Greaterf(t, child.(versionedItem).Version(), childVersionBefore, + "child SetResult must bump child version") + + // Hand the same slice back to the parent (pointers unchanged). + same := item.NestedTools() + item.SetNestedTools(same) + require.Greaterf(t, item.Version(), v0, + "parent SetNestedTools must bump even when child pointers are unchanged (in-place child mutation invalidates parent's pre-rendered output)") +} + +// TestAgenticFetchToolMessageItem_NestedChildInPlaceMutationBumpsParent +// is the agentic-fetch counterpart of the T5 regression test. +func TestAgenticFetchToolMessageItem_NestedChildInPlaceMutationBumpsParent(t *testing.T) { + t.Parallel() + + sty := styles.CharmtonePantera() + parent := message.ToolCall{ID: "fetch-parent", Name: "agentic_fetch", Input: `{}`, Finished: false} + item := NewAgenticFetchToolMessageItem(&sty, parent, nil, false) + + childTC := message.ToolCall{ID: "c1", Name: "fetch", Input: `{}`, Finished: false} + child := NewToolMessageItem(&sty, "msg", childTC, nil, false) + item.AddNestedTool(child) + + v0 := item.Version() + childVersionBefore := child.(versionedItem).Version() + + child.SetResult(&message.ToolResult{ToolCallID: "c1", Content: "ok"}) + require.Greaterf(t, child.(versionedItem).Version(), childVersionBefore, + "child SetResult must bump child version") + + same := item.NestedTools() + item.SetNestedTools(same) + require.Greaterf(t, item.Version(), v0, + "parent SetNestedTools must bump even when child pointers are unchanged") +} + +// TestBaseToolMessageItem_FinishedTransition covers §4.5.1 for +// tools: a still-running tool reports Finished() == false; once the +// tool call is marked finished and a result lands, Finished() +// returns true. Cancelled tools also become Finished. +func TestBaseToolMessageItem_FinishedTransition(t *testing.T) { + t.Parallel() + + sty := styles.CharmtonePantera() + tc := message.ToolCall{ID: "tc-fin", Name: "bash", Input: "{}", Finished: false} + item := NewToolMessageItem(&sty, "msg", tc, nil, false) + require.False(t, item.Finished(), "running tool must not be Finished()") + + tcFinished := tc + tcFinished.Finished = true + item.SetToolCall(tcFinished) + item.SetResult(&message.ToolResult{ToolCallID: "tc-fin", Content: "ok"}) + require.True(t, item.Finished(), "finished tool with result must be Finished()") + + // Canceled tool with no result is also Finished. + tcCanceled := message.ToolCall{ID: "tc-cancel", Name: "bash", Input: "{}", Finished: false} + canceled := NewToolMessageItem(&sty, "msg", tcCanceled, nil, true) + require.True(t, canceled.Finished(), "canceled tool must be Finished()") +} diff --git a/internal/ui/completions/item.go b/internal/ui/completions/item.go index 3e99408dcc8e04288d5775dc01e17bcdd42a59a4..b149e58a50ab5dbe6025640fc3edd05fbc0aa246 100644 --- a/internal/ui/completions/item.go +++ b/internal/ui/completions/item.go @@ -1,6 +1,8 @@ package completions import ( + "slices" + "charm.land/lipgloss/v2" "github.com/charmbracelet/crush/internal/ui/list" "github.com/charmbracelet/x/ansi" @@ -23,6 +25,8 @@ type ResourceCompletionValue struct { // CompletionItem represents an item in the completions list. type CompletionItem struct { + *list.Versioned + text string value any match fuzzy.Match @@ -38,6 +42,7 @@ type CompletionItem struct { // NewCompletionItem creates a new completion item. func NewCompletionItem(text string, value any, normalStyle, focusedStyle, matchStyle lipgloss.Style) *CompletionItem { return &CompletionItem{ + Versioned: list.NewVersioned(), text: text, value: value, normalStyle: normalStyle, @@ -46,6 +51,15 @@ func NewCompletionItem(text string, value any, normalStyle, focusedStyle, matchS } } +// Finished implements list.Item. Completion items render purely from +// (text, match, focus); any mutation (SetMatch / SetFocused) bumps +// Version() so the frozen cache entry invalidates on the next +// render. Marking them finished lets the F6 list memo skip the +// per-line work for the steady completions popup. +func (c *CompletionItem) Finished() bool { + return true +} + // Text returns the display text of the item. func (c *CompletionItem) Text() string { return c.text @@ -63,16 +77,34 @@ func (c *CompletionItem) Filter() string { // SetMatch implements [list.MatchSettable]. func (c *CompletionItem) SetMatch(m fuzzy.Match) { + if sameFuzzyMatch(c.match, m) { + return + } c.cache = nil c.match = m + c.Bump() +} + +// sameFuzzyMatch reports whether two fuzzy.Match values are +// observably equal. Because Match contains a slice (MatchedIndexes) +// it is not directly comparable with ==; we compare the scalar +// fields and then walk the indexes. SetMatch uses this to skip +// gratuitous version bumps when the same match is reapplied. +func sameFuzzyMatch(a, b fuzzy.Match) bool { + return a.Str == b.Str && + a.Index == b.Index && + a.Score == b.Score && + slices.Equal(a.MatchedIndexes, b.MatchedIndexes) } // SetFocused implements [list.Focusable]. func (c *CompletionItem) SetFocused(focused bool) { - if c.focused != focused { - c.cache = nil + if c.focused == focused { + return } + c.cache = nil c.focused = focused + c.Bump() } // Render implements [list.Item]. diff --git a/internal/ui/completions/version_bump_test.go b/internal/ui/completions/version_bump_test.go new file mode 100644 index 0000000000000000000000000000000000000000..5d3c620c1ed17aa7477754e0cef1d68aa43026ee --- /dev/null +++ b/internal/ui/completions/version_bump_test.go @@ -0,0 +1,85 @@ +package completions + +import ( + "testing" + + "charm.land/lipgloss/v2" + "github.com/sahilm/fuzzy" + "github.com/stretchr/testify/require" +) + +// TestCompletionItem_MutatorsBumpVersion covers F6 §4.5 for the +// completions popup: SetMatch and SetFocused must bump Version() +// when the observable state changes, and dedupe (no bump) when the +// supplied value is identical to the current state. Without +// dedupe, the steady completions popup would invalidate the +// list-level memo on every keystroke that produced the same match. +func TestCompletionItem_MutatorsBumpVersion(t *testing.T) { + t.Parallel() + + mkItem := func() *CompletionItem { + return NewCompletionItem( + "internal/ui/chat/user.go", + FileCompletionValue{Path: "internal/ui/chat/user.go"}, + lipgloss.NewStyle(), + lipgloss.NewStyle(), + lipgloss.NewStyle(), + ) + } + + t.Run("SetFocused", func(t *testing.T) { + t.Parallel() + item := mkItem() + + // First transition (false -> true) must bump. + before := item.Version() + item.SetFocused(true) + require.Greater(t, item.Version(), before, "SetFocused(true) must bump") + + // Re-applying the same focus state must not bump. + stable := item.Version() + item.SetFocused(true) + require.Equal(t, stable, item.Version(), "SetFocused with same value must not bump") + + // Transition back must bump. + item.SetFocused(false) + require.Greater(t, item.Version(), stable, "SetFocused(false) must bump") + }) + + t.Run("SetMatch", func(t *testing.T) { + t.Parallel() + item := mkItem() + + match := fuzzy.Match{ + Str: "user", + Index: 0, + Score: 5, + MatchedIndexes: []int{0, 1, 2, 3}, + } + before := item.Version() + item.SetMatch(match) + require.Greater(t, item.Version(), before, "SetMatch with new value must bump") + + // Re-applying an equivalent match (same fields, equal slice + // contents) must not bump. + stable := item.Version() + same := fuzzy.Match{ + Str: "user", + Index: 0, + Score: 5, + MatchedIndexes: []int{0, 1, 2, 3}, + } + item.SetMatch(same) + require.Equal(t, stable, item.Version(), "SetMatch with equivalent value must not bump") + + // A different match (different MatchedIndexes) must bump. + different := fuzzy.Match{ + Str: "user", + Index: 0, + Score: 5, + MatchedIndexes: []int{0, 2}, + } + item.SetMatch(different) + require.Greater(t, item.Version(), stable, "SetMatch with different indexes must bump") + }) +} diff --git a/internal/ui/dialog/commands_item.go b/internal/ui/dialog/commands_item.go index a71229716d30876bc4e0a0d68dc45a2f27896745..8f656fb388e49eaf9d4b419142c936b4305b1945 100644 --- a/internal/ui/dialog/commands_item.go +++ b/internal/ui/dialog/commands_item.go @@ -3,12 +3,14 @@ package dialog import ( "strings" + "github.com/charmbracelet/crush/internal/ui/list" "github.com/charmbracelet/crush/internal/ui/styles" "github.com/sahilm/fuzzy" ) // CommandItem wraps a uicmd.Command to implement the ListItem interface. type CommandItem struct { + *list.Versioned id string title string shortcut string @@ -20,19 +22,26 @@ type CommandItem struct { focused bool } -var _ ListItem = &CommandItem{} +var _ ListItem = &CommandItem{Versioned: list.NewVersioned()} // NewCommandItem creates a new CommandItem. func NewCommandItem(t *styles.Styles, id, title, shortcut string, action Action) *CommandItem { return &CommandItem{ - id: id, - t: t, - title: title, - shortcut: shortcut, - action: action, + Versioned: list.NewVersioned(), + id: id, + t: t, + title: title, + shortcut: shortcut, + action: action, } } +// Finished implements list.Item. Command items are render-stable +// outside of explicit SetFocused / SetMatch. +func (c *CommandItem) Finished() bool { + return true +} + // WithAliases returns the CommandItem with the given aliases for filtering. func (c *CommandItem) WithAliases(aliases ...string) *CommandItem { c.aliases = aliases @@ -54,16 +63,26 @@ func (c *CommandItem) ID() string { // SetFocused implements ListItem. func (c *CommandItem) SetFocused(focused bool) { - if c.focused != focused { - c.cache = nil + if c.focused == focused { + return } + c.cache = nil c.focused = focused + if c.Versioned != nil { + c.Bump() + } } // SetMatch implements ListItem. func (c *CommandItem) SetMatch(m fuzzy.Match) { + if sameFuzzyMatch(c.m, m) { + return + } c.cache = nil c.m = m + if c.Versioned != nil { + c.Bump() + } } // Action returns the action associated with the command item. diff --git a/internal/ui/dialog/models_item.go b/internal/ui/dialog/models_item.go index 8147f8e4b56445531e7f7997dd970ca0172fcc5e..08c62b9d4150967dade66edab155280ca6b8fe68 100644 --- a/internal/ui/dialog/models_item.go +++ b/internal/ui/dialog/models_item.go @@ -5,6 +5,7 @@ import ( "charm.land/lipgloss/v2" "github.com/charmbracelet/crush/internal/config" "github.com/charmbracelet/crush/internal/ui/common" + "github.com/charmbracelet/crush/internal/ui/list" "github.com/charmbracelet/crush/internal/ui/styles" "github.com/charmbracelet/x/ansi" "github.com/sahilm/fuzzy" @@ -12,6 +13,7 @@ import ( // ModelGroup represents a group of model items. type ModelGroup struct { + *list.Versioned Title string Items []*ModelItem configured bool @@ -21,6 +23,7 @@ type ModelGroup struct { // NewModelGroup creates a new ModelGroup. func NewModelGroup(t *styles.Styles, title string, configured bool, items ...*ModelItem) ModelGroup { return ModelGroup{ + Versioned: list.NewVersioned(), Title: title, Items: items, configured: configured, @@ -28,6 +31,11 @@ func NewModelGroup(t *styles.Styles, title string, configured bool, items ...*Mo } } +// Finished implements list.Item. Model groups are immutable headers. +func (m *ModelGroup) Finished() bool { + return true +} + // AppendItems appends [ModelItem]s to the group. func (m *ModelGroup) AppendItems(items ...*ModelItem) { m.Items = append(m.Items, items...) @@ -50,6 +58,8 @@ func (m *ModelGroup) Render(width int) string { // ModelItem represents a list item for a model type. type ModelItem struct { + *list.Versioned + prov catwalk.Provider model catwalk.Model modelType ModelType @@ -61,6 +71,12 @@ type ModelItem struct { showProvider bool } +// Finished implements list.Item. Model items are render-stable +// outside of explicit SetFocused / SetMatch. +func (m *ModelItem) Finished() bool { + return true +} + // SelectedModel returns this model item as a [config.SelectedModel] instance. func (m *ModelItem) SelectedModel() config.SelectedModel { return config.SelectedModel{ @@ -81,6 +97,7 @@ var _ ListItem = &ModelItem{} // NewModelItem creates a new ModelItem. func NewModelItem(t *styles.Styles, prov catwalk.Provider, model catwalk.Model, typ ModelType, showProvider bool) *ModelItem { return &ModelItem{ + Versioned: list.NewVersioned(), prov: prov, model: model, modelType: typ, @@ -117,14 +134,24 @@ func (m *ModelItem) Render(width int) string { // SetFocused implements ListItem. func (m *ModelItem) SetFocused(focused bool) { - if m.focused != focused { - m.cache = nil + if m.focused == focused { + return } + m.cache = nil m.focused = focused + if m.Versioned != nil { + m.Bump() + } } // SetMatch implements ListItem. func (m *ModelItem) SetMatch(fm fuzzy.Match) { + if sameFuzzyMatch(m.m, fm) { + return + } m.cache = nil m.m = fm + if m.Versioned != nil { + m.Bump() + } } diff --git a/internal/ui/dialog/reasoning.go b/internal/ui/dialog/reasoning.go index a0aa664ad150b1b9dc126e784b9114865c9f58b4..74faf4e146aaf160c02b5ca3873923e168d2bed7 100644 --- a/internal/ui/dialog/reasoning.go +++ b/internal/ui/dialog/reasoning.go @@ -40,6 +40,7 @@ type Reasoning struct { // ReasoningItem represents a reasoning effort list item. type ReasoningItem struct { + *list.Versioned effort string title string isCurrent bool @@ -49,6 +50,12 @@ type ReasoningItem struct { focused bool } +// Finished implements list.Item. Reasoning items are render-stable +// outside of explicit SetFocused / SetMatch. +func (r *ReasoningItem) Finished() bool { + return true +} + var ( _ Dialog = (*Reasoning)(nil) _ ListItem = (*ReasoningItem)(nil) @@ -243,6 +250,7 @@ func (r *Reasoning) setReasoningItems() error { selectedIndex := 0 for i, effort := range model.ReasoningLevels { item := &ReasoningItem{ + Versioned: list.NewVersioned(), effort: effort, title: common.FormatReasoningEffort(effort), isCurrent: effort == currentEffort, @@ -272,16 +280,26 @@ func (r *ReasoningItem) ID() string { // SetFocused sets the focus state of the reasoning item. func (r *ReasoningItem) SetFocused(focused bool) { - if r.focused != focused { - r.cache = nil + if r.focused == focused { + return } + r.cache = nil r.focused = focused + if r.Versioned != nil { + r.Bump() + } } // SetMatch sets the fuzzy match for the reasoning item. func (r *ReasoningItem) SetMatch(m fuzzy.Match) { + if sameFuzzyMatch(r.m, m) { + return + } r.cache = nil r.m = m + if r.Versioned != nil { + r.Bump() + } } // Render returns the string representation of the reasoning item. diff --git a/internal/ui/dialog/sessions_item.go b/internal/ui/dialog/sessions_item.go index 0de64efa667d955eef4869349b6ac060704d235e..b7b8b00d3f3f914db7dc6cb13571ad6bd9899bed 100644 --- a/internal/ui/dialog/sessions_item.go +++ b/internal/ui/dialog/sessions_item.go @@ -2,6 +2,7 @@ package dialog import ( "fmt" + "slices" "strings" "time" @@ -17,6 +18,19 @@ import ( "github.com/sahilm/fuzzy" ) +// sameFuzzyMatch reports whether two fuzzy.Match values are +// observably equal. Because Match contains a slice (MatchedIndexes) +// it is not directly comparable with ==; we compare the scalar +// fields and then walk the indexes. The dialog list items use this +// to skip gratuitous version bumps when SetMatch reapplies the same +// match. +func sameFuzzyMatch(a, b fuzzy.Match) bool { + return a.Str == b.Str && + a.Index == b.Index && + a.Score == b.Score && + slices.Equal(a.MatchedIndexes, b.MatchedIndexes) +} + // ListItem represents a selectable and searchable item in a dialog list. type ListItem interface { list.FilterableItem @@ -29,6 +43,7 @@ type ListItem interface { // SessionItem wraps a [session.Session] to implement the [ListItem] interface. type SessionItem struct { + *list.Versioned session.Session t *styles.Styles sessionsMode sessionsMode @@ -38,6 +53,13 @@ type SessionItem struct { focused bool } +// Finished implements list.Item. Session items are render-stable +// outside of explicit SetFocused / SetMatch calls, both of which +// bump Version() and therefore invalidate the F6 frozen entry. +func (s *SessionItem) Finished() bool { + return true +} + var _ ListItem = &SessionItem{} // Filter returns the filterable value of the session. @@ -52,8 +74,14 @@ func (s *SessionItem) ID() string { // SetMatch sets the fuzzy match for the session item. func (s *SessionItem) SetMatch(m fuzzy.Match) { + if sameFuzzyMatch(s.m, m) { + return + } s.cache = nil s.m = m + if s.Versioned != nil { + s.Bump() + } } // InputValue returns the updated title value @@ -177,10 +205,14 @@ func renderItem(t ListItemStyles, title string, info string, focused bool, width // SetFocused sets the focus state of the session item. func (s *SessionItem) SetFocused(focused bool) { - if s.focused != focused { - s.cache = nil + if s.focused == focused { + return } + s.cache = nil s.focused = focused + if s.Versioned != nil { + s.Bump() + } } // sessionItems takes a slice of [session.Session]s and convert them to a slice @@ -188,7 +220,7 @@ func (s *SessionItem) SetFocused(focused bool) { func sessionItems(t *styles.Styles, mode sessionsMode, sessions ...session.Session) []list.FilterableItem { items := make([]list.FilterableItem, len(sessions)) for i, s := range sessions { - item := &SessionItem{Session: s, t: t, sessionsMode: mode} + item := &SessionItem{Versioned: list.NewVersioned(), Session: s, t: t, sessionsMode: mode} if mode == sessionsModeUpdating { item.updateTitleInput = textinput.New() item.updateTitleInput.SetVirtualCursor(false) diff --git a/internal/ui/dialog/version_bump_test.go b/internal/ui/dialog/version_bump_test.go new file mode 100644 index 0000000000000000000000000000000000000000..111ff23526e07bf7800488cdaa49bf84dbd758f9 --- /dev/null +++ b/internal/ui/dialog/version_bump_test.go @@ -0,0 +1,219 @@ +package dialog + +import ( + "testing" + + "charm.land/catwalk/pkg/catwalk" + "github.com/charmbracelet/crush/internal/session" + "github.com/charmbracelet/crush/internal/ui/list" + "github.com/charmbracelet/crush/internal/ui/styles" + "github.com/sahilm/fuzzy" + "github.com/stretchr/testify/require" +) + +// versionedItem is the cross-cutting interface every dialog list +// item must satisfy under F6: every documented mutator must bump +// the shared version counter so the list-level memo invalidates +// frozen entries. +type versionedItem interface { + list.Item + Version() uint64 +} + +// requireBump asserts that running mutate() advances the item's +// Version(). +func requireBump(t *testing.T, name string, item versionedItem, mutate func()) { + t.Helper() + before := item.Version() + mutate() + after := item.Version() + require.Greaterf(t, after, before, "%s must bump Version() (before=%d, after=%d)", name, before, after) +} + +// requireNoBump asserts that running mutate() leaves the item's +// Version() unchanged. Used to lock in the dedupe contract: a +// mutator called with a value identical to the current state must +// not gratuitously invalidate the list cache. +func requireNoBump(t *testing.T, name string, item versionedItem, mutate func()) { + t.Helper() + before := item.Version() + mutate() + after := item.Version() + require.Equalf(t, before, after, "%s must NOT bump Version() when state is unchanged (before=%d, after=%d)", name, before, after) +} + +// equivMatch returns a fuzzy.Match whose fields and indexes are +// equivalent to the supplied seed but allocated as a fresh struct +// so callers exercise the value-equality dedupe path rather than +// referential equality. +func equivMatch(seed fuzzy.Match) fuzzy.Match { + return fuzzy.Match{ + Str: seed.Str, + Index: seed.Index, + Score: seed.Score, + MatchedIndexes: append([]int(nil), seed.MatchedIndexes...), + } +} + +// TestCommandItem_MutatorsBumpVersion covers F6 §4.5 for the +// commands palette items: SetFocused and SetMatch bump Version() +// on observable change and dedupe otherwise. +func TestCommandItem_MutatorsBumpVersion(t *testing.T) { + t.Parallel() + + sty := styles.CharmtonePantera() + item := NewCommandItem(&sty, "id", "Title", "ctrl+t", nil) + + requireBump(t, "SetFocused[true]", item, func() { + item.SetFocused(true) + }) + requireNoBump(t, "SetFocused[true again]", item, func() { + item.SetFocused(true) + }) + requireBump(t, "SetFocused[false]", item, func() { + item.SetFocused(false) + }) + + match := fuzzy.Match{ + Str: "Title", + Index: 0, + Score: 5, + MatchedIndexes: []int{0, 1, 2}, + } + requireBump(t, "SetMatch[new]", item, func() { + item.SetMatch(match) + }) + requireNoBump(t, "SetMatch[same]", item, func() { + item.SetMatch(equivMatch(match)) + }) + requireBump(t, "SetMatch[different]", item, func() { + item.SetMatch(fuzzy.Match{ + Str: "Title", + Index: 0, + Score: 5, + MatchedIndexes: []int{0, 2}, + }) + }) +} + +// TestModelItem_MutatorsBumpVersion covers F6 §4.5 for the model +// picker items. +func TestModelItem_MutatorsBumpVersion(t *testing.T) { + t.Parallel() + + sty := styles.CharmtonePantera() + prov := catwalk.Provider{ID: "openai", Name: "OpenAI"} + model := catwalk.Model{ID: "gpt-4", Name: "GPT-4"} + item := NewModelItem(&sty, prov, model, ModelTypeLarge, true) + + requireBump(t, "SetFocused[true]", item, func() { + item.SetFocused(true) + }) + requireNoBump(t, "SetFocused[true again]", item, func() { + item.SetFocused(true) + }) + + match := fuzzy.Match{ + Str: "GPT-4", + Index: 0, + Score: 5, + MatchedIndexes: []int{0, 1, 2}, + } + requireBump(t, "SetMatch[new]", item, func() { + item.SetMatch(match) + }) + requireNoBump(t, "SetMatch[same]", item, func() { + item.SetMatch(equivMatch(match)) + }) + requireBump(t, "SetMatch[different]", item, func() { + item.SetMatch(fuzzy.Match{ + Str: "GPT-4", + Index: 0, + Score: 5, + MatchedIndexes: []int{1}, + }) + }) +} + +// TestSessionItem_MutatorsBumpVersion covers F6 §4.5 for the +// sessions dialog items. +func TestSessionItem_MutatorsBumpVersion(t *testing.T) { + t.Parallel() + + sty := styles.CharmtonePantera() + item := &SessionItem{ + Versioned: list.NewVersioned(), + Session: session.Session{ID: "sess-1", Title: "My Session"}, + t: &sty, + } + + requireBump(t, "SetFocused[true]", item, func() { + item.SetFocused(true) + }) + requireNoBump(t, "SetFocused[true again]", item, func() { + item.SetFocused(true) + }) + + match := fuzzy.Match{ + Str: "My Session", + Index: 0, + Score: 5, + MatchedIndexes: []int{0, 1, 2}, + } + requireBump(t, "SetMatch[new]", item, func() { + item.SetMatch(match) + }) + requireNoBump(t, "SetMatch[same]", item, func() { + item.SetMatch(equivMatch(match)) + }) + requireBump(t, "SetMatch[different]", item, func() { + item.SetMatch(fuzzy.Match{ + Str: "My Session", + Index: 0, + Score: 5, + MatchedIndexes: []int{3, 4}, + }) + }) +} + +// TestReasoningItem_MutatorsBumpVersion covers F6 §4.5 for the +// reasoning effort dialog items. +func TestReasoningItem_MutatorsBumpVersion(t *testing.T) { + t.Parallel() + + sty := styles.CharmtonePantera() + item := &ReasoningItem{ + Versioned: list.NewVersioned(), + effort: "medium", + title: "Medium", + t: &sty, + } + + requireBump(t, "SetFocused[true]", item, func() { + item.SetFocused(true) + }) + requireNoBump(t, "SetFocused[true again]", item, func() { + item.SetFocused(true) + }) + + match := fuzzy.Match{ + Str: "Medium", + Index: 0, + Score: 5, + MatchedIndexes: []int{0, 1, 2}, + } + requireBump(t, "SetMatch[new]", item, func() { + item.SetMatch(match) + }) + requireNoBump(t, "SetMatch[same]", item, func() { + item.SetMatch(equivMatch(match)) + }) + requireBump(t, "SetMatch[different]", item, func() { + item.SetMatch(fuzzy.Match{ + Str: "Medium", + Index: 0, + Score: 5, + MatchedIndexes: []int{2, 3}, + }) + }) +} diff --git a/internal/ui/list/item.go b/internal/ui/list/item.go index 7ac87212889dbc58773b409b5a4a96ec47d1fede..68c34a896cffbf8fdd839e4dc2b7da43af6fe679 100644 --- a/internal/ui/list/item.go +++ b/internal/ui/list/item.go @@ -7,10 +7,66 @@ import ( ) // Item represents a single item in the lazy-loaded list. +// +// Items participate in the list-level render memo (F6). The cache key +// for each item is (pointer, width, version). Items must: +// +// - Bump their version (via the embedded *Versioned helper) on every +// mutation that changes the rendered output. +// - Return Finished() == true once their rendered output will not +// change again unless an explicit mutator is invoked. Frozen +// entries are emitted verbatim — no Render call — until either +// Version() bumps, the viewport width changes, or the list +// explicitly invalidates the entry. type Item interface { // Render returns the string representation of the item for the given // width. Render(width int) string + + // Version returns a monotonic counter that the list-level cache + // uses to detect mutations. Items must increment the version + // (via Versioned.Bump) on every state change that would alter + // the rendered output. + Version() uint64 + + // Finished reports whether the item's rendered output has reached + // a terminal state and may be frozen by the list cache. Items + // that animate, stream, or otherwise still mutate must return + // false. A finished item that later mutates must bump its + // version on the mutation; the cache treats version bumps as + // implicit unfreeze + invalidate. + Finished() bool +} + +// Versioned is a tiny embeddable helper that satisfies Item.Version() +// and provides a Bump() method to call from every state-mutating +// method. Items typically embed *Versioned alongside their other +// helpers; see chat.AssistantMessageItem for the canonical wiring. +// +// Bump() is not safe for concurrent use; callers must hold whatever +// synchronization their item type already requires for state +// mutations. The list itself never reads Version() from a goroutine +// other than the UI thread. +type Versioned struct { + v uint64 +} + +// NewVersioned returns a fresh *Versioned at version zero. +func NewVersioned() *Versioned { + return &Versioned{} +} + +// Version returns the current version counter. +func (vc *Versioned) Version() uint64 { + return vc.v +} + +// Bump advances the version counter by one. Mutators on items that +// affect the rendered output must call Bump exactly once per +// observable state change. Bumping more than once per change is +// harmless other than a single extra cache miss. +func (vc *Versioned) Bump() { + vc.v++ } // RawRenderable represents an item that can provide a raw rendering @@ -45,13 +101,15 @@ type MouseClickable interface { // SpacerItem is a spacer item that adds vertical space in the list. type SpacerItem struct { + *Versioned Height int } // NewSpacerItem creates a new [SpacerItem] with the specified height. func NewSpacerItem(height int) *SpacerItem { return &SpacerItem{ - Height: max(0, height-1), + Versioned: NewVersioned(), + Height: max(0, height-1), } } @@ -59,3 +117,10 @@ func NewSpacerItem(height int) *SpacerItem { func (s *SpacerItem) Render(width int) string { return strings.Repeat("\n", s.Height) } + +// Finished implements Item. SpacerItems are immutable in practice and +// safe to freeze; any mutation goes through Versioned.Bump which +// invalidates the frozen entry. +func (s *SpacerItem) Finished() bool { + return true +} diff --git a/internal/ui/list/list.go b/internal/ui/list/list.go index f6c3fdc44501923f37a1943fa69d4432a6d4720c..a4e30bac2dcd02013910551b68a71740c8987d21 100644 --- a/internal/ui/list/list.go +++ b/internal/ui/list/list.go @@ -33,9 +33,40 @@ type List struct { // renderCallbacks is a list of callbacks to apply when rendering items. renderCallbacks []func(idx, selectedIdx int, item Item) Item + + // cache is the F6 list-level render memo, keyed by item pointer. + // Each entry stores the rendered content, a pre-split slice of + // lines (so AtBottom / Render / VisibleItemIndices / + // findItemAtY all share one render per frame), the height, and + // the keys that govern invalidation (width and version). The + // frozen flag mirrors §4.5.1: once a Finished() item is + // rendered, subsequent draws return the stored output verbatim + // without calling back into Render. + cache map[Item]*listCacheEntry + + // freezeSuppressed marks items the list must not freeze on the + // next render even when their Finished() reports true. This is + // the §4.5.1 selection-drag escape hatch (option (a)): items + // inside an active selection range render as live items so that + // per-line highlight overlays land on the latest content. Cleared + // on EndSelectionDrag. + freezeSuppressed map[Item]struct{} +} + +// listCacheEntry is the per-item entry in the list-level render memo. +type listCacheEntry struct { + width int + version uint64 + frozen bool + content string + lines []string + height int } -// renderedItem holds the rendered content and height of an item. +// renderedItem is the legacy view of a cached entry returned by getItem. +// Internal callers that don't need the line slice keep using this +// shape; functions that walk lines (Render) take the slice off the +// cache entry directly. type renderedItem struct { content string height int @@ -46,6 +77,8 @@ func NewList(items ...Item) *List { l := new(List) l.items = items l.selectedIdx = -1 + l.cache = make(map[Item]*listCacheEntry) + l.freezeSuppressed = make(map[Item]struct{}) return l } @@ -59,8 +92,13 @@ func (l *List) RegisterRenderCallback(cb RenderCallback) { l.renderCallbacks = append(l.renderCallbacks, cb) } -// SetSize sets the size of the list viewport. +// SetSize sets the size of the list viewport. A width change drops the +// entire render cache because every entry's wrapped output depends on +// width; a height-only change is a no-op for the cache. func (l *List) SetSize(width, height int) { + if l.width != width { + l.invalidateAll() + } l.width = width l.height = height } @@ -144,12 +182,42 @@ func (l *List) lastOffsetItem() (int, int, int) { } // getItem renders (if needed) and returns the item at the given index. +// The result is served from the F6 cache when possible — see +// renderItemEntry for the cache-key semantics. func (l *List) getItem(idx int) renderedItem { if idx < 0 || idx >= len(l.items) { return renderedItem{} } + entry := l.renderItemEntry(idx) + if entry == nil { + return renderedItem{} + } + return renderedItem{content: entry.content, height: entry.height} +} + +// renderItemEntry returns the cache entry for the given index, populating +// the cache on miss. The result must not be retained past the next +// invalidation (SetSize width change, SetItems, etc.). +// +// Render callbacks always run, even for frozen entries: callbacks +// are how the list discovers per-frame state changes (selection, +// highlight range) and they bump the item's version when those +// changes affect the rendered output. A frozen item whose callback +// run is a no-op (same focus, same highlight) keeps its stored +// version and the cache hit is preserved on the post-callback +// version check. +func (l *List) renderItemEntry(idx int) *listCacheEntry { + if idx < 0 || idx >= len(l.items) { + return nil + } - item := l.items[idx] + rawItem := l.items[idx] + entry := l.cache[rawItem] + + // Run render callbacks. Callbacks may mutate the item (focus, + // highlight) which in turn bumps its version when state actually + // changes. We capture the post-callback version below. + item := rawItem if len(l.renderCallbacks) > 0 { for _, cb := range l.renderCallbacks { if it := cb(idx, l.selectedIdx, item); it != nil { @@ -158,15 +226,128 @@ func (l *List) getItem(idx int) renderedItem { } } + version := rawItem.Version() + if entry != nil && entry.width == l.width && entry.version == version { + // Cache hit — frozen or unfrozen, the entry content is + // still correct because no version bump landed since the + // last render. Selection-drag suppression turns this into + // a miss only if the entry is frozen. + if !entry.frozen { + return entry + } + if _, suppressed := l.freezeSuppressed[rawItem]; !suppressed { + return entry + } + } + rendered := item.Render(l.width) rendered = strings.TrimRight(rendered, "\n") - height := strings.Count(rendered, "\n") + 1 - ri := renderedItem{ - content: rendered, - height: height, + lines := strings.Split(rendered, "\n") + height := len(lines) + + // Re-read the version after Render so that any version bumps + // caused by Render itself (e.g. an item that mutates internal + // state during rendering) are captured. Without this we would + // freeze a stale entry under the post-render version. + finalVersion := rawItem.Version() + + frozen := false + if rawItem.Finished() { + if _, suppressed := l.freezeSuppressed[rawItem]; !suppressed { + frozen = true + } + } + + if entry == nil { + entry = &listCacheEntry{} + l.cache[rawItem] = entry + } + entry.width = l.width + entry.version = finalVersion + entry.frozen = frozen + entry.content = rendered + entry.lines = lines + entry.height = height + return entry +} + +// invalidateAll drops every cache entry. Called on width changes. +func (l *List) invalidateAll() { + for k := range l.cache { + delete(l.cache, k) } +} - return ri +// Invalidate drops the cache entry for the given item, forcing a +// re-render on the next getItem call. No-op if the item is not in +// the cache. +func (l *List) Invalidate(item Item) { + delete(l.cache, item) +} + +// InvalidateFrozen drops the frozen flag (and stored content) for the +// given item. Equivalent to Invalidate but exposed under the F6 +// frozen-items vocabulary so external callers can express intent. +func (l *List) InvalidateFrozen(item Item) { + delete(l.cache, item) +} + +// retainCacheFor drops every cache entry whose key is not in the given +// item set. Used by SetItems to keep entries for stable items while +// dropping entries for removed ones. +func (l *List) retainCacheFor(items []Item) { + if len(l.cache) == 0 { + return + } + keep := make(map[Item]struct{}, len(items)) + for _, it := range items { + keep[it] = struct{}{} + } + for k := range l.cache { + if _, ok := keep[k]; !ok { + delete(l.cache, k) + } + } +} + +// BeginSelectionDrag marks the items in the inclusive [startIdx, endIdx] +// range as un-freezable for the duration of an active selection drag. +// Frozen entries inside the range are dropped so the next render +// reflects live selection-overlay output. The corresponding +// EndSelectionDrag clears the suppression set and lets items +// re-freeze on their next render. Indices outside the items slice +// are clipped silently. +func (l *List) BeginSelectionDrag(startIdx, endIdx int) { + if len(l.items) == 0 { + return + } + if startIdx > endIdx { + startIdx, endIdx = endIdx, startIdx + } + startIdx = max(startIdx, 0) + endIdx = min(endIdx, len(l.items)-1) + for i := startIdx; i <= endIdx; i++ { + it := l.items[i] + l.freezeSuppressed[it] = struct{}{} + // Drop any cached frozen entry so the next render rebuilds + // it as a live (un-frozen) entry that picks up the + // selection overlay. + if entry, ok := l.cache[it]; ok && entry.frozen { + delete(l.cache, it) + } + } +} + +// EndSelectionDrag clears the selection-drag freeze suppression. Items +// inside the previous range will re-freeze on their next render once +// their Finished() reports true again. +func (l *List) EndSelectionDrag() { + for k := range l.freezeSuppressed { + delete(l.freezeSuppressed, k) + // Drop the cache entry so the next render produces a clean + // (un-highlighted) frozen entry. + delete(l.cache, k) + } } // ScrollToIndex scrolls the list to the given item index. @@ -288,8 +469,11 @@ func (l *List) Render() string { linesNeeded := l.height for linesNeeded > 0 && currentIdx < len(l.items) { - item := l.getItem(currentIdx) - itemLines := strings.Split(item.content, "\n") + entry := l.renderItemEntry(currentIdx) + if entry == nil { + break + } + itemLines := entry.lines itemHeight := len(itemLines) if currentOffset >= 0 && currentOffset < itemHeight { @@ -348,18 +532,15 @@ func (l *List) PrependItems(items ...Item) { } } -// SetItems sets the items in the list. +// SetItems sets the items in the list. Cache entries for items that +// remain after the swap are preserved; entries for removed items are +// dropped. func (l *List) SetItems(items ...Item) { - l.setItems(true, items...) -} - -// setItems sets the items in the list. If evict is true, it clears the -// rendered item cache. -func (l *List) setItems(evict bool, items ...Item) { l.items = items l.selectedIdx = min(l.selectedIdx, len(l.items)-1) l.offsetIdx = min(l.offsetIdx, len(l.items)-1) l.offsetLine = 0 + l.retainCacheFor(items) } // AppendItems appends items to the list. @@ -373,9 +554,16 @@ func (l *List) RemoveItem(idx int) { return } + removed := l.items[idx] + // Remove the item l.items = append(l.items[:idx], l.items[idx+1:]...) + // Drop the cache entry for the removed item; entries for stable + // items stay valid because they are keyed by pointer, not index. + delete(l.cache, removed) + delete(l.freezeSuppressed, removed) + // Adjust selection if needed if l.selectedIdx == idx { l.selectedIdx = -1 diff --git a/internal/ui/list/list_test.go b/internal/ui/list/list_test.go new file mode 100644 index 0000000000000000000000000000000000000000..5f672b3fcd00cb7175c54e4e8415e1b4224f3a11 --- /dev/null +++ b/internal/ui/list/list_test.go @@ -0,0 +1,401 @@ +package list + +import ( + "strconv" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// trackedItem is a test helper that counts Render calls. The body of +// Render is the item's content concatenated with the call counter so +// that "served from cache" vs "freshly rendered" is observable from +// the rendered string itself. +type trackedItem struct { + *Versioned + id string + body string + finished bool + renderHits int +} + +func newTrackedItem(id, body string, finished bool) *trackedItem { + return &trackedItem{ + Versioned: NewVersioned(), + id: id, + body: body, + finished: finished, + } +} + +func (t *trackedItem) Render(width int) string { + t.renderHits++ + return t.body + ":w=" + strconv.Itoa(width) +} + +func (t *trackedItem) Finished() bool { + return t.finished +} + +// TestList_RenderMemo_PointerKey covers the F6 invariant that the +// list-level cache is keyed by item pointer, not slice index, so +// PrependItems and AppendItems do not shift cached entries to the +// wrong item. +func TestList_RenderMemo_PointerKey(t *testing.T) { + t.Parallel() + + a := newTrackedItem("a", "alpha", false) + b := newTrackedItem("b", "bravo", false) + c := newTrackedItem("c", "charlie", false) + + l := NewList(a, b, c) + l.SetSize(40, 10) + + // First render populates the cache for every item. + first := l.Render() + require.Equal(t, 1, a.renderHits) + require.Equal(t, 1, b.renderHits) + require.Equal(t, 1, c.renderHits) + + // Prepending a new item must not shift the existing entries to + // the wrong key. The existing items render exactly once more + // only if their cache was lost, which would be a bug. Scroll to + // the top so the prepended item is visible and gets rendered. + z := newTrackedItem("z", "zulu", false) + l.PrependItems(z) + l.ScrollToTop() + _ = l.Render() + require.Equal(t, 1, z.renderHits, "prepended item rendered once") + require.Equal(t, 1, a.renderHits, "stable item must keep its cached entry across PrependItems") + require.Equal(t, 1, b.renderHits, "stable item must keep its cached entry across PrependItems") + require.Equal(t, 1, c.renderHits, "stable item must keep its cached entry across PrependItems") + + // AppendItems is symmetric. + d := newTrackedItem("d", "delta", false) + l.AppendItems(d) + _ = l.Render() + require.Equal(t, 1, d.renderHits, "appended item rendered once") + require.Equal(t, 1, a.renderHits) + require.Equal(t, 1, b.renderHits) + require.Equal(t, 1, c.renderHits) + + // The output is non-trivial. + require.Contains(t, first, "alpha") +} + +// TestList_SetSize_WidthChangeInvalidates covers the F6 invariant +// that a width change drops every cached entry but a height-only +// change leaves the cache intact. +func TestList_SetSize_WidthChangeInvalidates(t *testing.T) { + t.Parallel() + + a := newTrackedItem("a", "alpha", false) + b := newTrackedItem("b", "bravo", false) + + l := NewList(a, b) + l.SetSize(40, 10) + _ = l.Render() + require.Equal(t, 1, a.renderHits) + require.Equal(t, 1, b.renderHits) + + // Height-only change: no invalidation. + l.SetSize(40, 20) + _ = l.Render() + require.Equal(t, 1, a.renderHits, "height-only change must keep cache entries") + require.Equal(t, 1, b.renderHits, "height-only change must keep cache entries") + + // Width change: every entry invalidates. + l.SetSize(80, 20) + _ = l.Render() + require.Equal(t, 2, a.renderHits, "width change must invalidate cache entries") + require.Equal(t, 2, b.renderHits, "width change must invalidate cache entries") +} + +// TestList_RemoveItem_DropsEntry covers the F6 invariant that +// RemoveItem drops the cache entry for the removed item but leaves +// the surviving entries in place. +func TestList_RemoveItem_DropsEntry(t *testing.T) { + t.Parallel() + + a := newTrackedItem("a", "alpha", false) + b := newTrackedItem("b", "bravo", false) + c := newTrackedItem("c", "charlie", false) + + l := NewList(a, b, c) + l.SetSize(40, 10) + _ = l.Render() + require.Equal(t, 1, a.renderHits) + require.Equal(t, 1, b.renderHits) + require.Equal(t, 1, c.renderHits) + + l.RemoveItem(1) // remove b + _ = l.Render() + // a and c still cached. + require.Equal(t, 1, a.renderHits, "stable item must keep cached entry across RemoveItem") + require.Equal(t, 1, c.renderHits, "stable item must keep cached entry across RemoveItem") + // The removed item's entry is dropped — verify by re-adding b + // and confirming it renders as if fresh. + l.AppendItems(b) + _ = l.Render() + require.Equal(t, 2, b.renderHits, "re-added item must re-render") +} + +// TestList_FrozenItem_NotReRendered covers §4.5.1: items that report +// Finished() == true on entry creation are marked frozen after the +// first render and are never re-rendered until width change or +// version bump. +func TestList_FrozenItem_NotReRendered(t *testing.T) { + t.Parallel() + + a := newTrackedItem("a", "alpha", true) + b := newTrackedItem("b", "bravo", true) + + l := NewList(a, b) + l.SetSize(40, 10) + _ = l.Render() + require.Equal(t, 1, a.renderHits, "frozen items render exactly once on first draw") + require.Equal(t, 1, b.renderHits, "frozen items render exactly once on first draw") + + // Many subsequent renders must not re-render frozen items. + for range 5 { + _ = l.Render() + } + require.Equal(t, 1, a.renderHits, "frozen items must not re-render across redraws") + require.Equal(t, 1, b.renderHits, "frozen items must not re-render across redraws") +} + +// TestList_FrozenItem_TransitionsAfterFinish covers §4.5.1: a +// streaming item that later reports Finished() == true transitions +// to frozen on the first render after finish. +func TestList_FrozenItem_TransitionsAfterFinish(t *testing.T) { + t.Parallel() + + a := newTrackedItem("a", "alpha", false) // streaming + l := NewList(a) + l.SetSize(40, 10) + + // While unfinished, every render rebuilds the cache because the + // item's Finished() is false. + for range 3 { + // Bump the version to simulate a streaming delta. + a.Bump() + _ = l.Render() + } + require.Equal(t, 3, a.renderHits) + + // Item finishes; on the next render it freezes. + a.finished = true + a.Bump() + _ = l.Render() + require.Equal(t, 4, a.renderHits, "post-finish render still happens once") + + for range 5 { + _ = l.Render() + } + require.Equal(t, 4, a.renderHits, "frozen after finish, no further renders") +} + +// TestList_FrozenItem_VersionBumpUnfreezes covers §4.5.1: a frozen +// item that gets a version bump (unexpectedly mutated) is unfrozen +// and re-rendered — no stale output. +func TestList_FrozenItem_VersionBumpUnfreezes(t *testing.T) { + t.Parallel() + + a := newTrackedItem("a", "alpha", true) + l := NewList(a) + l.SetSize(40, 10) + + _ = l.Render() + _ = l.Render() + require.Equal(t, 1, a.renderHits) + + a.Bump() + _ = l.Render() + require.Equal(t, 2, a.renderHits, "version bump must invalidate frozen entry") + + // Re-renders without bumping go back to cache hits. + _ = l.Render() + _ = l.Render() + require.Equal(t, 2, a.renderHits, "post-bump render re-freezes") +} + +// TestList_FrozenItem_ResizeUnfreezes covers §4.5.1: resize +// invalidates frozen entries. +func TestList_FrozenItem_ResizeUnfreezes(t *testing.T) { + t.Parallel() + + a := newTrackedItem("a", "alpha", true) + l := NewList(a) + l.SetSize(40, 10) + + _ = l.Render() + require.Equal(t, 1, a.renderHits) + + l.SetSize(80, 10) + _ = l.Render() + require.Equal(t, 2, a.renderHits, "width change must invalidate frozen entry") +} + +// TestList_FrozenItem_SelectionDragUnfreeze covers §4.5.1: an active +// selection-drag span must un-freeze items inside the range; ending +// the drag re-freezes them. +func TestList_FrozenItem_SelectionDragUnfreeze(t *testing.T) { + t.Parallel() + + a := newTrackedItem("a", "alpha", true) + b := newTrackedItem("b", "bravo", true) + c := newTrackedItem("c", "charlie", true) + + l := NewList(a, b, c) + l.SetSize(40, 10) + _ = l.Render() + require.Equal(t, 1, a.renderHits) + require.Equal(t, 1, b.renderHits) + require.Equal(t, 1, c.renderHits) + + // Begin a selection drag spanning items 0..1. Items inside the + // range must re-render (they re-render exactly once because + // the un-freeze drops the cached entry, and the selection + // suppression keeps them un-frozen until the drag ends). + l.BeginSelectionDrag(0, 1) + _ = l.Render() + require.Equal(t, 2, a.renderHits, "drag-spanned item must re-render once on entering the drag") + require.Equal(t, 2, b.renderHits, "drag-spanned item must re-render once on entering the drag") + require.Equal(t, 1, c.renderHits, "out-of-range item must remain frozen") + + // While the drag is active, items inside the range are NOT + // frozen. Subsequent renders without state changes still + // trigger re-renders (because version+width hit but frozen=false + // also matches; we still re-use the cache — no, actually with + // our implementation we DO cache unfrozen entries by version). + _ = l.Render() + require.Equal(t, 2, a.renderHits, "unfrozen but version-stable hits the cache") + require.Equal(t, 2, b.renderHits, "unfrozen but version-stable hits the cache") + + // End the drag. Items inside the range re-render once and + // re-freeze. + l.EndSelectionDrag() + _ = l.Render() + require.Equal(t, 3, a.renderHits, "post-drag render re-freezes the entry") + require.Equal(t, 3, b.renderHits, "post-drag render re-freezes the entry") + + // Subsequent renders are cache hits again. + for range 3 { + _ = l.Render() + } + require.Equal(t, 3, a.renderHits, "frozen after drag end") + require.Equal(t, 3, b.renderHits, "frozen after drag end") +} + +// TestList_RenderOutputStableAcrossDraws is the F6 byte-equality +// invariant: rendering the same list multiple times must produce the +// same bytes. +func TestList_RenderOutputStableAcrossDraws(t *testing.T) { + t.Parallel() + + items := make([]Item, 0, 5) + for i := range 5 { + items = append(items, newTrackedItem(strconv.Itoa(i), "item-"+strconv.Itoa(i), i%2 == 0)) + } + l := NewList(items...) + l.SetSize(40, 20) + + first := l.Render() + for range 4 { + require.Equal(t, first, l.Render(), "render output must be byte-stable across draws") + } + // And the output is non-trivial. + require.True(t, strings.Contains(first, "item-0")) +} + +// TestList_SetItems_PointerOverlapRetainsCache covers F6 §4.5 +// invalidation semantics for SetItems. When the new slice shares +// some pointers with the previous slice (a typical "swap a few +// items, keep the rest" scenario), the cache entries for the +// surviving items must be retained — re-rendering them would defeat +// the memo. Entries for the items that were removed must be +// dropped so they can't serve stale output if the same pointer is +// re-introduced later. +func TestList_SetItems_PointerOverlapRetainsCache(t *testing.T) { + t.Parallel() + + a := newTrackedItem("a", "alpha", false) + b := newTrackedItem("b", "bravo", false) + c := newTrackedItem("c", "charlie", false) + d := newTrackedItem("d", "delta", false) + + l := NewList(a, b, c) + l.SetSize(40, 10) + _ = l.Render() + require.Equal(t, 1, a.renderHits) + require.Equal(t, 1, b.renderHits) + require.Equal(t, 1, c.renderHits) + + // Replace the slice with one that shares a and c (b is dropped, + // d is added). a and c must keep their cache entries; d renders + // once on the next draw. + l.SetItems(a, c, d) + _ = l.Render() + require.Equal(t, 1, a.renderHits, "stable item must keep cached entry across SetItems") + require.Equal(t, 1, c.renderHits, "stable item must keep cached entry across SetItems") + require.Equal(t, 1, d.renderHits, "new item renders once") + + // Re-introducing b after it was dropped must rebuild its + // entry (its previous cache entry was invalidated by SetItems). + l.SetItems(a, b, c) + _ = l.Render() + require.Equal(t, 2, b.renderHits, "re-introduced item must re-render — its old entry was dropped") + // a and c remained throughout both swaps. + require.Equal(t, 1, a.renderHits, "stable item retained across multiple SetItems") + require.Equal(t, 1, c.renderHits, "stable item retained across multiple SetItems") +} + +// TestList_SetItems_AllNewDropsEveryEntry covers F6 §4.5: when the +// SetItems slice has no pointer overlap with the previous slice, +// every cache entry from the previous slice is dropped. This is +// the pure-replace case (e.g. session switch). +func TestList_SetItems_AllNewDropsEveryEntry(t *testing.T) { + t.Parallel() + + a := newTrackedItem("a", "alpha", false) + b := newTrackedItem("b", "bravo", false) + c := newTrackedItem("c", "charlie", false) + + l := NewList(a, b, c) + l.SetSize(40, 10) + _ = l.Render() + require.Equal(t, 1, a.renderHits) + require.Equal(t, 1, b.renderHits) + require.Equal(t, 1, c.renderHits) + + // Replace with a fully disjoint slice. Every entry from the + // previous slice must be dropped. + x := newTrackedItem("x", "xray", false) + y := newTrackedItem("y", "yankee", false) + l.SetItems(x, y) + _ = l.Render() + require.Equal(t, 1, x.renderHits, "new item renders once") + require.Equal(t, 1, y.renderHits, "new item renders once") + + // Re-introducing the originals must rebuild every entry. + l.SetItems(a, b, c) + _ = l.Render() + require.Equal(t, 2, a.renderHits, "previously-dropped item must re-render") + require.Equal(t, 2, b.renderHits, "previously-dropped item must re-render") + require.Equal(t, 2, c.renderHits, "previously-dropped item must re-render") +} + +// TestVersioned_BumpMonotonic covers the basic Versioned contract: +// Version() starts at zero and Bump() advances it monotonically. +func TestVersioned_BumpMonotonic(t *testing.T) { + t.Parallel() + + v := NewVersioned() + require.Equal(t, uint64(0), v.Version()) + v.Bump() + require.Equal(t, uint64(1), v.Version()) + v.Bump() + v.Bump() + require.Equal(t, uint64(3), v.Version()) +} diff --git a/internal/ui/model/layout_test.go b/internal/ui/model/layout_test.go index f7db5fa07d748fab0f963854d7f54138e9ffa0cc..ea4c33f1c3ce459054b01e66a1e32f4d8cc031de 100644 --- a/internal/ui/model/layout_test.go +++ b/internal/ui/model/layout_test.go @@ -20,6 +20,8 @@ type testMessageItem struct { func (m testMessageItem) ID() string { return m.id } func (m testMessageItem) Render(int) string { return m.text } func (m testMessageItem) RawRender(int) string { return m.text } +func (m testMessageItem) Version() uint64 { return 0 } +func (m testMessageItem) Finished() bool { return true } var _ chat.MessageItem = testMessageItem{}