fix(site/src/pages/AgentsPage): polish advisor UI (#25002)

Polishes the advisor tool card so the header uses a clearer lightbulb icon, inline pill metadata, wrapped/clamped question text, and a separate advice pill before the rendered response.

Adds Storybook coverage for a long advice plus 1.8k-character question peak state, including collapse/expand behavior.

Refs https://linear.app/codercom/issue/CODAGT-322/improve-advisor-icon-and-duplicate-loading-ui

<details>
<summary>Coder Agents disclosure</summary>

This PR was generated by Coder Agents.

</details>
This commit is contained in:
Thomas Kosiewski
2026-05-07 11:06:14 +02:00
committed by GitHub
parent 6fa7e84761
commit 10a22a4753
5 changed files with 115 additions and 73 deletions
@@ -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();
},
};
@@ -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<AdvisorToolProps> = ({
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 (
<ToolCollapsible
@@ -64,21 +52,44 @@ export const AdvisorTool: React.FC<AdvisorToolProps> = ({
hasContent
defaultExpanded
headerClassName="items-start"
header={
header={(expanded) => (
<>
<ToolIcon name="advisor" isError={showError} isRunning={isRunning} />
<div className="flex min-w-0 flex-1 flex-col gap-0.5">
<div className="flex min-w-0 items-center gap-2">
<div className="flex min-w-0 items-center gap-2 leading-4">
<ToolIcon
name="advisor"
isError={showError}
isRunning={isRunning}
/>
<ToolLabel
name="advisor"
args={{ question: questionText }}
result={resultType ? { type: resultType } : undefined}
/>
<span className="text-2xs text-content-secondary">
{headerStatus}
</span>
{isRunning && (
<span className="shrink-0 rounded-full border border-solid border-border-default px-2 text-[13px] leading-4 text-content-secondary">
{RUNNING_MESSAGE}
</span>
)}
{advisorModelText && (
<span className="min-w-0 truncate rounded-full border border-solid border-border-default px-2 text-[13px] leading-4 text-content-secondary">
{advisorModelText}
</span>
)}
{remainingUses !== undefined && (
<span className="shrink-0 rounded-full border border-solid border-border-default px-2 text-[13px] leading-4 text-content-secondary">
{remainingUses.toLocaleString("en-US")} uses left
</span>
)}
</div>
<span className="block truncate text-sm text-content-primary">
<span
className={cn(
"ml-6 block whitespace-normal break-words text-[13px]",
"font-normal leading-5 text-content-primary",
"[overflow-wrap:anywhere]",
!expanded && "line-clamp-2",
)}
>
{questionText}
</span>
</div>
@@ -90,7 +101,7 @@ export const AdvisorTool: React.FC<AdvisorToolProps> = ({
<LoaderIcon className="mt-0.5 h-3.5 w-3.5 shrink-0 animate-spin motion-reduce:animate-none text-content-secondary" />
) : null}
</>
}
)}
>
<ScrollArea
className="mt-1.5 rounded-md border border-solid border-border-default bg-surface-primary"
@@ -100,9 +111,8 @@ export const AdvisorTool: React.FC<AdvisorToolProps> = ({
>
<div className="space-y-3 px-3 py-2">
{isRunning ? (
<div className="flex items-center gap-2 text-sm text-content-secondary">
<LoaderIcon className="h-4 w-4 shrink-0 animate-spin motion-reduce:animate-none" />
<span>{RUNNING_MESSAGE}</span>
<div role="status" className="text-sm text-content-secondary">
Reviewing context and preparing guidance.
</div>
) : showLimitReached ? (
<div
@@ -131,29 +141,16 @@ export const AdvisorTool: React.FC<AdvisorToolProps> = ({
</div>
</div>
) : (
<div className="space-y-3">
<Response>{adviceText || EMPTY_ADVICE_MESSAGE}</Response>
{hasMetadata && (
<div className="flex flex-wrap items-center gap-x-4 gap-y-1 border-t border-solid border-border-default pt-2 text-2xs text-content-secondary">
{advisorModelText && (
<span>
Advisor model:{" "}
<span className="font-medium text-content-primary">
{advisorModelText}
</span>
</span>
)}
{remainingUses !== undefined && (
<span>
Remaining uses:{" "}
<span className="font-medium text-content-primary">
{remainingUses.toLocaleString("en-US")}
</span>
</span>
)}
</div>
)}
</div>
<section className="space-y-2" aria-label="Advisor advice">
<div>
<span className="inline-flex rounded-full border border-solid border-border-default px-2 text-[13px] leading-4 text-content-secondary">
Advice
</span>
</div>
<Response className="[&_h1]:mb-2 [&_h1]:mt-3 [&_h1]:text-[15px] [&_h2]:mb-1.5 [&_h2]:mt-3 [&_h2]:text-sm [&_h3]:mb-1 [&_h3]:mt-2.5 [&_h3]:text-[13px] [&_h4]:mt-2 [&_h4]:text-[13px] [&_h5]:text-xs [&_h6]:text-xs">
{adviceText || EMPTY_ADVICE_MESSAGE}
</Response>
</section>
)}
</div>
</ScrollArea>
@@ -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<ToolCollapsibleProps> = ({
headerClassName,
}) => {
const [expanded, setExpanded] = useState(defaultExpanded);
const renderedHeader =
typeof header === "function" ? header(expanded) : header;
return (
<div className={className}>
{hasContent ? (
@@ -35,7 +39,7 @@ export const ToolCollapsible: FC<ToolCollapsibleProps> = ({
headerClassName,
)}
>
{header}
{renderedHeader}
<ChevronDownIcon
className={cn(
"h-3 w-3 shrink-0 text-current transition-transform",
@@ -50,7 +54,7 @@ export const ToolCollapsible: FC<ToolCollapsibleProps> = ({
headerClassName,
)}
>
{header}
{renderedHeader}
</div>
)}
{expanded && hasContent && children}
@@ -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 <ClipboardListIcon className={base} />;
case "advisor":
return <BrainCircuitIcon className={base} />;
return <LightbulbIcon className={base} />;
case "computer":
return <MonitorIcon className={base} />;
case "read_skill":
@@ -202,7 +202,9 @@ export const ToolLabel: React.FC<{
}
case "advisor":
return (
<span className="truncate text-sm text-content-secondary">Advisor</span>
<span className="truncate text-[13px] leading-4 text-content-secondary">
Advisor
</span>
);
case "read_skill": {
const skillName = parsed ? asString(parsed.name) : "";