diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.stories.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.stories.tsx index 9dc92963b9..197e0877f2 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.stories.tsx @@ -5,6 +5,15 @@ import { Tool } from "./Tool"; const sampleQuestion = "Should we extract a shared helper for tool result parsing before refactoring the agents page tool cards?"; +const longQuestion = [ + "We are planning a risky refactor of the advisor tool UI after several rounds of feedback from designers, frontend engineers, and dogfood users. The goal is to keep the card readable when the advisor includes a long prompt, a model name, a remaining-use count, and an expanded body with long markdown guidance.", + "Before changing the layout further, I want advice on whether the metadata should remain inline with the title, move into compact chips, wrap onto a second line, or disappear behind a details affordance when horizontal space is tight. Please weigh readability, scanability, accessibility, and consistency with adjacent tool cards.", + "The edge case I care about most is a real agent asking a verbose strategic question that includes implementation history, user feedback, test expectations, and design constraints in one tool call. The card should still make the question easy to read, avoid truncating important context, and keep the advisor identity, model, and usage details visually distinct.", + "Assume the answer may contain multiple markdown sections, bullets, and code references. The UI should not become visually heavy, the header should not look like one blended text block, the question should wrap naturally, and the body should remain scrollable without pushing nearby chat messages too far away.", + "Please recommend the safest layout and interaction behavior for this peak state, including where the metadata belongs, how much emphasis the long question should receive, whether the expanded state should stay open by default, and which details should be visible to users versus only useful for debugging.", + "Also call out any accessibility risks from nested buttons, long labels, dense metadata, color-only separators, or scroll regions, and suggest a practical test plan that Storybook can cover without adding brittle assertions about exact Tailwind class names.", +].join(" "); + const sampleAdvice = [ "# Quick summary", "", @@ -83,25 +92,17 @@ export const SuccessfulAdvice: Story = { const canvas = within(canvasElement); expect(canvas.getByText(sampleQuestion)).toBeInTheDocument(); expect(await canvas.findByText("Quick summary")).toBeInTheDocument(); - // Guards against a regression where `resolvedResultType` drops to - // undefined: the advice body would still render via the fallback - // branch, but the header badge would silently switch to - // "No guidance" instead of "Guidance ready". - expect(canvas.getByText("Guidance ready")).toBeInTheDocument(); + expect(canvas.getByText("Advice")).toBeInTheDocument(); + expect(canvas.queryByText("Guidance ready")).not.toBeInTheDocument(); + expect(canvas.getByText("GPT-5 Advisor")).toBeInTheDocument(); + expect(canvas.getByText("3 uses left")).toBeInTheDocument(); expect( - canvas.getByText( + canvas.queryByText( (_, element) => element?.textContent?.replace(/\s+/g, " ").trim() === "Advisor model: GPT-5 Advisor", ), - ).toBeInTheDocument(); - expect( - canvas.getByText( - (_, element) => - element?.textContent?.replace(/\s+/g, " ").trim() === - "Remaining uses: 3", - ), - ).toBeInTheDocument(); + ).not.toBeInTheDocument(); }, }; @@ -113,11 +114,10 @@ export const Running: Story = { play: async ({ canvasElement }) => { const canvas = within(canvasElement); expect(canvas.getByText(sampleQuestion)).toBeInTheDocument(); - // "Consulting advisor…" appears in both the header status badge and - // the body spinner label, so we expect exactly two matches. Asserting - // the count keeps the coverage for the body indicator even if the - // header ever stops rendering the same string. - expect(canvas.getAllByText("Consulting advisor…")).toHaveLength(2); + expect(canvas.getAllByText("Consulting advisor…")).toHaveLength(1); + expect( + canvas.getByText("Reviewing context and preparing guidance."), + ).toBeInTheDocument(); }, }; @@ -198,7 +198,7 @@ export const EmptyAdvice: Story = { expect( canvas.getByText("Advisor returned no guidance."), ).toBeInTheDocument(); - expect(canvas.getByText("No guidance")).toBeInTheDocument(); + expect(canvas.queryByText("No guidance")).not.toBeInTheDocument(); }, }; @@ -316,3 +316,42 @@ export const LongAdvice: Story = { expect(viewport.scrollTop).toBeGreaterThan(0); }, }; + +export const LongAdviceLongQuestion: Story = { + name: "Long Advice + long question", + args: { + status: "completed", + args: { question: longQuestion }, + result: { + type: "advice", + advice: longAdvice, + advisor_model: "GPT-5 Advisor", + remaining_uses: 12, + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const toggle = canvas.getByRole("button"); + const question = canvas.getByText(longQuestion); + + expect(question).toBeInTheDocument(); + const expandedQuestionHeight = question.getBoundingClientRect().height; + expect(expandedQuestionHeight).toBeGreaterThan(40); + expect(await canvas.findByText("Follow-up questions")).toBeInTheDocument(); + expect(canvas.getByText("Advice")).toBeInTheDocument(); + expect(canvas.getByText("GPT-5 Advisor")).toBeInTheDocument(); + expect(canvas.getByText("12 uses left")).toBeInTheDocument(); + + await userEvent.click(toggle); + expect(toggle).toHaveAttribute("aria-expanded", "false"); + expect(question.getBoundingClientRect().height).toBeLessThan( + expandedQuestionHeight, + ); + expect(canvas.queryByText("Follow-up questions")).not.toBeInTheDocument(); + + await userEvent.click(toggle); + expect(toggle).toHaveAttribute("aria-expanded", "true"); + expect(question.getBoundingClientRect().height).toBeGreaterThan(40); + expect(await canvas.findByText("Follow-up questions")).toBeInTheDocument(); + }, +}; diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx index 31ef622628..bf18de0fef 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx @@ -1,6 +1,7 @@ import { CircleAlertIcon, LoaderIcon, TriangleAlertIcon } from "lucide-react"; import type React from "react"; import { ScrollArea } from "#/components/ScrollArea/ScrollArea"; +import { cn } from "#/utils/cn"; import { Response } from "../Response"; import { ToolCollapsible } from "./ToolCollapsible"; import { ToolIcon } from "./ToolIcon"; @@ -44,19 +45,6 @@ export const AdvisorTool: React.FC = ({ const isRunning = status === "running"; const showLimitReached = resultType === "limit_reached"; const showError = isError || resultType === "error"; - const hasAdvice = resultType === "advice" && adviceText.length > 0; - const hasMetadata = - advisorModelText.length > 0 || remainingUses !== undefined; - - const headerStatus = isRunning - ? RUNNING_MESSAGE - : showLimitReached - ? "Limit reached" - : showError - ? "Request failed" - : hasAdvice - ? "Guidance ready" - : "No guidance"; return ( = ({ hasContent defaultExpanded headerClassName="items-start" - header={ + header={(expanded) => ( <> -
-
+
+ - - {headerStatus} - + {isRunning && ( + + {RUNNING_MESSAGE} + + )} + {advisorModelText && ( + + {advisorModelText} + + )} + {remainingUses !== undefined && ( + + {remainingUses.toLocaleString("en-US")} uses left + + )}
- + {questionText}
@@ -90,7 +101,7 @@ export const AdvisorTool: React.FC = ({ ) : null} - } + )} > = ({ >
{isRunning ? ( -
- - {RUNNING_MESSAGE} +
+ Reviewing context and preparing guidance.
) : showLimitReached ? (
= ({
) : ( -
- {adviceText || EMPTY_ADVICE_MESSAGE} - {hasMetadata && ( -
- {advisorModelText && ( - - Advisor model:{" "} - - {advisorModelText} - - - )} - {remainingUses !== undefined && ( - - Remaining uses:{" "} - - {remainingUses.toLocaleString("en-US")} - - - )} -
- )} -
+
+
+ + Advice + +
+ + {adviceText || EMPTY_ADVICE_MESSAGE} + +
)}
diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/ToolCollapsible.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/ToolCollapsible.tsx index d9dfd1fbcf..2ff075a1b7 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/ToolCollapsible.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/ToolCollapsible.tsx @@ -3,9 +3,11 @@ import type { FC, ReactNode } from "react"; import { useState } from "react"; import { cn } from "#/utils/cn"; +type ToolCollapsibleHeader = ReactNode | ((expanded: boolean) => ReactNode); + interface ToolCollapsibleProps { children: ReactNode; - header: ReactNode; + header: ToolCollapsibleHeader; hasContent?: boolean; defaultExpanded?: boolean; className?: string; @@ -21,6 +23,8 @@ export const ToolCollapsible: FC = ({ headerClassName, }) => { const [expanded, setExpanded] = useState(defaultExpanded); + const renderedHeader = + typeof header === "function" ? header(expanded) : header; return (
{hasContent ? ( @@ -35,7 +39,7 @@ export const ToolCollapsible: FC = ({ headerClassName, )} > - {header} + {renderedHeader} = ({ headerClassName, )} > - {header} + {renderedHeader}
)} {expanded && hasContent && children} diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/ToolIcon.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/ToolIcon.tsx index 76f33be2e0..f4d99ef1c9 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/ToolIcon.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/ToolIcon.tsx @@ -1,10 +1,10 @@ import { BookOpenIcon, BotIcon, - BrainCircuitIcon, ClipboardListIcon, FileIcon, FilePenIcon, + LightbulbIcon, MonitorIcon, PlayIcon, PlusCircleIcon, @@ -107,7 +107,7 @@ export const ToolIcon: React.FC<{ case "propose_plan": return ; case "advisor": - return ; + return ; case "computer": return ; case "read_skill": diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/ToolLabel.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/ToolLabel.tsx index 026d68923e..68df2fcf76 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/ToolLabel.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/ToolLabel.tsx @@ -202,7 +202,9 @@ export const ToolLabel: React.FC<{ } case "advisor": return ( - Advisor + + Advisor + ); case "read_skill": { const skillName = parsed ? asString(parsed.name) : "";