From fa786041b037138bd76d69ce1f9f867772e33b7f Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Fri, 28 Aug 2026 22:06:38 +0200 Subject: [PATCH] fix: persist failed summaries and stop double-rendering memory mutations Stamp the failed marker on the aggregated summary part server-side, mirroring how ON_RUN_STEP_CLOSED already stamps runStepStatus. The SDK aggregator ignores a complete event that carries no summary, so an errored round's partial deltas were saved unmarked and a reload re-rendered the truncated text under "Conversation summarized". Skip memory attachments that carry a toolCallId in the message-level MemoryArtifacts decoration. Those calls already render inline as a MemoryCall card with the same key, value and outcome, so one mutation appeared twice. Attachments with no originating call keep the decoration as their only surface. Keep a reasoning-bearing group expanded when Open Thinking Dropdowns by Default is on. The folded-in THINK part only renders inside the group body, which is not even mounted while collapsed, so the preference had no visible effect until the action group was opened by hand. Give same-named place results unique keys. Several branches of one chain share a name and often carry no identifier, so rows whose address, rating and map target all differ collided on one React key. --- api/server/controllers/agents/callbacks.js | 17 ++++++++++++++ .../Chat/Messages/Content/MemoryArtifacts.tsx | 19 ++++++++++++---- .../Chat/Messages/Content/ToolCallGroup.tsx | 13 ++++++++++- .../__tests__/MemoryArtifacts.test.tsx | 22 +++++++++++++++++++ .../Chat/Messages/Content/verticals.tsx | 6 ++++- 5 files changed, 71 insertions(+), 6 deletions(-) diff --git a/api/server/controllers/agents/callbacks.js b/api/server/controllers/agents/callbacks.js index 0b1d597202..7e1b97a201 100644 --- a/api/server/controllers/agents/callbacks.js +++ b/api/server/controllers/agents/callbacks.js @@ -674,6 +674,23 @@ function getDefaultHandlers({ handlers[GraphEvents.ON_SUMMARIZE_COMPLETE] = { handle: async (_event, data) => { aggregateContent({ event: GraphEvents.ON_SUMMARIZE_COMPLETE, data }); + /** + * Stamped onto the aggregated part for the same reason as + * `runStepStatus` above: an errored round keeps whatever deltas it + * already streamed, and the SDK's aggregator ignores a complete event + * that carries no `summary`, so nothing records the failure. Without + * this the flag exists only on the live client message and a reload + * re-renders the truncated text under "Conversation summarized". + * Resolved through `stepMap` only, so a missing step degrades to the + * old behavior rather than marking an unrelated part. + */ + if (data?.error && contentParts) { + const index = stepMap?.get(data?.id)?.index; + const part = typeof index === 'number' ? contentParts[index] : undefined; + if (part?.type === ContentTypes.SUMMARY) { + part.failed = true; + } + } await emitForJob({ event: GraphEvents.ON_SUMMARIZE_COMPLETE, data, diff --git a/client/src/components/Chat/Messages/Content/MemoryArtifacts.tsx b/client/src/components/Chat/Messages/Content/MemoryArtifacts.tsx index 61d44ac983..387b7de6c3 100644 --- a/client/src/components/Chat/Messages/Content/MemoryArtifacts.tsx +++ b/client/src/components/Chat/Messages/Content/MemoryArtifacts.tsx @@ -8,13 +8,24 @@ import { useExpandCollapse, useLocalize } from '~/hooks'; import MemoryInfo from './MemoryInfo'; import { cn } from '~/utils'; +/** True for a memory attachment with no originating tool call. A call rendered + * inline as a `MemoryCall` card already shows the same key, value and outcome, + * so counting its attachment here too rendered one mutation twice ("Saved + * memory" beside "Updated saved memory"), on reload as well as live. Legacy + * attachments carry no `toolCallId` and keep this decoration as their only + * surface. */ +const isUnlinkedMemoryArtifact = ( + attachment?: TAttachment, +): attachment is TAttachment & { [Tools.memory]: MemoryArtifact } => + attachment?.[Tools.memory] != null && !attachment.toolCallId; + /** Layout-gate predicate for callers that arrange around this component * (e.g. the thinking-dot nudge). Must stay in agreement with the memo's - * collection condition inside the component — both key on - * `attachment[Tools.memory]`. The component itself guards on its memoized + * collection condition inside the component: both key on + * `isUnlinkedMemoryArtifact`. The component itself guards on its memoized * list instead, avoiding a second pass per render. */ export const hasMemoryArtifacts = (attachments?: TAttachment[]): boolean => - attachments?.some((attachment) => attachment?.[Tools.memory] != null) ?? false; + attachments?.some(isUnlinkedMemoryArtifact) ?? false; export default function MemoryArtifacts({ attachments }: { attachments?: TAttachment[] }) { const contentId = useId(); @@ -31,7 +42,7 @@ export default function MemoryArtifacts({ attachments }: { attachments?: TAttach } for (const attachment of attachments) { - if (attachment?.[Tools.memory] != null) { + if (isUnlinkedMemoryArtifact(attachment)) { result.push(attachment[Tools.memory]); if (!hasErrors && attachment[Tools.memory].type === 'error') { diff --git a/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx b/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx index 06625e680c..e85adf9151 100644 --- a/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx +++ b/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx @@ -1,4 +1,5 @@ import { useState, useRef, useMemo, useEffect, useCallback } from 'react'; +import { useAtomValue } from 'jotai'; import { useRecoilValue } from 'recoil'; import { Button } from '@librechat/client'; import { ChevronDown, MessageCircleQuestion, Users } from 'lucide-react'; @@ -25,6 +26,7 @@ import { resolveToolCallPhase } from '~/utils/toolCallPhase'; import { AttachmentGroup, ReasoningCompact } from './Parts'; import { isMemoryFailureOutput } from './Parts/MemoryCall'; import { isError, StackedToolIcons } from './ToolOutput'; +import { showThinkingAtom } from '~/store/showThinking'; import { isBashProgrammaticToolCall } from './routing'; import { ASK_USER_QUESTION } from '~/utils/approval'; import SearchVerticals from './verticals'; @@ -198,6 +200,7 @@ export default function ToolCallGroup({ const localize = useLocalize(); const mcpIconMap = useMCPIconMap(); const mcpServerNames = useMCPServerNames(); + const showThinking = useAtomValue(showThinkingAtom); const rootRef = useRef(null); const cancelLayoutReconcileRef = useRef<(() => void) | null>(null); const retainedForPendingApprovalRef = useRef(false); @@ -327,7 +330,15 @@ export default function ToolCallGroup({ /** Every group has >= 1 tool; collapse a completed one by default just like * a multi-tool group, so a lone tool-with-thinking group (a skill, say) * stays visually consistent with the larger groups around it. */ - const autoCollapse = !autoExpand && allCompleted && (count >= 1 || activityLabelText.length > 0); + /** A folded-in THINK part only renders inside this body, so auto-collapsing + * a completed reasoning-bearing group leaves "Open Thinking Dropdowns by + * Default" with no visible effect until the user opens the action group by + * hand (on reload the body is not even mounted). */ + const autoCollapse = + !autoExpand && + !(showThinking && hasReasoning) && + allCompleted && + (count >= 1 || activityLabelText.length > 0); const initialState = initialExpansionState?.userOverride === true ? initialExpansionState : null; const [isExpanded, setIsExpanded] = useState( initialState?.isExpanded ?? (autoExpand || !autoCollapse), diff --git a/client/src/components/Chat/Messages/Content/__tests__/MemoryArtifacts.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/MemoryArtifacts.test.tsx index 424d40c2b1..98e1cc5158 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/MemoryArtifacts.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/MemoryArtifacts.test.tsx @@ -128,6 +128,28 @@ describe('MemoryArtifacts', () => { expect(button).toHaveClass('text-status-error'); expect(screen.getByText('Memory Error')).toBeInTheDocument(); }); + + test('ignores attachments already rendered as an inline memory tool card', () => { + /** A `set_memory`/`delete_memory` call routes to `MemoryCall`, which + * shows the same key, value and outcome. Counting its attachment here + * too rendered one mutation twice. `toolCallId` is the discriminator. */ + const linked = { + ...createMemoryAttachment('update', 'memory1'), + toolCallId: 'call_abc123', + } as TAttachment; + + const { container } = render(); + + expect(container).toBeEmptyDOMElement(); + }); + + test('still renders a memory attachment carrying no tool call id', () => { + render(); + + fireEvent.click(screen.getByRole('button')); + + expect(screen.getByTestId('memory-artifact-update')).toBeInTheDocument(); + }); }); describe('Collapse/Expand Functionality', () => { diff --git a/client/src/components/Chat/Messages/Content/verticals.tsx b/client/src/components/Chat/Messages/Content/verticals.tsx index 4ae6c8166a..281a634849 100644 --- a/client/src/components/Chat/Messages/Content/verticals.tsx +++ b/client/src/components/Chat/Messages/Content/verticals.tsx @@ -176,7 +176,11 @@ function PlaceList({ places, label }: { places: PlaceResult[]; label: string }) > {places.slice(0, MAX_PLACES).map((place, i) => (
  • 0 && 'border-t border-border-light')} >