mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-22 18:13:27 +00:00
🩹 fix: Address Codex Review on the PTC Tool Trace
Five findings, all confirmed against the source before fixing. Scope the trace atoms to a message occurrence. The hook already documents that providers repeat a tool_call_id across turns and even within one message, and `call_id` restarts at :0 for every outer call — so two programs sharing `call_0` merged into one card. Key by (response message id, tool call id) via `ptcTraceKey`, mirroring `subagentProgressKey`; the event's `runId` already carries the message id and the card reads its own from MessageContext. Prune unsettled rows on resume. Inner calls are not content parts, so the resume snapshot cannot rebuild them, and `trackReplayEvent` only persists OAuth events — a call that settled during a disconnect left a spinner that never resolved. Settled rows are real history and stay. Make the argument preview budget-aware. Iterate keys rather than entries so the budget check can actually skip work, and clip against a bounded window so a multi-megabyte value is never collapsed in full to build a 40-character preview. Catch the resumable emission promise. The synchronous try/catch around the emitter cannot observe a rejected `emitChunk`, so a failing transport raised an unhandled rejection per event instead of dropping telemetry. Announce completion to assistive technology. The check glyph is decorative and a fast call renders no duration, so a settled row previously announced no outcome; each row now carries an sr-only status and the visible cell that duplicated it is hidden. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MBRQUsPXSvDBnjrSDoXWHP
This commit is contained in:
parent
1ddf8e24ce
commit
edbbefe898
14 changed files with 298 additions and 49 deletions
|
|
@ -187,6 +187,26 @@ describe('createPtcProgressEmitter', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('absorbs a rejected resumable emit instead of leaving an unhandled rejection', async () => {
|
||||
const { GenerationJobManager } = require('@librechat/api');
|
||||
const { createPtcProgressEmitter } = require('../callbacks');
|
||||
GenerationJobManager.emitChunk.mockRejectedValueOnce(new Error('transport down'));
|
||||
const unhandled = jest.fn();
|
||||
process.on('unhandledRejection', unhandled);
|
||||
|
||||
const emit = createPtcProgressEmitter({
|
||||
res: { write: jest.fn() },
|
||||
streamId: 'conversation-1',
|
||||
jobCreatedAt: 1234,
|
||||
});
|
||||
|
||||
expect(() => emit(ptcEvent)).not.toThrow();
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
process.off('unhandledRejection', unhandled);
|
||||
|
||||
expect(unhandled).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('drops the event once the response has closed', () => {
|
||||
const { sendEvent } = require('@librechat/api');
|
||||
const { createPtcProgressEmitter } = require('../callbacks');
|
||||
|
|
|
|||
|
|
@ -1106,7 +1106,16 @@ function createPtcProgressEmitter({ res, streamId = null, jobCreatedAt }) {
|
|||
}
|
||||
const payload = { event: StepEvents.ON_PTC_TOOL_CALL, data: event };
|
||||
if (streamId) {
|
||||
GenerationJobManager.emitChunk(streamId, payload, { expectedCreatedAt: jobCreatedAt });
|
||||
/* Absorb a rejected transport here. The emitter is called from a
|
||||
* synchronous try/catch inside `instrumentPtcToolMap`, which cannot
|
||||
* observe a rejected promise — without this catch a failed emit would
|
||||
* surface as an unhandled rejection on every affected inner call
|
||||
* instead of being dropped as the telemetry it is. */
|
||||
Promise.resolve(
|
||||
GenerationJobManager.emitChunk(streamId, payload, { expectedCreatedAt: jobCreatedAt }),
|
||||
).catch(() => {
|
||||
/* dropped: the trace is best-effort */
|
||||
});
|
||||
return;
|
||||
}
|
||||
sendEvent(res, payload);
|
||||
|
|
|
|||
|
|
@ -1,9 +1,11 @@
|
|||
import { useRecoilValue } from 'recoil';
|
||||
import { useTranslation } from 'react-i18next';
|
||||
import { isReportableRunStepDuration } from 'librechat-data-provider';
|
||||
import type { TranslationKeys } from '~/hooks';
|
||||
import type { PtcTraceEntry } from '~/store';
|
||||
import { cn, parseToolName, getRunStepDurationLabels } from '~/utils';
|
||||
import { ptcTraceByToolCallId } from '~/store';
|
||||
import { ptcTraceByToolCallId, ptcTraceKey } from '~/store';
|
||||
import { useMessageContext } from '~/Providers/MessageContext';
|
||||
import { useMCPServerNames } from '~/hooks/MCP';
|
||||
import { useLocalize } from '~/hooks';
|
||||
|
||||
|
|
@ -15,6 +17,15 @@ const STATUS_GLYPH: Record<PtcTraceEntry['status'], string> = {
|
|||
error: '✗',
|
||||
};
|
||||
|
||||
/** Spoken equivalent of the glyph. The glyph itself is decorative, and a
|
||||
* fast call renders no duration, so without this a completed row would
|
||||
* announce no outcome at all. */
|
||||
const STATUS_LABEL_KEYS: Record<PtcTraceEntry['status'], TranslationKeys> = {
|
||||
running: 'com_ui_ptc_trace_running',
|
||||
success: 'com_ui_ptc_trace_done',
|
||||
error: 'com_ui_ptc_trace_failed',
|
||||
};
|
||||
|
||||
/**
|
||||
* One line of the trace. Reads as a shell transcript — status glyph, the tool
|
||||
* being called, its arguments, and how long it took — with the failure message
|
||||
|
|
@ -44,6 +55,7 @@ function PtcTraceLine({ entry }: { entry: PtcTraceEntry }) {
|
|||
>
|
||||
{STATUS_GLYPH[entry.status]}
|
||||
</span>
|
||||
<span className="sr-only">{localize(STATUS_LABEL_KEYS[entry.status])}</span>
|
||||
<span className="shrink-0 text-text-primary">
|
||||
{parsed.mcpServer && (
|
||||
<>
|
||||
|
|
@ -64,7 +76,8 @@ function PtcTraceLine({ entry }: { entry: PtcTraceEntry }) {
|
|||
</span>
|
||||
</>
|
||||
) : (
|
||||
<span className="shrink-0 text-text-tertiary">
|
||||
/* The sr-only status above already speaks this cell's meaning. */
|
||||
<span className="shrink-0 text-text-tertiary" aria-hidden="true">
|
||||
{running ? localize('com_ui_ptc_trace_running') : ''}
|
||||
</span>
|
||||
)}
|
||||
|
|
@ -98,7 +111,12 @@ export default function PtcToolTrace({
|
|||
className?: string;
|
||||
}) {
|
||||
const localize = useLocalize();
|
||||
const entries = useRecoilValue(ptcTraceByToolCallId(toolCallId ?? ''));
|
||||
/** Scope to this card's own message: providers reuse `tool_call_id` across
|
||||
* turns, so the raw id alone would show a later program's calls here. */
|
||||
const { messageId } = useMessageContext();
|
||||
const entries = useRecoilValue(
|
||||
ptcTraceByToolCallId(toolCallId && messageId ? ptcTraceKey(messageId, toolCallId) : ''),
|
||||
);
|
||||
|
||||
if (entries.length === 0) {
|
||||
return null;
|
||||
|
|
|
|||
|
|
@ -2,7 +2,8 @@ import React from 'react';
|
|||
import { RecoilRoot } from 'recoil';
|
||||
import { render } from '@testing-library/react';
|
||||
import type { PtcTraceEntry } from '~/store/ptc';
|
||||
import { ptcTraceByToolCallId } from '~/store/ptc';
|
||||
import { ptcTraceByToolCallId, ptcTraceKey } from '~/store/ptc';
|
||||
import { MessageContext } from '~/Providers/MessageContext';
|
||||
import ExecuteCode from '../ExecuteCode';
|
||||
import store from '~/store';
|
||||
|
||||
|
|
@ -144,22 +145,25 @@ describe('ExecuteCode streaming follow-scroll', () => {
|
|||
|
||||
describe('ExecuteCode programmatic tool trace', () => {
|
||||
const TOOL_CALL_ID = 'call_ptc_1';
|
||||
const MESSAGE_ID = 'response-msg-1';
|
||||
|
||||
const renderWithTrace = (entries: PtcTraceEntry[]) =>
|
||||
render(
|
||||
<RecoilRoot
|
||||
initializeState={({ set }) => {
|
||||
set(store.autoExpandTools, true);
|
||||
set(ptcTraceByToolCallId(TOOL_CALL_ID), entries);
|
||||
set(ptcTraceByToolCallId(ptcTraceKey(MESSAGE_ID, TOOL_CALL_ID)), entries);
|
||||
}}
|
||||
>
|
||||
<ExecuteCode
|
||||
initialProgress={0.5}
|
||||
isSubmitting={true}
|
||||
toolCallId={TOOL_CALL_ID}
|
||||
args={{ lang: 'py', code: 'print(1)' }}
|
||||
output=""
|
||||
/>
|
||||
<MessageContext.Provider value={{ messageId: MESSAGE_ID, isExpanded: true }}>
|
||||
<ExecuteCode
|
||||
initialProgress={0.5}
|
||||
isSubmitting={true}
|
||||
toolCallId={TOOL_CALL_ID}
|
||||
args={{ lang: 'py', code: 'print(1)' }}
|
||||
output=""
|
||||
/>
|
||||
</MessageContext.Provider>
|
||||
</RecoilRoot>,
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -2,7 +2,8 @@ import React from 'react';
|
|||
import { RecoilRoot, useSetRecoilState } from 'recoil';
|
||||
import { render, screen } from '@testing-library/react';
|
||||
import type { PtcTraceEntry } from '~/store/ptc';
|
||||
import { ptcTraceByToolCallId } from '~/store/ptc';
|
||||
import { ptcTraceByToolCallId, ptcTraceKey } from '~/store/ptc';
|
||||
import { MessageContext } from '~/Providers/MessageContext';
|
||||
import PtcToolTrace from '../PtcToolTrace';
|
||||
|
||||
jest.mock('~/hooks', () => ({
|
||||
|
|
@ -13,6 +14,7 @@ jest.mock('~/hooks', () => ({
|
|||
com_ui_ptc_trace_title: 'Tool calls',
|
||||
com_ui_ptc_trace_running: 'running',
|
||||
com_ui_ptc_trace_failed: 'failed',
|
||||
com_ui_ptc_trace_done: 'done',
|
||||
com_ui_tool_name_code: 'Code',
|
||||
};
|
||||
if (key === 'com_ui_duration_seconds') {
|
||||
|
|
@ -31,10 +33,11 @@ jest.mock('~/utils', () => ({
|
|||
}));
|
||||
|
||||
const TOOL_CALL_ID = 'call_ptc_1';
|
||||
const MESSAGE_ID = 'response-msg-1';
|
||||
|
||||
function renderTrace(entries: PtcTraceEntry[]) {
|
||||
function renderTrace(entries: PtcTraceEntry[], messageId = MESSAGE_ID) {
|
||||
const Seed = () => {
|
||||
const set = useSetRecoilState(ptcTraceByToolCallId(TOOL_CALL_ID));
|
||||
const set = useSetRecoilState(ptcTraceByToolCallId(ptcTraceKey(MESSAGE_ID, TOOL_CALL_ID)));
|
||||
React.useEffect(() => {
|
||||
if (entries.length > 0) {
|
||||
set(entries);
|
||||
|
|
@ -46,7 +49,9 @@ function renderTrace(entries: PtcTraceEntry[]) {
|
|||
return render(
|
||||
<RecoilRoot>
|
||||
<Seed />
|
||||
<PtcToolTrace toolCallId={TOOL_CALL_ID} />
|
||||
<MessageContext.Provider value={{ messageId, isExpanded: false }}>
|
||||
<PtcToolTrace toolCallId={TOOL_CALL_ID} />
|
||||
</MessageContext.Provider>
|
||||
</RecoilRoot>,
|
||||
);
|
||||
}
|
||||
|
|
@ -67,7 +72,8 @@ describe('PtcToolTrace', () => {
|
|||
expect(screen.getByText('read_file')).toBeInTheDocument();
|
||||
expect(screen.getByText('path=a.ts')).toBeInTheDocument();
|
||||
expect(screen.getByText('write_file')).toBeInTheDocument();
|
||||
expect(screen.getByText('running')).toBeInTheDocument();
|
||||
/* Twice by design: the visible cell plus the screen-reader status. */
|
||||
expect(screen.getAllByText('running')).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('splits an MCP tool id into its server and tool name', () => {
|
||||
|
|
@ -95,6 +101,18 @@ describe('PtcToolTrace', () => {
|
|||
it('falls back to a generic failure label when no message came through', () => {
|
||||
renderTrace([{ callId: 'a', name: 'write_file', status: 'error' }]);
|
||||
|
||||
expect(screen.getByText('failed')).toBeInTheDocument();
|
||||
expect(screen.getAllByText('failed').length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('speaks a completion status even when the call was too fast to time', () => {
|
||||
renderTrace([{ callId: 'a', name: 'read_file', status: 'success' }]);
|
||||
|
||||
expect(screen.getByText('done')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('does not show another message\u2019s trace in this card', () => {
|
||||
renderTrace([{ callId: 'a', name: 'read_file', status: 'success' }], 'a-different-message');
|
||||
|
||||
expect(screen.queryByText('read_file')).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -236,6 +236,7 @@ jest.mock('~/hooks/SSE/useEventHandlers', () => {
|
|||
contentHandler: jest.fn(),
|
||||
resetContentHandler: jest.fn(),
|
||||
syncStepMessage: jest.fn(),
|
||||
prunePtcTraces: jest.fn(),
|
||||
clearStepMaps: mockClearStepMaps,
|
||||
flushPendingDeltas: jest.fn(),
|
||||
messageHandler: jest.fn(),
|
||||
|
|
|
|||
|
|
@ -20,7 +20,7 @@ import type {
|
|||
} from 'librechat-data-provider';
|
||||
import type { PtcTraceEntry } from '~/store/ptc';
|
||||
import { subagentProgressByToolCallId, subagentProgressKey } from '~/store/subagents';
|
||||
import { ptcTraceByToolCallId } from '~/store/ptc';
|
||||
import { ptcTraceByToolCallId, ptcTraceKey } from '~/store/ptc';
|
||||
import { resolveAskUserQuestionPart } from '~/utils/approval';
|
||||
import useStepHandler from '~/hooks/SSE/useStepHandler';
|
||||
|
||||
|
|
@ -3888,8 +3888,10 @@ describe('useStepHandler', () => {
|
|||
const stepHandler = useStepHandler(createHookParams());
|
||||
const read = useRecoilCallback(
|
||||
({ snapshot }) =>
|
||||
(toolCallId: string): PtcTraceEntry[] =>
|
||||
snapshot.getLoadable(ptcTraceByToolCallId(toolCallId)).valueOrThrow(),
|
||||
(messageId: string, toolCallId: string): PtcTraceEntry[] =>
|
||||
snapshot
|
||||
.getLoadable(ptcTraceByToolCallId(ptcTraceKey(messageId, toolCallId)))
|
||||
.valueOrThrow(),
|
||||
[],
|
||||
);
|
||||
return { ...stepHandler, read };
|
||||
|
|
@ -3898,10 +3900,12 @@ describe('useStepHandler', () => {
|
|||
);
|
||||
return {
|
||||
result: hookResult.result,
|
||||
getTrace: (toolCallId: string): PtcTraceEntry[] =>
|
||||
(hookResult.result.current as unknown as { read: (id: string) => PtcTraceEntry[] }).read(
|
||||
toolCallId,
|
||||
),
|
||||
getTrace: (toolCallId: string, messageId = 'response-msg-1'): PtcTraceEntry[] =>
|
||||
(
|
||||
hookResult.result.current as unknown as {
|
||||
read: (m: string, id: string) => PtcTraceEntry[];
|
||||
}
|
||||
).read(messageId, toolCallId),
|
||||
};
|
||||
};
|
||||
|
||||
|
|
@ -3910,6 +3914,7 @@ describe('useStepHandler', () => {
|
|||
call_id: 'call_ptc:0',
|
||||
name: 'read_file',
|
||||
status: 'running',
|
||||
runId: 'response-msg-1',
|
||||
...overrides,
|
||||
});
|
||||
|
||||
|
|
@ -4005,6 +4010,71 @@ describe('useStepHandler', () => {
|
|||
expect(getTrace('call_ptc')).toEqual([]);
|
||||
});
|
||||
|
||||
it('keeps a reused tool_call_id isolated across parent messages', () => {
|
||||
const { result, getTrace } = renderWithTraceReader();
|
||||
const submission = createSubmission();
|
||||
|
||||
act(() => {
|
||||
result.current.stepHandler(
|
||||
{ event: StepEvents.ON_PTC_TOOL_CALL, data: ptcEvent({ name: 'read_file' }) },
|
||||
submission,
|
||||
);
|
||||
result.current.stepHandler(
|
||||
{
|
||||
event: StepEvents.ON_PTC_TOOL_CALL,
|
||||
data: ptcEvent({ name: 'write_file', runId: 'response-msg-2' }),
|
||||
},
|
||||
submission,
|
||||
);
|
||||
});
|
||||
|
||||
expect(getTrace('call_ptc').map((e) => e.name)).toEqual(['read_file']);
|
||||
expect(getTrace('call_ptc', 'response-msg-2').map((e) => e.name)).toEqual(['write_file']);
|
||||
});
|
||||
|
||||
it('drops an envelope that cannot be scoped to a parent message', () => {
|
||||
const { result, getTrace } = renderWithTraceReader();
|
||||
const submission = createSubmission();
|
||||
|
||||
act(() => {
|
||||
result.current.stepHandler(
|
||||
{ event: StepEvents.ON_PTC_TOOL_CALL, data: ptcEvent({ runId: undefined }) },
|
||||
submission,
|
||||
);
|
||||
});
|
||||
|
||||
expect(getTrace('call_ptc')).toEqual([]);
|
||||
});
|
||||
|
||||
it('prunes rows still running across a resume gap and keeps settled ones', () => {
|
||||
const { result, getTrace } = renderWithTraceReader();
|
||||
const submission = createSubmission();
|
||||
|
||||
act(() => {
|
||||
result.current.stepHandler(
|
||||
{
|
||||
event: StepEvents.ON_PTC_TOOL_CALL,
|
||||
data: ptcEvent({ call_id: 'call_ptc:0', status: 'success', durationMs: 900 }),
|
||||
},
|
||||
submission,
|
||||
);
|
||||
result.current.stepHandler(
|
||||
{
|
||||
event: StepEvents.ON_PTC_TOOL_CALL,
|
||||
data: ptcEvent({ call_id: 'call_ptc:1', name: 'write_file' }),
|
||||
},
|
||||
submission,
|
||||
);
|
||||
});
|
||||
expect(getTrace('call_ptc')).toHaveLength(2);
|
||||
|
||||
act(() => {
|
||||
(result.current as unknown as { prunePtcTraces: () => void }).prunePtcTraces();
|
||||
});
|
||||
|
||||
expect(getTrace('call_ptc').map((e) => e.callId)).toEqual(['call_ptc:0']);
|
||||
});
|
||||
|
||||
it('releases the trace atoms on reset', () => {
|
||||
const { result, getTrace } = renderWithTraceReader();
|
||||
const submission = createSubmission();
|
||||
|
|
|
|||
|
|
@ -376,6 +376,7 @@ export default function useEventHandlers({
|
|||
clearStepMaps,
|
||||
resetSubagentAtoms,
|
||||
resetPtcAtoms,
|
||||
prunePtcTraces,
|
||||
syncStepMessage,
|
||||
cancelPendingDeltaFlush,
|
||||
flushPendingDeltas,
|
||||
|
|
@ -1211,6 +1212,7 @@ export default function useEventHandlers({
|
|||
createdHandler,
|
||||
titleHandler,
|
||||
syncStepMessage,
|
||||
prunePtcTraces,
|
||||
cancelPendingDeltaFlush,
|
||||
flushPendingDeltas,
|
||||
attachmentHandler,
|
||||
|
|
|
|||
|
|
@ -1137,6 +1137,7 @@ export default function useResumableSSE(
|
|||
createdHandler,
|
||||
titleHandler,
|
||||
syncStepMessage,
|
||||
prunePtcTraces,
|
||||
attachmentHandler,
|
||||
resetContentHandler,
|
||||
flushPendingDeltas,
|
||||
|
|
@ -2126,6 +2127,12 @@ export default function useResumableSSE(
|
|||
titleHandler(data.resumeState.titleEvent);
|
||||
}
|
||||
|
||||
/** PTC inner calls are not content parts, so the resume snapshot
|
||||
* can't rebuild them and their settling events aren't replayed.
|
||||
* Any row still `running` across this gap is unknowable — drop it
|
||||
* rather than leave a spinner that never resolves. */
|
||||
prunePtcTraces();
|
||||
|
||||
if (data.resumeState?.replayEvents?.length > 0) {
|
||||
logger.log(
|
||||
'ResumableSSE',
|
||||
|
|
@ -3474,6 +3481,7 @@ export default function useResumableSSE(
|
|||
contentHandler,
|
||||
resetContentHandler,
|
||||
syncStepMessage,
|
||||
prunePtcTraces,
|
||||
clearStepMaps,
|
||||
messageHandler,
|
||||
errorHandler,
|
||||
|
|
|
|||
|
|
@ -34,6 +34,7 @@ import {
|
|||
sandboxStartingByToolCallId,
|
||||
ptcTraceByToolCallId,
|
||||
subagentProgressKey,
|
||||
ptcTraceKey,
|
||||
} from '~/store';
|
||||
import { isAskUserQuestionPart, isAnsweredAskUserQuestionPart } from '~/utils/approval';
|
||||
import { MESSAGE_UPDATE_INTERVAL } from '~/common';
|
||||
|
|
@ -380,13 +381,16 @@ export default function useStepHandler({
|
|||
*/
|
||||
const applyPtcToolCall = useRecoilCallback(
|
||||
({ set }) =>
|
||||
(event: PtcToolCallEvent): void => {
|
||||
(event: PtcToolCallEvent, parentMessageId: string): void => {
|
||||
const { tool_call_id: toolCallId, call_id: callId, name, status } = event;
|
||||
if (!toolCallId || !callId) {
|
||||
/** No parent message means no occurrence to scope this to; a raw
|
||||
* `tool_call_id` would leak the rows into whichever card reused it. */
|
||||
if (!toolCallId || !callId || !parentMessageId) {
|
||||
return;
|
||||
}
|
||||
knownPtcAtomKeys.current.add(toolCallId);
|
||||
set(ptcTraceByToolCallId(toolCallId), (previous) => {
|
||||
const atomKey = ptcTraceKey(parentMessageId, toolCallId);
|
||||
knownPtcAtomKeys.current.add(atomKey);
|
||||
set(ptcTraceByToolCallId(atomKey), (previous) => {
|
||||
const index = previous.findIndex((entry) => entry.callId === callId);
|
||||
const entry = {
|
||||
callId,
|
||||
|
|
@ -410,14 +414,35 @@ export default function useStepHandler({
|
|||
const resetPtcAtoms = useRecoilCallback(
|
||||
({ reset }) =>
|
||||
(): void => {
|
||||
for (const toolCallId of knownPtcAtomKeys.current) {
|
||||
reset(ptcTraceByToolCallId(toolCallId));
|
||||
for (const atomKey of knownPtcAtomKeys.current) {
|
||||
reset(ptcTraceByToolCallId(atomKey));
|
||||
}
|
||||
knownPtcAtomKeys.current.clear();
|
||||
},
|
||||
[],
|
||||
);
|
||||
|
||||
/**
|
||||
* Drops rows still marked `running` after a stream gap. Inner calls carry no
|
||||
* durable state — they are not content parts, so the resume snapshot cannot
|
||||
* rebuild them and their settling events are not replayed — which means a
|
||||
* call that finished during a disconnect would otherwise show a spinner
|
||||
* forever. Settled rows are real history and stay.
|
||||
*/
|
||||
const prunePtcTraces = useRecoilCallback(
|
||||
({ set }) =>
|
||||
(): void => {
|
||||
for (const atomKey of knownPtcAtomKeys.current) {
|
||||
set(ptcTraceByToolCallId(atomKey), (previous) =>
|
||||
previous.some((entry) => entry.status === 'running')
|
||||
? previous.filter((entry) => entry.status !== 'running')
|
||||
: previous,
|
||||
);
|
||||
}
|
||||
},
|
||||
[],
|
||||
);
|
||||
|
||||
/**
|
||||
* Calculate content index for a run step.
|
||||
*
|
||||
|
|
@ -1331,7 +1356,13 @@ export default function useStepHandler({
|
|||
} else if (stepEvent.event === StepEvents.ON_SANDBOX_STARTING) {
|
||||
setSandboxStarting(stepEvent.data.tool_call_id);
|
||||
} else if (stepEvent.event === StepEvents.ON_PTC_TOOL_CALL) {
|
||||
applyPtcToolCall(stepEvent.data);
|
||||
/** `runId` is the response message id (the run configurable's
|
||||
* `run_id`), the same correlation the subagent path uses. */
|
||||
let responseMessageId = stepEvent.data.runId ?? '';
|
||||
if (responseMessageId === Constants.USE_PRELIM_RESPONSE_MESSAGE_ID) {
|
||||
responseMessageId = submission?.initialResponse?.messageId ?? '';
|
||||
}
|
||||
applyPtcToolCall(stepEvent.data, responseMessageId);
|
||||
} else if (stepEvent.event === StepEvents.ON_SUBAGENT_UPDATE) {
|
||||
let responseMessageId = stepEvent.data.runId;
|
||||
if (responseMessageId === Constants.USE_PRELIM_RESPONSE_MESSAGE_ID) {
|
||||
|
|
@ -1534,6 +1565,7 @@ export default function useStepHandler({
|
|||
clearStepMaps,
|
||||
resetSubagentAtoms,
|
||||
resetPtcAtoms,
|
||||
prunePtcTraces,
|
||||
syncStepMessage,
|
||||
cancelPendingDeltaFlush,
|
||||
flushPendingDeltas,
|
||||
|
|
|
|||
|
|
@ -1742,6 +1742,7 @@
|
|||
"com_ui_provider": "Provider",
|
||||
"com_ui_provider_api_keys_description": "Manage API keys for endpoints configured to use a user-provided key.",
|
||||
"com_ui_provider_api_keys_not_set": "No key set",
|
||||
"com_ui_ptc_trace_done": "done",
|
||||
"com_ui_ptc_trace_failed": "failed",
|
||||
"com_ui_ptc_trace_running": "running",
|
||||
"com_ui_ptc_trace_title": "Tool calls",
|
||||
|
|
|
|||
|
|
@ -15,10 +15,20 @@ export interface PtcTraceEntry {
|
|||
durationMs?: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Stable identity for one PTC invocation in its parent message. Providers
|
||||
* reuse `tool_call_id` across turns and agents, so the raw id is not
|
||||
* sufficient identity for live progress — the same rule `subagentProgressKey`
|
||||
* exists for. Without the message scope, a later program's rows would land in
|
||||
* an earlier card that happened to share `call_0`.
|
||||
*/
|
||||
export const ptcTraceKey = (parentMessageId: string, toolCallId: string) =>
|
||||
`${parentMessageId}\u0000${toolCallId}`;
|
||||
|
||||
/**
|
||||
* Live trace of the inner tool calls made by one PTC run step, in the order
|
||||
* the sandbox started them (`on_ptc_tool_call` SSE events). Keyed by the PTC
|
||||
* `tool_call_id` — the card the trace renders under.
|
||||
* the sandbox started them (`on_ptc_tool_call` SSE events). Keyed by
|
||||
* {@link ptcTraceKey} — one concrete invocation in one message.
|
||||
*
|
||||
* Session-scoped like the subagent ticker: inner calls open no run step, so
|
||||
* nothing persists them on the message. Cleared on conversation switch rather
|
||||
|
|
|
|||
|
|
@ -41,6 +41,34 @@ describe('summarizePtcArgs', () => {
|
|||
expect(summary.length).toBeLessThanOrEqual(97);
|
||||
});
|
||||
|
||||
it('stops once the preview budget is spent instead of visiting every key', () => {
|
||||
const seen: string[] = [];
|
||||
const probe: Record<string, unknown> = {};
|
||||
for (let i = 0; i < 40; i++) {
|
||||
Object.defineProperty(probe, `key${i}`, {
|
||||
enumerable: true,
|
||||
get() {
|
||||
seen.push(`key${i}`);
|
||||
return `value${i}`;
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
summarizePtcArgs(probe);
|
||||
|
||||
expect(seen.length).toBeLessThan(40);
|
||||
});
|
||||
|
||||
it('does not rewrite the whole of an oversized value to build a short preview', () => {
|
||||
const huge = 'a b '.repeat(500_000);
|
||||
const started = Date.now();
|
||||
const summary = summarizePtcArgs({ content: huge, path: 'a.ts' });
|
||||
|
||||
expect(summary).toContain('path=a.ts');
|
||||
expect(summary.length).toBeLessThanOrEqual(97);
|
||||
expect(Date.now() - started).toBeLessThan(150);
|
||||
});
|
||||
|
||||
it('falls back to the raw string for a non-object input', () => {
|
||||
expect(summarizePtcArgs('ls -la')).toBe('ls -la');
|
||||
});
|
||||
|
|
|
|||
|
|
@ -6,42 +6,70 @@ const ARGS_PREVIEW_MAX_CHARS = 96;
|
|||
/** Per-value budget, so one long string can't crowd out the other keys. */
|
||||
const ARGS_VALUE_MAX_CHARS = 40;
|
||||
const ERROR_PREVIEW_MAX_CHARS = 160;
|
||||
/**
|
||||
* Collapsing whitespace can only shorten a string, so a window this many times
|
||||
* the visible budget is always long enough to fill it. Slicing to the window
|
||||
* before rewriting matters: this runs synchronously ahead of every inner
|
||||
* `invoke`, and without it a multi-megabyte argument would be collapsed in
|
||||
* full to produce a forty-character preview.
|
||||
*/
|
||||
const CLIP_OVERSCAN = 4;
|
||||
|
||||
const truncate = (input: string, max: number): string =>
|
||||
input.length <= max ? input : `${input.slice(0, max)}…`;
|
||||
|
||||
const collapse = (input: string): string => input.replace(/\s+/g, ' ').trim();
|
||||
/** Bounded collapse-and-clip: never rewrites more of `input` than the budget
|
||||
* can possibly need, and marks any truncation it performed. */
|
||||
const clip = (input: string, max: number): string => {
|
||||
const window = input.length > max * CLIP_OVERSCAN ? input.slice(0, max * CLIP_OVERSCAN) : input;
|
||||
const collapsed = window.replace(/\s+/g, ' ').trim();
|
||||
if (collapsed.length <= max && window.length === input.length) {
|
||||
return collapsed;
|
||||
}
|
||||
return `${collapsed.slice(0, max)}…`;
|
||||
};
|
||||
|
||||
/**
|
||||
* Collapses an inner call's input into a single `key=value, key=value` line
|
||||
* for the CLI-style trace. Values are stringified and clipped individually;
|
||||
* the joined result is clipped again so the payload stays bounded no matter
|
||||
* how many keys the tool takes.
|
||||
* for the CLI-style trace. Values are clipped individually and iteration stops
|
||||
* as soon as the joined preview can no longer grow, so a call with a large
|
||||
* body or many keys costs the same as a small one.
|
||||
*/
|
||||
export function summarizePtcArgs(input: unknown): string {
|
||||
if (input == null) {
|
||||
return '';
|
||||
}
|
||||
if (typeof input === 'string') {
|
||||
return truncate(collapse(input), ARGS_PREVIEW_MAX_CHARS);
|
||||
return clip(input, ARGS_PREVIEW_MAX_CHARS);
|
||||
}
|
||||
if (typeof input !== 'object' || Array.isArray(input)) {
|
||||
return truncate(collapse(String(input)), ARGS_PREVIEW_MAX_CHARS);
|
||||
return clip(String(input), ARGS_PREVIEW_MAX_CHARS);
|
||||
}
|
||||
|
||||
const record = input as Record<string, unknown>;
|
||||
const entries: string[] = [];
|
||||
for (const [key, value] of Object.entries(input as Record<string, unknown>)) {
|
||||
let budget = ARGS_PREVIEW_MAX_CHARS;
|
||||
/* Keys, not entries: `Object.entries` would materialize every value before
|
||||
* the loop starts, so the budget check below could never skip the work it
|
||||
* exists to skip. */
|
||||
for (const key of Object.keys(record)) {
|
||||
if (budget <= 0) {
|
||||
break;
|
||||
}
|
||||
const value = record[key];
|
||||
if (value == null || value === '') {
|
||||
continue;
|
||||
}
|
||||
/* Strings are the values that get large (file bodies, request payloads),
|
||||
* and `clip` bounds them without touching the tail. Everything else is
|
||||
* small enough that serializing it first is cheaper than inspecting it. */
|
||||
const rendered = typeof value === 'string' ? value : safeStringify(value);
|
||||
if (rendered === '') {
|
||||
continue;
|
||||
}
|
||||
entries.push(`${key}=${truncate(collapse(rendered), ARGS_VALUE_MAX_CHARS)}`);
|
||||
const entry = `${key}=${clip(rendered, ARGS_VALUE_MAX_CHARS)}`;
|
||||
entries.push(entry);
|
||||
budget -= entry.length + 2;
|
||||
}
|
||||
|
||||
return truncate(entries.join(', '), ARGS_PREVIEW_MAX_CHARS);
|
||||
return clip(entries.join(', '), ARGS_PREVIEW_MAX_CHARS);
|
||||
}
|
||||
|
||||
function safeStringify(value: unknown): string {
|
||||
|
|
@ -126,8 +154,8 @@ export function instrumentPtcToolMap({
|
|||
call_id: callId,
|
||||
name,
|
||||
status: 'error',
|
||||
error: truncate(
|
||||
collapse(error instanceof Error ? error.message : String(error)),
|
||||
error: clip(
|
||||
error instanceof Error ? error.message : String(error),
|
||||
ERROR_PREVIEW_MAX_CHARS,
|
||||
),
|
||||
durationMs: Date.now() - startedAt,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue