CERBERUS/LibreChat
1
0
Fork 0
mirror of https://github.com/danny-avila/LibreChat.git synced 2026-08-27 04:07:05 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 13

🧩 refactor: Resolve Tool-Card State Once (#14934)

Browse source 
* 🧩 refactor: Resolve Tool-Card State Once (AI-1810)

Each tool card derived its state several times over — the visible label
from one expression, the `aria-live` announcement from another, the
icon and shimmer from a third, and since #14906 the follow-scroll from
a fourth. Nothing tied them together; they agreed only because each was
written to agree. Thirteen of the seventeen review findings on #14873
were instances of one derivation being updated and another left behind,
and #14892 added more.

`resolveToolCallPhase` is now the single source: one function encoding
the precedence rules, each of which a specific review finding
established, returning `running | completed | cancelled | failed`.
Everything the card shows reads that value.

`ProgressText` takes `phase` in place of the `error` + `errorSuffix`
pair, which encoded three terminal states in two booleans — `error`
meant cancelled, a present `errorSuffix` meant failed — and made every
consumer reconstruct the distinction. That shape is precisely what let
a duration render beside "failed" (Codex round 1 on #14892).

Two things fell out once the state had one home, both dead code rather
than deletions of behaviour:

- `progress` left `ProgressText` entirely; the phase already carries
  everything it was used to decide.
- The `useProgress` mask went with it. Passing 1 in still matters — it
  stops the 200ms interval — but masking the output no longer does,
  because the phase treats an explicit close as terminal outright. The
  "both halves are load-bearing" subtlety is now one half.

Scope: the nine cards that render the shared `ProgressText`. The three
with bespoke layouts (`WebSearch`, `SubagentCall`, `OpenAIImageGen`)
still resolve their own state and are the natural follow-up — they can
adopt the resolver without adopting the component.

Refactor-only. 4891/4891 client tests pass unchanged, including the
suites that encode the cancelled/failed precedence in both directions.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5

* 🐛 fix: Infer Cancellation From Reported Progress, Not The Animation

`useProgress` holds below 1 for ~200ms after a call reports completion:
it emits the previous value, then `0.99`, then `1` on a timeout. The
resolver read that animated value for its cancellation inference, so a
successful call whose submission ended inside that window rendered —
and announced — as "Cancelled".

The input is now split. `reportedProgress` is what the stream said and
drives the inference; `displayProgress` is the animated value and drives
`running` vs `completed`, so the label and shimmer still follow the
animation rather than snapping.

This restores `ToolCall` and `RetrievalCall`, whose previous predicates
used `initialProgress` and were immune, and additionally fixes
`useToolCallState`, which inferred from `rawProgress` and therefore
carried the bug already — every card the hook backs was exposed to it
before this PR.

Three tests cover the window: a reported-complete call mid-settle is
`running`, a genuinely unfinished one is still `cancelled`, and the card
settles to `completed` without a cancelled frame in between.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5

* 🧹 chore: Drop Unused Phase Predicates; Correct A Stale Comment

`isFailedPhase` and `isRunningPhase` had no callers — every consumer
compares the phase directly, which reads better than a wrapper. An
unused abstraction is the thing this PR argues against, so it should not
ship one.

The comment above the hook's resolver call still described "the raw
progress the legacy heuristic was written against", which stopped being
true when the input split into reported and display progress.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5

---------

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Danny Avila 2026-08-17 12:05:17 -04:00 • committed by GitHub
parent aa35cd42b1
commit df294fa474
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 483 additions and 200 deletions
Download patch file Download diff file Expand all files Collapse all files

18
client/src/components/Chat/Messages/Content/CodeAnalyze.tsx
View file

@ -1,6 +1,7 @@
import { useState, useEffect } from 'react';
import { useRecoilValue } from 'recoil';
import { Terminal } from 'lucide-react';
import type { ToolCallPhase } from '~/utils/toolCallPhase';
import { useProgress, useLocalize } from '~/hooks';
import ProgressText from './ProgressText';
import MarkdownLite from './MarkdownLite';
@ -46,14 +47,22 @@ export default function CodeAnalyze({
return acc;
}, '');
/**
* The legacy assistants-endpoint card: it never receives run-step metadata,
* so it genuinely has only these two states and maps them directly rather
* than through `resolveToolCallPhase`, which needs signals this card has no
* access to. The announcement and the icon below read this same value.
*/
const phase: ToolCallPhase = progress < 1 ? 'running' : 'completed';
return (
<>
<span className="sr-only" aria-live="polite" aria-atomic="true">
{progress < 1 ? localize('com_ui_analyzing') : localize('com_ui_analyzing_finished')}
{phase === 'running' ? localize('com_ui_analyzing') : localize('com_ui_analyzing_finished')}
</span>
<div className="my-1 flex items-center gap-2.5">
<ProgressText
progress={progress}
phase={phase}
onClick={handleToggleCode}
inProgressText={localize('com_ui_analyzing')}
finishedText={localize('com_ui_analyzing_finished')}
@ -61,7 +70,10 @@ export default function CodeAnalyze({
isExpanded={showCode}
icon={
<Terminal
className={cn('size-4 shrink-0 text-text-secondary', progress < 1 && 'animate-pulse')}
className={cn(
'size-4 shrink-0 text-text-secondary',
phase === 'running' && 'animate-pulse',
)}
aria-hidden="true"
/>
}

44
client/src/components/Chat/Messages/Content/Parts/BashCall.tsx
View file

@ -49,20 +49,15 @@ export default function BashCall({
const isWritingCommand = !command || !areToolCallArgsComplete(args);
const sandboxStarting = useRecoilValue(sandboxStartingByToolCallId(toolCallId ?? ''));
const { showCode, toggleCode, expandStyle, expandRef, progress, cancelled, hasError, hasOutput } =
useToolCallState(initialProgress, isSubmitting, output, !!command, onExpand, runStepStatus);
const highlighted = useLazyHighlight(command || undefined, 'bash');
const { ref: commandPaneRef, onScroll: onCommandPaneScroll } = useFollowScroll<HTMLDivElement>(
highlighted ?? command,
progress < 1 && !cancelled,
showCode,
);
const outputHasError = useMemo(() => ERROR_PATTERNS.test(output), [output]);
/** A backgrounded call's persisted output stays the dispatch handle until
* the detached run settles and patches it; render a background state
* instead of the handle JSON. Completion arrives live as the status marker
* attachment (also covers stdout-only runs) or as harvested files. */
* attachment (also covers stdout-only runs) or as harvested files.
*
* Resolved before the phase, which folds `backgroundFailed` in: the
* detached task's outcome is this card's outcome, and the dispatch step's
* own output cannot express it. */
const backgroundHandle = useMemo(() => parseBackgroundHandle(output), [output]);
const { fileAttachments, backgroundStatus } = useMemo(
() => splitBackgroundAttachments(attachments, toolCallId),
@ -77,6 +72,23 @@ export default function BashCall({
)
: null;
const { showCode, toggleCode, expandStyle, expandRef, phase, hasOutput } = useToolCallState({
initialProgress,
isSubmitting,
output,
hasInput: !!command,
onExpand,
runStepStatus,
extraError: backgroundFailed,
});
const highlighted = useLazyHighlight(command || undefined, 'bash');
const { ref: commandPaneRef, onScroll: onCommandPaneScroll } = useFollowScroll<HTMLDivElement>(
highlighted ?? command,
phase === 'running',
showCode,
);
const [isCopied, setIsCopied] = useState(false);
const timerRef = useRef<ReturnType<typeof setTimeout>>();
useEffect(() => () => clearTimeout(timerRef.current), []);
@ -110,11 +122,11 @@ export default function BashCall({
<>
<div className="relative my-1.5 flex h-5 shrink-0 items-center gap-2.5">
<ProgressText
progress={progress}
phase={phase}
onClick={toggleCode}
inProgressText={inProgressText}
finishedText={
cancelled
phase === 'cancelled'
? localize('com_ui_cancelled')
: (backgroundFinishedText ?? intent ?? localize('com_ui_command_finished'))
}
@ -127,23 +139,17 @@ export default function BashCall({
durationMs={
backgroundHandle == null && backgrounded !== true ? runStepDurationMs : undefined
}
errorSuffix={
(hasError && !cancelled) || backgroundFailed
? localize('com_ui_tool_failed')
: undefined
}
icon={
<LangIcon
lang="bash"
className={cn(
'size-4 shrink-0 text-text-secondary',
progress < 1 && !cancelled && !hasError && 'animate-pulse',
phase === 'running' && 'animate-pulse',
)}
/>
}
hasInput={!!command || hasOutput}
isExpanded={showCode}
error={cancelled}
/>
</div>
<div style={expandStyle}>

44
client/src/components/Chat/Messages/Content/Parts/ExecuteCode.tsx
View file

@ -87,20 +87,15 @@ export default function ExecuteCode({
const intent = useToolCallIntent(args);
const sandboxStarting = useRecoilValue(sandboxStartingByToolCallId(toolCallId ?? ''));
const { showCode, toggleCode, expandStyle, expandRef, progress, cancelled, hasError, hasOutput } =
useToolCallState(initialProgress, isSubmitting, output, !!code, onExpand, runStepStatus);
const highlighted = useLazyHighlight(code, lang);
const { ref: codePaneRef, onScroll: onCodePaneScroll } = useFollowScroll<HTMLPreElement>(
highlighted ?? code ?? '',
progress < 1 && !cancelled,
showCode,
);
const outputHasError = useMemo(() => ERROR_PATTERNS.test(output), [output]);
/** A backgrounded call's persisted output stays the dispatch handle until
* the detached run settles and patches it; render a background state
* instead of the handle JSON. Completion arrives live as the status marker
* attachment (also covers stdout-only runs) or as harvested files. */
* attachment (also covers stdout-only runs) or as harvested files.
*
* Resolved before the phase, which folds `backgroundFailed` in: the
* detached task's outcome is this card's outcome, and the dispatch step's
* own output cannot express it. */
const backgroundHandle = useMemo(() => parseBackgroundHandle(output), [output]);
const { fileAttachments, backgroundStatus } = useMemo(
() => splitBackgroundAttachments(attachments, toolCallId),
@ -115,18 +110,35 @@ export default function ExecuteCode({
)
: null;
const { showCode, toggleCode, expandStyle, expandRef, phase, hasOutput } = useToolCallState({
initialProgress,
isSubmitting,
output,
hasInput: !!code,
onExpand,
runStepStatus,
extraError: backgroundFailed,
});
const highlighted = useLazyHighlight(code, lang);
const { ref: codePaneRef, onScroll: onCodePaneScroll } = useFollowScroll<HTMLPreElement>(
highlighted ?? code ?? '',
phase === 'running',
showCode,
);
return (
<>
<div className="relative my-1.5 flex h-5 shrink-0 items-center gap-2.5">
<ProgressText
progress={progress}
phase={phase}
onClick={toggleCode}
inProgressText={
intent ??
(sandboxStarting ? localize('com_ui_sandbox_starting') : localize('com_ui_analyzing'))
}
finishedText={
cancelled
phase === 'cancelled'
? localize('com_ui_cancelled')
: (backgroundFinishedText ?? intent ?? localize('com_ui_analyzing_finished'))
}
@ -139,23 +151,17 @@ export default function ExecuteCode({
durationMs={
backgroundHandle == null && backgrounded !== true ? runStepDurationMs : undefined
}
errorSuffix={
(hasError && !cancelled) || backgroundFailed
? localize('com_ui_tool_failed')
: undefined
}
icon={
<SquareTerminal
className={cn(
'size-4 shrink-0 text-text-secondary',
progress < 1 && !cancelled && !hasError && 'animate-pulse',
phase === 'running' && 'animate-pulse',
)}
aria-hidden="true"
/>
}
hasInput={!!code?.length}
isExpanded={showCode}
error={cancelled}
/>
</div>
<div style={expandStyle}>

29
client/src/components/Chat/Messages/Content/Parts/FileAuthoringCall.tsx
View file

@ -152,20 +152,19 @@ export default function FileAuthoringCall({
previewLang = fileLang;
}
const { showCode, toggleCode, expandStyle, expandRef, progress, cancelled, hasError } =
useToolCallState(
initialProgress,
isSubmitting,
output,
!!filePath || !!preview,
onExpand,
runStepStatus,
);
const { showCode, toggleCode, expandStyle, expandRef, phase } = useToolCallState({
initialProgress,
isSubmitting,
output,
hasInput: !!filePath || !!preview,
onExpand,
runStepStatus,
});
const highlighted = useLazyHighlight(preview || undefined, previewLang);
const { ref: previewPaneRef, onScroll: onPreviewPaneScroll } = useFollowScroll<HTMLPreElement>(
highlighted ?? preview,
progress < 1 && !cancelled,
phase === 'running',
showCode,
);
const Icon = isCreate && !overwrote ? FilePlus2 : FilePenLine;
@ -179,7 +178,7 @@ export default function FileAuthoringCall({
<>
<div className="relative my-1.5 flex h-5 shrink-0 items-center gap-2.5">
<ProgressText
progress={progress}
phase={phase}
onClick={toggleCode}
inProgressText={
intent ??
@ -188,24 +187,22 @@ export default function FileAuthoringCall({
})
}
finishedText={
cancelled
phase === 'cancelled'
? localize('com_ui_cancelled')
: (intent ?? localize(finishedKey, { 0: fileName }))
}
durationMs={runStepDurationMs}
errorSuffix={hasError && !cancelled ? localize('com_ui_tool_failed') : undefined}
icon={
<Icon
className={cn(
'size-4 shrink-0 text-text-secondary',
progress < 1 && !cancelled && !hasError && 'animate-pulse',
phase === 'running' && 'animate-pulse',
)}
aria-hidden="true"
/>
}
hasInput={!!filePath || !!preview}
isExpanded={showCode}
error={cancelled}
/>
</div>
<div style={expandStyle}>
@ -226,7 +223,7 @@ export default function FileAuthoringCall({
<pre
className={cn(
'max-h-[300px] overflow-auto whitespace-pre-wrap break-words border-t border-border-light px-3 py-2.5 font-mono text-xs',
hasError ? 'text-status-error' : 'text-text-primary',
phase === 'failed' ? 'text-status-error' : 'text-text-primary',
)}
>
{output}

18
client/src/components/Chat/Messages/Content/Parts/ReadFileCall.tsx
View file

@ -89,8 +89,14 @@ export default function ReadFileCall({
const fileName = filePath.split('/').pop() || filePath;
const lang = useMemo(() => langFromPath(filePath), [filePath]);
const { showCode, toggleCode, expandStyle, expandRef, progress, cancelled, hasError, hasOutput } =
useToolCallState(initialProgress, isSubmitting, output, !!filePath, onExpand, runStepStatus);
const { showCode, toggleCode, expandStyle, expandRef, phase, hasOutput } = useToolCallState({
initialProgress,
isSubmitting,
output,
hasInput: !!filePath,
onExpand,
runStepStatus,
});
const highlighted = useLazyHighlight(hasOutput ? output : undefined, lang);
@ -98,28 +104,26 @@ export default function ReadFileCall({
<>
<div className="relative my-1.5 flex h-5 shrink-0 items-center gap-2.5">
<ProgressText
progress={progress}
phase={phase}
onClick={toggleCode}
inProgressText={intent ?? localize('com_ui_reading_file', { 0: fileName })}
finishedText={
cancelled
phase === 'cancelled'
? localize('com_ui_cancelled')
: (intent ?? localize('com_ui_read_file', { 0: fileName }))
}
durationMs={runStepDurationMs}
errorSuffix={hasError && !cancelled ? localize('com_ui_tool_failed') : undefined}
icon={
<FileText
className={cn(
'size-4 shrink-0 text-text-secondary',
progress < 1 && !cancelled && !hasError && 'animate-pulse',
phase === 'running' && 'animate-pulse',
)}
aria-hidden="true"
/>
}
hasInput={!!filePath || hasOutput}
isExpanded={showCode}
error={cancelled}
/>
</div>
<div style={expandStyle}>

18
client/src/components/Chat/Messages/Content/Parts/SkillCall.tsx
View file

@ -35,35 +35,39 @@ export default function SkillCall({
const skillName = useMemo(() => parseJsonField(args, 'skillName'), [args]);
const intent = useToolCallIntent(args);
const { showCode, toggleCode, expandStyle, expandRef, progress, cancelled, hasError, hasOutput } =
useToolCallState(initialProgress, isSubmitting, output, !!skillName, onExpand, runStepStatus);
const { showCode, toggleCode, expandStyle, expandRef, phase, hasOutput } = useToolCallState({
initialProgress,
isSubmitting,
output,
hasInput: !!skillName,
onExpand,
runStepStatus,
});
return (
<>
<div className="relative my-1.5 flex h-5 shrink-0 items-center gap-2.5">
<ProgressText
progress={progress}
phase={phase}
onClick={toggleCode}
inProgressText={intent ?? localize('com_ui_skill_running', { 0: skillName })}
finishedText={
cancelled
phase === 'cancelled'
? localize('com_ui_cancelled')
: (intent ?? localize('com_ui_skill_finished', { 0: skillName }))
}
durationMs={runStepDurationMs}
errorSuffix={hasError && !cancelled ? localize('com_ui_tool_failed') : undefined}
icon={
<ScrollText
className={cn(
'size-4 shrink-0 text-text-secondary',
progress < 1 && !cancelled && !hasError && 'animate-pulse',
phase === 'running' && 'animate-pulse',
)}
aria-hidden="true"
/>
}
hasInput={!!skillName || hasOutput}
isExpanded={showCode}
error={cancelled}
/>
</div>
<div style={expandStyle}>

12
client/src/components/Chat/Messages/Content/Parts/__tests__/BashCall.test.tsx
View file

@ -33,20 +33,20 @@ jest.mock('~/hooks', () => ({
jest.mock('~/components/Chat/Messages/Content/ProgressText', () => ({
__esModule: true,
/** Mirrors the real component's contract: one `phase` drives both the
* label and the failure suffix. */
default: ({
progress,
phase,
inProgressText,
finishedText,
errorSuffix,
}: {
progress: number;
phase: 'running' | 'completed' | 'cancelled' | 'failed';
inProgressText: string;
finishedText: string;
errorSuffix?: string;
}) => (
<div data-testid="progress-text">
{progress < 1 ? inProgressText : finishedText}
{errorSuffix != null ? ` — ${errorSuffix}` : ''}
{phase === 'running' ? inProgressText : finishedText}
{phase === 'failed' ? ' — tool failed' : ''}
</div>
),
}));

14
client/src/components/Chat/Messages/Content/Parts/__tests__/FileAuthoringCall.test.tsx
View file

@ -26,14 +26,16 @@ jest.mock('~/utils', () => ({
jest.mock('~/components/Chat/Messages/Content/ProgressText', () => ({
__esModule: true,
default: ({
progress,
phase,
inProgressText,
finishedText,
}: {
progress: number;
phase: 'running' | 'completed' | 'cancelled' | 'failed';
inProgressText: string;
finishedText: string;
}) => <div data-testid="progress-text">{progress < 1 ? inProgressText : finishedText}</div>,
}) => (
<div data-testid="progress-text">{phase === 'running' ? inProgressText : finishedText}</div>
),
}));
jest.mock('../CodeWindowHeader', () => ({
@ -64,14 +66,14 @@ jest.mock('../useLazyHighlight', () => {
jest.mock('../useToolCallState', () => ({
__esModule: true,
default: (initialProgress: number) => ({
/** Mirrors the real hook: options object in, a single `phase` out. */
default: ({ initialProgress }: { initialProgress: number }) => ({
showCode: true,
toggleCode: jest.fn(),
expandStyle: {},
expandRef: { current: null },
progress: initialProgress,
cancelled: false,
hasError: false,
phase: initialProgress < 1 ? 'running' : 'completed',
}),
}));

81
client/src/components/Chat/Messages/Content/Parts/useToolCallState.ts
View file

@ -1,7 +1,9 @@
import { useState, useEffect, useCallback } from 'react';
import { useRecoilValue } from 'recoil';
import type { PartMetadata } from 'librechat-data-provider';
import type { ToolCallPhase } from '~/utils/toolCallPhase';
import { isError } from '~/components/Chat/Messages/Content/ToolOutput';
import { resolveToolCallPhase } from '~/utils/toolCallPhase';
import { useProgress, useExpandCollapse } from '~/hooks';
import store from '~/store';
@ -10,24 +12,44 @@ interface ToolCallState {
toggleCode: () => void;
expandStyle: React.CSSProperties;
expandRef: React.RefObject<HTMLDivElement>;
progress: number;
cancelled: boolean;
hasError: boolean;
/**
* The card's settled state, resolved once. Label, announcement, icon and
* animation all read this — deriving any of them separately is what let
* them disagree.
*/
phase: ToolCallPhase;
hasOutput: boolean;
hasContent: boolean;
}
export default function useToolCallState(
initialProgress: number,
isSubmitting: boolean,
output: string,
hasInput: boolean,
onExpand?: () => void,
runStepStatus?: PartMetadata['runStepStatus'],
): ToolCallState {
export interface UseToolCallStateInput {
initialProgress: number;
isSubmitting: boolean;
output: string;
hasInput: boolean;
onExpand?: () => void;
runStepStatus?: PartMetadata['runStepStatus'];
/**
* A failure this call's own output cannot express — currently a
* backgrounded task that settled as `error`, where the dispatch step's
* output is the handle rather than the task's result. Folded into the
* phase so those cards resolve their state through the same path as
* every other card instead of patching the result afterwards.
*/
extraError?: boolean;
}
export default function useToolCallState({
initialProgress,
isSubmitting,
output,
hasInput,
onExpand,
runStepStatus,
extraError = false,
}: UseToolCallStateInput): ToolCallState {
const autoExpand = useRecoilValue(store.autoExpandTools);
const hasOutput = output.length > 0;
const hasError = (hasOutput && isError(output)) || runStepStatus === 'failed';
const hasContent = hasInput || hasOutput;
const [showCode, setShowCode] = useState(() => autoExpand && hasContent);
@ -41,12 +63,13 @@ export default function useToolCallState(
const isClosed = runStepStatus != null;
/**
* Both halves are load-bearing: passing 1 in stops `useProgress` scheduling
* its 200ms interval, and masking the result makes the terminal value
* observable on the same render rather than after the hook settles.
* Passing 1 in for a closed step stops `useProgress` scheduling its 200ms
* interval, which it keeps alive for any input below 1. The result no
* longer needs masking: the phase treats an explicit close as terminal
* outright, so a step that closes while the hook is still settling through
* 0.99 can no longer read as in-flight.
*/
const rawProgress = useProgress(isClosed ? 1 : initialProgress);
const progress = isClosed ? 1 : rawProgress;
const toggleCode = useCallback(() => {
setShowCode((prev) => {
const next = !prev;
@ -56,28 +79,26 @@ export default function useToolCallState(
return next;
});
}, [onExpand]);
/**
* The step's own terminal status wins when the run emitted one; the
* `isSubmitting` heuristic is a whole-message inference that cannot tell
* which step stopped. Kept as the fallback for messages saved before
* `on_run_step_closed` and endpoints that do not emit it.
*
* The status is authoritative on its own terms — it is deliberately not
* gated on `hasError`, so that parsing the output text can never demote a
* step the run reported as stopped back into an in-flight state.
* One resolution; everything the card shows is a read of this value. The
* two progress inputs are deliberately distinct — see `resolveToolCallPhase`
* for why the cancellation inference must not read the animated one.
*/
const cancelled = isClosed
? runStepStatus === 'cancelled'
: !isSubmitting && rawProgress < 1 && !hasError;
const phase = resolveToolCallPhase({
runStepStatus,
displayProgress: rawProgress,
reportedProgress: initialProgress,
isSubmitting,
hasError: (hasOutput && isError(output)) || extraError,
});
return {
showCode,
toggleCode,
expandStyle,
expandRef,
progress,
cancelled,
hasError,
phase,
hasOutput,
hasContent,
};

56
client/src/components/Chat/Messages/Content/ProgressText.tsx
View file

@ -3,6 +3,7 @@ import { Button } from '@librechat/client';
import { useTranslation } from 'react-i18next';
import * as Popover from '@radix-ui/react-popover';
import { isReportableRunStepDuration } from 'librechat-data-provider';
import type { ToolCallPhase } from '~/utils/toolCallPhase';
import { cn, getRunStepDurationLabels } from '~/utils';
import CancelledIcon from './CancelledIcon';
import { useLocalize } from '~/hooks';
@ -39,73 +40,60 @@ const Wrapper = ({ popover, children }: { popover: boolean; children: React.Reac
};
export default function ProgressText({
progress,
phase,
onClick,
inProgressText,
finishedText,
authText,
icon: iconProp,
subtitle,
errorSuffix,
durationMs,
hasInput = true,
popover = false,
isExpanded = false,
error = false,
}: {
progress: number;
/**
* The card's settled state, resolved once by the caller via
* `resolveToolCallPhase`. Replaces the former `error` + `errorSuffix`
* pair, which encoded three terminal states in two booleans — `error`
* meant cancelled, a present `errorSuffix` meant failed, and every
* consumer had to reconstruct the distinction. That shape is what let a
* duration render beside "failed" and a live region announce "completed"
* over a visibly failed card.
*/
phase: ToolCallPhase;
onClick?: () => void;
inProgressText: string;
finishedText: string;
authText?: string;
icon?: React.ReactNode;
subtitle?: string;
errorSuffix?: string;
/** Wall-clock duration of the run step, from `PartMetadata.runStepDurationMs`. */
durationMs?: number;
hasInput?: boolean;
popover?: boolean;
isExpanded?: boolean;
error?: boolean;
}) {
const localize = useLocalize();
/** For locale-aware decimal formatting of the sub-10s duration value. */
const { i18n } = useTranslation();
const getText = () => {
if (error) {
return finishedText;
}
if (progress < 1) {
return authText ?? inProgressText;
}
return finishedText;
};
const isRunning = phase === 'running';
const getIcon = () => {
if (error && !errorSuffix) {
return <CancelledIcon />;
}
return iconProp ?? null;
};
const text = getText();
const icon = getIcon();
const showShimmer = progress < 1 && !error;
/** Every branch below reads `phase`, so the label, the icon, the shimmer,
* the failure suffix and the duration cannot disagree about what state
* the card is in. */
const text = isRunning ? (authText ?? inProgressText) : finishedText;
const icon = phase === 'cancelled' ? <CancelledIcon /> : (iconProp ?? null);
const showShimmer = isRunning;
const errorSuffix = phase === 'failed' ? localize('com_ui_tool_failed') : undefined;
/**
* Shown only on a settled, successful card. While the step is still running
* the number would be stale the instant it rendered, and on a cancelled or
* failed card "how long it took" is not the fact the reader needs — that
* slot already carries the cancelled icon or the error suffix.
*
* Both terminal-failure channels must be checked: at every call site
* `error` carries cancellation while failure arrives as `errorSuffix`
* alone, so gating on `error` by itself would print a duration beside
* "failed". Gating here on the component's own props rather than on a
* separate caller-supplied flag keeps this consistent with the label
* beside it by construction; the callers only forward the number.
* slot already carries the cancelled icon or the failure suffix.
*/
const duration =
progress >= 1 && !error && !errorSuffix && isReportableRunStepDuration(durationMs)
phase === 'completed' && isReportableRunStepDuration(durationMs)
? getRunStepDurationLabels(durationMs, i18n.language)
: undefined;

45
client/src/components/Chat/Messages/Content/RetrievalCall.tsx
View file

@ -6,6 +6,7 @@ import { FileText, FileSpreadsheet, FileCode, FileImage, File } from 'lucide-rea
import type { TAttachment, TFile, PartMetadata } from 'librechat-data-provider';
import { useLocalize, useProgress, useExpandCollapse } from '~/hooks';
import { ToolIcon, OutputRenderer, isError } from './ToolOutput';
import { resolveToolCallPhase } from '~/utils/toolCallPhase';
import FilePreviewDialog from './FilePreviewDialog';
import { sortPagesByRelevance, cn } from '~/utils';
import { useToolCallIntent } from './Parts/intent';
@ -350,25 +351,25 @@ export default function RetrievalCall({
* mounted would otherwise render as still in progress for that window.
*/
const rawProgress = useProgress(isClosed ? 1 : initialProgress);
const progress = isClosed ? 1 : rawProgress;
const localize = useLocalize();
/** Model-authored live label (injected when file_search is opted into
* describe_intent); persists as the settled label. The sr-only live
* region below deliberately keeps its stable generic value. */
const intent = useToolCallIntent(args);
const errorState = (typeof output === 'string' && isError(output)) || runStepStatus === 'failed';
/**
* The step's own terminal status wins when the run emitted one; the
* `isSubmitting` heuristic is a whole-message inference that cannot tell
* which step stopped. Authoritative on its own terms — not gated on
* `errorState`, so output parsing cannot demote a stopped step back into an
* in-flight state. Fallback retained for messages saved before
* `on_run_step_closed` and endpoints that do not emit it.
* One resolution, read by the label, the live region and the icon alike.
* It also unifies two inputs that had drifted apart: the cancellation
* inference read `initialProgress` while the label read the animated
* `rawProgress`.
*/
const cancelled = isClosed
? runStepStatus === 'cancelled'
: !isSubmitting && initialProgress < 1 && !errorState;
const phase = resolveToolCallPhase({
runStepStatus,
displayProgress: rawProgress,
reportedProgress: initialProgress,
isSubmitting,
hasError: typeof output === 'string' && isError(output),
});
const hasOutput = !!output && !isError(output);
const autoExpand = useRecoilValue(store.autoExpandTools);
const [showOutput, setShowOutput] = useState(() => autoExpand && hasOutput);
@ -446,16 +447,16 @@ export default function RetrievalCall({
<div className="my-1">
<span className="sr-only" aria-live="polite" aria-atomic="true">
{(() => {
if (progress < 1 && !cancelled) {
if (phase === 'running') {
return localize('com_ui_searching_files');
}
if (cancelled) {
if (phase === 'cancelled') {
return localize('com_ui_cancelled');
}
/** Announced before the success string: a terminal step that errored
* must not reach the live region as "retrieved files", which would
* tell a screen-reader user the opposite of what the card shows. */
if (errorState) {
/** A terminal step that errored must not reach the live region as
* "retrieved files", which would tell a screen-reader user the
* opposite of what the card shows. */
if (phase === 'failed') {
return localize('com_ui_failed');
}
return intent ?? localize('com_ui_retrieved_files');
@ -463,24 +464,20 @@ export default function RetrievalCall({
</span>
<div className="relative my-1 flex h-5 shrink-0 items-center gap-2.5">
<ProgressText
progress={progress}
phase={phase}
onClick={hasOutput ? handleToggleOutput : undefined}
inProgressText={intent ?? localize('com_ui_searching_files')}
/** A cancelled step must not read "Retrieved files" beside a
* cancellation icon while the live region says "Cancelled". */
finishedText={
cancelled
phase === 'cancelled'
? localize('com_ui_cancelled')
: (intent ?? localize('com_ui_retrieved_files'))
}
durationMs={runStepDurationMs}
errorSuffix={errorState && !cancelled ? localize('com_ui_tool_failed') : undefined}
icon={
<ToolIcon type="file_search" isAnimating={progress < 1 && !cancelled && !errorState} />
}
icon={<ToolIcon type="file_search" isAnimating={phase === 'running'} />}
hasInput={hasOutput}
isExpanded={showOutput}
error={cancelled}
/>
</div>
<div style={expandStyle}>

42
client/src/components/Chat/Messages/Content/ToolCall.tsx
View file

@ -13,6 +13,7 @@ import type { TAttachment, PartMetadata } from 'librechat-data-provider';
import { useLocalize, useProgress, useExpandCollapse, useLazyCollapseBody } from '~/hooks';
import { ToolIcon, getToolIconType, isError } from './ToolOutput';
import { useMCPIconMap, useMCPServerNames } from '~/hooks/MCP';
import { resolveToolCallPhase } from '~/utils/toolCallPhase';
import { useToolCallIntent } from './Parts/intent';
import { AttachmentGroup } from './Parts';
import ToolCallInfo from './ToolCallInfo';
@ -156,10 +157,6 @@ export default function ToolCall({
* in-flight state.
*/
const isClosed = runStepStatus != null;
const cancelled = isClosed
? runStepStatus === 'cancelled'
: !isSubmitting && initialProgress < 1 && !hasError;
const errorState = hasError;
const args = useMemo(() => {
if (typeof _args === 'string') {
@ -191,8 +188,19 @@ export default function ToolCall({
* observable on the same render rather than after the hook settles.
*/
const rawProgress = useProgress(isClosed ? 1 : initialProgress);
const progress = isClosed ? 1 : rawProgress;
const showCancelled = cancelled || (errorState && !output);
/**
* One resolution, read by the label, the live region, the icon and the
* shimmer alike. It also unifies two inputs that had drifted apart: the
* cancellation inference read `initialProgress` while the label read the
* animated `rawProgress`.
*/
const phase = resolveToolCallPhase({
runStepStatus,
displayProgress: rawProgress,
reportedProgress: initialProgress,
isSubmitting,
hasError,
});
const handleToggleInfo = useCallback(() => {
mountBody();
@ -221,7 +229,7 @@ export default function ToolCall({
const intent = useToolCallIntent(_args);
const getFinishedText = () => {
if (cancelled) {
if (phase === 'cancelled') {
return localize('com_ui_cancelled');
}
/**
@ -229,7 +237,7 @@ export default function ToolCall({
* errored must not reach the live region as "completed", which would tell
* a screen-reader user the opposite of what the card shows.
*/
if (errorState) {
if (phase === 'failed') {
return function_name
? `${localize('com_ui_failed')}: ${function_name}`
: localize('com_ui_failed');
@ -258,7 +266,7 @@ export default function ToolCall({
announced once via getFinishedText. */}
<span className="sr-only" aria-live="polite" aria-atomic="true">
{(() => {
if (progress < 1 && !showCancelled) {
if (phase === 'running') {
return function_name
? localize('com_assistants_running_var', { 0: function_name })
: localize('com_assistants_running_action');
@ -272,7 +280,7 @@ export default function ToolCall({
data-tool-call-id={toolCallId}
>
<ProgressText
progress={progress}
phase={phase}
onClick={handleToggleInfo}
inProgressText={
intent ??
@ -281,22 +289,18 @@ export default function ToolCall({
: localize('com_assistants_running_action'))
}
authText={
!showCancelled && authDomain.length > 0 ? localize('com_ui_requires_auth') : undefined
phase === 'running' && authDomain.length > 0
? localize('com_ui_requires_auth')
: undefined
}
finishedText={getFinishedText()}
subtitle={subtitle}
durationMs={runStepDurationMs}
errorSuffix={errorState && !cancelled ? localize('com_ui_tool_failed') : undefined}
icon={
<ToolIcon
type={toolIconType}
iconUrl={mcpIconUrl}
isAnimating={progress < 1 && !showCancelled && !errorState}
/>
<ToolIcon type={toolIconType} iconUrl={mcpIconUrl} isAnimating={phase === 'running'} />
}
hasInput={hasInfo}
isExpanded={showInfo}
error={showCancelled}
/>
</div>
<div
@ -312,7 +316,7 @@ export default function ToolCall({
)}
</div>
</div>
{auth != null && auth && progress < 1 && !showCancelled && (
{auth != null && auth && phase === 'running' && (
<div className="flex w-full flex-col gap-2.5">
<div className="mb-1 mt-2">
<Button

20
client/src/components/Chat/Messages/Content/__tests__/ProgressText.test.tsx
View file

@ -28,7 +28,7 @@ jest.mock('../CancelledIcon', () => ({
}));
const defaults = {
progress: 1,
phase: 'completed' as const,
inProgressText: 'Running foo',
finishedText: 'Completed foo',
};
@ -52,25 +52,25 @@ describe('ProgressText duration', () => {
* is still the in-progress one.
*/
it('does not render while the step is still running', () => {
renderProgressText({ progress: 0.4, durationMs: 3500 });
renderProgressText({ phase: 'running', durationMs: 3500 });
expect(screen.queryByText('· 3.5s')).not.toBeInTheDocument();
});
/**
* On a cancelled or failed card the slot already carries the cancelled icon
* or the error suffix, and "how long it took" is not the fact the reader
* needs. The two states arrive through different props — `error` carries
* cancellation, `errorSuffix` alone carries failure — so both are pinned
* separately; gating on `error` alone rendered a duration beside "failed"
* (Codex round 1 on #14892).
* or the failure suffix, and "how long it took" is not the fact the reader
* needs. Both terminal-failure states are pinned: they used to arrive
* through two different props (`error` for cancellation, `errorSuffix` for
* failure), and gating on one of them alone rendered a duration beside
* "failed" (Codex round 1 on #14892). They are now one value.
*/
it('does not render on a cancelled card', () => {
renderProgressText({ error: true, durationMs: 3500 });
renderProgressText({ phase: 'cancelled', durationMs: 3500 });
expect(screen.queryByText('· 3.5s')).not.toBeInTheDocument();
});
it('does not render on a failed card, where failure arrives as errorSuffix alone', () => {
renderProgressText({ errorSuffix: 'failed', durationMs: 3500 });
it('does not render on a failed card', () => {
renderProgressText({ phase: 'failed', durationMs: 3500 });
expect(screen.queryByText('· 3.5s')).not.toBeInTheDocument();
expect(screen.queryByText('took 3.5 seconds')).not.toBeInTheDocument();
});

153
client/src/utils/__tests__/toolCallPhase.spec.ts Normal file
View file

@ -0,0 +1,153 @@
import type { ToolCallPhaseInput } from '../toolCallPhase';
import { resolveToolCallPhase } from '../toolCallPhase';
const resolve = (overrides: Partial<ToolCallPhaseInput> = {}) =>
resolveToolCallPhase({
displayProgress: 1,
reportedProgress: 1,
isSubmitting: false,
hasError: false,
...overrides,
});
describe('resolveToolCallPhase', () => {
describe('with an explicit run-step status', () => {
it('reports a completed close as completed', () => {
expect(resolve({ runStepStatus: 'completed' })).toBe('completed');
});
it('reports a failed close as failed', () => {
expect(resolve({ runStepStatus: 'failed' })).toBe('failed');
});
it('reports a cancelled close as cancelled', () => {
expect(resolve({ runStepStatus: 'cancelled' })).toBe('cancelled');
});
/**
* The status is authoritative on its own terms. Gating it on output
* parsing let a step the run reported as stopped be demoted back into an
* in-flight state.
*/
it('never returns running for a closed step, whatever progress says', () => {
expect(
resolve({ runStepStatus: 'completed', displayProgress: 0.4, reportedProgress: 0.4 }),
).toBe('completed');
expect(
resolve({
runStepStatus: 'cancelled',
displayProgress: 0.1,
reportedProgress: 0.1,
isSubmitting: true,
}),
).toBe('cancelled');
});
/** Explicit cancellation outranks a failure-shaped result. */
it('keeps a cancelled close cancelled even when the output parses as an error', () => {
expect(resolve({ runStepStatus: 'cancelled', hasError: true })).toBe('cancelled');
});
/** A completed close whose result reads as a failure is still a failure —
* the card must not present an error as a clean success. */
it('reports a completed close with error output as failed', () => {
expect(resolve({ runStepStatus: 'completed', hasError: true })).toBe('failed');
});
});
describe('under the legacy heuristic', () => {
it('reports an in-flight call as running', () => {
expect(resolve({ displayProgress: 0.4, reportedProgress: 0.4, isSubmitting: true })).toBe(
'running',
);
});
it('reports a settled call as completed', () => {
expect(resolve({ displayProgress: 1, reportedProgress: 1, isSubmitting: true })).toBe(
'completed',
);
});
/** Unfinished progress with the message no longer streaming is the only
* signal the pre-`on_run_step_closed` path had for a stop. */
it('infers cancellation from an unfinished call once submission ends', () => {
expect(resolve({ displayProgress: 0.4, reportedProgress: 0.4, isSubmitting: false })).toBe(
'cancelled',
);
});
/**
* The precedence inverts here, deliberately: the cancellation inference
* ("not submitting and not finished") is also satisfied by a genuine
* failure, so applying the explicit-close ordering relabelled real
* failures as user stops on historical messages.
*/
it('lets failure outrank the cancellation inference', () => {
expect(
resolve({
displayProgress: 0.4,
reportedProgress: 0.4,
isSubmitting: false,
hasError: true,
}),
).toBe('failed');
});
it('reports a failure even while the message is still streaming', () => {
expect(
resolve({
displayProgress: 0.4,
reportedProgress: 0.4,
isSubmitting: true,
hasError: true,
}),
).toBe('failed');
});
});
describe('the useProgress settle window', () => {
/**
* `useProgress` holds below 1 for ~200ms after a call reports completion
* (it emits `0.99`, then `1` on a timeout). Inferring cancellation from
* that animated value labelled — and announced — a successful call as
* "Cancelled" whenever submission ended inside the window
* (Codex round 1 on #14934).
*/
it('does not call a reported-complete call cancelled while the animation settles', () => {
expect(resolve({ displayProgress: 0.99, reportedProgress: 1, isSubmitting: false })).toBe(
'running',
);
});
/** The inference still fires for a call that genuinely never finished. */
it('still infers cancellation when the stream itself never reported completion', () => {
expect(resolve({ displayProgress: 0.99, reportedProgress: 0.4, isSubmitting: false })).toBe(
'cancelled',
);
});
/** Once the animation catches up the card settles, without a cancelled
* frame in between. */
it('settles to completed once the animation catches up', () => {
expect(resolve({ displayProgress: 1, reportedProgress: 1, isSubmitting: false })).toBe(
'completed',
);
});
});
/**
* The two precedence rules are opposites and have been collapsed into one
* another twice under review. Pinned side by side so a future edit cannot
* quietly unify them.
*/
it('applies opposite precedence for an explicit close and the heuristic', () => {
const withError = {
hasError: true,
displayProgress: 0.4,
reportedProgress: 0.4,
isSubmitting: false,
};
expect(resolve({ ...withError, runStepStatus: 'cancelled' })).toBe('cancelled');
expect(resolve(withError)).toBe('failed');
});
});

1
client/src/utils/index.ts
View file

@ -45,6 +45,7 @@ export * from './approval';
export * from './steer';
export * from './activityLabels';
export * from './runStepDuration';
export * from './toolCallPhase';
export * from './documentTitle';
export * from './reasoningLabels';
export * from './numbers';

88
client/src/utils/toolCallPhase.ts Normal file
View file

@ -0,0 +1,88 @@
import type { PartMetadata } from 'librechat-data-provider';
/**
* The settled state of one tool call, as every part of its card should read
* it: the visible label, the `aria-live` announcement, the icon, the shimmer,
* and whether a duration is worth showing.
*
* Before this existed each card derived those independently — the label from
* one expression, the announcement from another, the animation from a third —
* and a change to one kept missing the others. Thirteen of the seventeen
* review findings on #14873 were instances of that drift, and #14892 added a
* fourteenth. One value, read everywhere, is what stops it: two presentations
* of the same card can no longer disagree about what happened.
*/
export type ToolCallPhase = 'running' | 'completed' | 'cancelled' | 'failed';
export interface ToolCallPhaseInput {
/**
* The run's own terminal verdict from `on_run_step_closed`. Absent on parts
* saved before the event existed and on endpoints that never emit it, which
* is what the heuristic below is for.
*/
runStepStatus?: PartMetadata['runStepStatus'];
/**
* The animated value from `useProgress` — what the card is showing right
* now. Drives `running` vs `completed` so the label and shimmer follow the
* animation rather than snapping.
*/
displayProgress: number;
/**
* The progress the stream actually reported, before display animation.
* Kept separate because `useProgress` holds below 1 for ~200ms after a call
* reports completion (it emits `0.99`, then `1` on a timeout), and the
* cancellation inference must not read that lag as an unfinished call.
*/
reportedProgress: number;
/** Whether the whole message is still streaming — a message-level fact. */
isSubmitting: boolean;
/**
* Whether this call's own result reads as a failure: parsed error output,
* or a card-specific signal such as a backgrounded task settling as `error`.
*/
hasError: boolean;
}
/**
* Resolve a tool call's phase from every signal that bears on it.
*
* The precedence rules encoded here were each established by a specific
* review finding, and each is load-bearing:
*
* - **An explicit status is authoritative and never gated on output parsing.**
* Reading the result text can otherwise demote a step the run reported as
* stopped back into an in-flight state.
* - **Explicit cancellation outranks a failure-shaped result.** A step the
* user stopped is cancelled even if its partial output parses as an error.
* - **Under the legacy heuristic the opposite holds: failure outranks
* cancellation**, because that inference reads "not submitting and not
* finished" as a stop, which a genuine failure also satisfies. Applying the
* explicit-close precedence there relabelled real failures as user stops.
* - **A closed step is never `running`**, whatever progress says.
* - **Cancellation is inferred from reported progress, never from the
* animation.** `useProgress` lags a completed call by ~200ms; reading that
* lag would label — and announce — a successful call as cancelled whenever
* submission ended inside the window.
*/
export function resolveToolCallPhase({
runStepStatus,
displayProgress,
reportedProgress,
isSubmitting,
hasError,
}: ToolCallPhaseInput): ToolCallPhase {
if (runStepStatus != null) {
if (runStepStatus === 'cancelled') {
return 'cancelled';
}
return runStepStatus === 'failed' || hasError ? 'failed' : 'completed';
}
if (hasError) {
return 'failed';
}
if (!isSubmitting && reportedProgress < 1) {
return 'cancelled';
}
return displayProgress < 1 ? 'running' : 'completed';
}