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..7506b5a7b16 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?", ] @@ -77,25 +74,26 @@ 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"); + + 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(); + + 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 +104,47 @@ export const Running: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); + 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", { + name: /Weighing a refactor tradeoff/, + }); + expect(toggle).toHaveAttribute("aria-expanded", "false"); + 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", @@ -123,7 +154,7 @@ export const RunningWithStreamedAdvice: Story = { play: async ({ canvasElement }) => { const canvas = within(canvasElement); 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 +180,21 @@ export const LimitReached: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("Advisor limit reached.")).toBeInTheDocument(); + const toggle = canvas.getByRole("button", { + name: /Advisor limit reached/, + }); + expect(toggle).toHaveAttribute("aria-expanded", "false"); + + 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 +211,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 +237,13 @@ 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. + 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 +258,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 +277,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 +290,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 +299,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 +313,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 +323,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,50 +343,14 @@ export const PlainStringResult: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText(sampleQuestion)).toBeInTheDocument(); + 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(); - }, -}; - -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); + expect(await canvas.findByText(sampleQuestion)).toBeInTheDocument(); }, }; @@ -342,33 +362,22 @@ 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); 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..e8150923980 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx @@ -1,11 +1,9 @@ -import { CircleAlertIcon, TriangleAlertIcon } from "lucide-react"; +import { 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 +14,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 +24,26 @@ 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 = showLimitReached + ? "Advisor limit reached" + : 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} > - - - {({ expanded }) => ( - <> -
-
- - - {isRunning && ( - - {RUNNING_MESSAGE} - - )} - {advisorModelText && ( - - {advisorModelText} - - )} - {remainingUses !== undefined && ( - - {remainingUses.toLocaleString("en-US")} uses left - - )} -
- - {questionText} - -
- {headerStatus} - - - )} - - + + ) : null + } + /> -
- {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} /> ); };