git.iko.soy/crush
on main help sign in
fix(ui/chat): make keyboard expand work for assistant thinking blocks (#2791)

The assistant message type's toggle-expand method returned no value, so it silently failed to satisfy the expandable interface and the keyboard expand path skipped assistant thinking blocks. Expansion only worked via the mouse, which called the concrete method directly. Fixing the signature exposed a second bug: the mouse path then toggled twice per click (once inside the item, once via the generic interface path), netting no change. The item now just reports whether the click hit the thinking box; the generic path does the toggle, gated on that flag so clicks elsewhere do nothing. Adds a runtime regression test plus a compile-time interface assertion so this can't silently come back. Co-authored-by: Crush <crush@charm.land>

commitd7d25cf89fe2613851aa083267ccb0e4389f05a1 authorChristian Rocha <christian@rocha.is> dateSat 9 May 2026, 20:15 (5 months ago) parentff0e709
4 files changed+126−13
compare with main
unifiedsplit
internal/ui/chat/assistant.go+14−9 internal/ui/chat/assistant_test.go+54 internal/ui/model/chat.go+9−4 internal/ui/model/chat_expand_test.go+49
internal/ui/chat/assistant.go+14−9
@@ -35,6 +35,8 @@ type AssistantMessageItem struct {
3535 thinkingBoxHeight int // Tracks the rendered thinking box height for click detection.
3636}
3737
38+var _ Expandable = (*AssistantMessageItem)(nil)
39+
3840// NewAssistantMessageItem creates a new AssistantMessageItem.
3941func NewAssistantMessageItem(sty *styles.Styles, message *message.Message) MessageItem {
4042 a := &AssistantMessageItem{
@@ -249,23 +251,26 @@ func (a *AssistantMessageItem) SetMessage(message *message.Message) tea.Cmd {
249251 return nil
250252}
251253
252−// ToggleExpanded toggles the expanded state of the thinking box.
253−func (a *AssistantMessageItem) ToggleExpanded() {
254+// ToggleExpanded toggles the expanded state of the thinking box and returns
255+// whether the item is now expanded.
256+func (a *AssistantMessageItem) ToggleExpanded() bool {
254257 a.thinkingExpanded = !a.thinkingExpanded
255258 a.clearCache()
259+ return a.thinkingExpanded
256260}
257261
258−// HandleMouseClick implements MouseClickable.
262+// HandleMouseClick implements MouseClickable. It signals (via a true return)
263+// that the click lies on the thinking box so the caller can invoke
264+// [AssistantMessageItem.ToggleExpanded] through the generic [Expandable]
265+// path. Toggling here directly would double-toggle because the caller always
266+// runs the generic path after a handled click.
259267func (a *AssistantMessageItem) HandleMouseClick(btn ansi.MouseButton, x, y int) bool {
260268 if btn != ansi.MouseLeft {
261269 return false
262270 }
263− // check if the click is within the thinking box
264− if a.thinkingBoxHeight > 0 && y < a.thinkingBoxHeight {
265− a.ToggleExpanded()
266− return true
267− }
268− return false
271+ // Only the thinking box is clickable; other regions of the assistant
272+ // message should not trigger expansion.
273+ return a.thinkingBoxHeight > 0 && y < a.thinkingBoxHeight
269274}
270275
271276// HandleKeyEvent implements KeyEventHandler.
internal/ui/chat/assistant_test.gonew file+54
@@ -0,0 +1,54 @@
1+package chat
2+
3+import (
4+ "testing"
5+
6+ "github.com/charmbracelet/crush/internal/message"
7+ "github.com/charmbracelet/crush/internal/ui/styles"
8+ "github.com/charmbracelet/x/ansi"
9+ "github.com/stretchr/testify/require"
10+)
11+
12+// TestAssistantMessageItemExpandable guards the Expandable contract on
13+// AssistantMessageItem. The earlier implementation returned no value, which
14+// meant the type silently did not satisfy chat.Expandable and the
15+// keyboard-driven expand path in model/chat.go skipped thinking blocks.
16+func TestAssistantMessageItemExpandable(t *testing.T) {
17+ t.Parallel()
18+
19+ sty := styles.CharmtonePantera()
20+ msg := &message.Message{ID: "m1", Role: message.Assistant}
21+ item := NewAssistantMessageItem(&sty, msg)
22+
23+ exp, ok := item.(Expandable)
24+ require.True(t, ok, "AssistantMessageItem must satisfy Expandable")
25+
26+ require.True(t, exp.ToggleExpanded(), "first toggle should report expanded")
27+ require.False(t, exp.ToggleExpanded(), "second toggle should report collapsed")
28+}
29+
30+// TestAssistantMessageItemHandleMouseClick ensures HandleMouseClick does not
31+// toggle expansion on its own. The generic Expandable path in
32+// model/chat.go does the toggle; doing it here too would double-toggle and
33+// net to no change.
34+func TestAssistantMessageItemHandleMouseClick(t *testing.T) {
35+ t.Parallel()
36+
37+ sty := styles.CharmtonePantera()
38+ msg := &message.Message{ID: "m2", Role: message.Assistant}
39+ item := NewAssistantMessageItem(&sty, msg).(*AssistantMessageItem)
40+ item.thinkingBoxHeight = 5
41+
42+ // Click inside the thinking box signals handled but must not mutate
43+ // the expanded state.
44+ require.True(t, item.HandleMouseClick(ansi.MouseLeft, 0, 2))
45+ require.False(t, item.thinkingExpanded, "HandleMouseClick must not toggle expansion on its own")
46+
47+ // Click outside the thinking box is ignored entirely.
48+ require.False(t, item.HandleMouseClick(ansi.MouseLeft, 0, 10))
49+ require.False(t, item.thinkingExpanded)
50+
51+ // Non-left button is ignored.
52+ require.False(t, item.HandleMouseClick(ansi.MouseRight, 0, 2))
53+ require.False(t, item.thinkingExpanded)
54+}
internal/ui/model/chat.go+9−4
@@ -603,10 +603,15 @@ func (m *Chat) HandleDelayedClick(msg DelayedClickMsg) bool {
603603 selectedItem := m.list.SelectedItem()
604604 if clickable, ok := selectedItem.(list.MouseClickable); ok {
605605 handled := clickable.HandleMouseClick(ansi.MouseButton1, msg.X, msg.Y)
606− // Toggle expansion if applicable.
607− if expandable, ok := selectedItem.(chat.Expandable); ok {
608− if !expandable.ToggleExpanded() {
609− m.ScrollToIndex(m.list.Selected())
606+ // Toggle expansion only when the item signalled it handled the
607+ // click. Items like AssistantMessageItem only report handled when
608+ // the click is on their expandable region, so this avoids
609+ // toggling expansion for clicks outside the clickable area.
610+ if handled {
611+ if expandable, ok := selectedItem.(chat.Expandable); ok {
612+ if !expandable.ToggleExpanded() {
613+ m.ScrollToIndex(m.list.Selected())
614+ }
610615 }
611616 }
612617 if m.AtBottom() {
internal/ui/model/chat_expand_test.gonew file+49
@@ -0,0 +1,49 @@
1+package model
2+
3+import (
4+ "testing"
5+
6+ "github.com/charmbracelet/crush/internal/message"
7+ "github.com/charmbracelet/crush/internal/ui/chat"
8+ "github.com/stretchr/testify/require"
9+)
10+
11+// TestChatToggleExpandedSelectedItem_AssistantMessage is the regression test
12+// for §4.8.1: before the fix, AssistantMessageItem.ToggleExpanded returned no
13+// value so the type did not satisfy chat.Expandable, and the keyboard-driven
14+// ToggleExpandedSelectedItem path silently skipped thinking blocks. This
15+// wires a real Chat with an AssistantMessageItem, selects it, invokes
16+// ToggleExpandedSelectedItem, and asserts the thinking block actually
17+// flipped.
18+func TestChatToggleExpandedSelectedItem_AssistantMessage(t *testing.T) {
19+ t.Parallel()
20+
21+ u := newTestUI()
22+
23+ msg := &message.Message{ID: "m-assist", Role: message.Assistant}
24+ item := chat.NewAssistantMessageItem(u.com.Styles, msg)
25+
26+ // The keyboard expand path uses the generic Expandable interface;
27+ // verifying satisfaction at runtime guards the contract.
28+ exp, ok := item.(chat.Expandable)
29+ require.True(t, ok, "AssistantMessageItem must satisfy chat.Expandable")
30+
31+ u.chat.SetMessages(item)
32+ u.chat.SetSelected(0)
33+
34+ // First keyboard toggle should expand. Immediately follow with a
35+ // direct ToggleExpanded: it flips the now-expanded item back to
36+ // collapsed and returns false. If the keyboard path had silently
37+ // skipped the item (the bug), the item would still be collapsed and
38+ // the direct toggle would return true.
39+ u.chat.ToggleExpandedSelectedItem()
40+ require.False(t, exp.ToggleExpanded(),
41+ "keyboard toggle did not expand the assistant thinking block")
42+
43+ // Second keyboard toggle against the re-collapsed item should expand
44+ // it again. Direct ToggleExpanded then returns false because it
45+ // re-collapses.
46+ u.chat.ToggleExpandedSelectedItem()
47+ require.False(t, exp.ToggleExpanded(),
48+ "second keyboard toggle did not re-expand the assistant thinking block")
49+}