From 889ea9baa3ff2a8fd44dfb27b4cb99c546850d11 Mon Sep 17 00:00:00 2001 From: jariy17 Date: Tue, 18 Aug 2026 20:20:46 +0000 Subject: [PATCH 1/4] refactor(eval): replace core.eval.simulate with invokeDataset + per-type example classes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split the monolithic core.eval.simulate into two composable EvalClient calls the handler orchestrates — invokeDataset (replay) then startBatchEvaluation (grade) — and model each dataset type as a class that owns its parse + run + ground truth. - src/core/eval/dataset/: types (Example interface, RunContext), predefined + simulated example classes, DatasetLoader (shape-classify → switch → new), and a generic runExamples pool. Replaces src/core/eval/simulate.ts. - core.eval.invokeDataset returns { sessions, invoked, failed } with neutral inline ground truth; the batch-evaluation simulate handler wraps it as sessionMetadata and submits startBatchEvaluation. No Core method calls a sibling. - readDatasetText reuses readLocalDatasetFile + downloadDatasetToTemp (drops the combined loadDatasetFile); dispatch is a build() switch whose non-exhaustiveness on a new DatasetSchemaType fails the build (no map, no assertNever). - Carries the simulate fixes: sparse multi-turn ground truth keeps turn position (input.prompt per turn), both-row refusal, NotImplementedError for simulated, per-example failure isolation, 180s ingestion wait, AbortSignal, JSON-only. - Tests: dataset unit tests + a golden snapshot of the inline ground-truth shape; handler test asserts the invokeDataset→startBatchEvaluation composition. --- src/core/eval.tsx | 189 ++++++++---------- .../__snapshots__/dataset.test.ts.snap | 35 ++++ src/core/eval/dataset/dataset.test.ts | 137 +++++++++++++ src/core/eval/dataset/load.ts | 73 +++++++ src/core/eval/dataset/predefined.ts | 67 +++++++ src/core/eval/dataset/run.ts | 29 +++ src/core/eval/dataset/simulated.ts | 27 +++ src/core/eval/dataset/types.ts | 20 ++ src/core/eval/simulate.ts | 120 ----------- .../eval/batch-evaluation/simulate/index.tsx | 44 +++- .../simulate/simulate.test.tsx | 66 ++++-- src/handlers/eval/types.tsx | 54 ++--- src/testing/TestCoreClient.tsx | 29 ++- 13 files changed, 611 insertions(+), 279 deletions(-) create mode 100644 src/core/eval/dataset/__snapshots__/dataset.test.ts.snap create mode 100644 src/core/eval/dataset/dataset.test.ts create mode 100644 src/core/eval/dataset/load.ts create mode 100644 src/core/eval/dataset/predefined.ts create mode 100644 src/core/eval/dataset/run.ts create mode 100644 src/core/eval/dataset/simulated.ts create mode 100644 src/core/eval/dataset/types.ts delete mode 100644 src/core/eval/simulate.ts diff --git a/src/core/eval.tsx b/src/core/eval.tsx index 816ed8463..e53c89cf5 100644 --- a/src/core/eval.tsx +++ b/src/core/eval.tsx @@ -108,8 +108,8 @@ import type { LlmAsAJudgeUpdate, SessionSourceValue, SessionTrace, - SimulateInput, - SimulateResult, + InvokeDatasetInput, + InvokeDatasetResult, SpanRecord, StartBatchEvaluationInput, UpdateConfigurationBundleInput, @@ -117,7 +117,9 @@ import type { } from "../handlers/eval/types"; import { atomicWrite, atomicWriteStream, readTextFile, renderJsonTemplate } from "../io"; import { invokeRuntime } from "./invokeRuntime"; -import { loadDatasetFile, runScenarios, toSessionMetadata } from "./eval/simulate"; +import { DatasetLoader } from "./eval/dataset/load"; +import { runExamples } from "./eval/dataset/run"; +import type { RunContext } from "./eval/dataset/types"; import { normalizeRuntimeInvokeRequest } from "../handlers/runtime/invoke/request"; import { isTerminalStatus, readEvaluationResults } from "./batchEvaluationResults"; import { applyExampleIds, diffExamples, indexRemoteById, parseJsonl } from "./datasetDiff"; @@ -514,118 +516,103 @@ export class EvalClient implements CoreEvalClient { }; } - async simulate( - input: SimulateInput, + async invokeDataset( + input: InvokeDatasetInput, options: CoreOptions, signal?: AbortSignal, - ): Promise { - // Load scenarios: a local JSONL path directly, else download the dataset id. - let tempDatasetPath: string | undefined; - const path = (await Bun.file(input.dataset).exists()) - ? input.dataset - : (tempDatasetPath = await this.downloadDatasetToTemp( - input.dataset, - input.datasetVersion, - options, - signal, - )); - try { - const scenarios = await loadDatasetFile(path); - const byId = new Map(scenarios.map((s) => [s.scenarioId, s])); - - // Resolve the runtime once; normalizeRuntimeInvokeRequest validates auth + fills - // accountId. Invoke is the extracted free fn — no RuntimeClient dependency. - const runtime = await this.clients - .control(toClientConfig(options)) - .send(new GetAgentRuntimeCommand({ agentRuntimeId: input.runtimeId })); - const deps = { clients: this.clients, fetch: this.fetch, logger: this.logger }; - - const { ok, failed, firstError } = await runScenarios(scenarios, async (scenario) => { - // One session per scenario; the id is generated client-side (session id is a - // client-owned input per the AgentCore docs, needed on the request before any - // response and reused across turns). Turns run sequentially against the SAME - // session so the conversation — and its per-turn traces — accumulate in order, - // matching the per-turn ground truth in toSessionMetadata. - const runtimeSessionId = randomUUID(); - try { - for (const turn of scenario.turns) { - const request = normalizeRuntimeInvokeRequest(runtime, { - runtimeId: input.runtimeId, - qualifier: input.qualifier, - payload: renderJsonTemplate(input.payloadTemplate, { input: turn.input }), - contentType: "application/json", - accept: "application/json", - applicationHeaders: input.headers, - bearerToken: input.bearerToken, - runtimeSessionId, - runtimeUserId: input.userId, - }); - const response = await invokeRuntime(deps, request, options, signal); - for await (const _chunk of response.body) { - // Drain each turn's stream so it completes before the next turn. - } - } - } catch (error) { - this.logger.debug( - `simulate: invoke failed for scenario "${scenario.scenarioId}": ${(error as Error).message}`, - ); - throw error; - } - return { scenarioId: scenario.scenarioId, sessionId: runtimeSessionId }; - }); + ): Promise { + // readDatasetText owns the local-vs-id fetch + temp cleanup; DatasetLoader is the + // pure parse into Example instances (each dispatches its own replay via run()). + const examples = DatasetLoader.load( + await this.readDatasetText(input.dataset, input.datasetVersion, options, signal), + ); - if (ok.length === 0) { - const detail = firstError ? `; first error: ${firstError.message}` : ""; - throw new InputValidationError( - `no scenarios could be invoked (${failed} failed) — nothing to evaluate${detail}`, + // Resolve the runtime once; normalizeRuntimeInvokeRequest validates auth + fills + // accountId. Invoke is the extracted free fn — no RuntimeClient dependency. + const runtime = await this.clients + .control(toClientConfig(options)) + .send(new GetAgentRuntimeCommand({ agentRuntimeId: input.runtimeId }), { + abortSignal: signal, + }); + const deps = { clients: this.clients, fetch: this.fetch, logger: this.logger }; + + const { ok, failed, firstError } = await runExamples(examples, async (example) => { + // One session per example; the id is client-generated (a client-owned input per + // the AgentCore docs, needed before any response) and reused across turns so the + // conversation and its per-turn traces accumulate in order. + const sessionId = randomUUID(); + const ctx: RunContext = { + invokeOnce: async (payload) => { + const request = normalizeRuntimeInvokeRequest(runtime, { + runtimeId: input.runtimeId, + qualifier: input.qualifier, + payload: renderJsonTemplate(input.payloadTemplate, { input: payload }), + contentType: "application/json", + accept: "application/json", + applicationHeaders: input.headers, + bearerToken: input.bearerToken, + runtimeSessionId: sessionId, + runtimeUserId: input.userId, + }); + const response = await invokeRuntime(deps, request, options, signal); + // Read the body to completion (frees the socket, feeds an actor loop later); + // a scripted example ignores the returned text. + let text = ""; + const decoder = new TextDecoder(); + for await (const chunk of response.body) text += decoder.decode(chunk, { stream: true }); + text += decoder.decode(); + return { text }; + }, + }; + try { + const groundTruth = await example.run(ctx); + return { exampleId: example.exampleId, sessionId, groundTruth }; + } catch (error) { + this.logger.debug( + `invokeDataset: invoke failed for example "${example.exampleId}" (${example.schemaType}): ${(error as Error).message}`, ); + throw error; } - if (failed > 0) { - this.logger.warn(`simulate: ${failed} scenario(s) failed to invoke and were dropped`); - } + }); - // AgentCore takes ~30s-3min to emit spans for a freshly invoked session; submit - // too early and the service reads an empty log group and marks every session - // failed. Wait once for the batch to be safely ingestible (matches the old CLI's - // 180s wait). Skipped when disabled via `SIMULATE_INGESTION_WAIT_MS=0` (tests). - const waitMs = Number(process.env.SIMULATE_INGESTION_WAIT_MS ?? 180_000); - if (waitMs > 0) { - this.logger.info( - `waiting ${Math.round(waitMs / 1000)}s for span ingestion before submitting`, - ); - await sleep(waitMs, undefined, { signal }); - } + if (failed > 0) { + this.logger.warn(`invokeDataset: ${failed} example(s) failed to invoke and were dropped`); + } - const job = await this.startBatchEvaluation( - { - name: input.name, - description: input.description, - evaluatorIds: input.evaluatorIds, - source: { - origin: "agent", - agent: input.runtimeId, - endpoint: input.qualifier, - sessionIds: ok.map((r) => r.sessionId), - }, - groundTruth: ok.map((r) => toSessionMetadata(byId.get(r.scenarioId)!, r.sessionId)), - kmsKeyArn: input.kmsKeyArn, - }, - options, + // AgentCore takes ~30s-3min to emit spans for a freshly invoked session; grade too + // early and the service reads an empty log group and marks every session failed. Wait + // once. Skipped when disabled via `SIMULATE_INGESTION_WAIT_MS=0` (tests). + const waitMs = Number(process.env.SIMULATE_INGESTION_WAIT_MS ?? 180_000); + if (ok.length > 0 && waitMs > 0) { + this.logger.info( + `waiting ${Math.round(waitMs / 1000)}s for span ingestion before evaluating`, ); + await sleep(waitMs, undefined, { signal }); + } - return { - batchEvaluationId: job.batchEvaluationId, - status: job.status, - scenariosInvoked: ok.length, - scenariosFailed: failed, - }; + return { sessions: ok, invoked: ok.length, failed, firstError }; + } + + // readDatasetText resolves a dataset ref to its JSONL text: a local path directly, else + // download the dataset id to a temp file (cleaned up here). Both funnel through the + // shared readLocalDatasetFile so replay and updateDatasetExamples read files the same way. + private async readDatasetText( + ref: string, + version: string | undefined, + options: CoreOptions, + signal?: AbortSignal, + ): Promise { + if (await Bun.file(ref).exists()) return readLocalDatasetFile(ref, signal); + const path = await this.downloadDatasetToTemp(ref, version, options, signal); + try { + return await readLocalDatasetFile(path, signal); } finally { - if (tempDatasetPath) await unlink(tempDatasetPath).catch(() => {}); + await unlink(path).catch(() => {}); } } // downloadDatasetToTemp streams a dataset version's JSONL to a temp file so - // loadDatasetFile can read it — reuses downloadDataset rather than re-fetching. + // readDatasetText can read it — reuses downloadDataset rather than re-fetching. private async downloadDatasetToTemp( id: string, version: string | undefined, diff --git a/src/core/eval/dataset/__snapshots__/dataset.test.ts.snap b/src/core/eval/dataset/__snapshots__/dataset.test.ts.snap new file mode 100644 index 000000000..079d038fc --- /dev/null +++ b/src/core/eval/dataset/__snapshots__/dataset.test.ts.snap @@ -0,0 +1,35 @@ +// Bun Snapshot v1, https://bun.sh/docs/test/snapshots + +exports[`PredefinedExample ground truth maps to the expected inline shape [golden] 1`] = ` +{ + "assertions": [ + { + "text": "stays polite", + }, + { + "text": "does not promise a date", + }, + ], + "expectedTrajectory": { + "toolNames": [ + "refund_lookup", + "refund_create", + ], + }, + "turns": [ + { + "input": { + "prompt": "I want a refund", + }, + }, + { + "expectedResponse": { + "text": "Refund started", + }, + "input": { + "prompt": "order 123", + }, + }, + ], +} +`; diff --git a/src/core/eval/dataset/dataset.test.ts b/src/core/eval/dataset/dataset.test.ts new file mode 100644 index 000000000..26c615c8e --- /dev/null +++ b/src/core/eval/dataset/dataset.test.ts @@ -0,0 +1,137 @@ +import { test, expect, describe } from "bun:test"; +import { DatasetLoader } from "./load"; +import { PredefinedExample } from "./predefined"; +import { SimulatedExample } from "./simulated"; +import type { RunContext, TurnResult } from "./types"; + +const row = (o: object) => JSON.stringify(o); + +// A fake transport that records what was said and returns a canned reply. No AWS. +function recordingCtx(reply = ""): { ctx: RunContext; calls: string[] } { + const calls: string[] = []; + const ctx: RunContext = { + invokeOnce: async (payload): Promise => { + calls.push(payload); + return { text: reply }; + }, + }; + return { ctx, calls }; +} + +describe("DatasetLoader.load", () => { + test("builds a PredefinedExample from a turns row", () => { + const [e] = DatasetLoader.load(row({ example_id: "x", turns: [{ input: "hi" }] })); + expect(e).toBeInstanceOf(PredefinedExample); + expect(e!.schemaType).toBe("AGENTCORE_EVALUATION_PREDEFINED_V1"); + expect(e!.exampleId).toBe("x"); + }); + + test("accepts the legacy scenario_id as the example id", () => { + const [e] = DatasetLoader.load(row({ scenario_id: "legacy", turns: [{ input: "hi" }] })); + expect(e!.exampleId).toBe("legacy"); + }); + + test("refuses a row that is both predefined and simulated", () => { + expect(() => + DatasetLoader.load(row({ example_id: "x", turns: [{ input: "a" }], actor_profile: {} })), + ).toThrow(/both 'turns' and 'actor_profile'/); + }); + + test("refuses a row that is neither", () => { + expect(() => DatasetLoader.load(row({ example_id: "x" }))).toThrow( + /neither 'turns' nor 'actor_profile'/, + ); + }); + + test("names a simulated row instead of blaming the data", () => { + expect(() => + DatasetLoader.load(row({ example_id: "x", actor_profile: { goal: "g" } })), + ).toThrow(/simulated example/); + }); + + test("rejects duplicate example ids", () => { + const two = [ + row({ example_id: "a", turns: [{ input: "1" }] }), + row({ example_id: "a", turns: [{ input: "2" }] }), + ].join("\n"); + expect(() => DatasetLoader.load(two)).toThrow(/duplicate example_id: "a"/); + }); + + test("rejects a missing example id", () => { + expect(() => DatasetLoader.load(row({ turns: [{ input: "hi" }] }))).toThrow( + /missing 'example_id'/, + ); + }); + + test("rejects an invalid JSON line", () => { + expect(() => DatasetLoader.load("{not json")).toThrow(/not valid JSON/); + }); + + test("rejects an empty dataset", () => { + expect(() => DatasetLoader.load("\n \n")).toThrow(/no examples/); + }); + + test("ignores blank lines between rows", () => { + const examples = DatasetLoader.load( + [ + row({ example_id: "a", turns: [{ input: "1" }] }), + "", + row({ example_id: "b", turns: [{ input: "2" }] }), + ].join("\n"), + ); + expect(examples.map((e) => e.exampleId)).toEqual(["a", "b"]); + }); +}); + +describe("SimulatedExample", () => { + test("construction throws NotImplementedError (never replayed)", () => { + expect(() => new SimulatedExample("x", { actor_profile: {} })).toThrow(/cannot replay yet/); + }); +}); + +describe("PredefinedExample", () => { + test("constructor rejects a row with no turns", () => { + expect(() => new PredefinedExample("x", { turns: [] })).toThrow(/has no turns/); + }); + + test("run replays every turn in order, on one session", async () => { + const { ctx, calls } = recordingCtx(); + await new PredefinedExample("x", { turns: [{ input: "a" }, { input: "b" }] }).run(ctx); + expect(calls).toEqual(["a", "b"]); + }); + + test("sparse expectations keep their turn position", async () => { + const { ctx } = recordingCtx(); + const gt = await new PredefinedExample("x", { + turns: [{ input: "t1" }, { input: "t2" }, { input: "t3", expected_response: "42" }], + }).run(ctx); + // Not 1 — filtering the two expectation-less turns would renumber the rest and score + // turn 3's "42" against turn 1. + expect(gt!.turns).toHaveLength(3); + expect(gt!.turns![2]!.expectedResponse).toEqual({ text: "42" }); + expect(gt!.turns![0]!.input).toEqual({ prompt: "t1" }); + expect(gt!.turns![0]!.expectedResponse).toBeUndefined(); + }); + + test("an example with no ground truth returns undefined", async () => { + const { ctx } = recordingCtx(); + const gt = await new PredefinedExample("x", { turns: [{ input: "t1" }] }).run(ctx); + expect(gt).toBeUndefined(); + }); + + // Golden: the full inline ground-truth shape for a representative example (assertions + + // trajectory + sparse turns). Locks the exact wire shape handed to the grader so a + // regression in the mapping is caught, not just its parts. + test("ground truth maps to the expected inline shape [golden]", async () => { + const { ctx } = recordingCtx(); + const gt = await new PredefinedExample("orders-1", { + turns: [ + { input: "I want a refund" }, + { input: "order 123", expected_response: "Refund started" }, + ], + assertions: ["stays polite", "does not promise a date"], + expected_trajectory: ["refund_lookup", "refund_create"], + }).run(ctx); + expect(gt).toMatchSnapshot(); + }); +}); diff --git a/src/core/eval/dataset/load.ts b/src/core/eval/dataset/load.ts new file mode 100644 index 000000000..516e94191 --- /dev/null +++ b/src/core/eval/dataset/load.ts @@ -0,0 +1,73 @@ +import type { DatasetSchemaType } from "@aws-sdk/client-bedrock-agentcore-control"; +import { InputValidationError } from "../../../errors"; +import type { Example } from "./types"; +import { PredefinedExample } from "./predefined"; +import { SimulatedExample } from "./simulated"; + +// DatasetLoader parses dataset JSONL into Example instances. Pure — no I/O, no AWS — so +// it's unit-testable with a plain string; fetching the text (local file or dataset id) is +// the caller's job. +export class DatasetLoader { + static load(text: string): Example[] { + const examples: Example[] = []; + const seen = new Set(); + for (const line of text.split("\n")) { + const trimmed = line.trim(); + if (!trimmed) continue; + let row: Record; + try { + row = JSON.parse(trimmed) as Record; + } catch { + throw new InputValidationError("dataset contains a line that is not valid JSON"); + } + + // The id is the join key between a session and its ground truth, so a missing or + // duplicate id silently misassigns ground truth to the wrong session. + const exampleId = String(row.example_id ?? row.scenario_id ?? ""); + if (!exampleId) { + throw new InputValidationError("dataset example is missing 'example_id'"); + } + if (seen.has(exampleId)) { + throw new InputValidationError(`dataset has a duplicate example_id: "${exampleId}"`); + } + seen.add(exampleId); + + examples.push(DatasetLoader.build(row, exampleId)); + } + if (examples.length === 0) throw new InputValidationError("dataset has no examples"); + return examples; + } + + // Classify by row shape — a local JSONL carries no schemaType, and AWS's own SDK + // dispatches this way, so a file the SDK accepts this CLI must accept too. Refuse a + // both-row: AWS's `if "turns" in raw` silently drops the actor profile, which reads as + // a passing run of the wrong test. + private static build(row: Record, exampleId: string): Example { + const hasTurns = Array.isArray(row.turns); + const hasActor = row.actor_profile != null; + if (hasTurns && hasActor) { + throw new InputValidationError( + `example "${exampleId}" has both 'turns' and 'actor_profile' — one row cannot be both`, + ); + } + if (!hasTurns && !hasActor) { + throw new InputValidationError( + `example "${exampleId}" has neither 'turns' nor 'actor_profile'`, + ); + } + + // `: DatasetSchemaType` types the scrutinee as the full SDK enum, so when a member is + // added the switch stops being exhaustive, build can reach its end without returning, + // and the compiler flags it (TS2366). The `new X(exampleId, row)` sites enforce the + // constructor shape — no map, no assertNever. + const schemaType: DatasetSchemaType = hasTurns + ? "AGENTCORE_EVALUATION_PREDEFINED_V1" + : "AGENTCORE_EVALUATION_SIMULATED_V1"; + switch (schemaType) { + case "AGENTCORE_EVALUATION_PREDEFINED_V1": + return new PredefinedExample(exampleId, row); + case "AGENTCORE_EVALUATION_SIMULATED_V1": + return new SimulatedExample(exampleId, row); + } + } +} diff --git a/src/core/eval/dataset/predefined.ts b/src/core/eval/dataset/predefined.ts new file mode 100644 index 000000000..5695f242c --- /dev/null +++ b/src/core/eval/dataset/predefined.ts @@ -0,0 +1,67 @@ +import type { InlineGroundTruth } from "@aws-sdk/client-bedrock-agentcore"; +import { InputValidationError } from "../../../errors"; +import type { Example, RunContext } from "./types"; + +type Turn = { input: string; expectedResponse?: string }; + +// A predefined example has scripted turns: replay each verbatim, ignore the reply. Owns +// its parse (from the raw row) and its ground-truth mapping — everything predefined in +// one place. +export class PredefinedExample implements Example { + readonly schemaType = "AGENTCORE_EVALUATION_PREDEFINED_V1" as const; + readonly turns: Turn[]; + readonly assertions?: string[]; + readonly expectedTrajectory?: string[]; + + // Parsing happens in the constructor (validation-at-boundary). Fields are assigned in + // the body, not field initializers, so there's no "used before init" hazard. + constructor( + readonly exampleId: string, + row: Record, + ) { + const turns = (Array.isArray(row.turns) ? row.turns : []).map((t: Record) => ({ + input: String(t.input ?? ""), + expectedResponse: t.expected_response as string | undefined, + })); + if (turns.length === 0) { + throw new InputValidationError(`example "${exampleId}" has no turns`); + } + this.turns = turns; + this.assertions = row.assertions as string[] | undefined; + this.expectedTrajectory = row.expected_trajectory as string[] | undefined; + } + + // Turns share one session, so they run sequentially and awaited: racing them would + // interleave the conversation and misalign the per-turn traces with the ground truth. + async run(ctx: RunContext): Promise { + for (const turn of this.turns) await ctx.invokeOnce(turn.input); + return this.groundTruth(); + } + + // One entry per turn, each carrying its prompt in `input`, so a turn with no expected + // response still occupies its slot. Filtering the sparse turns out renumbers the rest, + // scoring turn 3's expectation against turn 1; the service's alignment rule is + // undocumented, so carrying the prompt is correct whether it aligns by index or content. + private groundTruth(): InlineGroundTruth | undefined { + const turns = this.turns.some((t) => t.expectedResponse !== undefined) + ? this.turns.map((t) => ({ + input: { prompt: t.input }, + ...(t.expectedResponse !== undefined && { + expectedResponse: { text: t.expectedResponse }, + }), + })) + : []; + const assertions = this.assertions?.map((text) => ({ text })); + const inline: InlineGroundTruth = { + // Omit empty arrays: the service rejects zero-length `assertions`/`turns` + // (documented min-1) rather than reading them as "no data". + ...(assertions && assertions.length > 0 && { assertions }), + ...(this.expectedTrajectory?.length && { + expectedTrajectory: { toolNames: this.expectedTrajectory }, + }), + ...(turns.length > 0 && { turns }), + }; + // An all-empty inline is not "no ground truth" — return undefined so the caller omits it. + return Object.keys(inline).length > 0 ? inline : undefined; + } +} diff --git a/src/core/eval/dataset/run.ts b/src/core/eval/dataset/run.ts new file mode 100644 index 000000000..1c9aeabc1 --- /dev/null +++ b/src/core/eval/dataset/run.ts @@ -0,0 +1,29 @@ +// runExamples runs `worker` over every item with bounded concurrency. A failed worker +// doesn't sink the run — the failure is counted (so the caller can report on all-failed) +// and its item dropped. Returns ok results + the first error, which the caller surfaces +// to explain a total failure. Generic in the item type; not eval-specific. +export type ExampleRun = { ok: Result[]; failed: number; firstError?: Error }; + +export async function runExamples( + items: Item[], + worker: (item: Item) => Promise, + concurrency = 5, +): Promise> { + const ok: Result[] = []; + let failed = 0; + let firstError: Error | undefined; + let next = 0; + const run = async (): Promise => { + while (next < items.length) { + const item = items[next++]!; + try { + ok.push(await worker(item)); + } catch (error) { + failed++; + if (!firstError) firstError = error instanceof Error ? error : new Error(String(error)); + } + } + }; + await Promise.all(Array.from({ length: Math.min(concurrency, items.length) }, run)); + return { ok, failed, firstError }; +} diff --git a/src/core/eval/dataset/simulated.ts b/src/core/eval/dataset/simulated.ts new file mode 100644 index 000000000..7428fef84 --- /dev/null +++ b/src/core/eval/dataset/simulated.ts @@ -0,0 +1,27 @@ +import type { InlineGroundTruth } from "@aws-sdk/client-bedrock-agentcore"; +import { NotImplementedError } from "../../../errors"; +import type { Example, RunContext } from "./types"; + +// A simulated example carries an actor profile instead of scripted turns: replaying it +// needs an LLM "user" to generate each next message from the agent's reply, which this +// command does not run yet. Throw at construction (= load time) so the user fails early +// with a clear message rather than a per-row "has no turns" misdiagnosis mid-run. +export class SimulatedExample implements Example { + readonly schemaType = "AGENTCORE_EVALUATION_SIMULATED_V1" as const; + + constructor( + readonly exampleId: string, + _row: Record, + ) { + throw new NotImplementedError( + `example "${exampleId}" is a simulated example (actor_profile), which this ` + + `command cannot replay yet — it has no scripted turns`, + ); + } + + // Unreachable today (the constructor throws). Implements the interface; the actor loop + // lands here when simulated ships. + run(_ctx: RunContext): Promise { + throw new NotImplementedError("simulated example replay is not implemented"); + } +} diff --git a/src/core/eval/dataset/types.ts b/src/core/eval/dataset/types.ts new file mode 100644 index 000000000..d6a9faf72 --- /dev/null +++ b/src/core/eval/dataset/types.ts @@ -0,0 +1,20 @@ +import type { DatasetSchemaType } from "@aws-sdk/client-bedrock-agentcore-control"; +import type { InlineGroundTruth } from "@aws-sdk/client-bedrock-agentcore"; + +// TurnResult is the agent's reply to one turn. A record (not a bare string) so a future +// tool-branching dataset type can widen it by a field without touching every example. +export type TurnResult = { text: string }; + +// RunContext is the per-session transport handed to run(): one call = one turn. The +// session id, auth, and payload templating are bound by the caller (the machine), so an +// example only decides what to say next, never how the request is built. +export type RunContext = { invokeOnce: (payload: string) => Promise }; + +// Example is the contract each dataset type implements: identity plus a self-describing +// run. An interface, not a base class — there is no shared state or behaviour to inherit, +// and the machine dispatches by calling run(), so nothing needs a common superclass. +export interface Example { + readonly schemaType: DatasetSchemaType; + readonly exampleId: string; + run(ctx: RunContext): Promise; +} diff --git a/src/core/eval/simulate.ts b/src/core/eval/simulate.ts deleted file mode 100644 index 81fc3f843..000000000 --- a/src/core/eval/simulate.ts +++ /dev/null @@ -1,120 +0,0 @@ -import type { SessionMetadataShape } from "@aws-sdk/client-bedrock-agentcore"; -import { InputValidationError } from "../../errors"; - -// Scenario is one dataset row for replay. Local to this module on purpose — it is -// not a handler contract, so it does not live in handlers/eval/types.tsx. Field -// names mirror the dataset JSONL (snake_case in, camelCase here). -export type Scenario = { - scenarioId: string; - turns: { input: string; expectedResponse?: string }[]; - assertions?: string[]; - expectedTrajectory?: string[]; -}; - -// parseScenarios reads dataset JSONL (one scenario per line) into Scenario records. -// Each scenario needs a non-empty, unique `scenario_id` — the id is the join key -// between the session created for it and its ground truth, so a missing/duplicate -// id silently misassigns ground truth to the wrong session. -export function parseScenarios(text: string): Scenario[] { - const scenarios: Scenario[] = []; - const seen = new Set(); - for (const line of text.split("\n")) { - const trimmed = line.trim(); - if (!trimmed) continue; - let row: Record; - try { - row = JSON.parse(trimmed) as Record; - } catch { - throw new InputValidationError("dataset contains a line that is not valid JSON"); - } - const scenario = toScenario(row); - if (!scenario.scenarioId) { - throw new InputValidationError("dataset scenario is missing 'scenario_id'"); - } - if (seen.has(scenario.scenarioId)) { - throw new InputValidationError( - `dataset has a duplicate scenario_id: "${scenario.scenarioId}"`, - ); - } - if (scenario.turns.length === 0) { - throw new InputValidationError(`scenario "${scenario.scenarioId}" has no turns`); - } - seen.add(scenario.scenarioId); - scenarios.push(scenario); - } - if (scenarios.length === 0) throw new InputValidationError("dataset has no scenarios"); - return scenarios; -} - -function toScenario(row: Record): Scenario { - const turns = Array.isArray(row.turns) ? row.turns : []; - return { - scenarioId: String(row.scenario_id ?? ""), - turns: turns.map((t: Record) => ({ - input: String(t.input ?? ""), - expectedResponse: t.expected_response as string | undefined, - })), - assertions: row.assertions as string[] | undefined, - expectedTrajectory: row.expected_trajectory as string[] | undefined, - }; -} - -// loadDatasetFile reads scenarios from a local JSONL path. The dataset-id path is -// handled by the caller (downloadDataset to a temp file, then this). -export async function loadDatasetFile(path: string): Promise { - return parseScenarios(await Bun.file(path).text()); -} - -// runScenarios runs `worker` over every scenario with bounded concurrency. A failed -// worker doesn't sink the run — the failure is captured (so the caller can report -// on all-failed) but drops that scenario. Returns ok results + the first error we -// saw, which the caller can surface to explain a total failure. -export type ScenarioRun = { ok: T[]; failed: number; firstError?: Error }; -export async function runScenarios( - scenarios: Scenario[], - worker: (scenario: Scenario) => Promise, - concurrency = 5, -): Promise> { - const ok: T[] = []; - let failed = 0; - let firstError: Error | undefined; - let next = 0; - const run = async (): Promise => { - while (next < scenarios.length) { - const scenario = scenarios[next++]!; - try { - ok.push(await worker(scenario)); - } catch (error) { - failed++; - if (!firstError) firstError = error instanceof Error ? error : new Error(String(error)); - } - } - }; - await Promise.all(Array.from({ length: Math.min(concurrency, scenarios.length) }, run)); - return { ok, failed, firstError }; -} - -// toSessionMetadata maps a scenario's ground truth onto the session the replay -// created — the batch service's per-session ground-truth shape (inline arm). -// Omit array fields when empty: the service rejects zero-length `turns` / -// `assertions` (`length >= 1`) rather than treating an empty array as "no data". -export function toSessionMetadata(scenario: Scenario, sessionId: string): SessionMetadataShape { - const turns = scenario.turns - .filter((t) => t.expectedResponse !== undefined) - .map((t) => ({ expectedResponse: { text: t.expectedResponse! } })); - const assertions = scenario.assertions?.map((text) => ({ text })); - return { - sessionId, - testScenarioId: scenario.scenarioId, - groundTruth: { - inline: { - ...(assertions && assertions.length > 0 && { assertions }), - ...(scenario.expectedTrajectory && - scenario.expectedTrajectory.length > 0 && { - expectedTrajectory: { toolNames: scenario.expectedTrajectory }, - }), - ...(turns.length > 0 && { turns }), - }, - }, - }; -} diff --git a/src/handlers/eval/batch-evaluation/simulate/index.tsx b/src/handlers/eval/batch-evaluation/simulate/index.tsx index 276e8f1b8..56fc0bbce 100644 --- a/src/handlers/eval/batch-evaluation/simulate/index.tsx +++ b/src/handlers/eval/batch-evaluation/simulate/index.tsx @@ -57,7 +57,10 @@ export const createSimulateBatchEvaluationHandler = (core: Core, _io: AppIO) => const interrupt = () => controller.abort(); process.once("SIGINT", interrupt); try { - const result = await core.eval.simulate( + const opts = coreOptsFromCtx(ctx); + + // (1) Replay the dataset → one graded-ready session per example. + const r = await core.eval.invokeDataset( { runtimeId: flags["runtime-id"], qualifier: flags["qualifier"], @@ -67,15 +70,46 @@ export const createSimulateBatchEvaluationHandler = (core: Core, _io: AppIO) => userId: flags["user-id"], dataset: flags["dataset"], datasetVersion: flags["dataset-version"], - evaluatorIds: flags["evaluator"], + }, + opts, + controller.signal, + ); + if (r.invoked === 0) { + const detail = r.firstError ? `; first error: ${r.firstError.message}` : ""; + throw new InputValidationError( + `no examples could be invoked (${r.failed} failed) — nothing to evaluate${detail}`, + ); + } + + // (2) Grade via the batch service — the example's neutral ground truth crosses + // over as sessionMetadata (inline arm). + const job = await core.eval.startBatchEvaluation( + { name: flags["name"], description: flags["description"], + evaluatorIds: flags["evaluator"], + source: { + origin: "agent", + agent: flags["runtime-id"], + endpoint: flags["qualifier"], + sessionIds: r.sessions.map((s) => s.sessionId), + }, + groundTruth: r.sessions.map((s) => ({ + sessionId: s.sessionId, + testScenarioId: s.exampleId, + ...(s.groundTruth && { groundTruth: { inline: s.groundTruth } }), + })), kmsKeyArn: flags["kms-key-arn"], }, - coreOptsFromCtx(ctx), - controller.signal, + opts, ); - ctx.require(JsonRendererKey).renderJson(result); + + ctx.require(JsonRendererKey).renderJson({ + batchEvaluationId: job.batchEvaluationId, + status: job.status, + examplesInvoked: r.invoked, + examplesFailed: r.failed, + }); } catch (error) { // A Ctrl-C exits quietly; the half-created sessions grade nothing. if (controller.signal.aborted) return; diff --git a/src/handlers/eval/batch-evaluation/simulate/simulate.test.tsx b/src/handlers/eval/batch-evaluation/simulate/simulate.test.tsx index fb0c6149d..d840ba204 100644 --- a/src/handlers/eval/batch-evaluation/simulate/simulate.test.tsx +++ b/src/handlers/eval/batch-evaluation/simulate/simulate.test.tsx @@ -6,18 +6,22 @@ import { testIO, TestGlobalConfigAccessor, } from "../../../../testing"; -import type { SimulateResult } from "../../types"; +import type { InvokeDatasetResult } from "../../types"; -const RESULT: SimulateResult = { - batchEvaluationId: "batch-eval-sim", - status: "RUNNING", - scenariosInvoked: 3, - scenariosFailed: 0, +// Two invoked sessions; the handler feeds these into startBatchEvaluation and renders +// the job it returns (DEFAULT_START_BATCH_EVAL_RESPONSE: batch-eval-test / RUNNING). +const INVOKE_RESULT: InvokeDatasetResult = { + sessions: [ + { exampleId: "e1", sessionId: "s1", groundTruth: { assertions: [{ text: "polite" }] } }, + { exampleId: "e2", sessionId: "s2" }, + ], + invoked: 2, + failed: 0, }; async function run(args: string[], configure?: (core: TestCoreClient) => void) { const core = new TestCoreClient(); - core.eval.setSimulateResponse(RESULT); + core.eval.setInvokeDatasetResponse(INVOKE_RESULT); configure?.(core); const io = testIO(); const root = createRootHandler(core, { @@ -113,20 +117,56 @@ describe("eval batch-evaluation simulate", () => { await expect(run(["eval", "batch-evaluation", "simulate", ...args])).rejects.toThrow(expected); }); - test("maps flags to the simulate input and renders the result", async () => { - const { core, stdout } = await run([...BASE, "--qualifier", "PROD", "--header", "x-a:1"]); - expect(JSON.parse(stdout)).toEqual(RESULT); - const call = core.eval.calls.find((c) => c.method === "simulate"); + test("passes runtime-level flags to invokeDataset (no evaluator/name leak)", async () => { + const { core } = await run([...BASE, "--qualifier", "PROD", "--header", "x-a:1"]); + const call = core.eval.calls.find((c) => c.method === "invokeDataset"); expect(call?.args[0]).toMatchObject({ runtimeId: "r-1", qualifier: "PROD", payloadTemplate: '{"prompt":"{input}"}', headers: [["x-a", "1"]], dataset: "/tmp/ds.jsonl", - evaluatorIds: ["Builtin.Helpfulness"], - name: "sim-1", }); + // Grader-only flags are NOT part of the invokeDataset (runtime-level) input. + expect(call?.args[0]).not.toHaveProperty("evaluatorIds"); + expect(call?.args[0]).not.toHaveProperty("name"); // Handler wires an AbortSignal (Ctrl-C) through to Core. expect(call?.args[2]).toBeInstanceOf(AbortSignal); }); + + test("composes startBatchEvaluation over the created sessions + wrapped ground truth", async () => { + const { core, stdout } = await run(BASE); + + // Rendered output is the batch job + invoked/failed counts. + expect(JSON.parse(stdout)).toEqual({ + batchEvaluationId: "batch-eval-test", + status: "RUNNING", + examplesInvoked: 2, + examplesFailed: 0, + }); + + const start = core.eval.calls.find((c) => c.method === "startBatchEvaluation"); + expect(start?.args[0]).toMatchObject({ + name: "sim-1", + evaluatorIds: ["Builtin.Helpfulness"], + source: { origin: "agent", agent: "r-1", sessionIds: ["s1", "s2"] }, + // e1's inline GT is wrapped; e2 (no GT) omits the member. + groundTruth: [ + { + sessionId: "s1", + testScenarioId: "e1", + groundTruth: { inline: { assertions: [{ text: "polite" }] } }, + }, + { sessionId: "s2", testScenarioId: "e2" }, + ], + }); + }); + + test("refuses to grade when nothing was invoked", async () => { + await expect( + run(BASE, (core) => + core.eval.setInvokeDatasetResponse({ sessions: [], invoked: 0, failed: 3 }), + ), + ).rejects.toThrow(/no examples could be invoked \(3 failed\)/); + }); }); diff --git a/src/handlers/eval/types.tsx b/src/handlers/eval/types.tsx index 07687048e..e5a407cab 100644 --- a/src/handlers/eval/types.tsx +++ b/src/handlers/eval/types.tsx @@ -34,6 +34,7 @@ import type { ListBatchEvaluationsResponse, StartBatchEvaluationResponse, SessionMetadataShape, + InlineGroundTruth, EvaluationReferenceInput, EvaluationResultContent, DataSourceConfig as DataPlaneDataSourceConfig, @@ -205,34 +206,37 @@ export type StartBatchEvaluationInput = { kmsKeyArn?: string; }; -// SimulateInput is the CLI-facing shape for `batch-evaluation simulate` — dataset -// replay + batch grade. SDK-native fields only (no RuntimeInvokeRequest/EvaluateInput -// leak): Core invokes each scenario against the runtime, then submits a batch -// evaluation scoped to the sessions it created. Invoke fields mirror `runtime invoke`. -export type SimulateInput = { +// InvokeDatasetInput is the runtime-level shape for replaying a dataset: invoke each +// example against the runtime, one client-generated session per example. Runtime fields +// only — no evaluator/name/kms (those belong to the grader the handler composes on top, +// e.g. startBatchEvaluation). Invoke fields mirror `runtime invoke`. +export type InvokeDatasetInput = { runtimeId: string; qualifier?: string; - payloadTemplate: string; // e.g. {"prompt":"{input}"} — {input} is the scenario's turn input + payloadTemplate: string; // e.g. {"prompt":"{input}"} — {input} is the example's turn input headers?: [string, string][]; bearerToken?: string; - // simulate always creates a fresh session per scenario. Reusing one session - // across scenarios interleaves unrelated turns and collides ground-truth keys. userId?: string; dataset: string; // local JSONL path or a dataset id datasetVersion?: string; - evaluatorIds: string[]; - name: string; - description?: string; - kmsKeyArn?: string; }; -// SimulateResult reports the submitted job plus how many scenarios were actually -// invoked vs dropped (a failed invoke is skipped, not fatal, unless all fail). -export type SimulateResult = { - batchEvaluationId?: string; - status?: string; - scenariosInvoked: number; - scenariosFailed: number; +// InvokedSession is one replayed example: the session created for it plus its neutral +// ground truth. Grader-agnostic — the batch handler wraps `groundTruth` as +// SessionMetadataShape; a future ondemand handler adapts it to EvaluationReferenceInput. +export type InvokedSession = { + exampleId: string; + sessionId: string; + groundTruth?: InlineGroundTruth; +}; + +// InvokeDatasetResult reports the created sessions plus how many examples were invoked +// vs dropped (a failed invoke is skipped, not fatal). firstError explains a total failure. +export type InvokeDatasetResult = { + sessions: InvokedSession[]; + invoked: number; + failed: number; + firstError?: Error; }; // SpanRecord is one OTel span/log document — the parsed `@message` JSON of a @@ -359,14 +363,14 @@ export interface CoreEvalClient { // Evaluate API and returns per-session scores. No job, no CloudWatch — the // trace read happened in getTracesForAgent. evaluate(input: EvaluateInput, options: CoreOptions): Promise; - // simulate replays a dataset against the runtime (invoke per scenario, client-side) - // then submits a batch evaluation over the sessions it created. No dataset API - // exists service-side, so the CLI creates the sessions and the service grades them. - simulate( - input: SimulateInput, + // invokeDataset replays a dataset against the runtime (invoke per example, client-side, + // one session each) and returns the created sessions + neutral ground truth. Grader- + // agnostic: the handler composes it with startBatchEvaluation (or, later, evaluate). + invokeDataset( + input: InvokeDatasetInput, options: CoreOptions, signal?: AbortSignal, - ): Promise; + ): Promise; createOnlineEvaluationConfig( input: CreateOnlineEvalInput, diff --git a/src/testing/TestCoreClient.tsx b/src/testing/TestCoreClient.tsx index 63392c571..f53012ee1 100644 --- a/src/testing/TestCoreClient.tsx +++ b/src/testing/TestCoreClient.tsx @@ -133,8 +133,8 @@ import type { DatasetUpdateProgressEvent, EvaluateInput, EvaluateResult, - SimulateInput, - SimulateResult, + InvokeDatasetInput, + InvokeDatasetResult, GetBatchEvaluationResult, GetTracesInput, LlmAsAJudgeUpdate, @@ -1405,11 +1405,10 @@ export class TestEvalClient implements CoreEvalClient { sessionsEvaluated: 0, results: [], }; - private simulateResponse: SimulateResult = { - batchEvaluationId: "batch-eval-test", - status: "RUNNING", - scenariosInvoked: 0, - scenariosFailed: 0, + private invokeDatasetResponse: InvokeDatasetResult = { + sessions: [], + invoked: 0, + failed: 0, }; private error?: Error; @@ -1730,20 +1729,20 @@ export class TestEvalClient implements CoreEvalClient { return this.evaluateResponse; } - // setSimulateResponse sets what simulate resolves to (when not erroring). - setSimulateResponse(response: SimulateResult): this { - this.simulateResponse = response; + // setInvokeDatasetResponse sets what invokeDataset resolves to (when not erroring). + setInvokeDatasetResponse(response: InvokeDatasetResult): this { + this.invokeDatasetResponse = response; return this; } - async simulate( - input: SimulateInput, + async invokeDataset( + input: InvokeDatasetInput, options: CoreOptions, signal?: AbortSignal, - ): Promise { - this.calls.push({ method: "simulate", args: [input, options, signal] }); + ): Promise { + this.calls.push({ method: "invokeDataset", args: [input, options, signal] }); if (this.error) throw this.error; - return this.simulateResponse; + return this.invokeDatasetResponse; } async createOnlineEvaluationConfig( From cb7b78b937a1934dc743fb5f87d4eb15837aaae9 Mon Sep 17 00:00:00 2001 From: jariy17 Date: Tue, 18 Aug 2026 20:32:58 +0000 Subject: [PATCH 2/4] fix(eval): reject non-object dataset rows and turn entries instead of crashing A JSONL line that is valid JSON but not an object (e.g. bare `null`) threw a raw TypeError (`null is not an object`) when DatasetLoader dereferenced `row.example_id`; the same happened in PredefinedExample for a non-object turn entry (`turns: [null]`). Both now surface a clear InputValidationError at the parse boundary. Adds guard tests plus behavior locks for CRLF endings, unicode ids, empty assertions/trajectory omission, and `expected_response: ""`. --- src/core/eval/dataset/dataset.test.ts | 54 +++++++++++++++++++++++++++ src/core/eval/dataset/load.ts | 10 ++++- src/core/eval/dataset/predefined.ts | 16 ++++++-- 3 files changed, 74 insertions(+), 6 deletions(-) diff --git a/src/core/eval/dataset/dataset.test.ts b/src/core/eval/dataset/dataset.test.ts index 26c615c8e..9c5efb5a6 100644 --- a/src/core/eval/dataset/dataset.test.ts +++ b/src/core/eval/dataset/dataset.test.ts @@ -81,6 +81,29 @@ describe("DatasetLoader.load", () => { ); expect(examples.map((e) => e.exampleId)).toEqual(["a", "b"]); }); + + // `null` is valid JSON but has no fields; dereferencing it once threw a raw TypeError + // instead of a clean validation error. + test.each([["null"], ["[1,2,3]"], ["42"], ['"hi"'], ["true"]])( + "rejects a non-object row (%s) with a clear error", + (line) => { + expect(() => DatasetLoader.load(line)).toThrow(/not a JSON object/); + }, + ); + + test("handles CRLF line endings", () => { + const crlf = + row({ example_id: "a", turns: [{ input: "1" }] }) + + "\r\n" + + row({ example_id: "b", turns: [{ input: "2" }] }) + + "\r\n"; + expect(DatasetLoader.load(crlf).map((e) => e.exampleId)).toEqual(["a", "b"]); + }); + + test("preserves unicode example ids", () => { + const [e] = DatasetLoader.load(row({ example_id: "café-日本-🎉", turns: [{ input: "1" }] })); + expect(e!.exampleId).toBe("café-日本-🎉"); + }); }); describe("SimulatedExample", () => { @@ -94,6 +117,37 @@ describe("PredefinedExample", () => { expect(() => new PredefinedExample("x", { turns: [] })).toThrow(/has no turns/); }); + test("constructor rejects a non-object turn entry", () => { + expect(() => new PredefinedExample("x", { turns: [null] })).toThrow(/turn 1 is not an object/); + }); + + test("omits empty assertions and expected_trajectory arrays", async () => { + const { ctx } = recordingCtx(); + // Only the expectation-bearing turn should survive to ground truth; the empty + // assertions/trajectory arrays are dropped (the service rejects zero-length ones). + const gt = await new PredefinedExample("x", { + turns: [{ input: "t1", expected_response: "r1" }], + assertions: [], + expected_trajectory: [], + }).run(ctx); + expect(gt).toBeDefined(); + expect(gt!.assertions).toBeUndefined(); + expect(gt!.expectedTrajectory).toBeUndefined(); + expect(gt!.turns).toHaveLength(1); + }); + + // A deliberate `expected_response: ""` means "expect an empty reply" — distinct from + // omitting the field. The `!== undefined` guard honors it: the turn carries + // expectedResponse { text: "" } rather than being treated as expectation-less. + test('treats expected_response "" as a real expectation', async () => { + const { ctx } = recordingCtx(); + const gt = await new PredefinedExample("x", { + turns: [{ input: "t1", expected_response: "" }], + }).run(ctx); + expect(gt!.turns).toHaveLength(1); + expect(gt!.turns![0]!.expectedResponse).toEqual({ text: "" }); + }); + test("run replays every turn in order, on one session", async () => { const { ctx, calls } = recordingCtx(); await new PredefinedExample("x", { turns: [{ input: "a" }, { input: "b" }] }).run(ctx); diff --git a/src/core/eval/dataset/load.ts b/src/core/eval/dataset/load.ts index 516e94191..8aba42160 100644 --- a/src/core/eval/dataset/load.ts +++ b/src/core/eval/dataset/load.ts @@ -14,12 +14,18 @@ export class DatasetLoader { for (const line of text.split("\n")) { const trimmed = line.trim(); if (!trimmed) continue; - let row: Record; + let parsed: unknown; try { - row = JSON.parse(trimmed) as Record; + parsed = JSON.parse(trimmed); } catch { throw new InputValidationError("dataset contains a line that is not valid JSON"); } + // Reject non-object rows before dereferencing: `null` (valid JSON) would throw a + // raw TypeError, and an array/primitive can never carry the fields a row needs. + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { + throw new InputValidationError("dataset contains a line that is not a JSON object"); + } + const row = parsed as Record; // The id is the join key between a session and its ground truth, so a missing or // duplicate id silently misassigns ground truth to the wrong session. diff --git a/src/core/eval/dataset/predefined.ts b/src/core/eval/dataset/predefined.ts index 5695f242c..fcc626251 100644 --- a/src/core/eval/dataset/predefined.ts +++ b/src/core/eval/dataset/predefined.ts @@ -19,10 +19,18 @@ export class PredefinedExample implements Example { readonly exampleId: string, row: Record, ) { - const turns = (Array.isArray(row.turns) ? row.turns : []).map((t: Record) => ({ - input: String(t.input ?? ""), - expectedResponse: t.expected_response as string | undefined, - })); + const turns = (Array.isArray(row.turns) ? row.turns : []).map((t: unknown, i: number) => { + // A non-object entry (e.g. `null`) has no fields to read; dereferencing it would + // throw a raw TypeError, so reject it here with the same boundary-validation intent. + if (typeof t !== "object" || t === null) { + throw new InputValidationError(`example "${exampleId}" turn ${i + 1} is not an object`); + } + const turn = t as Record; + return { + input: String(turn.input ?? ""), + expectedResponse: turn.expected_response as string | undefined, + }; + }); if (turns.length === 0) { throw new InputValidationError(`example "${exampleId}" has no turns`); } From 347534754152a5d1edd5e386132f46b8acc796d1 Mon Sep 17 00:00:00 2001 From: jariy17 Date: Tue, 18 Aug 2026 20:33:04 +0000 Subject: [PATCH 3/4] test(eval): cover runExamples failure isolation + handler sessionMetadata golden runExamples: lock per-example failure isolation (one worker throws, the rest run and are returned; firstError captured and non-Error throws wrapped), exactly-once processing, the concurrency bound, and the empty-input case. Adds a golden snapshot of the sessionMetadata the simulate handler builds, pinning the `{ inline: gt }` wrapping and the omitted-member case for a session with no GT. --- src/core/eval/dataset/run.test.ts | 62 +++++++++++++++++++ .../__snapshots__/simulate.test.tsx.snap | 23 +++++++ .../simulate/simulate.test.tsx | 10 +++ 3 files changed, 95 insertions(+) create mode 100644 src/core/eval/dataset/run.test.ts create mode 100644 src/handlers/eval/batch-evaluation/simulate/__snapshots__/simulate.test.tsx.snap diff --git a/src/core/eval/dataset/run.test.ts b/src/core/eval/dataset/run.test.ts new file mode 100644 index 000000000..7ebe17629 --- /dev/null +++ b/src/core/eval/dataset/run.test.ts @@ -0,0 +1,62 @@ +import { test, expect, describe } from "bun:test"; +import { runExamples } from "./run"; + +describe("runExamples", () => { + test("isolates a failing worker: it is counted, the rest still run", async () => { + const { ok, failed, firstError } = await runExamples([1, 2, 3, 4], async (n) => { + if (n === 2) throw new Error("boom 2"); + return n * 10; + }); + // The three survivors return; order among them is completion order, so compare as a set. + expect(new Set(ok)).toEqual(new Set([10, 30, 40])); + expect(failed).toBe(1); + expect(firstError?.message).toBe("boom 2"); + }); + + test("wraps a non-Error throw as an Error for firstError", async () => { + const { failed, firstError } = await runExamples([1], async () => { + throw "just a string"; + }); + expect(failed).toBe(1); + expect(firstError).toBeInstanceOf(Error); + expect(firstError?.message).toBe("just a string"); + }); + + test("processes every item exactly once", async () => { + const seen: number[] = []; + const items = Array.from({ length: 23 }, (_, i) => i); + const { ok } = await runExamples(items, async (n) => { + seen.push(n); + return n; + }); + expect(ok).toHaveLength(23); + expect([...seen].sort((a, b) => a - b)).toEqual(items); + }); + + test("never exceeds the concurrency bound", async () => { + let inFlight = 0; + let peak = 0; + await runExamples( + Array.from({ length: 20 }, (_, i) => i), + async () => { + inFlight++; + peak = Math.max(peak, inFlight); + await new Promise((r) => setTimeout(r, 1)); + inFlight--; + }, + 3, + ); + expect(peak).toBeLessThanOrEqual(3); + }); + + test("empty input runs no workers and reports nothing invoked", async () => { + let called = false; + const { ok, failed, firstError } = await runExamples([], async () => { + called = true; + }); + expect(called).toBe(false); + expect(ok).toEqual([]); + expect(failed).toBe(0); + expect(firstError).toBeUndefined(); + }); +}); diff --git a/src/handlers/eval/batch-evaluation/simulate/__snapshots__/simulate.test.tsx.snap b/src/handlers/eval/batch-evaluation/simulate/__snapshots__/simulate.test.tsx.snap new file mode 100644 index 000000000..ddc632555 --- /dev/null +++ b/src/handlers/eval/batch-evaluation/simulate/__snapshots__/simulate.test.tsx.snap @@ -0,0 +1,23 @@ +// Bun Snapshot v1, https://bun.sh/docs/test/snapshots + +exports[`eval batch-evaluation simulate builds the sessionMetadata ground-truth shape [golden] 1`] = ` +[ + { + "groundTruth": { + "inline": { + "assertions": [ + { + "text": "polite", + }, + ], + }, + }, + "sessionId": "s1", + "testScenarioId": "e1", + }, + { + "sessionId": "s2", + "testScenarioId": "e2", + }, +] +`; diff --git a/src/handlers/eval/batch-evaluation/simulate/simulate.test.tsx b/src/handlers/eval/batch-evaluation/simulate/simulate.test.tsx index d840ba204..2375a3b3f 100644 --- a/src/handlers/eval/batch-evaluation/simulate/simulate.test.tsx +++ b/src/handlers/eval/batch-evaluation/simulate/simulate.test.tsx @@ -162,6 +162,16 @@ describe("eval batch-evaluation simulate", () => { }); }); + // Golden: the exact evaluationMetadata (sessionMetadata) the handler builds from the + // invoked sessions. Locks the `{ inline: gt }` wrapping and the omitted-member case for + // a session with no ground truth — the wire shape the batch service reads. + test("builds the sessionMetadata ground-truth shape [golden]", async () => { + const { core } = await run(BASE); + const start = core.eval.calls.find((c) => c.method === "startBatchEvaluation"); + const input = start!.args[0] as { groundTruth: unknown }; + expect(input.groundTruth).toMatchSnapshot(); + }); + test("refuses to grade when nothing was invoked", async () => { await expect( run(BASE, (core) => From 98f4b1abab7d43101c748fd8a9bf6077f27b199e Mon Sep 17 00:00:00 2001 From: jariy17 Date: Tue, 18 Aug 2026 20:45:57 +0000 Subject: [PATCH 4/4] style(eval): trim code comments to why-not-how, drop restatements --- src/core/eval.tsx | 23 +++++++------------ src/core/eval/dataset/load.ts | 20 +++++----------- src/core/eval/dataset/predefined.ts | 23 ++++++------------- src/core/eval/dataset/run.ts | 6 ++--- src/core/eval/dataset/simulated.ts | 8 ++----- src/core/eval/dataset/types.ts | 13 ++++------- .../eval/batch-evaluation/simulate/index.tsx | 9 +++----- 7 files changed, 33 insertions(+), 69 deletions(-) diff --git a/src/core/eval.tsx b/src/core/eval.tsx index e53c89cf5..1ed650d71 100644 --- a/src/core/eval.tsx +++ b/src/core/eval.tsx @@ -521,14 +521,11 @@ export class EvalClient implements CoreEvalClient { options: CoreOptions, signal?: AbortSignal, ): Promise { - // readDatasetText owns the local-vs-id fetch + temp cleanup; DatasetLoader is the - // pure parse into Example instances (each dispatches its own replay via run()). const examples = DatasetLoader.load( await this.readDatasetText(input.dataset, input.datasetVersion, options, signal), ); - // Resolve the runtime once; normalizeRuntimeInvokeRequest validates auth + fills - // accountId. Invoke is the extracted free fn — no RuntimeClient dependency. + // Resolve the runtime once, reused for every session. const runtime = await this.clients .control(toClientConfig(options)) .send(new GetAgentRuntimeCommand({ agentRuntimeId: input.runtimeId }), { @@ -537,9 +534,8 @@ export class EvalClient implements CoreEvalClient { const deps = { clients: this.clients, fetch: this.fetch, logger: this.logger }; const { ok, failed, firstError } = await runExamples(examples, async (example) => { - // One session per example; the id is client-generated (a client-owned input per - // the AgentCore docs, needed before any response) and reused across turns so the - // conversation and its per-turn traces accumulate in order. + // One session per example; the id is a client-owned input per the AgentCore docs, + // reused across turns so the conversation and its per-turn traces stay in order. const sessionId = randomUUID(); const ctx: RunContext = { invokeOnce: async (payload) => { @@ -555,8 +551,7 @@ export class EvalClient implements CoreEvalClient { runtimeUserId: input.userId, }); const response = await invokeRuntime(deps, request, options, signal); - // Read the body to completion (frees the socket, feeds an actor loop later); - // a scripted example ignores the returned text. + // Read to completion to free the socket; a scripted example ignores the text. let text = ""; const decoder = new TextDecoder(); for await (const chunk of response.body) text += decoder.decode(chunk, { stream: true }); @@ -579,9 +574,8 @@ export class EvalClient implements CoreEvalClient { this.logger.warn(`invokeDataset: ${failed} example(s) failed to invoke and were dropped`); } - // AgentCore takes ~30s-3min to emit spans for a freshly invoked session; grade too - // early and the service reads an empty log group and marks every session failed. Wait - // once. Skipped when disabled via `SIMULATE_INGESTION_WAIT_MS=0` (tests). + // AgentCore emits spans ~30s-3min after invoke; grade too early and it reads an empty + // log group and fails every session. Disabled via SIMULATE_INGESTION_WAIT_MS=0 (tests). const waitMs = Number(process.env.SIMULATE_INGESTION_WAIT_MS ?? 180_000); if (ok.length > 0 && waitMs > 0) { this.logger.info( @@ -593,9 +587,8 @@ export class EvalClient implements CoreEvalClient { return { sessions: ok, invoked: ok.length, failed, firstError }; } - // readDatasetText resolves a dataset ref to its JSONL text: a local path directly, else - // download the dataset id to a temp file (cleaned up here). Both funnel through the - // shared readLocalDatasetFile so replay and updateDatasetExamples read files the same way. + // Resolve a dataset ref to JSONL text: a local path directly, else download the id to a + // temp file (cleaned up here). Reuses readLocalDatasetFile so replay reads like update. private async readDatasetText( ref: string, version: string | undefined, diff --git a/src/core/eval/dataset/load.ts b/src/core/eval/dataset/load.ts index 8aba42160..86c948d35 100644 --- a/src/core/eval/dataset/load.ts +++ b/src/core/eval/dataset/load.ts @@ -4,9 +4,7 @@ import type { Example } from "./types"; import { PredefinedExample } from "./predefined"; import { SimulatedExample } from "./simulated"; -// DatasetLoader parses dataset JSONL into Example instances. Pure — no I/O, no AWS — so -// it's unit-testable with a plain string; fetching the text (local file or dataset id) is -// the caller's job. +// Pure parse (no I/O) so it's testable with a plain string; the caller fetches the text. export class DatasetLoader { static load(text: string): Example[] { const examples: Example[] = []; @@ -20,15 +18,13 @@ export class DatasetLoader { } catch { throw new InputValidationError("dataset contains a line that is not valid JSON"); } - // Reject non-object rows before dereferencing: `null` (valid JSON) would throw a - // raw TypeError, and an array/primitive can never carry the fields a row needs. + // Reject non-object rows before dereferencing — `null` is valid JSON and would throw. if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { throw new InputValidationError("dataset contains a line that is not a JSON object"); } const row = parsed as Record; - // The id is the join key between a session and its ground truth, so a missing or - // duplicate id silently misassigns ground truth to the wrong session. + // The id joins a session to its ground truth — a missing/duplicate one misassigns it. const exampleId = String(row.example_id ?? row.scenario_id ?? ""); if (!exampleId) { throw new InputValidationError("dataset example is missing 'example_id'"); @@ -45,9 +41,7 @@ export class DatasetLoader { } // Classify by row shape — a local JSONL carries no schemaType, and AWS's own SDK - // dispatches this way, so a file the SDK accepts this CLI must accept too. Refuse a - // both-row: AWS's `if "turns" in raw` silently drops the actor profile, which reads as - // a passing run of the wrong test. + // dispatches this way. Refuse a both-row rather than silently dropping the actor profile. private static build(row: Record, exampleId: string): Example { const hasTurns = Array.isArray(row.turns); const hasActor = row.actor_profile != null; @@ -62,10 +56,8 @@ export class DatasetLoader { ); } - // `: DatasetSchemaType` types the scrutinee as the full SDK enum, so when a member is - // added the switch stops being exhaustive, build can reach its end without returning, - // and the compiler flags it (TS2366). The `new X(exampleId, row)` sites enforce the - // constructor shape — no map, no assertNever. + // Typed as the full SDK enum so a new member makes the switch non-exhaustive and + // build fails to compile (TS2366) — the build guard, no map or assertNever needed. const schemaType: DatasetSchemaType = hasTurns ? "AGENTCORE_EVALUATION_PREDEFINED_V1" : "AGENTCORE_EVALUATION_SIMULATED_V1"; diff --git a/src/core/eval/dataset/predefined.ts b/src/core/eval/dataset/predefined.ts index fcc626251..cd6c61d71 100644 --- a/src/core/eval/dataset/predefined.ts +++ b/src/core/eval/dataset/predefined.ts @@ -4,24 +4,18 @@ import type { Example, RunContext } from "./types"; type Turn = { input: string; expectedResponse?: string }; -// A predefined example has scripted turns: replay each verbatim, ignore the reply. Owns -// its parse (from the raw row) and its ground-truth mapping — everything predefined in -// one place. export class PredefinedExample implements Example { readonly schemaType = "AGENTCORE_EVALUATION_PREDEFINED_V1" as const; readonly turns: Turn[]; readonly assertions?: string[]; readonly expectedTrajectory?: string[]; - // Parsing happens in the constructor (validation-at-boundary). Fields are assigned in - // the body, not field initializers, so there's no "used before init" hazard. constructor( readonly exampleId: string, row: Record, ) { const turns = (Array.isArray(row.turns) ? row.turns : []).map((t: unknown, i: number) => { - // A non-object entry (e.g. `null`) has no fields to read; dereferencing it would - // throw a raw TypeError, so reject it here with the same boundary-validation intent. + // Reject a non-object entry here; dereferencing it below would throw a raw TypeError. if (typeof t !== "object" || t === null) { throw new InputValidationError(`example "${exampleId}" turn ${i + 1} is not an object`); } @@ -39,17 +33,16 @@ export class PredefinedExample implements Example { this.expectedTrajectory = row.expected_trajectory as string[] | undefined; } - // Turns share one session, so they run sequentially and awaited: racing them would - // interleave the conversation and misalign the per-turn traces with the ground truth. + // Sequential and awaited: the turns share one session, so racing them would interleave + // the conversation and misalign per-turn traces with the ground truth. async run(ctx: RunContext): Promise { for (const turn of this.turns) await ctx.invokeOnce(turn.input); return this.groundTruth(); } - // One entry per turn, each carrying its prompt in `input`, so a turn with no expected - // response still occupies its slot. Filtering the sparse turns out renumbers the rest, - // scoring turn 3's expectation against turn 1; the service's alignment rule is - // undocumented, so carrying the prompt is correct whether it aligns by index or content. + // Emit every turn (carrying its prompt), not just those with an expectation: filtering + // renumbers the rest, scoring turn 3's expectation against turn 1. The service's + // alignment rule is undocumented, so the prompt keeps index and content matching both valid. private groundTruth(): InlineGroundTruth | undefined { const turns = this.turns.some((t) => t.expectedResponse !== undefined) ? this.turns.map((t) => ({ @@ -61,15 +54,13 @@ export class PredefinedExample implements Example { : []; const assertions = this.assertions?.map((text) => ({ text })); const inline: InlineGroundTruth = { - // Omit empty arrays: the service rejects zero-length `assertions`/`turns` - // (documented min-1) rather than reading them as "no data". + // Omit empty arrays — the service rejects zero-length assertions/turns (min-1). ...(assertions && assertions.length > 0 && { assertions }), ...(this.expectedTrajectory?.length && { expectedTrajectory: { toolNames: this.expectedTrajectory }, }), ...(turns.length > 0 && { turns }), }; - // An all-empty inline is not "no ground truth" — return undefined so the caller omits it. return Object.keys(inline).length > 0 ? inline : undefined; } } diff --git a/src/core/eval/dataset/run.ts b/src/core/eval/dataset/run.ts index 1c9aeabc1..6f82a98b9 100644 --- a/src/core/eval/dataset/run.ts +++ b/src/core/eval/dataset/run.ts @@ -1,7 +1,5 @@ -// runExamples runs `worker` over every item with bounded concurrency. A failed worker -// doesn't sink the run — the failure is counted (so the caller can report on all-failed) -// and its item dropped. Returns ok results + the first error, which the caller surfaces -// to explain a total failure. Generic in the item type; not eval-specific. +// A failed worker is counted and dropped, not thrown — the caller reports all-failed via +// firstError. Bounded concurrency because each item invokes a live runtime. export type ExampleRun = { ok: Result[]; failed: number; firstError?: Error }; export async function runExamples( diff --git a/src/core/eval/dataset/simulated.ts b/src/core/eval/dataset/simulated.ts index 7428fef84..e5f9d7076 100644 --- a/src/core/eval/dataset/simulated.ts +++ b/src/core/eval/dataset/simulated.ts @@ -2,10 +2,8 @@ import type { InlineGroundTruth } from "@aws-sdk/client-bedrock-agentcore"; import { NotImplementedError } from "../../../errors"; import type { Example, RunContext } from "./types"; -// A simulated example carries an actor profile instead of scripted turns: replaying it -// needs an LLM "user" to generate each next message from the agent's reply, which this -// command does not run yet. Throw at construction (= load time) so the user fails early -// with a clear message rather than a per-row "has no turns" misdiagnosis mid-run. +// Not shipped: replaying a simulated example needs an LLM actor we don't run yet. Throw +// at construction (= load time) so the user fails early, not with a per-row misdiagnosis. export class SimulatedExample implements Example { readonly schemaType = "AGENTCORE_EVALUATION_SIMULATED_V1" as const; @@ -19,8 +17,6 @@ export class SimulatedExample implements Example { ); } - // Unreachable today (the constructor throws). Implements the interface; the actor loop - // lands here when simulated ships. run(_ctx: RunContext): Promise { throw new NotImplementedError("simulated example replay is not implemented"); } diff --git a/src/core/eval/dataset/types.ts b/src/core/eval/dataset/types.ts index d6a9faf72..17ffce7b4 100644 --- a/src/core/eval/dataset/types.ts +++ b/src/core/eval/dataset/types.ts @@ -1,18 +1,15 @@ import type { DatasetSchemaType } from "@aws-sdk/client-bedrock-agentcore-control"; import type { InlineGroundTruth } from "@aws-sdk/client-bedrock-agentcore"; -// TurnResult is the agent's reply to one turn. A record (not a bare string) so a future -// tool-branching dataset type can widen it by a field without touching every example. +// A record, not a bare string, so a future tool-branching type can widen it by a field. export type TurnResult = { text: string }; -// RunContext is the per-session transport handed to run(): one call = one turn. The -// session id, auth, and payload templating are bound by the caller (the machine), so an -// example only decides what to say next, never how the request is built. +// The per-session transport handed to run(): one call = one turn. Session id, auth, and +// templating are bound by the caller, so an example only decides what to say next. export type RunContext = { invokeOnce: (payload: string) => Promise }; -// Example is the contract each dataset type implements: identity plus a self-describing -// run. An interface, not a base class — there is no shared state or behaviour to inherit, -// and the machine dispatches by calling run(), so nothing needs a common superclass. +// An interface, not a base class: no shared state to inherit, and the machine dispatches +// by calling run(). export interface Example { readonly schemaType: DatasetSchemaType; readonly exampleId: string; diff --git a/src/handlers/eval/batch-evaluation/simulate/index.tsx b/src/handlers/eval/batch-evaluation/simulate/index.tsx index 56fc0bbce..ad6f5c4d0 100644 --- a/src/handlers/eval/batch-evaluation/simulate/index.tsx +++ b/src/handlers/eval/batch-evaluation/simulate/index.tsx @@ -7,9 +7,8 @@ import type { Core } from "../../../types"; import { coreOptsFromCtx } from "../../../utils"; import { parseRuntimeInvokeHeaders } from "../../../runtime/invoke/request"; -// batch-evaluation simulate replays a dataset against a runtime (invoke per scenario) -// then submits a batch evaluation over the sessions it created. Invoke flags mirror -// `runtime invoke`; content-type/accept are fixed to application/json. +// Composes invokeDataset (replay) → startBatchEvaluation (grade). Invoke flags mirror +// `runtime invoke`. export const createSimulateBatchEvaluationHandler = (core: Core, _io: AppIO) => createHandler({ name: "simulate", @@ -59,7 +58,6 @@ export const createSimulateBatchEvaluationHandler = (core: Core, _io: AppIO) => try { const opts = coreOptsFromCtx(ctx); - // (1) Replay the dataset → one graded-ready session per example. const r = await core.eval.invokeDataset( { runtimeId: flags["runtime-id"], @@ -81,8 +79,7 @@ export const createSimulateBatchEvaluationHandler = (core: Core, _io: AppIO) => ); } - // (2) Grade via the batch service — the example's neutral ground truth crosses - // over as sessionMetadata (inline arm). + // The example's neutral ground truth crosses over as sessionMetadata (inline arm). const job = await core.eval.startBatchEvaluation( { name: flags["name"],