mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-27 12:13:30 +00:00
🧩 refactor: Extend Step Status To Remaining Cards; Separate Cancelled From Failed (#14873)
* 🧩 fix: Extend Explicit Step Status To Specialized Tool Cards Follow-up to #14871, which covered the generic tool card and the five sharing `useToolCallState` but left the cards carrying their own cancellation logic on the whole-message heuristic. Each needed its own treatment rather than a forwarded prop: - `RetrievalCall` is the exact analog of the reviewed shape. - `WebSearch` feeds `effectiveProgress` into `finalizing` and `complete` as well, so forcing it terminal naively would strand a cancelled final search as "finalizing" forever. A closed step now settles on its own status instead of waiting for the submission to end. - `OpenAIImageGen` resolves through `computeCancelled`, which now short-circuits on explicit status ahead of both the agent and legacy paths — the legacy path has no submitting signal at all, so this is the first real stop signal it has ever had. In all three the status is authoritative on its own terms: never gated on the output-parsing error check, a closed step forces progress complete so it cannot keep animating, and `failed` reports as an error even when the output text looks benign. The prior heuristic remains the fallback for messages saved before `on_run_step_closed`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 * 🧩 fix: Resolve Subagent Card State From Closed Step Status `SubagentCall` uses a tri-state (`running`/`cancelled`/`finished`) built from the subagent's own phase envelopes plus `!isSubmitting`, so it could not distinguish "this subagent was stopped" from "the parent stream ended for some other reason" — the distinction `on_run_step_closed` exists to make. A closed step now resolves the tri-state directly: `cancelled` maps to the cancelled state, `completed` and `failed` both count as finished, and `failed` additionally reports as an error. The phase-and-isSubmitting inference remains the fallback for messages predating the event. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 * 🩹 fix: Give `failed` Its Own Terminal Path In Each Card Codex found the same mistake in three places: I resolved `cancelled` carefully and let `failed` fall through into a success or in-progress path. Forcing a closed step's progress to 1 made that visible rather than latent. - `OpenAIImageGen`: `hasError` did not account for the status, so a failed generation rendered and announced as a finished image. Updated to match every other card. - `WebSearch`: a failed close left `complete` false and dropped the card into the streaming branch, shimmering forever. `error` now folds into the `cancelled` early-return, which is where an errored search has always gone — rather than inventing a failure UI this component has never had. - `RetrievalCall`: the live region did not consult `errorState`, so a failed retrieval announced "retrieved files" while the card showed a failure. Announces the failure first now, mirroring the same fix made to `ToolCall` in #14871. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 * 🧯 fix: Separate Cancelled From Failed, Stop Terminal-Step Timers Codex round 2. Two distinct classes, plus the same leak in already-merged code. Rendering — cancellation and failure were collapsed: - `OpenAIImageGen` fed `cancelled` into `ProgressText`'s `error` prop, so a user-stopped generation read "image generation failed" visually while the live region announced "cancelled". Meanwhile a `failed` close reached neither consumer, since `computeCancelled` returns false for it — adding the status to `hasError` alone changed nothing. `ProgressText` now takes `cancelled` alongside `error`, and both the card and the live region resolve the two states independently. Timers — masking a hook's output does not stop it: - `useProgress` keeps a 200ms interval alive whenever its input is below 1, and a closed step usually never receives the completion that would raise it. Every site that masked the result now passes the terminal value in instead, so a closed card schedules nothing. - The agent-style image ticker had the same problem one layer up: its interval effect ignored the close entirely and its cleanup keyed on `cancelled`, so a step closed as `failed` mid-submission kept rerendering for up to ~50s. Both effects now observe the close. - `ToolCall` and `useToolCallState` carried the identical masking from #14871; fixed here rather than left as a known leak in merged code. Also dropped a JSDoc line that narrated its own assignments. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 * ✅ fix: Failure Outranks Cancellation; Update The Test That Encoded The Conflation Codex round 3, plus the CI failure it explains. - Failure now takes precedence over cancellation in both `ProgressText` and the image-gen live region. The legacy inference folds `hasError` into its cancellation signal, so checking `cancelled` first relabelled a genuine failure on an older saved message as a user stop — a regression introduced by the previous commit. - `RetrievalCall` passed `finishedText={intent ?? 'Retrieved files'}` regardless of state, so a cancelled retrieval read "Retrieved files" beside a cancellation icon while the live region announced "Cancelled". The finished label is now cancellation-aware. - `OpenAIImageGen.test.tsx` asserted `data-error === 'true'` for a heuristically cancelled step — the exact conflation this work removes. Updated to the new contract and extended with cases for explicit cancellation, explicit failure with benign output, and failure precedence under the legacy inference. Verified locally for the first time in this work stream: building the `@librechat/client` and `data-provider` workspaces made the client suite runnable here. 15/15 in the image-gen spec, 851/851 across `Content` and `hooks/SSE`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 * 🧹 style: Drop Narrating Comment From Cancellation Test The test name and the `data-cancelled` / `data-error` expectations state the contract on their own; the JSDoc above them only restated it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 * ⚖️ fix: Explicit Cancellation Outranks Parsed Errors; Force Progress Synchronously Codex round 5, both findings consequences of round 3's fixes. - Failure precedence was applied unconditionally, so a step explicitly closed as `cancelled` whose output happens to be error-formatted — aborting a tool can itself produce one — reported failure despite an authoritative status saying otherwise. Precedence is now scoped to the legacy inference, which is the only path that folds `hasError` into its own cancellation signal. Explicit cancellation wins. - Passing 1 into `useProgress` stops its interval but does not make the returned value 1 on that render: the hook settles through 0.99 and a 200ms timeout. For a step closing while mounted, that window rendered a failed retrieval as "Searching files" and left a completed one shimmering. Both halves are needed — pass 1 in to stop the timer, mask the result so the terminal value is observable immediately — in `RetrievalCall`, `ToolCall`, `useToolCallState` and `OpenAIImageGen`. Regression test added for the cancellation-outranks-error case. 16/16 in the image-gen spec, 851/851 across Content and hooks/SSE. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 * 🛑 fix: Explicit Subagent Cancellation Outranks Child Error Phase Codex round 6. - `SubagentCall` checked `hasError` before cancellation, so a subagent explicitly closed as `cancelled` whose last live envelope carried the `error` phase — which aborting a child can produce — rendered "Agent errored" instead of cancelled. Scoped the same way as the image path: the live error phase is suppressed when the authoritative close says cancelled. - Removed a narrating comment in `WebSearch`; `isClosed`, `effectiveProgress`, `finalizing` and `complete` name the flow. 852/852 across Content and hooks/SSE. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 * 🧽 style: Sweep The Narrating Failure Comments Codex flagged two; the same one-line paraphrase of `runStepStatus === 'failed'` had been copied into five files, so all five are removed rather than leaving three to surface next round. The named `hasError` / `errorState` / `error` booleans carry it. The remaining comments in these files explain non-obvious behavior rather than restating code — why `useProgress` needs both the terminal argument and the mask, why an errored web search renders as nothing, and which precedence applies to the legacy inference versus an explicit close. 852/852 across Content and hooks/SSE. Typecheck baseline is now 1 error (`useRum.ts`, unrelated) rather than 487, since building the `@librechat/client` workspace resolved the module errors that were masking it — identical with this diff and at the branch point. 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
GitHub
parent
a23ab9d16e
commit
db675209e8
9 changed files with 230 additions and 44 deletions
|
|
@ -226,6 +226,7 @@ const Part = memo(function Part({
|
|||
<ImageGen
|
||||
initialProgress={toolCall.progress ?? 0.1}
|
||||
isSubmitting={isSubmitting}
|
||||
runStepStatus={toolCall.runStepStatus}
|
||||
toolName={toolCall.name}
|
||||
args={toolCall.args ?? ''}
|
||||
output={toolCall.output ?? ''}
|
||||
|
|
@ -278,6 +279,7 @@ const Part = memo(function Part({
|
|||
output={toolCall.output ?? ''}
|
||||
initialProgress={toolCall.progress ?? 0.1}
|
||||
isSubmitting={isSubmitting}
|
||||
runStepStatus={toolCall.runStepStatus}
|
||||
attachments={attachments}
|
||||
persistedContent={persistedContent}
|
||||
hideAttachments={hideAttachments}
|
||||
|
|
@ -331,6 +333,7 @@ const Part = memo(function Part({
|
|||
output={toolCall.output ?? ''}
|
||||
initialProgress={toolCall.progress ?? 0.1}
|
||||
isSubmitting={isSubmitting}
|
||||
runStepStatus={toolCall.runStepStatus}
|
||||
attachments={attachments}
|
||||
isLast={isLast}
|
||||
onExpand={onToolExpand}
|
||||
|
|
@ -341,6 +344,7 @@ const Part = memo(function Part({
|
|||
<RetrievalCall
|
||||
initialProgress={toolCall.progress ?? 0.1}
|
||||
isSubmitting={isSubmitting}
|
||||
runStepStatus={toolCall.runStepStatus}
|
||||
args={toolCall.args}
|
||||
output={toolCall.output ?? undefined}
|
||||
attachments={attachments}
|
||||
|
|
@ -403,6 +407,7 @@ const Part = memo(function Part({
|
|||
<RetrievalCall
|
||||
initialProgress={toolCall.progress ?? 0.1}
|
||||
isSubmitting={isSubmitting}
|
||||
runStepStatus={toolCall.runStepStatus}
|
||||
output={(toolCall as { output?: string }).output}
|
||||
attachments={attachments}
|
||||
onExpand={onToolExpand}
|
||||
|
|
@ -418,6 +423,7 @@ const Part = memo(function Part({
|
|||
initialProgress={toolCall.progress ?? 0.1}
|
||||
args={toolCall.function.arguments as string}
|
||||
isSubmitting={isSubmitting}
|
||||
runStepStatus={toolCall.runStepStatus}
|
||||
toolName={toolCall.function.name}
|
||||
output={toolCall.function.output ?? ''}
|
||||
/>
|
||||
|
|
|
|||
|
|
@ -1,7 +1,12 @@
|
|||
import { useState, useEffect, useRef, useCallback } from 'react';
|
||||
import { AGENT_STYLE_TOOLS } from '.';
|
||||
import { PixelCard } from '@librechat/client';
|
||||
import type { TAttachment, TFile, TAttachmentMetadata } from 'librechat-data-provider';
|
||||
import type {
|
||||
TAttachment,
|
||||
TFile,
|
||||
TAttachmentMetadata,
|
||||
PartMetadata,
|
||||
} from 'librechat-data-provider';
|
||||
import { ToolIcon, isError } from '~/components/Chat/Messages/Content/ToolOutput';
|
||||
import Image from '~/components/Chat/Messages/Content/Image';
|
||||
import { useProgress, useLocalize } from '~/hooks';
|
||||
|
|
@ -13,7 +18,18 @@ function computeCancelled(
|
|||
isSubmitting: boolean | undefined,
|
||||
initialProgress: number,
|
||||
hasError: boolean,
|
||||
runStepStatus?: PartMetadata['runStepStatus'],
|
||||
): boolean {
|
||||
/**
|
||||
* The step's own terminal status wins when the run emitted one. Checked
|
||||
* first and not gated on `hasError`, so output parsing cannot demote a step
|
||||
* the run reported as stopped. Everything below is the pre-existing
|
||||
* inference, kept for messages saved before `on_run_step_closed` and for the
|
||||
* legacy path that has no submitting signal at all.
|
||||
*/
|
||||
if (runStepStatus != null) {
|
||||
return runStepStatus === 'cancelled';
|
||||
}
|
||||
if (isSubmitting !== undefined) {
|
||||
return (!isSubmitting && initialProgress < 1) || hasError;
|
||||
}
|
||||
|
|
@ -33,6 +49,7 @@ export default function OpenAIImageGen({
|
|||
output,
|
||||
attachments,
|
||||
hideAttachments = false,
|
||||
runStepStatus,
|
||||
}: {
|
||||
initialProgress: number;
|
||||
isSubmitting?: boolean;
|
||||
|
|
@ -41,6 +58,7 @@ export default function OpenAIImageGen({
|
|||
output?: string | null;
|
||||
attachments?: TAttachment[];
|
||||
hideAttachments?: boolean;
|
||||
runStepStatus?: PartMetadata['runStepStatus'];
|
||||
}) {
|
||||
const localize = useLocalize();
|
||||
/** Model-authored live label (injected when the tool is opted into
|
||||
|
|
@ -48,11 +66,16 @@ export default function OpenAIImageGen({
|
|||
const intent = useToolCallIntent(_args);
|
||||
const isAgentStyle = toolName != null && AGENT_STYLE_TOOLS.has(toolName);
|
||||
const [agentProgress, setAgentProgress] = useState(initialProgress);
|
||||
const legacyProgress = useProgress(isAgentStyle ? 1 : initialProgress);
|
||||
const progress = isAgentStyle ? agentProgress : legacyProgress;
|
||||
const isClosed = runStepStatus != null;
|
||||
/** Passing 1 in stops `useProgress` scheduling its interval; masking the
|
||||
* result makes the terminal value observable on the same render, since the
|
||||
* hook settles through 0.99 and a 200ms timeout. */
|
||||
const legacyProgress = useProgress(isAgentStyle || isClosed ? 1 : initialProgress);
|
||||
const livingProgress = isAgentStyle ? agentProgress : legacyProgress;
|
||||
const progress = isClosed ? 1 : livingProgress;
|
||||
const intervalRef = useRef<NodeJS.Timeout | null>(null);
|
||||
|
||||
const hasError = typeof output === 'string' && isError(output);
|
||||
const hasError = (typeof output === 'string' && isError(output)) || runStepStatus === 'failed';
|
||||
|
||||
/**
|
||||
* Determines if the image generation was cancelled.
|
||||
|
|
@ -60,7 +83,14 @@ export default function OpenAIImageGen({
|
|||
* - Legacy path (isSubmitting undefined): in-progress (0 < progress < 1) is never cancelled
|
||||
* because legacy image gen lacks a submitting signal — only errors cancel.
|
||||
*/
|
||||
const cancelled = computeCancelled(isSubmitting, initialProgress, hasError);
|
||||
const cancelled = computeCancelled(isSubmitting, initialProgress, hasError, runStepStatus);
|
||||
/**
|
||||
* An explicit `cancelled` close is authoritative and outranks an
|
||||
* error-formatted output — aborting a tool can itself produce one. The
|
||||
* legacy inference keeps the opposite precedence, because it folds
|
||||
* `hasError` into its own cancellation signal.
|
||||
*/
|
||||
const reportsError = hasError && runStepStatus !== 'cancelled';
|
||||
|
||||
let width: number | undefined;
|
||||
let height: number | undefined;
|
||||
|
|
@ -133,7 +163,7 @@ export default function OpenAIImageGen({
|
|||
return;
|
||||
}
|
||||
|
||||
if (isSubmitting) {
|
||||
if (isSubmitting && !isClosed) {
|
||||
setAgentProgress(initialProgress);
|
||||
|
||||
if (intervalRef.current) {
|
||||
|
|
@ -179,20 +209,20 @@ export default function OpenAIImageGen({
|
|||
clearInterval(intervalRef.current);
|
||||
}
|
||||
};
|
||||
}, [isSubmitting, initialProgress, quality, isAgentStyle]);
|
||||
}, [isSubmitting, initialProgress, quality, isAgentStyle, isClosed]);
|
||||
|
||||
useEffect(() => {
|
||||
if (!isAgentStyle) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (initialProgress >= 1 || cancelled) {
|
||||
if (initialProgress >= 1 || cancelled || isClosed) {
|
||||
setAgentProgress(initialProgress);
|
||||
if (intervalRef.current) {
|
||||
clearInterval(intervalRef.current);
|
||||
}
|
||||
}
|
||||
}, [initialProgress, cancelled, isAgentStyle]);
|
||||
}, [initialProgress, cancelled, isAgentStyle, isClosed]);
|
||||
|
||||
useEffect(() => {
|
||||
updateDimensions();
|
||||
|
|
@ -216,10 +246,10 @@ export default function OpenAIImageGen({
|
|||
<>
|
||||
<span className="sr-only" aria-live="polite" aria-atomic="true">
|
||||
{(() => {
|
||||
if (progress < 1 && !cancelled) {
|
||||
if (progress < 1 && !cancelled && !reportsError) {
|
||||
return '';
|
||||
}
|
||||
if (cancelled && hasError) {
|
||||
if (reportsError) {
|
||||
return localize('com_ui_image_gen_failed');
|
||||
}
|
||||
if (cancelled) {
|
||||
|
|
@ -230,7 +260,13 @@ export default function OpenAIImageGen({
|
|||
</span>
|
||||
<div className="relative my-1 flex h-5 shrink-0 items-center gap-2">
|
||||
<ToolIcon type="image_gen" isAnimating={isInProgress} />
|
||||
<ProgressText progress={progress} error={cancelled} toolName={toolName} intent={intent} />
|
||||
<ProgressText
|
||||
progress={progress}
|
||||
error={reportsError}
|
||||
cancelled={cancelled}
|
||||
toolName={toolName}
|
||||
intent={intent}
|
||||
/>
|
||||
</div>
|
||||
{isAgentStyle && !hideAttachments && (
|
||||
<div className="relative mb-2 flex w-full justify-start">
|
||||
|
|
|
|||
|
|
@ -5,11 +5,15 @@ import { cn } from '~/utils';
|
|||
export default function ProgressText({
|
||||
progress,
|
||||
error,
|
||||
cancelled,
|
||||
toolName = '',
|
||||
intent,
|
||||
}: {
|
||||
progress: number;
|
||||
error?: boolean;
|
||||
/** Stopped rather than failed — kept separate from `error` so a generation
|
||||
* the user cancelled is not labelled a failure. */
|
||||
cancelled?: boolean;
|
||||
toolName?: string;
|
||||
/** Model-authored label; wins over the phase texts (error state excepted). */
|
||||
intent?: string;
|
||||
|
|
@ -17,9 +21,15 @@ export default function ProgressText({
|
|||
const localize = useLocalize();
|
||||
|
||||
const getText = () => {
|
||||
/** Failure outranks cancellation: the legacy inference folds errors into
|
||||
* its cancellation signal, so checking `cancelled` first would relabel a
|
||||
* genuine failure as a user stop. */
|
||||
if (error) {
|
||||
return localize('com_ui_image_gen_failed');
|
||||
}
|
||||
if (cancelled) {
|
||||
return localize('com_ui_cancelled');
|
||||
}
|
||||
if (intent != null) {
|
||||
return intent;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -9,7 +9,13 @@ import {
|
|||
OGDialogContent,
|
||||
OGDialogDescription,
|
||||
} from '@librechat/client';
|
||||
import type { Agents, TAttachment, TMessage, TMessageContentParts } from 'librechat-data-provider';
|
||||
import type {
|
||||
Agents,
|
||||
TAttachment,
|
||||
TMessage,
|
||||
TMessageContentParts,
|
||||
PartMetadata,
|
||||
} from 'librechat-data-provider';
|
||||
import type { PartWithIndex } from '~/components/Chat/Messages/Content/ParallelContent';
|
||||
import type { SubagentTickerLine } from '~/utils/subagentContent';
|
||||
import ToolCallGroup from '~/components/Chat/Messages/Content/ToolCallGroup';
|
||||
|
|
@ -36,6 +42,9 @@ interface SubagentCallProps {
|
|||
* tool_call's `progress` and any terminal subagent envelope — to decide
|
||||
* whether the subagent is `running`, `cancelled`, or `finished`. */
|
||||
isSubmitting?: boolean;
|
||||
/** Terminal lifecycle status from `on_run_step_closed`, when the run
|
||||
* emitted one. Authoritative over the `isSubmitting` inference. */
|
||||
runStepStatus?: PartMetadata['runStepStatus'];
|
||||
args?: string | Record<string, unknown>;
|
||||
output?: string | null;
|
||||
attachments?: TAttachment[];
|
||||
|
|
@ -165,6 +174,7 @@ export default function SubagentCall({
|
|||
toolCallId,
|
||||
initialProgress,
|
||||
isSubmitting = false,
|
||||
runStepStatus,
|
||||
args,
|
||||
output,
|
||||
attachments,
|
||||
|
|
@ -198,9 +208,25 @@ export default function SubagentCall({
|
|||
* - `running`: the parent is still streaming and no terminal signal has
|
||||
* arrived yet.
|
||||
*/
|
||||
const hasError = progress?.status === 'error';
|
||||
const finished = initialProgress >= 1 || progress?.status === 'stop' || hasError;
|
||||
const cancelled = !isSubmitting && !finished;
|
||||
/**
|
||||
* A closed run step resolves the tri-state directly. It is the only signal
|
||||
* that distinguishes "this subagent was stopped" from "the parent stream
|
||||
* ended for some other reason", which the `!isSubmitting` inference below
|
||||
* cannot tell apart. That inference stays as the fallback for messages
|
||||
* saved before `on_run_step_closed` and endpoints that do not emit it.
|
||||
*/
|
||||
const isClosed = runStepStatus != null;
|
||||
/**
|
||||
* An explicit `cancelled` close outranks a live `error` phase: aborting a
|
||||
* child can surface through its execution as an error, and the run's own
|
||||
* status is the authority on why it stopped.
|
||||
*/
|
||||
const hasError =
|
||||
(progress?.status === 'error' || runStepStatus === 'failed') && runStepStatus !== 'cancelled';
|
||||
const finished = isClosed
|
||||
? runStepStatus !== 'cancelled'
|
||||
: initialProgress >= 1 || progress?.status === 'stop' || hasError;
|
||||
const cancelled = isClosed ? runStepStatus === 'cancelled' : !isSubmitting && !finished;
|
||||
const running = !finished && !cancelled;
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -27,10 +27,6 @@ export default function useToolCallState(
|
|||
): ToolCallState {
|
||||
const autoExpand = useRecoilValue(store.autoExpandTools);
|
||||
const hasOutput = output.length > 0;
|
||||
/**
|
||||
* A step the run closed as `failed` is an error even when its output does
|
||||
* not look like one — the run knows something the output text does not.
|
||||
*/
|
||||
const hasError = (hasOutput && isError(output)) || runStepStatus === 'failed';
|
||||
const hasContent = hasInput || hasOutput;
|
||||
|
||||
|
|
@ -43,13 +39,13 @@ export default function useToolCallState(
|
|||
}
|
||||
}, [autoExpand, hasContent]);
|
||||
|
||||
/**
|
||||
* A closed step is terminal by definition, so it must never animate. Forcing
|
||||
* the bar complete keeps the shimmer from outliving a step that closed
|
||||
* without ever emitting a completion event.
|
||||
*/
|
||||
const isClosed = runStepStatus != null;
|
||||
const rawProgress = useProgress(initialProgress);
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
const rawProgress = useProgress(isClosed ? 1 : initialProgress);
|
||||
const progress = isClosed ? 1 : rawProgress;
|
||||
const toggleCode = useCallback(() => {
|
||||
setShowCode((prev) => {
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ import { useRecoilValue } from 'recoil';
|
|||
import { Tools } from 'librechat-data-provider';
|
||||
import { TooltipAnchor } from '@librechat/client';
|
||||
import { FileText, FileSpreadsheet, FileCode, FileImage, File } from 'lucide-react';
|
||||
import type { TAttachment, TFile } from 'librechat-data-provider';
|
||||
import type { TAttachment, TFile, PartMetadata } from 'librechat-data-provider';
|
||||
import { useLocalize, useProgress, useExpandCollapse } from '~/hooks';
|
||||
import { ToolIcon, OutputRenderer, isError } from './ToolOutput';
|
||||
import FilePreviewDialog from './FilePreviewDialog';
|
||||
|
|
@ -329,6 +329,7 @@ export default function RetrievalCall({
|
|||
output,
|
||||
attachments,
|
||||
onExpand,
|
||||
runStepStatus,
|
||||
}: {
|
||||
initialProgress: number;
|
||||
isSubmitting: boolean;
|
||||
|
|
@ -336,16 +337,36 @@ export default function RetrievalCall({
|
|||
output?: string;
|
||||
attachments?: TAttachment[];
|
||||
onExpand?: () => void;
|
||||
runStepStatus?: PartMetadata['runStepStatus'];
|
||||
}) {
|
||||
const progress = useProgress(initialProgress);
|
||||
const isClosed = runStepStatus != null;
|
||||
/**
|
||||
* Both halves are load-bearing. Passing 1 in stops `useProgress` scheduling
|
||||
* its 200ms interval, which it keeps alive for any input below 1. Masking
|
||||
* the result makes the terminal value observable on the same render — the
|
||||
* hook settles through 0.99 and a 200ms timeout, so a step closing while
|
||||
* 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);
|
||||
const cancelled = !isSubmitting && initialProgress < 1 && !errorState;
|
||||
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.
|
||||
*/
|
||||
const cancelled = isClosed
|
||||
? runStepStatus === 'cancelled'
|
||||
: !isSubmitting && initialProgress < 1 && !errorState;
|
||||
const hasOutput = !!output && !isError(output);
|
||||
const autoExpand = useRecoilValue(store.autoExpandTools);
|
||||
const [showOutput, setShowOutput] = useState(() => autoExpand && hasOutput);
|
||||
|
|
@ -429,6 +450,12 @@ export default function RetrievalCall({
|
|||
if (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) {
|
||||
return localize('com_ui_failed');
|
||||
}
|
||||
return intent ?? localize('com_ui_retrieved_files');
|
||||
})()}
|
||||
</span>
|
||||
|
|
@ -437,7 +464,13 @@ export default function RetrievalCall({
|
|||
progress={progress}
|
||||
onClick={hasOutput ? handleToggleOutput : undefined}
|
||||
inProgressText={intent ?? localize('com_ui_searching_files')}
|
||||
finishedText={intent ?? localize('com_ui_retrieved_files')}
|
||||
/** A cancelled step must not read "Retrieved files" beside a
|
||||
* cancellation icon while the live region says "Cancelled". */
|
||||
finishedText={
|
||||
cancelled
|
||||
? localize('com_ui_cancelled')
|
||||
: (intent ?? localize('com_ui_retrieved_files'))
|
||||
}
|
||||
errorSuffix={errorState && !cancelled ? localize('com_ui_tool_failed') : undefined}
|
||||
icon={
|
||||
<ToolIcon type="file_search" isAnimating={progress < 1 && !cancelled && !errorState} />
|
||||
|
|
|
|||
|
|
@ -139,8 +139,6 @@ export default function ToolCall({
|
|||
window.open(auth, '_blank', 'noopener,noreferrer');
|
||||
}, [auth, isMCPToolCall, mcpServerName, actionId]);
|
||||
|
||||
/** A step the run closed as `failed` is an error even when its output text
|
||||
* does not parse as one. */
|
||||
const hasError = (typeof output === 'string' && isError(output)) || runStepStatus === 'failed';
|
||||
/**
|
||||
* The step's own terminal status wins when the run emitted one. The
|
||||
|
|
@ -184,8 +182,12 @@ export default function ToolCall({
|
|||
return parsedAuthUrl?.hostname ?? '';
|
||||
}, [parsedAuthUrl]);
|
||||
|
||||
/** A closed step is terminal, so it must never keep animating. */
|
||||
const rawProgress = useProgress(initialProgress);
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
const rawProgress = useProgress(isClosed ? 1 : initialProgress);
|
||||
const progress = isClosed ? 1 : rawProgress;
|
||||
const showCancelled = cancelled || (errorState && !output);
|
||||
|
||||
|
|
|
|||
|
|
@ -2,7 +2,12 @@ import { useMemo, useState, useEffect } from 'react';
|
|||
import { useRecoilValue } from 'recoil';
|
||||
import { Tools } from 'librechat-data-provider';
|
||||
import { Globe, ChevronDown } from 'lucide-react';
|
||||
import type { TAttachment, ValidSource, SearchResultData } from 'librechat-data-provider';
|
||||
import type {
|
||||
TAttachment,
|
||||
ValidSource,
|
||||
SearchResultData,
|
||||
PartMetadata,
|
||||
} from 'librechat-data-provider';
|
||||
import { FaviconImage, getCleanDomain } from '~/components/Web/SourceHovercard';
|
||||
import { StackedFavicons } from '~/components/Web/Sources';
|
||||
import { useLocalize, useExpandCollapse } from '~/hooks';
|
||||
|
|
@ -85,6 +90,7 @@ export default function WebSearch({
|
|||
output,
|
||||
attachments,
|
||||
onExpand,
|
||||
runStepStatus,
|
||||
}: {
|
||||
isLast?: boolean;
|
||||
isSubmitting: boolean;
|
||||
|
|
@ -93,13 +99,17 @@ export default function WebSearch({
|
|||
initialProgress: number;
|
||||
attachments?: TAttachment[];
|
||||
onExpand?: () => void;
|
||||
runStepStatus?: PartMetadata['runStepStatus'];
|
||||
}) {
|
||||
const localize = useLocalize();
|
||||
/** Model-authored live label (web_search carries `intent` natively);
|
||||
* persists as the settled label like the other tool cards. */
|
||||
const intent = useToolCallIntent(args);
|
||||
const { searchResults } = useSearchContext();
|
||||
const error = typeof output === 'string' && output.toLowerCase().includes('error processing');
|
||||
const error =
|
||||
(typeof output === 'string' && output.toLowerCase().includes('error processing')) ||
|
||||
runStepStatus === 'failed';
|
||||
const isClosed = runStepStatus != null;
|
||||
|
||||
// Server tool calls (srvtoolu_) never receive ON_RUN_STEP_COMPLETED, so progress
|
||||
// stays at the default 0.1. Treat the search as complete if attachments have results.
|
||||
|
|
@ -108,15 +118,27 @@ export default function WebSearch({
|
|||
attachments?.some((att) => att.type === Tools.web_search && att[Tools.web_search]) ?? false,
|
||||
[attachments],
|
||||
);
|
||||
const effectiveProgress = hasResults && !isSubmitting ? 1 : progress;
|
||||
const cancelled = (!isSubmitting && effectiveProgress < 1) || error === true;
|
||||
const effectiveProgress = isClosed || (hasResults && !isSubmitting) ? 1 : progress;
|
||||
/**
|
||||
* `error` folds into this branch deliberately: an errored search has always
|
||||
* rendered as nothing (the `cancelled` early-return below), so a step closed
|
||||
* as `failed` lands in the same place rather than inventing a failure UI
|
||||
* this component has never had — or worse, falling through to the streaming
|
||||
* branch and shimmering forever.
|
||||
*/
|
||||
const cancelled = isClosed
|
||||
? runStepStatus === 'cancelled' || error
|
||||
: (!isSubmitting && effectiveProgress < 1) || error === true;
|
||||
|
||||
const finalizing = isSubmitting && isLast && effectiveProgress === 1;
|
||||
const finalizing = !isClosed && isSubmitting && isLast && effectiveProgress === 1;
|
||||
/** A search that is the message's FINAL part stays "finalizing" only while
|
||||
* the submission is live — afterwards it must settle like any other call,
|
||||
* or the completed label (and its settled intent announcement) never
|
||||
* renders and the card shimmers forever. */
|
||||
const complete = effectiveProgress === 1 && !finalizing && (!isLast || !isSubmitting);
|
||||
* renders and the card shimmers forever. A closed step settles immediately
|
||||
* on its own status instead of waiting for the submission to end. */
|
||||
const complete = isClosed
|
||||
? !cancelled
|
||||
: effectiveProgress === 1 && !finalizing && (!isLast || !isSubmitting);
|
||||
|
||||
const ownTurn = useMemo((): string => {
|
||||
if (!attachments) {
|
||||
|
|
|
|||
|
|
@ -38,8 +38,21 @@ jest.mock('../ToolOutput', () => ({
|
|||
|
||||
jest.mock('../Parts/OpenAIImageGen/ProgressText', () => ({
|
||||
__esModule: true,
|
||||
default: ({ progress, error }: { progress: number; error: boolean }) => (
|
||||
<div data-testid="progress-text" data-progress={progress} data-error={String(error)} />
|
||||
default: ({
|
||||
progress,
|
||||
error,
|
||||
cancelled,
|
||||
}: {
|
||||
progress: number;
|
||||
error: boolean;
|
||||
cancelled?: boolean;
|
||||
}) => (
|
||||
<div
|
||||
data-testid="progress-text"
|
||||
data-progress={progress}
|
||||
data-error={String(error)}
|
||||
data-cancelled={String(cancelled === true)}
|
||||
/>
|
||||
),
|
||||
}));
|
||||
|
||||
|
|
@ -158,6 +171,48 @@ describe('OpenAIImageGen', () => {
|
|||
it('shows cancelled state when not submitting and incomplete', () => {
|
||||
render(<OpenAIImageGen {...defaultProps} isSubmitting={false} initialProgress={0.5} />);
|
||||
const progressText = screen.getByTestId('progress-text');
|
||||
expect(progressText).toHaveAttribute('data-cancelled', 'true');
|
||||
expect(progressText).toHaveAttribute('data-error', 'false');
|
||||
});
|
||||
|
||||
it('reports a run-step cancellation as cancelled, not failed', () => {
|
||||
render(<OpenAIImageGen {...defaultProps} isSubmitting={true} runStepStatus="cancelled" />);
|
||||
const progressText = screen.getByTestId('progress-text');
|
||||
expect(progressText).toHaveAttribute('data-cancelled', 'true');
|
||||
expect(progressText).toHaveAttribute('data-error', 'false');
|
||||
});
|
||||
|
||||
it('reports a run-step failure as an error even when the output is benign', () => {
|
||||
render(<OpenAIImageGen {...defaultProps} isSubmitting={true} runStepStatus="failed" />);
|
||||
const progressText = screen.getByTestId('progress-text');
|
||||
expect(progressText).toHaveAttribute('data-error', 'true');
|
||||
expect(progressText).toHaveAttribute('data-cancelled', 'false');
|
||||
});
|
||||
|
||||
it('lets an explicit cancellation outrank an error-formatted output', () => {
|
||||
render(
|
||||
<OpenAIImageGen
|
||||
{...defaultProps}
|
||||
output="Error processing tool call"
|
||||
isSubmitting={true}
|
||||
runStepStatus="cancelled"
|
||||
/>,
|
||||
);
|
||||
const progressText = screen.getByTestId('progress-text');
|
||||
expect(progressText).toHaveAttribute('data-cancelled', 'true');
|
||||
expect(progressText).toHaveAttribute('data-error', 'false');
|
||||
});
|
||||
|
||||
it('keeps failure precedence when the legacy inference folds an error into cancellation', () => {
|
||||
render(
|
||||
<OpenAIImageGen
|
||||
{...defaultProps}
|
||||
output="Error processing tool call"
|
||||
isSubmitting={false}
|
||||
initialProgress={0.5}
|
||||
/>,
|
||||
);
|
||||
const progressText = screen.getByTestId('progress-text');
|
||||
expect(progressText).toHaveAttribute('data-error', 'true');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue