mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-21 17:44:11 +00:00
fix(client): address Codex review findings on tool group and summary UI
Replace the raw green/red diff palette in DiffView and CodeWindowHeader with the existing status-success/status-error semantic roles, so a customized or versioned theme moves the diff colors with it. Compose the shared Button primitive in the ask option rows instead of recreating hover, focus-visible and disabled appearance locally, and swap the raw text-white on the checked badge for text-on-status. Prefer a THINK part's generated reasoning_label in the grouped rendering path, matching the standalone Reasoning path, with the generic Thinking/Thoughts text only as a fallback. Derive grouped tool completion and failure from the run step's terminal status through the shared resolveToolCallPhase helper, so a step the run closed as failed with empty or benign output no longer reads as running and then reports success while the card beside it shows a failure. Carry an explicit failed flag on a summarize round that ended in error, so the partial deltas already streamed into the slot are no longer presented under the "Conversation summarized" label. Also drop seven orphaned English locale keys this branch left unused, which were failing the unused-i18n static check.
This commit is contained in:
parent
05170bdc94
commit
62cf3879d2
12 changed files with 299 additions and 142 deletions
|
|
@ -166,6 +166,7 @@ const Part = memo(function Part({
|
|||
provider={part.provider}
|
||||
tokenCount={part.tokenCount}
|
||||
summarizing={part.summarizing}
|
||||
failed={part.failed}
|
||||
/>
|
||||
);
|
||||
} else if (part.type === ContentTypes.ACTIVITY_LABEL) {
|
||||
|
|
|
|||
|
|
@ -31,8 +31,8 @@ export default function CodeWindowHeader({ language, code, diffStats }: CodeWind
|
|||
{language}
|
||||
{diffStats && (
|
||||
<span className="flex items-center gap-1 font-normal tabular-nums" aria-hidden="true">
|
||||
<span className="text-green-600 dark:text-green-400">+{diffStats.additions}</span>
|
||||
<span className="text-red-600 dark:text-red-400">-{diffStats.deletions}</span>
|
||||
<span className="text-status-success">+{diffStats.additions}</span>
|
||||
<span className="text-status-error">-{diffStats.deletions}</span>
|
||||
</span>
|
||||
)}
|
||||
</span>
|
||||
|
|
|
|||
|
|
@ -121,8 +121,8 @@ export default function DiffView({ parsed }: { parsed: ParsedDiff }) {
|
|||
key={index}
|
||||
className={cn(
|
||||
'flex',
|
||||
line.type === 'add' && 'bg-green-500/10',
|
||||
line.type === 'del' && 'bg-red-500/10',
|
||||
line.type === 'add' && 'bg-status-success-subtle',
|
||||
line.type === 'del' && 'bg-status-error-subtle',
|
||||
)}
|
||||
>
|
||||
{hasLineNumbers && (
|
||||
|
|
@ -134,8 +134,8 @@ export default function DiffView({ parsed }: { parsed: ParsedDiff }) {
|
|||
className={cn(
|
||||
'shrink-0 select-none text-center font-semibold',
|
||||
hasLineNumbers ? 'w-5' : 'w-6',
|
||||
line.type === 'add' && 'text-green-600 dark:text-green-400',
|
||||
line.type === 'del' && 'text-red-600 dark:text-red-400',
|
||||
line.type === 'add' && 'text-status-success',
|
||||
line.type === 'del' && 'text-status-error',
|
||||
)}
|
||||
>
|
||||
{LINE_MARKERS[line.type]}
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@ import { cn } from '~/utils';
|
|||
|
||||
type SummaryProps = Pick<
|
||||
SummaryContentPart,
|
||||
'content' | 'model' | 'provider' | 'tokenCount' | 'summarizing'
|
||||
'content' | 'model' | 'provider' | 'tokenCount' | 'summarizing' | 'failed'
|
||||
>;
|
||||
|
||||
function useCopyToClipboard(content?: string) {
|
||||
|
|
@ -203,115 +203,120 @@ const FloatingSummaryBar = memo(
|
|||
},
|
||||
);
|
||||
|
||||
const Summary = memo(({ content, model, provider, tokenCount, summarizing }: SummaryProps) => {
|
||||
const contentId = useId();
|
||||
const localize = useLocalize();
|
||||
const [isExpanded, setIsExpanded] = useState(false);
|
||||
const [isBarVisible, setIsBarVisible] = useState(false);
|
||||
const containerRef = useRef<HTMLDivElement>(null);
|
||||
const { isSubmitting, isLatestMessage } = useMessageContext();
|
||||
const Summary = memo(
|
||||
({ content, model, provider, tokenCount, summarizing, failed }: SummaryProps) => {
|
||||
const contentId = useId();
|
||||
const localize = useLocalize();
|
||||
const [isExpanded, setIsExpanded] = useState(false);
|
||||
const [isBarVisible, setIsBarVisible] = useState(false);
|
||||
const containerRef = useRef<HTMLDivElement>(null);
|
||||
const { isSubmitting, isLatestMessage } = useMessageContext();
|
||||
|
||||
const text = useMemo(
|
||||
() =>
|
||||
(content ?? [])
|
||||
.map((block) => ('text' in block && typeof block.text === 'string' ? block.text : ''))
|
||||
.join(''),
|
||||
[content],
|
||||
);
|
||||
const { isCopied, handleCopy } = useCopyToClipboard(text);
|
||||
const text = useMemo(
|
||||
() =>
|
||||
(content ?? [])
|
||||
.map((block) => ('text' in block && typeof block.text === 'string' ? block.text : ''))
|
||||
.join(''),
|
||||
[content],
|
||||
);
|
||||
const { isCopied, handleCopy } = useCopyToClipboard(text);
|
||||
|
||||
const handleClick = useCallback((e: MouseEvent<HTMLButtonElement>) => {
|
||||
e.preventDefault();
|
||||
setIsExpanded((prev) => !prev);
|
||||
}, []);
|
||||
const handleClick = useCallback((e: MouseEvent<HTMLButtonElement>) => {
|
||||
e.preventDefault();
|
||||
setIsExpanded((prev) => !prev);
|
||||
}, []);
|
||||
|
||||
const handleFocus = useCallback(() => setIsBarVisible(true), []);
|
||||
const handleBlur = useCallback((e: FocusEvent) => {
|
||||
if (!containerRef.current?.contains(e.relatedTarget as Node)) {
|
||||
setIsBarVisible(false);
|
||||
const handleFocus = useCallback(() => setIsBarVisible(true), []);
|
||||
const handleBlur = useCallback((e: FocusEvent) => {
|
||||
if (!containerRef.current?.contains(e.relatedTarget as Node)) {
|
||||
setIsBarVisible(false);
|
||||
}
|
||||
}, []);
|
||||
const handleMouseEnter = useCallback(() => setIsBarVisible(true), []);
|
||||
const handleMouseLeave = useCallback(() => {
|
||||
if (!containerRef.current?.contains(document.activeElement)) {
|
||||
setIsBarVisible(false);
|
||||
}
|
||||
}, []);
|
||||
|
||||
const effectiveIsSubmitting = isLatestMessage ? isSubmitting : false;
|
||||
const isActivelyStreaming = !!summarizing && !!effectiveIsSubmitting;
|
||||
|
||||
const meta = useMemo(() => {
|
||||
const parts: string[] = [];
|
||||
if (provider || model) {
|
||||
parts.push([provider, model].filter(Boolean).join('/'));
|
||||
}
|
||||
if (tokenCount != null && tokenCount > 0) {
|
||||
parts.push(`${tokenCount} ${localize('com_ui_tokens')}`);
|
||||
}
|
||||
return parts.length > 0 ? parts.join(' \u00b7 ') : undefined;
|
||||
}, [model, provider, tokenCount, localize]);
|
||||
|
||||
/** A failed round keeps whatever deltas already streamed in, so the label
|
||||
* must not claim the conversation was summarized. */
|
||||
const label = useMemo(() => {
|
||||
if (isActivelyStreaming) {
|
||||
return localize('com_ui_summarizing');
|
||||
}
|
||||
return failed
|
||||
? localize('com_ui_summarize_failed')
|
||||
: localize('com_ui_conversation_summarized');
|
||||
}, [isActivelyStreaming, failed, localize]);
|
||||
|
||||
if (!summarizing && !text) {
|
||||
return null;
|
||||
}
|
||||
}, []);
|
||||
const handleMouseEnter = useCallback(() => setIsBarVisible(true), []);
|
||||
const handleMouseLeave = useCallback(() => {
|
||||
if (!containerRef.current?.contains(document.activeElement)) {
|
||||
setIsBarVisible(false);
|
||||
}
|
||||
}, []);
|
||||
|
||||
const effectiveIsSubmitting = isLatestMessage ? isSubmitting : false;
|
||||
const isActivelyStreaming = !!summarizing && !!effectiveIsSubmitting;
|
||||
|
||||
const meta = useMemo(() => {
|
||||
const parts: string[] = [];
|
||||
if (provider || model) {
|
||||
parts.push([provider, model].filter(Boolean).join('/'));
|
||||
}
|
||||
if (tokenCount != null && tokenCount > 0) {
|
||||
parts.push(`${tokenCount} ${localize('com_ui_tokens')}`);
|
||||
}
|
||||
return parts.length > 0 ? parts.join(' \u00b7 ') : undefined;
|
||||
}, [model, provider, tokenCount, localize]);
|
||||
|
||||
const label = useMemo(
|
||||
() =>
|
||||
isActivelyStreaming
|
||||
? localize('com_ui_summarizing')
|
||||
: localize('com_ui_conversation_summarized'),
|
||||
[isActivelyStreaming, localize],
|
||||
);
|
||||
|
||||
if (!summarizing && !text) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return (
|
||||
<div
|
||||
ref={containerRef}
|
||||
className="group/summary"
|
||||
onMouseEnter={handleMouseEnter}
|
||||
onMouseLeave={handleMouseLeave}
|
||||
onFocus={handleFocus}
|
||||
onBlur={handleBlur}
|
||||
>
|
||||
<div className="group/summary-container">
|
||||
<div className="mb-2 pb-2 pt-2">
|
||||
<SummaryButton
|
||||
isExpanded={isExpanded}
|
||||
onClick={handleClick}
|
||||
label={label}
|
||||
content={text}
|
||||
contentId={contentId}
|
||||
showCopyButton={!isActivelyStreaming}
|
||||
isCopied={isCopied}
|
||||
onCopy={handleCopy}
|
||||
/>
|
||||
</div>
|
||||
<div
|
||||
id={contentId}
|
||||
role="region"
|
||||
aria-label={label}
|
||||
aria-hidden={!isExpanded || undefined}
|
||||
className={cn('grid transition-all duration-300 ease-out', isExpanded && 'mb-4')}
|
||||
style={{
|
||||
gridTemplateRows: isExpanded ? '1fr' : '0fr',
|
||||
}}
|
||||
>
|
||||
<div className="relative overflow-hidden">
|
||||
<SummaryContent meta={meta}>{text}</SummaryContent>
|
||||
<FloatingSummaryBar
|
||||
isVisible={isBarVisible && isExpanded}
|
||||
return (
|
||||
<div
|
||||
ref={containerRef}
|
||||
className="group/summary"
|
||||
onMouseEnter={handleMouseEnter}
|
||||
onMouseLeave={handleMouseLeave}
|
||||
onFocus={handleFocus}
|
||||
onBlur={handleBlur}
|
||||
>
|
||||
<div className="group/summary-container">
|
||||
<div className="mb-2 pb-2 pt-2">
|
||||
<SummaryButton
|
||||
isExpanded={isExpanded}
|
||||
onClick={handleClick}
|
||||
label={label}
|
||||
content={text}
|
||||
contentId={contentId}
|
||||
showCopyButton={!isActivelyStreaming}
|
||||
isCopied={isCopied}
|
||||
onCopy={handleCopy}
|
||||
/>
|
||||
</div>
|
||||
<div
|
||||
id={contentId}
|
||||
role="region"
|
||||
aria-label={label}
|
||||
aria-hidden={!isExpanded || undefined}
|
||||
className={cn('grid transition-all duration-300 ease-out', isExpanded && 'mb-4')}
|
||||
style={{
|
||||
gridTemplateRows: isExpanded ? '1fr' : '0fr',
|
||||
}}
|
||||
>
|
||||
<div className="relative overflow-hidden">
|
||||
<SummaryContent meta={meta}>{text}</SummaryContent>
|
||||
<FloatingSummaryBar
|
||||
isVisible={isBarVisible && isExpanded}
|
||||
onClick={handleClick}
|
||||
content={text}
|
||||
contentId={contentId}
|
||||
isCopied={isCopied}
|
||||
onCopy={handleCopy}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
});
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
SummaryContent.displayName = 'SummaryContent';
|
||||
SummaryButton.displayName = 'SummaryButton';
|
||||
|
|
|
|||
|
|
@ -158,8 +158,8 @@ describe('FileAuthoringCall', () => {
|
|||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByText('old line').closest('div')).toHaveClass('bg-red-500/10');
|
||||
expect(screen.getByText('new line').closest('div')).toHaveClass('bg-green-500/10');
|
||||
expect(screen.getByText('old line').closest('div')).toHaveClass('bg-status-error-subtle');
|
||||
expect(screen.getByText('new line').closest('div')).toHaveClass('bg-status-success-subtle');
|
||||
expect(screen.queryByText(/--- old_text/)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
|
|
@ -178,8 +178,8 @@ describe('FileAuthoringCall', () => {
|
|||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByText('missing text').closest('div')).toHaveClass('bg-red-500/10');
|
||||
expect(screen.getByText('replacement').closest('div')).toHaveClass('bg-green-500/10');
|
||||
expect(screen.getByText('missing text').closest('div')).toHaveClass('bg-status-error-subtle');
|
||||
expect(screen.getByText('replacement').closest('div')).toHaveClass('bg-status-success-subtle');
|
||||
expect(screen.getByText(/matched 0 locations/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
|
|
@ -202,10 +202,10 @@ describe('FileAuthoringCall', () => {
|
|||
expect(screen.getByTestId('code-window-header')).toHaveAttribute('data-language', 'SKILL.md');
|
||||
expect(screen.queryByText(/--- old_text/)).not.toBeInTheDocument();
|
||||
expect(screen.getByText('description: Old behavior').closest('div')).toHaveClass(
|
||||
'bg-red-500/10',
|
||||
'bg-status-error-subtle',
|
||||
);
|
||||
expect(screen.getByText('description: New behavior').closest('div')).toHaveClass(
|
||||
'bg-green-500/10',
|
||||
'bg-status-success-subtle',
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -239,9 +239,11 @@ describe('FileAuthoringCall', () => {
|
|||
);
|
||||
|
||||
expect(screen.getByText('description: Old behavior').closest('div')).toHaveClass(
|
||||
'bg-red-500/10',
|
||||
'bg-status-error-subtle',
|
||||
);
|
||||
expect(screen.getByText('description: New beh').closest('div')).toHaveClass(
|
||||
'bg-status-success-subtle',
|
||||
);
|
||||
expect(screen.getByText('description: New beh').closest('div')).toHaveClass('bg-green-500/10');
|
||||
});
|
||||
|
||||
it('streams batched edit_file previews from a partial edits array', () => {
|
||||
|
|
@ -260,10 +262,10 @@ describe('FileAuthoringCall', () => {
|
|||
);
|
||||
|
||||
const diff = screen.getByTestId('diff-view');
|
||||
expect(screen.getByText('first old').closest('div')).toHaveClass('bg-red-500/10');
|
||||
expect(screen.getByText('first new').closest('div')).toHaveClass('bg-green-500/10');
|
||||
expect(screen.getByText('second old').closest('div')).toHaveClass('bg-red-500/10');
|
||||
expect(screen.getByText('second n').closest('div')).toHaveClass('bg-green-500/10');
|
||||
expect(screen.getByText('first old').closest('div')).toHaveClass('bg-status-error-subtle');
|
||||
expect(screen.getByText('first new').closest('div')).toHaveClass('bg-status-success-subtle');
|
||||
expect(screen.getByText('second old').closest('div')).toHaveClass('bg-status-error-subtle');
|
||||
expect(screen.getByText('second n').closest('div')).toHaveClass('bg-status-success-subtle');
|
||||
expect(diff).toBeInTheDocument();
|
||||
});
|
||||
|
||||
|
|
@ -284,8 +286,8 @@ describe('FileAuthoringCall', () => {
|
|||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByText('first old').closest('div')).toHaveClass('bg-red-500/10');
|
||||
expect(screen.getByText('second new').closest('div')).toHaveClass('bg-green-500/10');
|
||||
expect(screen.getByText('first old').closest('div')).toHaveClass('bg-status-error-subtle');
|
||||
expect(screen.getByText('second new').closest('div')).toHaveClass('bg-status-success-subtle');
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,75 @@
|
|||
import React from 'react';
|
||||
import { render, screen } from '@testing-library/react';
|
||||
import Summary from '../Summary';
|
||||
|
||||
jest.mock('~/hooks', () => ({
|
||||
useLocalize: () => (key: string) => key,
|
||||
}));
|
||||
|
||||
jest.mock('jotai', () => ({
|
||||
useAtomValue: () => 'text-sm',
|
||||
}));
|
||||
|
||||
jest.mock('~/store/fontSize', () => ({
|
||||
fontSizeAtom: {},
|
||||
}));
|
||||
|
||||
const mockMessageContext = { current: { isSubmitting: false, isLatestMessage: false } };
|
||||
|
||||
jest.mock('~/Providers', () => ({
|
||||
useMessageContext: () => mockMessageContext.current,
|
||||
}));
|
||||
|
||||
jest.mock('@librechat/client', () => ({
|
||||
Button: ({ children, ...props }: React.ComponentProps<'button'>) => (
|
||||
<button type="button" {...props}>
|
||||
{children}
|
||||
</button>
|
||||
),
|
||||
Clipboard: () => <span />,
|
||||
CheckMark: () => <span />,
|
||||
TooltipAnchor: ({ children }: { children?: React.ReactNode }) => <>{children}</>,
|
||||
}));
|
||||
|
||||
jest.mock('lucide-react', () => ({
|
||||
ScrollText: () => <span />,
|
||||
ChevronDown: () => <span />,
|
||||
ChevronUp: () => <span />,
|
||||
}));
|
||||
|
||||
const partialSummary = [{ type: 'text', text: 'Half a summary before the error' }] as never;
|
||||
|
||||
describe('Summary', () => {
|
||||
beforeEach(() => {
|
||||
mockMessageContext.current = { isSubmitting: false, isLatestMessage: false };
|
||||
});
|
||||
|
||||
it('does not present a failed round as a completed summary', () => {
|
||||
/** A failed summarize round keeps the deltas that already streamed in, so
|
||||
* the only thing separating it from a success is this flag. */
|
||||
render(<Summary content={partialSummary} summarizing={false} failed />);
|
||||
|
||||
expect(screen.getByText('com_ui_summarize_failed')).toBeInTheDocument();
|
||||
expect(screen.queryByText('com_ui_conversation_summarized')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('labels a settled round with partial text as summarized when it did not fail', () => {
|
||||
render(<Summary content={partialSummary} summarizing={false} />);
|
||||
|
||||
expect(screen.getByText('com_ui_conversation_summarized')).toBeInTheDocument();
|
||||
expect(screen.queryByText('com_ui_summarize_failed')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('keeps the in-flight label while the round is still streaming', () => {
|
||||
mockMessageContext.current = { isSubmitting: true, isLatestMessage: true };
|
||||
render(<Summary content={partialSummary} summarizing />);
|
||||
|
||||
expect(screen.getByText('com_ui_summarizing')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('renders nothing when a failed round produced no text at all', () => {
|
||||
const { container } = render(<Summary content={[]} summarizing={false} failed />);
|
||||
|
||||
expect(container).toBeEmptyDOMElement();
|
||||
});
|
||||
});
|
||||
|
|
@ -8,6 +8,7 @@ import type {
|
|||
TMessageContentParts,
|
||||
Agents,
|
||||
FunctionToolCall,
|
||||
PartMetadata,
|
||||
} from 'librechat-data-provider';
|
||||
import type { PartWithIndex } from './ParallelContent';
|
||||
import {
|
||||
|
|
@ -20,6 +21,7 @@ import {
|
|||
import { useLocalize, useExpandCollapse, scheduleMessageContentLayoutReconcile } from '~/hooks';
|
||||
import { ToolAuthWarning, ToolAuthWarningContext } from './auth';
|
||||
import { useMCPIconMap, useMCPServerNames } from '~/hooks/MCP';
|
||||
import { resolveToolCallPhase } from '~/utils/toolCallPhase';
|
||||
import { AttachmentGroup, ReasoningCompact } from './Parts';
|
||||
import { isError, StackedToolIcons } from './ToolOutput';
|
||||
import { isBashProgrammaticToolCall } from './routing';
|
||||
|
|
@ -39,6 +41,34 @@ function hasFailedOutput(output: unknown): boolean {
|
|||
return typeof output === 'string' && isError(output);
|
||||
}
|
||||
|
||||
/**
|
||||
* Group metadata must agree with the individual card, which resolves its
|
||||
* outcome from the run step's terminal verdict through `resolveToolCallPhase`.
|
||||
* Reading output alone leaves a step the run closed as `failed` with empty or
|
||||
* benign output counted as neither failed nor finished, so the group sits on
|
||||
* "Running" until the response ends and then reports that it ran successfully
|
||||
* with no failures while the card beside it shows the failure. A closed step
|
||||
* is never running. Parts carrying no terminal status keep the legacy
|
||||
* output/progress signal, which is all the older endpoints emit.
|
||||
*/
|
||||
function resolveOutcome(
|
||||
runStepStatus: PartMetadata['runStepStatus'],
|
||||
completed: boolean,
|
||||
hasError: boolean,
|
||||
): Pick<ToolMeta, 'hasOutput' | 'failed'> {
|
||||
if (runStepStatus == null) {
|
||||
return { hasOutput: completed, failed: hasError };
|
||||
}
|
||||
const phase = resolveToolCallPhase({
|
||||
runStepStatus,
|
||||
displayProgress: 1,
|
||||
reportedProgress: 1,
|
||||
isSubmitting: false,
|
||||
hasError,
|
||||
});
|
||||
return { hasOutput: phase !== 'running', failed: phase === 'failed' };
|
||||
}
|
||||
|
||||
function hasPendingAuth(part: TMessageContentParts): boolean {
|
||||
if (part.type !== ContentTypes.TOOL_CALL) {
|
||||
return false;
|
||||
|
|
@ -63,6 +93,9 @@ function getToolMeta(part: TMessageContentParts): ToolMeta | null {
|
|||
if (!toolCall) {
|
||||
return null;
|
||||
}
|
||||
/** Terminal verdict lives on the outer `tool_call` object (`PartMetadata`),
|
||||
* not on the per-variant payload, so it is read before any narrowing cast. */
|
||||
const runStepStatus = toolCall.runStepStatus;
|
||||
|
||||
const isStandard =
|
||||
'args' in toolCall && (!toolCall.type || toolCall.type === ToolCallTypes.TOOL_CALL);
|
||||
|
|
@ -75,7 +108,11 @@ function getToolMeta(part: TMessageContentParts): ToolMeta | null {
|
|||
const completed = !!tc.output || tc.progress === 1;
|
||||
const name = tc.name ?? '';
|
||||
const iconName = isBashProgrammaticToolCall(name, tc.args) ? Tools.bash_tool : name;
|
||||
return { name, iconName, hasOutput: completed, failed: hasFailedOutput(tc.output) };
|
||||
return {
|
||||
name,
|
||||
iconName,
|
||||
...resolveOutcome(runStepStatus, completed, hasFailedOutput(tc.output)),
|
||||
};
|
||||
}
|
||||
|
||||
if (toolCall.type === ToolCallTypes.CODE_INTERPRETER) {
|
||||
|
|
@ -83,8 +120,7 @@ function getToolMeta(part: TMessageContentParts): ToolMeta | null {
|
|||
return {
|
||||
name: 'code_interpreter',
|
||||
iconName: 'code_interpreter',
|
||||
hasOutput: (ci?.outputs?.length ?? 0) > 0,
|
||||
failed: false,
|
||||
...resolveOutcome(runStepStatus, (ci?.outputs?.length ?? 0) > 0, false),
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -93,8 +129,7 @@ function getToolMeta(part: TMessageContentParts): ToolMeta | null {
|
|||
return {
|
||||
name: 'file_search',
|
||||
iconName: 'file_search',
|
||||
hasOutput: !!output,
|
||||
failed: hasFailedOutput(output),
|
||||
...resolveOutcome(runStepStatus, !!output, hasFailedOutput(output)),
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -103,8 +138,7 @@ function getToolMeta(part: TMessageContentParts): ToolMeta | null {
|
|||
return {
|
||||
name: fn.name,
|
||||
iconName: fn.name,
|
||||
hasOutput: !!fn.output,
|
||||
failed: hasFailedOutput(fn.output),
|
||||
...resolveOutcome(runStepStatus, !!fn.output, hasFailedOutput(fn.output)),
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -540,9 +574,12 @@ export default function ToolCallGroup({
|
|||
const streaming = isSubmitting && idx === lastContentIdx;
|
||||
const isAfterTool =
|
||||
partIndex > 0 && parts[partIndex - 1]?.part.type === ContentTypes.TOOL_CALL;
|
||||
const label = streaming
|
||||
? localize('com_ui_thinking')
|
||||
: localize('com_ui_thoughts');
|
||||
/** Mirrors the standalone `Reasoning` path: the authored
|
||||
* label wins, generic text is only a fallback. */
|
||||
const generatedLabel = part.reasoning_label?.trim();
|
||||
const label =
|
||||
generatedLabel ||
|
||||
(streaming ? localize('com_ui_thinking') : localize('com_ui_thoughts'));
|
||||
return (
|
||||
<ReasoningCompact
|
||||
key={`reasoning-${idx}`}
|
||||
|
|
|
|||
|
|
@ -821,6 +821,36 @@ describe('ToolCallGroup image hoisting', () => {
|
|||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('honors a terminal failed run step whose output reads as benign', () => {
|
||||
const closedFailure = {
|
||||
type: ContentTypes.TOOL_CALL,
|
||||
[ContentTypes.TOOL_CALL]: {
|
||||
id: 'c2',
|
||||
name: 'create_file',
|
||||
args: '{}',
|
||||
output: '',
|
||||
runStepStatus: 'failed',
|
||||
},
|
||||
} as unknown as TMessageContentParts;
|
||||
|
||||
renderGroup({
|
||||
...baseProps,
|
||||
isSubmitting: true,
|
||||
parts: [
|
||||
{ part: makePart('c1', 'created', 'create_file'), idx: 0 },
|
||||
{ part: closedFailure, idx: 1 },
|
||||
],
|
||||
lastContentIdx: 1,
|
||||
});
|
||||
|
||||
/** The run closed this step as failed, so the group must read it as
|
||||
* 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' }),
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('labels a homogeneous ask_user_question group as its own category', () => {
|
||||
renderGroup({
|
||||
...baseProps,
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { Check } from 'lucide-react';
|
||||
import { Button } from '@librechat/client';
|
||||
import { cn } from '~/utils';
|
||||
|
||||
export interface AskOption {
|
||||
|
|
@ -11,6 +12,11 @@ export interface AskOption {
|
|||
* (composer popover and chat card), so the same question looks and behaves
|
||||
* identically wherever it renders. Numbered badges double as the popover's
|
||||
* digit shortcuts; multi-select swaps the number for a check.
|
||||
*
|
||||
* Hover, focus-visible and disabled appearance come from the shared `Button`
|
||||
* primitive rather than being recreated here, so these rows cannot drift from
|
||||
* it. Only the row geometry and the selected fill stay local, both on
|
||||
* semantic roles so a customized theme moves them too.
|
||||
*/
|
||||
export default function AskOptions({
|
||||
options,
|
||||
|
|
@ -38,22 +44,20 @@ export default function AskOptions({
|
|||
{options.map((option, index) => {
|
||||
const isChecked = multiSelect && checked.includes(index);
|
||||
return (
|
||||
<button
|
||||
<Button
|
||||
key={option.value}
|
||||
ref={(el) => {
|
||||
if (optionRefs) {
|
||||
optionRefs.current[index] = el;
|
||||
}
|
||||
}}
|
||||
type="button"
|
||||
variant="ghost"
|
||||
role={multiSelect ? 'checkbox' : undefined}
|
||||
aria-checked={multiSelect ? isChecked : undefined}
|
||||
disabled={locked}
|
||||
className={cn(
|
||||
'flex w-full items-center gap-2.5 rounded-lg px-2.5 py-2 text-left text-sm text-text-primary',
|
||||
'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-border-heavy',
|
||||
selected === index ? 'bg-surface-active' : 'hover:bg-surface-hover',
|
||||
locked && 'cursor-not-allowed opacity-60',
|
||||
'flex h-auto w-full justify-start gap-2.5 whitespace-normal px-2.5 py-2 text-left text-sm font-normal text-text-primary',
|
||||
selected === index && 'bg-surface-active hover:bg-surface-active',
|
||||
)}
|
||||
onClick={() => onActivate(index)}
|
||||
>
|
||||
|
|
@ -61,14 +65,14 @@ export default function AskOptions({
|
|||
className={cn(
|
||||
'flex size-5 shrink-0 items-center justify-center rounded-md text-[11px] font-medium tabular-nums transition-colors',
|
||||
isChecked
|
||||
? 'bg-surface-submit text-white'
|
||||
? 'bg-surface-submit text-text-on-status'
|
||||
: 'bg-surface-tertiary text-text-secondary',
|
||||
)}
|
||||
>
|
||||
{isChecked ? <Check className="size-3.5" aria-hidden="true" /> : index + 1}
|
||||
</span>
|
||||
<span className="min-w-0 flex-1 [overflow-wrap:anywhere]">{option.label}</span>
|
||||
</button>
|
||||
</Button>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -1467,8 +1467,13 @@ export default function useStepHandler({
|
|||
* content-position == step-index invariant that `updateContent`
|
||||
* writes rely on. Flipping the flag alone hides an empty row
|
||||
* (`Summary` renders null without text) and matches the persisted
|
||||
* message, which retains the part server-side.
|
||||
* message, which retains the part server-side. An errored round
|
||||
* carries `failed` so partial deltas that already streamed in are
|
||||
* not presented as a completed summary.
|
||||
*/
|
||||
if (completeData.error) {
|
||||
return { ...part, summarizing: false, failed: true } as SummaryContentPart;
|
||||
}
|
||||
return { ...part, summarizing: false } as SummaryContentPart;
|
||||
});
|
||||
if (didFinalize) {
|
||||
|
|
|
|||
|
|
@ -2354,21 +2354,16 @@
|
|||
"com_ui_subagent_turn": "Turn",
|
||||
"com_ui_subagent_running": "Running agent",
|
||||
"com_ui_subagent_scroll_to_bottom": "Scroll to latest",
|
||||
"com_ui_subagent_status_done": "Done",
|
||||
"com_ui_subagent_status_failed": "Failed",
|
||||
"com_ui_subagent_status_running": "Running",
|
||||
"com_ui_subagent_status_stopped": "Stopped",
|
||||
"com_ui_subagent_thought_count": "{{0}} thoughts",
|
||||
"com_ui_subagent_ticker_error": "Error",
|
||||
"com_ui_subagent_ticker_reasoning": "Reasoning",
|
||||
"com_ui_subagent_ticker_tool_done": "done",
|
||||
"com_ui_subagent_ticker_using": "Using",
|
||||
"com_ui_subagent_ticker_writing": "Writing",
|
||||
"com_ui_subagent_tool_count": "{{0}} tools",
|
||||
"com_ui_subagent_waiting": "Working…",
|
||||
"com_ui_submit": "Submit",
|
||||
"com_ui_submit_decisions": "Submit {{0}} decisions",
|
||||
"com_ui_submitting": "Submitting…",
|
||||
"com_ui_summarize_failed": "Summarization failed",
|
||||
"com_ui_summarized": "Summarized",
|
||||
"com_ui_summarizing": "Summarizing...",
|
||||
"com_ui_support_contact": "Support Contact",
|
||||
|
|
@ -2511,7 +2506,6 @@
|
|||
"com_ui_use_micrphone": "Use microphone",
|
||||
"com_ui_use_prompt": "Use Prompt",
|
||||
"com_ui_used": "Used",
|
||||
"com_ui_used_n_tools": "Used {{0}} tools",
|
||||
"com_ui_used_one_tool": "Used 1 tool",
|
||||
"com_ui_user": "User",
|
||||
"com_ui_user_group_permissions": "User & Group Permissions",
|
||||
|
|
|
|||
|
|
@ -714,6 +714,10 @@ export type SummaryContentPart = {
|
|||
content?: Array<{ type: ContentTypes.TEXT; text: string }>;
|
||||
tokenCount?: number;
|
||||
summarizing?: boolean;
|
||||
/** A summarize round that ended in error. Partial deltas already streamed
|
||||
* into this slot are kept, so the renderer needs this to avoid presenting
|
||||
* truncated text under the "Conversation summarized" label. */
|
||||
failed?: boolean;
|
||||
summaryVersion?: number;
|
||||
model?: string;
|
||||
provider?: string;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue