Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
80 changes: 47 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,35 @@ 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.
return test.step('wait for a new Ollie message to render', async () => {
const deadline = Date.now() + timeoutMs;

await expect
.poll(async () => this.messages().count(), { timeout: timeoutMs, intervals })
.toBeGreaterThanOrEqual(minCount);

const reply = this.messages().last();
await expect
.poll(async () => ((await reply.textContent()) ?? '').trim().length, {
timeout: Math.max(0, deadline - Date.now()),
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