mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-27 04:07:05 +00:00
🧭 fix: Re-Anchor Parent Activity Phase Bounds (#14741)
* test: cover parent activity phase finalization * test(e2e): stabilize parent phase coverage * fix(agents): reanchor parent activity phase bounds * fix(agents): preserve delayed tools in activity phases * test(agents): keep phase slice bounds typed * fix(agents): preserve sparse activity phase bounds * test(e2e): read structured phase replies
This commit is contained in:
parent
ba29a6c5d6
commit
236ee6c1ab
9 changed files with 645 additions and 20 deletions
|
|
@ -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];
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -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'
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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' };
|
||||
|
|
|
|||
235
e2e/specs/mock/activity-phases.spec.ts
Normal file
235
e2e/specs/mock/activity-phases.spec.ts
Normal file
|
|
@ -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<PersistedContentPart | null>;
|
||||
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<LabelRequest[]> {
|
||||
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<LabelRequest[]> {
|
||||
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<string> {
|
||||
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<PersistedMessage[]>(
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
|
@ -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' } },
|
||||
|
|
|
|||
|
|
@ -194,14 +194,9 @@ function findTrackedStart(
|
|||
parts: ReadonlyArray<LooseContentPart | null | undefined>,
|
||||
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<LooseContentPart | null | undefined>,
|
||||
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<LooseContentPart | null | undefined>,
|
||||
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;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue