Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions tests_end_to_end/e2e/core/sdk/python-sdk-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ export interface PythonSdkClient {
tags?: string[];
thread_id?: string;
feedback_scores?: Array<{ name: string; value: number; reason?: string }>;
error_info?: { exception_type: string; message: string; traceback?: string };
duration_seconds?: number;
spans: Array<{
name: string;
type?: 'general' | 'llm' | 'tool';
Expand Down
139 changes: 139 additions & 0 deletions tests_end_to_end/e2e/fixtures/explain-traces.fixture.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
import { test as baseTest } from './annotation-queue.fixture';

export interface ExplainTraceRef {
id: string;
name: string;
errorType: string;
/** Seeded trace duration in seconds; null means left unset (renders as Duration "NA"). */
durationSeconds: number | null;
/** Seeded LLM-span cost; null means no cost-bearing span (renders as Cost "-"). */
cost: number | null;
/**
* Source of a case-insensitive RegExp expected to appear in Ollie's error
* explanation for this trace. Ollie's phrasing is non-deterministic (see
* ollie-explain.spec.ts), so this is anchored to concrete details of the
* seeded error (exception type / message) rather than generic
* error-adjacent vocabulary ("error", "fail", ...) — those turned out to be
* skippable depending on phrasing (e.g. a rate-limit explanation that never
* says "fail" or "exceed"), while the concrete subject (rate limit,
* document store, timeout, permissions, faiss) reliably recurs because
* Ollie is grounded in that seeded content.
*/
errorKeywordSource: string;
}

export interface ExplainTracesFixtures {
explainTraces: ExplainTraceRef[];
}

/**
* Five traces shaped for the Ollie "Explain" cell feature: each has a distinct
* trace-level error, and distinct duration/cost, with one trace leaving both
* duration and cost unset so the explain button's N/A path (Duration "NA",
* Cost "-") gets covered too — per `explainTargets.ts`, those cells stay
* explainable even at N/A, unlike the error cell which vetoes on no error.
*/
const SEEDS: Array<{
suffix: string;
errorType: string;
errorMessage: string;
/** RegExp source (case-insensitive) expected in Ollie's explanation of this error. */
errorKeyword: string;
durationSeconds: number | null;
cost: number | null;
}> = [
{
suffix: 'rate-limit',
errorType: 'RuntimeError',
errorMessage: 'Model returned status 429: rate limit exceeded',
errorKeyword: 'rate.?limit',
durationSeconds: 2,
cost: 0.0005,
},
{
suffix: 'missing-context',
errorType: 'ValueError',
errorMessage: 'Missing required context: document store unavailable',
errorKeyword: 'context|document store',
durationSeconds: 15,
cost: 0.02,
},
{
suffix: 'tool-timeout',
errorType: 'TimeoutError',
errorMessage: 'Tool call timed out after 30 s',
errorKeyword: 'timeout',
durationSeconds: 60,
cost: 0.5,
},
{
suffix: 'auth-failure',
errorType: 'PermissionError',
errorMessage: 'API key does not have access to model claude-3-opus',
errorKeyword: 'permission|access|api key',
durationSeconds: 180,
cost: 2.0,
},
{
suffix: 'quota-exceeded',
errorType: 'ImportError',
errorMessage: "Required dependency 'faiss' is not installed",
errorKeyword: 'faiss|install|depend',
durationSeconds: null,
cost: null,
},
];

export const test = baseTest.extend<ExplainTracesFixtures>({
explainTraces: async ({ sdkClient, project, testNamespace }, use, testInfo) => {
const refs: ExplainTraceRef[] = [];
for (const seed of SEEDS) {
const name = `${testNamespace}-explain-${seed.suffix}`;
const spans =
seed.cost === null
? []
: [
{
name: 'llm-call',
type: 'llm' as const,
model: 'gpt-4o',
provider: 'openai',
input: { prompt: 'seed prompt' },
output: { completion: 'seed completion' },
usage: { prompt_tokens: 10, completion_tokens: 5, total_tokens: 15 },
total_cost: seed.cost,
},
];
const created = await sdkClient.python.createNestedTrace({
project_name: project.name,
name,
input: { user: `trigger for ${seed.suffix}` },
tags: ['explain', seed.suffix],
error_info: {
exception_type: seed.errorType,
message: seed.errorMessage,
},
duration_seconds: seed.durationSeconds ?? undefined,
spans,
});
refs.push({
id: created.id,
name: created.name,
errorType: seed.errorType,
durationSeconds: seed.durationSeconds,
cost: seed.cost,
errorKeywordSource: seed.errorKeyword,
});
}

await testInfo.attach('opik.explainTraces', {
body: JSON.stringify(refs, null, 2),
contentType: 'application/json',
});

await use(refs);
// No explicit teardown — the project fixture's deleteProject cascades.
},
});

export { expect } from './annotation-queue.fixture';
3 changes: 2 additions & 1 deletion tests_end_to_end/e2e/fixtures/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export { test, expect } from './annotation-queue.fixture';
export { test, expect } from './explain-traces.fixture';
export type { ProjectFixtures } from './project.fixture';
export type { ScratchDir, ScratchDirFixtures } from './scratch-dir.fixture';
export type {
Expand Down Expand Up @@ -45,4 +45,5 @@ export type {
AnnotationQueueTraceRef,
AnnotationQueueFixtures,
} from './annotation-queue.fixture';
export type { ExplainTraceRef, ExplainTracesFixtures } from './explain-traces.fixture';
export type { ProjectRef } from '../core/backend';
91 changes: 90 additions & 1 deletion tests_end_to_end/e2e/pom/logs.page.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,23 @@
import { test, type Page, type Locator } from '@playwright/test';
import { test, expect, type Page, type Locator } from '@playwright/test';
import { loadEnvConfig } from '../config/env.config';
import { TracePanelPage } from './trace-panel.page';
import { ThreadPanelPage } from './thread-panel.page';

export type ExplainKind = 'error' | 'duration' | 'cost';

// Maps an explain kind to the Traces table column id (used in data-cell-id)
// and the owl trigger's aria-label, per apps/opik-frontend/src/plugins/comet/explain/registry.ts.
const EXPLAIN_COLUMN: Record<ExplainKind, string> = {
error: 'error_info',
duration: 'duration',
cost: 'total_estimated_cost',
};
const EXPLAIN_LABEL: Record<ExplainKind, string> = {
error: 'Explain error',
duration: 'Explain duration',
cost: 'Explain cost',
};

export class LogsPage {
private projectId: string | null = null;

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

/** The Errors/Duration/Estimated cost cell for a trace row, keyed by Ollie explain kind. */
explainCell(traceId: string, kind: ExplainKind): Locator {
return this.page.locator(`[data-cell-id="${traceId}_${EXPLAIN_COLUMN[kind]}"]`);
}

/**
* Hover a trace's Errors/Duration/Estimated cost cell and click its Ollie
* "Explain" owl trigger, opening the popover. The trigger only renders once
* the Ollie assistant bridge handshake (mounted via the page's assistant
* sidebar) completes, which can lag a beat after the table itself is
* interactive — so this polls hover+lookup rather than asserting once.
*/
async openExplain(traceId: string, kind: ExplainKind, timeoutMs = 60_000): Promise<void> {
return test.step(`open Ollie explain (${kind}) for trace ${traceId}`, async () => {
const cell = this.explainCell(traceId, kind);
const button = cell.getByRole('button', { name: EXPLAIN_LABEL[kind] });
await expect
.poll(
async () => {
await cell.hover();
return button.count();
},
{ timeout: timeoutMs, intervals: [500, 1000, 2000] },
)
.toBeGreaterThan(0);
await button.click();
});
}

/**
* Wait for the open Ollie explain popover to settle (loading -> done/error)
* and return its rendered text. Scoped to the last `[role="status"]` live
* region on the page — Radix unmounts a closed popover's content, so only
* the currently-open one's region should be present.
*/
async readExplanation(timeoutMs = 60_000): Promise<string> {
return test.step('wait for Ollie explain popover to settle', async () => {
const status = this.page.locator('[role="status"]').last();
await expect(status).toHaveAttribute('aria-busy', 'false', { timeout: timeoutMs });
const text = ((await status.textContent()) ?? '').trim();
if (!text) {
throw new Error('Ollie explain popover settled but rendered no text');
}
return text;
});
}

/** Close the open Ollie explain popover. */
async closeExplain(): Promise<void> {
return test.step('close Ollie explain popover', async () => {
await this.page.keyboard.press('Escape');
});
}

/**
* The "Continue conversation" link in the currently open Ollie explain
* popover. Only rendered once the popover has settled with text (see
* ExplainPopover.tsx) — call after `readExplanation()`.
*/
continueConversationButton(): Locator {
return this.page.getByRole('button', { name: 'Continue conversation' });
}

/**
* Click "Continue conversation" to hand the explain popover's question +
* cached answer off to the Ollie sidebar chat. This closes the popover as
* a side effect (see ExplainPopover's onContinue).
*/
async continueConversation(): Promise<void> {
return test.step('continue the Ollie explain conversation in the sidebar', async () => {
await this.continueConversationButton().click();
});
}

// --- Threads tab ---

/** The Threads/Traces/Spans tab toggle for "Threads". */
Expand Down
76 changes: 43 additions & 33 deletions tests_end_to_end/e2e/pom/ollie.page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,24 +68,8 @@ export class OlliePage {
await this.sendButton().click();

// The user echo and the assistant reply each mount as a [data-message-id]
// node. Wait for both to land (count grows by 2), then for the reply's
// text to be non-empty (it streams in after the bubble appears).
await expect
.poll(async () => this.messages().count(), {
timeout: timeoutMs,
intervals: [500, 1000, 2000],
})
.toBeGreaterThanOrEqual(before + 2);

const reply = this.messages().last();
await expect
.poll(async () => ((await reply.textContent()) ?? '').trim().length, {
timeout: timeoutMs,
intervals: [500, 1000, 2000],
})
.toBeGreaterThan(0);

return ((await reply.textContent()) ?? '').trim();
// node, so wait for both to land (count grows by 2).
return this.awaitNewMessage(before + 2, timeoutMs, [500, 1000, 2000]);
});
}

Expand Down Expand Up @@ -128,6 +112,21 @@ export class OlliePage {
});
}

/**
* Wait for an Explain popover's "Continue conversation" hand-off to land in
* the chat. The bridge posts the question and the popover's already-settled
* answer as a pair of new messages (see `chat:continue` in explainStore.ts
* — "carries the verbatim Q&A already shown"), not a fresh generation, so
* this only waits for them to render, not for streaming. `beforeCount` is
* the message count read just before clicking "Continue conversation".
* Returns the last message's (the answer's) text.
*/
async awaitContinuedConversation(beforeCount: number, timeoutMs = 30_000): Promise<string> {
return test.step('wait for the continued conversation to render in the sidebar', async () => {
return this.awaitNewMessage(beforeCount + 2, timeoutMs, [300, 600, 1200]);
});
}

/**
* Run the `/analyze` flow from the greeting action button and wait for a
* non-empty assistant response to render. Ollie is a non-deterministic agent,
Expand All @@ -138,21 +137,7 @@ export class OlliePage {
return test.step('run /analyze and await a response', async () => {
const before = await this.messages().count();
await this.analyzeButton().click();
await expect
.poll(async () => this.messages().count(), {
timeout: timeoutMs,
intervals: [1000, 2000, 5000],
})
.toBeGreaterThan(before);

const reply = this.messages().last();
await expect
.poll(async () => ((await reply.textContent()) ?? '').trim().length, {
timeout: timeoutMs,
intervals: [1000, 2000, 5000],
})
.toBeGreaterThan(0);
return ((await reply.textContent()) ?? '').trim();
return this.awaitNewMessage(before + 1, timeoutMs, [1000, 2000, 5000]);
});
}

Expand Down Expand Up @@ -304,6 +289,31 @@ export class OlliePage {

// ── private helpers ─────────────────────────────────────────────────────

/**
* Wait for the message list to reach `minCount` messages, then for the last
* one's text to be non-empty (it streams in after the bubble appears).
* Returns the last message's (the reply's) text.
*/
private async awaitNewMessage(
minCount: number,
timeoutMs: number,
intervals: number[],
): Promise<string> {
Comment thread
natagh23 marked this conversation as resolved.
await expect
.poll(async () => this.messages().count(), { timeout: timeoutMs, intervals })
.toBeGreaterThanOrEqual(minCount);

const reply = this.messages().last();
Comment thread
natagh23 marked this conversation as resolved.
Outdated
await expect
.poll(async () => ((await reply.textContent()) ?? '').trim().length, {
timeout: timeoutMs,
intervals,
})
.toBeGreaterThan(0);

return ((await reply.textContent()) ?? '').trim();
}

// Anchor on the testid added to the host iframe in AssistantSidebar.tsx, OR
// on the `title="Assistant"` attribute already present on deployed builds.
// The `,` selector keeps the POM working against a cloud env running an older
Expand Down
Loading
Loading