diff --git a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx index 550cac3d32..0b3e95f1d4 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx @@ -1629,6 +1629,64 @@ export const AssistantMessageCopyButton: Story = { }, }; +/** + * Assistant messages that end with a tool call get no copy button, + * because the action row would otherwise render directly below the + * tool row instead of below copyable text. + */ +export const NoCopyButtonAfterTrailingToolCall: Story = { + args: { + ...defaultArgs, + parsedMessages: buildMessages([ + { + ...baseMessage, + id: 1, + role: "user", + content: [{ type: "text", text: "Run the tests" }], + }, + { + ...baseMessage, + id: 2, + role: "assistant", + content: [ + { type: "text", text: "Running the test suite now." }, + { + type: "tool-call", + tool_call_id: "call-exec-1", + tool_name: "execute", + args: { command: "make test" }, + }, + ], + }, + { + ...baseMessage, + id: 3, + role: "tool", + content: [ + { + type: "tool-result", + tool_call_id: "call-exec-1", + tool_name: "execute", + result: { output: "ok" }, + }, + ], + }, + ]), + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await canvas.findByText("Running the test suite now."); + // The assistant message ends with a tool call, so no copy button. + const actions = canvas.getAllByTestId("message-actions"); + expect(actions).toHaveLength(1); + for (const actionRow of actions) { + expect( + within(actionRow).getByRole("button", { name: "Copy message" }), + ).toBeInTheDocument(); + } + }, +}; + /** Persisted ask-user-question answers survive reloads. */ export const AskUserQuestionSubmittedAnswer: Story = { args: { @@ -1918,7 +1976,7 @@ export const MultiAssistantTurnCopyButton: Story = { }; /** - * Regression: thinking-only assistant messages must have consistent + * Thinking-only assistant messages must have consistent * bottom spacing before the next user bubble. A spacer div fills the * gap that would normally come from the invisible action bar. */ @@ -1957,6 +2015,40 @@ export const ThinkingOnlyAssistantSpacing: Story = { // it should still have visible text and a spacer element. expect(canvas.getByText("Explain this code")).toBeInTheDocument(); expect(canvas.getByText("Any progress?")).toBeInTheDocument(); + expect(canvas.getByTestId("assistant-bottom-spacer")).toBeInTheDocument(); + }, +}; + +/** No following bubble to space against; the spacer would be a dangling blank. */ +export const NoSpacerAfterTrailingThinkingMessage: Story = { + args: { + ...defaultArgs, + parsedMessages: buildMessages([ + { + ...baseMessage, + id: 1, + role: "user", + content: [{ type: "text", text: "Explain this code" }], + }, + { + ...baseMessage, + id: 2, + role: "assistant", + content: [ + { + type: "reasoning", + text: "Let me think about this step by step.", + }, + ], + }, + ]), + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect(canvas.getByText("Explain this code")).toBeInTheDocument(); + expect( + canvas.queryByTestId("assistant-bottom-spacer"), + ).not.toBeInTheDocument(); }, }; diff --git a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx index 7d4e0a6b70..9b3812b6be 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx @@ -28,7 +28,6 @@ import { Message, MessageContent, Response, - Shimmer, Tool, } from "../ChatElements"; import { WebSearchSources } from "../ChatElements/tools"; @@ -38,8 +37,7 @@ import { ReadFileTool, } from "../ChatElements/tools/ReadFileTool"; import type { SubagentVariant } from "../ChatElements/tools/subagentDescriptor"; -import { ToolCollapsible } from "../ChatElements/tools/ToolCollapsible"; -import { ToolIcon } from "../ChatElements/tools/ToolIcon"; +import { ToolCall } from "../ChatElements/tools/ToolCall"; import { ImageLightbox } from "../ImageLightbox"; import { TextPreviewDialog } from "../TextPreviewDialog"; import { @@ -156,24 +154,19 @@ const ReasoningDisclosure = memo<{ return (
- setManualToggle(open)} - header={ - <> - - {isStreaming ? ( - - {title} - - ) : ( - {title} - )} - - } > - {hasText && ( + +
- )} - + +
); }, @@ -533,6 +526,11 @@ const ChatMessageItem = memo<{ hasActiveStream?: boolean; isAwaitingFirstStreamChunk?: boolean; + // The bottom spacer fakes the height of the hidden action bar so + // chain-end messages keep even spacing before the next bubble. + // The last transcript message has nothing after it, so the spacer + // would render as a dangling blank at the end of the chat. + isLastMessage?: boolean; // When true, renders a gradient overlay inside the bubble // that fades text out toward the bottom. Used by the sticky // overlay to indicate truncated content. @@ -561,6 +559,7 @@ const ChatMessageItem = memo<{ hideActions = false, hasActiveStream = false, isAwaitingFirstStreamChunk = false, + isLastMessage = false, fadeFromBottom = false, onImplementPlan, onSendAskUserQuestionResponse, @@ -744,7 +743,7 @@ const ChatMessageItem = memo<{ )} )} - {displayState.needsAssistantBottomSpacer && ( + {displayState.needsAssistantBottomSpacer && !isLastMessage && (
)} {previewImage && ( @@ -1289,6 +1288,7 @@ export const ConversationTimeline = memo( hideActions={!isLastInChain} hasActiveStream={Boolean(hasActiveStream)} isAwaitingFirstStreamChunk={Boolean(isAwaitingFirstStreamChunk)} + isLastMessage={msgIdx === displayMessages.length - 1} mcpServers={mcpServers} subagentTitles={subagentTitles} subagentVariants={subagentVariants} diff --git a/site/src/pages/AgentsPage/components/ChatConversation/StreamingOutput.stories.tsx b/site/src/pages/AgentsPage/components/ChatConversation/StreamingOutput.stories.tsx index b2324de155..9527269db8 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/StreamingOutput.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/StreamingOutput.stories.tsx @@ -56,7 +56,7 @@ export const ReconnectingAfterDisconnect: Story = { expect(canvasElement.textContent).toMatch(/reconnecting in \d+s/i); }); expect(canvas.queryByText("Unexpected error")).not.toBeInTheDocument(); - expect(canvas.getByTestId("live-activity-slot")).not.toBeVisible(); + expect(canvas.queryByTestId("live-activity-slot")).not.toBeInTheDocument(); expect(canvas.queryByText("Thinking...")).not.toBeInTheDocument(); }, }; @@ -79,7 +79,7 @@ export const RetryWithVisibleReason: Story = { expect( canvas.getByText(/anthropic returned an unexpected error/i), ).toBeVisible(); - expect(canvas.getByTestId("live-activity-slot")).not.toBeVisible(); + expect(canvas.queryByTestId("live-activity-slot")).not.toBeInTheDocument(); expect(canvas.queryByText("Thinking...")).not.toBeInTheDocument(); expect(canvas.getByText(/attempt 1/i)).toBeVisible(); expect(canvas.queryByText(/please try again/i)).not.toBeInTheDocument(); @@ -259,7 +259,7 @@ export const StartingShowsThinkingActivity: Story = { }, }; -export const ResponseKeepsActivitySlotReserved: Story = { +export const ResponseDoesNotRenderActivitySlot: Story = { args: { streamState: responseStreamState.streamState, streamTools: responseStreamState.streamTools, @@ -267,7 +267,7 @@ export const ResponseKeepsActivitySlotReserved: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByTestId("live-activity-slot")).not.toBeVisible(); + expect(canvas.queryByTestId("live-activity-slot")).not.toBeInTheDocument(); }, }; @@ -291,7 +291,7 @@ export const RunningToolsSuppressThinkingActivity: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByTestId("live-activity-slot")).not.toBeVisible(); + expect(canvas.queryByTestId("live-activity-slot")).not.toBeInTheDocument(); expect( canvas.getByRole("button", { name: /expand command/i }), ).toBeVisible(); diff --git a/site/src/pages/AgentsPage/components/ChatConversation/StreamingOutput.tsx b/site/src/pages/AgentsPage/components/ChatConversation/StreamingOutput.tsx index 99cb9b30f4..56cb79014d 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/StreamingOutput.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/StreamingOutput.tsx @@ -1,7 +1,6 @@ import type { FC } from "react"; import type { UrlTransform } from "streamdown"; import type * as TypesGen from "#/api/typesGenerated"; -import { cn } from "#/utils/cn"; import { ConversationItem, Message, @@ -19,18 +18,10 @@ import type { MergedTool, StreamState } from "./types"; const hasCalloutLiveStatus = (liveStatus: LiveStatusModel): boolean => liveStatus.phase === "retrying" || liveStatus.phase === "reconnecting"; -const LiveActivitySlot: FC<{ - visible: boolean; - detached: boolean; -}> = ({ visible, detached }) => ( +const LiveActivitySlot: FC = () => (
@@ -72,8 +63,6 @@ export const StreamingOutput: FC<{ streamState, streamTools, }); - const hasVisibleFlowContent = - shouldShowBlocks || hasCalloutLiveStatus(liveStatus); const conversationItemProps = { role: "assistant" as const }; @@ -98,10 +87,7 @@ export const StreamingOutput: FC<{ {hasCalloutLiveStatus(liveStatus) && ( )} - + {showActivity && }
diff --git a/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.test.ts b/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.test.ts index 883f72280e..d7487a7f64 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.test.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.test.ts @@ -206,6 +206,82 @@ describe("deriveMessageDisplayState", () => { expect(getDisplayState(message).hasCopyableContent).toBe(false); }); + it("does not mark assistant messages ending with a tool call as copyable", () => { + const tool: MergedTool = { + id: "execute-1", + name: "execute", + args: { command: "pnpm test" }, + isError: false, + status: "completed", + }; + const message = buildMessage( + [{ type: "text", text: "Running the tests now." }], + "assistant", + ); + + const state = getDisplayState(message, { + parsed: parsed({ + markdown: "Running the tests now.", + tools: [tool], + blocks: [ + { type: "response", text: "Running the tests now." }, + { type: "tool", id: tool.id }, + ], + }), + }); + + expect(state.hasCopyableContent).toBe(false); + }); + + it("marks assistant messages ending with text after a tool call as copyable", () => { + const tool: MergedTool = { + id: "execute-1", + name: "execute", + args: { command: "pnpm test" }, + isError: false, + status: "completed", + }; + const message = buildMessage( + [{ type: "text", text: "All tests passed." }], + "assistant", + ); + + const state = getDisplayState(message, { + parsed: parsed({ + markdown: "All tests passed.", + tools: [tool], + blocks: [ + { type: "tool", id: tool.id }, + { type: "response", text: "All tests passed." }, + ], + }), + }); + + expect(state.hasCopyableContent).toBe(true); + }); + + it("does not mark assistant messages ending with a thinking block as copyable", () => { + // Intended: the action row renders below the whole message, so a + // trailing thinking disclosure has the same visual problem as a + // trailing tool call even though copyable markdown exists. + const message = buildMessage( + [{ type: "text", text: "Here is my answer." }], + "assistant", + ); + + const state = getDisplayState(message, { + parsed: parsed({ + markdown: "Here is my answer.", + blocks: [ + { type: "response", text: "Here is my answer." }, + { type: "thinking", text: "Reconsidering the edge cases." }, + ], + }), + }); + + expect(state.hasCopyableContent).toBe(false); + }); + it("shows the assistant spacer for reasoning messages when no suppressing flags apply", () => { const message = buildMessage( [{ type: "reasoning", text: "I should think before answering." }], diff --git a/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts b/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts index c0cc884cd3..08a6a47657 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts @@ -72,10 +72,14 @@ const getRenderableContentState = (parsed: ParsedMessageContent) => { const hasThinkingOnlyContent = visibleBlocks.length > 0 && visibleBlocks.every((block) => block.type === "thinking"); + const endsWithResponseBlock = + visibleBlocks.length > 0 && + visibleBlocks[visibleBlocks.length - 1].type === "response"; return { hasRenderableContent, hasThinkingOnlyContent, + endsWithResponseBlock, }; }; @@ -139,9 +143,16 @@ export const deriveMessageDisplayState = ({ const hasUserMessageBody = userInlineContent.length > 0 || Boolean(parsed.markdown.trim()); const hasFileBlocks = userFileBlocks.length > 0; + const { hasThinkingOnlyContent, endsWithResponseBlock } = + getRenderableContentState(parsed); + // The copy action row renders below the whole message, so assistant + // messages only get one when the last visible block is text. + // Otherwise the button would sit under a tool call with nothing + // copyable directly above it. const hasCopyableContent = - Boolean(parsed.markdown.trim()) && !hasFileAttachments; - const { hasThinkingOnlyContent } = getRenderableContentState(parsed); + Boolean(parsed.markdown.trim()) && + !hasFileAttachments && + (isUser || endsWithResponseBlock); const needsAssistantBottomSpacer = !hideActions && !hasActiveStream && diff --git a/site/src/pages/AgentsPage/components/ChatElements/TranscriptRow.tsx b/site/src/pages/AgentsPage/components/ChatElements/TranscriptRow.tsx index c4dacabe18..2155a664d8 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/TranscriptRow.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/TranscriptRow.tsx @@ -6,10 +6,7 @@ type TranscriptRowProps = ComponentPropsWithRef<"div"> & { asChild?: boolean; }; -/** - * Some transcript rows bypass ToolCollapsible, so they need one shared place - * to keep the collapsed row height aligned across the chat timeline. - */ +/** Consistent min-height for transcript rows that bypass the ToolCall primitives. */ export const TranscriptRow: FC = ({ asChild = false, className, diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/ToolCollapsible.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/ToolCollapsible.tsx deleted file mode 100644 index 257af63137..0000000000 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/ToolCollapsible.tsx +++ /dev/null @@ -1,104 +0,0 @@ -import { ChevronDownIcon } from "lucide-react"; -import type { FC, ReactNode } from "react"; -import { useState } from "react"; -import { cn } from "#/utils/cn"; -import { TranscriptRow } from "../TranscriptRow"; - -type ToolCollapsibleAriaLabel = string | ((expanded: boolean) => string); -type ToolCollapsibleHeader = ReactNode | ((expanded: boolean) => ReactNode); - -interface ToolCollapsibleProps { - children: ReactNode; - header: ToolCollapsibleHeader; - headerActions?: ReactNode; - headerStatus?: ReactNode; - hasContent?: boolean; - defaultExpanded?: boolean; - expanded?: boolean; - onExpandedChange?: (expanded: boolean) => void; - ariaLabel?: ToolCollapsibleAriaLabel; - className?: string; - headerClassName?: string; -} - -export const ToolCollapsible: FC = ({ - children, - header, - headerActions, - headerStatus, - hasContent = true, - defaultExpanded = false, - expanded: expandedProp, - onExpandedChange, - ariaLabel, - className, - headerClassName, -}) => { - const [uncontrolledExpanded, setUncontrolledExpanded] = - useState(defaultExpanded); - const expanded = expandedProp ?? uncontrolledExpanded; - const renderedHeader = - typeof header === "function" ? header(expanded) : header; - const toggleExpanded = () => { - const nextExpanded = !expanded; - if (expandedProp === undefined) { - setUncontrolledExpanded(nextExpanded); - } - onExpandedChange?.(nextExpanded); - }; - const headerButton = hasContent ? ( - - - - ) : ( - - {renderedHeader} - {headerStatus} - - ); - - return ( -
- {headerActions ? ( -
- {headerButton} -
- {headerActions} -
-
- ) : ( - headerButton - )} - {expanded && hasContent && children} -
- ); -}; diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/WebSearchSources.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/WebSearchSources.tsx index 0ede427fd2..090b4f3fa4 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/WebSearchSources.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/WebSearchSources.tsx @@ -1,17 +1,13 @@ import { ExternalLinkIcon, GlobeIcon } from "lucide-react"; import type { FC } from "react"; import { cn } from "#/utils/cn"; -import { ToolCollapsible } from "./ToolCollapsible"; +import { ToolCall } from "./ToolCall"; interface WebSearchSourcesProps { sources: Array<{ url: string; title: string }>; } -/** - * Renders web search sources as a collapsible tool card, consistent - * with other tool call renderings. The collapsed header shows a globe - * icon and "Searched N sources"; expanding reveals clickable pills. - */ +/** Collapsible web-search result pills, styled as a ToolCall row. */ const WebSearchSources: FC = ({ sources }) => { // Deduplicate sources by URL, keeping the first occurrence. const unique = (() => { @@ -32,23 +28,24 @@ const WebSearchSources: FC = ({ sources }) => { const detail = unique.length === 1 ? "1 result" : `${unique.length} results`; return ( - 0} - header={ - <> + 0}> + + - - Searched {detail} - - - } - > -
- {unique.map((source) => ( - - ))} -
- + + + Searched {detail} + + + + +
+ {unique.map((source) => ( + + ))} +
+ + ); }; diff --git a/site/src/pages/AgentsPage/components/ChatPageContent.stories.tsx b/site/src/pages/AgentsPage/components/ChatPageContent.stories.tsx index 51b1287316..481046dd28 100644 --- a/site/src/pages/AgentsPage/components/ChatPageContent.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatPageContent.stories.tsx @@ -37,6 +37,8 @@ const buildThinkingSpacerStore = () => { text: "I should think before answering.", }, ]), + // A following message is needed so the spacer renders. + buildMessage(3, "user", [{ type: "text", text: "Any progress?" }]), ]); return store;