From e93b2b0951c94657daacfa18eb082246fb7682ce Mon Sep 17 00:00:00 2001 From: Thomas Brugman Date: Sat, 22 Nov 2025 12:47:49 +0000 Subject: [PATCH 1/2] Add ThinkingSpinner component and integrate into StatusIndicator --- cli/src/ui/components/StatusIndicator.tsx | 3 +- cli/src/ui/components/ThinkingSpinner.tsx | 32 +++++++ .../__tests__/StatusIndicator.test.tsx | 12 ++- .../__tests__/ThinkingSpinner.test.tsx | 88 +++++++++++++++++++ 4 files changed, 133 insertions(+), 2 deletions(-) create mode 100644 cli/src/ui/components/ThinkingSpinner.tsx create mode 100644 cli/src/ui/components/__tests__/ThinkingSpinner.test.tsx diff --git a/cli/src/ui/components/StatusIndicator.tsx b/cli/src/ui/components/StatusIndicator.tsx index e3dd38f0906..46b4aae4b8b 100644 --- a/cli/src/ui/components/StatusIndicator.tsx +++ b/cli/src/ui/components/StatusIndicator.tsx @@ -8,6 +8,7 @@ import { Box, Text } from "ink" import { useHotkeys } from "../../state/hooks/useHotkeys.js" import { useTheme } from "../../state/hooks/useTheme.js" import { HotkeyBadge } from "./HotkeyBadge.js" +import { ThinkingSpinner } from "./ThinkingSpinner.js" import { useAtomValue } from "jotai" import { isStreamingAtom } from "../../state/atoms/ui.js" import { hasResumeTaskAtom } from "../../state/atoms/extension.js" @@ -43,7 +44,7 @@ export const StatusIndicator: React.FC = ({ disabled = fal {/* Status text on the left */} - {isStreaming && Thinking...} + {isStreaming && } {hasResumeTask && Task ready to resume} diff --git a/cli/src/ui/components/ThinkingSpinner.tsx b/cli/src/ui/components/ThinkingSpinner.tsx new file mode 100644 index 00000000000..23021614e8a --- /dev/null +++ b/cli/src/ui/components/ThinkingSpinner.tsx @@ -0,0 +1,32 @@ +/** + * ThinkingSpinner - Animated spinner for the thinking state + * Shows an animated loading spinner with "Thinking..." text + */ + +import React, { useEffect, useState } from "react" +import { Text } from "ink" + +interface ThinkingSpinnerProps { + color?: string +} + +const SPINNER_FRAMES = ["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"] +const ANIMATION_INTERVAL = 80 // ms per frame + +/** + * Displays an animated spinner with "Thinking..." text + * Uses Braille Unicode characters for smooth animation + */ +export const ThinkingSpinner: React.FC = ({ color = "gray" }) => { + const [frameIndex, setFrameIndex] = useState(0) + + useEffect(() => { + const interval = setInterval(() => { + setFrameIndex((prev) => (prev + 1) % SPINNER_FRAMES.length) + }, ANIMATION_INTERVAL) + + return () => clearInterval(interval) + }, []) + + return {SPINNER_FRAMES[frameIndex]} Thinking... +} diff --git a/cli/src/ui/components/__tests__/StatusIndicator.test.tsx b/cli/src/ui/components/__tests__/StatusIndicator.test.tsx index 3d0a31d856c..f8f46dc2e85 100644 --- a/cli/src/ui/components/__tests__/StatusIndicator.test.tsx +++ b/cli/src/ui/components/__tests__/StatusIndicator.test.tsx @@ -4,7 +4,7 @@ import React from "react" import { render } from "ink-testing-library" -import { describe, it, expect, vi, beforeEach } from "vitest" +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest" import { Provider as JotaiProvider } from "jotai" import { createStore } from "jotai" import { StatusIndicator } from "../StatusIndicator.js" @@ -20,11 +20,20 @@ vi.mock("../../../state/hooks/useWebviewMessage.js", () => ({ }), })) +// Mock timers for ThinkingSpinner animation +vi.useFakeTimers() + describe("StatusIndicator", () => { let store: ReturnType beforeEach(() => { store = createStore() + // Reset animation frame for each test + vi.clearAllTimers() + }) + + afterEach(() => { + vi.clearAllTimers() }) it("should not render when disabled", () => { @@ -55,6 +64,7 @@ describe("StatusIndicator", () => { ) const output = lastFrame() + // ThinkingSpinner contains animated frame plus "Thinking..." text expect(output).toContain("Thinking...") expect(output).toContain("to cancel") // Should show either Ctrl+X or Cmd+X depending on platform diff --git a/cli/src/ui/components/__tests__/ThinkingSpinner.test.tsx b/cli/src/ui/components/__tests__/ThinkingSpinner.test.tsx new file mode 100644 index 00000000000..043ca1965bd --- /dev/null +++ b/cli/src/ui/components/__tests__/ThinkingSpinner.test.tsx @@ -0,0 +1,88 @@ +/** + * Tests for ThinkingSpinner component + */ + +import React from "react" +import { render } from "ink-testing-library" +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest" +import { ThinkingSpinner } from "../ThinkingSpinner.js" + +// Mock timers for animation +vi.useFakeTimers() + +const SPINNER_FRAMES = ["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"] + +describe("ThinkingSpinner", () => { + beforeEach(() => { + vi.clearAllTimers() + }) + + afterEach(() => { + vi.clearAllTimers() + }) + + it("should render with default gray color", () => { + const { lastFrame } = render() + const output = lastFrame() + expect(output).toContain("Thinking...") + }) + + it("should render with custom color", () => { + const { lastFrame } = render() + const output = lastFrame() + expect(output).toContain("Thinking...") + }) + + it("should display initial spinner frame", () => { + const { lastFrame } = render() + const output = lastFrame() + // Should contain the first frame (⠋) and "Thinking..." + expect(output).toContain(SPINNER_FRAMES[0]) + expect(output).toContain("Thinking...") + }) + + it("should animate through frames on interval", () => { + const { lastFrame, rerender } = render() + + let output = lastFrame() + expect(output).toContain(SPINNER_FRAMES[0]) + + // Advance animation by one frame (80ms) + vi.advanceTimersByTime(80) + rerender() + output = lastFrame() + expect(output).toContain(SPINNER_FRAMES[1]) + + // Advance animation by another frame + vi.advanceTimersByTime(80) + rerender() + output = lastFrame() + expect(output).toContain(SPINNER_FRAMES[2]) + }) + + it("should cycle through all frames and restart", () => { + const { lastFrame, rerender } = render() + + // Cycle through all frames + for (let i = 0; i < SPINNER_FRAMES.length; i++) { + const output = lastFrame() + expect(output).toContain(SPINNER_FRAMES[i]) + vi.advanceTimersByTime(80) + rerender() + } + + // After cycling through all frames, should be back at the start + const output = lastFrame() + expect(output).toContain(SPINNER_FRAMES[0]) + }) + + it("should clean up interval on unmount", () => { + const { unmount } = render() + const clearIntervalSpy = vi.spyOn(global, "clearInterval") + + unmount() + + expect(clearIntervalSpy).toHaveBeenCalled() + clearIntervalSpy.mockRestore() + }) +}) From 5cca57d4e76e50cf65a5fa8c3193ad4ec8a1cbc7 Mon Sep 17 00:00:00 2001 From: Thomas Date: Sat, 22 Nov 2025 17:55:37 +0000 Subject: [PATCH 2/2] Adress review comment --- cli/src/services/__tests__/fileSearch.test.ts | 4 +- cli/src/state/atoms/extension.ts | 18 +++++++ cli/src/state/atoms/ui.ts | 50 +++++++------------ .../useIsProcessingSubscription.test.tsx | 25 +++++++++- cli/src/types/messages.ts | 1 + .../__tests__/StatusIndicator.test.tsx | 6 ++- src/core/task/Task.ts | 14 ++++++ src/core/webview/ClineProvider.ts | 1 + src/shared/ExtensionMessage.ts | 1 + 9 files changed, 85 insertions(+), 35 deletions(-) diff --git a/cli/src/services/__tests__/fileSearch.test.ts b/cli/src/services/__tests__/fileSearch.test.ts index ea113c703aa..1d8e5943500 100644 --- a/cli/src/services/__tests__/fileSearch.test.ts +++ b/cli/src/services/__tests__/fileSearch.test.ts @@ -90,7 +90,7 @@ describe("FileSearchService", () => { const results2 = await fileSearchService.getAllFiles(cwd) expect(results1).toBe(results2) // Now cached again - }) + }, 10000) it("should clear cache for specific workspace", async () => { const cwd1 = process.cwd() @@ -106,6 +106,6 @@ describe("FileSearchService", () => { // cwd1 should be re-fetched, cwd2 should be cached const newResults1 = await fileSearchService.getAllFiles(cwd1) expect(newResults1).not.toBe(results1) - }) + }, 10000) }) }) diff --git a/cli/src/state/atoms/extension.ts b/cli/src/state/atoms/extension.ts index 45bb8202400..ba3e381aa8f 100644 --- a/cli/src/state/atoms/extension.ts +++ b/cli/src/state/atoms/extension.ts @@ -152,6 +152,13 @@ export const hasActiveTaskAtom = atom((get) => { */ export const taskResumedViaContinueAtom = atom(false) +/** + * Atom to track the timestamp of the last API stream activity + * Updated whenever a chunk arrives from the API stream (text, reasoning, usage, etc.) + * Used to detect if the model has stopped sending tokens + */ +export const lastActivityTimestampAtom = atom(0) + /** * Derived atom to check if there's a resume_task ask pending * This checks if the last message is a resume_task or resume_completed_task @@ -243,6 +250,9 @@ export const updateExtensionStateAtom = atom(null, (get, set, state: ExtensionSt set(customModesAtom, state.customModes || []) set(mcpServersAtom, state.mcpServers || []) set(cwdAtom, state.cwd || null) + if (state.lastActivityTimestamp) { + set(lastActivityTimestampAtom, state.lastActivityTimestamp) + } } else { // Clear all derived atoms set(chatMessagesAtom, []) @@ -459,6 +469,14 @@ export const clearExtensionStateAtom = atom(null, (get, set) => { set(streamingMessagesSetAtom, new Set()) }) +/** + * Action atom to update the last API stream activity timestamp + * Called whenever a chunk arrives from the API stream + */ +export const updateLastActivityTimestampAtom = atom(null, (_get, set) => { + set(lastActivityTimestampAtom, Date.now()) +}) + /** * Helper function to calculate message content length for versioning * Used to determine which version of a message is newer diff --git a/cli/src/state/atoms/ui.ts b/cli/src/state/atoms/ui.ts index 59cd4dc734c..deb002ba071 100644 --- a/cli/src/state/atoms/ui.ts +++ b/cli/src/state/atoms/ui.ts @@ -13,7 +13,7 @@ import type { FileMentionSuggestion, FileMentionContext, } from "../../services/autocomplete.js" -import { chatMessagesAtom } from "./extension.js" +import { chatMessagesAtom, lastActivityTimestampAtom } from "./extension.js" import { splitMessages } from "../../ui/messages/utils/messageCompletion.js" import { textBufferStringAtom, textBufferCursorAtom, setTextAtom, clearTextAtom } from "./textBuffer.js" import { commitCompletionTimeout } from "../../parallel/parallel.js" @@ -66,21 +66,31 @@ export const isCommittingParallelModeAtom = atom(false) */ export const commitCountdownSecondsAtom = atomWithReset(commitCompletionTimeout / 1000) +/** + * Timeout (in ms) to consider the stream inactive if no chunks received + * This detects when the model has stopped sending tokens + */ +const STREAMING_ACTIVITY_TIMEOUT_MS = 3000 // 3 seconds + /** * Derived atom to check if the extension is currently streaming/processing - * This mimics the webview's isStreaming logic from ChatView.tsx (lines 550-592) + * Uses real API activity tracking instead of state inference * * Returns true when: - * - The last message is partial (still being streamed) - * - There's an active API request that hasn't finished yet (no cost field) + * - Chunks are actively arriving from the API stream (within STREAMING_ACTIVITY_TIMEOUT_MS) + * - This is determined by lastActivityTimestamp being recent * * Returns false when: + * - No API activity for more than STREAMING_ACTIVITY_TIMEOUT_MS * - There's a tool currently asking for approval (waiting for user input) * - No messages exist - * - All messages are complete + * + * This provides accurate detection of actual model activity rather than inferring + * from partial messages and api_req_started status. */ export const isStreamingAtom = atom((get) => { const messages = get(chatMessagesAtom) + const lastActivityTimestamp = get(lastActivityTimestampAtom) if (messages.length === 0) { return false @@ -100,32 +110,10 @@ export const isStreamingAtom = atom((get) => { return false } - // Check if the last message is partial (still streaming) - if (lastMessage.partial === true) { - return true - } - - // Check if there's an active API request without a cost (not finished) - // Find the last api_req_started message - for (let i = messages.length - 1; i >= 0; i--) { - const msg = messages[i] - if (msg?.say === "api_req_started") { - try { - const data = JSON.parse(msg.text || "{}") - // If cost is undefined, the API request hasn't finished yet - if (data.cost === undefined) { - return true - } - } catch { - // If we can't parse, assume not streaming - return false - } - // Found an api_req_started with cost, so it's finished - break - } - } - - return false + // Check if there's recent API activity + // If we've received a chunk within the timeout period, we're streaming + const timeSinceLastActivity = Date.now() - lastActivityTimestamp + return timeSinceLastActivity < STREAMING_ACTIVITY_TIMEOUT_MS }) // ============================================================================ diff --git a/cli/src/state/hooks/__tests__/useIsProcessingSubscription.test.tsx b/cli/src/state/hooks/__tests__/useIsProcessingSubscription.test.tsx index 0ceae6e2009..554d695aaaf 100644 --- a/cli/src/state/hooks/__tests__/useIsProcessingSubscription.test.tsx +++ b/cli/src/state/hooks/__tests__/useIsProcessingSubscription.test.tsx @@ -6,7 +6,7 @@ import { describe, it, expect, beforeEach } from "vitest" import { createStore } from "jotai" import { isStreamingAtom } from "../../atoms/ui.js" -import { chatMessagesAtom, updateChatMessagesAtom } from "../../atoms/extension.js" +import { chatMessagesAtom, updateChatMessagesAtom, lastActivityTimestampAtom } from "../../atoms/extension.js" import type { ExtensionChatMessage } from "../../../types/messages.js" describe("isStreamingAtom Logic", () => { @@ -14,6 +14,8 @@ describe("isStreamingAtom Logic", () => { beforeEach(() => { store = createStore() + // Default: no activity (timestamp = 0) so streaming detection depends on explicit setup per test + store.set(lastActivityTimestampAtom, 0) }) describe("isStreaming state management", () => { @@ -22,6 +24,8 @@ describe("isStreamingAtom Logic", () => { }) it("should be true when last message is partial", () => { + // Set recent activity for streaming detection + store.set(lastActivityTimestampAtom, Date.now()) const partialMessage: ExtensionChatMessage = { ts: Date.now(), type: "say", @@ -34,6 +38,7 @@ describe("isStreamingAtom Logic", () => { }) it("should be false when last message is complete", () => { + // No recent activity = not streaming const completeMessage: ExtensionChatMessage = { ts: Date.now(), type: "say", @@ -46,6 +51,8 @@ describe("isStreamingAtom Logic", () => { }) it("should be false when tool is asking for approval", () => { + // Even with recent activity, tool ask blocks streaming + store.set(lastActivityTimestampAtom, Date.now()) const toolMessage: ExtensionChatMessage = { ts: Date.now(), type: "ask", @@ -57,6 +64,8 @@ describe("isStreamingAtom Logic", () => { }) it("should be true when API request hasn't finished (no cost)", () => { + // Set recent activity for streaming detection + store.set(lastActivityTimestampAtom, Date.now()) const apiReqMessage: ExtensionChatMessage = { ts: Date.now(), type: "say", @@ -68,6 +77,7 @@ describe("isStreamingAtom Logic", () => { }) it("should be false when API request has finished (has cost)", () => { + // No recent activity = not streaming const apiReqMessage: ExtensionChatMessage = { ts: Date.now(), type: "say", @@ -81,6 +91,7 @@ describe("isStreamingAtom Logic", () => { describe("Message handling scenarios", () => { it("should not be streaming for completion_result ask message", () => { + // No recent activity = not streaming const completionMessage: ExtensionChatMessage = { ts: Date.now(), type: "ask", @@ -93,6 +104,7 @@ describe("isStreamingAtom Logic", () => { }) it("should not be streaming for followup ask message", () => { + // No recent activity = not streaming const followupMessage: ExtensionChatMessage = { ts: Date.now(), type: "ask", @@ -105,6 +117,8 @@ describe("isStreamingAtom Logic", () => { }) it("should not be streaming for tool approval ask message", () => { + // Tool ask explicitly blocks streaming, even with recent activity + store.set(lastActivityTimestampAtom, Date.now()) const toolMessage: ExtensionChatMessage = { ts: Date.now(), type: "ask", @@ -117,6 +131,7 @@ describe("isStreamingAtom Logic", () => { }) it("should not be streaming for command approval ask message", () => { + // No recent activity = not streaming const commandMessage: ExtensionChatMessage = { ts: Date.now(), type: "ask", @@ -129,6 +144,7 @@ describe("isStreamingAtom Logic", () => { }) it("should not be streaming for complete say messages", () => { + // No recent activity = not streaming const sayMessage: ExtensionChatMessage = { ts: Date.now(), type: "say", @@ -142,6 +158,8 @@ describe("isStreamingAtom Logic", () => { }) it("should be streaming for partial say messages", () => { + // Set recent activity for streaming detection + store.set(lastActivityTimestampAtom, Date.now()) const sayMessage: ExtensionChatMessage = { ts: Date.now(), type: "say", @@ -155,6 +173,7 @@ describe("isStreamingAtom Logic", () => { }) it("should handle multiple messages with last being complete", () => { + // No recent activity = not streaming const messages: ExtensionChatMessage[] = [ { ts: Date.now(), @@ -188,6 +207,8 @@ describe("isStreamingAtom Logic", () => { }) it("should handle message updates via updateChatMessagesAtom", () => { + // Set recent activity initially for streaming + store.set(lastActivityTimestampAtom, Date.now()) const initialMessages: ExtensionChatMessage[] = [ { ts: Date.now(), @@ -201,6 +222,8 @@ describe("isStreamingAtom Logic", () => { store.set(chatMessagesAtom, initialMessages) expect(store.get(isStreamingAtom)).toBe(true) + // Clear activity timestamp before updating to completion + store.set(lastActivityTimestampAtom, 0) const updatedMessages: ExtensionChatMessage[] = [ ...initialMessages, { diff --git a/cli/src/types/messages.ts b/cli/src/types/messages.ts index 1eeced51d96..9ba00b4b07d 100644 --- a/cli/src/types/messages.ts +++ b/cli/src/types/messages.ts @@ -87,6 +87,7 @@ export interface ExtensionState { cwd?: string organizationAllowList?: OrganizationAllowList routerModels?: RouterModels + lastActivityTimestamp?: number // Track actual API stream activity for more accurate streaming detection [key: string]: unknown } diff --git a/cli/src/ui/components/__tests__/StatusIndicator.test.tsx b/cli/src/ui/components/__tests__/StatusIndicator.test.tsx index f8f46dc2e85..85e941c64a2 100644 --- a/cli/src/ui/components/__tests__/StatusIndicator.test.tsx +++ b/cli/src/ui/components/__tests__/StatusIndicator.test.tsx @@ -9,7 +9,7 @@ import { Provider as JotaiProvider } from "jotai" import { createStore } from "jotai" import { StatusIndicator } from "../StatusIndicator.js" import { showFollowupSuggestionsAtom } from "../../../state/atoms/ui.js" -import { chatMessagesAtom } from "../../../state/atoms/extension.js" +import { chatMessagesAtom, lastActivityTimestampAtom } from "../../../state/atoms/extension.js" import type { ExtensionChatMessage } from "../../../types/messages.js" // Mock the hooks @@ -28,6 +28,8 @@ describe("StatusIndicator", () => { beforeEach(() => { store = createStore() + // Default: no activity (timestamp = 0) so tests must explicitly set recent activity if needed + store.set(lastActivityTimestampAtom, 0) // Reset animation frame for each test vi.clearAllTimers() }) @@ -47,6 +49,8 @@ describe("StatusIndicator", () => { }) it("should show Thinking status and cancel hotkey when streaming", () => { + // Set recent activity for streaming detection + store.set(lastActivityTimestampAtom, Date.now()) // Set up a partial message to trigger streaming state const partialMessage: ExtensionChatMessage = { type: "say", diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index b458867c757..89e0a0e13e8 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -282,6 +282,9 @@ export class Task extends EventEmitter implements TaskLike { private askResponseImages?: string[] public lastMessageTs?: number + // Streaming Activity Tracking + lastStreamActivityTimestamp: number = 0 + // Tool Use consecutiveMistakeCount: number = 0 consecutiveMistakeLimit: number @@ -765,6 +768,13 @@ export class Task extends EventEmitter implements TaskLike { } } + private trackStreamActivity() { + // Update the last stream activity timestamp and notify the provider + // This is called whenever a chunk arrives from the API stream + this.lastStreamActivityTimestamp = Date.now() + this.providerRef.deref()?.postStateToWebview() + } + private findMessageByTimestamp(ts: number): ClineMessage | undefined { for (let i = this.clineMessages.length - 1; i >= 0; i--) { if (this.clineMessages[i].ts === ts) { @@ -2120,6 +2130,7 @@ export class Task extends EventEmitter implements TaskLike { let reasoningMessage = "" let pendingGroundingSources: GroundingSource[] = [] this.isStreaming = true + this.lastStreamActivityTimestamp = Date.now() // Initialize activity timestamp at stream start // kilocode_change start const assistantToolUses = new Array() @@ -2141,6 +2152,9 @@ export class Task extends EventEmitter implements TaskLike { continue } + // Track actual stream activity for accurate streaming detection + this.trackStreamActivity() + switch (chunk.type) { case "reasoning": { reasoningMessage += chunk.text diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 1e5027e2333..ddae39b8720 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -2243,6 +2243,7 @@ ${prompt} openRouterUseMiddleOutTransform, featureRoomoteControlEnabled, virtualQuotaActiveModel, // kilocode_change: Include virtual quota active model in state + lastActivityTimestamp: this.getCurrentTask()?.lastStreamActivityTimestamp, } } diff --git a/src/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts index e56959d9cc2..bfb869b3195 100644 --- a/src/shared/ExtensionMessage.ts +++ b/src/shared/ExtensionMessage.ts @@ -496,6 +496,7 @@ export type ExtensionState = Pick< featureRoomoteControlEnabled: boolean virtualQuotaActiveModel?: { id: string; info: ModelInfo } // kilocode_change: Add virtual quota active model for UI display showTimestamps?: boolean // kilocode_change: Show timestamps in chat messages + lastActivityTimestamp?: number // Timestamp of the last activity in the current task } export interface ClineSayTool {