mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-27 12:13:30 +00:00
fix(client): address Codex review findings
This commit is contained in:
Marco Beretta
parent
e01eac1892
commit
8abb379cdc
15 changed files with 250 additions and 106 deletions
2
.github/workflows/codegraph-select.yml
vendored
2
.github/workflows/codegraph-select.yml
vendored
|
|
@ -59,7 +59,7 @@ jobs:
|
|||
RESP=$(curl -sS --fail-with-body -m 45 -H "Authorization: Bearer $TOKEN" \
|
||||
-H 'content-type: application/json' --data-binary @body.json "$URL/v1/select"); RC=$?
|
||||
if [ "$RC" -ne 0 ] || [ -z "$RESP" ] || ! echo "$RESP" | jq -e .selected >/dev/null 2>&1; then
|
||||
note "_codegraph unavailable (curl exit $RC: ${RESP:0:120}); skipped — full CI runs as always_"
|
||||
note "_codegraph unavailable (curl exit $RC: ${RESP:0:120}); skipped, full CI runs as always_"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
|
|
|
|||
|
|
@ -2,8 +2,8 @@ import { useContext, useMemo, useState } from 'react';
|
|||
import { Button, TextareaAutosize, TooltipAnchor } from '@librechat/client';
|
||||
import { ChevronUp, MessageCircleQuestion, TriangleAlert } from 'lucide-react';
|
||||
import type { Agents } from 'librechat-data-provider';
|
||||
import { splitOtherOption, ASK_USER_DECLINED_ANSWER } from '~/utils/approval';
|
||||
import { useApprovalContext, useAskSubmitStatus, useResumeSubmit } from './ApprovalContext';
|
||||
import { splitOtherOption, ASK_USER_DECLINED_ANSWER } from '~/utils/approval';
|
||||
import useAskAnswerMode from '~/hooks/Input/useAskAnswerMode';
|
||||
import AskOptions from '~/components/Chat/ask/options';
|
||||
import { ChatContext } from '~/Providers/ChatContext';
|
||||
|
|
@ -127,7 +127,7 @@ function AskUserQuestionSingle({
|
|||
};
|
||||
|
||||
/** `answerMode.skip()` is gated on answer mode being ACTIVE, which a
|
||||
* question moved to the chat is not — and that is precisely when this card
|
||||
* question moved to the chat is not. That is precisely when this card
|
||||
* is the only surface left. Decline through the answer path instead,
|
||||
* which is gated on the live pause rather than on answer mode. */
|
||||
const handleSkip = () => {
|
||||
|
|
@ -161,7 +161,7 @@ function AskUserQuestionSingle({
|
|||
* The live card shares its view-transition-name with the popover panel, so
|
||||
* collapse/expand morphs one surface into the other. The placeholder copy
|
||||
* is `visibility: hidden` (out of the tab order and the a11y tree) and
|
||||
* carries NO transition name — duplicate names would void the morph — but
|
||||
* carries NO transition name because duplicate names would void the morph, but
|
||||
* it still occupies the card's exact footprint, so the thread reserves the
|
||||
* space while the question lives in the composer and nothing reflows when
|
||||
* it moves back.
|
||||
|
|
|
|||
|
|
@ -307,6 +307,8 @@ const Part = memo(function Part({
|
|||
output={toolCall.output ?? ''}
|
||||
initialProgress={toolCall.progress ?? 0.1}
|
||||
isSubmitting={isSubmitting}
|
||||
runStepStatus={toolCall.runStepStatus}
|
||||
runStepDurationMs={toolCall.runStepDurationMs}
|
||||
attachments={attachments}
|
||||
hideAttachments={hideAttachments}
|
||||
onExpand={onToolExpand}
|
||||
|
|
|
|||
|
|
@ -19,7 +19,7 @@ export interface ParsedDiff {
|
|||
/**
|
||||
* Parses unified-diff text into typed lines with old/new line numbers when the
|
||||
* hunk headers carry them. Also understands the argless `@@` separators and
|
||||
* `--- old_text` / `+++ new_text` markers the streaming args preview emits —
|
||||
* `--- old_text` / `+++ new_text` markers the streaming args preview emits.
|
||||
* file headers and `\ No newline` markers are dropped (the window header
|
||||
* already names the file).
|
||||
*/
|
||||
|
|
@ -30,13 +30,18 @@ export function parseUnifiedDiff(diff: string): ParsedDiff {
|
|||
let hasLineNumbers = false;
|
||||
let oldLine: number | undefined;
|
||||
let newLine: number | undefined;
|
||||
let inHunk = false;
|
||||
|
||||
for (const raw of diff.replace(/\n$/, '').split('\n')) {
|
||||
if (raw.startsWith('--- ') || raw.startsWith('+++ ') || raw.startsWith('\\')) {
|
||||
if (!inHunk && (raw.startsWith('--- ') || raw.startsWith('+++ '))) {
|
||||
continue;
|
||||
}
|
||||
if (raw.startsWith('\\')) {
|
||||
continue;
|
||||
}
|
||||
const hunk = HUNK_HEADER.exec(raw);
|
||||
if (hunk) {
|
||||
inHunk = true;
|
||||
oldLine = Number(hunk[1]);
|
||||
newLine = Number(hunk[2]);
|
||||
hasLineNumbers = true;
|
||||
|
|
@ -44,6 +49,7 @@ export function parseUnifiedDiff(diff: string): ParsedDiff {
|
|||
continue;
|
||||
}
|
||||
if (raw === '@@') {
|
||||
inHunk = true;
|
||||
oldLine = undefined;
|
||||
newLine = undefined;
|
||||
lines.push({ type: 'hunk', text: '' });
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import { useMemo } from 'react';
|
||||
import { Brain } from 'lucide-react';
|
||||
import type { TAttachment } from 'librechat-data-provider';
|
||||
import { Tools } from 'librechat-data-provider';
|
||||
import type { PartMetadata, TAttachment } from 'librechat-data-provider';
|
||||
import ProgressText from '~/components/Chat/Messages/Content/ProgressText';
|
||||
import { toolPanelSpacingClassName } from '../disclosure';
|
||||
import useToolCallState from './useToolCallState';
|
||||
|
|
@ -11,9 +12,21 @@ import { cn } from '~/utils';
|
|||
|
||||
type MemoryToolName = 'set_memory' | 'delete_memory';
|
||||
|
||||
export function isMemoryFailureOutput(toolName: MemoryToolName, output: string): boolean {
|
||||
const trimmed = output.trim();
|
||||
if (!trimmed) {
|
||||
return false;
|
||||
}
|
||||
return toolName === 'set_memory'
|
||||
? !/^(?:Memory set for key|Memory saved\b)/i.test(trimmed)
|
||||
: !/^Memory deleted(?: for key|\.)/i.test(trimmed);
|
||||
}
|
||||
|
||||
export default function MemoryCall({
|
||||
toolName,
|
||||
isSubmitting,
|
||||
runStepStatus,
|
||||
runStepDurationMs,
|
||||
initialProgress = 0.1,
|
||||
args,
|
||||
output = '',
|
||||
|
|
@ -24,6 +37,8 @@ export default function MemoryCall({
|
|||
toolName: MemoryToolName;
|
||||
initialProgress: number;
|
||||
isSubmitting: boolean;
|
||||
runStepStatus?: PartMetadata['runStepStatus'];
|
||||
runStepDurationMs?: PartMetadata['runStepDurationMs'];
|
||||
args?: string | Record<string, unknown>;
|
||||
output?: string;
|
||||
attachments?: TAttachment[];
|
||||
|
|
@ -35,64 +50,83 @@ export default function MemoryCall({
|
|||
const memoryKey = useMemo(() => parseJsonField(args, 'key'), [args]);
|
||||
const memoryValue = useMemo(() => parseJsonField(args, 'value'), [args]);
|
||||
const hasPanel = !!memoryKey || !!memoryValue;
|
||||
const memoryFailed = useMemo(
|
||||
() =>
|
||||
isMemoryFailureOutput(toolName, output) ||
|
||||
(attachments?.some((attachment) => attachment?.[Tools.memory]?.type === 'error') ?? false),
|
||||
[attachments, toolName, output],
|
||||
);
|
||||
|
||||
const { showCode, toggleCode, expandStyle, expandRef, progress, cancelled, hasError } =
|
||||
useToolCallState(initialProgress, isSubmitting, output, hasPanel, onExpand);
|
||||
const { showCode, toggleCode, expandStyle, expandRef, phase } = useToolCallState({
|
||||
initialProgress,
|
||||
isSubmitting,
|
||||
output,
|
||||
hasInput: hasPanel || memoryFailed || runStepStatus === 'failed',
|
||||
onExpand,
|
||||
runStepStatus,
|
||||
extraError: memoryFailed,
|
||||
});
|
||||
let finishedText = localize(isSave ? 'com_ui_memory_saved' : 'com_ui_memory_removed');
|
||||
if (phase === 'cancelled') {
|
||||
finishedText = localize('com_ui_cancelled');
|
||||
} else if (phase === 'failed') {
|
||||
finishedText = localize('com_ui_memory');
|
||||
}
|
||||
|
||||
return (
|
||||
<>
|
||||
<div className="relative my-1 flex h-5 shrink-0 items-center gap-2.5">
|
||||
<ProgressText
|
||||
progress={progress}
|
||||
phase={phase}
|
||||
onClick={toggleCode}
|
||||
inProgressText={localize(isSave ? 'com_ui_memory_saving' : 'com_ui_memory_deleting')}
|
||||
finishedText={
|
||||
cancelled
|
||||
? localize('com_ui_cancelled')
|
||||
: localize(isSave ? 'com_ui_memory_saved' : 'com_ui_memory_removed')
|
||||
}
|
||||
finishedText={finishedText}
|
||||
durationMs={runStepDurationMs}
|
||||
subtitle={memoryKey || undefined}
|
||||
errorSuffix={hasError && !cancelled ? localize('com_ui_tool_failed') : undefined}
|
||||
icon={
|
||||
<Brain
|
||||
className={cn(
|
||||
'size-4 shrink-0 text-text-secondary',
|
||||
progress < 1 && !cancelled && !hasError && 'animate-pulse',
|
||||
phase === 'running' && 'animate-pulse',
|
||||
)}
|
||||
aria-hidden="true"
|
||||
/>
|
||||
}
|
||||
hasInput={hasPanel || hasError}
|
||||
hasInput={hasPanel || phase === 'failed'}
|
||||
isExpanded={showCode}
|
||||
error={cancelled}
|
||||
/>
|
||||
</div>
|
||||
<div style={expandStyle}>
|
||||
<div className="overflow-hidden" ref={expandRef}>
|
||||
{(hasPanel || hasError) && (
|
||||
{(hasPanel || phase === 'failed') && (
|
||||
<div
|
||||
className={cn(
|
||||
toolPanelSpacingClassName,
|
||||
'overflow-hidden rounded-lg border border-border-light bg-surface-secondary p-3',
|
||||
)}
|
||||
>
|
||||
{memoryKey && (
|
||||
<div className="mb-1 text-xs font-bold uppercase tracking-wide text-text-secondary">
|
||||
{memoryKey}
|
||||
</div>
|
||||
)}
|
||||
{isSave && memoryValue && (
|
||||
<div className="whitespace-pre-wrap text-sm text-text-primary">{memoryValue}</div>
|
||||
)}
|
||||
{!isSave && (
|
||||
<div className="text-sm italic text-text-secondary">
|
||||
{localize('com_ui_memory_deleted')}
|
||||
</div>
|
||||
)}
|
||||
{hasError && (
|
||||
<pre className="mt-2 whitespace-pre-wrap break-words font-mono text-xs text-red-600 dark:text-red-400">
|
||||
{phase === 'failed' ? (
|
||||
<pre className="whitespace-pre-wrap break-words font-mono text-xs text-status-error">
|
||||
{output}
|
||||
</pre>
|
||||
) : (
|
||||
<>
|
||||
{memoryKey && (
|
||||
<div className="mb-1 text-xs font-bold uppercase tracking-wide text-text-secondary">
|
||||
{memoryKey}
|
||||
</div>
|
||||
)}
|
||||
{isSave && memoryValue && (
|
||||
<div className="whitespace-pre-wrap text-sm text-text-primary">
|
||||
{memoryValue}
|
||||
</div>
|
||||
)}
|
||||
{!isSave && (
|
||||
<div className="text-sm italic text-text-secondary">
|
||||
{localize('com_ui_memory_deleted')}
|
||||
</div>
|
||||
)}
|
||||
</>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
|
|
|
|||
|
|
@ -5,9 +5,9 @@ import { ContentTypes } from 'librechat-data-provider';
|
|||
import type { MouseEvent, FocusEvent } from 'react';
|
||||
import { ThinkingContent, ThinkingButton, FloatingThinkingBar, useInViewport } from './Thinking';
|
||||
import { disclosureChevronClassName } from '~/components/Chat/Messages/Content/disclosure';
|
||||
import CopyButton from '~/components/Messages/Content/CopyButton';
|
||||
import { useLocalize, useExpandCollapse, useLazyCollapseBody } from '~/hooks';
|
||||
import useSmoothStreaming from '~/hooks/Messages/useSmoothStreaming';
|
||||
import CopyButton from '~/components/Messages/Content/CopyButton';
|
||||
import { showThinkingAtom } from '~/store/showThinking';
|
||||
import { fontSizeAtom } from '~/store/fontSize';
|
||||
import { useMessageContext } from '~/Providers';
|
||||
|
|
@ -21,7 +21,7 @@ const stripThinkTags = (reasoning: string): string =>
|
|||
|
||||
const PEEK_SENTENCES = 4;
|
||||
|
||||
/** Tail of streaming reasoning — the last few sentences — for the collapsed
|
||||
/** Tail of streaming reasoning, specifically the last few sentences, for the collapsed
|
||||
* live peek. Bounds work on long reasoning by scanning only the trailing
|
||||
* slice before splitting on sentence boundaries. */
|
||||
const lastSentences = (text: string): string => {
|
||||
|
|
@ -41,7 +41,7 @@ const PEEK_FADE =
|
|||
|
||||
/**
|
||||
* Collapsed live preview of streaming reasoning. Mirrors the expanded thought
|
||||
* panel — same rounded outline and text treatment — but with a border instead
|
||||
* panel. It uses the same rounded outline and text treatment, but with a border instead
|
||||
* of a surface fill, showing the trailing few sentences in a short,
|
||||
* bottom-pinned window whose top and bottom edges fade out, so the newest
|
||||
* thought stays in view while older lines scroll up and dissolve (the "thinking
|
||||
|
|
@ -256,7 +256,7 @@ type ReasoningCompactProps = {
|
|||
* Compact reasoning row for use INSIDE a ToolCallGroup. Keeps the tool-row
|
||||
* header rhythm (icon + label + chevron) so an interleaved thought reads as a
|
||||
* sibling of the surrounding tool calls, while retaining the standalone
|
||||
* {@link Reasoning} affordances — a hover-revealed copy button on the header and
|
||||
* {@link Reasoning} affordances: a hover-revealed copy button on the header and
|
||||
* a floating collapse + copy bar inside the rounded content panel.
|
||||
*/
|
||||
export const ReasoningCompact = memo(
|
||||
|
|
|
|||
|
|
@ -0,0 +1,42 @@
|
|||
import { parseUnifiedDiff } from '../DiffView';
|
||||
|
||||
describe('parseUnifiedDiff', () => {
|
||||
it('drops file headers but keeps changed lines that begin with header markers', () => {
|
||||
const parsed = parseUnifiedDiff(
|
||||
[
|
||||
'--- a/example.txt',
|
||||
'+++ b/example.txt',
|
||||
'@@ -4,2 +4,2 @@',
|
||||
'--- deleted text',
|
||||
'+++ added text',
|
||||
' unchanged',
|
||||
].join('\n'),
|
||||
);
|
||||
|
||||
expect(parsed).toEqual({
|
||||
additions: 1,
|
||||
deletions: 1,
|
||||
hasLineNumbers: true,
|
||||
lines: [
|
||||
{ type: 'hunk', text: '@@ -4,2 +4,2 @@' },
|
||||
{ type: 'del', text: '-- deleted text', oldLine: 4 },
|
||||
{ type: 'add', text: '++ added text', newLine: 4 },
|
||||
{ type: 'context', text: 'unchanged', oldLine: 5, newLine: 5 },
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it('keeps the same changed lines after an argless streaming hunk marker', () => {
|
||||
const parsed = parseUnifiedDiff(
|
||||
['--- old_text', '+++ new_text', '@@', '--- deleted text', '+++ added text'].join('\n'),
|
||||
);
|
||||
|
||||
expect(parsed.lines).toEqual([
|
||||
{ type: 'hunk', text: '' },
|
||||
{ type: 'del', text: '-- deleted text', oldLine: undefined },
|
||||
{ type: 'add', text: '++ added text', newLine: undefined },
|
||||
]);
|
||||
expect(parsed.additions).toBe(1);
|
||||
expect(parsed.deletions).toBe(1);
|
||||
});
|
||||
});
|
||||
|
|
@ -12,6 +12,7 @@ jest.mock('~/hooks', () => ({
|
|||
com_ui_memory_deleting: 'Deleting memory',
|
||||
com_ui_memory_removed: 'Deleted memory',
|
||||
com_ui_memory_deleted: 'Memory deleted',
|
||||
com_ui_memory: 'Memory',
|
||||
com_ui_cancelled: 'Cancelled',
|
||||
com_ui_tool_failed: 'failed',
|
||||
};
|
||||
|
|
@ -26,24 +27,22 @@ jest.mock('~/utils', () => ({
|
|||
jest.mock('~/components/Chat/Messages/Content/ProgressText', () => ({
|
||||
__esModule: true,
|
||||
default: ({
|
||||
progress,
|
||||
phase,
|
||||
inProgressText,
|
||||
finishedText,
|
||||
subtitle,
|
||||
errorSuffix,
|
||||
hasInput,
|
||||
}: {
|
||||
progress: number;
|
||||
phase: 'running' | 'completed' | 'cancelled' | 'failed';
|
||||
inProgressText: string;
|
||||
finishedText: string;
|
||||
subtitle?: string;
|
||||
errorSuffix?: string;
|
||||
hasInput?: boolean;
|
||||
}) => (
|
||||
<div data-testid="progress-text" data-has-input={String(!!hasInput)}>
|
||||
{progress < 1 ? inProgressText : finishedText}
|
||||
{phase === 'running' ? inProgressText : finishedText}
|
||||
{subtitle ? ` ${subtitle}` : ''}
|
||||
{errorSuffix ? ` ${errorSuffix}` : ''}
|
||||
{phase === 'failed' ? ' failed' : ''}
|
||||
</div>
|
||||
),
|
||||
}));
|
||||
|
|
@ -54,17 +53,30 @@ jest.mock('../Attachment', () => ({
|
|||
|
||||
jest.mock('../useToolCallState', () => ({
|
||||
__esModule: true,
|
||||
default: (initialProgress: number, _isSubmitting: boolean, output: string) => ({
|
||||
showCode: true,
|
||||
toggleCode: jest.fn(),
|
||||
expandStyle: {},
|
||||
expandRef: { current: null },
|
||||
progress: initialProgress,
|
||||
cancelled: false,
|
||||
hasError: output.startsWith('Error'),
|
||||
hasOutput: output.length > 0,
|
||||
hasContent: true,
|
||||
}),
|
||||
default: ({
|
||||
initialProgress,
|
||||
runStepStatus,
|
||||
extraError,
|
||||
}: {
|
||||
initialProgress: number;
|
||||
runStepStatus?: 'completed' | 'cancelled' | 'failed';
|
||||
extraError?: boolean;
|
||||
}) => {
|
||||
let phase: 'running' | 'completed' | 'cancelled' | 'failed' =
|
||||
initialProgress < 1 ? 'running' : 'completed';
|
||||
if (runStepStatus === 'cancelled') {
|
||||
phase = 'cancelled';
|
||||
} else if (runStepStatus === 'failed' || extraError) {
|
||||
phase = 'failed';
|
||||
}
|
||||
return {
|
||||
showCode: true,
|
||||
toggleCode: jest.fn(),
|
||||
expandStyle: {},
|
||||
expandRef: { current: null },
|
||||
phase,
|
||||
};
|
||||
},
|
||||
}));
|
||||
|
||||
describe('MemoryCall', () => {
|
||||
|
|
@ -75,14 +87,14 @@ describe('MemoryCall', () => {
|
|||
initialProgress={1}
|
||||
isSubmitting={false}
|
||||
args={{ key: 'preferences', value: 'Prefers dark mode.' }}
|
||||
output="Memory saved."
|
||||
output={'Memory set for key "preferences" (4 tokens)'}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByTestId('progress-text')).toHaveTextContent('Saved memory preferences');
|
||||
expect(screen.getByText('preferences')).toHaveClass('font-bold', 'uppercase');
|
||||
expect(screen.getByText('Prefers dark mode.')).toBeInTheDocument();
|
||||
expect(screen.queryByText('Memory saved.')).not.toBeInTheDocument();
|
||||
expect(screen.queryByText(/Memory set for key/)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('labels an in-flight save without a parsed key', () => {
|
||||
|
|
@ -107,7 +119,7 @@ describe('MemoryCall', () => {
|
|||
initialProgress={1}
|
||||
isSubmitting={false}
|
||||
args={{ key: 'outdated_note' }}
|
||||
output="Memory deleted."
|
||||
output={'Memory deleted for key "outdated_note"'}
|
||||
/>,
|
||||
);
|
||||
|
||||
|
|
@ -116,18 +128,43 @@ describe('MemoryCall', () => {
|
|||
expect(screen.getByText('Memory deleted')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('surfaces the error output inside the panel', () => {
|
||||
it.each([
|
||||
['set_memory', 'Invalid key "invalid". Must be one of: preferences'],
|
||||
['set_memory', 'Memory storage would exceed limit. Cannot save this memory.'],
|
||||
['set_memory', 'Failed to set memory for key "preferences"'],
|
||||
['set_memory', '{"type":"content_filter","message":"Blocked"}'],
|
||||
['delete_memory', 'Failed to delete memory for key "preferences"'],
|
||||
] as const)('surfaces %s failure output instead of optimistic content', (toolName, output) => {
|
||||
render(
|
||||
<MemoryCall
|
||||
toolName={toolName}
|
||||
initialProgress={1}
|
||||
isSubmitting={false}
|
||||
args={{ key: 'preferences', value: 'requested value' }}
|
||||
output={output}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByTestId('progress-text')).toHaveTextContent('Memory preferences failed');
|
||||
expect(screen.getByText(output)).toBeInTheDocument();
|
||||
expect(screen.queryByText('requested value')).not.toBeInTheDocument();
|
||||
expect(screen.queryByText('Memory deleted')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('honors an explicit failed run step even when the output resembles success', () => {
|
||||
render(
|
||||
<MemoryCall
|
||||
toolName="set_memory"
|
||||
initialProgress={1}
|
||||
isSubmitting={false}
|
||||
args={{ key: 'preferences', value: 'value' }}
|
||||
output="Error: memory storage full"
|
||||
runStepStatus="failed"
|
||||
args={{ key: 'preferences', value: 'requested value' }}
|
||||
output={'Memory set for key "preferences" (4 tokens)'}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByTestId('progress-text')).toHaveTextContent('failed');
|
||||
expect(screen.getByText('Error: memory storage full')).toBeInTheDocument();
|
||||
expect(screen.getByTestId('progress-text')).toHaveTextContent('Memory preferences failed');
|
||||
expect(screen.getByText(/Memory set for key/)).toBeInTheDocument();
|
||||
expect(screen.queryByText('requested value')).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -442,5 +442,4 @@ describe('SubagentCall', () => {
|
|||
expect(rendered.getSelection()?.durable).toBeUndefined();
|
||||
expect(rendered.getSelection()?.legacyOutput).toBe(output);
|
||||
});
|
||||
|
||||
});
|
||||
|
|
|
|||
|
|
@ -153,7 +153,7 @@ export default function ToolCallGroup({
|
|||
const retainedForPendingApprovalRef = useRef(false);
|
||||
|
||||
/** `parts` may include interleaved reasoning ("Thoughts") parts that render
|
||||
* inside the body but are not actions — count and summarize only the real
|
||||
* inside the body but are not actions. Count and summarize only the real
|
||||
* tool calls so the header and stacked icons stay accurate. */
|
||||
const toolMetadata = useMemo(
|
||||
() => parts.map((p) => getToolMeta(p.part)).filter((m): m is ToolMeta => m != null),
|
||||
|
|
@ -233,7 +233,7 @@ export default function ToolCallGroup({
|
|||
}, [toolMetadata, localize, mcpServerNames]);
|
||||
|
||||
/** Reasoning interleaved with the tool calls renders inside the body but is
|
||||
* hidden while collapsed — note it in the header's accessible label so screen
|
||||
* hidden while collapsed. Note it in the header's accessible label so screen
|
||||
* readers know the group also contains thoughts. */
|
||||
const hasReasoning = useMemo(
|
||||
() => parts.some((p) => p.part.type === ContentTypes.THINK),
|
||||
|
|
@ -255,7 +255,7 @@ export default function ToolCallGroup({
|
|||
|
||||
/** `ask_user_question` calls form their own category, mirroring subagents:
|
||||
* a homogeneous group reads "Asking/Asked N questions" (never "Used N
|
||||
* tools — ask_user_question") with a question glyph. */
|
||||
* tools: ask_user_question") with a question glyph. */
|
||||
const allAskQuestions =
|
||||
activitySummary.askQuestionCount > 0 && activitySummary.askQuestionCount === count;
|
||||
/** Past tense once the turn is settled — matches the Asking/Asked record
|
||||
|
|
@ -264,7 +264,7 @@ export default function ToolCallGroup({
|
|||
const askQuestionsDone = allAskQuestions && (allCompleted || !isSubmitting);
|
||||
|
||||
/** For a single-tool group, lead with the tool's own (capitalized) label
|
||||
* instead of the generic "Used 1 tool — name", which reads awkwardly. */
|
||||
* instead of the generic "Used 1 tool: name", which reads awkwardly. */
|
||||
const singleToolLabel = useMemo(() => {
|
||||
const raw = getToolDisplayLabel(toolMetadata[0]?.name ?? '', localize, mcpServerNames);
|
||||
return raw ? raw.charAt(0).toUpperCase() + raw.slice(1) : '';
|
||||
|
|
|
|||
|
|
@ -95,23 +95,24 @@ describe('collapsing a live ask_user_question', () => {
|
|||
fireEvent.click(screen.getByTestId('collapse-from-popover'));
|
||||
|
||||
expect(screen.getByTestId('popover-visible').textContent).toBe('false');
|
||||
/** The pause is still live — the card has it now... */
|
||||
expect(screen.getByTestId('active').textContent).toBe('true');
|
||||
/** The pause is still live, but the card owns it outside answer mode. */
|
||||
expect(screen.getByTestId('active').textContent).toBe('false');
|
||||
expect(screen.getByText('North star or full funnel?')).toBeInTheDocument();
|
||||
/** ...and the composer is a composer again. */
|
||||
/** The composer is a normal composer again. */
|
||||
expect(screen.getByTestId('composer-locked').textContent).toBe('false');
|
||||
expect(screen.getByTestId('composer-answers').textContent).toBe('false');
|
||||
});
|
||||
|
||||
it('gives the collapsed card the popover’s dismiss', () => {
|
||||
it('moves the collapsed card back to the popover', () => {
|
||||
renderPause();
|
||||
fireEvent.click(screen.getByTestId('collapse-from-popover'));
|
||||
|
||||
expect(screen.getByLabelText('Expand')).toBeInTheDocument();
|
||||
fireEvent.click(screen.getByLabelText('Close'));
|
||||
fireEvent.click(screen.getByLabelText('Expand'));
|
||||
|
||||
/** Dismiss exits answer mode entirely, exactly as the popover’s × did. */
|
||||
expect(screen.getByTestId('active').textContent).toBe('false');
|
||||
expect(screen.getByTestId('active').textContent).toBe('true');
|
||||
expect(screen.getByTestId('popover-visible').textContent).toBe('true');
|
||||
expect(screen.getByTestId('composer-locked').textContent).toBe('true');
|
||||
expect(screen.queryByLabelText('Expand')).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -120,22 +121,22 @@ describe('collapsing a live ask_user_question', () => {
|
|||
mockLiveAsk = { actionId: 'act-1', question: mockSingle, messageId: 'message-1' };
|
||||
});
|
||||
|
||||
it('keeps the composer as the answer box, and still offers a dismiss', () => {
|
||||
it('releases the composer until the card moves back to the popover', () => {
|
||||
renderPause();
|
||||
expect(screen.getByTestId('composer-answers').textContent).toBe('true');
|
||||
expect(screen.getByTestId('composer-locked').textContent).toBe('false');
|
||||
|
||||
fireEvent.click(screen.getByTestId('collapse-from-popover'));
|
||||
|
||||
/** A single question is answered IN the composer, so it stays the answer
|
||||
* box past collapse — the card is only the display handing over. */
|
||||
expect(screen.getByTestId('composer-answers').textContent).toBe('true');
|
||||
/** The card now owns the answer, so the normal composer is available. */
|
||||
expect(screen.getByTestId('composer-answers').textContent).toBe('false');
|
||||
expect(screen.getByText('Which environment?')).toBeInTheDocument();
|
||||
|
||||
fireEvent.click(screen.getByLabelText('Close'));
|
||||
fireEvent.click(screen.getByLabelText('Answer from the message box'));
|
||||
|
||||
expect(screen.getByTestId('active').textContent).toBe('false');
|
||||
expect(screen.getByTestId('composer-answers').textContent).toBe('false');
|
||||
expect(screen.getByTestId('active').textContent).toBe('true');
|
||||
expect(screen.getByTestId('composer-answers').textContent).toBe('true');
|
||||
expect(screen.getByTestId('popover-visible').textContent).toBe('true');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -58,7 +58,7 @@ describe('Error — typed provider errors', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('Error — agent context budget errors', () => {
|
||||
describe('Error: agent context budget errors', () => {
|
||||
beforeAll(() => {
|
||||
/** CodeBlock observes its code bar; jsdom ships no IntersectionObserver. */
|
||||
(global as { IntersectionObserver?: unknown }).IntersectionObserver = class {
|
||||
|
|
|
|||
|
|
@ -61,6 +61,14 @@ const liveAsk = {
|
|||
question: { question: 'Pick one', options: [], multiSelect: false },
|
||||
} as unknown as ReturnType<typeof findLiveAskUserQuestion>;
|
||||
|
||||
const batchAsk = {
|
||||
...liveAsk,
|
||||
questions: [
|
||||
{ id: 'environment', question: 'Which environment?' },
|
||||
{ id: 'window', question: 'Which window?' },
|
||||
],
|
||||
} as typeof liveAsk;
|
||||
|
||||
describe('useAskAnswerMode', () => {
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
|
|
@ -95,15 +103,7 @@ describe('useAskAnswerMode', () => {
|
|||
});
|
||||
|
||||
it('locks the composer for a batch, and hands it back the moment it collapses', () => {
|
||||
mockUseGetMessages.mockReturnValue({
|
||||
data: {
|
||||
...liveAsk,
|
||||
questions: [
|
||||
{ id: 'environment', question: 'Which environment?' },
|
||||
{ id: 'window', question: 'Which window?' },
|
||||
],
|
||||
},
|
||||
});
|
||||
mockUseGetMessages.mockReturnValue({ data: batchAsk });
|
||||
|
||||
const { result } = renderHook(() => useAskAnswerMode('conversation-1'));
|
||||
|
||||
|
|
@ -120,6 +120,27 @@ describe('useAskAnswerMode', () => {
|
|||
expect(mockSubmitAskAnswer).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not move the normal composer draft into a collapsed batch answer', () => {
|
||||
mockUseGetMessages.mockReturnValue({ data: batchAsk });
|
||||
const { result } = renderHook(() => useAskAnswerMode('conversation-1'));
|
||||
|
||||
act(() => result.current.collapse());
|
||||
|
||||
expect(mockSetAnswerDraft).not.toHaveBeenCalled();
|
||||
expect(mockResetComposer).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not overwrite normal composer text when expanding a batch', () => {
|
||||
mockCollapsedIds = ['a1'];
|
||||
mockAnswerDraft = { actionId: 'a1', text: 'stale batch handoff' };
|
||||
mockUseGetMessages.mockReturnValue({ data: batchAsk });
|
||||
const { result } = renderHook(() => useAskAnswerMode('conversation-1'));
|
||||
|
||||
act(() => result.current.expand());
|
||||
|
||||
expect(mockSetComposerText).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('disables the query and forces liveAsk null for a new (unsaved) conversation', () => {
|
||||
mockUseGetMessages.mockReturnValue({ data: liveAsk });
|
||||
|
||||
|
|
|
|||
|
|
@ -18,7 +18,7 @@ import store from '~/store';
|
|||
* Action ids the user moved into the chat: the popover is hidden AND the
|
||||
* composer is released (answer mode off), so the chat card is the question's
|
||||
* only surface until the card's chevron moves it back. One state for one
|
||||
* user-visible concept — a hidden popover whose composer stayed armed was
|
||||
* user-visible concept. A hidden popover whose composer stayed armed was
|
||||
* indistinguishable from one whose composer did not.
|
||||
*/
|
||||
const collapsedAskActionsAtom = atom<string[]>({
|
||||
|
|
@ -136,7 +136,7 @@ export default function useAskAnswerMode(conversationId?: string | null) {
|
|||
const collapsed = answerable && collapsedIds.includes(liveAsk.actionId);
|
||||
/**
|
||||
* Answer mode: the popover is up AND the composer is the free-form answer
|
||||
* box. The two are deliberately the same condition — the composer's answer
|
||||
* box. The two are deliberately the same condition because the composer's answer
|
||||
* role is only discoverable while the popover explains it.
|
||||
*/
|
||||
const active = answerable && !collapsed;
|
||||
|
|
@ -157,7 +157,7 @@ export default function useAskAnswerMode(conversationId?: string | null) {
|
|||
const answerText = answerDraft.actionId === liveAsk?.actionId ? answerDraft.text : '';
|
||||
const setAnswerText = useCallback(
|
||||
(text: string) => {
|
||||
if (liveAsk) {
|
||||
if (liveAsk && !batchMode) {
|
||||
setAnswerDraft({ actionId: liveAsk.actionId, text });
|
||||
/** While the card owns the answer, `useAutoSave` is tracking the
|
||||
* conversation draft instead. Keep the dormant ask draft current so
|
||||
|
|
@ -167,7 +167,7 @@ export default function useAskAnswerMode(conversationId?: string | null) {
|
|||
}
|
||||
}
|
||||
},
|
||||
[liveAsk, saveDrafts, setAnswerDraft],
|
||||
[batchMode, liveAsk, saveDrafts, setAnswerDraft],
|
||||
);
|
||||
|
||||
/** Selection state is per-question: a new pause must never inherit a stale
|
||||
|
|
@ -184,31 +184,33 @@ export default function useAskAnswerMode(conversationId?: string | null) {
|
|||
* which morphTransition's synchronous flush requires. */
|
||||
const collapse = useCallback(() => {
|
||||
if (liveAsk) {
|
||||
const composerAnswer = formContext?.getValues('text') ?? answerText;
|
||||
const composerAnswer = !batchMode ? (formContext?.getValues('text') ?? answerText) : '';
|
||||
morphTransition(() => {
|
||||
setAnswerDraft({ actionId: liveAsk.actionId, text: composerAnswer });
|
||||
if (!saveDrafts) {
|
||||
formContext?.reset();
|
||||
if (!batchMode) {
|
||||
setAnswerDraft({ actionId: liveAsk.actionId, text: composerAnswer });
|
||||
if (!saveDrafts) {
|
||||
formContext?.reset();
|
||||
}
|
||||
}
|
||||
setCollapsedIds((prev) =>
|
||||
prev.includes(liveAsk.actionId) ? prev : [...prev, liveAsk.actionId],
|
||||
);
|
||||
});
|
||||
}
|
||||
}, [liveAsk, formContext, answerText, saveDrafts, setAnswerDraft, setCollapsedIds]);
|
||||
}, [liveAsk, batchMode, formContext, answerText, saveDrafts, setAnswerDraft, setCollapsedIds]);
|
||||
|
||||
const expand = useCallback(() => {
|
||||
if (liveAsk) {
|
||||
morphTransition(() => {
|
||||
/** Autosave restores the ask-specific draft after the key switch. If
|
||||
* drafts are disabled, perform that handoff directly. */
|
||||
if (!saveDrafts) {
|
||||
if (!batchMode && !saveDrafts) {
|
||||
formContext?.setValue('text', answerText);
|
||||
}
|
||||
setCollapsedIds((prev) => prev.filter((id) => id !== liveAsk.actionId));
|
||||
});
|
||||
}
|
||||
}, [liveAsk, saveDrafts, formContext, answerText, setCollapsedIds]);
|
||||
}, [liveAsk, batchMode, saveDrafts, formContext, answerText, setCollapsedIds]);
|
||||
|
||||
/** Pure check toggle: the keyboard highlight is steered only by the
|
||||
* composer's digit/arrow shortcuts, so a mouse toggle never leaves a
|
||||
|
|
|
|||
|
|
@ -779,7 +779,7 @@ function shapeSummarizationConfig(
|
|||
* Below this context budget a summarization cycle cannot make progress: the
|
||||
* summary allocation rounds down to a handful of tokens, the rewritten history
|
||||
* still overflows, and the graph re-triggers summarization on every step until
|
||||
* the recursion limit aborts the run — dozens of wasted LLM calls surfaced to
|
||||
* the recursion limit aborts the run. Dozens of wasted LLM calls surfaced to
|
||||
* the user as an opaque LangGraph error. Falling back to plain pruning instead
|
||||
* either fits the request or fails fast with the actionable `empty_messages`
|
||||
* token-budget breakdown. Matches the floor `initializeAgent` applies when the
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue