Skip to content

Commit e4725e4

Browse files
brandonkachenbrandonkachen
committed
refactor: consolidate stream state into single StreamStatus enum
Replace dual boolean flags (isWaitingForResponse, isStreaming) with a single StreamStatus enum ('idle' | 'waiting' | 'streaming') to make state transitions explicit and prevent invalid states. Changes: - Add StreamStatus type in use-message-queue - Update status indicator to use streamStatus directly - Remove duplicate useHasStatus hook logic - Simplify state transitions in use-send-message - Move queue preview to appear after status text on left side - Keep elapsed time display on right side throughout interaction Benefits: - Impossible to have inconsistent state combinations - Clearer state machine with explicit transitions - Single source of truth for status rendering logic - Easier to extend with new states in future
1 parent 7e0ada9 commit e4725e4

File tree

5 files changed

+226
-155
lines changed

5 files changed

+226
-155
lines changed

‎cli/src/chat.tsx‎

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ import {
99
MultilineInput,
1010
type MultilineInputHandle,
1111
} from './components/multiline-input'
12-
import { StatusIndicator, useHasStatus } from './components/status-indicator'
12+
import {
13+
StatusIndicator,
14+
StatusElapsedTime,
15+
} from './components/status-indicator'
1316
import { SuggestionMenu } from './components/suggestion-menu'
1417
import { SLASH_COMMANDS } from './data/slash-commands'
1518
import { useAgentValidation } from './hooks/use-agent-validation'
@@ -352,22 +355,24 @@ export const Chat = ({
352355

353356
const {
354357
queuedMessages,
355-
isStreaming,
356-
isWaitingForResponse,
358+
streamStatus,
357359
streamMessageIdRef,
358360
addToQueue,
359361
startStreaming,
360362
stopStreaming,
361-
setIsWaitingForResponse,
363+
setStreamStatus,
362364
setCanProcessQueue,
363-
setIsStreaming,
364365
} = useMessageQueue(
365366
(content: string) =>
366367
sendMessageRef.current?.({ content, agentMode }) ?? Promise.resolve(),
367368
isChainInProgressRef,
368369
activeAgentStreamsRef,
369370
)
370371

372+
// Derive boolean flags from streamStatus for convenience
373+
const isWaitingForResponse = streamStatus === 'waiting'
374+
const isStreaming = streamStatus !== 'idle'
375+
371376
const handleTimerEvent = useCallback(
372377
(event: SendMessageTimerEvent) => {
373378
const payload = {
@@ -406,10 +411,9 @@ export const Chat = ({
406411
isChainInProgressRef,
407412
setActiveSubagents,
408413
setIsChainInProgress,
409-
setIsWaitingForResponse,
414+
setStreamStatus,
410415
startStreaming,
411416
stopStreaming,
412-
setIsStreaming,
413417
setCanProcessQueue,
414418
abortControllerRef,
415419
agentId,
@@ -435,15 +439,6 @@ export const Chat = ({
435439
sendMessageRef,
436440
})
437441

438-
// Status is active when waiting for response or streaming
439-
const isStatusActive = isWaitingForResponse || isStreaming
440-
const hasStatus = useHasStatus({
441-
isActive: isStatusActive,
442-
clipboardMessage,
443-
timerStartTime,
444-
nextCtrlCWillExit,
445-
})
446-
447442
const handleSubmit = useCallback(
448443
() =>
449444
routeUserPrompt({
@@ -541,18 +536,25 @@ export const Chat = ({
541536

542537
const shouldShowQueuePreview = queuedMessages.length > 0
543538
const shouldShowStatusLine =
544-
hasStatus || shouldShowQueuePreview || !isAtBottom
539+
streamStatus !== 'idle' ||
540+
shouldShowQueuePreview ||
541+
!isAtBottom ||
542+
clipboardMessage != null ||
543+
nextCtrlCWillExit
545544

546545
const statusIndicatorNode = (
547546
<StatusIndicator
548547
clipboardMessage={clipboardMessage}
549-
isActive={isStatusActive}
550-
isWaitingForResponse={isWaitingForResponse}
548+
streamStatus={streamStatus}
551549
timerStartTime={timerStartTime}
552550
nextCtrlCWillExit={nextCtrlCWillExit}
553551
/>
554552
)
555553

554+
const elapsedTimeNode = (
555+
<StatusElapsedTime streamStatus={streamStatus} timerStartTime={timerStartTime} />
556+
)
557+
556558
const validationBanner = useValidationBanner({
557559
liveValidationErrors: validationErrors,
558560
loadedAgentsData,
@@ -652,8 +654,17 @@ export const Chat = ({
652654
width: '100%',
653655
}}
654656
>
655-
{/* Left section - queue preview */}
656-
<box style={{ flexGrow: 1, flexShrink: 1, flexBasis: 0 }}>
657+
{/* Left section */}
658+
<box
659+
style={{
660+
flexGrow: 1,
661+
flexShrink: 1,
662+
flexBasis: 0,
663+
flexDirection: 'row',
664+
gap: 2,
665+
}}
666+
>
667+
<text style={{ wrapMode: 'none' }}>{statusIndicatorNode}</text>
657668
{shouldShowQueuePreview && (
658669
<text style={{ wrapMode: 'none' }}>
659670
<span fg={theme.secondary} bg={theme.inputFocusedBg}>
@@ -688,7 +699,7 @@ export const Chat = ({
688699
)}
689700
</box>
690701

691-
{/* Right section - status indicator */}
702+
{/* Right section */}
692703
<box
693704
style={{
694705
flexGrow: 1,
@@ -698,9 +709,7 @@ export const Chat = ({
698709
justifyContent: 'flex-end',
699710
}}
700711
>
701-
{hasStatus && (
702-
<text style={{ wrapMode: 'none' }}>{statusIndicatorNode}</text>
703-
)}
712+
<text style={{ wrapMode: 'none' }}>{elapsedTimeNode}</text>
704713
</box>
705714
</box>
706715
)}

‎cli/src/components/__tests__/status-indicator.timer.test.tsx‎

Lines changed: 115 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,15 @@ import {
99
} from 'bun:test'
1010
import React from 'react'
1111

12-
import { StatusIndicator } from '../status-indicator'
12+
import { StatusIndicator, StatusElapsedTime } from '../status-indicator'
1313

1414
import '../../state/theme-store' // Initialize theme store
1515
import { renderToStaticMarkup } from 'react-dom/server'
1616

1717
import * as codebuffClient from '../../utils/codebuff-client'
1818

1919

20-
describe('StatusIndicator timer rendering', () => {
20+
describe('StatusIndicator state transitions', () => {
2121
let getClientSpy: ReturnType<typeof spyOn>
2222

2323
beforeEach(() => {
@@ -30,46 +30,120 @@ describe('StatusIndicator timer rendering', () => {
3030
getClientSpy.mockRestore()
3131
})
3232

33-
test('shows elapsed seconds when waiting for response', () => {
34-
const now = Date.now()
35-
const markup = renderToStaticMarkup(
36-
<StatusIndicator
37-
clipboardMessage={null}
38-
isActive={true}
39-
isWaitingForResponse={true}
40-
timerStartTime={now - 5000}
41-
nextCtrlCWillExit={false}
42-
/>,
43-
)
44-
45-
expect(markup).toContain('thinking...')
46-
47-
const inactiveMarkup = renderToStaticMarkup(
48-
<StatusIndicator
49-
clipboardMessage={null}
50-
isActive={false}
51-
isWaitingForResponse={false}
52-
timerStartTime={null}
53-
nextCtrlCWillExit={false}
54-
/>,
55-
)
56-
57-
expect(inactiveMarkup).toBe('')
33+
describe('StatusIndicator text states', () => {
34+
test('shows "thinking..." when waiting for first response (streamStatus = waiting)', () => {
35+
const now = Date.now()
36+
const markup = renderToStaticMarkup(
37+
<StatusIndicator
38+
clipboardMessage={null}
39+
streamStatus="waiting"
40+
timerStartTime={now - 5000}
41+
nextCtrlCWillExit={false}
42+
/>,
43+
)
44+
45+
// ShimmerText renders individual characters in spans
46+
expect(markup).toContain('t')
47+
expect(markup).toContain('h')
48+
expect(markup).toContain('i')
49+
expect(markup).toContain('n')
50+
expect(markup).toContain('k')
51+
expect(markup).not.toContain('w') // not "working"
52+
})
53+
54+
test('shows "working..." when streaming content (streamStatus = streaming)', () => {
55+
const now = Date.now()
56+
const markup = renderToStaticMarkup(
57+
<StatusIndicator
58+
clipboardMessage={null}
59+
streamStatus="streaming"
60+
timerStartTime={now - 5000}
61+
nextCtrlCWillExit={false}
62+
/>,
63+
)
64+
65+
// ShimmerText renders individual characters in spans
66+
expect(markup).toContain('w')
67+
expect(markup).toContain('o')
68+
expect(markup).toContain('r')
69+
expect(markup).toContain('k')
70+
})
71+
72+
test('shows nothing when inactive (streamStatus = idle)', () => {
73+
const markup = renderToStaticMarkup(
74+
<StatusIndicator
75+
clipboardMessage={null}
76+
streamStatus="idle"
77+
timerStartTime={null}
78+
nextCtrlCWillExit={false}
79+
/>,
80+
)
81+
82+
expect(markup).toBe('')
83+
})
5884
})
5985

60-
test('clipboard message takes priority over timer output', () => {
61-
const now = Date.now()
62-
const markup = renderToStaticMarkup(
63-
<StatusIndicator
64-
clipboardMessage="Copied!"
65-
isActive={true}
66-
isWaitingForResponse={true}
67-
timerStartTime={now - 12000}
68-
nextCtrlCWillExit={false}
69-
/>,
70-
)
71-
72-
expect(markup).toContain('Copied!')
73-
expect(markup).not.toContain('12s')
86+
describe('Priority states', () => {
87+
test('nextCtrlCWillExit takes highest priority', () => {
88+
const now = Date.now()
89+
const markup = renderToStaticMarkup(
90+
<StatusIndicator
91+
clipboardMessage="Copied!"
92+
streamStatus="waiting"
93+
timerStartTime={now - 5000}
94+
nextCtrlCWillExit={true}
95+
/>,
96+
)
97+
98+
expect(markup).toContain('Press Ctrl-C again to exit')
99+
expect(markup).not.toContain('Copied!')
100+
expect(markup).not.toContain('thinking')
101+
expect(markup).not.toContain('working')
102+
})
103+
104+
test('clipboard message takes priority over streaming states', () => {
105+
const now = Date.now()
106+
const markup = renderToStaticMarkup(
107+
<StatusIndicator
108+
clipboardMessage="Copied!"
109+
streamStatus="waiting"
110+
timerStartTime={now - 12000}
111+
nextCtrlCWillExit={false}
112+
/>,
113+
)
114+
115+
expect(markup).toContain('Copied!')
116+
// Shimmer text would contain individual characters, but clipboard message doesn't
117+
})
118+
})
119+
120+
describe('StatusElapsedTime', () => {
121+
test('shows nothing initially (useEffect not triggered in static render)', () => {
122+
const now = Date.now()
123+
const markup = renderToStaticMarkup(
124+
<StatusElapsedTime streamStatus="streaming" timerStartTime={now - 5000} />,
125+
)
126+
127+
// Static rendering doesn't trigger useEffect, so elapsed time starts at 0
128+
// In real usage, useEffect updates the elapsed time after mount
129+
expect(markup).toBe('')
130+
})
131+
132+
test('shows nothing when inactive', () => {
133+
const now = Date.now()
134+
const markup = renderToStaticMarkup(
135+
<StatusElapsedTime streamStatus="idle" timerStartTime={now - 5000} />,
136+
)
137+
138+
expect(markup).toBe('')
139+
})
140+
141+
test('shows nothing when timerStartTime is null', () => {
142+
const markup = renderToStaticMarkup(
143+
<StatusElapsedTime streamStatus="streaming" timerStartTime={null} />,
144+
)
145+
146+
expect(markup).toBe('')
147+
})
74148
})
75149
})

0 commit comments

Comments
 (0)