mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-09-02 12:01:14 +00:00
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.
This commit is contained in:
parent
25b9d5e3c9
commit
fa786041b0
5 changed files with 71 additions and 6 deletions
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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') {
|
||||
|
|
|
|||
|
|
@ -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<HTMLDivElement | null>(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),
|
||||
|
|
|
|||
|
|
@ -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(<MemoryArtifacts attachments={[linked]} />);
|
||||
|
||||
expect(container).toBeEmptyDOMElement();
|
||||
});
|
||||
|
||||
test('still renders a memory attachment carrying no tool call id', () => {
|
||||
render(<MemoryArtifacts attachments={[createMemoryAttachment('update', 'memory1')]} />);
|
||||
|
||||
fireEvent.click(screen.getByRole('button'));
|
||||
|
||||
expect(screen.getByTestId('memory-artifact-update')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Collapse/Expand Functionality', () => {
|
||||
|
|
|
|||
|
|
@ -176,7 +176,11 @@ function PlaceList({ places, label }: { places: PlaceResult[]; label: string })
|
|||
>
|
||||
{places.slice(0, MAX_PLACES).map((place, i) => (
|
||||
<li
|
||||
key={place.identifier || place.name || i}
|
||||
/** Several branches of one chain share a name and often carry no
|
||||
* identifier, so name alone collides across rows whose address,
|
||||
* rating and map target all differ. Verticals never stream, so the
|
||||
* index is a stable tiebreaker for a snapshot-rendered list. */
|
||||
key={place.identifier || `${place.name ?? ''}|${place.address ?? ''}|${i}`}
|
||||
className={cn(i > 0 && 'border-t border-border-light')}
|
||||
>
|
||||
<a
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue