From f9466f5524d3ca92512b0873130d0708314979b3 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 12 Aug 2026 12:50:19 +0000 Subject: [PATCH 1/4] feat: redesign the advisor tool row Surface the advisor call as a single-line status row whose label is the model-supplied intent (falling back to a status verb), and move the full question into the expandable card above the guidance. Drop the live quota counter from the header; the backend now returns an explicit instruction when the per-turn advisor budget is exhausted so the agent stops calling it, and the card's limit callout covers the user-facing case. --- coderd/x/chatd/chatadvisor/guidance.go | 3 + coderd/x/chatd/chatadvisor/runner.go | 1 + coderd/x/chatd/chatadvisor/tool_test.go | 2 +- coderd/x/chatd/chatadvisor/types.go | 4 +- .../tools/AdvisorTool.stories.tsx | 209 ++++++++++-------- .../ChatElements/tools/AdvisorTool.tsx | 168 +++++--------- .../components/ChatElements/tools/Tool.tsx | 8 +- 7 files changed, 182 insertions(+), 213 deletions(-) diff --git a/coderd/x/chatd/chatadvisor/guidance.go b/coderd/x/chatd/chatadvisor/guidance.go index c11cb7aced4..2f5f2bec93d 100644 --- a/coderd/x/chatd/chatadvisor/guidance.go +++ b/coderd/x/chatd/chatadvisor/guidance.go @@ -22,4 +22,7 @@ call with no tools, and returns concise guidance for the parent agent rather than the end user. Provide a brief question, no more than 2000 runes. Summarize context instead of pasting long logs or transcripts. ` + // LimitReachedAdvice is returned when the per-run advisor budget is + // exhausted, telling the parent agent to stop calling the advisor. + LimitReachedAdvice = "The advisor budget for this turn is exhausted. Do not call the advisor again this turn; proceed with your own judgment." ) diff --git a/coderd/x/chatd/chatadvisor/runner.go b/coderd/x/chatd/chatadvisor/runner.go index 2df22118302..da847ce1b0f 100644 --- a/coderd/x/chatd/chatadvisor/runner.go +++ b/coderd/x/chatd/chatadvisor/runner.go @@ -40,6 +40,7 @@ func (rt *Runtime) RunAdvisor( if !rt.tryAcquire() { return AdvisorResult{ Type: ResultTypeLimitReached, + Advice: LimitReachedAdvice, RemainingUses: 0, }, nil } diff --git a/coderd/x/chatd/chatadvisor/tool_test.go b/coderd/x/chatd/chatadvisor/tool_test.go index 0b69ee0abd0..3053d8b2149 100644 --- a/coderd/x/chatd/chatadvisor/tool_test.go +++ b/coderd/x/chatd/chatadvisor/tool_test.go @@ -370,7 +370,7 @@ func TestAdvisorToolReportsLimitReached(t *testing.T) { require.NoError(t, json.Unmarshal([]byte(second.Content), &result)) require.Equal(t, chatadvisor.ResultTypeLimitReached, result.Type) require.Equal(t, 0, result.RemainingUses) - require.Empty(t, result.Advice) + require.Equal(t, chatadvisor.LimitReachedAdvice, result.Advice) require.Empty(t, result.Error) require.Empty(t, result.AdvisorModel) } diff --git a/coderd/x/chatd/chatadvisor/types.go b/coderd/x/chatd/chatadvisor/types.go index 9a47208f0d1..ad2f3c8aeef 100644 --- a/coderd/x/chatd/chatadvisor/types.go +++ b/coderd/x/chatd/chatadvisor/types.go @@ -13,9 +13,9 @@ const ( ResultTypeError ResultType = "error" ) -// AdvisorArgs contains the tool-visible advisor question. type AdvisorArgs struct { - Question string `json:"question" description:"A brief question for the advisor. Must be 2000 runes or fewer. Summarize context instead of pasting long logs or transcripts."` + Question string `json:"question" description:"A brief question for the advisor. Must be 2000 runes or fewer. Summarize context instead of pasting long logs or transcripts."` + ModelIntent *string `json:"model_intent,omitempty" description:"A short, natural-language, present-participle phrase describing why you are consulting the advisor. This is shown to the user as the tool row title, so write it as a standalone action. Use plain English with no underscores or technical jargon. Do not restate the question. Keep it under 100 characters. Good examples: \"Weighing a refactor tradeoff\", \"Checking a migration strategy\", \"Planning a safe rollout\"."` } // AdvisorResult is the structured result returned by the advisor runtime. 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 bbec196bcf7..61fe992d0b7 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.stories.tsx @@ -6,12 +6,9 @@ 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.", + "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 row readable when the advisor includes a long prompt, 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 trailing text, or disappear 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 row should still make the question easy to scan, truncate gracefully, and keep the advisor identity visually distinct from the answer.", ].join(" "); const sampleAdvice = [ @@ -51,11 +48,11 @@ const longAdvice = [ "- Use markdown rendering for prose and code examples.", "- Preserve a subtle metadata footer for debugging and support.", "", - "The dedicated card should still behave like the existing tool cards, including collapse, expansion, and overflow handling for long guidance.", + "The dedicated row should still behave like the existing tool rows, including collapse, expansion, and overflow handling for long guidance.", "", ]).flat(), "## Follow-up questions", - "1. Should the card stay expanded by default?", + "1. Should the row stay expanded by default while running?", "2. Should limit states include remaining uses when the backend provides them?", "3. Should the error state surface the raw provider message or a friendlier summary?", ] @@ -70,6 +67,8 @@ const meta: Meta = { export default meta; type Story = StoryObj; +// Completed advice starts collapsed; expanding reveals the question and the +// markdown guidance in the card. export const SuccessfulAdvice: Story = { args: { status: "completed", @@ -77,25 +76,29 @@ export const SuccessfulAdvice: Story = { result: { type: "advice", advice: sampleAdvice, - advisor_model: "GPT-5 Advisor", + advisor_model: "openai/gpt-5.1", remaining_uses: 3, }, }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText(sampleQuestion)).toBeInTheDocument(); + const toggle = canvas.getByRole("button"); + + // Collapsed by default: the header is a status line and the question + // and advice live in the card until expanded. + expect(toggle).toHaveAttribute("aria-expanded", "false"); + expect(canvas.getByText("Consulted the advisor")).toBeInTheDocument(); + expect(canvas.queryByText(sampleQuestion)).not.toBeInTheDocument(); + expect(canvas.queryByText("Quick summary")).not.toBeInTheDocument(); + + // Model name and quota are not rendered in the header. + expect(canvas.queryByText("openai/gpt-5.1")).not.toBeInTheDocument(); + expect(canvas.queryByText("3 left")).not.toBeInTheDocument(); + + await userEvent.click(toggle); + expect(toggle).toHaveAttribute("aria-expanded", "true"); + expect(await canvas.findByText(sampleQuestion)).toBeInTheDocument(); expect(await canvas.findByText("Quick summary")).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.queryByText( - (_, element) => - element?.textContent?.replace(/\s+/g, " ").trim() === - "Advisor model: GPT-5 Advisor", - ), - ).not.toBeInTheDocument(); }, }; @@ -106,14 +109,49 @@ export const Running: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); + // Running rows open expanded so streamed guidance is visible live. + expect(canvas.getByRole("button")).toHaveAttribute("aria-expanded", "true"); + expect(canvas.getByText("Consulting the advisor")).toBeInTheDocument(); expect(canvas.getByText(sampleQuestion)).toBeInTheDocument(); - expect(canvas.getAllByText("Consulting advisor…")).toHaveLength(1); expect( canvas.getByText("Reviewing context and preparing guidance."), ).toBeInTheDocument(); }, }; +// When the model supplies a model_intent, it is the whole header label, +// matching how the exec tool renders its intent. +export const WithModelIntent: Story = { + args: { + status: "completed", + args: { + question: sampleQuestion, + model_intent: "Weighing a refactor tradeoff", + }, + // The backend surfaces model_intent as a top-level tool field, so the + // story passes it the same way the timeline does. + modelIntent: "Weighing a refactor tradeoff", + result: { + type: "advice", + advice: sampleAdvice, + remaining_uses: 2, + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const toggle = canvas.getByRole("button"); + expect(toggle).toHaveAttribute("aria-expanded", "false"); + expect( + canvas.getByText("Weighing a refactor tradeoff"), + ).toBeInTheDocument(); + expect(canvas.queryByText(/Consulted the advisor/)).not.toBeInTheDocument(); + expect(canvas.queryByText("2 left")).not.toBeInTheDocument(); + + await userEvent.click(toggle); + expect(await canvas.findByText(sampleQuestion)).toBeInTheDocument(); + }, +}; + export const RunningWithStreamedAdvice: Story = { args: { status: "running", @@ -122,8 +160,9 @@ export const RunningWithStreamedAdvice: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); + // Running rows are expanded, so the question shows in the card. expect(canvas.getByText(sampleQuestion)).toBeInTheDocument(); - expect(canvas.getByText("Consulting advisor…")).toBeInTheDocument(); + expect(canvas.getByText("Consulting the advisor")).toBeInTheDocument(); expect( await canvas.findByText( "Use the smaller diff while the advisor is still responding.", @@ -149,14 +188,20 @@ export const LimitReached: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("Advisor limit reached.")).toBeInTheDocument(); + const toggle = canvas.getByRole("button"); + expect(toggle).toHaveAttribute("aria-expanded", "false"); + + // The limit message lives in the body, so it appears once expanded. + await userEvent.click(toggle); + expect( + await canvas.findByText("Advisor limit reached."), + ).toBeInTheDocument(); expect( canvas.getByText( "You have reached the advisor limit for this conversation.", ), ).toBeInTheDocument(); - // Assert the semantic role screen readers rely on to announce the - // limit state. A refactor that drops role="status" should fail here. + // Screen readers announce the limit state via role="status". expect(canvas.getByRole("status")).toBeInTheDocument(); }, }; @@ -173,12 +218,17 @@ export const ErrorState: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("Advisor request failed.")).toBeInTheDocument(); + const toggle = canvas.getByRole("button"); + expect(toggle).toHaveAttribute("aria-expanded", "false"); + + await userEvent.click(toggle); + expect( + await canvas.findByText("Advisor request failed."), + ).toBeInTheDocument(); expect( canvas.getByText("The advisor service is temporarily unavailable."), ).toBeInTheDocument(); - // Assert the semantic role screen readers rely on to announce the - // error state. A refactor that drops role="alert" should fail here. + // Screen readers announce the error state via role="alert". expect(canvas.getByRole("alert")).toBeInTheDocument(); }, }; @@ -194,10 +244,14 @@ export const EmptyQuestion: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("No question provided.")).toBeInTheDocument(); - // Confirm the advice body still renders alongside the blank-question - // fallback, so a future refactor that suppresses the body for empty - // questions cannot pass silently. + // The fallback question is inside the card, hidden while collapsed. + expect(canvas.queryByText("No question provided.")).not.toBeInTheDocument(); + await userEvent.click(canvas.getByRole("button")); + expect( + await canvas.findByText("No question provided."), + ).toBeInTheDocument(); + // The advice body still renders after expanding, so a refactor that + // suppresses the body for empty questions cannot pass silently. expect(await canvas.findByText("Quick summary")).toBeInTheDocument(); }, }; @@ -212,8 +266,9 @@ export const EmptyAdvice: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); + await userEvent.click(canvas.getByRole("button")); expect( - canvas.getByText("Advisor returned no guidance."), + await canvas.findByText("Advisor returned no guidance."), ).toBeInTheDocument(); expect(canvas.queryByText("No guidance")).not.toBeInTheDocument(); }, @@ -230,7 +285,10 @@ export const BlankError: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("Advisor request failed.")).toBeInTheDocument(); + await userEvent.click(canvas.getByRole("button")); + expect( + await canvas.findByText("Advisor request failed."), + ).toBeInTheDocument(); expect( canvas.getByText("Advisor could not return guidance."), ).toBeInTheDocument(); @@ -240,7 +298,7 @@ export const BlankError: Story = { // Mirrors the backend path where a tool call is marked execution-failed // (status === "error") without a structured result payload. The renderer -// must fold the error status into the error signal so the card surfaces +// must fold the error status into the error signal so the row surfaces // the failure instead of falling through to "Advisor returned no guidance". export const StatusErrorWithoutResult: Story = { args: { @@ -249,7 +307,10 @@ export const StatusErrorWithoutResult: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("Advisor request failed.")).toBeInTheDocument(); + await userEvent.click(canvas.getByRole("button")); + expect( + await canvas.findByText("Advisor request failed."), + ).toBeInTheDocument(); expect( canvas.getByText("Advisor could not return guidance."), ).toBeInTheDocument(); @@ -260,8 +321,8 @@ export const StatusErrorWithoutResult: Story = { // Mirrors the backend path where a tool call is marked execution-failed // (status === "error") and the result payload is a raw string instead of // a structured object. AdvisorRenderer must route the string through the -// `errorMessage` branch so the failure surfaces in the error card rather -// than being rendered as advice text. +// `errorMessage` branch so the failure surfaces rather than being rendered +// as advice text. export const StatusErrorWithStringResult: Story = { args: { status: "error", @@ -270,7 +331,10 @@ export const StatusErrorWithStringResult: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("Advisor request failed.")).toBeInTheDocument(); + await userEvent.click(canvas.getByRole("button")); + expect( + await canvas.findByText("Advisor request failed."), + ).toBeInTheDocument(); expect(canvas.getByText("Connection timed out")).toBeInTheDocument(); expect(canvas.getByRole("alert")).toBeInTheDocument(); }, @@ -287,53 +351,20 @@ export const PlainStringResult: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText(sampleQuestion)).toBeInTheDocument(); + // Collapsed: the question is inside the card until expanded. + expect(canvas.queryByText(sampleQuestion)).not.toBeInTheDocument(); + await userEvent.click(canvas.getByRole("button")); expect( await canvas.findByText( "Prefer extracting a shared helper once two renderers need it.", ), ).toBeInTheDocument(); + expect(await canvas.findByText(sampleQuestion)).toBeInTheDocument(); }, }; -export const LongAdvice: Story = { - args: { - status: "completed", - args: { question: sampleQuestion }, - 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"); - - expect(toggle).toHaveAttribute("aria-expanded", "true"); - await userEvent.click(toggle); - expect(toggle).toHaveAttribute("aria-expanded", "false"); - expect(canvas.queryByText("Follow-up questions")).not.toBeInTheDocument(); - - await userEvent.click(toggle); - expect(toggle).toHaveAttribute("aria-expanded", "true"); - expect(await canvas.findByText("Follow-up questions")).toBeInTheDocument(); - - const scrollArea = canvas.getByTestId("advisor-tool-scroll-area"); - const viewport = scrollArea.querySelector( - "[data-radix-scroll-area-viewport]", - ); - if (!(viewport instanceof HTMLElement)) { - throw new globalThis.Error("Expected advisor scroll viewport."); - } - - viewport.scrollTop = viewport.scrollHeight; - viewport.dispatchEvent(new Event("scroll")); - expect(viewport.scrollTop).toBeGreaterThan(0); - }, -}; - +// A long question renders in full inside the card; the long guidance shares +// the same scrollable region. export const LongAdviceLongQuestion: Story = { name: "Long Advice + long question", args: { @@ -342,33 +373,23 @@ export const LongAdviceLongQuestion: Story = { result: { type: "advice", advice: longAdvice, - advisor_model: "GPT-5 Advisor", + advisor_model: "openai/gpt-5.1", 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); + // Collapsed: neither the question nor the advice is visible. expect(toggle).toHaveAttribute("aria-expanded", "false"); - expect(question.getBoundingClientRect().height).toBeLessThan( - expandedQuestionHeight, - ); + expect(canvas.queryByText(longQuestion)).not.toBeInTheDocument(); + expect(canvas.queryByText("12 left")).not.toBeInTheDocument(); 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(longQuestion)).toBeInTheDocument(); 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 12cf7200c66..9f5e310762e 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx @@ -1,11 +1,8 @@ -import { CircleAlertIcon, 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 { ToolCall } from "./ToolCall"; -import { ToolLabel } from "./ToolLabel"; -import type { ToolStatus } from "./utils"; +import { formatModelIntentLabel, type ToolStatus } from "./utils"; export type AdvisorToolResultType = "advice" | "limit_reached" | "error"; @@ -16,17 +13,9 @@ type AdvisorToolProps = { resultType?: AdvisorToolResultType; advice?: string; errorMessage?: string; - advisorModel?: string; - remainingUses?: number; + modelIntent?: string; }; -const FALLBACK_QUESTION = "No question provided."; -const FALLBACK_ERROR = "Advisor could not return guidance."; -const LIMIT_REACHED_MESSAGE = - "You have reached the advisor limit for this conversation."; -const RUNNING_MESSAGE = "Consulting advisor…"; -const EMPTY_ADVICE_MESSAGE = "Advisor returned no guidance."; - export const AdvisorTool: React.FC = ({ question, status, @@ -34,24 +23,25 @@ export const AdvisorTool: React.FC = ({ resultType, advice, errorMessage, - advisorModel, - remainingUses, + modelIntent, }) => { - const questionText = question.trim() || FALLBACK_QUESTION; + const questionText = question.trim() || "No question provided."; const adviceText = advice?.trim() ?? ""; - const advisorModelText = advisorModel?.trim() ?? ""; - const effectiveErrorMessage = errorMessage?.trim() || FALLBACK_ERROR; + const effectiveErrorMessage = + errorMessage?.trim() || "Advisor could not return guidance."; const isRunning = status === "running"; const showLimitReached = resultType === "limit_reached"; const showError = isError || resultType === "error"; - const headerStatus = showLimitReached ? ( - - ) : showError ? ( - - ) : ( - - ); + const intent = formatModelIntentLabel(modelIntent); + const label = + intent && !showError + ? intent + : isRunning + ? "Consulting the advisor" + : showError + ? "Failed to consult the advisor" + : "Consulted the advisor"; return ( = ({ isError={showError} errorMessage={effectiveErrorMessage} hasContent - defaultExpanded + defaultExpanded={isRunning} + ariaLabel={(expanded) => + expanded ? "Collapse advisor guidance" : "Expand advisor guidance" + } > - - - {({ expanded }) => ( - <> -
-
- - - {isRunning && ( - - {RUNNING_MESSAGE} - - )} - {advisorModelText && ( - - {advisorModelText} - - )} - {remainingUses !== undefined && ( - - {remainingUses.toLocaleString("en-US")} uses left - - )} -
- - {questionText} - -
- {headerStatus} - - - )} - - + + + + {label} + + + + -
- {isRunning && adviceText.length === 0 ? ( -
- Reviewing context and preparing guidance. -
- ) : showLimitReached ? ( -
- -
-

Advisor limit reached.

-

- {LIMIT_REACHED_MESSAGE} +

+

+ {questionText} +

+
+ {showError ? ( +
+

+ Advisor request failed.

-
-
- ) : showError ? ( -
- -
-

Advisor request failed.

-

+

{effectiveErrorMessage}

-
- ) : ( -
-
- - Advice - + ) : showLimitReached ? ( +
+

+ Advisor limit reached. +

+

+ You have reached the advisor limit for this conversation. +

+
+ ) : isRunning && adviceText.length === 0 ? ( +
+ Reviewing context and preparing guidance.
+ ) : ( - {adviceText || EMPTY_ADVICE_MESSAGE} + {adviceText || "Advisor returned no guidance."} -
- )} + )} +
diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx index f96212bedf3..299716a156e 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx @@ -698,6 +698,7 @@ const AdvisorRenderer: FC = ({ status, result, isError, + modelIntent, }) => { const parsedArgs = parseArgs(args); const question = parsedArgs ? asString(parsedArgs.question) : ""; @@ -723,10 +724,6 @@ const AdvisorRenderer: FC = ({ (typeof result === "string" && (hasError || resolvedResultType === "error") ? result : ""); - const advisorModel = rec ? asString(rec.advisor_model) : ""; - const remainingUses = rec - ? asNumber(rec.remaining_uses, { parseString: true }) - : undefined; return ( = ({ resultType={resolvedResultType} advice={advice} errorMessage={errorMessage || undefined} - advisorModel={advisorModel || undefined} - remainingUses={remainingUses} + modelIntent={modelIntent} /> ); }; From 424feb45a353162b1d7f8ad00fd2e0acd95fb762 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 12 Aug 2026 13:19:37 +0000 Subject: [PATCH 2/4] fix(site/src/pages/AgentsPage): surface advisor limit in header, drop FE4 comments When the advisor budget is exhausted, the collapsed row read as a success ("Consulted the advisor") with the warning only inside the expandable card. Label the header "Advisor limit reached" with a warning icon instead, and render it through ToolCall.Header's secondaryLabel slot. Also remove story comments that restate the assertions below them, per FE4. --- .../tools/AdvisorTool.stories.tsx | 17 ++++---------- .../ChatElements/tools/AdvisorTool.tsx | 23 +++++++++++-------- 2 files changed, 17 insertions(+), 23 deletions(-) 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 61fe992d0b7..57256f59f2f 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.stories.tsx @@ -67,8 +67,6 @@ const meta: Meta = { export default meta; type Story = StoryObj; -// Completed advice starts collapsed; expanding reveals the question and the -// markdown guidance in the card. export const SuccessfulAdvice: Story = { args: { status: "completed", @@ -84,14 +82,11 @@ export const SuccessfulAdvice: Story = { const canvas = within(canvasElement); const toggle = canvas.getByRole("button"); - // Collapsed by default: the header is a status line and the question - // and advice live in the card until expanded. expect(toggle).toHaveAttribute("aria-expanded", "false"); expect(canvas.getByText("Consulted the advisor")).toBeInTheDocument(); expect(canvas.queryByText(sampleQuestion)).not.toBeInTheDocument(); expect(canvas.queryByText("Quick summary")).not.toBeInTheDocument(); - // Model name and quota are not rendered in the header. expect(canvas.queryByText("openai/gpt-5.1")).not.toBeInTheDocument(); expect(canvas.queryByText("3 left")).not.toBeInTheDocument(); @@ -109,7 +104,6 @@ export const Running: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - // Running rows open expanded so streamed guidance is visible live. expect(canvas.getByRole("button")).toHaveAttribute("aria-expanded", "true"); expect(canvas.getByText("Consulting the advisor")).toBeInTheDocument(); expect(canvas.getByText(sampleQuestion)).toBeInTheDocument(); @@ -160,7 +154,6 @@ export const RunningWithStreamedAdvice: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - // Running rows are expanded, so the question shows in the card. expect(canvas.getByText(sampleQuestion)).toBeInTheDocument(); expect(canvas.getByText("Consulting the advisor")).toBeInTheDocument(); expect( @@ -191,7 +184,10 @@ export const LimitReached: Story = { const toggle = canvas.getByRole("button"); expect(toggle).toHaveAttribute("aria-expanded", "false"); - // The limit message lives in the body, so it appears once expanded. + // The exhausted budget is visible without expanding: the header shows a + // limit-specific label and warning icon rather than an apparent success. + expect(canvas.getByText("Advisor limit reached")).toBeInTheDocument(); + await userEvent.click(toggle); expect( await canvas.findByText("Advisor limit reached."), @@ -244,7 +240,6 @@ export const EmptyQuestion: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - // The fallback question is inside the card, hidden while collapsed. expect(canvas.queryByText("No question provided.")).not.toBeInTheDocument(); await userEvent.click(canvas.getByRole("button")); expect( @@ -351,7 +346,6 @@ export const PlainStringResult: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - // Collapsed: the question is inside the card until expanded. expect(canvas.queryByText(sampleQuestion)).not.toBeInTheDocument(); await userEvent.click(canvas.getByRole("button")); expect( @@ -363,8 +357,6 @@ export const PlainStringResult: Story = { }, }; -// A long question renders in full inside the card; the long guidance shares -// the same scrollable region. export const LongAdviceLongQuestion: Story = { name: "Long Advice + long question", args: { @@ -381,7 +373,6 @@ export const LongAdviceLongQuestion: Story = { const canvas = within(canvasElement); const toggle = canvas.getByRole("button"); - // Collapsed: neither the question nor the advice is visible. expect(toggle).toHaveAttribute("aria-expanded", "false"); expect(canvas.queryByText(longQuestion)).not.toBeInTheDocument(); expect(canvas.queryByText("12 left")).not.toBeInTheDocument(); diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx index 9f5e310762e..91d96fd17d1 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx @@ -1,3 +1,4 @@ +import { TriangleAlertIcon } from "lucide-react"; import type React from "react"; import { ScrollArea } from "#/components/ScrollArea/ScrollArea"; import { Response } from "../Response"; @@ -34,8 +35,9 @@ export const AdvisorTool: React.FC = ({ const showError = isError || resultType === "error"; const intent = formatModelIntentLabel(modelIntent); - const label = - intent && !showError + const label = showLimitReached + ? "Advisor limit reached" + : intent && !showError ? intent : isRunning ? "Consulting the advisor" @@ -55,14 +57,15 @@ export const AdvisorTool: React.FC = ({ expanded ? "Collapse advisor guidance" : "Expand advisor guidance" } > - - - - {label} - - - - + + ) : null + } + /> Date: Wed, 12 Aug 2026 13:56:13 +0000 Subject: [PATCH 3/4] fix(site/src/pages/AgentsPage): let advisor row accessible name reflect visible label The custom aria-label overrode the header button's descendant text, so screen readers announced only "Expand/Collapse advisor guidance" and lost the visible intent or status (including the limit-reached state). Drop the override so the name derives from the visible label; aria-expanded still conveys toggle state. --- .../tools/AdvisorTool.stories.tsx | 20 ++++++++++--------- .../ChatElements/tools/AdvisorTool.tsx | 3 --- 2 files changed, 11 insertions(+), 12 deletions(-) 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 57256f59f2f..81968d0708c 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.stories.tsx @@ -133,11 +133,12 @@ export const WithModelIntent: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - const toggle = canvas.getByRole("button"); + // The toggle's accessible name comes from the visible label (no + // aria-label override), so assistive tech announces the intent. + const toggle = canvas.getByRole("button", { + name: /Weighing a refactor tradeoff/, + }); expect(toggle).toHaveAttribute("aria-expanded", "false"); - expect( - canvas.getByText("Weighing a refactor tradeoff"), - ).toBeInTheDocument(); expect(canvas.queryByText(/Consulted the advisor/)).not.toBeInTheDocument(); expect(canvas.queryByText("2 left")).not.toBeInTheDocument(); @@ -181,12 +182,13 @@ export const LimitReached: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - const toggle = canvas.getByRole("button"); - expect(toggle).toHaveAttribute("aria-expanded", "false"); - // The exhausted budget is visible without expanding: the header shows a - // limit-specific label and warning icon rather than an apparent success. - expect(canvas.getByText("Advisor limit reached")).toBeInTheDocument(); + // limit-specific label and warning icon rather than an apparent success, + // and the toggle's accessible name includes that status text. + const toggle = canvas.getByRole("button", { + name: /Advisor limit reached/, + }); + expect(toggle).toHaveAttribute("aria-expanded", "false"); await userEvent.click(toggle); expect( diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx index 91d96fd17d1..e8150923980 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx @@ -53,9 +53,6 @@ export const AdvisorTool: React.FC = ({ errorMessage={effectiveErrorMessage} hasContent defaultExpanded={isRunning} - ariaLabel={(expanded) => - expanded ? "Collapse advisor guidance" : "Expand advisor guidance" - } > Date: Wed, 12 Aug 2026 14:12:03 +0000 Subject: [PATCH 4/4] refactor(site/src/pages/AgentsPage): drop FE4-violating assertion narration Remove the two comment blocks added alongside the limit-header and accessible-name assertions; they restated what the queries verify. The accessibility-role and backend-path comments that add non-obvious context stay. --- .../components/ChatElements/tools/AdvisorTool.stories.tsx | 5 ----- 1 file changed, 5 deletions(-) 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 81968d0708c..7506b5a7b16 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.stories.tsx @@ -133,8 +133,6 @@ export const WithModelIntent: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - // The toggle's accessible name comes from the visible label (no - // aria-label override), so assistive tech announces the intent. const toggle = canvas.getByRole("button", { name: /Weighing a refactor tradeoff/, }); @@ -182,9 +180,6 @@ export const LimitReached: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - // The exhausted budget is visible without expanding: the header shows a - // limit-specific label and warning icon rather than an apparent success, - // and the toggle's accessible name includes that status text. const toggle = canvas.getByRole("button", { name: /Advisor limit reached/, });