From fd6af8f832ea30e4680af6d3ace2b2f4e17f3d47 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Thu, 13 Aug 2026 01:10:12 -0500 Subject: [PATCH] feat(desktop): render the clarify (Recommended) label in tertiary text The card reads the labelled choices off the gateway request rather than the raw tool args -- the backend applies the label there, and the card only mounts once the request exists, so the args are a hydration-race fallback. RECOMMENDED_LABEL and bareChoice live in the clarify store so the component and the choice-length guard share one definition; without the guard a long option could be dropped for length the label added. --- .../assistant-ui/clarify-tool.test.tsx | 33 +++++++++++++++++ .../components/assistant-ui/clarify-tool.tsx | 35 +++++++++++++++++-- apps/desktop/src/store/clarify.ts | 14 +++++++- 3 files changed, 78 insertions(+), 4 deletions(-) diff --git a/apps/desktop/src/components/assistant-ui/clarify-tool.test.tsx b/apps/desktop/src/components/assistant-ui/clarify-tool.test.tsx index ecb4c6c2faaae..0b80ad68d4d54 100644 --- a/apps/desktop/src/components/assistant-ui/clarify-tool.test.tsx +++ b/apps/desktop/src/components/assistant-ui/clarify-tool.test.tsx @@ -297,6 +297,39 @@ describe('ClarifyTool keyboard navigation', () => { }) }) +describe('ClarifyTool recommended option', () => { + it('dims the (Recommended) label and answers with the choice the backend sent', async () => { + const request = vi.fn().mockResolvedValue({ ok: true }) + + $activeSessionId.set('session-1') + $gateway.set({ request } as never) + setClarifyRequest({ + choices: ['staging (Recommended)', 'production'], + question: 'Which deployment target?', + requestId: 'request-1', + sessionId: 'session-1' + }) + renderClarify() + + const recommended = screen.getByRole('button', { name: /staging/ }) + + // The label rides in its own muted span so the option text still reads first. + expect(recommended.querySelector('.text-\\(--ui-text-tertiary\\)')?.textContent).toBe('(Recommended)') + + fireEvent.click(recommended) + fireEvent.keyDown(window, { key: 'Enter' }) + + // The decorated string goes back verbatim; the tool strips the label before + // the agent ever sees the answer. + await waitFor(() => { + expect(request).toHaveBeenCalledWith('clarify.respond', { + answer: 'staging (Recommended)', + request_id: 'request-1' + }) + }) + }) +}) + describe('ClarifyTool pending marker', () => { it('marks a live choices card with its row count so type-to-focus yields exactly its keys', () => { renderLiveClarify() diff --git a/apps/desktop/src/components/assistant-ui/clarify-tool.tsx b/apps/desktop/src/components/assistant-ui/clarify-tool.tsx index 57e861042077f..f970c6d5727fb 100644 --- a/apps/desktop/src/components/assistant-ui/clarify-tool.tsx +++ b/apps/desktop/src/components/assistant-ui/clarify-tool.tsx @@ -25,7 +25,14 @@ import { useI18n } from '@/i18n' import { triggerHaptic } from '@/lib/haptics' import { CircleLetterA, Loader2, MessageQuestion } from '@/lib/icons' import { cn } from '@/lib/utils' -import { clearClarifyRequest, normalizeChoices, sessionClarifyRequest, warnDroppedChoices } from '@/store/clarify' +import { + bareChoice, + clearClarifyRequest, + normalizeChoices, + RECOMMENDED_LABEL, + sessionClarifyRequest, + warnDroppedChoices +} from '@/store/clarify' import { $gateway } from '@/store/gateway' import { notifyError } from '@/store/notifications' @@ -87,6 +94,22 @@ export function readClarifyResult(result: unknown): ClarifyResult { const letterFor = (index: number): string => String.fromCharCode(65 + index) +// The backend tags the agent's preferred option (`mark_recommended`); the card +// renders the label in tertiary text so the option itself still reads first. +function ChoiceLabel({ choice }: { choice: string }) { + const bare = bareChoice(choice) + + if (bare === choice) { + return <>{choice} + } + + return ( + <> + {bare} {RECOMMENDED_LABEL} + + ) +} + const OPTION_ROW_CLASS = 'flex w-full items-start gap-2 rounded-[0.25rem] px-1.5 py-1 text-left disabled:cursor-not-allowed disabled:opacity-50' @@ -182,7 +205,9 @@ function ChoiceButton({ type="button" > - {choice} + + + ) @@ -301,7 +326,11 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) { const question = fromArgs.question || matchingRequest?.question || '' const choices = useMemo( - () => fromArgs.choices ?? matchingRequest?.choices ?? [], + // Prefer the gateway request's choices over the raw tool args: the backend + // labels the recommended option there (`mark_recommended`), and the card + // only renders once `matchingRequest` exists, so the args are a fallback + // for a hydration race, not the normal path. + () => matchingRequest?.choices ?? fromArgs.choices ?? [], [fromArgs.choices, matchingRequest?.choices] ) diff --git a/apps/desktop/src/store/clarify.ts b/apps/desktop/src/store/clarify.ts index 6cdc41eb44aeb..f98d6e4036861 100644 --- a/apps/desktop/src/store/clarify.ts +++ b/apps/desktop/src/store/clarify.ts @@ -10,6 +10,17 @@ export interface ClarifyRequest { sessionId: string | null } +/** + * The backend labels the agent's recommended option by appending this to the + * first choice (`tools/clarify_tool.py::mark_recommended`). The renderer never + * writes it — it only styles it, and discounts it when measuring a choice so a + * long option isn't dropped for length the label added. + */ +export const RECOMMENDED_LABEL = '(Recommended)' + +export const bareChoice = (choice: string): string => + choice.endsWith(RECOMMENDED_LABEL) ? choice.slice(0, -RECOMMENDED_LABEL.length).trim() : choice + /** * Validate and normalize a choices array. * @@ -23,7 +34,8 @@ export function normalizeChoices(choices: unknown): string[] { } return choices.filter( - (c): c is string => typeof c === 'string' && c.trim().length > 0 && c.length <= 200 && !c.includes('\n') + (c): c is string => + typeof c === 'string' && c.trim().length > 0 && bareChoice(c).length <= 200 && !c.includes('\n') ) }