diff --git a/api/server/controllers/agents/client.js b/api/server/controllers/agents/client.js index 48a1063ab2..3ed4a0cc2b 100644 --- a/api/server/controllers/agents/client.js +++ b/api/server/controllers/agents/client.js @@ -2287,6 +2287,23 @@ class AgentClient extends BaseClient { if (!Array.isArray(previousParts) || !Array.isArray(this.contentParts)) { return; } + /** Preserve sparse coordinates when completion did not actually reshape + * the content. A phase can reserve a leading hole for a tool part whose + * SDK event lands after the phase closes; scanning retained identities + * in an unchanged array would skip that hole and move the bound past the + * delayed tool before it arrives. */ + if (previousParts.length === this.contentParts.length) { + let unchanged = true; + for (let index = 0; index < previousParts.length; index += 1) { + if (previousParts[index] !== this.contentParts[index]) { + unchanged = false; + break; + } + } + if (unchanged) { + return; + } + } const retainedIndexes = new Map(); for (let index = 0; index < this.contentParts.length; index += 1) { const part = this.contentParts[index]; diff --git a/api/server/controllers/agents/client.test.js b/api/server/controllers/agents/client.test.js index 5bfa1f6093..b68bc29f0c 100644 --- a/api/server/controllers/agents/client.test.js +++ b/api/server/controllers/agents/client.test.js @@ -187,7 +187,7 @@ describe('AgentClient - applyHideSequentialOutputsFilter', () => { expect(phase.activity_start_index).toBe(1); }); - it('rebases phase bounds over sparse content without treating holes as retained parts', () => { + it('rebases phase bounds over reshaped sparse content without retaining holes', () => { const reasoning = { type: ContentTypes.THINK, think: 'planning' }; const toolCall = toolCallPart('tc-sparse'); const phase = { @@ -203,12 +203,56 @@ describe('AgentClient - applyHideSequentialOutputsFilter', () => { contentParts[3] = phase; contentParts[4] = final; const previousParts = [...contentParts]; - const ctx = { options: { agent: {} }, contentParts }; + const ctx = { options: { agent: {} }, contentParts: [toolCall, phase, final] }; expect(() => AgentClient.prototype.rebaseActivityPhaseBounds.call(ctx, previousParts), ).not.toThrow(); - expect(phase.activity_start_index).toBe(2); + expect(phase.activity_start_index).toBe(0); + }); + + it('preserves a sparse phase reservation when completion does not reshape content', () => { + const firstTool = toolCallPart('tool-1'); + const secondTool = toolCallPart('tool-2'); + const firstLabel = { + type: ContentTypes.ACTIVITY_LABEL, + activity_label: 'Recorded the first result', + tool_call_ids: ['tool-1'], + }; + const secondLabel = { + type: ContentTypes.ACTIVITY_LABEL, + activity_label: 'Recorded the second result', + tool_call_ids: ['tool-2'], + }; + const phase = { + type: ContentTypes.ACTIVITY_LABEL, + activity_label: 'Verified both results', + activity_label_type: 'phase', + activity_start_index: 0, + }; + const final = { type: ContentTypes.TEXT, text: 'Final answer', phase: 'final_answer' }; + const contentParts = []; + contentParts[1] = { type: ContentTypes.TEXT, text: '', phase: 'final_answer' }; + contentParts[2] = firstLabel; + contentParts[3] = secondTool; + contentParts[4] = secondLabel; + contentParts[5] = phase; + contentParts[6] = final; + const previousParts = [...contentParts]; + const ctx = { options: { agent: {} }, contentParts }; + + AgentClient.prototype.rebaseActivityPhaseBounds.call(ctx, previousParts); + expect(phase.activity_start_index).toBe(0); + + contentParts[0] = firstTool; + const phaseChildren = contentParts.slice( + phase.activity_start_index, + contentParts.indexOf(phase), + ); + expect(phaseChildren.map((part) => part?.tool_call?.id).filter(Boolean)).toEqual([ + 'tool-1', + 'tool-2', + ]); }); }); diff --git a/e2e/config/librechat.e2e.yaml b/e2e/config/librechat.e2e.yaml index beae52cfb1..b129e0266d 100644 --- a/e2e/config/librechat.e2e.yaml +++ b/e2e/config/librechat.e2e.yaml @@ -122,7 +122,7 @@ endpoints: titleConvo: false modelDisplayLabel: 'Mock Provider D' - # Activity labels are enabled ONLY here. They get a dedicated endpoint + # Child-only activity labels are enabled here. They get a dedicated endpoint # because a label collapses its tool group (a labeled block auto-collapses # even at one call), which hides the tool cards other specs assert on — # enabling this on a shared endpoint broke steering.spec.ts. Provider D is @@ -142,6 +142,24 @@ endpoints: activityLabel: true activityModel: 'mock-label-model' + # Parent activity phases need at least two sequential logical activities + # and have their own generated summary. Keep that behavior isolated from + # Provider E's child-label coverage so each endpoint exercises one config + # gate without changing the rendering assumptions of unrelated specs. + - name: 'Mock Provider F' + apiKey: 'e2e-mock-key-f' + baseURL: 'http://127.0.0.1:8889/v1' + models: + default: + - 'mock-model-f' + fetch: false + titleConvo: false + modelDisplayLabel: 'Mock Provider F' + activityLabel: true + activityModel: 'mock-label-model' + activityPhaseLabel: true + activityPhaseModel: 'mock-phase-label-model' + modelSpecs: prioritize: true # Enforcement would reject sends from the non-spec paths addedEndpoints @@ -153,6 +171,7 @@ modelSpecs: - 'Mock Provider C' - 'Mock Provider D' - 'Mock Provider E' + - 'Mock Provider F' - 'agents' list: - name: 'e2e-mock-provider-a' diff --git a/e2e/setup/fake-label-server.js b/e2e/setup/fake-label-server.js index ad2e590b27..f001ddb83a 100644 --- a/e2e/setup/fake-label-server.js +++ b/e2e/setup/fake-label-server.js @@ -17,11 +17,18 @@ const http = require('http'); const PORT = Number(process.env.E2E_LABEL_PORT) || 8889; +const PHASE_PROMPT_MARKER = 'Summarize what this phase of an agent run accomplished'; /** Recorded label requests, newest last. */ const requests = []; /** Test-controlled response behavior; `reset` restores these defaults. */ -const DEFAULT_BEHAVIOR = { mode: 'ok', label: null, delayMs: 0 }; +const DEFAULT_BEHAVIOR = { + mode: 'ok', + label: null, + phaseLabel: null, + labelsByPrompt: {}, + delayMs: 0, +}; let behavior = { ...DEFAULT_BEHAVIOR }; let labelCount = 0; @@ -161,8 +168,16 @@ const server = http.createServer(async (req, res) => { } /** Whitespace-only output must fill null, leaving the block unlabeled. */ + const promptLabel = Object.entries(behavior.labelsByPrompt ?? {}).find(([needle]) => + prompt.includes(needle), + )?.[1]; + const isPhase = prompt.includes(PHASE_PROMPT_MARKER); const label = - behavior.mode === 'blank' ? ' ' : (behavior.label ?? `E2E activity label ${labelCount}`); + behavior.mode === 'blank' + ? ' ' + : ((isPhase ? behavior.phaseLabel : promptLabel) ?? + behavior.label ?? + `E2E activity label ${labelCount}`); if (body.stream === true) { sendStream(res, body.model, label); diff --git a/e2e/setup/fake-model.js b/e2e/setup/fake-model.js index 021e6ced64..68217213a6 100644 --- a/e2e/setup/fake-model.js +++ b/e2e/setup/fake-model.js @@ -34,6 +34,7 @@ const STEER_TOOL_REPLY_MARKER = 'E2E_STEER_TOOL_REPLY:'; const STEER_SPLIT_REPLY_MARKER = 'E2E_STEER_SPLIT_REPLY:'; const STEER_LATE_REPLY_MARKER = 'E2E_STEER_LATE_REPLY:'; const ACTIVITY_REPLY_MARKER = 'E2E_ACTIVITY_REPLY:'; +const ACTIVITY_PHASE_REPLY_MARKER = 'E2E_ACTIVITY_PHASE_REPLY:'; const ASK_USER_QUESTION_MARKER = 'E2E_ASK_USER_QUESTION:'; const RESUME_ICON_REPLY_MARKER = 'E2E_RESUME_ICON_REPLY:'; const FORCED_ERROR_MARKER = 'E2E_FORCED_ERROR:'; @@ -68,6 +69,7 @@ const STEER_SPLIT_FINAL_TEXT = 'E2E steer split reply done'; const STEER_LATE_FINAL_TEXT = 'E2E steer late reply done'; const SLOW_REPLY_CONTINUATION_TEXT = 'E2E slow reply continued'; const ACTIVITY_FINAL_TEXT = 'E2E activity reply done'; +const ACTIVITY_PHASE_FINAL_TEXT = 'E2E activity phase reply done'; const STEER_TOOL_NAME_PREFIX = 'remember_fact'; const ASK_USER_QUESTION_TOOL_NAME = 'ask_user_question'; const SLOW_CHUNK_DELAY_MS = Number(process.env.MOCK_LLM_SLOW_CHUNK_DELAY_MS) || 35; @@ -1150,6 +1152,57 @@ function activityReplyResponses(label, toolNames) { }; } +/** + * Three-turn run with two sequential tool batches for the parent activity-phase + * e2e. Each tool invocation produces its own `PostToolBatch`; the final model + * turn then closes a phase containing both logical activities. Keeping the + * batches sequential is essential because two parallel calls are one activity. + */ +function activityPhaseReplyResponses(label, toolNames) { + const toolName = Array.from(toolNames).find((name) => name.startsWith(STEER_TOOL_NAME_PREFIX)); + if (!toolName) { + return { + responses: [ + `E2E activity phase reply unavailable: no ${STEER_TOOL_NAME_PREFIX} tool advertised.`, + ], + }; + } + let invocation = 0; + return { + responses: [''], + resolveInvocation: async () => { + invocation += 1; + if (invocation === 1) { + return { + response: '', + toolCalls: [ + { + id: `call_e2e_activity_phase_alpha_${label}`, + name: toolName, + args: { fact: `activity phase alpha ${label}` }, + type: 'tool_call', + }, + ], + }; + } + if (invocation === 2) { + return { + response: '', + toolCalls: [ + { + id: `call_e2e_activity_phase_beta_${label}`, + name: toolName, + args: { fact: `activity phase beta ${label}` }, + type: 'tool_call', + }, + ], + }; + } + return { response: `${ACTIVITY_PHASE_FINAL_TEXT} ${label}` }; + }, + }; +} + /** * Pause a real agent run at the ask_user_question tool. The resume controller * rebuilds the graph with an empty input-message list, so the test hook selects @@ -2071,6 +2124,11 @@ function resolveResponses({ graph, messages, text, toolNames }) { return activityReplyResponses(activityLabel, toolNames); } + const activityPhaseLabel = getMarkerValue(text, ACTIVITY_PHASE_REPLY_MARKER); + if (activityPhaseLabel) { + return activityPhaseReplyResponses(activityPhaseLabel, toolNames); + } + const askUserQuestionLabel = getMarkerValue(text, ASK_USER_QUESTION_MARKER); if (askUserQuestionLabel) { return askUserQuestionResponses(askUserQuestionLabel, toolNames); diff --git a/e2e/specs/mock/activity-labels.spec.ts b/e2e/specs/mock/activity-labels.spec.ts index d139762cb9..b447953cd0 100644 --- a/e2e/specs/mock/activity-labels.spec.ts +++ b/e2e/specs/mock/activity-labels.spec.ts @@ -2,9 +2,9 @@ import { expect, test } from '@playwright/test'; import type { APIRequestContext, Page } from '@playwright/test'; import { NEW_CHAT_PATH, messagesView, selectMockEndpoint, sendMessage } from './helpers'; -/** The only endpoint with `activityLabel` in e2e/config/librechat.e2e.yaml. It - * is dedicated to this spec: a label auto-collapses its tool group, hiding the - * tool cards other specs assert on. Both are non-spec `addedEndpoints`, the +/** The endpoint dedicated to child-only `activityLabel` coverage. A label + * auto-collapses its tool group, hiding the tool cards other specs assert on. + * Both are non-spec `addedEndpoints`, the * path the ephemeral MCP dropdown rides (mirroring steering.spec.ts) — a * spec-backed endpoint would not surface the selector at all. */ const LABELED_ENDPOINT = { label: 'Mock Provider E', model: 'mock-model-e' }; diff --git a/e2e/specs/mock/activity-phases.spec.ts b/e2e/specs/mock/activity-phases.spec.ts new file mode 100644 index 0000000000..d11a0da28b --- /dev/null +++ b/e2e/specs/mock/activity-phases.spec.ts @@ -0,0 +1,235 @@ +import { expect, test } from '@playwright/test'; +import type { APIRequestContext, Page } from '@playwright/test'; +import { + NEW_CHAT_PATH, + fetchJson, + getAccessToken, + messagesView, + selectMockEndpoint, + sendMessage, +} from './helpers'; + +const PHASE_ENDPOINT = { label: 'Mock Provider F', model: 'mock-model-f' }; +const CHILD_LABEL_MODEL = 'mock-label-model'; +const PHASE_LABEL_MODEL = 'mock-phase-label-model'; +const MCP_SERVER_TITLE = 'E2E Memory'; +const LABEL_SERVER = `http://127.0.0.1:${process.env.E2E_LABEL_PORT || '8889'}`; +const PARENT_LABEL = 'Verified both memory facts across the sequential research phase'; +const FIRST_CHILD_LABEL = 'Recorded the first phase fact in memory'; +const SECOND_CHILD_LABEL = 'Recorded the second phase fact in memory'; + +type LabelRequest = { model?: string; stream: boolean; prompt: string }; + +type PersistedContentPart = { + type?: string; + text?: string | { value?: string }; + error?: string; + activity_label?: string; + activity_label_type?: string; + activity_start_index?: number; + activity_count?: number; + pending?: boolean; + tool_call?: { id?: string }; +}; + +type PersistedMessage = { + messageId: string; + text?: string; + content?: Array; + isCreatedByUser?: boolean; + error?: boolean; + unfinished?: boolean; +}; + +const uniqueLabel = () => `phase-${Date.now()}-${Math.floor(Math.random() * 1e4)}`; + +function phaseChildLabels(label: string) { + return { + first: `${FIRST_CHILD_LABEL} ${label}`, + second: `${SECOND_CHILD_LABEL} ${label}`, + }; +} + +async function resetLabelServer(request: APIRequestContext) { + const response = await request.post(`${LABEL_SERVER}/__e2e/reset`); + expect(response.ok()).toBeTruthy(); +} + +async function setPhaseLabels(request: APIRequestContext, label: string) { + const childLabels = phaseChildLabels(label); + const response = await request.post(`${LABEL_SERVER}/__e2e/behavior`, { + data: { + phaseLabel: PARENT_LABEL, + labelsByPrompt: { + [`activity phase alpha ${label}`]: childLabels.first, + [`activity phase beta ${label}`]: childLabels.second, + }, + }, + }); + expect(response.ok()).toBeTruthy(); +} + +async function getLabelRequests(request: APIRequestContext): Promise { + const response = await request.get(`${LABEL_SERVER}/__e2e/requests`); + expect(response.ok()).toBeTruthy(); + return (await response.json()).requests as LabelRequest[]; +} + +async function getLabelRequestsFor( + request: APIRequestContext, + label: string, +): Promise { + return (await getLabelRequests(request)).filter((entry) => entry.prompt.includes(label)); +} + +async function selectEphemeralMCP(page: Page) { + await page.getByRole('button', { name: 'MCP Servers', exact: true }).click(); + const serverItem = page.getByRole('menuitemcheckbox', { name: new RegExp(MCP_SERVER_TITLE) }); + await expect(serverItem).toBeVisible(); + await serverItem.click(); + await expect(serverItem).toHaveAttribute('aria-checked', 'true'); + await page.keyboard.press('Escape'); + await expect(page.getByRole('button', { name: new RegExp(MCP_SERVER_TITLE) })).toBeVisible(); +} + +function contentPartText(part: PersistedContentPart | null): string { + if (!part) { + return ''; + } + if (typeof part.text === 'string') { + return part.text; + } + if (typeof part.text?.value === 'string') { + return part.text.value; + } + if (typeof part.activity_label === 'string') { + return part.activity_label; + } + return part.error ?? ''; +} + +function messageText(message: PersistedMessage): string { + return [message.text, ...(message.content?.map(contentPartText) ?? [])] + .filter((value): value is string => Boolean(value)) + .join('\n'); +} + +async function getConversationId(page: Page): Promise { + await expect(page).toHaveURL(/\/c\/(?!new)[0-9a-fA-F-]{36}$/, { timeout: 15000 }); + const conversationId = new URL(page.url()).pathname.split('/').pop(); + if (!conversationId) { + throw new Error(`Could not parse conversation id from ${page.url()}`); + } + return conversationId; +} + +test.describe('parent activity phases', () => { + test.beforeEach(async ({ request }) => { + await resetLabelServer(request); + }); + + test('renders and persists two sequential activities under a clean parent phase', async ({ + page, + request, + }) => { + test.setTimeout(120000); + const label = uniqueLabel(); + const finalText = `E2E activity phase reply done ${label}`; + const firstToolCallId = `call_e2e_activity_phase_alpha_${label}`; + const secondToolCallId = `call_e2e_activity_phase_beta_${label}`; + const childLabels = phaseChildLabels(label); + await setPhaseLabels(request, label); + + await page.goto(NEW_CHAT_PATH, { timeout: 10000 }); + await selectMockEndpoint(page, PHASE_ENDPOINT); + await selectEphemeralMCP(page); + const run = await sendMessage(page, `E2E_ACTIVITY_PHASE_REPLY:${label}`); + expect(run.ok()).toBeTruthy(); + + const parent = messagesView(page).locator(`summary[aria-label="${PARENT_LABEL}"]`); + await expect(parent).toBeVisible({ timeout: 60000 }); + await expect(messagesView(page).getByText(finalText)).toBeVisible({ timeout: 60000 }); + await parent.click(); + await expect(messagesView(page).getByRole('button', { name: childLabels.first })).toBeVisible(); + await expect( + messagesView(page).getByRole('button', { name: childLabels.second }), + ).toBeVisible(); + + await expect.poll(async () => (await getLabelRequestsFor(request, label)).length).toBe(3); + const labelRequests = await getLabelRequestsFor(request, label); + const phaseRequest = labelRequests.find((entry) => entry.model === PHASE_LABEL_MODEL); + const childRequests = labelRequests.filter((entry) => entry !== phaseRequest); + expect(phaseRequest).toMatchObject({ model: PHASE_LABEL_MODEL, stream: false }); + expect(childRequests).toHaveLength(2); + expect(childRequests.map((entry) => entry.model)).toEqual([ + CHILD_LABEL_MODEL, + CHILD_LABEL_MODEL, + ]); + expect( + childRequests.some((entry) => entry.prompt.includes(`activity phase alpha ${label}`)), + ).toBe(true); + expect( + childRequests.some((entry) => entry.prompt.includes(`activity phase beta ${label}`)), + ).toBe(true); + + const conversationId = await getConversationId(page); + const token = await getAccessToken(page); + let assistant: PersistedMessage | undefined; + await expect + .poll( + async () => { + const messages = await fetchJson( + page, + `/api/messages/${encodeURIComponent(conversationId)}`, + token, + ); + assistant = messages.find( + (message) => + message.isCreatedByUser === false && messageText(message).includes(finalText), + ); + return assistant?.unfinished; + }, + { timeout: 30000 }, + ) + .toBe(false); + + expect(assistant).toBeDefined(); + expect(assistant?.error).not.toBe(true); + const content = assistant?.content ?? []; + expect(content.some((part) => part?.type === 'error')).toBe(false); + const phaseIndex = content.findIndex( + (part) => part?.type === 'activity_label' && part.activity_label_type === 'phase', + ); + expect(phaseIndex).toBeGreaterThanOrEqual(0); + const phasePart = content[phaseIndex]; + expect(phasePart).toMatchObject({ + type: 'activity_label', + activity_label: PARENT_LABEL, + activity_label_type: 'phase', + activity_count: 2, + pending: false, + }); + expect(phasePart?.activity_start_index).toBeGreaterThanOrEqual(0); + expect(phasePart?.activity_start_index).toBeLessThan(phaseIndex); + const phaseChildren = content.slice(phasePart?.activity_start_index ?? phaseIndex, phaseIndex); + expect(phaseChildren.map((part) => part?.tool_call?.id).filter(Boolean)).toEqual( + expect.arrayContaining([firstToolCallId, secondToolCallId]), + ); + expect(phaseChildren.map(contentPartText)).toEqual( + expect.arrayContaining([childLabels.first, childLabels.second]), + ); + const finalTextIndex = content.findIndex((part) => contentPartText(part).includes(finalText)); + expect(finalTextIndex).toBeGreaterThan(phaseIndex); + + await page.reload(); + const reloadedParent = messagesView(page).locator(`summary[aria-label="${PARENT_LABEL}"]`); + await expect(reloadedParent).toBeVisible({ timeout: 30000 }); + await expect(messagesView(page).getByText(finalText)).toBeVisible(); + await reloadedParent.click(); + await expect(messagesView(page).getByRole('button', { name: childLabels.first })).toBeVisible(); + await expect( + messagesView(page).getByRole('button', { name: childLabels.second }), + ).toBeVisible(); + expect(await getLabelRequestsFor(request, label)).toHaveLength(3); + }); +}); diff --git a/packages/api/src/agents/activityPhases/runtime.spec.ts b/packages/api/src/agents/activityPhases/runtime.spec.ts index a529e0fb3c..3c20c65d28 100644 --- a/packages/api/src/agents/activityPhases/runtime.spec.ts +++ b/packages/api/src/agents/activityPhases/runtime.spec.ts @@ -105,6 +105,221 @@ describe('createActivityPhaseWiring', () => { expect(emitLabelEvent).toHaveBeenCalledTimes(2); }); + it('reanchors a tool that lands after the phase hook observes its child label', async () => { + const parts: LooseContentPart[] = []; + const wiring = createActivityPhaseWiring({ + getContentParts: () => parts, + bumpIndexOffset: jest.fn(), + emitLabelEvent: jest.fn(async () => undefined), + trackPendingFill: jest.fn(), + generatePhase: jest.fn(async () => ({ label: 'Verified both delayed tool results' })), + }); + + /** The child-label hook can synchronously reserve its slot before the + * tool event reaches the shared content array. A tool-only provider turn + * can also leave an empty final-answer part between the tool and label; + * that invisible boundary must not strand the tool outside the phase. */ + parts[1] = { type: ContentTypes.TEXT, text: '', phase: 'final_answer' }; + parts[2] = { + type: ContentTypes.ACTIVITY_LABEL, + activity_label: 'Recorded the first delayed result', + tool_call_ids: ['tool-1'], + pending: false, + }; + await wiring.hook(batch('tool-1'), new AbortController().signal); + + parts[3] = { type: ContentTypes.TOOL_CALL, tool_call: { id: 'tool-2' } }; + parts[4] = { + type: ContentTypes.ACTIVITY_LABEL, + activity_label: 'Recorded the second delayed result', + tool_call_ids: ['tool-2'], + pending: false, + }; + await wiring.hook(batch('tool-2'), new AbortController().signal); + + wiring + .handlers({ [GraphEvents.ON_RUN_STEP]: { handle: jest.fn() } }) + ?.[GraphEvents.ON_RUN_STEP]?.handle( + GraphEvents.ON_RUN_STEP, + { + id: 'final-step', + stepDetails: { + type: StepTypes.MESSAGE_CREATION, + message_creation: { message_id: 'm', content_type: 'text', phase: 'final_answer' }, + }, + }, + undefined, + undefined, + ); + + expect(parts[5]).toMatchObject({ + activity_label_type: 'phase', + activity_start_index: 0, + activity_count: 2, + }); + parts[0] = { type: ContentTypes.TOOL_CALL, tool_call: { id: 'tool-1' } }; + expect(parts.slice(0, 5)).toEqual( + expect.arrayContaining([ + expect.objectContaining({ tool_call: { id: 'tool-1' } }), + expect.objectContaining({ tool_call: { id: 'tool-2' } }), + ]), + ); + }); + + it('does not claim a visible final answer for a later parent phase', async () => { + const parts: LooseContentPart[] = [ + { type: ContentTypes.TEXT, text: 'Earlier final answer', phase: 'final_answer' }, + ]; + const wiring = createActivityPhaseWiring({ + getContentParts: () => parts, + bumpIndexOffset: jest.fn(), + emitLabelEvent: jest.fn(async () => undefined), + trackPendingFill: jest.fn(), + generatePhase: jest.fn(async () => ({ label: 'Verified the later tool results' })), + }); + + parts[2] = { + type: ContentTypes.ACTIVITY_LABEL, + activity_label: 'Recorded the first later result', + tool_call_ids: ['tool-1'], + pending: false, + }; + await wiring.hook(batch('tool-1'), new AbortController().signal); + parts[3] = { type: ContentTypes.TOOL_CALL, tool_call: { id: 'tool-2' } }; + await wiring.hook(batch('tool-2'), new AbortController().signal); + + wiring + .handlers({ [GraphEvents.ON_RUN_STEP]: { handle: jest.fn() } }) + ?.[GraphEvents.ON_RUN_STEP]?.handle( + GraphEvents.ON_RUN_STEP, + { + id: 'final-step', + stepDetails: { + type: StepTypes.MESSAGE_CREATION, + message_creation: { message_id: 'm', content_type: 'text', phase: 'final_answer' }, + }, + }, + undefined, + undefined, + ); + + expect(parts[4]).toMatchObject({ + activity_label_type: 'phase', + activity_start_index: 1, + activity_count: 2, + }); + parts[1] = { type: ContentTypes.TOOL_CALL, tool_call: { id: 'tool-1' } }; + expect(parts.slice(1, 4)).toEqual( + expect.arrayContaining([ + expect.objectContaining({ tool_call: { id: 'tool-1' } }), + expect.objectContaining({ tool_call: { id: 'tool-2' } }), + ]), + ); + }); + + it('does not use repeated reasoning to reanchor a missing tool across a phase', async () => { + const repeatedReasoning = 'Compared the same deployment paths.'; + const parts: LooseContentPart[] = [ + { type: ContentTypes.THINK, think: repeatedReasoning }, + { + type: ContentTypes.ACTIVITY_LABEL, + activity_label: '', + activity_label_type: 'phase', + activity_start_index: 0, + activity_count: 2, + pending: false, + }, + ]; + const wiring = createActivityPhaseWiring({ + getContentParts: () => parts, + getStepIndex: (stepId) => { + if (stepId === 'missing-tool-reasoning') return 2; + if (stepId === 'current-reasoning') return 4; + return undefined; + }, + bumpIndexOffset: jest.fn(), + emitLabelEvent: jest.fn(async () => undefined), + trackPendingFill: jest.fn(), + generatePhase: jest.fn(async () => ({ label: 'Verified the current deployment path' })), + }); + const handlers = wiring.handlers({ + [GraphEvents.ON_RUN_STEP]: { handle: jest.fn() }, + [GraphEvents.ON_REASONING_DELTA]: { handle: jest.fn() }, + }); + + handlers?.[GraphEvents.ON_RUN_STEP]?.handle( + GraphEvents.ON_RUN_STEP, + { + id: 'missing-tool-reasoning', + stepDetails: { + type: StepTypes.MESSAGE_CREATION, + message_creation: { message_id: 'm', content_type: 'think' }, + }, + }, + undefined, + undefined, + ); + parts[2] = { type: ContentTypes.THINK, think: repeatedReasoning }; + handlers?.[GraphEvents.ON_REASONING_DELTA]?.handle( + GraphEvents.ON_REASONING_DELTA, + { + id: 'missing-tool-reasoning', + delta: { content: { type: ContentTypes.THINK, think: repeatedReasoning } }, + }, + undefined, + undefined, + ); + parts[3] = { + type: ContentTypes.ACTIVITY_LABEL, + activity_label: 'Recorded a result before its tool arrived', + tool_call_ids: ['missing-tool'], + pending: false, + }; + await wiring.hook(batch('missing-tool'), new AbortController().signal); + + handlers?.[GraphEvents.ON_RUN_STEP]?.handle( + GraphEvents.ON_RUN_STEP, + { + id: 'current-reasoning', + stepDetails: { + type: StepTypes.MESSAGE_CREATION, + message_creation: { message_id: 'm', content_type: 'think' }, + }, + }, + undefined, + undefined, + ); + parts[4] = { type: ContentTypes.THINK, think: repeatedReasoning }; + handlers?.[GraphEvents.ON_REASONING_DELTA]?.handle( + GraphEvents.ON_REASONING_DELTA, + { + id: 'current-reasoning', + delta: { content: { type: ContentTypes.THINK, think: repeatedReasoning } }, + }, + undefined, + undefined, + ); + + handlers?.[GraphEvents.ON_RUN_STEP]?.handle( + GraphEvents.ON_RUN_STEP, + { + id: 'final-step', + stepDetails: { + type: StepTypes.MESSAGE_CREATION, + message_creation: { message_id: 'm', content_type: 'text', phase: 'final_answer' }, + }, + }, + undefined, + undefined, + ); + + expect(parts[5]).toMatchObject({ + activity_label_type: 'phase', + activity_start_index: 2, + activity_count: 2, + }); + }); + it('does not spend a phase call on one logical activity', async () => { const parts: LooseContentPart[] = [ { type: ContentTypes.TOOL_CALL, tool_call: { id: 'tool-1' } }, diff --git a/packages/api/src/agents/activityPhases/runtime.ts b/packages/api/src/agents/activityPhases/runtime.ts index 577a896230..613b9d5bd3 100644 --- a/packages/api/src/agents/activityPhases/runtime.ts +++ b/packages/api/src/agents/activityPhases/runtime.ts @@ -194,14 +194,9 @@ function findTrackedStart( parts: ReadonlyArray, activity: TrackedActivity, ): number { - if (activity.toolCallIds != null && activity.toolCallIds.length > 0) { - const toolStart = findBatchStart(parts, new Set(activity.toolCallIds)); - if ( - parts[toolStart]?.type === ContentTypes.TOOL_CALL && - activity.toolCallIds.includes(String(parts[toolStart]?.tool_call?.id ?? '')) - ) { - return toolStart; - } + const toolStart = findTrackedToolStart(parts, activity); + if (toolStart != null) { + return toolStart; } const excerpt = activity.thinkingExcerpts?.[0]?.trim(); if (excerpt) { @@ -215,6 +210,22 @@ function findTrackedStart( return Math.min(activity.startIndex, Math.max(0, parts.length - 1)); } +function findTrackedToolStart( + parts: ReadonlyArray, + activity: TrackedActivity, +): number | undefined { + if (activity.toolCallIds != null && activity.toolCallIds.length > 0) { + const toolStart = findBatchStart(parts, new Set(activity.toolCallIds)); + if ( + parts[toolStart]?.type === ContentTypes.TOOL_CALL && + activity.toolCallIds.includes(String(parts[toolStart]?.tool_call?.id ?? '')) + ) { + return toolStart; + } + } + return undefined; +} + function findReasoningStart( parts: ReadonlyArray, text: string, @@ -458,13 +469,22 @@ export function createActivityPhaseWiring(deps: ActivityPhaseHostDeps): Activity generated += 1; const phaseIndex = generated - 1; - const snapshot = [...activities]; + const currentParts = deps.getContentParts(); + /** Child-label and phase hooks run independently. A child label can claim + * its slot before the corresponding tool part reaches the shared content + * array, leaving the phase hook with a fallback start index. Re-anchor + * from stable tool ids when they are available at close; the backward + * scan below claims unresolved leading slots without crossing visible + * answer content. */ + const snapshot = activities.map((activity) => { + const toolStart = findTrackedToolStart(currentParts, activity); + return toolStart != null ? { ...activity, startIndex: toolStart } : activity; + }); const contextSnapshot = [...assistantContext]; const totalActivityCount = activityCount; const failedCount = failedActivityCount; const partialCount = partialActivityCount; let startIndex = Math.min(...snapshot.map((activity) => activity.startIndex)); - const currentParts = deps.getContentParts(); /** Pull leading commentary/reasoning into the parent card. A prior phase * marker or steer is the only hard UI boundary; plain text can be * intermediate context on providers that do not expose phase metadata. */ @@ -473,7 +493,9 @@ export function createActivityPhaseWiring(deps: ActivityPhaseHostDeps): Activity if ( prior?.type === ContentTypes.STEER || (prior?.type === ContentTypes.ACTIVITY_LABEL && prior.activity_label_type === 'phase') || - (prior?.type === ContentTypes.TEXT && prior.phase === 'final_answer') + (prior?.type === ContentTypes.TEXT && + prior.phase === 'final_answer' && + textValue(prior.text).trim().length > 0) ) { break; }