diff --git a/api/server/controllers/agents/callbacks.js b/api/server/controllers/agents/callbacks.js index a42afb36257..9858b4d69ee 100644 --- a/api/server/controllers/agents/callbacks.js +++ b/api/server/controllers/agents/callbacks.js @@ -9,6 +9,7 @@ const { ErrorTypes, UsageEvents, getRunStepDurationMs, + getRunStepCloseMetadata, } = require('librechat-data-provider'); const { GraphEvents, @@ -595,6 +596,7 @@ function getDefaultHandlers({ const part = typeof index === 'number' ? contentParts[index] : undefined; if (part?.type === ContentTypes.TOOL_CALL && part.tool_call) { part.tool_call.runStepStatus = data.status; + Object.assign(part.tool_call, getRunStepCloseMetadata(data)); /** * The raw derivable duration, left unset rather than zeroed when * the event cannot support a trustworthy one — no `created_at`, diff --git a/client/src/components/Chat/Messages/Content/ActivityPhaseGroup.tsx b/client/src/components/Chat/Messages/Content/ActivityPhaseGroup.tsx index aa67c094f5b..8c6883891e6 100644 --- a/client/src/components/Chat/Messages/Content/ActivityPhaseGroup.tsx +++ b/client/src/components/Chat/Messages/Content/ActivityPhaseGroup.tsx @@ -1,6 +1,7 @@ import { memo, useId, useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { useAtomValue } from 'jotai'; import { Button } from '@librechat/client'; +import { useTranslation } from 'react-i18next'; import { ContentTypes } from 'librechat-data-provider'; import { Check, Lightbulb, ChevronDown, TriangleAlert } from 'lucide-react'; import type { TAttachment, TMessageContentParts } from 'librechat-data-provider'; @@ -28,11 +29,13 @@ import { useMCPIconMap, useMCPServerNames } from '~/hooks/MCP'; import { getActivityLabelText } from '~/utils/activityLabels'; import { getOutcomeStatus, summarizeSpan } from './outcome'; import { sandboxStartingByToolCallId } from '~/store'; +import useClockFormat from '~/hooks/useClockFormat'; +import { cn, getMessageTimestamp } from '~/utils'; import { StackedToolIcons } from './ToolOutput'; +import useTimeTick from '~/hooks/useTimeTick'; import { getSourceDomains } from './sources'; import { mapAttachments } from '~/utils/map'; import SearchVerticals from './verticals'; -import { cn } from '~/utils'; /** Matches `EXPAND_TRANSITION` so the panel and the label ticker resolve on * the same curve — two properties animating on two different easings is what @@ -319,6 +322,7 @@ function LivePhaseHeader({ * fail while a later one runs, and the line alone would never say so. The * hidden group header carries the same counts in the same words. */ const { failed, cancelled } = activity.outcome; + const total = activity.total; let combo = ''; if (painted.comboCount > 1) { combo = painted.isBackgroundTaskCheck @@ -327,9 +331,7 @@ function LivePhaseHeader({ } const failedNote = failed > 0 - ? localize(failed === 1 ? 'com_ui_one_action_failed' : 'com_ui_n_actions_failed', { - 0: String(failed), - }) + ? localize('com_ui_n_of_n_actions_failed', { 0: String(failed), 1: String(total) }) : ''; const cancelledNote = cancelled > 0 @@ -435,6 +437,30 @@ function LivePhaseHeader({ * readable, and one click away, without unfolding. Its own component so only * a collapsed card with a failure pays for the line's lookups. */ +function FailedPeekTime({ failedAt }: { failedAt: number | Date }) { + useTimeTick(); + const { i18n } = useTranslation(); + const hour12 = useClockFormat(); + const date = new Date(failedAt); + if (!Number.isFinite(date.getTime()) || date.getTime() > Date.now()) { + return null; + } + const timestamp = getMessageTimestamp(date.toISOString(), i18n.language, hour12); + if (timestamp == null) { + return null; + } + return ( + + ); +} + function FailedPeek({ parts, attachmentsById, @@ -476,6 +502,7 @@ function FailedPeek({ {first.detail} )} + {first.failedAt != null && } {count > 1 && ( {localize('com_ui_plus_n_more', { 0: String(count - 1) })} @@ -529,8 +556,9 @@ export default function ActivityPhaseGroup({ /** The span's failed calls, for the peek under a collapsed header and the * pill beside it. Read from the same parts the header's glyph and live * line read, so the three can never disagree about the count. */ - const failedCount = useMemo( - () => (outcomeParts == null ? 0 : summarizeSpan(outcomeParts, attachmentsById).failed), + const { failed: failedCount, total: toolCount } = useMemo( + () => + outcomeParts == null ? { failed: 0, total: 0 } : summarizeSpan(outcomeParts, attachmentsById), [outcomeParts, attachmentsById], ); @@ -830,7 +858,7 @@ export default function ActivityPhaseGroup({ aria-hidden="true" /> - + {peek} diff --git a/client/src/components/Chat/Messages/Content/ContentParts.tsx b/client/src/components/Chat/Messages/Content/ContentParts.tsx index 70bd2ea46c6..5e3b3993673 100644 --- a/client/src/components/Chat/Messages/Content/ContentParts.tsx +++ b/client/src/components/Chat/Messages/Content/ContentParts.tsx @@ -258,6 +258,8 @@ type ContentPartsProps = { hideAttachments?: boolean; /** Internal signal that this segment renders inside a completed phase card. */ withinActivityPhase?: boolean; + /** The parent phase owns the failure pill, including while it is live. */ + parentPhaseOwnsFailurePill?: boolean; /** Internal signal that a phase card already carries this message's * streaming cursor. A solitary empty provider slot looks like the initial * waiting state from inside its own segment, so without this it renders a @@ -309,6 +311,7 @@ const ContentPartsBody = memo(function ContentPartsBody({ foldLiveActivity = true, nestedActivityPhase = false, withinActivityPhase = false, + parentPhaseOwnsFailurePill = false, cursorOwnedElsewhere = false, hideAttachments = false, workspaceAttachmentsPartitioned = false, @@ -871,6 +874,7 @@ const ContentPartsBody = memo(function ContentPartsBody({ withinPhase = false, ownsCursor = false, hoisted = false, + underPhase = false, ) => { return ( ); @@ -1177,6 +1183,7 @@ const ContentPartsBody = memo(function ContentPartsBody({ onExpansionChange={(state) => handleGroupExpansionChange(groupId, state)} labelPart={group.labelPart} withinActivityPhase={withinActivityPhase} + parentPhaseOwnsFailurePill={parentPhaseOwnsFailurePill} />, ); return nodes; diff --git a/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx b/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx index c0080bf4cd5..f54ecf7fc8b 100644 --- a/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx +++ b/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx @@ -63,6 +63,8 @@ interface ToolCallGroupProps { * blocks the run, and hiding that card behind a second collapsed * disclosure would bury the action the run is waiting on. */ withinActivityPhase?: boolean; + /** The phase header owns the failure pill even while its live groups stay expandable. */ + parentPhaseOwnsFailurePill?: boolean; } export type ToolCallGroupExpansionState = { @@ -83,6 +85,7 @@ export default function ToolCallGroup({ onExpansionChange, labelPart, withinActivityPhase = false, + parentPhaseOwnsFailurePill = false, }: ToolCallGroupProps) { const localize = useLocalize(); const mcpIconMap = useMCPIconMap(); @@ -448,12 +451,10 @@ export default function ToolCallGroup({ } const failedNote = activitySummary.failedCount > 0 - ? localize( - activitySummary.failedCount === 1 - ? 'com_ui_one_action_failed' - : 'com_ui_n_actions_failed', - { 0: String(activitySummary.failedCount) }, - ) + ? localize('com_ui_n_of_n_actions_failed', { + 0: String(activitySummary.failedCount), + 1: String(count), + }) : ''; if (failedNote !== '') { groupDetailParts.push(failedNote); @@ -479,7 +480,8 @@ export default function ToolCallGroup({ * the header, which is also the way to the failed rows; the text keeps it * only for the accessible name. Inside a phase the pill is the phase's, * so the group's detail says it in text. */ - const showsFailurePill = !withinActivityPhase && activitySummary.failedCount > 0; + const showsFailurePill = + !withinActivityPhase && !parentPhaseOwnsFailurePill && activitySummary.failedCount > 0; const visibleGroupDetail = showsFailurePill ? groupDetailParts.filter((part) => part && part !== failedNote).join(' · ') : groupDetail; @@ -578,8 +580,12 @@ export default function ToolCallGroup({ aria-hidden="true" /> - {!withinActivityPhase && ( - + {!withinActivityPhase && !parentPhaseOwnsFailurePill && ( + )}
{ const expandCollapse = jest.requireActual('~/hooks/Messages/useExpandCollapse'); const lazyCollapseBody = jest.requireActual('~/hooks/Messages/useLazyCollapseBody'); return { - useLocalize: () => (key: string) => key, + useLocalize: () => (key: string, values?: Record) => + key === 'com_ui_n_of_n_actions_failed' ? `${values?.[0]}/${values?.[1]} failed` : key, useExpandCollapse: expandCollapse.default, useLazyCollapseBody: lazyCollapseBody.default, EXPAND_TRANSITION: expandCollapse.EXPAND_TRANSITION, @@ -469,6 +470,25 @@ describe('ActivityPhaseGroup failure fast path', () => { return
; }; + test('counts only tool calls in the phase, not reasoning, labels or missing stream slots', () => { + const thought = { + type: ContentTypes.THINK, + think: 'Checking a source', + } as TMessageContentParts; + render( + +
+ , + ); + + expect(screen.getByTestId('failed-reveal-pill')).toHaveTextContent('1/3 failed'); + expect(screen.getByRole('button', { name: 'com_ui_show_failed_one_of_n' })).toBeInTheDocument(); + }); + test('peeks the first failed call under a collapsed card and reaches it in one click', () => { const onReveal = jest.fn(); render( @@ -479,6 +499,7 @@ describe('ActivityPhaseGroup failure fast path', () => { const peek = screen.getByTestId('activity-phase-failed-peek'); expect(peek).toHaveTextContent('HTTP 429'); expect(peek).toHaveTextContent('com_ui_show_error'); + expect(screen.queryByTestId('activity-phase-failed-time')).not.toBeInTheDocument(); expect(screen.queryByTestId('phase-content')).not.toBeInTheDocument(); fireEvent.click(peek); @@ -488,6 +509,23 @@ describe('ActivityPhaseGroup failure fast path', () => { expect(onReveal).toHaveBeenCalledTimes(1); }); + test('shows when the first failure happened, even after the conversation is restored', () => { + const failedAt = Date.now() - 2 * 60_000; + const timed = toPart( + { name: 'fetch_page', runStepStatus: 'failed', runStepClosedAt: failedAt }, + 'timed', + ); + render( + +
+ , + ); + + const time = screen.getByTestId('activity-phase-failed-time'); + expect(time).toHaveAttribute('dateTime', new Date(failedAt).toISOString()); + expect(time).toHaveTextContent(/2 minutes ago/); + }); + test('keeps the error action and remaining count outside the shrinking peek label', () => { render( @@ -514,12 +552,23 @@ describe('ActivityPhaseGroup failure fast path', () => { , ); fireEvent.click(screen.getByRole('button', { name: LABEL })); - const pill = screen.getByRole('button', { name: 'com_ui_show_failed_n' }); + const pill = screen.getByRole('button', { name: 'com_ui_show_failed_n_of_n' }); + expect(pill).toHaveTextContent('2/2 failed'); fireEvent.click(pill); expect(onReveal).toHaveBeenCalledTimes(1); expect(screen.getByRole('button', { name: LABEL })).toHaveAttribute('aria-expanded', 'true'); }); + test('announces the same failed-over-total count while later calls are still running', () => { + render( + +
+ , + ); + expect(screen.getByTestId('failed-reveal-pill')).toHaveTextContent('1/2 failed'); + expect(screen.getByTestId('live-phase-outcome')).toHaveTextContent('1/2 failed'); + }); + test('a card with no failure shows neither pill nor peek', () => { render( diff --git a/client/src/components/Chat/Messages/Content/__tests__/ContentParts.integration.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/ContentParts.integration.test.tsx index 41905ee1e4d..0016a385e82 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/ContentParts.integration.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/ContentParts.integration.test.tsx @@ -13,6 +13,9 @@ jest.mock('~/hooks', () => ({ if (key === 'com_ui_running_n_actions') { return `Running ${values?.[0]} actions`; } + if (key === 'com_ui_n_of_n_actions_failed') { + return `${values?.[0]}/${values?.[1]} failed`; + } return key; }, useExpandCollapse: (isExpanded: boolean) => ({ @@ -652,6 +655,42 @@ describe('ContentParts — synthesized activity folds', () => { ); }); + it('shows one failure pill when an expanded live phase contains a running tool group', () => { + const failed = { + type: ContentTypes.TOOL_CALL, + tool_call: { + id: 't1', + name: `getTinyImage${MCP_DELIMITER}Everything`, + args: '{}', + output: 'image_returned', + runStepStatus: 'failed', + }, + } as TMessageContentParts; + const live = [failed, makeMcpToolCall('t2', false)]; + const props = { ...baseProps, isSubmitting: true, content: live }; + const { rerender } = renderContentParts(props); + + const phase = screen.getByTestId('activity-phase-card'); + expect(within(phase).getByTestId('failed-reveal-pill')).toHaveTextContent('1/2 failed'); + fireEvent.click(within(phase).getAllByRole('button')[0]); + + const group = screen.getByRole('button', { name: /Running 2 actions.*1\/2 failed/ }); + expect(group).toHaveAttribute('aria-expanded', 'true'); + expect(group).toHaveTextContent('1/2 failed'); + expect(screen.getAllByTestId('failed-reveal-pill')).toHaveLength(1); + + rerender( + + + , + ); + expect(screen.getAllByTestId('failed-reveal-pill')).toHaveLength(1); + }); + it('keeps the in-flight tool call inside the card while the run streams', () => { renderContentParts({ ...baseProps, diff --git a/client/src/components/Chat/Messages/Content/__tests__/LiveParity.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/LiveParity.test.tsx index 439b494a29c..fbea93b2e8e 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/LiveParity.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/LiveParity.test.tsx @@ -295,7 +295,7 @@ describe('live fold parity with the cards it hides', () => { mount([earlier, later], [artifact], true); const button = within(screen.getByTestId('activity-phase-card')).getAllByRole('button')[0]; - expect(within(button).getByTestId('live-phase-outcome')).toHaveTextContent('1 failed'); + expect(within(button).getByTestId('live-phase-outcome')).toHaveTextContent('1/2 failed'); expect(button).not.toHaveTextContent(/^Failed/); }); @@ -364,7 +364,7 @@ describe('live fold parity with the cards it hides', () => { }); it.each([ - ['error', 'Failed: Background tasks · 1 failed'], + ['error', 'Failed: Background tasks · 1/1 failed'], ['cancelled', 'Cancelled · 1 cancelled'], ])('keeps the %s verdict of a polled background task in the live fold', (status, label) => { const output = JSON.stringify({ @@ -466,13 +466,13 @@ describe('live fold parity with the cards it hides', () => { const outcome = screen.getByTestId('live-phase-outcome'); expect(combo).toHaveTextContent('×2'); - expect(outcome).toHaveTextContent('1 failed'); + expect(outcome).toHaveTextContent('1/2 failed'); /** The count reads with the line; the verdict stays where the row ends. */ expect(combo.compareDocumentPosition(outcome) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); expect(combo.parentElement).not.toContainElement(outcome); expect( within(screen.getByTestId('activity-phase-card')).getAllByRole('button')[0], - ).toHaveAccessibleName(/×2 · 1 failed$/); + ).toHaveAccessibleName(/×2 · 1\/2 failed$/); }); it('changes the multiplier with the throttled status line', () => { @@ -552,7 +552,7 @@ describe('live fold parity with the cards it hides', () => { * the tool, because "Failed lookup ×2" would blame both calls. */ expect(header).toHaveAccessibleName(/^Failed: lookup/); expect(screen.queryByTestId('live-phase-combo')).toBeNull(); - expect(screen.getByTestId('live-phase-outcome')).toHaveTextContent('1 failed'); + expect(screen.getByTestId('live-phase-outcome')).toHaveTextContent('1/2 failed'); }); it('resets the multiplier across an agent handoff', () => { @@ -815,8 +815,8 @@ describe('live fold parity with the cards it hides', () => { true, ); fireEvent.click(screen.getByRole('button', { name: 'Reviewed the work' })); - const group = screen.getByRole('button', { name: /Ran 2 actions.*1 failed/ }); - expect(group).toHaveAccessibleName(/1 failed/); + const group = screen.getByRole('button', { name: /Ran 2 actions.*1\/2 failed/ }); + expect(group).toHaveAccessibleName(/1\/2 failed/); expect(group.querySelector('.lucide-triangle-alert')).not.toBeNull(); }); @@ -883,8 +883,8 @@ describe('live fold parity with the cards it hides', () => { const button = within(screen.getByTestId('activity-phase-card')).getAllByRole('button')[0]; expect(button).toHaveTextContent('Querying the graph'); - expect(within(button).getByTestId('live-phase-outcome')).toHaveTextContent('1 failed'); - expect(button).toHaveAccessibleName(/Querying the graph.*1 failed/); + expect(within(button).getByTestId('live-phase-outcome')).toHaveTextContent('1/2 failed'); + expect(button).toHaveAccessibleName(/Querying the graph.*1\/2 failed/); }); it('announces a failure on the SAME call at once, without waiting for another source', () => { @@ -914,7 +914,7 @@ describe('live fold parity with the cards it hides', () => { jest.advanceTimersByTime(500); }); - expect(announcer()).toHaveTextContent('1 failed'); + expect(announcer()).toHaveTextContent('1/1 failed'); expect(announcer().closest('button')).toBeNull(); }); @@ -1284,7 +1284,7 @@ describe('live activity hardening transitions', () => { jest.advanceTimersByTime(500); }); const expected = { - error: /Failed.*1 failed/, + error: /Failed.*1\/1 failed/, cancelled: /Cancelled.*1 cancelled/, completed: 'Finished in background', }[status]; @@ -1313,8 +1313,10 @@ describe('live activity hardening transitions', () => { ...calls.slice(1), ]), ); - expect(screen.getAllByRole('button')[0]).toHaveAccessibleName(/Looking up item 1023.*1 failed/); - expect(screen.getByTestId('activity-phase-announcer')).toHaveTextContent('1 failed'); + expect(screen.getAllByRole('button')[0]).toHaveAccessibleName( + /Looking up item 1023.*1\/1024 failed/, + ); + expect(screen.getByTestId('activity-phase-announcer')).toHaveTextContent('1/1024 failed'); }); it('owns exactly one polite region across live-to-settled replacement', () => { diff --git a/client/src/components/Chat/Messages/Content/__tests__/ToolCallGroup.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/ToolCallGroup.test.tsx index 171f4a51d6d..f7a811d659f 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/ToolCallGroup.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/ToolCallGroup.test.tsx @@ -30,11 +30,8 @@ jest.mock('~/hooks', () => ({ if (key === 'com_ui_background_tasks_n_checks') { return `${values?.[0]} checks`; } - if (key === 'com_ui_n_actions_failed') { - return `${values?.[0]} failed`; - } - if (key === 'com_ui_one_action_failed') { - return '1 failed'; + if (key === 'com_ui_n_of_n_actions_failed') { + return `${values?.[0]}/${values?.[1]} failed`; } if (key === 'com_ui_n_actions_cancelled') { return `${values?.[0]} cancelled`; @@ -883,7 +880,7 @@ describe('ToolCallGroup image hoisting', () => { expect( screen.getByRole('button', { - name: 'Ran 3 actions, Create File ×2, Edit File · 1 failed', + name: 'Ran 3 actions, Create File ×2, Edit File · 1/3 failed', }), ).toBeInTheDocument(); }); @@ -955,9 +952,9 @@ describe('ToolCallGroup image hoisting', () => { }); it.each([ - ['error', '1 failed'], + ['error', '1/1 failed'], ['cancelled', '1 cancelled'], - ['interrupted', '1 failed'], + ['interrupted', '1/1 failed'], ])('reflects a %s task poll in the collapsed group', (status, suffix) => { renderGroup({ ...baseProps, @@ -995,7 +992,7 @@ describe('ToolCallGroup image hoisting', () => { Object.assign(part[ContentTypes.TOOL_CALL] ?? {}, { runStepStatus: 'completed' }); renderGroup({ ...baseProps, parts: [{ part, idx: 0 }], lastContentIdx: 0 }); expect( - screen.getByRole('button', { name: 'Checked background tasks, 1 failed' }), + screen.getByRole('button', { name: 'Checked background tasks, 1/1 failed' }), ).toBeInTheDocument(); }, ); @@ -1016,7 +1013,7 @@ describe('ToolCallGroup image hoisting', () => { lastContentIdx: 0, }); expect( - screen.getByRole('button', { name: 'Checked background tasks, 1 failed' }), + screen.getByRole('button', { name: 'Checked background tasks, 1/1 failed' }), ).toBeInTheDocument(); }); @@ -1058,14 +1055,14 @@ describe('ToolCallGroup image hoisting', () => { })), }; const { rerender } = renderGroup(props); - expect(screen.getByRole('button', { name: /· 1 failed$/ })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /· 1\/2 failed$/ })).toBeInTheDocument(); rerender( , ); - expect(screen.getByRole('button', { name: /· 1 failed$/ })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /· 1\/2 failed$/ })).toBeInTheDocument(); }); it('honors a terminal failed run step whose output reads as benign', () => { @@ -1094,7 +1091,7 @@ describe('ToolCallGroup image hoisting', () => { * 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' }), + screen.getByRole('button', { name: 'Ran 2 actions, Create File ×2 · 1/2 failed' }), ).toBeInTheDocument(); }); @@ -1308,10 +1305,10 @@ describe('ToolCallGroup failure fast path', () => { it('opens the group and asks its rows to open from the pill beside a standalone header', () => { const onReveal = jest.fn(); renderGroup(props(onReveal)); - const header = screen.getByRole('button', { name: /· 1 failed$/ }); + const header = screen.getByRole('button', { name: /· 1\/2 failed$/ }); expect(header).toHaveAttribute('aria-expanded', 'false'); - fireEvent.click(screen.getByRole('button', { name: 'com_ui_show_failed_one' })); + fireEvent.click(screen.getByRole('button', { name: 'com_ui_show_failed_one_of_n' })); expect(header).toHaveAttribute('aria-expanded', 'true'); /** Once per row the group rendered, after those rows mounted. */ @@ -1320,14 +1317,29 @@ describe('ToolCallGroup failure fast path', () => { it('shows the failure count once, on the pill, when it stands alone', () => { renderGroup(props(jest.fn())); - const header = screen.getByRole('button', { name: /· 1 failed$/ }); - expect(header).not.toHaveTextContent('1 failed'); - expect(screen.getByTestId('failed-reveal-pill')).toHaveTextContent('1 failed'); + const header = screen.getByRole('button', { name: /· 1\/2 failed$/ }); + expect(header).not.toHaveTextContent('1/2 failed'); + expect(screen.getByTestId('failed-reveal-pill')).toHaveTextContent('1/2 failed'); }); it("keeps the count in text inside a phase, where the pill is the phase's", () => { renderGroup({ ...props(jest.fn()), withinActivityPhase: true }); - expect(screen.getByRole('button', { name: /· 1 failed$/ })).toHaveTextContent('1 failed'); + expect(screen.getByRole('button', { name: /· 1\/2 failed$/ })).toHaveTextContent('1/2 failed'); + }); + + it('leaves the pill to a live phase without collapsing its running group', () => { + renderGroup({ + ...props(jest.fn()), + parts: [...failedParts, { part: makePart('c3', '', 'create_file'), idx: 2 }], + lastContentIdx: 2, + isSubmitting: true, + parentPhaseOwnsFailurePill: true, + }); + + const group = screen.getByRole('button', { name: /1\/3 failed$/ }); + expect(group).toHaveAttribute('aria-expanded', 'true'); + expect(group).toHaveTextContent('1/3 failed'); + expect(screen.queryByTestId('failed-reveal-pill')).not.toBeInTheDocument(); }); it('leaves the pill to the phase header when nested in one', () => { @@ -1344,7 +1356,7 @@ describe('ToolCallGroup failure fast path', () => { , ); - const header = screen.getByRole('button', { name: /· 1 failed$/ }); + const header = screen.getByRole('button', { name: /· 1\/2 failed$/ }); expect(header).toHaveAttribute('aria-expanded', 'false'); rerender( @@ -1358,7 +1370,7 @@ describe('ToolCallGroup failure fast path', () => { it('sets an open header in the primary colour over railed rows', () => { renderGroup(props(jest.fn())); - const header = screen.getByRole('button', { name: /· 1 failed$/ }); + const header = screen.getByRole('button', { name: /· 1\/2 failed$/ }); expect(header).not.toHaveClass('text-text-primary'); fireEvent.click(header); expect(header).toHaveClass('text-text-primary'); diff --git a/client/src/components/Chat/Messages/Content/__tests__/failed.test.ts b/client/src/components/Chat/Messages/Content/__tests__/failed.test.ts index 36e9119212b..e30ca82985a 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/failed.test.ts +++ b/client/src/components/Chat/Messages/Content/__tests__/failed.test.ts @@ -61,6 +61,42 @@ describe('getFailedLines', () => { ]); }); + it('records a host close time for a failed tool step', () => { + const failedAt = Date.now() - 60_000; + expect( + getFailedLines( + [toPart({ name: 'lookup', runStepStatus: 'failed', runStepClosedAt: failedAt }, 'failed')], + localize, + [], + ), + ).toEqual([{ text: 'Failed: lookup', detail: '', iconName: 'lookup', failedAt }]); + }); + + it('uses a detached task settlement time instead of the earlier dispatch time', () => { + const settledAt = new Date(); + const call = toPart( + { + name: 'lookup', + output: 'Error: tool call failed: timed out', + runStepClosedAt: Date.now() - 120_000, + backgrounded: true, + backgroundTask: { settledAt }, + }, + 'detached', + ); + expect(getFailedLines([call], localize, [])[0].failedAt).toBe(settledAt); + const withoutReceipt = toPart( + { + name: 'lookup', + output: 'Error: tool call failed: timed out', + backgrounded: true, + runStepClosedAt: Date.now() - 120_000, + }, + 'pending', + ); + expect(getFailedLines([withoutReceipt], localize, [])[0].failedAt).toBeUndefined(); + }); + it('is empty when nothing failed', () => { expect( getFailedLines([toPart({ name: 'lookup', output: 'rows' }, 'ok')], localize, []), diff --git a/client/src/components/Chat/Messages/Content/live.ts b/client/src/components/Chat/Messages/Content/live.ts index 8d9bcaabf5a..30a84e33dba 100644 --- a/client/src/components/Chat/Messages/Content/live.ts +++ b/client/src/components/Chat/Messages/Content/live.ts @@ -42,13 +42,15 @@ export type LiveActivity = { isBackgroundTaskCheck?: boolean; /** Failed and stopped calls anywhere in the span, not just the newest line. */ outcome: SpanOutcome; + /** All calls in the span, including ones still running. */ + total: number; }; type Localize = (phraseKey: TranslationKeys, options?: TOptions) => string; type LiveToolCall = Agents.ToolCall & { subagent_content?: TMessageContentParts[] } & Pick< PartMetadata, - 'runStepStatus' + 'runStepStatus' | 'runStepClosedAt' | 'backgrounded' > & { progress?: number }; /** @@ -393,6 +395,7 @@ export function getLiveActivity( return { ...newestLine(parts, localize, serverNames, span, preferLabels), outcome: { failed: span.failed, cancelled: span.cancelled }, + total: span.total, iconNames: getSpanIconNames(parts), }; } @@ -403,6 +406,8 @@ export type FailedLine = { /** The first line of what the tool returned, with the error prefix removed. */ detail: string; iconName: string; + /** The failure time, if the host recorded it. Detached tasks use settlement, not dispatch. */ + failedAt?: number | Date; }; const PROCESSING_PREFIX = /^Error processing tool:?\s*/i; @@ -456,10 +461,14 @@ export function getFailedLines( const subject = getToolCallIntent(toolCall.args) ?? (parsed.mcpServer ? parsed.toolName : getToolDisplayLabel(parsed.raw, localize, serverNames)); + const failedAt = + toolCall.backgroundTask?.settledAt ?? + (meta.background != null || toolCall.backgrounded ? undefined : toolCall.runStepClosedAt); lines.push({ text: subject ? localize('com_ui_failed_subject', { 0: subject }) : localize('com_ui_failed'), detail: firstErrorLine(toolCall.output), iconName: meta.iconName, + ...(failedAt != null && { failedAt }), }); } return lines; diff --git a/client/src/components/Chat/Messages/Content/outcome.ts b/client/src/components/Chat/Messages/Content/outcome.ts index 1191a48d2b3..2ec5748a04f 100644 --- a/client/src/components/Chat/Messages/Content/outcome.ts +++ b/client/src/components/Chat/Messages/Content/outcome.ts @@ -215,6 +215,8 @@ export function getOutcomeStatus({ } export type SpanSummary = SpanOutcome & { + /** Actual tool calls, excluding reasoning, labels and sparse slots. */ + total: number; /** Consecutive uses of the last tool, reset by another tool or an agent handoff. * Reasoning and labels describe the work without breaking its sequence. */ trailingToolCount: number; @@ -295,6 +297,7 @@ export function summarizeSpan( }; let failed = 0; let cancelled = 0; + let total = 0; let trailingToolCount = 0; let trailingTool: string | undefined; for (const part of parts) { @@ -304,6 +307,7 @@ export function summarizeSpan( } const meta = part == null ? null : metaOf(part); if (meta != null) { + total += 1; /** iconName retains full tool identity (including MCP names), with Bash * wrappers already normalized by the cached metadata resolver. */ trailingToolCount = meta.iconName === trailingTool ? trailingToolCount + 1 : 1; @@ -316,5 +320,5 @@ export function summarizeSpan( cancelled += 1; } } - return { failed, cancelled, trailingToolCount, metaOf }; + return { failed, cancelled, total, trailingToolCount, metaOf }; } diff --git a/client/src/components/Chat/Messages/Content/reveal.tsx b/client/src/components/Chat/Messages/Content/reveal.tsx index 8692ce6671a..33a0d5c858b 100644 --- a/client/src/components/Chat/Messages/Content/reveal.tsx +++ b/client/src/components/Chat/Messages/Content/reveal.tsx @@ -8,6 +8,7 @@ import { useState, } from 'react'; import { TriangleAlert } from 'lucide-react'; +import type { TranslationKeys } from '~/hooks'; import { useLocalize } from '~/hooks'; import { cn } from '~/utils'; @@ -103,10 +104,12 @@ export function useFailedReveal( */ export function FailedRevealPill({ count, + total, onReveal, className, }: { count: number; + total: number; onReveal: () => void; className?: string; }) { @@ -114,6 +117,12 @@ export function FailedRevealPill({ if (count === 0) { return null; } + let showFailedKey: TranslationKeys = 'com_ui_show_failed_n_of_n'; + if (count === 1 && total === 1) { + showFailedKey = 'com_ui_show_failed_one_of_one'; + } else if (count === 1) { + showFailedKey = 'com_ui_show_failed_one_of_n'; + } return ( ); } diff --git a/client/src/hooks/SSE/__tests__/useStepHandler.spec.ts b/client/src/hooks/SSE/__tests__/useStepHandler.spec.ts index 1d9f2de9260..ca8d4a7b5b9 100644 --- a/client/src/hooks/SSE/__tests__/useStepHandler.spec.ts +++ b/client/src/hooks/SSE/__tests__/useStepHandler.spec.ts @@ -1914,6 +1914,47 @@ describe('useStepHandler', () => { }); }); + describe('on_run_step_closed event', () => { + it('stamps the failure time onto the visible tool call', () => { + mockGetMessages.mockReturnValue([createResponseMessage()]); + const { result } = renderHook(() => useStepHandler(createHookParams())); + const submission = createSubmission(); + const closedAt = Date.now() - 1000; + + act(() => { + result.current.stepHandler( + { event: StepEvents.ON_RUN_STEP, data: createToolCallRunStep() }, + submission, + ); + result.current.stepHandler( + { + event: StepEvents.ON_RUN_STEP_CLOSED, + data: { + id: 'step-tool-1', + index: 0, + type: StepTypes.TOOL_CALLS, + status: 'failed', + created_at: closedAt - 3000, + closed_at: closedAt, + } as Agents.RunStepClosedEvent, + }, + submission, + ); + }); + + const messages = mockSetMessages.mock.lastCall?.[0] as TMessage[]; + const response = messages.find((message) => message.messageId === 'response-msg-1'); + expect(response?.content?.[0]).toMatchObject({ + type: ContentTypes.TOOL_CALL, + tool_call: { + runStepStatus: 'failed', + runStepDurationMs: 3000, + runStepClosedAt: closedAt, + }, + }); + }); + }); + describe('sandbox startup state', () => { const wrapper = ({ children }: React.PropsWithChildren) => React.createElement(RecoilRoot, null, React.createElement(IsolatedAtomStore, null, children)); diff --git a/client/src/hooks/SSE/useStepHandler.ts b/client/src/hooks/SSE/useStepHandler.ts index a8d4bc71d47..f6a08bf63f1 100644 --- a/client/src/hooks/SSE/useStepHandler.ts +++ b/client/src/hooks/SSE/useStepHandler.ts @@ -9,6 +9,7 @@ import { ToolCallTypes, getNonEmptyValue, getRunStepDurationMs, + getRunStepCloseMetadata, } from 'librechat-data-provider'; import type { Agents, @@ -1356,6 +1357,7 @@ export default function useStepHandler({ [ContentTypes.TOOL_CALL]: { ...existingToolCall, runStepStatus: closed.status, + ...getRunStepCloseMetadata(closed), ...(durationMs != null && { runStepDurationMs: durationMs }), }, }; diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index 65e8393b116..2970fc3ca44 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -1885,7 +1885,7 @@ "com_ui_my_prompts": "My Prompts", "com_ui_my_skills": "My Skills", "com_ui_n_actions_cancelled": "{{0}} cancelled", - "com_ui_n_actions_failed": "{{0}} failed", + "com_ui_n_of_n_actions_failed": "{{0}}/{{1}} failed", "com_ui_n_files": "{{0}} files", "com_ui_n_files_changed": "{{0}} files changed", "com_ui_n_searches": "{{0}} searches", @@ -1959,7 +1959,6 @@ "com_ui_on": "On", "com_ui_one_file_changed": "1 file changed", "com_ui_one_action_cancelled": "1 cancelled", - "com_ui_one_action_failed": "1 failed", "com_ui_open_archived_chat_new_tab_title": "{{title}} (opens in new tab)", "com_ui_open_as_artifact": "Open as artifact", "com_ui_open_project": "Open project", @@ -2361,8 +2360,9 @@ "com_ui_show_less": "Show less", "com_ui_show_more": "Show more", "com_ui_show_error": "Show error", - "com_ui_show_failed_one": "Show failed call", - "com_ui_show_failed_n": "Show {{0}} failed calls", + "com_ui_show_failed_one_of_one": "Show 1 failed call out of 1 call", + "com_ui_show_failed_one_of_n": "Show 1 failed call out of {{1}} calls", + "com_ui_show_failed_n_of_n": "Show {{0}} failed calls out of {{1}} calls", "com_ui_show_n_files": "Show {{0}} files", "com_ui_show_qr": "Show QR Code", "com_ui_sibling_navigation": "Sibling message navigation", diff --git a/packages/api/src/stream/implementations/RedisJobStore.ts b/packages/api/src/stream/implementations/RedisJobStore.ts index 36848f35225..c616170e577 100644 --- a/packages/api/src/stream/implementations/RedisJobStore.ts +++ b/packages/api/src/stream/implementations/RedisJobStore.ts @@ -1,6 +1,10 @@ import { logger } from '@librechat/data-schemas'; import { createContentAggregator } from '@librechat/agents'; -import { ContentTypes, getRunStepDurationMs } from 'librechat-data-provider'; +import { + ContentTypes, + getRunStepDurationMs, + getRunStepCloseMetadata, +} from 'librechat-data-provider'; import type { StandardGraph } from '@librechat/agents'; import type { Agents } from 'librechat-data-provider'; import type { Redis, Cluster } from 'ioredis'; @@ -4246,6 +4250,7 @@ export class RedisJobStore implements IJobStoreV2 { const part = index != null ? contentParts[index] : undefined; if (closed.status && part?.type === ContentTypes.TOOL_CALL && part.tool_call) { part.tool_call.runStepStatus = closed.status; + Object.assign(part.tool_call, getRunStepCloseMetadata(closed)); const durationMs = getRunStepDurationMs(closed); if (durationMs != null) { part.tool_call.runStepDurationMs = durationMs; diff --git a/packages/data-provider/src/runSteps.spec.ts b/packages/data-provider/src/runSteps.spec.ts index 547e1f27e72..f21453fea93 100644 --- a/packages/data-provider/src/runSteps.spec.ts +++ b/packages/data-provider/src/runSteps.spec.ts @@ -1,9 +1,33 @@ import { + getRunStepClosedAt, + getRunStepCloseMetadata, getRunStepDurationMs, isReportableRunStepDuration, MIN_REPORTABLE_RUN_STEP_DURATION_MS, } from './runSteps'; +describe('getRunStepClosedAt', () => { + it('accepts a close time even if the emitter did not report when the step opened', () => { + expect(getRunStepClosedAt({ closed_at: 1_750_000_000_000 })).toBe(1_750_000_000_000); + }); + + it('only stamps validated close times on saved content', () => { + expect(getRunStepCloseMetadata({ closed_at: 1_750_000_000_000 })).toEqual({ + runStepClosedAt: 1_750_000_000_000, + }); + expect(getRunStepCloseMetadata({ closed_at: NaN })).toEqual({}); + }); + + it('rejects absent, non-date, and clock-skewed stamps', () => { + expect(getRunStepClosedAt({})).toBeUndefined(); + expect(getRunStepClosedAt({ closed_at: NaN })).toBeUndefined(); + expect(getRunStepClosedAt({ closed_at: Infinity })).toBeUndefined(); + expect(getRunStepClosedAt({ closed_at: 0 })).toBeUndefined(); + expect(getRunStepClosedAt({ closed_at: 1e20 })).toBeUndefined(); + expect(getRunStepClosedAt({ created_at: 2000, closed_at: 1000 })).toBeUndefined(); + }); +}); + describe('getRunStepDurationMs', () => { it('returns the elapsed time between the two stamps', () => { expect(getRunStepDurationMs({ created_at: 1000, closed_at: 4500 })).toBe(3500); diff --git a/packages/data-provider/src/runSteps.ts b/packages/data-provider/src/runSteps.ts index 445b14f2960..ff8233c1777 100644 --- a/packages/data-provider/src/runSteps.ts +++ b/packages/data-provider/src/runSteps.ts @@ -14,6 +14,26 @@ export interface RunStepTimestamps { closed_at?: number; } +/** Host-reported close time; omit absent or unusable clocks instead of guessing. */ +export function getRunStepClosedAt(closed: RunStepTimestamps): number | undefined { + const { closed_at: closedAt, created_at: createdAt } = closed; + if ( + typeof closedAt !== 'number' || + !Number.isFinite(closedAt) || + closedAt <= 0 || + !Number.isFinite(new Date(closedAt).getTime()) || + (typeof createdAt === 'number' && Number.isFinite(createdAt) && createdAt > closedAt) + ) { + return undefined; + } + return closedAt; +} + +export function getRunStepCloseMetadata(closed: RunStepTimestamps): { runStepClosedAt?: number } { + const closedAt = getRunStepClosedAt(closed); + return closedAt == null ? {} : { runStepClosedAt: closedAt }; +} + /** * Below this, a duration is noise rather than information: sub-second tool * calls are the common case, and labelling every one of them `· 0.3s` adds a diff --git a/packages/data-provider/src/types/content.ts b/packages/data-provider/src/types/content.ts index bdeeac8c7e7..d751a15afd2 100644 --- a/packages/data-provider/src/types/content.ts +++ b/packages/data-provider/src/types/content.ts @@ -160,6 +160,8 @@ export type PartMetadata = { * at render time. */ runStepDurationMs?: number; + /** Host-reported close time in epoch milliseconds, when valid. Absent on older content. */ + runStepClosedAt?: number; /** * Stamped by the background harvester when a detached task's final output * replaces the dispatch handle in `tool_call.output`. The handle JSON and