From 2fb03118bbf25fdcd9d4f521edc84cea9af4d5e2 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sat, 1 Aug 2026 18:25:53 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=92=AC=20feat:=20Interim=20Progress=20Car?= =?UTF-8?q?d=20for=20Streaming=20Q&A=20Calls=20(#14576)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 💬 feat: Interim Progress Card for Streaming ask_user_question Calls * 🔍 fix: Match Progress Card Against Every Live Ask Pause, Not Newest Only * ⏳ feat: Hold Streaming Cursor Under Answered Question While Resume Is In Flight --- .../Messages/Content/AskUserQuestionCall.tsx | 114 +++++++++++------- .../Content/AskUserQuestionProgress.tsx | 71 +++++++++++ .../components/Chat/Messages/Content/Part.tsx | 1 + .../__tests__/AskUserQuestionCall.test.tsx | 48 ++++++++ .../AskUserQuestionProgress.test.tsx | 100 +++++++++++++++ client/src/utils/approval.spec.ts | 42 +++++++ client/src/utils/approval.ts | 37 ++++++ 7 files changed, 370 insertions(+), 43 deletions(-) create mode 100644 client/src/components/Chat/Messages/Content/AskUserQuestionProgress.tsx create mode 100644 client/src/components/Chat/Messages/Content/__tests__/AskUserQuestionProgress.test.tsx diff --git a/client/src/components/Chat/Messages/Content/AskUserQuestionCall.tsx b/client/src/components/Chat/Messages/Content/AskUserQuestionCall.tsx index 8dba7f078d..535cf93338 100644 --- a/client/src/components/Chat/Messages/Content/AskUserQuestionCall.tsx +++ b/client/src/components/Chat/Messages/Content/AskUserQuestionCall.tsx @@ -1,6 +1,9 @@ import { MessageCircleQuestion, TriangleAlert } from 'lucide-react'; import { getSubmittedAskAnswer, parseAskUserQuestionArgs } from '~/utils/approval'; +import AskUserQuestionProgress from './AskUserQuestionProgress'; +import EmptyText from './Parts/EmptyText'; import { useLocalize } from '~/hooks'; +import Container from './Container'; /** * Static rendering of a COMPLETED (or abandoned) `ask_user_question` tool call — @@ -15,12 +18,14 @@ export default function AskUserQuestionCall({ toolCallId, isSubmitting = false, failed = false, + showCursor = false, }: { args: string | Record | undefined; output: string; toolCallId?: string; isSubmitting?: boolean; failed?: boolean; + showCursor?: boolean; }) { const localize = useLocalize(); const question = parseAskUserQuestionArgs(args); @@ -37,33 +42,51 @@ export default function AskUserQuestionCall({ * While the turn is live and unanswered, the INTERACTIVE card (rendered from * the pendingAction's synthetic part) owns the question UI — rendering the * durable record too would duplicate it with a misleading "no answer" line. - * Once the user answers, the submit handler stamps `output` onto this part, - * so the record takes over immediately; an abandoned pause only shows its - * "no answer" state after the turn is no longer submitting. + * Until that pause actually starts (args still streaming, interrupt not yet + * delivered) the progress card fills the gap. Once the user answers, the + * submit handler stamps `output` onto this part, so the record takes over + * immediately; an abandoned pause only shows its "no answer" state after + * the turn is no longer submitting. */ if (!answered && !failed && isSubmitting) { - return null; + return ; } + /** + * The run resumes the moment the pause resolves (an answer submits, or a + * schema-rejected call auto-retries), but its first token takes a beat to + * arrive — hold the streaming cursor under the settled card so the turn + * never looks stalled between the answer and the resumed text. + */ + const resumingCursor = + isSubmitting && showCursor ? ( + + + + ) : null; + if (failed) { return ( -
-
-
- {question?.question != null && ( -

- {question.question} + <> +

+
+
+ {question?.question != null && ( +

+ {question.question} +

+ )} +

+ {localize('com_ui_question_failed_description')}

- )} -

- {localize('com_ui_question_failed_description')} -

-
+
+ {resumingCursor} + ); } @@ -88,29 +111,34 @@ export default function AskUserQuestionCall({ const answerLabel = exactLabel ?? mappedMultiLabel ?? effectiveOutput; return ( -
-
-
+ {resumingCursor} + ); } diff --git a/client/src/components/Chat/Messages/Content/AskUserQuestionProgress.tsx b/client/src/components/Chat/Messages/Content/AskUserQuestionProgress.tsx new file mode 100644 index 0000000000..228d9bda0b --- /dev/null +++ b/client/src/components/Chat/Messages/Content/AskUserQuestionProgress.tsx @@ -0,0 +1,71 @@ +import { useContext } from 'react'; +import { MessageCircleQuestion } from 'lucide-react'; +import { collectLiveAskToolCallIds } from '~/utils/approval'; +import { useGetMessagesByConvoId } from '~/data-provider'; +import { ChatContext } from '~/Providers/ChatContext'; +import parseJsonField from './Parts/parseJsonField'; +import { useLocalize } from '~/hooks'; + +/** + * Interim card for an `ask_user_question` call whose pause hasn't gone + * interactive yet: the window between the model starting the call and the + * interrupt's synthetic part arriving (which hands the UI to the popover / + * interactive card). Without it a long question, many options, or several + * parallel questions leave a dead gap after the last streamed token. + * + * The question text streams in live: `question` is the schema's first (and + * only required) property, so providers emit it as the first args key and + * `parseJsonField`'s partial-JSON path can render it delta by delta. + * + * Mounted only for a live, unanswered call ({@link AskUserQuestionCall} + * gates on `isSubmitting`), so the live-ask subscription below never runs + * for the settled cards in history. + */ +export default function AskUserQuestionProgress({ + args, + toolCallId, +}: { + args: string | Record | undefined; + toolCallId?: string; +}) { + const localize = useLocalize(); + const conversationId = useContext(ChatContext)?.conversation?.conversationId; + const enabled = conversationId != null && conversationId !== 'new'; + const { data: livePauses } = useGetMessagesByConvoId(enabled ? conversationId : '', { + enabled, + select: collectLiveAskToolCallIds, + }); + const question = parseJsonField(args, 'question'); + + /** + * THIS call's pause went interactive: the popover (or the interactive card) + * now owns the question's UI. Checked against every live pause, not just the + * newest, so a sibling pause arriving later can never resurrect this card + * next to its own interactive one. An unattributed live ask (no + * `tool_call_id` on older payloads) can't be matched to a call, so treat it + * as owning every placeholder rather than duplicating the question on screen. + */ + const interactive = + livePauses != null && + (livePauses.hasUnattributed || (toolCallId != null && livePauses.ids.includes(toolCallId))); + if (interactive) { + return null; + } + + return ( +
+
+
+ {question.length > 0 ? ( +

{question}

+ ) : ( + + ); +} diff --git a/client/src/components/Chat/Messages/Content/Part.tsx b/client/src/components/Chat/Messages/Content/Part.tsx index e06aedf44c..86274c750a 100644 --- a/client/src/components/Chat/Messages/Content/Part.tsx +++ b/client/src/components/Chat/Messages/Content/Part.tsx @@ -238,6 +238,7 @@ const Part = memo(function Part({ output={typeof toolCall.output === 'string' ? toolCall.output : ''} toolCallId={toolCall.id} isSubmitting={isSubmitting} + showCursor={showCursor} failed={'inputValidationError' in toolCall && toolCall.inputValidationError === true} /> ); diff --git a/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestionCall.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestionCall.test.tsx index c206af61a0..6909e2904e 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestionCall.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestionCall.test.tsx @@ -26,12 +26,35 @@ jest.mock('~/utils/approval', () => ({ }, })); +jest.mock('../AskUserQuestionProgress', () => ({ + __esModule: true, + default: () => { + const { createElement } = jest.requireActual('react'); + return createElement('div', { 'data-testid': 'ask-progress' }); + }, +})); + +jest.mock('../Container', () => ({ + __esModule: true, + default: ({ children }: { children: React.ReactNode }) => { + const { createElement } = jest.requireActual('react'); + return createElement('div', null, children); + }, +})); + describe('AskUserQuestionCall', () => { const args = JSON.stringify({ question: 'How would you like me to get the data?', options: [{ label: 'Use public data', value: 'public' }], }); + test('renders the progress card while the call is live and unanswered', () => { + render(); + + expect(screen.getByTestId('ask-progress')).toBeInTheDocument(); + expect(screen.queryByText('You answered:')).not.toBeInTheDocument(); + }); + test('renders a successful tool result as the user answer', () => { render(); @@ -39,6 +62,31 @@ describe('AskUserQuestionCall', () => { expect(screen.getByText('Use public data')).toBeInTheDocument(); }); + test('holds the streaming cursor under the answered card while the resume is in flight', () => { + const { container } = render( + , + ); + + expect(screen.getByText('You answered:')).toBeInTheDocument(); + expect(container.querySelector('.result-thinking')).not.toBeNull(); + }); + + test('shows no cursor once the record is not the streaming tail', () => { + const settled = render(); + expect(settled.container.querySelector('.result-thinking')).toBeNull(); + + const midStream = render( + , + ); + expect(midStream.container.querySelector('.result-thinking')).toBeNull(); + }); + test('renders schema rejection as an internal question failure, not a user answer', () => { const output = 'Error processing tool: Received tool input did not match expected schema ' + diff --git a/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestionProgress.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestionProgress.test.tsx new file mode 100644 index 0000000000..5f96dbc67f --- /dev/null +++ b/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestionProgress.test.tsx @@ -0,0 +1,100 @@ +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import AskUserQuestionProgress from '../AskUserQuestionProgress'; + +const translations: Record = { + com_ui_asking: 'Asking', +}; + +jest.mock('~/hooks', () => ({ + useLocalize: () => (key: string) => translations[key] ?? key, +})); + +jest.mock('~/Providers/ChatContext', () => { + const { createContext } = jest.requireActual('react'); + return { + ChatContext: createContext({ conversation: { conversationId: 'convo-1' } }), + }; +}); + +let mockLivePauses: { ids: string[]; hasUnattributed: boolean } = { + ids: [], + hasUnattributed: false, +}; + +jest.mock('~/data-provider', () => ({ + useGetMessagesByConvoId: () => ({ data: mockLivePauses }), +})); + +describe('AskUserQuestionProgress', () => { + beforeEach(() => { + mockLivePauses = { ids: [], hasUnattributed: false }; + }); + + test('streams the question text from partial args', () => { + render( + , + ); + + expect(screen.getByText('Asking')).toBeInTheDocument(); + expect(screen.getByText('Which environment should I dep')).toBeInTheDocument(); + }); + + test('decodes JSON escapes in the streaming question', () => { + render( + , + ); + + expect(screen.getByText('Café or "bar"')).toBeInTheDocument(); + }); + + test('renders a skeleton line before any question text streams', () => { + render(); + + expect(screen.getByText('Asking')).toBeInTheDocument(); + expect(screen.getByRole('status')).toBeInTheDocument(); + }); + + test('hides once the interactive pause for this call is live', () => { + mockLivePauses = { ids: ['call_1'], hasUnattributed: false }; + + const { container } = render( + , + ); + + expect(container).toBeEmptyDOMElement(); + }); + + test('hides for an unattributed live pause (no tool_call_id on the payload)', () => { + mockLivePauses = { ids: [], hasUnattributed: true }; + + const { container } = render( + , + ); + + expect(container).toBeEmptyDOMElement(); + }); + + test("stays visible while a DIFFERENT call's pause is interactive", () => { + mockLivePauses = { ids: ['call_1'], hasUnattributed: false }; + + render( + , + ); + + expect(screen.getByText('Second question?')).toBeInTheDocument(); + }); + + test('hides when its own pause is live alongside a newer sibling pause', () => { + mockLivePauses = { ids: ['call_1', 'call_2'], hasUnattributed: false }; + + const { container } = render( + , + ); + + expect(container).toBeEmptyDOMElement(); + }); +}); diff --git a/client/src/utils/approval.spec.ts b/client/src/utils/approval.spec.ts index f41c7775c8..02a313ff22 100644 --- a/client/src/utils/approval.spec.ts +++ b/client/src/utils/approval.spec.ts @@ -11,6 +11,7 @@ import { resolveAskUserQuestionPart, getSubmittedAskAnswer, findLiveAskUserQuestion, + collectLiveAskToolCallIds, isAnsweredAskUserQuestionPart, splitOtherOption, } from './approval'; @@ -477,6 +478,47 @@ describe('findLiveAskUserQuestion', () => { }); }); +describe('collectLiveAskToolCallIds', () => { + const attributedAsk = (actionId: string, toolCallId: string) => + askAction({ + actionId, + payload: { + type: 'ask_user_question', + question: { question: 'Q?' }, + tool_call_id: toolCallId, + }, + }); + + it('collects every live pause id, not just the newest, and drops answered ones', () => { + const first = applyPendingAction(msg({ content: [] }), attributedAsk('a-first', 'call_1')); + const both = applyPendingAction(first, attributedAsk('a-second', 'call_2')); + + expect(collectLiveAskToolCallIds([both])).toEqual({ + ids: ['call_1', 'call_2'], + hasUnattributed: false, + }); + + resolveAskUserQuestionPart(both, 'a-first', 'Ada'); + + // `both` is the pre-answer copy — the answered pause must still drop out. + expect(collectLiveAskToolCallIds([both])).toEqual({ + ids: ['call_2'], + hasUnattributed: false, + }); + }); + + it('flags unattributed pauses and handles non-array input', () => { + const unattributed = applyPendingAction( + msg({ content: [] }), + askAction({ actionId: 'a-unattributed' }), + ); + + expect(collectLiveAskToolCallIds([unattributed])).toEqual({ ids: [], hasUnattributed: true }); + expect(collectLiveAskToolCallIds(null)).toEqual({ ids: [], hasUnattributed: false }); + expect(collectLiveAskToolCallIds(undefined)).toEqual({ ids: [], hasUnattributed: false }); + }); +}); + describe('isAnsweredAskUserQuestionPart', () => { it('marks only cards whose question was actually answered', () => { const live = applyPendingAction(msg({ content: [] }), askAction({ actionId: 'a-open' })); diff --git a/client/src/utils/approval.ts b/client/src/utils/approval.ts index 205de248c4..b6cdb146e0 100644 --- a/client/src/utils/approval.ts +++ b/client/src/utils/approval.ts @@ -447,6 +447,43 @@ export function findLiveAskUserQuestion( return null; } +/** + * EVERY live (unanswered) ask pause across the conversation, as the set of + * tool_call_ids their synthetic parts attribute, plus whether any live part + * lacks attribution (older payloads). Unlike {@link findLiveAskUserQuestion} + * (newest-only, the popover's signal), this lets a per-call surface — the + * streaming progress card — test whether ITS OWN pause is live even when a + * newer sibling pause exists. + */ +export function collectLiveAskToolCallIds(messages: TMessage[] | null | undefined): { + ids: string[]; + hasUnattributed: boolean; +} { + const ids: string[] = []; + let hasUnattributed = false; + if (!Array.isArray(messages)) { + return { ids, hasUnattributed }; + } + for (const message of messages) { + const content = message?.content; + if (!Array.isArray(content)) { + continue; + } + for (const part of content) { + if (!isAskUserQuestionPart(part) || isAnsweredAskUserQuestionPart(part)) { + continue; + } + const toolCallId = (part as unknown as AskUserQuestionPart)[ASK_USER_QUESTION].tool_call_id; + if (toolCallId == null) { + hasUnattributed = true; + } else { + ids.push(toolCallId); + } + } + } + return { ids, hasUnattributed }; +} + /** * Applies a {@link Agents.PendingAction} onto the target response message, * dispatching on the interrupt type. Pure — returns a new message only when the