fix(site/src/pages/AgentsPage/components): gate transcript copy actions and keep live thinking row in flow (#26214)

2026-09-22 13:10:21 +08:00 · 2026-06-11 10:08:10 +01:00
parent 5ab25b3ff6
commit 68efed86fa
10 changed files with 232 additions and 175 deletions
@@ -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();
 	},
 };

@@ -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 (
 			<div data-transcript-row="">
-				<ToolCollapsible
+				<ToolCall.Root
 					className="w-full"
+					status={isStreaming ? "running" : "completed"}
+					hasContent={hasText}
 					expanded={expanded}
 					onExpandedChange={(open) => setManualToggle(open)}
-					header={
-						<>
-							<ToolIcon name="thinking" isError={false} />
-							{isStreaming ? (
-								<Shimmer as="span" className="text-[13px] leading-6">
-									{title}
-								</Shimmer>
-							) : (
-								<span className="text-[13px] leading-6">{title}</span>
-							)}
-						</>
-					}
 				>
-					{hasText && (
+					<ToolCall.Header
+						iconName="thinking"
+						label={title}
+						showStatus={false}
+					/>
+					<ToolCall.Content>
 						<div
 							ref={previewScrollRef}
 							className={cn(
@@ -189,8 +182,8 @@ const ReasoningDisclosure = memo<{
 								{body}
 							</Response>
 						</div>
-					)}
-				</ToolCollapsible>
+					</ToolCall.Content>
+				</ToolCall.Root>
 			</div>
 		);
 	},
@@ -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<{
 								)}
 						</div>
 					)}
-				{displayState.needsAssistantBottomSpacer && (
+				{displayState.needsAssistantBottomSpacer && !isLastMessage && (
 					<div className="min-h-6" data-testid="assistant-bottom-spacer" />
 				)}
 				{previewImage && (
@@ -1289,6 +1288,7 @@ export const ConversationTimeline = memo<ConversationTimelineProps>(
 								hideActions={!isLastInChain}
 								hasActiveStream={Boolean(hasActiveStream)}
 								isAwaitingFirstStreamChunk={Boolean(isAwaitingFirstStreamChunk)}
+								isLastMessage={msgIdx === displayMessages.length - 1}
 								mcpServers={mcpServers}
 								subagentTitles={subagentTitles}
 								subagentVariants={subagentVariants}
@@ -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();
@@ -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 = () => (
 	<div
 		data-testid="live-activity-slot"
-		aria-hidden={!visible}
-		className={cn(
-			"flex items-center gap-2 text-content-secondary",
-			detached ? "pointer-events-none absolute left-0 top-full mt-2" : "h-6",
-			!visible && "invisible",
-		)}
+		className="flex h-6 items-center gap-2 text-content-secondary"
 	>
 		<ToolIcon name="thinking" isError={false} />
 		<Shimmer as="span" className="text-[13px] leading-6">
@@ -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) && (
 							<ChatStatusCallout status={liveStatus} />
 						)}
-						<LiveActivitySlot
-							visible={showActivity}
-							detached={hasVisibleFlowContent}
-						/>
+						{showActivity && <LiveActivitySlot />}
 					</div>
 				</MessageContent>
 			</Message>
@@ -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." }],
@@ -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 &&
@@ -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<TranscriptRowProps> = ({
 	asChild = false,
 	className,
@@ -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<ToolCollapsibleProps> = ({
-	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 ? (
-		<TranscriptRow
-			asChild
-			className={cn(
-				"m-0 cursor-pointer gap-2 border-0 bg-transparent p-0 text-left font-[inherit] text-[inherit] text-content-secondary transition-colors hover:text-content-primary",
-				headerActions ? "min-w-0 flex-1" : "w-full",
-				headerClassName,
-			)}
-		>
-			<button
-				type="button"
-				aria-expanded={expanded}
-				aria-label={
-					typeof ariaLabel === "function" ? ariaLabel(expanded) : ariaLabel
-				}
-				onClick={toggleExpanded}
-			>
-				{renderedHeader}
-				{headerStatus}
-				<ChevronDownIcon
-					className={cn(
-						"size-3 shrink-0 text-current transition-transform",
-						expanded ? "rotate-0" : "-rotate-90",
-					)}
-				/>
-			</button>
-		</TranscriptRow>
-	) : (
-		<TranscriptRow
-			className={cn(
-				"gap-2 text-content-secondary",
-				headerActions && "min-w-0 flex-1",
-				headerClassName,
-			)}
-		>
-			{renderedHeader}
-			{headerStatus}
-		</TranscriptRow>
-	);
-
-	return (
-		<div className={className}>
-			{headerActions ? (
-				<div className="flex w-full items-center gap-2">
-					{headerButton}
-					<div className="flex shrink-0 items-center gap-1">
-						{headerActions}
-					</div>
-				</div>
-			) : (
-				headerButton
-			)}
-			{expanded && hasContent && children}
-		</div>
-	);
-};
@@ -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<WebSearchSourcesProps> = ({ sources }) => {
 	// Deduplicate sources by URL, keeping the first occurrence.
 	const unique = (() => {
@@ -32,23 +28,24 @@ const WebSearchSources: FC<WebSearchSourcesProps> = ({ sources }) => {
 	const detail = unique.length === 1 ? "1 result" : `${unique.length} results`;

 	return (
-		<ToolCollapsible
-			hasContent={unique.length > 0}
-			header={
-				<>
+		<ToolCall.Root status="completed" hasContent={unique.length > 0}>
+			<ToolCall.HeaderButton>
+				<ToolCall.LeadingIcon>
 					<GlobeIcon className="size-4 shrink-0 stroke-[1.5] text-current" />
-					<span className="text-[13px] leading-6">
-						Searched <span className="text-content-secondary/60">{detail}</span>
-					</span>
-				</>
-			}
-		>
-			<div className="mt-1.5 flex flex-wrap items-center gap-1.5">
-				{unique.map((source) => (
-					<SourcePill key={source.url} source={source} />
-				))}
-			</div>
-		</ToolCollapsible>
+				</ToolCall.LeadingIcon>
+				<ToolCall.Label>
+					Searched <span className="text-content-secondary/60">{detail}</span>
+				</ToolCall.Label>
+				<ToolCall.Chevron />
+			</ToolCall.HeaderButton>
+			<ToolCall.Content>
+				<div className="mt-1.5 flex flex-wrap items-center gap-1.5">
+					{unique.map((source) => (
+						<SourcePill key={source.url} source={source} />
+					))}
+				</div>
+			</ToolCall.Content>
+		</ToolCall.Root>
 	);
 };

@@ -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;