From 6bb44447d4d09b8d57296d97acd677cd0d222384 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Thu, 16 Apr 2026 15:16:29 +0200 Subject: [PATCH] refactor(site): address plan-mode frontend review feedback (#24426) > This PR was authored by Mux on behalf of Mike. Address all 8 frontend review comments from DanielleMaywood on #24236. ## Changes **Low-risk cleanups:** - Inlined module-level `toolBadgeClassName` in `AgentChatInput.tsx` - Inlined `planModeInvisibleCharWarning` variable in `PlanModeInstructionsSettings.tsx` - Replaced loose `nilUUID` and `clearPlanMode` sentinels in `chats.ts` with a `toChatPlanModePayload()` helper and inline nil UUID. Page-level mutation callers now pass `undefined` to clear plan mode instead of `""`. **Tool state management:** - Replaced manual `isSubmitting`/`setIsSubmitting` in `ProposePlanTool.tsx` with `useMutation` - Overhauled `AskUserQuestionTool.tsx`: replaced `cloneAnswer` with `structuredClone`, replaced `setIsSubmitting` with `useMutation`, extracted `QuestionStep`, `QuestionOption`, `OtherQuestionOption`, and `AnsweredQuestionText` components **Timeline complexity reduction:** - Extracted `deriveMessageDisplayState()` into `messageHelpers.ts` (pure, testable) - Extracted user-message rendering into `UserMessageContent.tsx` - Reduced `ConversationTimeline.tsx` by ~285 lines ## Validation - TypeScript: pass - Lint (Biome + ESLint): pass - React Compiler: 247 functions, 0 diagnostics - Storybook tests: 42/42 pass (ProposePlanTool 9, ConversationTimeline 22, AskUserQuestionTool 11) --- site/src/api/queries/chats.ts | 24 +- .../AgentsPage/components/AgentChatInput.tsx | 10 +- .../ChatConversation/ConversationTimeline.tsx | 285 +--------- .../ChatConversation/UserMessageContent.tsx | 235 ++++++++ .../ChatConversation/messageHelpers.ts | 85 +++ .../tools/AskUserQuestionTool.tsx | 521 +++++++++++------- .../ChatElements/tools/ProposePlanTool.tsx | 35 +- .../PlanModeInstructionsSettings.tsx | 7 +- 8 files changed, 701 insertions(+), 501 deletions(-) create mode 100644 site/src/pages/AgentsPage/components/ChatConversation/UserMessageContent.tsx create mode 100644 site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts diff --git a/site/src/api/queries/chats.ts b/site/src/api/queries/chats.ts index eefd2d6ad2..e5d413eca4 100644 --- a/site/src/api/queries/chats.ts +++ b/site/src/api/queries/chats.ts @@ -211,7 +211,6 @@ export const cancelChatListRefetches = (queryClient: QueryClient) => { }; const DEFAULT_CHAT_PAGE_LIMIT = 50; -const nilUUID = "00000000-0000-0000-0000-000000000000"; type UpdateChatWorkspaceVariables = { chatId: string; @@ -220,10 +219,17 @@ type UpdateChatWorkspaceVariables = { type UpdateChatPlanModeVariables = { chatId: string; - planMode?: ChatPlanModeOrClear; + planMode?: TypesGen.ChatPlanMode; }; -const clearPlanMode = "" satisfies ChatPlanModeOrClear; +const CLEAR_PLAN_MODE_WIRE_VALUE = "" satisfies ChatPlanModeOrClear; + +const toChatPlanModePayload = ( + planMode: TypesGen.ChatPlanMode | undefined, +): ChatPlanModeOrClear => { + // The API expects an empty string on the wire to clear plan mode. + return planMode ?? CLEAR_PLAN_MODE_WIRE_VALUE; +}; export const infiniteChats = (opts?: { q?: string; archived?: boolean }) => { const limit = DEFAULT_CHAT_PAGE_LIMIT; @@ -407,7 +413,7 @@ export const unarchiveChat = (queryClient: QueryClient) => ({ export const updateChatPlanMode = (queryClient: QueryClient) => ({ mutationFn: ({ chatId, planMode }: UpdateChatPlanModeVariables) => API.experimental.updateChat(chatId, { - plan_mode: planMode ?? clearPlanMode, + plan_mode: toChatPlanModePayload(planMode), }), onMutate: async ({ chatId, planMode }: UpdateChatPlanModeVariables) => { await queryClient.cancelQueries({ @@ -421,16 +427,15 @@ export const updateChatPlanMode = (queryClient: QueryClient) => ({ const previousChat = queryClient.getQueryData( chatKey(chatId), ); - const nextPlanMode = planMode === clearPlanMode ? undefined : planMode; updateInfiniteChatsCache(queryClient, (chats) => chats.map((chat) => - chat.id === chatId ? { ...chat, plan_mode: nextPlanMode } : chat, + chat.id === chatId ? { ...chat, plan_mode: planMode } : chat, ), ); if (previousChat) { queryClient.setQueryData(chatKey(chatId), { ...previousChat, - plan_mode: nextPlanMode, + plan_mode: planMode, }); } return { previousChat }; @@ -466,7 +471,10 @@ export const updateChatPlanMode = (queryClient: QueryClient) => ({ export const updateChatWorkspace = (queryClient: QueryClient) => ({ mutationFn: ({ chatId, workspaceId }: UpdateChatWorkspaceVariables) => API.experimental.updateChat(chatId, { - workspace_id: workspaceId ?? nilUUID, + workspace_id: + workspaceId ?? + // The API uses the nil UUID to clear the workspace association. + "00000000-0000-0000-0000-000000000000", }), onMutate: async ({ chatId, workspaceId }: UpdateChatWorkspaceVariables) => { await queryClient.cancelQueries({ diff --git a/site/src/pages/AgentsPage/components/AgentChatInput.tsx b/site/src/pages/AgentsPage/components/AgentChatInput.tsx index be99684ce6..b3f6439a48 100644 --- a/site/src/pages/AgentsPage/components/AgentChatInput.tsx +++ b/site/src/pages/AgentsPage/components/AgentChatInput.tsx @@ -171,16 +171,16 @@ type ToolBadgeData = | ({ kind: "attached-workspace" } & AttachedWorkspaceInfo) | { kind: "mcp"; server: TypesGen.MCPServerConfig }; -const toolBadgeClassName = - "inline-flex shrink-0 items-center gap-1 rounded-full bg-surface-secondary px-2 py-0.5 text-xs font-medium text-content-secondary"; - const ToolBadge: FC<{ badge: ToolBadgeData; onRemoveWorkspace?: () => void; onRemoveMcp?: (serverId: string) => void; className?: string; }> = ({ badge, onRemoveWorkspace, onRemoveMcp, className }) => { - const badgeCls = cn(toolBadgeClassName, className); + const badgeCls = cn( + "inline-flex shrink-0 items-center gap-1 rounded-full bg-surface-secondary px-2 py-0.5 text-xs font-medium text-content-secondary", + className, + ); if (badge.kind === "attached-workspace") { return ( @@ -958,7 +958,7 @@ export const AgentChatInput: FC = ({ /> )} {planModeEnabled && ( - + Planning diff --git a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx index ca687c90fe..9c13153f82 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx @@ -1,9 +1,8 @@ -import { FileTextIcon, PencilIcon } from "lucide-react"; +import { PencilIcon } from "lucide-react"; import { type FC, Fragment, memo, - useEffect, useLayoutEffect, useRef, useState, @@ -18,12 +17,6 @@ import { TooltipTrigger, } from "#/components/Tooltip/Tooltip"; import { cn } from "#/utils/cn"; -import { - decodeInlineTextAttachment, - fetchTextAttachmentContent, - formatTextAttachmentPreview, -} from "../../utils/fetchTextAttachment"; -import { ImageThumbnail } from "../AgentChatInput"; import { ConversationItem, Message, @@ -33,9 +26,9 @@ import { Tool, } from "../ChatElements"; import { WebSearchSources } from "../ChatElements/tools"; -import { FileReferenceChip } from "../ChatMessageInput/FileReferenceNode"; import { ImageLightbox } from "../ImageLightbox"; import { TextPreviewDialog } from "../TextPreviewDialog"; +import { deriveMessageDisplayState } from "./messageHelpers"; import { getEditableUserMessagePayload } from "./messageParsing"; import { useSmoothStreamingText } from "./SmoothText"; import type { @@ -44,6 +37,7 @@ import type { ParsedMessageEntry, RenderBlock, } from "./types"; +import { FileBlock, UserMessageContent } from "./UserMessageContent"; const getChatMessageTextContent = ( content: readonly TypesGen.ChatMessagePart[] | undefined, @@ -123,136 +117,6 @@ const SmoothedResponse = memo<{ ); }); -const InlineTextAttachmentButton: FC<{ - content: string; - onPreview?: (content: string) => void; - isPlaceholder?: boolean; -}> = ({ content, onPreview, isPlaceholder }) => { - return ( - - ); -}; - -const TextAttachmentButton: FC<{ - fileId: string; - onPreview?: (content: string) => void; -}> = ({ fileId, onPreview }) => { - const [content, setContent] = useState(null); - const controllerRef = useRef(null); - - useEffect(() => { - return () => controllerRef.current?.abort(); - }, []); - - return ( - { - if (content !== null) { - onPreview?.(content); - return; - } - - controllerRef.current?.abort(); - const controller = new AbortController(); - controllerRef.current = controller; - - let fetchedContent: string; - try { - fetchedContent = await fetchTextAttachmentContent( - fileId, - controller.signal, - ); - } catch (err) { - if (controllerRef.current === controller) { - controllerRef.current = null; - } - if (err instanceof Error && err.name === "AbortError") { - return; - } - console.error("Failed to load text attachment:", err); - return; - } - - if (controllerRef.current === controller) { - controllerRef.current = null; - } - setContent(fetchedContent); - onPreview?.(fetchedContent); - }} - /> - ); -}; - -type FileRenderBlock = Extract; - -const FileBlock: FC<{ - block: FileRenderBlock; - onImageClick?: (src: string) => void; - onTextFileClick?: (content: string) => void; -}> = ({ block, onImageClick, onTextFileClick }) => { - if (block.media_type === "text/plain") { - if (block.file_id) { - return ( - - ); - } - if (block.data != null) { - return ( - - ); - } - } - if (!block.media_type.startsWith("image/")) { - return null; - } - const src = block.file_id - ? `/api/experimental/chats/files/${block.file_id}` - : `data:${block.media_type};base64,${block.data}`; - return ( - - ); -}; - // Shared block renderer used by both ChatMessageItem (historical // messages) and StreamingOutput (live stream). Encapsulates the // response / thinking / tool / file / sources switch so both @@ -522,67 +386,19 @@ const ChatMessageItem = memo<{ const isUser = message.role === "user"; const [previewImage, setPreviewImage] = useState(null); const [previewText, setPreviewText] = useState(null); - if ( - parsed.toolResults.length > 0 && - parsed.toolCalls.length === 0 && - parsed.markdown === "" && - parsed.reasoning === "" - ) { + const displayState = deriveMessageDisplayState({ + message, + parsed, + hideActions, + }); + if (displayState.shouldHide) { return null; } - // Hide messages that consist entirely of provider-executed - // tool results. The parser skips these parts, so the parsed - // output is empty and would show a "no renderable content" - // fallback. - const parts = message.content ?? []; - if ( - parts.length > 0 && - parts.every((p) => p.type === "tool-result" && p.provider_executed) - ) { - return null; - } - - // Hide messages that consist entirely of context-file - // and/or skill parts. These are metadata for the context - // indicator, not conversation content. - if ( - parts.length > 0 && - parts.every((p) => p.type === "context-file" || p.type === "skill") - ) { - return null; - } const hasRenderableContent = parsed.blocks.length > 0 || parsed.tools.length > 0 || parsed.sources.length > 0; - // Pre-compute the inline content for user messages so we - // avoid a filter + map inside the JSX return path. - const userInlineContent = isUser - ? parsed.blocks.filter( - ( - b, - ): b is - | Extract - | Extract => - b.type === "response" || b.type === "file-reference", - ) - : []; - const userFileBlocks = isUser - ? parsed.blocks.filter( - (b): b is Extract => b.type === "file", - ) - : []; - const hasUserMessageBody = - userInlineContent.length > 0 || Boolean(parsed.markdown?.trim()); - const hasFileBlocks = userFileBlocks.length > 0; - const hasCopyableContent = Boolean(parsed.markdown.trim()); - const needsAssistantBottomSpacer = - !hideActions && - !isUser && - !hasCopyableContent && - (Boolean(parsed.reasoning) || parsed.sources.length > 0); - const conversationItemProps: { role: "user" | "assistant" } = { role: isUser ? "user" : "assistant", }; @@ -596,74 +412,14 @@ const ChatMessageItem = memo<{ > {isUser ? ( - - -
- {(hasUserMessageBody || hasFileBlocks) && ( -
- {hasUserMessageBody && ( - - {userInlineContent.length > 0 - ? userInlineContent.map((block, i) => - block.type === "response" ? ( - {block.text} - ) : ( - - ), - ) - : parsed.markdown || ""} - - )} -
- )} - {hasFileBlocks && ( -
- {userFileBlocks.map((block, i) => ( - - ))} -
- )} - {fadeFromBottom && ( -
- )} -
- - + ) : ( @@ -703,12 +459,13 @@ const ChatMessageItem = memo<{ )} {!hideActions && - (hasCopyableContent || (isUser && onEditUserMessage)) && ( + (displayState.hasCopyableContent || + (isUser && onEditUserMessage)) && (
- {hasCopyableContent && ( + {displayState.hasCopyableContent && ( } + {displayState.needsAssistantBottomSpacer &&
} {previewImage && ( void; + isPlaceholder?: boolean; +}> = ({ content, onPreview, isPlaceholder }) => { + return ( + + ); +}; + +const TextAttachmentButton: FC<{ + fileId: string; + onPreview?: (content: string) => void; +}> = ({ fileId, onPreview }) => { + const [content, setContent] = useState(null); + const controllerRef = useRef(null); + + useEffect(() => { + return () => controllerRef.current?.abort(); + }, []); + + return ( + { + if (content !== null) { + onPreview?.(content); + return; + } + + controllerRef.current?.abort(); + const controller = new AbortController(); + controllerRef.current = controller; + + let fetchedContent: string; + try { + fetchedContent = await fetchTextAttachmentContent( + fileId, + controller.signal, + ); + } catch (error) { + if (controllerRef.current === controller) { + controllerRef.current = null; + } + if (error instanceof Error && error.name === "AbortError") { + return; + } + console.error("Failed to load text attachment:", error); + return; + } + + if (controllerRef.current === controller) { + controllerRef.current = null; + } + setContent(fetchedContent); + onPreview?.(fetchedContent); + }} + /> + ); +}; + +export const FileBlock: FC<{ + block: UserFileRenderBlock; + onImageClick?: (src: string) => void; + onTextFileClick?: (content: string) => void; +}> = ({ block, onImageClick, onTextFileClick }) => { + if (block.media_type === "text/plain") { + if (block.file_id) { + return ( + + ); + } + if (block.data != null) { + return ( + + ); + } + } + if (!block.media_type.startsWith("image/")) { + return null; + } + const src = block.file_id + ? `/api/experimental/chats/files/${block.file_id}` + : `data:${block.media_type};base64,${block.data}`; + return ( + + ); +}; + +const renderUserInlineBlock = (block: UserInlineRenderBlock, index: number) => { + if (block.type === "response") { + return {block.text}; + } + + return ( + + ); +}; + +export const UserMessageContent: FC<{ + displayState: MessageDisplayState; + markdown: string; + isEditing?: boolean; + fadeFromBottom?: boolean; + onImageClick?: (src: string) => void; + onTextFileClick?: (content: string) => void; +}> = ({ + displayState, + markdown, + isEditing = false, + fadeFromBottom = false, + onImageClick, + onTextFileClick, +}) => { + return ( + + +
+ {(displayState.hasUserMessageBody || displayState.hasFileBlocks) && ( +
+ {displayState.hasUserMessageBody && ( + + {displayState.userInlineContent.length > 0 + ? displayState.userInlineContent.map((block, index) => + renderUserInlineBlock(block, index), + ) + : markdown || ""} + + )} +
+ )} + {displayState.hasFileBlocks && ( +
+ {displayState.userFileBlocks.map((block, index) => ( + + ))} +
+ )} + {fadeFromBottom && ( +
+ )} +
+ + + ); +}; diff --git a/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts b/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts new file mode 100644 index 0000000000..f39253c05d --- /dev/null +++ b/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts @@ -0,0 +1,85 @@ +import type * as TypesGen from "#/api/typesGenerated"; +import type { ParsedMessageContent, RenderBlock } from "./types"; + +export type UserInlineRenderBlock = + | Extract + | Extract; + +export type UserFileRenderBlock = Extract; + +export type MessageDisplayState = { + shouldHide: boolean; + userInlineContent: UserInlineRenderBlock[]; + userFileBlocks: UserFileRenderBlock[]; + hasUserMessageBody: boolean; + hasFileBlocks: boolean; + hasCopyableContent: boolean; + needsAssistantBottomSpacer: boolean; +}; + +const isUserInlineRenderBlock = ( + block: RenderBlock, +): block is UserInlineRenderBlock => + block.type === "response" || block.type === "file-reference"; + +const isUserFileRenderBlock = ( + block: RenderBlock, +): block is UserFileRenderBlock => block.type === "file"; + +const isProviderToolResultOnlyMessage = ( + parts: readonly TypesGen.ChatMessagePart[], +): boolean => + parts.length > 0 && + parts.every((part) => part.type === "tool-result" && part.provider_executed); + +const isMetadataOnlyMessage = ( + parts: readonly TypesGen.ChatMessagePart[], +): boolean => + parts.length > 0 && + parts.every((part) => part.type === "context-file" || part.type === "skill"); + +export const deriveMessageDisplayState = ({ + message, + parsed, + hideActions, +}: { + message: TypesGen.ChatMessage; + parsed: ParsedMessageContent; + hideActions: boolean; +}): MessageDisplayState => { + const isUser = message.role === "user"; + const userInlineContent = isUser + ? parsed.blocks.filter(isUserInlineRenderBlock) + : []; + const userFileBlocks = isUser + ? parsed.blocks.filter(isUserFileRenderBlock) + : []; + const hasUserMessageBody = + userInlineContent.length > 0 || Boolean(parsed.markdown.trim()); + const hasFileBlocks = userFileBlocks.length > 0; + const hasCopyableContent = Boolean(parsed.markdown.trim()); + const needsAssistantBottomSpacer = + !hideActions && + !isUser && + !hasCopyableContent && + (Boolean(parsed.reasoning) || parsed.sources.length > 0); + const hasToolResultsOnly = + parsed.toolResults.length > 0 && + parsed.toolCalls.length === 0 && + parsed.markdown === "" && + parsed.reasoning === ""; + const parts = message.content ?? []; + + return { + shouldHide: + hasToolResultsOnly || + isProviderToolResultOnlyMessage(parts) || + isMetadataOnlyMessage(parts), + userInlineContent, + userFileBlocks, + hasUserMessageBody, + hasFileBlocks, + hasCopyableContent, + needsAssistantBottomSpacer, + }; +}; diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/AskUserQuestionTool.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/AskUserQuestionTool.tsx index 7c1ae7f8c0..609c0aaf8c 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/AskUserQuestionTool.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/AskUserQuestionTool.tsx @@ -1,5 +1,6 @@ import { LoaderIcon, TriangleAlertIcon } from "lucide-react"; import { type FC, type FormEvent, useId, useState } from "react"; +import { useMutation } from "react-query"; import { Button } from "#/components/Button/Button"; import { Input } from "#/components/Input/Input"; import { RadioGroup, RadioGroupItem } from "#/components/RadioGroup/RadioGroup"; @@ -51,29 +52,26 @@ const filterQuestionOptions = (question: AskUserQuestion): AskUserQuestion => ({ ), }); -const formatAnswer = (answer: QuestionAnswer): string => - answer.kind === "other" - ? `Other: ${answer.text.trim()}` - : answer.label || `Option ${answer.optionIndex + 1}`; - -const cloneAnswer = ( - answer: QuestionAnswer | undefined, +const getDefaultAnswer = ( + question: AskUserQuestion, ): QuestionAnswer | undefined => { - if (!answer) { + const firstOption = question.options[0]; + if (!firstOption) { return undefined; } - if (answer.kind === "other") { - return { kind: "other", text: answer.text }; - } - return { kind: "option", - label: answer.label, - optionIndex: answer.optionIndex, + label: firstOption.label || "Option 1", + optionIndex: 0, }; }; +const formatAnswer = (answer: QuestionAnswer): string => + answer.kind === "other" + ? `Other: ${answer.text.trim()}` + : answer.label || `Option ${answer.optionIndex + 1}`; + const isAnswerValid = ( answer: QuestionAnswer | undefined, ): answer is QuestionAnswer => { @@ -117,6 +115,243 @@ const formatOutgoingMessage = ( .join("\n"); }; +const getSubmissionErrorMessage = (error: unknown): string | undefined => { + if (!error) { + return undefined; + } + + if (error instanceof Error) { + return error.message; + } + + return "Failed to submit your answer."; +}; + +type SelectableAnswerOptionProps = { + id: string; + value: string; + label: string; + description: string; + isInteractive: boolean; + isSubmitting: boolean; +}; + +const SelectableAnswerOption: FC = ({ + id, + value, + label, + description, + isInteractive, + isSubmitting, +}) => { + const isEnabled = isInteractive && !isSubmitting; + + return ( + + ); +}; + +type QuestionOptionProps = { + questionIdBase: string; + option: AskUserQuestion["options"][number]; + optionIndex: number; + isInteractive: boolean; + isSubmitting: boolean; +}; + +const QuestionOption: FC = ({ + questionIdBase, + option, + optionIndex, + isInteractive, + isSubmitting, +}) => { + return ( + + ); +}; + +type OtherQuestionOptionProps = { + questionHeader: string; + questionIdBase: string; + optionIndex: number; + answer: QuestionAnswer | undefined; + isInteractive: boolean; + isSubmitting: boolean; + onTextChange: (text: string) => void; +}; + +const OtherQuestionOption: FC = ({ + questionHeader, + questionIdBase, + optionIndex, + answer, + isInteractive, + isSubmitting, + onTextChange, +}) => { + const isOtherSelected = answer?.kind === "other"; + + return ( +
+ + {isOtherSelected && ( +
+ { + onTextChange(event.currentTarget.value); + }} + /> +
+ )} +
+ ); +}; + +type QuestionStepProps = { + question: AskUserQuestion; + questionIndex: number; + questionCount: number; + idPrefix: string; + answer: QuestionAnswer | undefined; + isInteractive: boolean; + isSubmitting: boolean; + onOptionChange: (value: string) => void; + onOtherTextChange: (text: string) => void; +}; + +const QuestionStep: FC = ({ + question, + questionIndex, + questionCount, + idPrefix, + answer, + isInteractive, + isSubmitting, + onOptionChange, + onOtherTextChange, +}) => { + const questionHeader = getQuestionHeader(question, questionIndex); + const questionText = getQuestionText(question); + const questionIdBase = `${idPrefix}-question-${questionIndex}`; + const questionHeaderId = `${questionIdBase}-header`; + const questionTextId = `${questionIdBase}-text`; + const showProgress = isInteractive && questionCount > 1; + + return ( +
+ {showProgress && ( +

+ Question {questionIndex + 1} of {questionCount} +

+ )} +
+

+ {questionHeader} +

+

+ {questionText} +

+
+
+ + {question.options.map((option, optionIndex) => { + return ( + + ); + })} + + +
+
+ ); +}; + +type AnsweredQuestionTextProps = { + question: AskUserQuestion; + questionIndex: number; + idPrefix: string; +}; + +const AnsweredQuestionText: FC = ({ + question, + questionIndex, + idPrefix, +}) => { + return ( +

+ {getQuestionText(question)} +

+ ); +}; + export const AskUserQuestionTool: FC = ({ questions, status, @@ -130,25 +365,24 @@ export const AskUserQuestionTool: FC = ({ const idPrefix = useId(); const filteredQuestions = questions.map(filterQuestionOptions); const [answers, setAnswers] = useState>( - () => - filteredQuestions.map((question) => { - const firstOption = question.options[0]; - if (!firstOption) { - return undefined; - } - return { - kind: "option" as const, - label: firstOption.label || "Option 1", - optionIndex: 0, - }; - }), + () => filteredQuestions.map(getDefaultAnswer), ); const [currentQuestionIndex, setCurrentQuestionIndex] = useState(0); - const [isSubmitting, setIsSubmitting] = useState(false); - const [submitError, setSubmitError] = useState(); const [submittedResponseText, setSubmittedResponseText] = useState< string | null >(null); + const submitAnswerMutation = useMutation({ + mutationFn: async (message: string) => { + if (!onSubmitAnswer) { + return; + } + + await onSubmitAnswer(message); + }, + onSuccess: (_data, message) => { + setSubmittedResponseText(message); + }, + }); const isRunning = status === "running"; const displayedSubmittedResponseText = previousResponseText ?? submittedResponseText; @@ -166,10 +400,30 @@ export const AskUserQuestionTool: FC = ({ isLatestAskUserQuestion && !hasSubmittedResponse && Boolean(onSubmitAnswer); + const isSubmitting = submitAnswerMutation.isPending; + const submitError = getSubmissionErrorMessage(submitAnswerMutation.error); const canAdvanceToNextQuestion = isAnswerValid(currentAnswer); const canSubmitAllAnswers = filteredQuestions.every((_, questionIndex) => isAnswerValid(answers[questionIndex]), ); + const isWizard = filteredQuestions.length > 1; + const isFinalQuestion = activeQuestionIndex >= filteredQuestions.length - 1; + const visibleQuestions = + isInteractive && isWizard + ? [ + { + question: filteredQuestions[activeQuestionIndex], + questionIndex: activeQuestionIndex, + }, + ] + : filteredQuestions.map((question, questionIndex) => ({ + question, + questionIndex, + })); + + const resetSubmitState = () => { + submitAnswerMutation.reset(); + }; const setAnswerAtIndex = ( questionIndex: number, @@ -180,7 +434,7 @@ export const AskUserQuestionTool: FC = ({ nextAnswers[questionIndex] = nextAnswer; return nextAnswers; }); - setSubmitError(undefined); + resetSubmitState(); }; const handleOptionChange = ( @@ -216,7 +470,7 @@ export const AskUserQuestionTool: FC = ({ setCurrentQuestionIndex((currentIndex) => { return Math.max(currentIndex - 1, 0); }); - setSubmitError(undefined); + resetSubmitState(); }; const handleNext = () => { @@ -227,16 +481,16 @@ export const AskUserQuestionTool: FC = ({ setCurrentQuestionIndex((currentIndex) => { return Math.min(currentIndex + 1, filteredQuestions.length - 1); }); - setSubmitError(undefined); + resetSubmitState(); }; - const handleSubmit = async () => { + const handleSubmit = () => { if (!onSubmitAnswer || !isInteractive || !canSubmitAllAnswers) { return; } const finalizedAnswers = filteredQuestions.map((_, questionIndex) => { - return cloneAnswer(answers[questionIndex]); + return answers[questionIndex]; }); if (!finalizedAnswers.every(isAnswerValid)) { return; @@ -246,22 +500,8 @@ export const AskUserQuestionTool: FC = ({ filteredQuestions, finalizedAnswers, ); - setIsSubmitting(true); - setSubmitError(undefined); - try { - await onSubmitAnswer(outgoingMessage); - } catch (error) { - setSubmitError( - error instanceof Error - ? error.message - : "Failed to submit your answer.", - ); - setIsSubmitting(false); - return; - } - - setSubmittedResponseText(outgoingMessage); - setIsSubmitting(false); + resetSubmitState(); + submitAnswerMutation.mutate(outgoingMessage); }; const handleFormSubmit = (event: FormEvent) => { @@ -270,15 +510,12 @@ export const AskUserQuestionTool: FC = ({ return; } - if ( - filteredQuestions.length > 1 && - activeQuestionIndex < filteredQuestions.length - 1 - ) { + if (isWizard && !isFinalQuestion) { handleNext(); return; } - void handleSubmit(); + handleSubmit(); }; if (isError) { @@ -324,161 +561,42 @@ export const AskUserQuestionTool: FC = ({ ); } - const visibleQuestions = - isInteractive && filteredQuestions.length > 1 - ? [ - { - question: filteredQuestions[activeQuestionIndex], - questionIndex: activeQuestionIndex, - }, - ] - : filteredQuestions.map((question, questionIndex) => ({ - question, - questionIndex, - })); - const content = ( <>
{visibleQuestions.map(({ question, questionIndex }) => { - const questionHeader = getQuestionHeader(question, questionIndex); - const questionText = getQuestionText(question); - const questionIdBase = `${idPrefix}-question-${questionIndex}`; - const questionHeaderId = `${questionIdBase}-header`; - const questionTextId = `${questionIdBase}-text`; - const answer = answers[questionIndex]; - const isOtherSelected = answer?.kind === "other"; - const optionCount = question.options.length; - const showProgress = isInteractive && filteredQuestions.length > 1; - + const questionKey = `${question.header}-${question.question}-${questionIndex}`; if (showAnsweredState) { return ( -

- {questionText} -

+ ); } return ( -
- {showProgress && ( -

- Question {questionIndex + 1} of {filteredQuestions.length} -

- )} -
-

- {questionHeader} -

-

- {questionText} -

-
-
- { - handleOptionChange(questionIndex, question, value); - }} - > - {question.options.map((option, optionIndex) => { - const optionId = `${questionIdBase}-option-${optionIndex}`; - - return ( - - ); - })} - {(() => { - const otherOptionId = `${questionIdBase}-option-${optionCount}`; - return ( -
- - {isOtherSelected && ( -
- { - setAnswerAtIndex(questionIndex, { - kind: "other", - text: event.currentTarget.value, - }); - }} - /> -
- )} -
- ); - })()} - -
-
+ { + handleOptionChange(questionIndex, question, value); + }} + onOtherTextChange={(text) => { + setAnswerAtIndex(questionIndex, { + kind: "other", + text, + }); + }} + /> ); })}
@@ -506,7 +624,7 @@ export const AskUserQuestionTool: FC = ({ {isInteractive && (
- {filteredQuestions.length > 1 && ( + {isWizard && ( )} - {filteredQuestions.length > 1 && - activeQuestionIndex < filteredQuestions.length - 1 ? ( + {isWizard && !isFinalQuestion ? ( Implement plan diff --git a/site/src/pages/AgentsPage/components/PlanModeInstructionsSettings.tsx b/site/src/pages/AgentsPage/components/PlanModeInstructionsSettings.tsx index 25d33c10fa..6ff1cbf5d9 100644 --- a/site/src/pages/AgentsPage/components/PlanModeInstructionsSettings.tsx +++ b/site/src/pages/AgentsPage/components/PlanModeInstructionsSettings.tsx @@ -59,7 +59,6 @@ export const PlanModeInstructionsSettings: FC< const planModeInvisibleCharCount = countInvisibleCharacters( form.values.plan_mode_instructions, ); - const planModeInvisibleCharWarning = `This text contains ${planModeInvisibleCharCount} invisible Unicode ${planModeInvisibleCharCount !== 1 ? "characters" : "character"} that could hide content. These will be stripped on save.`; const isPlanModeInstructionsDisabled = isAnyPromptSaving || !hasLoadedPlanModeInstructions; @@ -94,7 +93,11 @@ export const PlanModeInstructionsSettings: FC< /> {planModeInvisibleCharCount > 0 && ( - {planModeInvisibleCharWarning} + + This text contains {planModeInvisibleCharCount} invisible Unicode{" "} + {planModeInvisibleCharCount !== 1 ? "characters" : "character"} that + could hide content. These will be stripped on save. + )}