Skip to content

Commit 79aaa97

Browse files
authored
[OPIK-7421] test: add Ollie explain E2E release-gate test (#7564)
* test for Explain Ollie feature * change tier of testing * fix duplication * fix baz comments
1 parent 2735c6d commit 79aaa97

8 files changed

Lines changed: 426 additions & 35 deletions

File tree

tests_end_to_end/e2e/core/sdk/python-sdk-client.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ export interface PythonSdkClient {
1717
tags?: string[];
1818
thread_id?: string;
1919
feedback_scores?: Array<{ name: string; value: number; reason?: string }>;
20+
error_info?: { exception_type: string; message: string; traceback?: string };
21+
duration_seconds?: number;
2022
spans: Array<{
2123
name: string;
2224
type?: 'general' | 'llm' | 'tool';
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
import { test as baseTest } from './annotation-queue.fixture';
2+
3+
export interface ExplainTraceRef {
4+
id: string;
5+
name: string;
6+
errorType: string;
7+
/** Seeded trace duration in seconds; null means left unset (renders as Duration "NA"). */
8+
durationSeconds: number | null;
9+
/** Seeded LLM-span cost; null means no cost-bearing span (renders as Cost "-"). */
10+
cost: number | null;
11+
/**
12+
* Source of a case-insensitive RegExp expected to appear in Ollie's error
13+
* explanation for this trace. Ollie's phrasing is non-deterministic (see
14+
* ollie-explain.spec.ts), so this is anchored to concrete details of the
15+
* seeded error (exception type / message) rather than generic
16+
* error-adjacent vocabulary ("error", "fail", ...) — those turned out to be
17+
* skippable depending on phrasing (e.g. a rate-limit explanation that never
18+
* says "fail" or "exceed"), while the concrete subject (rate limit,
19+
* document store, timeout, permissions, faiss) reliably recurs because
20+
* Ollie is grounded in that seeded content.
21+
*/
22+
errorKeywordSource: string;
23+
}
24+
25+
export interface ExplainTracesFixtures {
26+
explainTraces: ExplainTraceRef[];
27+
}
28+
29+
/**
30+
* Five traces shaped for the Ollie "Explain" cell feature: each has a distinct
31+
* trace-level error, and distinct duration/cost, with one trace leaving both
32+
* duration and cost unset so the explain button's N/A path (Duration "NA",
33+
* Cost "-") gets covered too — per `explainTargets.ts`, those cells stay
34+
* explainable even at N/A, unlike the error cell which vetoes on no error.
35+
*/
36+
const SEEDS: Array<{
37+
suffix: string;
38+
errorType: string;
39+
errorMessage: string;
40+
/** RegExp source (case-insensitive) expected in Ollie's explanation of this error. */
41+
errorKeyword: string;
42+
durationSeconds: number | null;
43+
cost: number | null;
44+
}> = [
45+
{
46+
suffix: 'rate-limit',
47+
errorType: 'RuntimeError',
48+
errorMessage: 'Model returned status 429: rate limit exceeded',
49+
errorKeyword: 'rate.?limit',
50+
durationSeconds: 2,
51+
cost: 0.0005,
52+
},
53+
{
54+
suffix: 'missing-context',
55+
errorType: 'ValueError',
56+
errorMessage: 'Missing required context: document store unavailable',
57+
errorKeyword: 'context|document store',
58+
durationSeconds: 15,
59+
cost: 0.02,
60+
},
61+
{
62+
suffix: 'tool-timeout',
63+
errorType: 'TimeoutError',
64+
errorMessage: 'Tool call timed out after 30 s',
65+
errorKeyword: 'timeout',
66+
durationSeconds: 60,
67+
cost: 0.5,
68+
},
69+
{
70+
suffix: 'auth-failure',
71+
errorType: 'PermissionError',
72+
errorMessage: 'API key does not have access to model claude-3-opus',
73+
errorKeyword: 'permission|access|api key',
74+
durationSeconds: 180,
75+
cost: 2.0,
76+
},
77+
{
78+
suffix: 'quota-exceeded',
79+
errorType: 'ImportError',
80+
errorMessage: "Required dependency 'faiss' is not installed",
81+
errorKeyword: 'faiss|install|depend',
82+
durationSeconds: null,
83+
cost: null,
84+
},
85+
];
86+
87+
export const test = baseTest.extend<ExplainTracesFixtures>({
88+
explainTraces: async ({ sdkClient, project, testNamespace }, use, testInfo) => {
89+
const refs: ExplainTraceRef[] = [];
90+
for (const seed of SEEDS) {
91+
const name = `${testNamespace}-explain-${seed.suffix}`;
92+
const spans =
93+
seed.cost === null
94+
? []
95+
: [
96+
{
97+
name: 'llm-call',
98+
type: 'llm' as const,
99+
model: 'gpt-4o',
100+
provider: 'openai',
101+
input: { prompt: 'seed prompt' },
102+
output: { completion: 'seed completion' },
103+
usage: { prompt_tokens: 10, completion_tokens: 5, total_tokens: 15 },
104+
total_cost: seed.cost,
105+
},
106+
];
107+
const created = await sdkClient.python.createNestedTrace({
108+
project_name: project.name,
109+
name,
110+
input: { user: `trigger for ${seed.suffix}` },
111+
tags: ['explain', seed.suffix],
112+
error_info: {
113+
exception_type: seed.errorType,
114+
message: seed.errorMessage,
115+
},
116+
duration_seconds: seed.durationSeconds ?? undefined,
117+
spans,
118+
});
119+
refs.push({
120+
id: created.id,
121+
name: created.name,
122+
errorType: seed.errorType,
123+
durationSeconds: seed.durationSeconds,
124+
cost: seed.cost,
125+
errorKeywordSource: seed.errorKeyword,
126+
});
127+
}
128+
129+
await testInfo.attach('opik.explainTraces', {
130+
body: JSON.stringify(refs, null, 2),
131+
contentType: 'application/json',
132+
});
133+
134+
await use(refs);
135+
// No explicit teardown — the project fixture's deleteProject cascades.
136+
},
137+
});
138+
139+
export { expect } from './annotation-queue.fixture';

tests_end_to_end/e2e/fixtures/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
export { test, expect } from './annotation-queue.fixture';
1+
export { test, expect } from './explain-traces.fixture';
22
export type { ProjectFixtures } from './project.fixture';
33
export type { ScratchDir, ScratchDirFixtures } from './scratch-dir.fixture';
44
export type {
@@ -45,4 +45,5 @@ export type {
4545
AnnotationQueueTraceRef,
4646
AnnotationQueueFixtures,
4747
} from './annotation-queue.fixture';
48+
export type { ExplainTraceRef, ExplainTracesFixtures } from './explain-traces.fixture';
4849
export type { ProjectRef } from '../core/backend';

tests_end_to_end/e2e/pom/logs.page.ts

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,23 @@
1-
import { test, type Page, type Locator } from '@playwright/test';
1+
import { test, expect, type Page, type Locator } from '@playwright/test';
22
import { loadEnvConfig } from '../config/env.config';
33
import { TracePanelPage } from './trace-panel.page';
44
import { ThreadPanelPage } from './thread-panel.page';
55

6+
export type ExplainKind = 'error' | 'duration' | 'cost';
7+
8+
// Maps an explain kind to the Traces table column id (used in data-cell-id)
9+
// and the owl trigger's aria-label, per apps/opik-frontend/src/plugins/comet/explain/registry.ts.
10+
const EXPLAIN_COLUMN: Record<ExplainKind, string> = {
11+
error: 'error_info',
12+
duration: 'duration',
13+
cost: 'total_estimated_cost',
14+
};
15+
const EXPLAIN_LABEL: Record<ExplainKind, string> = {
16+
error: 'Explain error',
17+
duration: 'Explain duration',
18+
cost: 'Explain cost',
19+
};
20+
621
export class LogsPage {
722
private projectId: string | null = null;
823

@@ -116,6 +131,80 @@ export class LogsPage {
116131
return this.page.locator('tr[data-row-id]');
117132
}
118133

134+
/** The Errors/Duration/Estimated cost cell for a trace row, keyed by Ollie explain kind. */
135+
explainCell(traceId: string, kind: ExplainKind): Locator {
136+
return this.page.locator(`[data-cell-id="${traceId}_${EXPLAIN_COLUMN[kind]}"]`);
137+
}
138+
139+
/**
140+
* Hover a trace's Errors/Duration/Estimated cost cell and click its Ollie
141+
* "Explain" owl trigger, opening the popover. The trigger only renders once
142+
* the Ollie assistant bridge handshake (mounted via the page's assistant
143+
* sidebar) completes, which can lag a beat after the table itself is
144+
* interactive — so this polls hover+lookup rather than asserting once.
145+
*/
146+
async openExplain(traceId: string, kind: ExplainKind, timeoutMs = 60_000): Promise<void> {
147+
return test.step(`open Ollie explain (${kind}) for trace ${traceId}`, async () => {
148+
const cell = this.explainCell(traceId, kind);
149+
const button = cell.getByRole('button', { name: EXPLAIN_LABEL[kind] });
150+
await expect
151+
.poll(
152+
async () => {
153+
await cell.hover();
154+
return button.count();
155+
},
156+
{ timeout: timeoutMs, intervals: [500, 1000, 2000] },
157+
)
158+
.toBeGreaterThan(0);
159+
await button.click();
160+
});
161+
}
162+
163+
/**
164+
* Wait for the open Ollie explain popover to settle (loading -> done/error)
165+
* and return its rendered text. Scoped to the last `[role="status"]` live
166+
* region on the page — Radix unmounts a closed popover's content, so only
167+
* the currently-open one's region should be present.
168+
*/
169+
async readExplanation(timeoutMs = 60_000): Promise<string> {
170+
return test.step('wait for Ollie explain popover to settle', async () => {
171+
const status = this.page.locator('[role="status"]').last();
172+
await expect(status).toHaveAttribute('aria-busy', 'false', { timeout: timeoutMs });
173+
const text = ((await status.textContent()) ?? '').trim();
174+
if (!text) {
175+
throw new Error('Ollie explain popover settled but rendered no text');
176+
}
177+
return text;
178+
});
179+
}
180+
181+
/** Close the open Ollie explain popover. */
182+
async closeExplain(): Promise<void> {
183+
return test.step('close Ollie explain popover', async () => {
184+
await this.page.keyboard.press('Escape');
185+
});
186+
}
187+
188+
/**
189+
* The "Continue conversation" link in the currently open Ollie explain
190+
* popover. Only rendered once the popover has settled with text (see
191+
* ExplainPopover.tsx) — call after `readExplanation()`.
192+
*/
193+
continueConversationButton(): Locator {
194+
return this.page.getByRole('button', { name: 'Continue conversation' });
195+
}
196+
197+
/**
198+
* Click "Continue conversation" to hand the explain popover's question +
199+
* cached answer off to the Ollie sidebar chat. This closes the popover as
200+
* a side effect (see ExplainPopover's onContinue).
201+
*/
202+
async continueConversation(): Promise<void> {
203+
return test.step('continue the Ollie explain conversation in the sidebar', async () => {
204+
await this.continueConversationButton().click();
205+
});
206+
}
207+
119208
// --- Threads tab ---
120209

121210
/** The Threads/Traces/Spans tab toggle for "Threads". */

tests_end_to_end/e2e/pom/ollie.page.ts

Lines changed: 47 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -68,24 +68,8 @@ export class OlliePage {
6868
await this.sendButton().click();
6969

7070
// The user echo and the assistant reply each mount as a [data-message-id]
71-
// node. Wait for both to land (count grows by 2), then for the reply's
72-
// text to be non-empty (it streams in after the bubble appears).
73-
await expect
74-
.poll(async () => this.messages().count(), {
75-
timeout: timeoutMs,
76-
intervals: [500, 1000, 2000],
77-
})
78-
.toBeGreaterThanOrEqual(before + 2);
79-
80-
const reply = this.messages().last();
81-
await expect
82-
.poll(async () => ((await reply.textContent()) ?? '').trim().length, {
83-
timeout: timeoutMs,
84-
intervals: [500, 1000, 2000],
85-
})
86-
.toBeGreaterThan(0);
87-
88-
return ((await reply.textContent()) ?? '').trim();
71+
// node, so wait for both to land (count grows by 2).
72+
return this.awaitNewMessage(before + 2, timeoutMs, [500, 1000, 2000]);
8973
});
9074
}
9175

@@ -128,6 +112,21 @@ export class OlliePage {
128112
});
129113
}
130114

115+
/**
116+
* Wait for an Explain popover's "Continue conversation" hand-off to land in
117+
* the chat. The bridge posts the question and the popover's already-settled
118+
* answer as a pair of new messages (see `chat:continue` in explainStore.ts
119+
* — "carries the verbatim Q&A already shown"), not a fresh generation, so
120+
* this only waits for them to render, not for streaming. `beforeCount` is
121+
* the message count read just before clicking "Continue conversation".
122+
* Returns the last message's (the answer's) text.
123+
*/
124+
async awaitContinuedConversation(beforeCount: number, timeoutMs = 30_000): Promise<string> {
125+
return test.step('wait for the continued conversation to render in the sidebar', async () => {
126+
return this.awaitNewMessage(beforeCount + 2, timeoutMs, [300, 600, 1200]);
127+
});
128+
}
129+
131130
/**
132131
* Run the `/analyze` flow from the greeting action button and wait for a
133132
* non-empty assistant response to render. Ollie is a non-deterministic agent,
@@ -138,21 +137,7 @@ export class OlliePage {
138137
return test.step('run /analyze and await a response', async () => {
139138
const before = await this.messages().count();
140139
await this.analyzeButton().click();
141-
await expect
142-
.poll(async () => this.messages().count(), {
143-
timeout: timeoutMs,
144-
intervals: [1000, 2000, 5000],
145-
})
146-
.toBeGreaterThan(before);
147-
148-
const reply = this.messages().last();
149-
await expect
150-
.poll(async () => ((await reply.textContent()) ?? '').trim().length, {
151-
timeout: timeoutMs,
152-
intervals: [1000, 2000, 5000],
153-
})
154-
.toBeGreaterThan(0);
155-
return ((await reply.textContent()) ?? '').trim();
140+
return this.awaitNewMessage(before + 1, timeoutMs, [1000, 2000, 5000]);
156141
});
157142
}
158143

@@ -304,6 +289,35 @@ export class OlliePage {
304289

305290
// ── private helpers ─────────────────────────────────────────────────────
306291

292+
/**
293+
* Wait for the message list to reach `minCount` messages, then for the last
294+
* one's text to be non-empty (it streams in after the bubble appears).
295+
* Returns the last message's (the reply's) text.
296+
*/
297+
private async awaitNewMessage(
298+
minCount: number,
299+
timeoutMs: number,
300+
intervals: number[],
301+
): Promise<string> {
302+
return test.step('wait for a new Ollie message to render', async () => {
303+
const deadline = Date.now() + timeoutMs;
304+
305+
await expect
306+
.poll(async () => this.messages().count(), { timeout: timeoutMs, intervals })
307+
.toBeGreaterThanOrEqual(minCount);
308+
309+
const reply = this.messages().last();
310+
await expect
311+
.poll(async () => ((await reply.textContent()) ?? '').trim().length, {
312+
timeout: Math.max(0, deadline - Date.now()),
313+
intervals,
314+
})
315+
.toBeGreaterThan(0);
316+
317+
return ((await reply.textContent()) ?? '').trim();
318+
});
319+
}
320+
307321
// Anchor on the testid added to the host iframe in AssistantSidebar.tsx, OR
308322
// on the `title="Assistant"` attribute already present on deployed builds.
309323
// The `,` selector keeps the POM working against a cloud env running an older

0 commit comments

Comments
 (0)