Detailed changes
@@ -35,6 +35,8 @@ type AssistantMessageItem struct {
thinkingBoxHeight int // Tracks the rendered thinking box height for click detection.
}
+var _ Expandable = (*AssistantMessageItem)(nil)
+
// NewAssistantMessageItem creates a new AssistantMessageItem.
func NewAssistantMessageItem(sty *styles.Styles, message *message.Message) MessageItem {
a := &AssistantMessageItem{
@@ -249,23 +251,26 @@ func (a *AssistantMessageItem) SetMessage(message *message.Message) tea.Cmd {
return nil
}
-// ToggleExpanded toggles the expanded state of the thinking box.
-func (a *AssistantMessageItem) ToggleExpanded() {
+// ToggleExpanded toggles the expanded state of the thinking box and returns
+// whether the item is now expanded.
+func (a *AssistantMessageItem) ToggleExpanded() bool {
a.thinkingExpanded = !a.thinkingExpanded
a.clearCache()
+ return a.thinkingExpanded
}
-// HandleMouseClick implements MouseClickable.
+// HandleMouseClick implements MouseClickable. It signals (via a true return)
+// that the click lies on the thinking box so the caller can invoke
+// [AssistantMessageItem.ToggleExpanded] through the generic [Expandable]
+// path. Toggling here directly would double-toggle because the caller always
+// runs the generic path after a handled click.
func (a *AssistantMessageItem) HandleMouseClick(btn ansi.MouseButton, x, y int) bool {
if btn != ansi.MouseLeft {
return false
}
- // check if the click is within the thinking box
- if a.thinkingBoxHeight > 0 && y < a.thinkingBoxHeight {
- a.ToggleExpanded()
- return true
- }
- return false
+ // Only the thinking box is clickable; other regions of the assistant
+ // message should not trigger expansion.
+ return a.thinkingBoxHeight > 0 && y < a.thinkingBoxHeight
}
// HandleKeyEvent implements KeyEventHandler.
@@ -0,0 +1,54 @@
+package chat
+
+import (
+ "testing"
+
+ "github.com/charmbracelet/crush/internal/message"
+ "github.com/charmbracelet/crush/internal/ui/styles"
+ "github.com/charmbracelet/x/ansi"
+ "github.com/stretchr/testify/require"
+)
+
+// TestAssistantMessageItemExpandable guards the Expandable contract on
+// AssistantMessageItem. The earlier implementation returned no value, which
+// meant the type silently did not satisfy chat.Expandable and the
+// keyboard-driven expand path in model/chat.go skipped thinking blocks.
+func TestAssistantMessageItemExpandable(t *testing.T) {
+ t.Parallel()
+
+ sty := styles.CharmtonePantera()
+ msg := &message.Message{ID: "m1", Role: message.Assistant}
+ item := NewAssistantMessageItem(&sty, msg)
+
+ exp, ok := item.(Expandable)
+ require.True(t, ok, "AssistantMessageItem must satisfy Expandable")
+
+ require.True(t, exp.ToggleExpanded(), "first toggle should report expanded")
+ require.False(t, exp.ToggleExpanded(), "second toggle should report collapsed")
+}
+
+// TestAssistantMessageItemHandleMouseClick ensures HandleMouseClick does not
+// toggle expansion on its own. The generic Expandable path in
+// model/chat.go does the toggle; doing it here too would double-toggle and
+// net to no change.
+func TestAssistantMessageItemHandleMouseClick(t *testing.T) {
+ t.Parallel()
+
+ sty := styles.CharmtonePantera()
+ msg := &message.Message{ID: "m2", Role: message.Assistant}
+ item := NewAssistantMessageItem(&sty, msg).(*AssistantMessageItem)
+ item.thinkingBoxHeight = 5
+
+ // Click inside the thinking box signals handled but must not mutate
+ // the expanded state.
+ require.True(t, item.HandleMouseClick(ansi.MouseLeft, 0, 2))
+ require.False(t, item.thinkingExpanded, "HandleMouseClick must not toggle expansion on its own")
+
+ // Click outside the thinking box is ignored entirely.
+ require.False(t, item.HandleMouseClick(ansi.MouseLeft, 0, 10))
+ require.False(t, item.thinkingExpanded)
+
+ // Non-left button is ignored.
+ require.False(t, item.HandleMouseClick(ansi.MouseRight, 0, 2))
+ require.False(t, item.thinkingExpanded)
+}
@@ -603,10 +603,15 @@ func (m *Chat) HandleDelayedClick(msg DelayedClickMsg) bool {
selectedItem := m.list.SelectedItem()
if clickable, ok := selectedItem.(list.MouseClickable); ok {
handled := clickable.HandleMouseClick(ansi.MouseButton1, msg.X, msg.Y)
- // Toggle expansion if applicable.
- if expandable, ok := selectedItem.(chat.Expandable); ok {
- if !expandable.ToggleExpanded() {
- m.ScrollToIndex(m.list.Selected())
+ // Toggle expansion only when the item signalled it handled the
+ // click. Items like AssistantMessageItem only report handled when
+ // the click is on their expandable region, so this avoids
+ // toggling expansion for clicks outside the clickable area.
+ if handled {
+ if expandable, ok := selectedItem.(chat.Expandable); ok {
+ if !expandable.ToggleExpanded() {
+ m.ScrollToIndex(m.list.Selected())
+ }
}
}
if m.AtBottom() {
@@ -0,0 +1,49 @@
+package model
+
+import (
+ "testing"
+
+ "github.com/charmbracelet/crush/internal/message"
+ "github.com/charmbracelet/crush/internal/ui/chat"
+ "github.com/stretchr/testify/require"
+)
+
+// TestChatToggleExpandedSelectedItem_AssistantMessage is the regression test
+// for §4.8.1: before the fix, AssistantMessageItem.ToggleExpanded returned no
+// value so the type did not satisfy chat.Expandable, and the keyboard-driven
+// ToggleExpandedSelectedItem path silently skipped thinking blocks. This
+// wires a real Chat with an AssistantMessageItem, selects it, invokes
+// ToggleExpandedSelectedItem, and asserts the thinking block actually
+// flipped.
+func TestChatToggleExpandedSelectedItem_AssistantMessage(t *testing.T) {
+ t.Parallel()
+
+ u := newTestUI()
+
+ msg := &message.Message{ID: "m-assist", Role: message.Assistant}
+ item := chat.NewAssistantMessageItem(u.com.Styles, msg)
+
+ // The keyboard expand path uses the generic Expandable interface;
+ // verifying satisfaction at runtime guards the contract.
+ exp, ok := item.(chat.Expandable)
+ require.True(t, ok, "AssistantMessageItem must satisfy chat.Expandable")
+
+ u.chat.SetMessages(item)
+ u.chat.SetSelected(0)
+
+ // First keyboard toggle should expand. Immediately follow with a
+ // direct ToggleExpanded: it flips the now-expanded item back to
+ // collapsed and returns false. If the keyboard path had silently
+ // skipped the item (the bug), the item would still be collapsed and
+ // the direct toggle would return true.
+ u.chat.ToggleExpandedSelectedItem()
+ require.False(t, exp.ToggleExpanded(),
+ "keyboard toggle did not expand the assistant thinking block")
+
+ // Second keyboard toggle against the re-collapsed item should expand
+ // it again. Direct ToggleExpanded then returns false because it
+ // re-collapses.
+ u.chat.ToggleExpandedSelectedItem()
+ require.False(t, exp.ToggleExpanded(),
+ "second keyboard toggle did not re-expand the assistant thinking block")
+}