diff --git a/apps/client/src/features/ai-chat/components/message-item.tsx b/apps/client/src/features/ai-chat/components/message-item.tsx index 3fc83ded..255f5722 100644 --- a/apps/client/src/features/ai-chat/components/message-item.tsx +++ b/apps/client/src/features/ai-chat/components/message-item.tsx @@ -5,6 +5,7 @@ import ToolCallCard from "@/features/ai-chat/components/tool-call-card.tsx"; import ChatErrorAlert from "@/features/ai-chat/components/chat-error-alert.tsx"; import ChatStoppedNotice from "@/features/ai-chat/components/chat-stopped-notice.tsx"; import { ToolUiPart, isToolPart } from "@/features/ai-chat/utils/tool-parts.tsx"; +import { assistantMessageHasVisibleContent } from "@/features/ai-chat/utils/message-content.ts"; import { renderChatMarkdown } from "@/features/ai-chat/utils/markdown.ts"; import { resolveAssistantName } from "@/features/ai-chat/utils/assistant-name.ts"; import { describeChatError } from "@/features/ai-chat/utils/error-message.ts"; @@ -66,6 +67,16 @@ export default function MessageItem({ ); } + // An assistant message with nothing visible to render yet (an empty streaming + // text part, or a reasoning/step-start part while the model is still thinking) + // renders nothing here. The standalone TypingIndicator stands in for the nascent + // bubble (name + dots) until real content arrives, so exactly one element owns + // the agent name during the pre-content gap and the layout never jumps. Persisted + // errored/aborted turns DO have visible content per the helper (metadata.error / + // finishReason === "aborted"), so their banners below still render — this early + // return won't fire for them. + if (!assistantMessageHasVisibleContent(message)) return null; + return ( diff --git a/apps/client/src/features/ai-chat/components/message-list.tsx b/apps/client/src/features/ai-chat/components/message-list.tsx index fca37b9b..a1bc14f5 100644 --- a/apps/client/src/features/ai-chat/components/message-list.tsx +++ b/apps/client/src/features/ai-chat/components/message-list.tsx @@ -5,6 +5,7 @@ import type { UIMessage } from "@ai-sdk/react"; import MessageItem from "@/features/ai-chat/components/message-item.tsx"; import TypingIndicator from "@/features/ai-chat/components/typing-indicator.tsx"; import { isToolPart, toolRunState, ToolUiPart } from "@/features/ai-chat/utils/tool-parts.tsx"; +import { assistantMessageHasVisibleContent } from "@/features/ai-chat/utils/message-content.ts"; import classes from "@/features/ai-chat/components/ai-chat.module.css"; interface MessageListProps { @@ -79,13 +80,18 @@ export function showTypingIndicator(messages: UIMessage[], isStreaming: boolean) /** * Whether the standalone typing indicator should render its own assistant-name - * label. True only when it stands in for a not-yet-started assistant row (it IS - * the nascent bubble). False once an assistant row exists at the tail, because - * that row's MessageItem already shows the same name — avoids a duplicate label. + * label. The indicator OWNS the name while the tail assistant row has no visible + * content yet (an empty streaming text part, or reasoning/step-start while the + * model is still thinking): in that gap the assistant MessageItem renders nothing, + * so the indicator stands in for the nascent bubble (name + dots) at a constant + * gap. It hides the name only once that row shows visible content, because then + * MessageItem draws the same name — avoids a duplicate stacked label and the + * layout jump that switching owners mid-stream used to cause. */ export function typingIndicatorShowsName(messages: UIMessage[]): boolean { const last = messages[messages.length - 1]; - return !(last && last.role === "assistant"); + if (!last || last.role !== "assistant") return true; + return !assistantMessageHasVisibleContent(last); } /** diff --git a/apps/client/src/features/ai-chat/components/typing-indicator-shows-name.test.ts b/apps/client/src/features/ai-chat/components/typing-indicator-shows-name.test.ts index 1d48a28a..7d1dde0b 100644 --- a/apps/client/src/features/ai-chat/components/typing-indicator-shows-name.test.ts +++ b/apps/client/src/features/ai-chat/components/typing-indicator-shows-name.test.ts @@ -4,10 +4,14 @@ import { typingIndicatorShowsName } from "@/features/ai-chat/components/message- /** * Pure-helper tests for whether the standalone "Thinking…" indicator renders its - * own dimmed assistant-name label. It should only show the name while it stands - * in for a not-yet-started assistant row; once an assistant row exists at the - * tail, that row's MessageItem already shows the same name, so the indicator - * must show only the dots to avoid a duplicate stacked label. + * own dimmed assistant-name label. The indicator OWNS the name while the tail + * assistant row has no visible content yet (an empty streaming text part, or + * reasoning/step-start while the model is still thinking) — in that gap the + * assistant MessageItem renders nothing, so the indicator stands in for the + * nascent bubble (name + dots). It hides the name only once the tail assistant + * row shows visible content, because then MessageItem draws the same name — this + * avoids a duplicate stacked label and the layout jump that switching owners + * mid-stream used to cause. */ const msg = ( role: "user" | "assistant", @@ -25,16 +29,24 @@ describe("typingIndicatorShowsName", () => { ).toBe(true); }); - it("hides the name when an assistant row exists at the tail", () => { + it("shows the name when the tail assistant row has no visible content yet (empty text part)", () => { + // The empty streaming text part has no visible content, so MessageItem renders + // nothing and the indicator owns the name (the nascent bubble). expect( typingIndicatorShowsName([msg("assistant", [{ type: "text", text: "" }])]), - ).toBe(false); + ).toBe(true); }); - it("hides the name when the assistant row's last part is a tool", () => { + it("hides the name once the tail assistant row shows content (a tool part)", () => { const doneTool = { type: "tool-getPage", state: "output-available" } as unknown as UIMessage["parts"][number]; expect( typingIndicatorShowsName([msg("assistant", [doneTool])]), ).toBe(false); }); + + it("hides the name once the tail assistant row shows content (non-empty text)", () => { + expect( + typingIndicatorShowsName([msg("assistant", [{ type: "text", text: "answer" }])]), + ).toBe(false); + }); }); diff --git a/apps/client/src/features/ai-chat/utils/message-content.test.ts b/apps/client/src/features/ai-chat/utils/message-content.test.ts new file mode 100644 index 00000000..b837baa9 --- /dev/null +++ b/apps/client/src/features/ai-chat/utils/message-content.test.ts @@ -0,0 +1,94 @@ +import { describe, expect, it } from "vitest"; +import type { UIMessage } from "@ai-sdk/react"; +import { assistantMessageHasVisibleContent } from "@/features/ai-chat/utils/message-content.ts"; + +/** + * Pure-helper tests for `assistantMessageHasVisibleContent`, the single source of + * truth shared by MessageItem (whether to render the bubble) and + * typingIndicatorShowsName (whether the standalone indicator owns the name). It + * must mirror MessageItem's render decisions exactly so exactly one element owns + * the agent name during the pre-content "thinking" gap. + */ +const msg = ( + parts: UIMessage["parts"], + metadata?: unknown, +): UIMessage => + ({ + id: Math.random().toString(), + role: "assistant", + parts, + metadata, + }) as UIMessage; + +describe("assistantMessageHasVisibleContent", () => { + it("is false for an empty text part", () => { + expect(assistantMessageHasVisibleContent(msg([{ type: "text", text: "" }]))).toBe(false); + }); + + it("is false for a whitespace-only text part", () => { + expect(assistantMessageHasVisibleContent(msg([{ type: "text", text: " " }]))).toBe(false); + }); + + it("is true for a non-empty text part", () => { + expect(assistantMessageHasVisibleContent(msg([{ type: "text", text: "answer" }]))).toBe(true); + }); + + it("is true for a tool part", () => { + const toolPart = { type: "tool-getPage", state: "output-available" } as unknown as UIMessage["parts"][number]; + expect(assistantMessageHasVisibleContent(msg([toolPart]))).toBe(true); + }); + + it("is true when metadata.error is set (persisted error banner)", () => { + expect( + assistantMessageHasVisibleContent(msg([{ type: "text", text: "" }], { error: "boom" })), + ).toBe(true); + }); + + it("is true when metadata.finishReason is 'aborted' (persisted stopped notice)", () => { + expect( + assistantMessageHasVisibleContent(msg([], { finishReason: "aborted" })), + ).toBe(true); + }); + + it("is false for a message with no parts and no metadata", () => { + expect(assistantMessageHasVisibleContent(msg([]))).toBe(false); + }); + + it("is false for an unsupported part kind (reasoning)", () => { + const reasoning = { type: "reasoning", text: "let me think" } as unknown as UIMessage["parts"][number]; + expect(assistantMessageHasVisibleContent(msg([reasoning]))).toBe(false); + }); + + it("is true for a running tool part (input-available)", () => { + // Tool visibility does not depend on tool state: MessageItem renders a + // ToolCallCard for any tool part, so a still-running tool is visible. + const runningTool = { type: "tool-getPage", state: "input-available" } as unknown as UIMessage["parts"][number]; + expect(assistantMessageHasVisibleContent(msg([runningTool]))).toBe(true); + }); + + it("is true for an empty leading text part followed by a non-empty one", () => { + // An empty leading text part followed by a non-empty one is still visible + // (mirrors the real streaming sequence where text arrives incrementally). + expect( + assistantMessageHasVisibleContent( + msg([{ type: "text", text: "" }, { type: "text", text: "answer" }]), + ), + ).toBe(true); + }); + + it("is false for an empty completed turn (finishReason 'stop')", () => { + // A completed turn with no text/tools and a non-aborted finishReason renders + // nothing — this is intentional (hiding a dangling name-only row), distinct + // from the `aborted`/`error` cases which DO render. + expect( + assistantMessageHasVisibleContent(msg([{ type: "text", text: "" }], { finishReason: "stop" })), + ).toBe(false); + }); + + it("is false for a parts-less message (the `?? []` guard makes it safe)", () => { + // The `?? []` guard makes a parts-less object safe instead of throwing. + expect( + assistantMessageHasVisibleContent({ id: "x", role: "assistant" } as unknown as UIMessage), + ).toBe(false); + }); +}); diff --git a/apps/client/src/features/ai-chat/utils/message-content.ts b/apps/client/src/features/ai-chat/utils/message-content.ts new file mode 100644 index 00000000..e718e18a --- /dev/null +++ b/apps/client/src/features/ai-chat/utils/message-content.ts @@ -0,0 +1,39 @@ +import type { UIMessage } from "@ai-sdk/react"; +import { isToolPart } from "@/features/ai-chat/utils/tool-parts.tsx"; + +/** + * Whether an assistant `UIMessage` has anything visible to render in its bubble. + * + * This mirrors MessageItem's render decisions EXACTLY and is the single source of + * truth shared by both MessageItem (to decide whether to render the bubble at all) + * and typingIndicatorShowsName (to decide whether the standalone "Thinking…" + * indicator owns the dimmed agent-name label). Keeping one helper guarantees the + * two stay in lockstep, so exactly one element owns the name during the pre-content + * "thinking" gap and the layout never reflows mid-stream. + * + * An assistant message has visible content iff ANY of: + * - a `text` part whose trimmed length > 0 (non-empty markdown), OR + * - ANY tool part (`isToolPart(part.type)`), OR + * - `metadata.error` is truthy (a persisted error banner renders), OR + * - `metadata.finishReason === "aborted"` (a persisted "response stopped" notice). + * Empty/whitespace-only text parts and unsupported part kinds (reasoning, sources, + * files, step-start) are NOT visible. + */ +export function assistantMessageHasVisibleContent(message: UIMessage): boolean { + const meta = message.metadata as + | { error?: string; finishReason?: string } + | undefined; + // Persisted errored/aborted turns always render their banner/notice. + if (meta?.error) return true; + if (meta?.finishReason === "aborted") return true; + + // `parts` may be empty (a nascent streaming message has no parts yet). + // `?? []` also guards a sparse/partial message object (metadata-only, no + // `parts`) so iterating cannot throw — it does not change behavior for any + // current input. + for (const part of message.parts ?? []) { + if (part.type === "text" && part.text.trim().length > 0) return true; + if (isToolPart(part.type)) return true; + } + return false; +}