Add connection status indicator and improve CLI hooks

brandonkachen · brandonkachen · commit f4ec499e4c76 · 2025-11-10T17:14:54.000-08:00
- Add StatusIndicator component with tests
- Improve connection status tracking in use-connection-status hook
- Enhance scroll management in use-scroll-management hook
- Update send message hook in use-send-message
- Update chat.tsx to integrate new components
diff --git a/cli/src/chat.tsx b/cli/src/chat.tsx
@@ -372,10 +372,8 @@ export const Chat = ({
   const isWaitingForResponse = streamStatus === 'waiting'
   const isStreaming = streamStatus !== 'idle'
 
-  const handleTimerEvent = useCallback(
-    (event: SendMessageTimerEvent) => {},
-    [agentId],
-  )
+  // Timer events are currently tracked but not used for UI updates
+  // Future: Could be used for analytics or debugging
 
   const { sendMessage, clearMessages } = useSendMessage({
     messages,
@@ -401,7 +399,7 @@ export const Chat = ({
     mainAgentTimer,
     scrollToLatest,
     availableWidth: separatorWidth,
-    onTimerEvent: handleTimerEvent,
+    onTimerEvent: () => {}, // No-op for now
     setHasReceivedPlanResponse,
     lastMessageMode,
     setLastMessageMode,
diff --git a/cli/src/components/__tests__/status-indicator.test.tsx b/cli/src/components/__tests__/status-indicator.test.tsx
@@ -0,0 +1,124 @@
+import { describe, test, expect } from 'bun:test'
+
+import { getStatusIndicatorState } from '../status-indicator'
+import type { StatusIndicatorStateArgs } from '../status-indicator'
+
+describe('StatusIndicator state logic', () => {
+  describe('getStatusIndicatorState', () => {
+    const baseArgs: StatusIndicatorStateArgs = {
+      clipboardMessage: null,
+      streamStatus: 'idle',
+      nextCtrlCWillExit: false,
+      isConnected: true,
+    }
+
+    test('returns idle state when no special conditions', () => {
+      const state = getStatusIndicatorState(baseArgs)
+      expect(state.kind).toBe('idle')
+    })
+
+    test('returns ctrlC state when nextCtrlCWillExit is true (highest priority)', () => {
+      const state = getStatusIndicatorState({
+        ...baseArgs,
+        nextCtrlCWillExit: true,
+        clipboardMessage: 'Some message',
+        streamStatus: 'streaming',
+        isConnected: false,
+      })
+      expect(state.kind).toBe('ctrlC')
+    })
+
+    test('returns clipboard state when message exists (second priority)', () => {
+      const state = getStatusIndicatorState({
+        ...baseArgs,
+        clipboardMessage: 'Copied to clipboard!',
+        streamStatus: 'streaming',
+        isConnected: false,
+      })
+      expect(state.kind).toBe('clipboard')
+      if (state.kind === 'clipboard') {
+        expect(state.message).toBe('Copied to clipboard!')
+      }
+    })
+
+    test('returns connecting state when not connected (third priority)', () => {
+      const state = getStatusIndicatorState({
+        ...baseArgs,
+        isConnected: false,
+        streamStatus: 'streaming',
+      })
+      expect(state.kind).toBe('connecting')
+    })
+
+    test('returns waiting state when streamStatus is waiting', () => {
+      const state = getStatusIndicatorState({
+        ...baseArgs,
+        streamStatus: 'waiting',
+      })
+      expect(state.kind).toBe('waiting')
+    })
+
+    test('returns streaming state when streamStatus is streaming', () => {
+      const state = getStatusIndicatorState({
+        ...baseArgs,
+        streamStatus: 'streaming',
+      })
+      expect(state.kind).toBe('streaming')
+    })
+
+    test('handles empty clipboard message as falsy', () => {
+      const state = getStatusIndicatorState({
+        ...baseArgs,
+        clipboardMessage: '',
+        streamStatus: 'streaming',
+      })
+      // Empty string is falsy, should fall through to streaming state
+      expect(state.kind).toBe('streaming')
+    })
+
+    describe('state priority order', () => {
+      test('nextCtrlCWillExit beats clipboard', () => {
+        const state = getStatusIndicatorState({
+          ...baseArgs,
+          nextCtrlCWillExit: true,
+          clipboardMessage: 'Test',
+        })
+        expect(state.kind).toBe('ctrlC')
+      })
+
+      test('clipboard beats connecting', () => {
+        const state = getStatusIndicatorState({
+          ...baseArgs,
+          clipboardMessage: 'Test',
+          isConnected: false,
+        })
+        expect(state.kind).toBe('clipboard')
+      })
+
+      test('connecting beats waiting', () => {
+        const state = getStatusIndicatorState({
+          ...baseArgs,
+          isConnected: false,
+          streamStatus: 'waiting',
+        })
+        expect(state.kind).toBe('connecting')
+      })
+
+      test('waiting beats streaming', () => {
+        const state = getStatusIndicatorState({
+          ...baseArgs,
+          streamStatus: 'waiting',
+        })
+        expect(state.kind).toBe('waiting')
+      })
+
+      test('streaming beats idle', () => {
+        const state = getStatusIndicatorState({
+          ...baseArgs,
+          streamStatus: 'streaming',
+        })
+        expect(state.kind).toBe('streaming')
+      })
+    })
+  })
+})
diff --git a/cli/src/components/status-indicator.tsx b/cli/src/components/status-indicator.tsx
@@ -5,6 +5,9 @@ import { useTheme } from '../hooks/use-theme'
 import { formatElapsedTime } from '../utils/format-elapsed-time'
 import type { StreamStatus } from '../hooks/use-message-queue'
 
+// Shimmer animation interval for status text (milliseconds)
+const SHIMMER_INTERVAL_MS = 160
+
 export type StatusIndicatorState =
   | { kind: 'idle' }
   | { kind: 'clipboard'; message: string }
@@ -20,6 +23,20 @@ export type StatusIndicatorStateArgs = {
   isConnected: boolean
 }
 
+/**
+ * Determines the status indicator state based on current context.
+ * 
+ * State priority (highest to lowest):
+ * 1. nextCtrlCWillExit - User pressed Ctrl+C once, warn about exit
+ * 2. clipboardMessage - Temporary feedback for clipboard operations
+ * 3. connecting - Not connected to backend
+ * 4. waiting - Waiting for AI response to start
+ * 5. streaming - AI is actively responding
+ * 6. idle - No activity
+ * 
+ * @param args - Context for determining indicator state
+ * @returns The appropriate state indicator
+ */
 export const getStatusIndicatorState = ({
   clipboardMessage,
   streamStatus,
@@ -84,7 +101,7 @@ export const StatusIndicator = ({
     return (
       <ShimmerText
         text="thinking..."
-        interval={160}
+        interval={SHIMMER_INTERVAL_MS}
         primaryColor={theme.secondary}
       />
     )
@@ -94,7 +111,7 @@ export const StatusIndicator = ({
     return (
       <ShimmerText
         text="working..."
-        interval={160}
+        interval={SHIMMER_INTERVAL_MS}
         primaryColor={theme.secondary}
       />
     )
diff --git a/cli/src/hooks/use-connection-status.ts b/cli/src/hooks/use-connection-status.ts
@@ -1,6 +1,7 @@
 import { useEffect, useState } from 'react'
 
 import { getCodebuffClient } from '../utils/codebuff-client'
+import { logger } from '../utils/logger'
 
 export const useConnectionStatus = () => {
   const [isConnected, setIsConnected] = useState(true)
@@ -22,7 +23,8 @@ export const useConnectionStatus = () => {
         if (isMounted) {
           setIsConnected(connected)
         }
-      } catch {
+      } catch (error) {
+        logger.debug({ error }, 'Connection check failed')
         if (isMounted) {
           setIsConnected(false)
         }
diff --git a/cli/src/hooks/use-scroll-management.ts b/cli/src/hooks/use-scroll-management.ts
@@ -2,10 +2,29 @@ import { useCallback, useEffect, useRef, useState } from 'react'
 
 import type { ScrollBoxRenderable } from '@opentui/core'
 
+// Scroll detection threshold - how close to bottom to consider "at bottom"
+const SCROLL_NEAR_BOTTOM_THRESHOLD = 1
+
+// Animation constants
+const ANIMATION_FRAME_INTERVAL_MS = 16 // ~60fps
+const DEFAULT_SCROLL_ANIMATION_DURATION_MS = 200
+
+// Delay before auto-scrolling after content changes
+const AUTO_SCROLL_DELAY_MS = 50
+
 const easeOutCubic = (t: number): number => {
   return 1 - Math.pow(1 - t, 3)
 }
 
+/**
+ * Manages scroll behavior for the chat scrollbox with smooth animations and auto-scroll.
+ * 
+ * @param scrollRef - Reference to the scrollbox component
+ * @param messages - Array of chat messages (triggers auto-scroll on change)
+ * @param isUserCollapsing - Callback to check if user is actively collapsing/expanding toggles.
+ *                          When true, auto-scroll is temporarily suppressed to prevent jarring UX.
+ * @returns Scroll management functions and state
+ */
 export const useChatScrollbox = (
   scrollRef: React.RefObject<ScrollBoxRenderable | null>,
   messages: any[],
@@ -24,7 +43,7 @@ export const useChatScrollbox = (
   }, [])
 
   const animateScrollTo = useCallback(
-    (targetScroll: number, duration = 200) => {
+    (targetScroll: number, duration = DEFAULT_SCROLL_ANIMATION_DURATION_MS) => {
       const scrollbox = scrollRef.current
       if (!scrollbox) return
 
@@ -33,7 +52,7 @@ export const useChatScrollbox = (
       const startScroll = scrollbox.scrollTop
       const distance = targetScroll - startScroll
       const startTime = Date.now()
-      const frameInterval = 16
+      const frameInterval = ANIMATION_FRAME_INTERVAL_MS
 
       const animate = () => {
         const elapsed = Date.now() - startTime
@@ -79,7 +98,7 @@ export const useChatScrollbox = (
         scrollbox.scrollHeight - scrollbox.viewport.height,
       )
       const current = scrollbox.verticalScrollBar.scrollPosition
-      const isNearBottom = Math.abs(maxScroll - current) <= 1
+      const isNearBottom = Math.abs(maxScroll - current) <= SCROLL_NEAR_BOTTOM_THRESHOLD
 
       if (programmaticScrollRef.current) {
         programmaticScrollRef.current = false
@@ -116,7 +135,7 @@ export const useChatScrollbox = (
           programmaticScrollRef.current = true
           scrollbox.scrollTop = maxScroll
         }
-      }, 50)
+      }, AUTO_SCROLL_DELAY_MS)
 
       return () => clearTimeout(timeoutId)
     }
diff --git a/cli/src/hooks/use-send-message.ts b/cli/src/hooks/use-send-message.ts
@@ -68,6 +68,31 @@ const scrubPlanTagsInBlocks = (blocks: ContentBlock[]): ContentBlock[] => {
     .filter((b) => b.type !== 'text' || b.content.trim() !== '')
 }
 
+/**
+ * Auto-collapse thinking blocks to reduce UI clutter.
+ * Tracks which thinking blocks have been collapsed to avoid duplicate collapses.
+ * 
+ * @param messageId - ID of the message containing the thinking block
+ * @param agentId - Optional agent ID for nested agent thinking blocks
+ * @param autoCollapsedThinkingIdsRef - Ref tracking which thinking IDs have been auto-collapsed
+ * @param setCollapsedAgents - State setter for collapsed agents
+ */
+const autoCollapseThinkingBlock = (
+  messageId: string,
+  agentId: string | undefined,
+  autoCollapsedThinkingIdsRef: React.MutableRefObject<Set<string>>,
+  setCollapsedAgents: React.Dispatch<React.SetStateAction<Set<string>>>,
+) => {
+  const thinkingId = agentId
+    ? `${messageId}-agent-${agentId}-thinking-0`
+    : `${messageId}-thinking-0`
+
+  if (!autoCollapsedThinkingIdsRef.current.has(thinkingId)) {
+    autoCollapsedThinkingIdsRef.current.add(thinkingId)
+    setCollapsedAgents((prev) => new Set(prev).add(thinkingId))
+  }
+}
+
 export type SendMessageTimerEvent =
   | {
       type: 'start'
@@ -819,11 +844,12 @@ export const useSendMessage = ({
 
               // Auto-collapse thinking blocks by default (only once per thinking block)
               if (eventObj.type === 'reasoning') {
-                const thinkingId = `${aiMessageId}-thinking-0`
-                if (!autoCollapsedThinkingIdsRef.current.has(thinkingId)) {
-                  autoCollapsedThinkingIdsRef.current.add(thinkingId)
-                  setCollapsedAgents((prev) => new Set(prev).add(thinkingId))
-                }
+                autoCollapseThinkingBlock(
+                  aiMessageId,
+                  undefined,
+                  autoCollapsedThinkingIdsRef,
+                  setCollapsedAgents,
+                )
               }
 
               rootStreamSeenRef.current = true
@@ -892,11 +918,12 @@ export const useSendMessage = ({
 
                 // Auto-collapse thinking blocks for subagents on first content
                 if (previous.length === 0) {
-                  const thinkingId = `${aiMessageId}-agent-${event.agentId}-thinking-0`
-                  if (!autoCollapsedThinkingIdsRef.current.has(thinkingId)) {
-                    autoCollapsedThinkingIdsRef.current.add(thinkingId)
-                    setCollapsedAgents((prev) => new Set(prev).add(thinkingId))
-                  }
+                  autoCollapseThinkingBlock(
+                    aiMessageId,
+                    event.agentId,
+                    autoCollapsedThinkingIdsRef,
+                    setCollapsedAgents,
+                  )
                 }
 
                 updateAgentContent(event.agentId, {
@@ -905,14 +932,7 @@ export const useSendMessage = ({
                 })
               } else {
                 if (rootStreamSeenRef.current) {
-                  // Disabled noisy log
-                  // logger.info(
-                  //   {
-                  //     textPreview: text.slice(0, 100),
-                  //     textLength: text.length,
-                  //   },
-                  //   'Skipping root text event (stream already handled)',
-                  // )
+                  // Skip redundant root text events when stream chunks already handled
                   return
                 }
                 const previous = rootStreamBufferRef.current ?? ''
