mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-09-27 19:22:06 +00:00
🐾 style: Show the Multiplier Only on a Generic Tool Line (#16225)
* Count the tool only while the line is its generic label A streamed intent already names the work this call is doing, so a repeat count beside it reads as a claim that the sentence happened N times. The count now rides the generic Running/Ran label alone, and is dropped for an intent, a failure or cancellation verdict, and a background handle line. * fix: Omit Repeat Counts From Sandbox Startup Labels * test: Bound Client Recovery Worker Activation Waits --------- Co-authored-by: Lia <lia@librechat.ai>
This commit is contained in:
parent
418d62d52d
commit
75e0102478
5 changed files with 229 additions and 52 deletions
|
|
@ -299,15 +299,13 @@ function LivePhaseHeader({
|
|||
const sandboxStarting = useAtomValue(
|
||||
sandboxStartingByToolCallId(activity.pendingToolCallId ?? ''),
|
||||
);
|
||||
const text =
|
||||
sandboxStarting && activity.pendingToolCallId != null
|
||||
? localize('com_ui_sandbox_starting')
|
||||
: activity.text;
|
||||
const showSandboxStartup = sandboxStarting && activity.pendingToolCallId != null;
|
||||
const text = showSandboxStartup ? localize('com_ui_sandbox_starting') : activity.text;
|
||||
/** Startup describes this call, not the repeated tool. Throttle its text
|
||||
* and suppressed count together so neither can paint with the old value. */
|
||||
const comboCount = showSandboxStartup ? 1 : activity.comboCount;
|
||||
const { source } = activity;
|
||||
const line = useMemo(
|
||||
() => ({ text, source, comboCount: activity.comboCount }),
|
||||
[text, source, activity.comboCount],
|
||||
);
|
||||
const line = useMemo(() => ({ text, source, comboCount }), [text, source, comboCount]);
|
||||
const previewRef = useRef<HTMLSpanElement>(null);
|
||||
/** The full width the line may occupy, which is the flex track rather than
|
||||
* the label box: the box shrinks to its text whenever the multiplier rides
|
||||
|
|
|
|||
|
|
@ -1621,7 +1621,9 @@ describe('ContentParts — live activity fold', () => {
|
|||
expect(screen.getByTestId('activity-phase-announcer')).toHaveTextContent(
|
||||
'Reading the lens file',
|
||||
);
|
||||
expect(liveHeader()).toHaveAccessibleName('Querying ×2');
|
||||
/** No multiplier: both calls named their own work, so the line is a
|
||||
* sentence about the newest one rather than the tool's label. */
|
||||
expect(liveHeader()).toHaveAccessibleName('Querying');
|
||||
});
|
||||
|
||||
it('leaves a span of legacy Assistants calls, which the header cannot name, unfolded', () => {
|
||||
|
|
|
|||
|
|
@ -201,9 +201,18 @@ describe('live combo aggregation', () => {
|
|||
for (const intent of ['Checking', 'Checking the', 'Checking the last file']) {
|
||||
parts[parts.length - 1] = toPart({ name: 'lookup', args: { intent } }, 'tail');
|
||||
readFirst.mockClear();
|
||||
expect(activity(parts).comboCount).toBe(1024);
|
||||
/** A tail that names its own work hides the count, but the span pass
|
||||
* still reaches the head, and must do so exactly once per delta. */
|
||||
expect(activity(parts).comboCount).toBe(1);
|
||||
expect(readFirst).toHaveBeenCalledTimes(1);
|
||||
}
|
||||
|
||||
/** The same 1,024 parts under a generic tail: the whole suffix counts, on
|
||||
* the same single prefix read. */
|
||||
parts[parts.length - 1] = toPart({ name: 'lookup', output: 'ok' }, 'tail');
|
||||
readFirst.mockClear();
|
||||
expect(activity(parts).comboCount).toBe(1024);
|
||||
expect(readFirst).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('counts only the suffix, ignoring descriptive metadata and sparse slots', () => {
|
||||
|
|
@ -394,14 +403,10 @@ describe('live fold parity with the cards it hides', () => {
|
|||
|
||||
it('changes the multiplier with the throttled status line', () => {
|
||||
jest.useFakeTimers();
|
||||
const first = toPart(
|
||||
{ name: 'create_file', args: '{"intent":"Creating the first file"}', output: '' },
|
||||
'first',
|
||||
);
|
||||
const second = toPart(
|
||||
{ name: 'create_file', args: '{"intent":"Creating the second file"}', output: '' },
|
||||
'second',
|
||||
);
|
||||
/** Generic lines, because only those carry a count: the multiplier has to
|
||||
* arrive with the line it belongs to, not a paint ahead of it. */
|
||||
const first = toPart({ name: 'create_file', output: 'created' }, 'first');
|
||||
const second = toPart({ name: 'create_file', output: '' }, 'second');
|
||||
const view = mount([first], undefined, true);
|
||||
view.rerender(
|
||||
<QueryClientProvider client={new QueryClient()}>
|
||||
|
|
@ -421,11 +426,59 @@ describe('live fold parity with the cards it hides', () => {
|
|||
);
|
||||
|
||||
const header = within(screen.getByTestId('activity-phase-card')).getAllByRole('button')[0];
|
||||
expect(header).toHaveAccessibleName('Creating the first file');
|
||||
expect(header).toHaveAccessibleName('Ran Create File');
|
||||
act(() => {
|
||||
jest.advanceTimersByTime(500);
|
||||
});
|
||||
expect(header).toHaveAccessibleName('Creating the second file ×2');
|
||||
expect(header).toHaveAccessibleName('Running Create File ×2');
|
||||
});
|
||||
|
||||
it('drops the multiplier as soon as the call names its own work', () => {
|
||||
/** A count modifies the tool's name. Once the line is a sentence about
|
||||
* this call, `×2` reads as a claim about the sentence. */
|
||||
const view = mount(
|
||||
[
|
||||
toPart({ name: 'create_file', output: 'created' }, 'first'),
|
||||
toPart({ name: 'create_file', args: '{"intent":"Creating the second file"}' }, 'second'),
|
||||
],
|
||||
undefined,
|
||||
true,
|
||||
);
|
||||
const header = within(screen.getByTestId('activity-phase-card')).getAllByRole('button')[0];
|
||||
|
||||
expect(header).toHaveAccessibleName('Creating the second file');
|
||||
expect(screen.queryByTestId('live-phase-combo')).toBeNull();
|
||||
|
||||
/** The same pair without the intent still counts, so the suppression is
|
||||
* the line's doing and not a lost count. */
|
||||
view.unmount();
|
||||
mount(
|
||||
[
|
||||
toPart({ name: 'create_file', output: 'created' }, 'first'),
|
||||
toPart({ name: 'create_file', output: '' }, 'second'),
|
||||
],
|
||||
undefined,
|
||||
true,
|
||||
);
|
||||
expect(screen.getByTestId('live-phase-combo')).toHaveTextContent('×2');
|
||||
});
|
||||
|
||||
it('drops the multiplier on a line that reports how the call ended', () => {
|
||||
mount(
|
||||
[
|
||||
toPart({ name: 'lookup', output: 'rows' }, 'first'),
|
||||
toPart({ name: 'lookup', output: 'rows', runStepStatus: 'failed' }, 'second'),
|
||||
],
|
||||
undefined,
|
||||
true,
|
||||
);
|
||||
const header = within(screen.getByTestId('activity-phase-card')).getAllByRole('button')[0];
|
||||
|
||||
/** The span's verdict still counts the failure; the line does not count
|
||||
* the tool, because "Failed lookup ×2" would blame both calls. */
|
||||
expect(header).toHaveAccessibleName(/^Failed: lookup/);
|
||||
expect(screen.queryByTestId('live-phase-combo')).toBeNull();
|
||||
expect(screen.getByTestId('live-phase-outcome')).toHaveTextContent('1 failed');
|
||||
});
|
||||
|
||||
it('resets the multiplier across an agent handoff', () => {
|
||||
|
|
@ -933,7 +986,12 @@ describe('live activity hardening transitions', () => {
|
|||
|
||||
function SandboxEvent() {
|
||||
const setStarting = useSetAtom(sandboxStartingByToolCallId('sandbox-call'));
|
||||
return <button onClick={() => setStarting(true)}>{'Start sandbox'}</button>;
|
||||
return (
|
||||
<>
|
||||
<button onClick={() => setStarting(true)}>{'Start sandbox'}</button>
|
||||
<button onClick={() => setStarting(false)}>{'Clear sandbox startup'}</button>
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
||||
it.each([
|
||||
|
|
@ -948,20 +1006,83 @@ describe('live activity hardening transitions', () => {
|
|||
* reads the same sandbox signal instead, and stays one card throughout. */
|
||||
jest.useFakeTimers();
|
||||
const call = { name, args, output: '' };
|
||||
const view = render(frame([toPart(call, 'sandbox-call')], <SandboxEvent />));
|
||||
const earlier = toPart({ ...call, output: 'ok' }, 'earlier');
|
||||
const view = render(frame([earlier, toPart(call, 'sandbox-call')], <SandboxEvent />));
|
||||
const card = screen.getByTestId('activity-phase-card');
|
||||
const header = within(card).getByRole('button');
|
||||
expect(screen.queryByTestId('tool-call')).toBeNull();
|
||||
expect(header).toHaveAccessibleName(/×2$/);
|
||||
expect(screen.getByTestId('live-phase-combo')).toHaveTextContent('×2');
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Start sandbox' }));
|
||||
expect(header).toHaveAccessibleName(/×2$/);
|
||||
act(() => {
|
||||
jest.advanceTimersByTime(500);
|
||||
});
|
||||
expect(header).toHaveAccessibleName('Starting sandbox environment');
|
||||
expect(screen.queryByTestId('live-phase-combo')).toBeNull();
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Clear sandbox startup' }));
|
||||
expect(header).toHaveAccessibleName('Starting sandbox environment');
|
||||
expect(screen.queryByTestId('live-phase-combo')).toBeNull();
|
||||
act(() => {
|
||||
jest.advanceTimersByTime(500);
|
||||
});
|
||||
expect(header).toHaveAccessibleName(/×2$/);
|
||||
expect(screen.getByTestId('live-phase-combo')).toHaveTextContent('×2');
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Start sandbox' }));
|
||||
act(() => {
|
||||
jest.advanceTimersByTime(500);
|
||||
});
|
||||
expect(card).toHaveTextContent('Starting sandbox');
|
||||
expect(header).toHaveAccessibleName('Starting sandbox environment');
|
||||
/** Output can arrive before the transient startup flag is cleared. */
|
||||
view.rerender(
|
||||
frame([
|
||||
earlier,
|
||||
toPart({ ...call, output: 'ok', runStepStatus: 'completed' }, 'sandbox-call'),
|
||||
]),
|
||||
);
|
||||
act(() => {
|
||||
jest.advanceTimersByTime(500);
|
||||
});
|
||||
expect(screen.getByTestId('activity-phase-card')).toBe(card);
|
||||
expect(header).toHaveAccessibleName(/^Ran .* ×2$/);
|
||||
expect(screen.getByTestId('live-phase-combo')).toHaveTextContent('×2');
|
||||
});
|
||||
|
||||
it('keeps startup and intent labels uncounted, then counts a new generic call', () => {
|
||||
jest.useFakeTimers();
|
||||
const earlier = toPart({ name: Tools.execute_code, output: 'ok' }, 'earlier');
|
||||
const pending = toPart({ name: Tools.execute_code, output: '' }, 'sandbox-call');
|
||||
const view = render(frame([earlier, pending], <SandboxEvent />));
|
||||
const header = within(screen.getByTestId('activity-phase-card')).getByRole('button');
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Start sandbox' }));
|
||||
act(() => {
|
||||
jest.advanceTimersByTime(500);
|
||||
});
|
||||
expect(header).toHaveAccessibleName('Starting sandbox environment');
|
||||
expect(screen.queryByTestId('live-phase-combo')).toBeNull();
|
||||
|
||||
const named = toPart(
|
||||
{ name: Tools.execute_code, args: '{"intent":"Checking the data', output: '' },
|
||||
'sandbox-call',
|
||||
);
|
||||
view.rerender(frame([earlier, named]));
|
||||
act(() => {
|
||||
jest.advanceTimersByTime(500);
|
||||
});
|
||||
expect(header).toHaveAccessibleName('Checking the data');
|
||||
expect(screen.queryByTestId('live-phase-combo')).toBeNull();
|
||||
|
||||
view.rerender(
|
||||
frame([toPart({ ...call, output: 'ok', runStepStatus: 'completed' }, 'sandbox-call')]),
|
||||
frame([earlier, named, toPart({ name: Tools.execute_code, output: '' }, 'next-call')]),
|
||||
);
|
||||
expect(screen.getByTestId('activity-phase-card')).toBe(card);
|
||||
act(() => {
|
||||
jest.advanceTimersByTime(500);
|
||||
});
|
||||
expect(header).toHaveAccessibleName(/^Running .* ×3$/);
|
||||
expect(screen.getByTestId('live-phase-combo')).toHaveTextContent('×3');
|
||||
});
|
||||
|
||||
it('holds one card across a run of code calls whose intent is not the first key', () => {
|
||||
|
|
@ -1084,7 +1205,8 @@ describe('live activity hardening transitions', () => {
|
|||
jest.advanceTimersByTime(500);
|
||||
});
|
||||
const header = screen.getByRole('button');
|
||||
expect(header).toHaveAccessibleName('Checking the next file ×2');
|
||||
expect(header).toHaveAccessibleName('Checking the next file');
|
||||
expect(screen.queryByTestId('live-phase-combo')).toBeNull();
|
||||
expect(header.querySelector('.absolute[aria-hidden="true"]')).toBeNull();
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -34,7 +34,9 @@ export type LiveActivity = {
|
|||
* the content array; the header reads the same signal for this call rather
|
||||
* than unfolding the span to let the card say it. */
|
||||
pendingToolCallId?: string;
|
||||
/** Consecutive uses of the newest tool, including the current call. */
|
||||
/** Consecutive uses of the newest tool, including the current call. Above 1
|
||||
* only while the line is the tool's own generic label, which is the only
|
||||
* thing a count of that tool can modify. */
|
||||
comboCount: number;
|
||||
/** Failed and stopped calls anywhere in the span, not just the newest line. */
|
||||
outcome: SpanOutcome;
|
||||
|
|
@ -84,13 +86,22 @@ export function needsReader(part: TMessageContentParts | undefined): boolean {
|
|||
return Array.isArray(toolCall.subagent_content) && toolCall.subagent_content.some(needsReader);
|
||||
}
|
||||
|
||||
/**
|
||||
* A tool's line, and whether it is the generic label rather than a line about
|
||||
* this one call. `generic` is what a repeat count may modify: `Running Code ×3`
|
||||
* counts runs of Code, while `Checking the PR head ×3` would claim that
|
||||
* sentence happened three times — a call that names its own work already says
|
||||
* which work it is doing, so the count only confuses it.
|
||||
*/
|
||||
type ToolLine = { text: string; generic: boolean };
|
||||
|
||||
function toolCallLine(
|
||||
part: TMessageContentParts,
|
||||
toolCall: LiveToolCall,
|
||||
localize: Localize,
|
||||
serverNames: readonly string[],
|
||||
span: SpanSummary,
|
||||
): string {
|
||||
): ToolLine {
|
||||
const intent = getToolCallIntent(toolCall.args);
|
||||
const label = getToolDisplayLabel(toolCall.name ?? '', localize, serverNames);
|
||||
/** The verdict comes from the resolver the group header uses, so a collapsed
|
||||
|
|
@ -98,30 +109,39 @@ function toolCallLine(
|
|||
* or a stop — whichever channel reported it. */
|
||||
const meta = span.metaOf(part);
|
||||
if (meta?.cancelled === true) {
|
||||
return localize('com_ui_cancelled');
|
||||
return { text: localize('com_ui_cancelled'), generic: false };
|
||||
}
|
||||
if (meta?.failed === true) {
|
||||
/** Reads as the hidden card does: `ToolCall` uses the same template. */
|
||||
const subject = intent ?? label;
|
||||
return subject ? localize('com_ui_failed_subject', { 0: subject }) : localize('com_ui_failed');
|
||||
return {
|
||||
text: subject ? localize('com_ui_failed_subject', { 0: subject }) : localize('com_ui_failed'),
|
||||
generic: false,
|
||||
};
|
||||
}
|
||||
/** Ahead of the intent, as on `BashCall`/`ExecuteCode`: a returned handle
|
||||
* is not a result, and "Ran …" would turn ongoing work into a success. */
|
||||
if (meta?.background != null) {
|
||||
return localize(
|
||||
meta.background === 'running' ? 'com_ui_background_running' : 'com_ui_background_finished',
|
||||
);
|
||||
return {
|
||||
text: localize(
|
||||
meta.background === 'running' ? 'com_ui_background_running' : 'com_ui_background_finished',
|
||||
),
|
||||
generic: false,
|
||||
};
|
||||
}
|
||||
if (intent != null) {
|
||||
return intent;
|
||||
return { text: intent, generic: false };
|
||||
}
|
||||
if (!label) {
|
||||
return localize('com_assistants_running_action');
|
||||
return { text: localize('com_assistants_running_action'), generic: true };
|
||||
}
|
||||
return localize(
|
||||
meta?.hasOutput === true ? 'com_assistants_completed_function' : 'com_assistants_running_var',
|
||||
{ 0: label },
|
||||
);
|
||||
return {
|
||||
text: localize(
|
||||
meta?.hasOutput === true ? 'com_assistants_completed_function' : 'com_assistants_running_var',
|
||||
{ 0: label },
|
||||
),
|
||||
generic: true,
|
||||
};
|
||||
}
|
||||
|
||||
/** Bounds the sentence scan on long reasoning, like the streaming peek. */
|
||||
|
|
@ -280,13 +300,17 @@ function newestLine(
|
|||
}
|
||||
const toolCall = getStandardToolCall(part);
|
||||
if (toolCall != null) {
|
||||
const line = toolCallLine(part, toolCall, localize, serverNames, span);
|
||||
return {
|
||||
text: toolCallLine(part, toolCall, localize, serverNames, span),
|
||||
text: line.text,
|
||||
/** Provider ids repeat across batches, so the position is part of the
|
||||
* identity: a second call reusing an id is a new line, not the first
|
||||
* one still growing. */
|
||||
source: `tool:${toolCall.id ?? ''}:${position}`,
|
||||
comboCount: Math.max(1, span.trailingToolCount),
|
||||
/** Counted for the generic label alone: as soon as the call names its
|
||||
* own work, or reports how it ended, the count has nothing left to
|
||||
* multiply and reads as a claim about that sentence. */
|
||||
comboCount: line.generic ? Math.max(1, span.trailingToolCount) : 1,
|
||||
...(isAwaitingStartup(part, toolCall, span) && { pendingToolCallId: toolCall.id }),
|
||||
};
|
||||
}
|
||||
|
|
@ -298,7 +322,8 @@ function newestLine(
|
|||
* The newest nameable activity in a span: the last tool call's own line (its
|
||||
* streamed intent, else the generic text its card would show), a filled batch
|
||||
* label once one lands after it, or the thought streaming after both. Later parts win, so the
|
||||
* header always reads as the bottom line of the list it stands for.
|
||||
* header always reads as the bottom line of the list it stands for. A repeat
|
||||
* count rides the generic label only, never a line that names one call.
|
||||
*
|
||||
* Runs on every streamed delta. Outcomes and the tool combo share one full-span
|
||||
* pass so late failures cannot disappear; the line stops at the newest nameable
|
||||
|
|
|
|||
|
|
@ -14,7 +14,8 @@ const root = fileURLToPath(new URL('../', import.meta.url));
|
|||
// without requiring a database or identity provider. It does not simulate an active model run.
|
||||
test(
|
||||
'an old tab retains its bundle identity and draft across a worker update',
|
||||
{ timeout: 30000 },
|
||||
/** Allow setup plus both activations; individual browser waits remain bounded. */
|
||||
{ timeout: 60000 },
|
||||
async () => {
|
||||
const temporary = await mkdtemp(path.join(tmpdir(), 'librechat-builds-'));
|
||||
const appHtml = await readFile(path.join(root, 'client/index.html'), 'utf8');
|
||||
|
|
@ -55,7 +56,10 @@ test(
|
|||
window.__lcRumPush('before-bootstrap');
|
||||
installRumBootstrap(window);
|
||||
window.fixtureVersion = ${JSON.stringify(version)};
|
||||
navigator.serviceWorker.register('/sw.js');`,
|
||||
navigator.serviceWorker.register('/sw.js').then(
|
||||
registration => { window.fixtureRegistration = registration; },
|
||||
error => { window.fixtureWorkerError = String(error); },
|
||||
);`,
|
||||
);
|
||||
await build({
|
||||
root: fixture,
|
||||
|
|
@ -71,14 +75,25 @@ test(
|
|||
);
|
||||
}
|
||||
await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve));
|
||||
browser = await chromium.launch({ headless: true, channel: process.env.PLAYWRIGHT_CHANNEL });
|
||||
browser = await chromium.launch({
|
||||
headless: true,
|
||||
channel: process.env.PLAYWRIGHT_CHANNEL,
|
||||
timeout: 10000,
|
||||
});
|
||||
const page = await browser.newPage();
|
||||
page.setDefaultTimeout(10000);
|
||||
const url = `http://127.0.0.1:${server.address().port}`;
|
||||
await page.goto(`${url}/c/example`);
|
||||
await page.waitForFunction(() => window.fixtureVersion === 'A');
|
||||
await page.evaluate(() => navigator.serviceWorker.ready);
|
||||
await page.waitForFunction(() => !!navigator.serviceWorker.controller);
|
||||
/** `ready` has no timeout and a controller can still be activating. Finish
|
||||
* A's handshake before clearing its events or requesting another worker. */
|
||||
await page.waitForFunction(
|
||||
() =>
|
||||
window.fixtureWorkerError ||
|
||||
(window.fixtureRegistration?.active?.state === 'activated' &&
|
||||
navigator.serviceWorker.controller?.state === 'activated'),
|
||||
);
|
||||
assert.equal(await page.evaluate(() => window.fixtureWorkerError), undefined);
|
||||
await page.getByLabel('Draft').fill('Keep my unsent text');
|
||||
const firstId = await page.evaluate(() => window.__lcRumQueue[0].attributes.clientBuildId);
|
||||
assert.match(firstId, /^index\..+\.js$/);
|
||||
|
|
@ -93,15 +108,30 @@ test(
|
|||
|
||||
await page.evaluate(() => {
|
||||
window.__lcRumQueue.length = 0;
|
||||
window.fixturePreviousController = navigator.serviceWorker.controller;
|
||||
});
|
||||
serving = 'B';
|
||||
await page.evaluate(async () => {
|
||||
const registration = await navigator.serviceWorker.getRegistration();
|
||||
await registration.update();
|
||||
/** `evaluate` does not bound an awaited update promise. Observe its result
|
||||
* through the timed wait, and require B's activation, not a late A ping. */
|
||||
await page.evaluate(() => {
|
||||
window.fixtureRegistration.update().then(
|
||||
() => {
|
||||
window.fixtureUpdateFinished = true;
|
||||
},
|
||||
(error) => {
|
||||
window.fixtureWorkerError = String(error);
|
||||
},
|
||||
);
|
||||
});
|
||||
await page.waitForFunction(() =>
|
||||
window.__lcRumQueue.some((event) => event.type === 'sw-ping'),
|
||||
await page.waitForFunction(
|
||||
() =>
|
||||
window.fixtureWorkerError ||
|
||||
(window.fixtureUpdateFinished &&
|
||||
navigator.serviceWorker.controller !== window.fixturePreviousController &&
|
||||
navigator.serviceWorker.controller?.state === 'activated' &&
|
||||
window.__lcRumQueue.some((event) => event.type === 'sw-ping')),
|
||||
);
|
||||
assert.equal(await page.evaluate(() => window.fixtureWorkerError), undefined);
|
||||
// Outlive the worker's unresponsive-client deadline to catch an unwanted navigation.
|
||||
await page.waitForTimeout(2000);
|
||||
assert.equal(await page.getByLabel('Draft').inputValue(), 'Keep my unsent text');
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue