From 13877d697824625fe372e270bc5942bfe1cc7c76 Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Fri, 28 Aug 2026 21:44:52 +0200 Subject: [PATCH] fix(client): address eighth round of Codex review findings Compose the shared Button for the compact reasoning disclosure, keeping only the row geometry local so the focus ring and disabled handling come from the shared recipe. Lazy-mount the compact reasoning body with useLazyCollapseBody, as the standalone panel already does. Collapsed is the default whenever thoughts are hidden, and a streaming THINK part re-renders on every delta, so the full text was being mounted and laid out behind an invisible panel. Classify grouped memory failures with isMemoryFailureOutput. Those tools report failure in prose that generic isError parsing does not recognize, so a persisted call with no terminal status showed a failed card inside a group reporting success with no failure count. Announce the diff statistics to assistive technology. The +/- glyphs carry their meaning through color and sign, so aria-hidden left screen-reader users without the change summary; the counts are now also exposed as localized screen-reader text. --- .../Content/Parts/CodeWindowHeader.tsx | 19 +++++-- .../Chat/Messages/Content/Parts/Reasoning.tsx | 52 ++++++++++++------- .../Parts/__tests__/ReasoningCompact.test.tsx | 15 ++++++ .../Chat/Messages/Content/ToolCallGroup.tsx | 11 +++- client/src/locales/en/translation.json | 1 + 5 files changed, 74 insertions(+), 24 deletions(-) diff --git a/client/src/components/Chat/Messages/Content/Parts/CodeWindowHeader.tsx b/client/src/components/Chat/Messages/Content/Parts/CodeWindowHeader.tsx index 693f0ee52b..93ee1851f2 100644 --- a/client/src/components/Chat/Messages/Content/Parts/CodeWindowHeader.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/CodeWindowHeader.tsx @@ -30,10 +30,21 @@ export default function CodeWindowHeader({ language, code, diffStats }: CodeWind {language} {diffStats && ( - + <> + + {/* The glyphs above carry the meaning visually through color and + sign, which neither reaches a screen reader, so the counts are + announced as words instead of being dropped. */} + + {localize('com_ui_diff_stats', { + 0: String(diffStats.additions), + 1: String(diffStats.deletions), + })} + + )} diff --git a/client/src/components/Chat/Messages/Content/Parts/Reasoning.tsx b/client/src/components/Chat/Messages/Content/Parts/Reasoning.tsx index d05a121429..9b46300939 100644 --- a/client/src/components/Chat/Messages/Content/Parts/Reasoning.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/Reasoning.tsx @@ -3,7 +3,7 @@ import copy from 'copy-to-clipboard'; import { useAtomValue } from 'jotai'; import { Lightbulb, ChevronDown } from 'lucide-react'; import { ContentTypes } from 'librechat-data-provider'; -import { disclosureChevronVariants } from '@librechat/client'; +import { Button, disclosureChevronVariants } from '@librechat/client'; import type { MouseEvent, FocusEvent } from 'react'; import { ThinkingContent, ThinkingButton, ThinkingLabel, FloatingThinkingBar, useInViewport } from './Thinking'; import { useLocalize, useExpandCollapse, useLazyCollapseBody } from '~/hooks'; @@ -292,13 +292,22 @@ export const ReasoningCompact = memo( const containerRef = useRef(null); const { ref: headerRef, inViewport: headerInViewport } = useInViewport(); const { style: expandStyle, ref: expandRef } = useExpandCollapse(isExpanded); + /** Collapsed is the default whenever thoughts are hidden, and a streaming + * THINK part re-renders on every delta. Keeping the full text mounted + * behind the invisible panel made long reasoning streams progressively + * more expensive on top of the visible peek. */ + const { shouldRenderBody, mountBody, handleTransitionEnd } = useLazyCollapseBody(isExpanded); const reasoningText = useMemo(() => stripThinkTags(reasoning), [reasoning]); - const handleToggle = useCallback((e: MouseEvent) => { - e.preventDefault(); - setIsExpanded((prev) => !prev); - }, []); + const handleToggle = useCallback( + (e: MouseEvent) => { + e.preventDefault(); + mountBody(); + setIsExpanded((prev) => !prev); + }, + [mountBody], + ); const handleCopy = useCallback(() => { if (!copy(reasoningText, { format: 'text/plain' })) { @@ -334,12 +343,12 @@ export const ReasoningCompact = memo( onBlur={handleBlur} >
- + {isExpanded && (
-
-

{reasoningText}

- -
+ {shouldRenderBody && ( +
+

+ {reasoningText} +

+ +
+ )}
diff --git a/client/src/components/Chat/Messages/Content/Parts/__tests__/ReasoningCompact.test.tsx b/client/src/components/Chat/Messages/Content/Parts/__tests__/ReasoningCompact.test.tsx index 33d2ba48c8..c41d4161f7 100644 --- a/client/src/components/Chat/Messages/Content/Parts/__tests__/ReasoningCompact.test.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/__tests__/ReasoningCompact.test.tsx @@ -8,6 +8,11 @@ jest.mock('~/hooks', () => ({ style: { display: 'grid', gridTemplateRows: isExpanded ? '1fr' : '0fr' }, ref: { current: null }, }), + useLazyCollapseBody: (isExpanded: boolean) => ({ + shouldRenderBody: isExpanded, + mountBody: jest.fn(), + handleTransitionEnd: jest.fn(), + }), })); jest.mock('../Thinking', () => ({ @@ -59,4 +64,14 @@ describe('ReasoningCompact', () => { expect(row).toHaveClass('mb-1', 'mt-0'); expect(row).not.toHaveClass('mt-1'); }); + + it('leaves the full reasoning text unmounted while collapsed', () => { + /** Collapsed is the default whenever thoughts are hidden, and a streaming + * THINK part re-renders on every delta, so keeping the whole text mounted + * behind the invisible panel cost layout work for nothing. */ + render(); + + expect(screen.getByRole('button', { name: 'Thoughts' })).toBeInTheDocument(); + expect(screen.queryByText('A long stream of reasoning')).not.toBeInTheDocument(); + }); }); diff --git a/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx b/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx index 020a8766a6..e316e8bca5 100644 --- a/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx +++ b/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx @@ -23,6 +23,7 @@ import { ToolAuthWarning, ToolAuthWarningContext } from './auth'; import { useMCPIconMap, useMCPServerNames } from '~/hooks/MCP'; import { resolveToolCallPhase } from '~/utils/toolCallPhase'; import { AttachmentGroup, ReasoningCompact } from './Parts'; +import { isMemoryFailureOutput } from './Parts/MemoryCall'; import { isError, StackedToolIcons } from './ToolOutput'; import { isBashProgrammaticToolCall } from './routing'; import { ASK_USER_QUESTION } from '~/utils/approval'; @@ -115,10 +116,18 @@ 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; + /** Memory tools report failure in prose ("Invalid key ...") that generic + * `isError` parsing does not recognize, so `MemoryCall` classifies it with + * its own predicate. Reuse that here or a persisted call with no terminal + * status shows a failed card inside a group claiming success. */ + const failedOutput = + name === 'set_memory' || name === 'delete_memory' + ? isMemoryFailureOutput(name, tc.output ?? '') + : hasFailedOutput(tc.output); return { name, iconName, - ...resolveOutcome(runStepStatus, completed, hasFailedOutput(tc.output)), + ...resolveOutcome(runStepStatus, completed, failedOutput), }; } diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index 3aeda040e7..51fc7dcba3 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -1197,6 +1197,7 @@ "com_ui_description_placeholder": "Optional: Enter a description to display for the prompt", "com_ui_detailed": "Detailed", "com_ui_details": "Details", + "com_ui_diff_stats": "{{0}} additions, {{1}} deletions", "com_ui_disabled": "Disabled", "com_ui_disabling": "Disabling...", "com_ui_dismiss": "Dismiss",