diff --git a/client/src/components/Chat/Messages/Content/Part.tsx b/client/src/components/Chat/Messages/Content/Part.tsx index 9052e8a643..a4b3be7e1c 100644 --- a/client/src/components/Chat/Messages/Content/Part.tsx +++ b/client/src/components/Chat/Messages/Content/Part.tsx @@ -166,6 +166,7 @@ const Part = memo(function Part({ provider={part.provider} tokenCount={part.tokenCount} summarizing={part.summarizing} + failed={part.failed} /> ); } else if (part.type === ContentTypes.ACTIVITY_LABEL) { diff --git a/client/src/components/Chat/Messages/Content/Parts/CodeWindowHeader.tsx b/client/src/components/Chat/Messages/Content/Parts/CodeWindowHeader.tsx index 6057bc0bf8..693f0ee52b 100644 --- a/client/src/components/Chat/Messages/Content/Parts/CodeWindowHeader.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/CodeWindowHeader.tsx @@ -31,8 +31,8 @@ export default function CodeWindowHeader({ language, code, diffStats }: CodeWind {language} {diffStats && ( )} diff --git a/client/src/components/Chat/Messages/Content/Parts/DiffView.tsx b/client/src/components/Chat/Messages/Content/Parts/DiffView.tsx index 9b2c5337e8..ecff8b7e9c 100644 --- a/client/src/components/Chat/Messages/Content/Parts/DiffView.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/DiffView.tsx @@ -121,8 +121,8 @@ export default function DiffView({ parsed }: { parsed: ParsedDiff }) { key={index} className={cn( 'flex', - line.type === 'add' && 'bg-green-500/10', - line.type === 'del' && 'bg-red-500/10', + line.type === 'add' && 'bg-status-success-subtle', + line.type === 'del' && 'bg-status-error-subtle', )} > {hasLineNumbers && ( @@ -134,8 +134,8 @@ export default function DiffView({ parsed }: { parsed: ParsedDiff }) { className={cn( 'shrink-0 select-none text-center font-semibold', hasLineNumbers ? 'w-5' : 'w-6', - line.type === 'add' && 'text-green-600 dark:text-green-400', - line.type === 'del' && 'text-red-600 dark:text-red-400', + line.type === 'add' && 'text-status-success', + line.type === 'del' && 'text-status-error', )} > {LINE_MARKERS[line.type]} diff --git a/client/src/components/Chat/Messages/Content/Parts/Summary.tsx b/client/src/components/Chat/Messages/Content/Parts/Summary.tsx index 1398ec286f..770a98a9e1 100644 --- a/client/src/components/Chat/Messages/Content/Parts/Summary.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/Summary.tsx @@ -12,7 +12,7 @@ import { cn } from '~/utils'; type SummaryProps = Pick< SummaryContentPart, - 'content' | 'model' | 'provider' | 'tokenCount' | 'summarizing' + 'content' | 'model' | 'provider' | 'tokenCount' | 'summarizing' | 'failed' >; function useCopyToClipboard(content?: string) { @@ -203,115 +203,120 @@ const FloatingSummaryBar = memo( }, ); -const Summary = memo(({ content, model, provider, tokenCount, summarizing }: SummaryProps) => { - const contentId = useId(); - const localize = useLocalize(); - const [isExpanded, setIsExpanded] = useState(false); - const [isBarVisible, setIsBarVisible] = useState(false); - const containerRef = useRef(null); - const { isSubmitting, isLatestMessage } = useMessageContext(); +const Summary = memo( + ({ content, model, provider, tokenCount, summarizing, failed }: SummaryProps) => { + const contentId = useId(); + const localize = useLocalize(); + const [isExpanded, setIsExpanded] = useState(false); + const [isBarVisible, setIsBarVisible] = useState(false); + const containerRef = useRef(null); + const { isSubmitting, isLatestMessage } = useMessageContext(); - const text = useMemo( - () => - (content ?? []) - .map((block) => ('text' in block && typeof block.text === 'string' ? block.text : '')) - .join(''), - [content], - ); - const { isCopied, handleCopy } = useCopyToClipboard(text); + const text = useMemo( + () => + (content ?? []) + .map((block) => ('text' in block && typeof block.text === 'string' ? block.text : '')) + .join(''), + [content], + ); + const { isCopied, handleCopy } = useCopyToClipboard(text); - const handleClick = useCallback((e: MouseEvent) => { - e.preventDefault(); - setIsExpanded((prev) => !prev); - }, []); + const handleClick = useCallback((e: MouseEvent) => { + e.preventDefault(); + setIsExpanded((prev) => !prev); + }, []); - const handleFocus = useCallback(() => setIsBarVisible(true), []); - const handleBlur = useCallback((e: FocusEvent) => { - if (!containerRef.current?.contains(e.relatedTarget as Node)) { - setIsBarVisible(false); + const handleFocus = useCallback(() => setIsBarVisible(true), []); + const handleBlur = useCallback((e: FocusEvent) => { + if (!containerRef.current?.contains(e.relatedTarget as Node)) { + setIsBarVisible(false); + } + }, []); + const handleMouseEnter = useCallback(() => setIsBarVisible(true), []); + const handleMouseLeave = useCallback(() => { + if (!containerRef.current?.contains(document.activeElement)) { + setIsBarVisible(false); + } + }, []); + + const effectiveIsSubmitting = isLatestMessage ? isSubmitting : false; + const isActivelyStreaming = !!summarizing && !!effectiveIsSubmitting; + + const meta = useMemo(() => { + const parts: string[] = []; + if (provider || model) { + parts.push([provider, model].filter(Boolean).join('/')); + } + if (tokenCount != null && tokenCount > 0) { + parts.push(`${tokenCount} ${localize('com_ui_tokens')}`); + } + return parts.length > 0 ? parts.join(' \u00b7 ') : undefined; + }, [model, provider, tokenCount, localize]); + + /** A failed round keeps whatever deltas already streamed in, so the label + * must not claim the conversation was summarized. */ + const label = useMemo(() => { + if (isActivelyStreaming) { + return localize('com_ui_summarizing'); + } + return failed + ? localize('com_ui_summarize_failed') + : localize('com_ui_conversation_summarized'); + }, [isActivelyStreaming, failed, localize]); + + if (!summarizing && !text) { + return null; } - }, []); - const handleMouseEnter = useCallback(() => setIsBarVisible(true), []); - const handleMouseLeave = useCallback(() => { - if (!containerRef.current?.contains(document.activeElement)) { - setIsBarVisible(false); - } - }, []); - const effectiveIsSubmitting = isLatestMessage ? isSubmitting : false; - const isActivelyStreaming = !!summarizing && !!effectiveIsSubmitting; - - const meta = useMemo(() => { - const parts: string[] = []; - if (provider || model) { - parts.push([provider, model].filter(Boolean).join('/')); - } - if (tokenCount != null && tokenCount > 0) { - parts.push(`${tokenCount} ${localize('com_ui_tokens')}`); - } - return parts.length > 0 ? parts.join(' \u00b7 ') : undefined; - }, [model, provider, tokenCount, localize]); - - const label = useMemo( - () => - isActivelyStreaming - ? localize('com_ui_summarizing') - : localize('com_ui_conversation_summarized'), - [isActivelyStreaming, localize], - ); - - if (!summarizing && !text) { - return null; - } - - return ( -
-
-
- -
-
-
- {text} - +
+
+
+
+
+ {text} + +
+
-
- ); -}); + ); + }, +); SummaryContent.displayName = 'SummaryContent'; SummaryButton.displayName = 'SummaryButton'; diff --git a/client/src/components/Chat/Messages/Content/Parts/__tests__/FileAuthoringCall.test.tsx b/client/src/components/Chat/Messages/Content/Parts/__tests__/FileAuthoringCall.test.tsx index 17f30b60df..d65575ecbc 100644 --- a/client/src/components/Chat/Messages/Content/Parts/__tests__/FileAuthoringCall.test.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/__tests__/FileAuthoringCall.test.tsx @@ -158,8 +158,8 @@ describe('FileAuthoringCall', () => { />, ); - expect(screen.getByText('old line').closest('div')).toHaveClass('bg-red-500/10'); - expect(screen.getByText('new line').closest('div')).toHaveClass('bg-green-500/10'); + expect(screen.getByText('old line').closest('div')).toHaveClass('bg-status-error-subtle'); + expect(screen.getByText('new line').closest('div')).toHaveClass('bg-status-success-subtle'); expect(screen.queryByText(/--- old_text/)).not.toBeInTheDocument(); }); @@ -178,8 +178,8 @@ describe('FileAuthoringCall', () => { />, ); - expect(screen.getByText('missing text').closest('div')).toHaveClass('bg-red-500/10'); - expect(screen.getByText('replacement').closest('div')).toHaveClass('bg-green-500/10'); + expect(screen.getByText('missing text').closest('div')).toHaveClass('bg-status-error-subtle'); + expect(screen.getByText('replacement').closest('div')).toHaveClass('bg-status-success-subtle'); expect(screen.getByText(/matched 0 locations/)).toBeInTheDocument(); }); @@ -202,10 +202,10 @@ describe('FileAuthoringCall', () => { expect(screen.getByTestId('code-window-header')).toHaveAttribute('data-language', 'SKILL.md'); expect(screen.queryByText(/--- old_text/)).not.toBeInTheDocument(); expect(screen.getByText('description: Old behavior').closest('div')).toHaveClass( - 'bg-red-500/10', + 'bg-status-error-subtle', ); expect(screen.getByText('description: New behavior').closest('div')).toHaveClass( - 'bg-green-500/10', + 'bg-status-success-subtle', ); }); @@ -239,9 +239,11 @@ describe('FileAuthoringCall', () => { ); expect(screen.getByText('description: Old behavior').closest('div')).toHaveClass( - 'bg-red-500/10', + 'bg-status-error-subtle', + ); + expect(screen.getByText('description: New beh').closest('div')).toHaveClass( + 'bg-status-success-subtle', ); - expect(screen.getByText('description: New beh').closest('div')).toHaveClass('bg-green-500/10'); }); it('streams batched edit_file previews from a partial edits array', () => { @@ -260,10 +262,10 @@ describe('FileAuthoringCall', () => { ); const diff = screen.getByTestId('diff-view'); - expect(screen.getByText('first old').closest('div')).toHaveClass('bg-red-500/10'); - expect(screen.getByText('first new').closest('div')).toHaveClass('bg-green-500/10'); - expect(screen.getByText('second old').closest('div')).toHaveClass('bg-red-500/10'); - expect(screen.getByText('second n').closest('div')).toHaveClass('bg-green-500/10'); + expect(screen.getByText('first old').closest('div')).toHaveClass('bg-status-error-subtle'); + expect(screen.getByText('first new').closest('div')).toHaveClass('bg-status-success-subtle'); + expect(screen.getByText('second old').closest('div')).toHaveClass('bg-status-error-subtle'); + expect(screen.getByText('second n').closest('div')).toHaveClass('bg-status-success-subtle'); expect(diff).toBeInTheDocument(); }); @@ -284,8 +286,8 @@ describe('FileAuthoringCall', () => { />, ); - expect(screen.getByText('first old').closest('div')).toHaveClass('bg-red-500/10'); - expect(screen.getByText('second new').closest('div')).toHaveClass('bg-green-500/10'); + expect(screen.getByText('first old').closest('div')).toHaveClass('bg-status-error-subtle'); + expect(screen.getByText('second new').closest('div')).toHaveClass('bg-status-success-subtle'); }); }); diff --git a/client/src/components/Chat/Messages/Content/Parts/__tests__/Summary.test.tsx b/client/src/components/Chat/Messages/Content/Parts/__tests__/Summary.test.tsx new file mode 100644 index 0000000000..0f34c13e55 --- /dev/null +++ b/client/src/components/Chat/Messages/Content/Parts/__tests__/Summary.test.tsx @@ -0,0 +1,75 @@ +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import Summary from '../Summary'; + +jest.mock('~/hooks', () => ({ + useLocalize: () => (key: string) => key, +})); + +jest.mock('jotai', () => ({ + useAtomValue: () => 'text-sm', +})); + +jest.mock('~/store/fontSize', () => ({ + fontSizeAtom: {}, +})); + +const mockMessageContext = { current: { isSubmitting: false, isLatestMessage: false } }; + +jest.mock('~/Providers', () => ({ + useMessageContext: () => mockMessageContext.current, +})); + +jest.mock('@librechat/client', () => ({ + Button: ({ children, ...props }: React.ComponentProps<'button'>) => ( + + ), + Clipboard: () => , + CheckMark: () => , + TooltipAnchor: ({ children }: { children?: React.ReactNode }) => <>{children}, +})); + +jest.mock('lucide-react', () => ({ + ScrollText: () => , + ChevronDown: () => , + ChevronUp: () => , +})); + +const partialSummary = [{ type: 'text', text: 'Half a summary before the error' }] as never; + +describe('Summary', () => { + beforeEach(() => { + mockMessageContext.current = { isSubmitting: false, isLatestMessage: false }; + }); + + it('does not present a failed round as a completed summary', () => { + /** A failed summarize round keeps the deltas that already streamed in, so + * the only thing separating it from a success is this flag. */ + render(); + + expect(screen.getByText('com_ui_summarize_failed')).toBeInTheDocument(); + expect(screen.queryByText('com_ui_conversation_summarized')).not.toBeInTheDocument(); + }); + + it('labels a settled round with partial text as summarized when it did not fail', () => { + render(); + + expect(screen.getByText('com_ui_conversation_summarized')).toBeInTheDocument(); + expect(screen.queryByText('com_ui_summarize_failed')).not.toBeInTheDocument(); + }); + + it('keeps the in-flight label while the round is still streaming', () => { + mockMessageContext.current = { isSubmitting: true, isLatestMessage: true }; + render(); + + expect(screen.getByText('com_ui_summarizing')).toBeInTheDocument(); + }); + + it('renders nothing when a failed round produced no text at all', () => { + const { container } = render(); + + expect(container).toBeEmptyDOMElement(); + }); +}); diff --git a/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx b/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx index 7e4fa50830..b5f602b8f3 100644 --- a/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx +++ b/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx @@ -8,6 +8,7 @@ import type { TMessageContentParts, Agents, FunctionToolCall, + PartMetadata, } from 'librechat-data-provider'; import type { PartWithIndex } from './ParallelContent'; import { @@ -20,6 +21,7 @@ import { import { useLocalize, useExpandCollapse, scheduleMessageContentLayoutReconcile } from '~/hooks'; import { ToolAuthWarning, ToolAuthWarningContext } from './auth'; import { useMCPIconMap, useMCPServerNames } from '~/hooks/MCP'; +import { resolveToolCallPhase } from '~/utils/toolCallPhase'; import { AttachmentGroup, ReasoningCompact } from './Parts'; import { isError, StackedToolIcons } from './ToolOutput'; import { isBashProgrammaticToolCall } from './routing'; @@ -39,6 +41,34 @@ function hasFailedOutput(output: unknown): boolean { return typeof output === 'string' && isError(output); } +/** + * Group metadata must agree with the individual card, which resolves its + * outcome from the run step's terminal verdict through `resolveToolCallPhase`. + * Reading output alone leaves a step the run closed as `failed` with empty or + * benign output counted as neither failed nor finished, so the group sits on + * "Running" until the response ends and then reports that it ran successfully + * with no failures while the card beside it shows the failure. A closed step + * is never running. Parts carrying no terminal status keep the legacy + * output/progress signal, which is all the older endpoints emit. + */ +function resolveOutcome( + runStepStatus: PartMetadata['runStepStatus'], + completed: boolean, + hasError: boolean, +): Pick { + if (runStepStatus == null) { + return { hasOutput: completed, failed: hasError }; + } + const phase = resolveToolCallPhase({ + runStepStatus, + displayProgress: 1, + reportedProgress: 1, + isSubmitting: false, + hasError, + }); + return { hasOutput: phase !== 'running', failed: phase === 'failed' }; +} + function hasPendingAuth(part: TMessageContentParts): boolean { if (part.type !== ContentTypes.TOOL_CALL) { return false; @@ -63,6 +93,9 @@ function getToolMeta(part: TMessageContentParts): ToolMeta | null { if (!toolCall) { return null; } + /** Terminal verdict lives on the outer `tool_call` object (`PartMetadata`), + * not on the per-variant payload, so it is read before any narrowing cast. */ + const runStepStatus = toolCall.runStepStatus; const isStandard = 'args' in toolCall && (!toolCall.type || toolCall.type === ToolCallTypes.TOOL_CALL); @@ -75,7 +108,11 @@ function getToolMeta(part: TMessageContentParts): ToolMeta | null { const completed = !!tc.output || tc.progress === 1; const name = tc.name ?? ''; const iconName = isBashProgrammaticToolCall(name, tc.args) ? Tools.bash_tool : name; - return { name, iconName, hasOutput: completed, failed: hasFailedOutput(tc.output) }; + return { + name, + iconName, + ...resolveOutcome(runStepStatus, completed, hasFailedOutput(tc.output)), + }; } if (toolCall.type === ToolCallTypes.CODE_INTERPRETER) { @@ -83,8 +120,7 @@ function getToolMeta(part: TMessageContentParts): ToolMeta | null { return { name: 'code_interpreter', iconName: 'code_interpreter', - hasOutput: (ci?.outputs?.length ?? 0) > 0, - failed: false, + ...resolveOutcome(runStepStatus, (ci?.outputs?.length ?? 0) > 0, false), }; } @@ -93,8 +129,7 @@ function getToolMeta(part: TMessageContentParts): ToolMeta | null { return { name: 'file_search', iconName: 'file_search', - hasOutput: !!output, - failed: hasFailedOutput(output), + ...resolveOutcome(runStepStatus, !!output, hasFailedOutput(output)), }; } @@ -103,8 +138,7 @@ function getToolMeta(part: TMessageContentParts): ToolMeta | null { return { name: fn.name, iconName: fn.name, - hasOutput: !!fn.output, - failed: hasFailedOutput(fn.output), + ...resolveOutcome(runStepStatus, !!fn.output, hasFailedOutput(fn.output)), }; } @@ -540,9 +574,12 @@ export default function ToolCallGroup({ const streaming = isSubmitting && idx === lastContentIdx; const isAfterTool = partIndex > 0 && parts[partIndex - 1]?.part.type === ContentTypes.TOOL_CALL; - const label = streaming - ? localize('com_ui_thinking') - : localize('com_ui_thoughts'); + /** Mirrors the standalone `Reasoning` path: the authored + * label wins, generic text is only a fallback. */ + const generatedLabel = part.reasoning_label?.trim(); + const label = + generatedLabel || + (streaming ? localize('com_ui_thinking') : localize('com_ui_thoughts')); return ( { ).toBeInTheDocument(); }); + it('honors a terminal failed run step whose output reads as benign', () => { + const closedFailure = { + type: ContentTypes.TOOL_CALL, + [ContentTypes.TOOL_CALL]: { + id: 'c2', + name: 'create_file', + args: '{}', + output: '', + runStepStatus: 'failed', + }, + } as unknown as TMessageContentParts; + + renderGroup({ + ...baseProps, + isSubmitting: true, + parts: [ + { part: makePart('c1', 'created', 'create_file'), idx: 0 }, + { part: closedFailure, idx: 1 }, + ], + lastContentIdx: 1, + }); + + /** The run closed this step as failed, so the group must read it as + * settled (past tense while still submitting) and count it as a + * failure even though the empty output never parses as an error. */ + expect( + screen.getByRole('button', { name: 'Ran 2 actions, Create File ×2 · 1 failed' }), + ).toBeInTheDocument(); + }); + it('labels a homogeneous ask_user_question group as its own category', () => { renderGroup({ ...baseProps, diff --git a/client/src/components/Chat/ask/options.tsx b/client/src/components/Chat/ask/options.tsx index a9e13a2ccf..88301c58fd 100644 --- a/client/src/components/Chat/ask/options.tsx +++ b/client/src/components/Chat/ask/options.tsx @@ -1,4 +1,5 @@ import { Check } from 'lucide-react'; +import { Button } from '@librechat/client'; import { cn } from '~/utils'; export interface AskOption { @@ -11,6 +12,11 @@ export interface AskOption { * (composer popover and chat card), so the same question looks and behaves * identically wherever it renders. Numbered badges double as the popover's * digit shortcuts; multi-select swaps the number for a check. + * + * Hover, focus-visible and disabled appearance come from the shared `Button` + * primitive rather than being recreated here, so these rows cannot drift from + * it. Only the row geometry and the selected fill stay local, both on + * semantic roles so a customized theme moves them too. */ export default function AskOptions({ options, @@ -38,22 +44,20 @@ export default function AskOptions({ {options.map((option, index) => { const isChecked = multiSelect && checked.includes(index); return ( - + ); })}
diff --git a/client/src/hooks/SSE/useStepHandler.ts b/client/src/hooks/SSE/useStepHandler.ts index c56bef6068..78f6894e09 100644 --- a/client/src/hooks/SSE/useStepHandler.ts +++ b/client/src/hooks/SSE/useStepHandler.ts @@ -1467,8 +1467,13 @@ export default function useStepHandler({ * content-position == step-index invariant that `updateContent` * writes rely on. Flipping the flag alone hides an empty row * (`Summary` renders null without text) and matches the persisted - * message, which retains the part server-side. + * message, which retains the part server-side. An errored round + * carries `failed` so partial deltas that already streamed in are + * not presented as a completed summary. */ + if (completeData.error) { + return { ...part, summarizing: false, failed: true } as SummaryContentPart; + } return { ...part, summarizing: false } as SummaryContentPart; }); if (didFinalize) { diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index 15cea1e1b3..3aeda040e7 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -2354,21 +2354,16 @@ "com_ui_subagent_turn": "Turn", "com_ui_subagent_running": "Running agent", "com_ui_subagent_scroll_to_bottom": "Scroll to latest", - "com_ui_subagent_status_done": "Done", - "com_ui_subagent_status_failed": "Failed", - "com_ui_subagent_status_running": "Running", - "com_ui_subagent_status_stopped": "Stopped", - "com_ui_subagent_thought_count": "{{0}} thoughts", "com_ui_subagent_ticker_error": "Error", "com_ui_subagent_ticker_reasoning": "Reasoning", "com_ui_subagent_ticker_tool_done": "done", "com_ui_subagent_ticker_using": "Using", "com_ui_subagent_ticker_writing": "Writing", - "com_ui_subagent_tool_count": "{{0}} tools", "com_ui_subagent_waiting": "Working…", "com_ui_submit": "Submit", "com_ui_submit_decisions": "Submit {{0}} decisions", "com_ui_submitting": "Submitting…", + "com_ui_summarize_failed": "Summarization failed", "com_ui_summarized": "Summarized", "com_ui_summarizing": "Summarizing...", "com_ui_support_contact": "Support Contact", @@ -2511,7 +2506,6 @@ "com_ui_use_micrphone": "Use microphone", "com_ui_use_prompt": "Use Prompt", "com_ui_used": "Used", - "com_ui_used_n_tools": "Used {{0}} tools", "com_ui_used_one_tool": "Used 1 tool", "com_ui_user": "User", "com_ui_user_group_permissions": "User & Group Permissions", diff --git a/packages/data-provider/src/types/assistants.ts b/packages/data-provider/src/types/assistants.ts index 672d262834..28b45bc904 100644 --- a/packages/data-provider/src/types/assistants.ts +++ b/packages/data-provider/src/types/assistants.ts @@ -714,6 +714,10 @@ export type SummaryContentPart = { content?: Array<{ type: ContentTypes.TEXT; text: string }>; tokenCount?: number; summarizing?: boolean; + /** A summarize round that ended in error. Partial deltas already streamed + * into this slot are kept, so the renderer needs this to avoid presenting + * truncated text under the "Conversation summarized" label. */ + failed?: boolean; summaryVersion?: number; model?: string; provider?: string;