diff --git a/site/src/pages/AgentsPage/components/ChatConversation/StreamingOutput.stories.tsx b/site/src/pages/AgentsPage/components/ChatConversation/StreamingOutput.stories.tsx index 62d6099565..60593bb61f 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/StreamingOutput.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/StreamingOutput.stories.tsx @@ -5,6 +5,7 @@ import { buildLiveStatus, buildReconnectState, buildRetryState, + buildStreamRenderState, FIXTURE_NOW, } from "./storyFixtures"; @@ -252,3 +253,28 @@ export const RetryStartupTimeout: Story = { ).not.toBeInTheDocument(); }, }; + +/** + * During streaming, if only tool-call blocks have arrived (no text + * or reasoning), the "Thinking..." indicator should still be visible + * alongside the tool cards. + */ +export const ThinkingDuringStreamingWithToolCalls: Story = { + args: { + ...buildStreamRenderState([ + { + type: "tool-call", + tool_name: "execute", + tool_call_id: "tc-1", + args: { command: "ls -la" }, + }, + ]), + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + // "Thinking..." should still be visible during streaming + // when only tool-call blocks have arrived. + const matches = canvas.getAllByText("Thinking..."); + expect(matches.length).toBeGreaterThanOrEqual(1); + }, +}; diff --git a/site/src/pages/AgentsPage/components/ChatConversation/StreamingOutput.tsx b/site/src/pages/AgentsPage/components/ChatConversation/StreamingOutput.tsx index f83fbc34b2..e7b67a6847 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/StreamingOutput.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/StreamingOutput.tsx @@ -1,17 +1,51 @@ import type { FC } from "react"; import type { UrlTransform } from "streamdown"; import type * as TypesGen from "#/api/typesGenerated"; -import { ConversationItem, Message, MessageContent } from "../ChatElements"; +import { + ConversationItem, + Message, + MessageContent, + Response, + Shimmer, +} from "../ChatElements"; import { ChatStatusCallout } from "./ChatStatusCallout"; import { BlockList } from "./ConversationTimeline"; import type { LiveStatusModel } from "./liveStatusModel"; -import type { MergedTool, StreamState } from "./types"; +import type { MergedTool, RenderBlock, StreamState } from "./types"; const hasTransientLiveStatus = (liveStatus: LiveStatusModel): boolean => liveStatus.phase === "starting" || liveStatus.phase === "retrying" || liveStatus.phase === "reconnecting"; +/** + * True when the block list contains at least one text or reasoning + * block. Tool-call and other non-text blocks don't count because + * they don't replace the "Thinking..." placeholder visually. + */ +const hasTextOrReasoningBlock = (blocks: readonly RenderBlock[]): boolean => + blocks.some((b) => b.type === "response" || b.type === "thinking"); + +/** + * Stateless "Thinking..." shimmer used during the streaming phase + * when no text or reasoning blocks have arrived yet. Unlike the + * `StartingPlaceholder` in `ChatStatusCallout`, this has no + * delayed-startup timer — the streaming phase is transient and + * will be replaced as soon as real content arrives. + */ +const StreamingThinkingPlaceholder: FC = () => ( +
+ + Thinking... + +
+ + Thinking... + +
+
+); + export const StreamingOutput: FC<{ streamState: StreamState | null; streamTools: readonly MergedTool[]; @@ -40,13 +74,25 @@ export const StreamingOutput: FC<{ const isStreaming = liveStatus.phase === "streaming"; const shouldShowBlocks = liveStatus.phase === "streaming" || liveStatus.hasAccumulatedOutput; - const shouldShowStatusCallout = hasTransientLiveStatus(liveStatus); + const blocks = shouldShowBlocks ? (streamState?.blocks ?? []) : []; + + // During streaming, keep showing the "Thinking..." indicator + // until text or reasoning blocks arrive. This bridges the + // visual gap between the "starting" phase placeholder and the + // first visible content, preventing the indicator from + // flickering away when only tool-call parts (or whitespace- + // only text deltas) have been received so far. + const needsStreamingThinking = + isStreaming && !hasTextOrReasoningBlock(blocks); + + const shouldShowStatusCallout = + hasTransientLiveStatus(liveStatus) || needsStreamingThinking; + if (!shouldShowBlocks && !shouldShowStatusCallout) { return null; } const conversationItemProps = { role: "assistant" as const }; - const blocks = shouldShowBlocks ? (streamState?.blocks ?? []) : []; return ( @@ -66,7 +112,8 @@ export const StreamingOutput: FC<{ mcpServers={mcpServers} /> )} - {shouldShowStatusCallout && ( + {needsStreamingThinking && } + {!needsStreamingThinking && hasTransientLiveStatus(liveStatus) && ( { expect(selectIsAwaitingFirstStreamChunk(store.getSnapshot())).toBe(false); }); - it("returns false during pending status even when stream state is null", () => { + it("returns true during pending status when latest message is from user", () => { const store = createChatStore(); store.setChatStatus("pending"); store.upsertDurableMessage(makeMessage(1, "user", "hello")); - // "pending" should NOT be treated as awaiting because the - // transport drops message_part events during pending status. + // "pending" with a user message as latest means the user + // just submitted and is waiting for the server to start. + expect(selectIsAwaitingFirstStreamChunk(store.getSnapshot())).toBe(true); + }); + + it("returns false during pending status when latest message is from assistant", () => { + const store = createChatStore(); + store.setChatStatus("pending"); + store.upsertDurableMessage(makeMessage(1, "user", "hello")); + store.upsertDurableMessage(makeMessage(2, "assistant", "calling tool")); + + // "pending" with an assistant message as latest means a + // tool-call cycle is in progress, not a fresh user send. expect(selectIsAwaitingFirstStreamChunk(store.getSnapshot())).toBe(false); }); @@ -675,4 +686,18 @@ describe("selectIsAwaitingFirstStreamChunk", () => { // "Thinking..." should appear immediately. expect(selectIsAwaitingFirstStreamChunk(store.getSnapshot())).toBe(true); }); + + it("returns true when WS delivers user message + status:pending (fresh send)", () => { + const store = createChatStore(); + // Simulate the WS batch: [message(user), status:pending]. + // This is the exact event order from the server when the + // user sends a message. "Thinking..." must appear during + // the pending phase so there is no visual gap before the + // server transitions to running. + store.upsertDurableMessage(makeMessage(1, "user", "sweet ty")); + store.setChatStatus("pending"); + store.clearStreamState(); + + expect(selectIsAwaitingFirstStreamChunk(store.getSnapshot())).toBe(true); + }); }); diff --git a/site/src/pages/AgentsPage/components/ChatConversation/chatStore.ts b/site/src/pages/AgentsPage/components/ChatConversation/chatStore.ts index 0b5821df10..6619fc1224 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/chatStore.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/chatStore.ts @@ -551,17 +551,26 @@ export const selectIsAwaitingFirstStreamChunk = ( const latestMessage = selectLatestDurableMessage(state); const latestMessageNeedsAssistantResponse = !latestMessage || latestMessage.role !== "assistant"; - // Only treat "running" as awaiting a first chunk. During "pending" - // status the transport drops incoming message_part events - // (shouldApplyMessagePart returns false), so streamState can never - // transition away from null. Including "pending" here caused the - // "Response startup is taking longer than expected" warning to - // fire spuriously during multi-turn tool-call cycles. - return ( - state.streamState === null && - state.chatStatus === "running" && - latestMessageNeedsAssistantResponse - ); + // Show the "Thinking..." indicator when the store has no stream + // data yet and the conversation is waiting for an assistant + // response. For "running" status we use the existing broad + // check (any non-assistant latest message). For "pending" we + // restrict to the case where the latest message is explicitly + // a user message — this covers the fresh-send flow (user just + // submitted and the server hasn't started streaming yet) while + // avoiding a spurious indicator during multi-turn tool-call + // cycles, where the latest durable message is a tool result + // and the assistant response is still being assembled. + if (state.streamState !== null || !latestMessageNeedsAssistantResponse) { + return false; + } + if (state.chatStatus === "running") { + return true; + } + if (state.chatStatus === "pending" && latestMessage?.role === "user") { + return true; + } + return false; }; export const useChatSelector = ( diff --git a/site/src/pages/AgentsPage/components/ChatConversation/streamState.test.ts b/site/src/pages/AgentsPage/components/ChatConversation/streamState.test.ts index ee8ae55cb5..b11984f90a 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/streamState.test.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/streamState.test.ts @@ -72,6 +72,22 @@ describe("applyMessagePartToStreamState", () => { expect(result).toBe(prev); }); + it("returns prev for text part with whitespace-only text", () => { + const result = applyMessagePartToStreamState(null, { + type: "text", + text: " ", + }); + expect(result).toBeNull(); + }); + + it("returns prev for reasoning part with whitespace-only text", () => { + const result = applyMessagePartToStreamState(null, { + type: "reasoning", + text: " \n\t ", + }); + expect(result).toBeNull(); + }); + it("creates tool call entry from tool-call part", () => { const result = applyMessagePartToStreamState(null, { type: "tool-call", diff --git a/site/src/pages/AgentsPage/components/ChatConversation/streamState.ts b/site/src/pages/AgentsPage/components/ChatConversation/streamState.ts index d51b333240..8602c89981 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/streamState.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/streamState.ts @@ -21,7 +21,10 @@ export const applyMessagePartToStreamState = ( switch (part.type) { case "text": { - if (!part.text) { + // Skip empty and whitespace-only deltas so they don't + // create a non-null StreamState with empty blocks, which + // would prematurely end the "starting" phase. + if (!part.text?.trim()) { return prev; } return { @@ -30,7 +33,7 @@ export const applyMessagePartToStreamState = ( }; } case "reasoning": { - if (!part.text) { + if (!part.text?.trim()) { return prev; } return {