From 94c0500dcb664da448cc2fe012c140626d20b041 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Fri, 10 Jul 2026 18:00:41 +0200 Subject: [PATCH 01/11] refactor(appkit): dedupe databricks credential resolution in agent eval CLI Signed-off-by: MarioCadenas --- .../shared/src/cli/commands/agent/eval.ts | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/shared/src/cli/commands/agent/eval.ts b/packages/shared/src/cli/commands/agent/eval.ts index 5e0fefd3d..7ff54f140 100644 --- a/packages/shared/src/cli/commands/agent/eval.ts +++ b/packages/shared/src/cli/commands/agent/eval.ts @@ -197,24 +197,24 @@ async function runAgentEval( ): Promise { const runner = await loadRunner(); + // Databricks credentials shared by auth resolution and the workspace client: + // an explicit flag/DATABRICKS_* env wins, else the SDK resolves from the CLI + // profile. + const credentials = { + profile: opts.profile ?? process.env.DATABRICKS_CONFIG_PROFILE, + host: opts.databricksHost ?? process.env.DATABRICKS_HOST, + token: opts.databricksToken ?? process.env.DATABRICKS_TOKEN, + }; + // Resolve Databricks host + bearer the AppKit-native way: an explicit - // host/token (or DATABRICKS_* env) wins; otherwise the SDK mints an OAuth - // token from the CLI profile — so no hand-set PAT is required. - const auth: Auth = - (await runner.resolveDatabricksAuth({ - profile: opts.profile ?? process.env.DATABRICKS_CONFIG_PROFILE, - host: opts.databricksHost ?? process.env.DATABRICKS_HOST, - token: opts.databricksToken ?? process.env.DATABRICKS_TOKEN, - })) ?? {}; + // host/token wins; otherwise the SDK mints an OAuth token from the CLI + // profile — so no hand-set PAT is required. + const auth: Auth = (await runner.resolveDatabricksAuth(credentials)) ?? {}; // Managed-dataset reads: a workspace client (same profile/host/token) + a SQL // warehouse. Only needed by evals that declare `dataset`. const warehouseId = opts.warehouseId ?? process.env.DATABRICKS_WAREHOUSE_ID; - const workspaceClient = runner.resolveWorkspaceClient({ - profile: opts.profile ?? process.env.DATABRICKS_CONFIG_PROFILE, - host: opts.databricksHost ?? process.env.DATABRICKS_HOST, - token: opts.databricksToken ?? process.env.DATABRICKS_TOKEN, - }); + const workspaceClient = runner.resolveWorkspaceClient(credentials); let summary: EvalRunSummary; try { From 3217c7f5735ee5e678064371bbf80bc97ff7d8d0 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Fri, 10 Jul 2026 18:20:36 +0200 Subject: [PATCH 02/11] feat(appkit): assert tool-call arguments in agent evals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The eval driver captured tool-call names but discarded their arguments. Parse the function-call `arguments` JSON into `DriveResult.toolCallDetails` (the later `done` event's fuller args win over the initial `added`), and expose `t.calledToolWith(name, expected)` — passes when a call to `name` had args that deep-contain `expected` (nested-aware partial match; extra args ignored). Gate by default, like `calledTool`. `toolCalls: string[]` is unchanged. Signed-off-by: MarioCadenas --- packages/appkit/src/evals/http-driver.ts | 60 ++++++++++--- packages/appkit/src/evals/run-eval.ts | 36 ++++++++ .../src/evals/tests/http-driver.test.ts | 84 ++++++++++++++++++- .../appkit/src/evals/tests/run-eval.test.ts | 84 ++++++++++++++++++- packages/appkit/src/evals/types.ts | 11 +++ 5 files changed, 262 insertions(+), 13 deletions(-) diff --git a/packages/appkit/src/evals/http-driver.ts b/packages/appkit/src/evals/http-driver.ts index 4b113c359..1664ca25c 100644 --- a/packages/appkit/src/evals/http-driver.ts +++ b/packages/appkit/src/evals/http-driver.ts @@ -26,22 +26,34 @@ export interface HttpDriverOptions { /** Mutable running totals accumulated while draining one turn's SSE stream. */ interface DriveState { reply: string; - toolCalls: string[]; - seen: Set; + /** Captured tool calls, keyed by `call_id ?? name` (dedupe). */ + toolCalls: Map; ok: boolean; traceId?: string; } -/** Record a `function_call` output item once per call id (deduped). */ +/** + * Record a `function_call` output item, keyed by `call_id ?? name`, capturing + * its parsed arguments. A later `done` event's fuller args win over the initial + * `added` event's (often empty) args. + */ function recordToolCall( - item: { type?: string; name?: string; call_id?: string } | undefined, + item: + | { type?: string; name?: string; call_id?: string; arguments?: string } + | undefined, state: DriveState, ): void { if (item?.type !== "function_call" || !item.name) return; const key = item.call_id ?? item.name; - if (state.seen.has(key)) return; - state.seen.add(key); - state.toolCalls.push(item.name); + const args = parseArgs(item.arguments); + const existing = state.toolCalls.get(key); + if (!existing) { + state.toolCalls.set(key, { name: item.name, args }); + } else if (Object.keys(args).length > 0) { + // The initial `added` event may carry empty args while the later `done` + // carries the full arguments — keep the fuller set. + existing.args = args; + } } /** Apply an `appkit.metadata` event's thread/trace ids. */ @@ -54,6 +66,24 @@ function applyMetadata( if (data?.mlflowTraceId) state.traceId = data.mlflowTraceId; } +/** A single captured tool call, deduped by `call_id ?? name`. */ +type ToolCall = { name: string; args: Record }; + +/** Parse a function-call `arguments` JSON string; `{}` on missing/invalid. */ +function parseArgs(raw: unknown): Record { + if (typeof raw !== "string" || raw.trim() === "") return {}; + try { + const parsed = JSON.parse(raw); + return parsed !== null && + typeof parsed === "object" && + !Array.isArray(parsed) + ? (parsed as Record) + : {}; + } catch { + return {}; + } +} + /** Parse a single Responses-API SSE `data:` payload into the running totals. */ function applyEvent( event: Record, @@ -73,6 +103,7 @@ function applyEvent( type?: string; name?: string; call_id?: string; + arguments?: string; content?: Array<{ text?: string }>; }; recordToolCall(item, state); @@ -147,13 +178,19 @@ export function createHttpDriver(options: HttpDriverOptions): EvalDriver { signal, }); } catch { - return { reply: "", toolCalls: [], succeeded: false }; + return { + reply: "", + toolCalls: [], + toolCallDetails: [], + succeeded: false, + }; } if (!res.ok || !res.body) { return { reply: "", toolCalls: [], + toolCallDetails: [], succeeded: false, sessionId: threadId, }; @@ -161,8 +198,7 @@ export function createHttpDriver(options: HttpDriverOptions): EvalDriver { const state: DriveState = { reply: "", - toolCalls: [], - seen: new Set(), + toolCalls: new Map(), ok: true, }; const setThread = (id: string) => { @@ -193,9 +229,11 @@ export function createHttpDriver(options: HttpDriverOptions): EvalDriver { // throwing, so mark an aborted turn failed explicitly. if (signal.aborted) state.ok = false; + const toolCallDetails = [...state.toolCalls.values()]; return { reply: state.reply, - toolCalls: state.toolCalls, + toolCalls: toolCallDetails.map((c) => c.name), + toolCallDetails, succeeded: state.ok, sessionId: threadId, traceId: state.traceId, diff --git a/packages/appkit/src/evals/run-eval.ts b/packages/appkit/src/evals/run-eval.ts index c0ec2a57f..f54cbde4c 100644 --- a/packages/appkit/src/evals/run-eval.ts +++ b/packages/appkit/src/evals/run-eval.ts @@ -3,6 +3,7 @@ import { judgeClosedQA, judgeCustom, judgeFactuality } from "./judge"; import type { AssertionHandle, AssertionResult, + DriveResult, EvalDefinition, EvalDriver, EvalResult, @@ -21,6 +22,24 @@ class SkipSignal extends Error { } } +/** + * Deep partial match: every key in `expected` is present in `actual` and equal, + * recursing into nested plain objects so extra actual keys are ignored. + */ +function deepContains(actual: unknown, expected: unknown): boolean { + if (isPlainObject(expected)) { + if (!isPlainObject(actual)) return false; + return Object.keys(expected).every((key) => + deepContains(actual[key], expected[key]), + ); + } + return actual === expected; +} + +function isPlainObject(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + export interface RunEvalOptions { /** Stable id for the eval (e.g. its file path relative to the evals dir). */ id: string; @@ -45,6 +64,7 @@ export async function runEval( let reply = ""; let lastInput = ""; let toolCalls: string[] = []; + let toolCallDetails: DriveResult["toolCallDetails"] = []; let sessionId: string | undefined; let lastTraceId: string | undefined; let lastSucceeded = false; @@ -98,6 +118,7 @@ export async function runEval( const r = await options.driver.send(message); reply = r.reply; toolCalls = r.toolCalls; + toolCallDetails = r.toolCallDetails; sessionId = r.sessionId; lastSucceeded = r.succeeded; if (r.traceId) lastTraceId = r.traceId; @@ -138,6 +159,21 @@ export async function runEval( })`, ); }, + calledToolWith(name, expected) { + const matching = toolCallDetails.filter((c) => c.name === name); + const pass = matching.some((c) => deepContains(c.args, expected)); + const seen = matching.length + ? matching.map((c) => JSON.stringify(c.args)).join(", ") + : "not called"; + return record( + `calledToolWith(${name})`, + pass, + undefined, + `expected tool "${name}" to be called with ${JSON.stringify( + expected, + )} (args seen: ${seen})`, + ); + }, check(value: string, matcher: Matcher) { const m = matcher(value); return record("check", m.pass, m.score, m.detail); diff --git a/packages/appkit/src/evals/tests/http-driver.test.ts b/packages/appkit/src/evals/tests/http-driver.test.ts index 99e9a1eed..5c57e7ae3 100644 --- a/packages/appkit/src/evals/tests/http-driver.test.ts +++ b/packages/appkit/src/evals/tests/http-driver.test.ts @@ -1,7 +1,15 @@ import { createServer, type Server } from "node:http"; import type { AddressInfo } from "node:net"; -import { afterAll, beforeAll, describe, expect, test } from "vitest"; +import { + afterAll, + afterEach, + beforeAll, + describe, + expect, + test, + vi, +} from "vitest"; import { createHttpDriver } from "../http-driver"; @@ -103,6 +111,22 @@ afterAll(async () => { await new Promise((resolve) => server.close(() => resolve())); }); +afterEach(() => { + vi.restoreAllMocks(); +}); + +/** Build a mock SSE `Response` from a list of Responses-API events. */ +function sseResponse(events: Array>): Response { + const body = events.map((e) => `data: ${JSON.stringify(e)}\n`).join("\n"); + const stream = new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode(body)); + controller.close(); + }, + }); + return new Response(stream, { status: 200 }); +} + describe("createHttpDriver", () => { test("captures the reply and succeeds on a normal stream", async () => { const driver = createHttpDriver({ baseUrl, path: "/ok" }); @@ -142,4 +166,62 @@ describe("createHttpDriver", () => { expect(result.succeeded).toBe(false); expect(Date.now() - started).toBeLessThan(2000); }); + + test("captures tool-call names and parses their arguments", async () => { + // `added` carries empty args; `done` carries the full JSON string. + vi.spyOn(globalThis, "fetch").mockResolvedValue( + sseResponse([ + { + type: "response.output_item.added", + item: { + type: "function_call", + name: "get_weather", + call_id: "c1", + arguments: "", + }, + }, + { + type: "response.output_item.done", + item: { + type: "function_call", + name: "get_weather", + call_id: "c1", + arguments: '{"city":"Paris","units":"metric"}', + }, + }, + { type: "response.output_text.delta", delta: "Sunny" }, + ]), + ); + + const driver = createHttpDriver({ baseUrl: "http://localhost:3000" }); + const result = await driver.send("weather in Paris?"); + + expect(result.reply).toBe("Sunny"); + expect(result.toolCalls).toEqual(["get_weather"]); + expect(result.toolCallDetails).toEqual([ + { name: "get_weather", args: { city: "Paris", units: "metric" } }, + ]); + expect(result.succeeded).toBe(true); + }); + + test("defaults args to {} when the arguments JSON is malformed", async () => { + vi.spyOn(globalThis, "fetch").mockResolvedValue( + sseResponse([ + { + type: "response.output_item.done", + item: { + type: "function_call", + name: "broken", + call_id: "c1", + arguments: "{not json", + }, + }, + ]), + ); + + const driver = createHttpDriver({ baseUrl: "http://localhost:3000" }); + const result = await driver.send("go"); + + expect(result.toolCallDetails).toEqual([{ name: "broken", args: {} }]); + }); }); diff --git a/packages/appkit/src/evals/tests/run-eval.test.ts b/packages/appkit/src/evals/tests/run-eval.test.ts index 57b4aea7b..9e10b86c7 100644 --- a/packages/appkit/src/evals/tests/run-eval.test.ts +++ b/packages/appkit/src/evals/tests/run-eval.test.ts @@ -11,6 +11,7 @@ function fakeDriver(result: Partial): EvalDriver { send: async () => ({ reply: "", toolCalls: [], + toolCallDetails: [], succeeded: true, ...result, }), @@ -55,6 +56,82 @@ describe("runEval", () => { expect(result.assertions[0].pass).toBe(false); }); + test("calledToolWith passes when a call's args deep-contain the expected", async () => { + const def = defineEval({ + async test(t) { + await t.send("weather in Paris?"); + t.calledToolWith("get_weather", { city: "Paris" }); + }, + }); + const result = await runEval(def, { + id: "args-match", + driver: fakeDriver({ + toolCalls: ["get_weather"], + toolCallDetails: [ + { name: "get_weather", args: { city: "Paris", units: "metric" } }, + ], + }), + }); + expect(result.passed).toBe(true); + expect(result.assertions[0].pass).toBe(true); + }); + + test("calledToolWith fails when the tool was called with different args", async () => { + const def = defineEval({ + async test(t) { + await t.send("weather in Paris?"); + t.calledToolWith("get_weather", { city: "Paris" }); + }, + }); + const result = await runEval(def, { + id: "args-mismatch", + driver: fakeDriver({ + toolCalls: ["get_weather"], + toolCallDetails: [{ name: "get_weather", args: { city: "London" } }], + }), + }); + expect(result.passed).toBe(false); + expect(result.assertions[0].pass).toBe(false); + }); + + test("calledToolWith fails when the tool was not called", async () => { + const def = defineEval({ + async test(t) { + await t.send("hi"); + t.calledToolWith("get_weather", { city: "Paris" }); + }, + }); + const result = await runEval(def, { + id: "args-not-called", + driver: fakeDriver({ toolCalls: [], toolCallDetails: [] }), + }); + expect(result.passed).toBe(false); + expect(result.assertions[0].pass).toBe(false); + expect(result.assertions[0].detail).toContain("not called"); + }); + + test("calledToolWith matches nested args and ignores extra keys", async () => { + const def = defineEval({ + async test(t) { + await t.send("book it"); + t.calledToolWith("book", { where: { city: "Paris" } }); + }, + }); + const result = await runEval(def, { + id: "args-nested", + driver: fakeDriver({ + toolCalls: ["book"], + toolCallDetails: [ + { + name: "book", + args: { where: { city: "Paris", zip: "75001" }, when: "today" }, + }, + ], + }), + }); + expect(result.passed).toBe(true); + }); + test("soft failures don't fail the eval unless strict", async () => { const def = defineEval({ async test(t) { @@ -159,7 +236,12 @@ describe("runEval", () => { test("t.reset() forwards to the driver to start a fresh conversation", async () => { const reset = vi.fn(); const driver: EvalDriver = { - send: async () => ({ reply: "", toolCalls: [], succeeded: true }), + send: async () => ({ + reply: "", + toolCalls: [], + toolCallDetails: [], + succeeded: true, + }), reset, }; const def = defineEval({ diff --git a/packages/appkit/src/evals/types.ts b/packages/appkit/src/evals/types.ts index 2619530d5..009411f02 100644 --- a/packages/appkit/src/evals/types.ts +++ b/packages/appkit/src/evals/types.ts @@ -56,6 +56,8 @@ export interface DriveResult { reply: string; /** Names of tools the agent called during the turn. */ toolCalls: string[]; + /** Tool calls with their parsed arguments, in call order. */ + toolCallDetails: Array<{ name: string; args: Record }>; /** Whether the turn completed without an agent/stream error. */ succeeded: boolean; /** Thread/session id, when the driver exposes one. */ @@ -107,6 +109,15 @@ export interface TestContext { succeeded(): AssertionHandle; /** Assert a tool was called during the run (gate by default). */ calledTool(name: string): AssertionHandle; + /** + * Assert a tool was called with arguments that deep-contain `expected`: every + * key in `expected` must equal the actual argument (recursively for nested + * objects), so extra arguments are ignored. Gate by default. + */ + calledToolWith( + name: string, + expected: Record, + ): AssertionHandle; /** Assert a value against a matcher, e.g. `t.check(t.reply, includes("Sunny"))`. */ check(value: string, matcher: Matcher): AssertionHandle; /** From 07ee8ff79c2a83901689a5448d97b0c71cc2f36a Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Fri, 10 Jul 2026 18:25:47 +0200 Subject: [PATCH 03/11] feat(appkit): replay multi-turn dataset rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A managed-dataset row's `inputs.messages` can be a full conversation, not just one question. Add `userTurns(input)` to extract every user-message content in order; the example dataset eval replays them against one thread so the agent sees the accumulating conversation. Interleaved assistant/system turns are ignored — the agent generates its own responses. Single-user-turn rows are unchanged (one send). Signed-off-by: MarioCadenas --- .../server/agents/query/evals/dataset.eval.ts | 28 ++++++------ packages/appkit/src/evals/dataset.ts | 19 ++++++++ packages/appkit/src/evals/index.ts | 1 + .../appkit/src/evals/tests/dataset.test.ts | 44 ++++++++++++++++++- 4 files changed, 78 insertions(+), 14 deletions(-) diff --git a/apps/dev-playground/server/agents/query/evals/dataset.eval.ts b/apps/dev-playground/server/agents/query/evals/dataset.eval.ts index 98cdd9d3e..25184283d 100644 --- a/apps/dev-playground/server/agents/query/evals/dataset.eval.ts +++ b/apps/dev-playground/server/agents/query/evals/dataset.eval.ts @@ -1,4 +1,8 @@ -import { defineEval, isJudgeConfigured } from "@databricks/appkit/beta"; +import { + defineEval, + isJudgeConfigured, + userTurns, +} from "@databricks/appkit/beta"; /** * Dataset-driven eval: runs once per row of a Databricks managed evaluation @@ -13,17 +17,12 @@ import { defineEval, isJudgeConfigured } from "@databricks/appkit/beta"; * Row shape produced by the MLflow managed-dataset UI: * inputs {"messages":[{"role":"user","content":"..."}]} * expectations {"guidelines":{"value":["...","..."]}} (optional) + * + * A row's `messages` can be a full multi-turn conversation. We replay each USER + * turn in order against one shared thread (below); interleaved assistant turns + * in the row are ignored — the agent generates its own responses. */ -/** Pull the last user message out of an MLflow `{messages:[...]}` input. */ -function userMessage(input: Record): string { - const messages = Array.isArray(input.messages) - ? (input.messages as Array<{ role?: string; content?: string }>) - : []; - const last = [...messages].reverse().find((m) => m.role === "user"); - return last?.content ?? ""; -} - /** Read `expectations.guidelines` — the UI wraps the array as `{value: [...]}`. */ function guidelines(expected: Record | undefined): string[] { const g = (expected?.guidelines as { value?: unknown } | undefined)?.value; @@ -35,9 +34,12 @@ export default defineEval({ // Point at your own managed evaluation dataset (catalog.schema.table). dataset: { table: "main.mario.appkit_eval_dataset" }, async test(t) { - // One turn per row. For a multi-turn conversation, call `t.send` again - // (same thread); to start an independent turn in the same test, `t.reset()`. - await t.send(userMessage(t.input)); + // Replay every user turn in the row against one thread, so the agent sees + // the accumulating conversation. A single-user-turn row sends once. The + // runner gives each row a fresh driver, so rows don't bleed into each other. + for (const turn of userTurns(t.input)) { + await t.send(turn); + } t.succeeded(); // Each guideline is judged against the reply — gate by default, so a miss diff --git a/packages/appkit/src/evals/dataset.ts b/packages/appkit/src/evals/dataset.ts index 0a18b10df..4edc49918 100644 --- a/packages/appkit/src/evals/dataset.ts +++ b/packages/appkit/src/evals/dataset.ts @@ -21,6 +21,25 @@ export interface ReadEvalDatasetOptions { limit?: number; } +/** + * Extract every user-message content, in order, from an MLflow + * `{"messages":[{"role":"user","content":"..."}]}` input. A dataset row can + * carry a full multi-turn conversation; replaying these against one thread (one + * `t.send` per returned string) lets the agent see the accumulating history. + * + * Only `role === "user"` turns are returned — any interleaved `assistant`/ + * `system` messages in the row are ignored, since the agent generates its own + * responses; you never inject the dataset's assistant turns. A single-user-turn + * row yields a one-element array (backward compatible); a row with no `messages` + * yields `[]`. + */ +export function userTurns(input: Record): string[] { + const messages = Array.isArray(input.messages) + ? (input.messages as Array<{ role?: string; content?: string }>) + : []; + return messages.filter((m) => m.role === "user").map((m) => m.content ?? ""); +} + /** A managed eval dataset is a UC table; only 3-level names are valid. */ const UC_TABLE = /^[A-Za-z0-9_]+\.[A-Za-z0-9_]+\.[A-Za-z0-9_]+$/; diff --git a/packages/appkit/src/evals/index.ts b/packages/appkit/src/evals/index.ts index 3714c5378..d4e65d07c 100644 --- a/packages/appkit/src/evals/index.ts +++ b/packages/appkit/src/evals/index.ts @@ -11,6 +11,7 @@ export { type DatasetRow, type ReadEvalDatasetOptions, readEvalDataset, + userTurns, } from "./dataset"; export { defineEval } from "./define-eval"; export { type DiscoveredEval, discoverEvalFiles } from "./discover"; diff --git a/packages/appkit/src/evals/tests/dataset.test.ts b/packages/appkit/src/evals/tests/dataset.test.ts index 312e44100..2773ebdaf 100644 --- a/packages/appkit/src/evals/tests/dataset.test.ts +++ b/packages/appkit/src/evals/tests/dataset.test.ts @@ -8,7 +8,7 @@ vi.mock("../../connectors", () => ({ }, })); -import { readEvalDataset } from "../dataset"; +import { readEvalDataset, userTurns } from "../dataset"; const client = {} as never; @@ -102,3 +102,45 @@ describe("readEvalDataset", () => { expect(executeStatement).not.toHaveBeenCalled(); }); }); + +describe("userTurns", () => { + test("returns all user contents in order", () => { + expect( + userTurns({ + messages: [ + { role: "user", content: "first" }, + { role: "user", content: "second" }, + ], + }), + ).toEqual(["first", "second"]); + }); + + test("ignores assistant/system turns, keeps user order", () => { + expect( + userTurns({ + messages: [ + { role: "system", content: "be helpful" }, + { role: "user", content: "hi" }, + { role: "assistant", content: "hello" }, + { role: "user", content: "follow up" }, + ], + }), + ).toEqual(["hi", "follow up"]); + }); + + test("a single user message yields one turn", () => { + expect( + userTurns({ messages: [{ role: "user", content: "only" }] }), + ).toEqual(["only"]); + }); + + test("missing content becomes an empty string", () => { + expect(userTurns({ messages: [{ role: "user" }] })).toEqual([""]); + }); + + test("missing or non-array messages yields []", () => { + expect(userTurns({})).toEqual([]); + expect(userTurns({ messages: "nope" })).toEqual([]); + expect(userTurns({ messages: [] })).toEqual([]); + }); +}); From c4bbaf764eae3813eb1310226de65b8eee640b91 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Fri, 10 Jul 2026 18:40:50 +0200 Subject: [PATCH 04/11] feat(appkit): gate eval runs on aggregate pass rate Add `--min-pass-rate <0..1>` to the agent eval CLI: instead of requiring every eval to pass, exit non-zero only when the aggregate pass rate falls below the threshold. `summarize()` now returns `passRate` (passed / scored, excluding skips; 1 when nothing scored). Without the flag, behavior is unchanged (any gate failure fails the run). Signed-off-by: MarioCadenas --- packages/appkit/src/evals/report.ts | 4 ++++ .../appkit/src/evals/tests/report.test.ts | 13 ++++++++--- .../shared/src/cli/commands/agent/eval.ts | 23 +++++++++++++++++-- 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/packages/appkit/src/evals/report.ts b/packages/appkit/src/evals/report.ts index 2e42c5b1c..409484254 100644 --- a/packages/appkit/src/evals/report.ts +++ b/packages/appkit/src/evals/report.ts @@ -7,6 +7,8 @@ export interface EvalSummary { skipped: number; /** True when no eval failed (skips don't count as failures). */ allPassed: boolean; + /** Fraction of scored (non-skipped) evals that passed, 0..1 (1 when none scored). */ + passRate: number; } export function summarize(results: EvalResult[]): EvalSummary { @@ -18,12 +20,14 @@ export function summarize(results: EvalResult[]): EvalSummary { else if (r.passed) passed++; else failed++; } + const scored = passed + failed; return { total: results.length, passed, failed, skipped, allPassed: failed === 0, + passRate: scored === 0 ? 1 : passed / scored, }; } diff --git a/packages/appkit/src/evals/tests/report.test.ts b/packages/appkit/src/evals/tests/report.test.ts index 49b80fe89..2d5ea605e 100644 --- a/packages/appkit/src/evals/tests/report.test.ts +++ b/packages/appkit/src/evals/tests/report.test.ts @@ -30,18 +30,25 @@ const results: EvalResult[] = [ ]; describe("eval reporting", () => { - test("summarize counts pass/fail/skip and allPassed", () => { + test("summarize counts pass/fail/skip, allPassed, and passRate", () => { expect(summarize(results)).toEqual({ total: 3, passed: 1, failed: 1, skipped: 1, allPassed: false, + passRate: 0.5, // 1 passed of 2 scored; the skip is excluded }); }); - test("summarize allPassed is true when nothing failed", () => { - expect(summarize([results[0], results[2]]).allPassed).toBe(true); + test("summarize allPassed is true and passRate is 1 when nothing failed", () => { + const s = summarize([results[0], results[2]]); + expect(s.allPassed).toBe(true); + expect(s.passRate).toBe(1); // 1 passed of 1 scored (skip excluded) + }); + + test("passRate is 1 when every eval was skipped (nothing scored)", () => { + expect(summarize([results[2]]).passRate).toBe(1); }); test("formatEvalResults shows status, failing assertions, and a summary line", () => { diff --git a/packages/shared/src/cli/commands/agent/eval.ts b/packages/shared/src/cli/commands/agent/eval.ts index 7ff54f140..6e0af2b95 100644 --- a/packages/shared/src/cli/commands/agent/eval.ts +++ b/packages/shared/src/cli/commands/agent/eval.ts @@ -53,7 +53,7 @@ interface EvalRunner { evalGlyph(result: unknown): string; formatEvalDetail(result: unknown): string[]; formatSummaryLine(results: unknown[]): string; - summarize(results: unknown[]): { allPassed: boolean }; + summarize(results: unknown[]): { allPassed: boolean; passRate: number }; } /** @@ -96,6 +96,7 @@ interface EvalOptions { judgeModel?: string; concurrency?: number; warehouseId?: string; + minPassRate?: string; } /** Resolved Databricks host + bearer (either field may be absent). */ @@ -252,7 +253,21 @@ async function runAgentEval( ); } - if (!runner.summarize(summary.results).allPassed) { + const stats = runner.summarize(summary.results); + const minPassRate = opts.minPassRate + ? Number.parseFloat(opts.minPassRate) + : undefined; + if (minPassRate !== undefined && !Number.isNaN(minPassRate)) { + // Threshold mode: gate on the aggregate pass rate rather than requiring + // every eval to pass. + const ok = stats.passRate >= minPassRate; + console.log( + `Pass rate ${(stats.passRate * 100).toFixed(0)}% (threshold ${( + minPassRate * 100 + ).toFixed(0)}%) — ${ok ? "OK" : "below threshold"}`, + ); + if (!ok) process.exitCode = 1; + } else if (!stats.allPassed) { process.exitCode = 1; } } @@ -304,4 +319,8 @@ export const agentEvalCommand = new Command("eval") "--judge-model ", "Databricks serving endpoint to use as the LLM judge for t.judge.* (default: APPKIT_JUDGE_MODEL)", ) + .option( + "--min-pass-rate ", + "Gate on aggregate pass rate (0..1) instead of requiring every eval to pass; exit 1 when below", + ) .action(runAgentEval); From 5b07b9aea02204715f9b735fbac7cbda45567fb2 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Mon, 13 Jul 2026 13:27:31 +0200 Subject: [PATCH 05/11] feat(appkit): enforce eval timeout, load evals.config.ts, filter by tag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire three eval fields that were declared but inert: - `timeoutMs` — runEval races the test against a per-eval timeout and records a clean non-passing result ("eval timed out after Nms") instead of hanging. Precedence: def.timeoutMs > runner/CLI --timeout > unbounded. Timer is always cleared. - `evals.config.ts` (defineEvalConfig) — discovered per-agent and loaded via the tsx loader; its maxConcurrency/timeoutMs apply as defaults (CLI flag > config > built-in). Judge model still comes from the CLI (needs creds the config lacks). - `tags` + `--tag ` — run only evals whose tags intersect the filter. Signed-off-by: MarioCadenas --- packages/appkit/src/evals/define-eval.ts | 7 +- packages/appkit/src/evals/discover.ts | 35 +++- packages/appkit/src/evals/index.ts | 9 +- packages/appkit/src/evals/run-eval.ts | 35 +++- packages/appkit/src/evals/run-evals.ts | 178 +++++++++++++++--- .../appkit/src/evals/tests/discover.test.ts | 21 ++- .../src/evals/tests/resolve-default.test.ts | 43 ++++- .../appkit/src/evals/tests/run-eval.test.ts | 59 ++++++ packages/appkit/src/evals/types.ts | 17 ++ .../shared/src/cli/commands/agent/eval.ts | 21 +++ 10 files changed, 391 insertions(+), 34 deletions(-) diff --git a/packages/appkit/src/evals/define-eval.ts b/packages/appkit/src/evals/define-eval.ts index 3e31cb191..815e08050 100644 --- a/packages/appkit/src/evals/define-eval.ts +++ b/packages/appkit/src/evals/define-eval.ts @@ -1,4 +1,4 @@ -import type { EvalDefinition } from "./types"; +import type { EvalConfig, EvalDefinition } from "./types"; /** * Define an agent eval. Default-export the result from a @@ -25,3 +25,8 @@ export function defineEval(def: EvalDefinition): EvalDefinition { } return def; } + +/** Define per-directory eval config. Default-export from `evals.config.ts`. */ +export function defineEvalConfig(config: EvalConfig): EvalConfig { + return config; +} diff --git a/packages/appkit/src/evals/discover.ts b/packages/appkit/src/evals/discover.ts index 47029c3ad..63dfa0fc6 100644 --- a/packages/appkit/src/evals/discover.ts +++ b/packages/appkit/src/evals/discover.ts @@ -1,4 +1,4 @@ -import { type Dirent, readdirSync } from "node:fs"; +import { type Dirent, existsSync, readdirSync } from "node:fs"; import path from "node:path"; import { agentDirNames } from "../core/agent/agent-dirs"; @@ -14,6 +14,14 @@ export interface DiscoveredEval { agent: string; } +/** A per-agent `evals.config.ts` found under `server/agents//evals/`. */ +export interface DiscoveredEvalConfig { + /** Absolute path to the `evals.config.ts` file. */ + file: string; + /** The agent id whose evals this config applies to. */ + agent: string; +} + /** Recursively collect `*.eval.ts` files under `dir`. Empty when `dir` is absent. */ function evalFilesIn(dir: string): string[] { try { @@ -58,3 +66,28 @@ export function discoverEvalFiles(rootDir: string): DiscoveredEval[] { (a, b) => a.agent.localeCompare(b.agent) || a.id.localeCompare(b.id), ); } + +/** + * Discover the per-agent `evals.config.ts` (from {@link defineEvalConfig}) at + * `/server/agents//evals/evals.config.ts`. Config is per-agent: + * each agent's config applies only to that agent's evals. Agents without a + * config file are omitted. Returns a stable, sorted list. + */ +export function discoverEvalConfigs(rootDir: string): DiscoveredEvalConfig[] { + const agentsDir = path.join(rootDir, CODE_AGENTS_SOURCE_DIR); + const out: DiscoveredEvalConfig[] = []; + + let entries: Dirent[]; + try { + entries = readdirSync(agentsDir, { withFileTypes: true }); + } catch { + return out; + } + + for (const agent of agentDirNames(entries)) { + const file = path.join(agentsDir, agent, "evals", "evals.config.ts"); + if (existsSync(file)) out.push({ file, agent }); + } + + return out.sort((a, b) => a.agent.localeCompare(b.agent)); +} diff --git a/packages/appkit/src/evals/index.ts b/packages/appkit/src/evals/index.ts index d4e65d07c..937169bc9 100644 --- a/packages/appkit/src/evals/index.ts +++ b/packages/appkit/src/evals/index.ts @@ -13,8 +13,13 @@ export { readEvalDataset, userTurns, } from "./dataset"; -export { defineEval } from "./define-eval"; -export { type DiscoveredEval, discoverEvalFiles } from "./discover"; +export { defineEval, defineEvalConfig } from "./define-eval"; +export { + type DiscoveredEval, + type DiscoveredEvalConfig, + discoverEvalConfigs, + discoverEvalFiles, +} from "./discover"; export { createHttpDriver, type HttpDriverOptions } from "./http-driver"; export { configureJudge, diff --git a/packages/appkit/src/evals/run-eval.ts b/packages/appkit/src/evals/run-eval.ts index f54cbde4c..7129c9e40 100644 --- a/packages/appkit/src/evals/run-eval.ts +++ b/packages/appkit/src/evals/run-eval.ts @@ -22,6 +22,14 @@ class SkipSignal extends Error { } } +/** Rejects the test race when a per-eval timeout elapses. */ +class TimeoutSignal extends Error { + constructor(ms: number) { + super(`eval timed out after ${ms}ms`); + this.name = "TimeoutSignal"; + } +} + /** * Deep partial match: every key in `expected` is present in `actual` and equal, * recursing into nested plain objects so extra actual keys are ignored. @@ -49,6 +57,11 @@ export interface RunEvalOptions { strict?: boolean; /** Dataset row bound to `t.input`/`t.expected` for dataset-driven evals. */ row?: DatasetRow; + /** + * Runner-level default per-eval timeout (ms). `def.timeoutMs` wins over this; + * when both are unset the eval runs unbounded (current behavior). + */ + timeoutMs?: number; } /** @@ -208,8 +221,26 @@ export async function runEval( }, }; + // `def.timeoutMs` (per-eval) wins over the runner default; when both are + // unset the eval runs unbounded (undefined = no timeout). + const timeoutMs = def.timeoutMs ?? options.timeoutMs; + let timer: ReturnType | undefined; + try { - await def.test(t); + if (timeoutMs === undefined) { + await def.test(t); + } else { + // Race the test against a timeout; on elapse the sentinel rejects and we + // convert it to a non-passing result. The timer is cleared in `finally` + // so it can't keep the process alive after the test settles. + const timeout = new Promise((_, reject) => { + timer = setTimeout( + () => reject(new TimeoutSignal(timeoutMs)), + timeoutMs, + ); + }); + await Promise.race([Promise.resolve(def.test(t)), timeout]); + } } catch (err) { if (err instanceof SkipSignal) { return { @@ -229,6 +260,8 @@ export async function runEval( error: err instanceof Error ? err.message : String(err), traceId: lastTraceId, }; + } finally { + if (timer) clearTimeout(timer); } const passed = assertions.every( diff --git a/packages/appkit/src/evals/run-evals.ts b/packages/appkit/src/evals/run-evals.ts index 2e82bc291..0dc0be2c4 100644 --- a/packages/appkit/src/evals/run-evals.ts +++ b/packages/appkit/src/evals/run-evals.ts @@ -3,14 +3,18 @@ import { pathToFileURL } from "node:url"; import { MlflowClient } from "../connectors/mlflow"; import type { WorkspaceClient } from "../workspace-client"; import { type DatasetRow, readEvalDataset } from "./dataset"; -import { type DiscoveredEval, discoverEvalFiles } from "./discover"; +import { + type DiscoveredEval, + discoverEvalConfigs, + discoverEvalFiles, +} from "./discover"; import { createHttpDriver } from "./http-driver"; import { configureJudge, teardownJudge } from "./judge"; import { type ReportOutcome, reportToMlflow } from "./mlflow-report"; import { createEvalRun, type FinishOutcome, finishEvalRun } from "./mlflow-run"; import { mapPool } from "./pool"; import { runEval } from "./run-eval"; -import type { EvalDefinition, EvalResult } from "./types"; +import type { EvalConfig, EvalDefinition, EvalResult } from "./types"; export interface RunEvalsOptions { /** Project root containing `server/agents/`. Defaults to `process.cwd()`. */ @@ -19,12 +23,15 @@ export interface RunEvalsOptions { baseUrl: string; /** Substring filter on `/` (or an exact agent id). */ filter?: string; + /** + * Only run evals whose `tags` intersect this list. Empty/undefined runs all. + * Tags live on the eval def, so filtering happens after each file is loaded. + */ + tags?: string[]; /** Soft assertion failures also fail the eval. */ strict?: boolean; /** Extra request headers for the driver (e.g. auth for a deployed app). */ headers?: Record; - /** Per-turn wall-clock timeout (ms) before a turn is failed. Defaults to 120s. */ - timeoutMs?: number; /** * Max evals to drive concurrently. Each eval opens one stream to the app as * the same user, so keep this at or below the app's @@ -58,6 +65,12 @@ export interface RunEvalsOptions { warehouseId?: string; /** Wall-clock timestamp (ms) for run create/finish — pass `Date.now()`. */ now?: number; + /** + * Default per-eval timeout (ms): `runEval` races the whole test against it and + * it also caps each driver turn. A per-eval `def.timeoutMs` overrides it, and + * it wins over an agent's `evals.config.ts` `timeoutMs`. Unbounded when unset. + */ + timeoutMs?: number; /** Progress callback, invoked as evals are discovered, started, and finished. */ onEvent?: (event: EvalProgress) => void; } @@ -75,12 +88,11 @@ export interface EvalRunSummary { } /** - * Load a `*.eval.ts` file and return its default-exported {@link EvalDefinition}. - * Uses tsx's programmatic loader so TypeScript eval files run without a build - * step. The specifier is indirected so the type checker doesn't try to resolve - * tsx's internal entry. + * Import a TypeScript file with tsx's programmatic loader so eval files run + * without a build step. The specifier is indirected so the type checker doesn't + * try to resolve tsx's internal entry. */ -async function loadEval(file: string): Promise { +async function tsImportFile(file: string): Promise { const tsxApi = "tsx/esm/api"; let tsImport: (specifier: string, parentURL: string) => Promise; try { @@ -92,8 +104,14 @@ async function loadEval(file: string): Promise { "Running .eval.ts files requires `tsx`. Install it as a dev dependency (`pnpm add -D tsx`).", ); } + return tsImport(pathToFileURL(file).href, import.meta.url); +} - const mod = await tsImport(pathToFileURL(file).href, import.meta.url); +/** + * Load a `*.eval.ts` file and return its default-exported {@link EvalDefinition}. + */ +async function loadEval(file: string): Promise { + const mod = await tsImportFile(file); const def = resolveEvalDefault(mod); if (!def) { throw new Error(`${file}: must default-export defineEval({ test })`); @@ -101,6 +119,37 @@ async function loadEval(file: string): Promise { return def; } +/** + * Load an `evals.config.ts` file and return its default-exported + * {@link EvalConfig}. A malformed/missing default surfaces as `undefined` so a + * bad config never aborts a whole run. + */ +async function loadEvalConfig(file: string): Promise { + const mod = await tsImportFile(file); + return resolveConfigDefault(mod); +} + +/** + * Unwrap the config default export across module-interop shapes (see + * {@link resolveEvalDefault}). A config has no `.test`, so the first plain + * object reached through the `default` chain is taken as the config. + */ +export function resolveConfigDefault(mod: unknown): EvalConfig | undefined { + const seen = new Set(); + let candidate: unknown = mod; + for (let i = 0; i < 4 && candidate && !seen.has(candidate); i++) { + const next = (candidate as { default?: unknown }).default; + if (next === undefined) { + return typeof candidate === "object" + ? (candidate as EvalConfig) + : undefined; + } + seen.add(candidate); + candidate = next; + } + return undefined; +} + /** * Unwrap the eval default export across module-interop shapes. Depending on * whether the eval file is treated as ESM or CJS, the value lands at @@ -143,7 +192,13 @@ async function runOne( mlflowRunId: runId, timeoutMs: options.timeoutMs, }); - return await runEval(def, { id, driver, strict: options.strict, row }); + return await runEval(def, { + id, + driver, + strict: options.strict, + row, + timeoutMs: options.timeoutMs, + }); } catch (err) { return { id, @@ -243,6 +298,19 @@ async function runDiscovered( } } +/** + * Whether an eval's `tags` satisfy a `--tag` filter: `true` when the filter is + * empty/undefined (no filtering), otherwise only when the eval shares at least + * one tag with it. An eval with no tags never matches a non-empty filter. + */ +export function matchesTags( + defTags: string[] | undefined, + filterTags: string[] | undefined, +): boolean { + if (!filterTags || filterTags.length === 0) return true; + return defTags?.some((t) => filterTags.includes(t)) ?? false; +} + /** Configure the LLM judge when judge creds were supplied; otherwise a no-op. */ async function maybeConfigureJudge(options: RunEvalsOptions): Promise { if (!options.judge) return; @@ -314,7 +382,61 @@ export async function runEvalsInDir( } const emit = options.onEvent ?? (() => {}); - const total = discovered.length; + + // Load each agent's `evals.config.ts` (best-effort, per-agent): its settings + // apply only to that agent's evals. A malformed/missing config never aborts + // the run — the agent just falls back to CLI options and built-in defaults. + const configs = new Map(); + for (const c of discoverEvalConfigs(root)) { + try { + const cfg = await loadEvalConfig(c.file); + if (cfg) configs.set(c.agent, cfg); + } catch { + // Ignore: fall back to CLI options / defaults for this agent. + } + } + + // Load each eval def and apply the `--tag` filter up front. Tags live on the + // def, so a tag miss removes the eval entirely (like the substring filter + // excludes files) rather than surfacing as a result. Load failures are kept + // so a broken file still reports as a non-passing result. + const loaded: Array<{ + d: DiscoveredEval; + def: EvalDefinition; + loadError?: string; + }> = []; + for (const d of discovered) { + let def: EvalDefinition; + try { + def = await loadEval(d.file); + } catch (err) { + loaded.push({ + d, + // No def loaded; placeholder def is never run (error short-circuits). + def: { test: () => {} }, + loadError: err instanceof Error ? err.message : String(err), + }); + continue; + } + if (!matchesTags(def.tags, options.tags)) continue; + loaded.push({ d, def }); + } + + // `evals.config.ts` `maxConcurrency` governs the single shared work pool, so + // it can't be applied per-agent without splitting the pool. The `--concurrency` + // flag wins; else the highest value any agent's config requests (the pool + // ceiling); else the built-in default. + const configMaxConcurrency = [...configs.values()] + .map((c) => c.maxConcurrency) + .filter((n): n is number => typeof n === "number") + .reduce( + (max, n) => (max === undefined ? n : Math.max(max, n)), + undefined, + ); + const concurrency = + options.concurrency ?? configMaxConcurrency ?? DEFAULT_CONCURRENCY; + + const total = loaded.length; emit({ type: "discovered", total }); // The judge sets OPENAI_* env vars globally (autoevals reads them per call), @@ -340,21 +462,23 @@ export async function runEvalsInDir( emit({ type: "run-created", runId }); } - // Run each eval through the bounded pool — one in-flight stream per eval, so - // the pool respects the server's per-user stream cap (see mapPool/concurrency). - // A dataset eval expands into per-row runs that execute serially within its - // slot; results preserve discovery order (mapPool writes by index) and row - // order within each file. `total` counts eval files, not dataset rows — per-row - // detail is carried in the result id (`[row i/n]`). - const perFile = await mapPool( - discovered, - options.concurrency ?? DEFAULT_CONCURRENCY, - async (d, index) => { - const fileResults: EvalResult[] = []; - await runDiscovered(d, index, total, runId, options, emit, fileResults); - return fileResults; - }, - ); + // Run each loaded (tag-filtered) eval through the bounded pool — one in-flight + // stream per eval, so the pool respects the server's per-user stream cap (see + // mapPool/concurrency). A dataset eval expands into per-row runs that execute + // serially within its slot; results preserve discovery order (mapPool writes + // by index) and row order within each file. Per-agent timeout is folded into + // the file's options (CLI wins over `evals.config.ts`; `def.timeoutMs` still + // overrides, applied inside runEval). `total` counts eval files, not dataset + // rows — per-row detail is carried in the result id (`[row i/n]`). + const perFile = await mapPool(loaded, concurrency, async ({ d }, index) => { + const fileResults: EvalResult[] = []; + const fileOptions: RunEvalsOptions = { + ...options, + timeoutMs: options.timeoutMs ?? configs.get(d.agent)?.timeoutMs, + }; + await runDiscovered(d, index, total, runId, fileOptions, emit, fileResults); + return fileResults; + }); const results = perFile.flat(); const summary: EvalRunSummary = { results }; diff --git a/packages/appkit/src/evals/tests/discover.test.ts b/packages/appkit/src/evals/tests/discover.test.ts index e93124d70..ac8e4df5f 100644 --- a/packages/appkit/src/evals/tests/discover.test.ts +++ b/packages/appkit/src/evals/tests/discover.test.ts @@ -4,7 +4,7 @@ import path from "node:path"; import { afterEach, beforeEach, describe, expect, test } from "vitest"; -import { discoverEvalFiles } from "../discover"; +import { discoverEvalConfigs, discoverEvalFiles } from "../discover"; let root: string; @@ -42,3 +42,22 @@ describe("discoverEvalFiles", () => { expect(discoverEvalFiles(root)).toEqual([]); }); }); + +describe("discoverEvalConfigs", () => { + test("finds each agent's evals.config.ts, omits agents without one", () => { + write("config/agents/support/evals/basic.eval.ts"); + write("config/agents/support/evals/evals.config.ts"); + write("config/agents/analyst/evals/sql.eval.ts"); + + const found = discoverEvalConfigs(root); + + expect(found.map((c) => c.agent)).toEqual(["support"]); + expect(found[0].file).toBe( + path.join(root, "config/agents/support/evals/evals.config.ts"), + ); + }); + + test("returns empty when there is no config/agents dir", () => { + expect(discoverEvalConfigs(root)).toEqual([]); + }); +}); diff --git a/packages/appkit/src/evals/tests/resolve-default.test.ts b/packages/appkit/src/evals/tests/resolve-default.test.ts index 2dcc8a449..0c3927343 100644 --- a/packages/appkit/src/evals/tests/resolve-default.test.ts +++ b/packages/appkit/src/evals/tests/resolve-default.test.ts @@ -1,6 +1,10 @@ import { describe, expect, test } from "vitest"; -import { resolveEvalDefault } from "../run-evals"; +import { + matchesTags, + resolveConfigDefault, + resolveEvalDefault, +} from "../run-evals"; const def = { description: "x", test: async () => {} }; @@ -24,3 +28,40 @@ describe("resolveEvalDefault (module interop)", () => { expect(resolveEvalDefault(null)).toBeUndefined(); }); }); + +const config = { maxConcurrency: 4, timeoutMs: 1000 }; + +describe("resolveConfigDefault (module interop)", () => { + test("pure ESM: mod.default", () => { + expect(resolveConfigDefault({ default: config })).toBe(config); + }); + + test("CJS __esModule double-wrap: mod.default.default", () => { + expect( + resolveConfigDefault({ default: { __esModule: true, default: config } }), + ).toBe(config); + }); + + test("no default export → undefined", () => { + expect(resolveConfigDefault(null)).toBeUndefined(); + expect(resolveConfigDefault(undefined)).toBeUndefined(); + }); +}); + +describe("matchesTags", () => { + test("no filter runs everything", () => { + expect(matchesTags(["a"], undefined)).toBe(true); + expect(matchesTags(undefined, [])).toBe(true); + expect(matchesTags(undefined, undefined)).toBe(true); + }); + + test("matches when tags intersect the filter", () => { + expect(matchesTags(["smoke", "slow"], ["smoke"])).toBe(true); + }); + + test("excludes when tags don't intersect or the def has none", () => { + expect(matchesTags(["slow"], ["smoke"])).toBe(false); + expect(matchesTags(undefined, ["smoke"])).toBe(false); + expect(matchesTags([], ["smoke"])).toBe(false); + }); +}); diff --git a/packages/appkit/src/evals/tests/run-eval.test.ts b/packages/appkit/src/evals/tests/run-eval.test.ts index 9e10b86c7..fdcff6269 100644 --- a/packages/appkit/src/evals/tests/run-eval.test.ts +++ b/packages/appkit/src/evals/tests/run-eval.test.ts @@ -269,4 +269,63 @@ describe("runEval", () => { }); expect(result.passed).toBe(true); }); + + test("def.timeoutMs turns a hanging test into a non-passing timeout result", async () => { + const def = defineEval({ + timeoutMs: 20, + async test() { + // Never resolves; only the timeout can settle the eval. + await new Promise(() => {}); + }, + }); + const result = await runEval(def, { id: "hang", driver: fakeDriver({}) }); + expect(result.passed).toBe(false); + expect(result.error).toBe("eval timed out after 20ms"); + }); + + test("a fast eval passes well under the same timeout", async () => { + const def = defineEval({ + timeoutMs: 20, + async test(t) { + await t.send("hi"); + t.succeeded(); + }, + }); + const result = await runEval(def, { + id: "fast", + driver: fakeDriver({ succeeded: true }), + }); + expect(result.passed).toBe(true); + expect(result.error).toBeUndefined(); + }); + + test("RunEvalOptions.timeoutMs applies when the def has none", async () => { + const def = defineEval({ + async test() { + await new Promise(() => {}); + }, + }); + const result = await runEval(def, { + id: "runner-timeout", + driver: fakeDriver({}), + timeoutMs: 20, + }); + expect(result.passed).toBe(false); + expect(result.error).toBe("eval timed out after 20ms"); + }); + + test("def.timeoutMs overrides the runner-level default", async () => { + const def = defineEval({ + timeoutMs: 15, + async test() { + await new Promise(() => {}); + }, + }); + const result = await runEval(def, { + id: "per-eval-wins", + driver: fakeDriver({}), + timeoutMs: 5000, + }); + expect(result.error).toBe("eval timed out after 15ms"); + }); }); diff --git a/packages/appkit/src/evals/types.ts b/packages/appkit/src/evals/types.ts index 009411f02..55b4f0286 100644 --- a/packages/appkit/src/evals/types.ts +++ b/packages/appkit/src/evals/types.ts @@ -152,6 +152,13 @@ export interface EvalDefinition { description?: string; /** Target agent id. Defaults to the eval's parent `server/agents/` dir. */ agent?: string; + /** Free-form tags for filtering (see the runner's `tags` / `--tag` option). */ + tags?: string[]; + /** + * Per-eval timeout (ms): `runEval` races the test against it and records a + * non-passing result instead of hanging. Overrides the runner/CLI default. + */ + timeoutMs?: number; /** * Run this eval once per row of a Databricks managed evaluation dataset (a * Unity Catalog `catalog.schema.table` with `inputs`/`expectations` columns). @@ -163,6 +170,16 @@ export interface EvalDefinition { test(t: TestContext): Promise | void; } +/** Per-directory config from `evals.config.ts` (see {@link defineEvalConfig}). */ +export interface EvalConfig { + /** LLM judge config. Defaults to the agent's own serving endpoint. */ + judge?: { model?: string }; + /** Max evals to run concurrently. */ + maxConcurrency?: number; + /** Default per-eval timeout. */ + timeoutMs?: number; +} + /** The outcome of running one eval. */ export interface EvalResult { id: string; diff --git a/packages/shared/src/cli/commands/agent/eval.ts b/packages/shared/src/cli/commands/agent/eval.ts index 6e0af2b95..72600ed1b 100644 --- a/packages/shared/src/cli/commands/agent/eval.ts +++ b/packages/shared/src/cli/commands/agent/eval.ts @@ -25,6 +25,7 @@ interface EvalRunner { rootDir?: string; baseUrl: string; filter?: string; + tags?: string[]; strict?: boolean; headers?: Record; concurrency?: number; @@ -37,6 +38,7 @@ interface EvalRunner { judge?: { host: string; token: string; model: string }; workspaceClient?: unknown; warehouseId?: string; + timeoutMs?: number; onEvent?: (event: EvalProgress) => void; }): Promise; resolveDatabricksAuth(opts: { @@ -89,6 +91,7 @@ interface EvalOptions { strict?: boolean; root?: string; header?: string[]; + tag?: string[]; profile?: string; databricksHost?: string; databricksToken?: string; @@ -96,6 +99,7 @@ interface EvalOptions { judgeModel?: string; concurrency?: number; warehouseId?: string; + timeout?: string; minPassRate?: string; } @@ -217,12 +221,20 @@ async function runAgentEval( const warehouseId = opts.warehouseId ?? process.env.DATABRICKS_WAREHOUSE_ID; const workspaceClient = runner.resolveWorkspaceClient(credentials); + // Runner-level default per-eval timeout (ms). A per-eval `timeoutMs` wins. + const parsedTimeout = opts.timeout + ? Number.parseInt(opts.timeout, 10) + : undefined; + const timeoutMs = + parsedTimeout && parsedTimeout > 0 ? parsedTimeout : undefined; + let summary: EvalRunSummary; try { summary = await runner.runEvalsInDir({ rootDir: opts.root, baseUrl: opts.url, filter, + tags: opts.tag, strict: opts.strict, headers: opts.header ? parseHeaders(opts.header) : undefined, concurrency: opts.concurrency, @@ -230,6 +242,7 @@ async function runAgentEval( judge: resolveJudge(opts, auth), workspaceClient, warehouseId, + timeoutMs, onEvent: makeProgressReporter(runner, opts.url), }); } catch (err) { @@ -295,6 +308,10 @@ export const agentEvalCommand = new Command("eval") "--header ", "Extra request header as 'Key: value' (repeatable)", ) + .option( + "--tag ", + "Only run evals tagged with one of these tags (repeatable)", + ) .option( "--profile ", "Databricks CLI profile to authenticate with via OAuth (default: DATABRICKS_CONFIG_PROFILE)", @@ -319,6 +336,10 @@ export const agentEvalCommand = new Command("eval") "--judge-model ", "Databricks serving endpoint to use as the LLM judge for t.judge.* (default: APPKIT_JUDGE_MODEL)", ) + .option( + "--timeout ", + "Default per-eval timeout in ms (a per-eval timeoutMs overrides it)", + ) .option( "--min-pass-rate ", "Gate on aggregate pass rate (0..1) instead of requiring every eval to pass; exit 1 when below", From 8c2199967ee22e2ea21e56e047159005117721cc Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Mon, 13 Jul 2026 13:36:51 +0200 Subject: [PATCH 06/11] feat(appkit): retry evals that fail on infra errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `--retries ` / `RunEvalsOptions.retries`: re-run an eval up to N extra times when it fails on an infrastructure error (a thrown error or timeout — `result.error` set), to absorb transient turn/stream flakiness. Assertion failures are never retried — a wrong reply is real signal, and retrying a flaky judge until it passes would corrupt the result. Each attempt gets a fresh driver. Extracted as `runWithRetries` (unit-tested for attempt counting, stop-on-success, and no-retry-on-assertion-failure). Signed-off-by: MarioCadenas --- packages/appkit/src/evals/index.ts | 1 + packages/appkit/src/evals/run-evals.ts | 62 ++++++++++++++----- .../src/evals/tests/run-with-retries.test.ts | 62 +++++++++++++++++++ .../shared/src/cli/commands/agent/eval.ts | 15 +++++ 4 files changed, 123 insertions(+), 17 deletions(-) create mode 100644 packages/appkit/src/evals/tests/run-with-retries.test.ts diff --git a/packages/appkit/src/evals/index.ts b/packages/appkit/src/evals/index.ts index 937169bc9..a7d38e665 100644 --- a/packages/appkit/src/evals/index.ts +++ b/packages/appkit/src/evals/index.ts @@ -49,6 +49,7 @@ export { type EvalRunSummary, type RunEvalsOptions, runEvalsInDir, + runWithRetries, } from "./run-evals"; export type { AssertionHandle, diff --git a/packages/appkit/src/evals/run-evals.ts b/packages/appkit/src/evals/run-evals.ts index 0dc0be2c4..43e4154d8 100644 --- a/packages/appkit/src/evals/run-evals.ts +++ b/packages/appkit/src/evals/run-evals.ts @@ -71,6 +71,13 @@ export interface RunEvalsOptions { * it wins over an agent's `evals.config.ts` `timeoutMs`. Unbounded when unset. */ timeoutMs?: number; + /** + * Re-run an eval up to this many extra times when it fails on an + * infrastructure error (a thrown error or timeout — `result.error` set), to + * absorb transient turn/stream flakiness. Assertion failures are NEVER + * retried (a wrong reply is real signal, not flake). Defaults to `0`. + */ + retries?: number; /** Progress callback, invoked as evals are discovered, started, and finished. */ onEvent?: (event: EvalProgress) => void; } @@ -182,23 +189,25 @@ async function runOne( options: RunEvalsOptions, ): Promise { try { - // A fresh driver per row: each row is an independent conversation whose - // thread must not carry over the previous row's history. (Multiple - // `t.send`s within one row still share the thread — the driver's behavior.) - const driver = createHttpDriver({ - baseUrl: options.baseUrl, - agent: def.agent ?? d.agent, - headers: options.headers, - mlflowRunId: runId, - timeoutMs: options.timeoutMs, - }); - return await runEval(def, { - id, - driver, - strict: options.strict, - row, - timeoutMs: options.timeoutMs, - }); + // Retry only on an infrastructure error (`result.error` — a thrown error or + // timeout), to absorb transient turn/stream flakiness; assertion failures + // are real signal and returned on the first try. Each attempt gets a fresh + // driver, so its thread never carries over the failed attempt's history. + return await runWithRetries(options.retries ?? 0, () => + runEval(def, { + id, + driver: createHttpDriver({ + baseUrl: options.baseUrl, + agent: def.agent ?? d.agent, + headers: options.headers, + mlflowRunId: runId, + timeoutMs: options.timeoutMs, + }), + strict: options.strict, + row, + timeoutMs: options.timeoutMs, + }), + ); } catch (err) { return { id, @@ -298,6 +307,25 @@ async function runDiscovered( } } +/** + * Run `attempt` up to `1 + retries` times, stopping as soon as it returns a + * result without an `error` (infra failures — thrown errors or timeouts — set + * `error`; assertion failures do not, so a failed-but-completed eval is returned + * on the first try and never retried). Returns the last result when every + * attempt errored. `retries` below 0 is treated as 0. + */ +export async function runWithRetries( + retries: number, + attempt: (attemptNumber: number) => Promise, +): Promise { + const maxAttempts = 1 + Math.max(0, retries); + let result: EvalResult; + for (let n = 1; ; n++) { + result = await attempt(n); + if (!result.error || n >= maxAttempts) return result; + } +} + /** * Whether an eval's `tags` satisfy a `--tag` filter: `true` when the filter is * empty/undefined (no filtering), otherwise only when the eval shares at least diff --git a/packages/appkit/src/evals/tests/run-with-retries.test.ts b/packages/appkit/src/evals/tests/run-with-retries.test.ts new file mode 100644 index 000000000..f4bbd1614 --- /dev/null +++ b/packages/appkit/src/evals/tests/run-with-retries.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, test } from "vitest"; + +import { runWithRetries } from "../run-evals"; +import type { EvalResult } from "../types"; + +describe("runWithRetries", () => { + const errored = (n: number): EvalResult => ({ + id: `try-${n}`, + assertions: [], + passed: false, + error: "turn failed", + }); + const ok = (n: number): EvalResult => ({ + id: `try-${n}`, + assertions: [], + passed: true, + }); + const assertionFail = (n: number): EvalResult => ({ + id: `try-${n}`, + assertions: [{ label: "check", severity: "gate", pass: false }], + passed: false, + }); + + test("retries an infra error up to `retries` extra times, then returns the last", async () => { + let calls = 0; + const result = await runWithRetries(2, async (n) => { + calls = n; + return errored(n); + }); + expect(calls).toBe(3); // 1 initial + 2 retries + expect(result.error).toBe("turn failed"); + }); + + test("stops as soon as an attempt succeeds", async () => { + let calls = 0; + const result = await runWithRetries(5, async (n) => { + calls = n; + return n < 2 ? errored(n) : ok(n); + }); + expect(calls).toBe(2); // errored once, then ok + expect(result.passed).toBe(true); + }); + + test("never retries an assertion failure (no error set)", async () => { + let calls = 0; + const result = await runWithRetries(3, async (n) => { + calls = n; + return assertionFail(n); + }); + expect(calls).toBe(1); + expect(result.passed).toBe(false); + }); + + test("retries=0 runs exactly once", async () => { + let calls = 0; + await runWithRetries(0, async (n) => { + calls = n; + return errored(n); + }); + expect(calls).toBe(1); + }); +}); diff --git a/packages/shared/src/cli/commands/agent/eval.ts b/packages/shared/src/cli/commands/agent/eval.ts index 72600ed1b..5fce9eed9 100644 --- a/packages/shared/src/cli/commands/agent/eval.ts +++ b/packages/shared/src/cli/commands/agent/eval.ts @@ -39,6 +39,7 @@ interface EvalRunner { workspaceClient?: unknown; warehouseId?: string; timeoutMs?: number; + retries?: number; onEvent?: (event: EvalProgress) => void; }): Promise; resolveDatabricksAuth(opts: { @@ -100,6 +101,7 @@ interface EvalOptions { concurrency?: number; warehouseId?: string; timeout?: string; + retries?: string; minPassRate?: string; } @@ -228,6 +230,14 @@ async function runAgentEval( const timeoutMs = parsedTimeout && parsedTimeout > 0 ? parsedTimeout : undefined; + // Extra attempts for evals that fail on an infra error (turn/timeout). Junk + // or negative input falls back to no retries. + const parsedRetries = opts.retries + ? Number.parseInt(opts.retries, 10) + : undefined; + const retries = + parsedRetries && parsedRetries > 0 ? parsedRetries : undefined; + let summary: EvalRunSummary; try { summary = await runner.runEvalsInDir({ @@ -243,6 +253,7 @@ async function runAgentEval( workspaceClient, warehouseId, timeoutMs, + retries, onEvent: makeProgressReporter(runner, opts.url), }); } catch (err) { @@ -340,6 +351,10 @@ export const agentEvalCommand = new Command("eval") "--timeout ", "Default per-eval timeout in ms (a per-eval timeoutMs overrides it)", ) + .option( + "--retries ", + "Re-run an eval up to N times when it fails on an infra error (turn/timeout); assertion failures are not retried", + ) .option( "--min-pass-rate ", "Gate on aggregate pass rate (0..1) instead of requiring every eval to pass; exit 1 when below", From 0a3f04bb68413cbe1658608997c5274f6d0553ac Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Mon, 13 Jul 2026 13:51:43 +0200 Subject: [PATCH 07/11] feat(appkit): json and junit eval reporters Add `--reporter ` (default text) and `--output ` to the agent eval CLI. `formatResultsJson` emits `{summary, results}`; `formatResultsJUnit` emits a `` with a `` per eval (``/`` as appropriate, all values XML-escaped). In json/junit mode the per-eval streaming is suppressed and human banners go to stderr so stdout stays clean for piping or `--output`; exit-code and pass-rate gating are unchanged. Signed-off-by: MarioCadenas --- packages/appkit/src/evals/index.ts | 2 + packages/appkit/src/evals/report.ts | 64 +++++++++++++++ .../appkit/src/evals/tests/report.test.ts | 76 +++++++++++++++++- .../shared/src/cli/commands/agent/eval.ts | 79 ++++++++++++++++--- 4 files changed, 209 insertions(+), 12 deletions(-) diff --git a/packages/appkit/src/evals/index.ts b/packages/appkit/src/evals/index.ts index a7d38e665..afed3046a 100644 --- a/packages/appkit/src/evals/index.ts +++ b/packages/appkit/src/evals/index.ts @@ -40,6 +40,8 @@ export { formatEvalDetail, formatEvalHeadline, formatEvalResults, + formatResultsJson, + formatResultsJUnit, formatSummaryLine, summarize, } from "./report"; diff --git a/packages/appkit/src/evals/report.ts b/packages/appkit/src/evals/report.ts index 409484254..8b903f0d9 100644 --- a/packages/appkit/src/evals/report.ts +++ b/packages/appkit/src/evals/report.ts @@ -78,3 +78,67 @@ export function formatEvalResults(results: EvalResult[]): string { lines.push(formatSummaryLine(results)); return lines.join("\n"); } + +/** + * Render results as a machine-readable JSON report (2-space indented): + * `{ summary: EvalSummary, results: EvalResult[] }`. Faithful to the types — + * every field present on a result round-trips. + */ +export function formatResultsJson(results: EvalResult[]): string { + return JSON.stringify({ summary: summarize(results), results }, null, 2); +} + +/** Escape a value for use in XML text/attribute content. */ +function escapeXml(value: string): string { + return value + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); +} + +/** One-line reason a result failed: its error, else its failing gate labels. */ +function failureMessage(result: EvalResult): string { + if (result.error) return result.error; + const gates = result.assertions + .filter((a) => !a.pass && a.severity === "gate") + .map((a) => (a.detail ? `${a.label} — ${a.detail}` : a.label)); + return gates.length ? gates.join("; ") : "eval failed"; +} + +/** + * Render results as JUnit XML for standard CI test reporters: a single + * `` with one `` per result. + * Failures carry a `` (error or failing-gate summary); skips a + * ``. All attribute/text values are XML-escaped. + */ +export function formatResultsJUnit(results: EvalResult[]): string { + const s = summarize(results); + const lines: string[] = []; + lines.push(''); + lines.push( + ``, + ); + for (const r of results) { + const open = ` `); + lines.push( + r.skipped.reason + ? ` ` + : " ", + ); + lines.push(" "); + } else if (!r.passed) { + const message = failureMessage(r); + lines.push(`${open}>`); + lines.push(` `); + lines.push(" "); + } else { + lines.push(`${open}/>`); + } + } + lines.push(""); + return lines.join("\n"); +} diff --git a/packages/appkit/src/evals/tests/report.test.ts b/packages/appkit/src/evals/tests/report.test.ts index 2d5ea605e..c752e9d2f 100644 --- a/packages/appkit/src/evals/tests/report.test.ts +++ b/packages/appkit/src/evals/tests/report.test.ts @@ -1,6 +1,11 @@ import { describe, expect, test } from "vitest"; -import { formatEvalResults, summarize } from "../report"; +import { + formatEvalResults, + formatResultsJson, + formatResultsJUnit, + summarize, +} from "../report"; import type { EvalResult } from "../types"; const results: EvalResult[] = [ @@ -59,4 +64,73 @@ describe("eval reporting", () => { expect(out).toContain("a/skip (skipped: no data)"); expect(out).toContain("FAIL — 1 passed, 1 failed, 1 skipped (3 total)"); }); + + test("formatResultsJson round-trips summary and result fields", () => { + const parsed = JSON.parse(formatResultsJson(results)); + expect(parsed.summary).toEqual({ + total: 3, + passed: 1, + failed: 1, + skipped: 1, + allPassed: false, + passRate: 0.5, + }); + expect(parsed.results).toHaveLength(3); + const fail = parsed.results.find((r: EvalResult) => r.id === "a/fail"); + expect(fail.passed).toBe(false); + expect(fail.assertions).toEqual([ + { + label: "calledTool(x)", + severity: "gate", + pass: false, + detail: "not called", + }, + ]); + const skip = parsed.results.find((r: EvalResult) => r.id === "a/skip"); + expect(skip.skipped).toEqual({ reason: "no data" }); + }); + + test("formatResultsJson round-trips a result's error field", () => { + const errored: EvalResult[] = [ + { + id: "a/threw", + assertions: [], + passed: false, + error: "boom: turn failed", + }, + ]; + const parsed = JSON.parse(formatResultsJson(errored)); + expect(parsed.results[0].error).toBe("boom: turn failed"); + expect(parsed.summary.failed).toBe(1); + }); + + test("formatResultsJUnit emits suite counts, failure, skipped, and escapes special chars", () => { + const withSpecial: EvalResult[] = [ + ...results, + { + id: 'a/b & "q"', + assertions: [ + { + label: "check", + severity: "gate", + pass: false, + detail: 'reply had & "quote"', + }, + ], + passed: false, + }, + ]; + const xml = formatResultsJUnit(withSpecial); + expect(xml).toContain( + '', + ); + expect(xml).toContain(''); + expect(xml).toContain(""); + }); }); diff --git a/packages/shared/src/cli/commands/agent/eval.ts b/packages/shared/src/cli/commands/agent/eval.ts index 5fce9eed9..17e6f5a72 100644 --- a/packages/shared/src/cli/commands/agent/eval.ts +++ b/packages/shared/src/cli/commands/agent/eval.ts @@ -1,4 +1,5 @@ -import { Command } from "commander"; +import fs from "node:fs"; +import { Command, Option } from "commander"; interface EvalRunSummary { results: unknown[]; @@ -56,6 +57,8 @@ interface EvalRunner { evalGlyph(result: unknown): string; formatEvalDetail(result: unknown): string[]; formatSummaryLine(results: unknown[]): string; + formatResultsJson(results: unknown[]): string; + formatResultsJUnit(results: unknown[]): string; summarize(results: unknown[]): { allPassed: boolean; passRate: number }; } @@ -103,6 +106,8 @@ interface EvalOptions { timeout?: string; retries?: string; minPassRate?: string; + reporter?: "text" | "json" | "junit"; + output?: string; } /** Resolved Databricks host + bearer (either field may be absent). */ @@ -137,22 +142,29 @@ function resolveJudge(opts: EvalOptions, auth: Auth) { : undefined; } -/** Progress reporter: stream each eval as it runs instead of going silent. */ +/** + * Progress reporter: stream each eval as it runs instead of going silent. In a + * machine reporter (json/junit) the live per-eval streaming is suppressed and + * banners go to stderr (via `info`), keeping stdout clean for the report. + */ function makeProgressReporter( runner: EvalRunner, url: string, + machine: boolean, + info: (msg: string) => void, ): (event: EvalProgress) => void { return (event) => { switch (event.type) { case "discovered": - console.log( + info( `Running ${event.total} eval${event.total === 1 ? "" : "s"} against ${url}\n`, ); break; case "run-created": - console.log(`MLflow evaluation run: ${event.runId}\n`); + info(`MLflow evaluation run: ${event.runId}\n`); break; case "result": { + if (machine) break; // One full line per completion — evals run concurrently, so a split // "start … glyph" prefix would interleave into garbage. console.log( @@ -175,12 +187,17 @@ function formatFailureLine(f: { return ` ✗ trace ${f.traceId}: ${f.status ?? ""} ${f.error ?? ""}`.trim(); } -/** Print the MLflow assessment/finish outcome after a run that created one. */ +/** + * Print the MLflow assessment/finish outcome after a run that created one. The + * summary line goes through `info` (stderr under a machine reporter); per-trace + * failures and finish errors always go to stderr. + */ function printMlflowOutcome( mlflow: NonNullable, + info: (msg: string) => void, ): void { const { report, finish } = mlflow; - console.log( + info( `MLflow: ${report.written} assessment(s) written` + (report.skipped ? `, ${report.skipped} skipped` : "") + (report.failures.length ? `, ${report.failures.length} failed` : ""), @@ -238,6 +255,16 @@ async function runAgentEval( const retries = parsedRetries && parsedRetries > 0 ? parsedRetries : undefined; + // In a machine reporter (json/junit), stdout is reserved for the report (it + // may be piped), so human-facing lines go to stderr and the per-eval live + // streaming is suppressed. Text mode keeps its current stdout behavior. + const reporter = opts.reporter ?? "text"; + const machine = reporter !== "text"; + const info = (msg: string): void => { + if (machine) console.error(msg); + else console.log(msg); + }; + let summary: EvalRunSummary; try { summary = await runner.runEvalsInDir({ @@ -254,7 +281,7 @@ async function runAgentEval( warehouseId, timeoutMs, retries, - onEvent: makeProgressReporter(runner, opts.url), + onEvent: makeProgressReporter(runner, opts.url, machine, info), }); } catch (err) { // Setup failures (e.g. a bad --experiment for the MLflow run) reject before @@ -266,17 +293,35 @@ async function runAgentEval( process.exitCode = 1; return; } - console.log(`\n${runner.formatSummaryLine(summary.results)}`); + + // The final human summary always shows (stderr for machine reporters so it + // never pollutes the report on stdout/file). + info(`\n${runner.formatSummaryLine(summary.results)}`); if (summary.mlflow) { - printMlflowOutcome(summary.mlflow); + printMlflowOutcome(summary.mlflow, info); } else { - console.log( + info( "\nMLflow evaluation run skipped — pass --experiment (or set" + " MLFLOW_EXPERIMENT_ID) plus --profile/--databricks-host to create one.", ); } + // Machine-readable report: build the string with a pure formatter, then emit + // it to --output or stdout (kept clean of the human noise above). + if (machine) { + const report = + reporter === "json" + ? runner.formatResultsJson(summary.results) + : runner.formatResultsJUnit(summary.results); + if (opts.output) { + fs.writeFileSync(opts.output, `${report}\n`); + info(`Wrote ${reporter} report to ${opts.output}`); + } else { + process.stdout.write(`${report}\n`); + } + } + const stats = runner.summarize(summary.results); const minPassRate = opts.minPassRate ? Number.parseFloat(opts.minPassRate) @@ -285,7 +330,7 @@ async function runAgentEval( // Threshold mode: gate on the aggregate pass rate rather than requiring // every eval to pass. const ok = stats.passRate >= minPassRate; - console.log( + info( `Pass rate ${(stats.passRate * 100).toFixed(0)}% (threshold ${( minPassRate * 100 ).toFixed(0)}%) — ${ok ? "OK" : "below threshold"}`, @@ -359,4 +404,16 @@ export const agentEvalCommand = new Command("eval") "--min-pass-rate ", "Gate on aggregate pass rate (0..1) instead of requiring every eval to pass; exit 1 when below", ) + .addOption( + new Option( + "--reporter ", + "Report format: text (live console), json (dashboards), or junit (CI test reporters)", + ) + .choices(["text", "json", "junit"]) + .default("text"), + ) + .option( + "--output ", + "Write the json/junit report to this file instead of stdout (ignored for text)", + ) .action(runAgentEval); From 0a53849af320485e729a1bd628006bd3b86f2e9e Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Tue, 1 Sep 2026 18:27:04 +0200 Subject: [PATCH 08/11] test(appkit): use server/agents layout in eval config discovery fixtures The discoverEvalConfigs test still built fixtures under config/agents/ from the pre-rename stack; the runner now discovers per-agent configs under server/agents/ (the folder-per-agent layout). Align the fixtures so the test matches the runner. Signed-off-by: MarioCadenas --- packages/appkit/src/evals/run-evals.ts | 10 +++++++++- packages/appkit/src/evals/tests/discover.test.ts | 10 +++++----- packages/shared/src/cli/commands/agent/eval.ts | 1 + 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/appkit/src/evals/run-evals.ts b/packages/appkit/src/evals/run-evals.ts index 43e4154d8..111994a2a 100644 --- a/packages/appkit/src/evals/run-evals.ts +++ b/packages/appkit/src/evals/run-evals.ts @@ -504,7 +504,15 @@ export async function runEvalsInDir( ...options, timeoutMs: options.timeoutMs ?? configs.get(d.agent)?.timeoutMs, }; - await runDiscovered(d, index, total, runId, fileOptions, emit, fileResults); + await runDiscovered( + d, + index, + total, + runId, + fileOptions, + emit, + fileResults, + ); return fileResults; }); const results = perFile.flat(); diff --git a/packages/appkit/src/evals/tests/discover.test.ts b/packages/appkit/src/evals/tests/discover.test.ts index ac8e4df5f..cd2d949af 100644 --- a/packages/appkit/src/evals/tests/discover.test.ts +++ b/packages/appkit/src/evals/tests/discover.test.ts @@ -45,19 +45,19 @@ describe("discoverEvalFiles", () => { describe("discoverEvalConfigs", () => { test("finds each agent's evals.config.ts, omits agents without one", () => { - write("config/agents/support/evals/basic.eval.ts"); - write("config/agents/support/evals/evals.config.ts"); - write("config/agents/analyst/evals/sql.eval.ts"); + write("server/agents/support/evals/basic.eval.ts"); + write("server/agents/support/evals/evals.config.ts"); + write("server/agents/analyst/evals/sql.eval.ts"); const found = discoverEvalConfigs(root); expect(found.map((c) => c.agent)).toEqual(["support"]); expect(found[0].file).toBe( - path.join(root, "config/agents/support/evals/evals.config.ts"), + path.join(root, "server/agents/support/evals/evals.config.ts"), ); }); - test("returns empty when there is no config/agents dir", () => { + test("returns empty when there is no server/agents dir", () => { expect(discoverEvalConfigs(root)).toEqual([]); }); }); diff --git a/packages/shared/src/cli/commands/agent/eval.ts b/packages/shared/src/cli/commands/agent/eval.ts index 17e6f5a72..a638769ad 100644 --- a/packages/shared/src/cli/commands/agent/eval.ts +++ b/packages/shared/src/cli/commands/agent/eval.ts @@ -1,4 +1,5 @@ import fs from "node:fs"; + import { Command, Option } from "commander"; interface EvalRunSummary { From e918e4385a0972ea87eaac36d4b19448effdb81a Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Wed, 9 Sep 2026 12:20:05 +0200 Subject: [PATCH 09/11] chore(playground): drop example dataset eval that used a private UC table Signed-off-by: MarioCadenas --- .../server/agents/query/evals/dataset.eval.ts | 55 ------------------- 1 file changed, 55 deletions(-) delete mode 100644 apps/dev-playground/server/agents/query/evals/dataset.eval.ts diff --git a/apps/dev-playground/server/agents/query/evals/dataset.eval.ts b/apps/dev-playground/server/agents/query/evals/dataset.eval.ts deleted file mode 100644 index 25184283d..000000000 --- a/apps/dev-playground/server/agents/query/evals/dataset.eval.ts +++ /dev/null @@ -1,55 +0,0 @@ -import { - defineEval, - isJudgeConfigured, - userTurns, -} from "@databricks/appkit/beta"; - -/** - * Dataset-driven eval: runs once per row of a Databricks managed evaluation - * dataset (a Unity Catalog table with `inputs`/`expectations` columns). The - * runner binds each row to `t.input`/`t.expected`. - * - * Run it (reading the dataset needs a workspace client + warehouse; judging the - * guidelines needs a judge model): - * appkit agent eval dataset --root apps/dev-playground --url http://localhost:8000 \ - * --profile --warehouse --judge-model - * - * Row shape produced by the MLflow managed-dataset UI: - * inputs {"messages":[{"role":"user","content":"..."}]} - * expectations {"guidelines":{"value":["...","..."]}} (optional) - * - * A row's `messages` can be a full multi-turn conversation. We replay each USER - * turn in order against one shared thread (below); interleaved assistant turns - * in the row are ignored — the agent generates its own responses. - */ - -/** Read `expectations.guidelines` — the UI wraps the array as `{value: [...]}`. */ -function guidelines(expected: Record | undefined): string[] { - const g = (expected?.guidelines as { value?: unknown } | undefined)?.value; - return Array.isArray(g) ? g.map(String) : []; -} - -export default defineEval({ - description: "Query agent satisfies each dataset row's guidelines", - // Point at your own managed evaluation dataset (catalog.schema.table). - dataset: { table: "main.mario.appkit_eval_dataset" }, - async test(t) { - // Replay every user turn in the row against one thread, so the agent sees - // the accumulating conversation. A single-user-turn row sends once. The - // runner gives each row a fresh driver, so rows don't bleed into each other. - for (const turn of userTurns(t.input)) { - await t.send(turn); - } - t.succeeded(); - - // Each guideline is judged against the reply — gate by default, so a miss - // fails the eval (chain `.soft()` to only track it). Skipped cleanly when no - // judge model is configured, so the eval still exercises the dataset read + - // drive path without one. - if (isJudgeConfigured()) { - for (const guideline of guidelines(t.expected)) { - (await t.judge.closedQA(guideline)).atLeast(0.5); - } - } - }, -}); From 9a25a9f7bd28d9b8fb81fddfca2b9899c8f4b6b9 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Wed, 9 Sep 2026 16:58:05 +0200 Subject: [PATCH 10/11] fix(appkit): harden eval runner timeout, retry, concurrency, reporting Council review of the eval-suite PR surfaced correctness gaps in the newly-shipped surface. Fixes, each with tests: - timeout: thread an AbortSignal through the driver so a timed-out eval cancels its in-flight stream instead of leaking it, and pass the effective per-eval timeout to the driver - retry: retry turn/transport failures (driver `succeeded: false`), not just thrown errors; only retry a failing eval; jittered exponential backoff; coerce a non-finite `retries` to avoid an infinite loop - calledToolWith: match array args element-for-element and require expected keys to be present; report arg key names, not values (CWE-532) - concurrency: honor the lowest `maxConcurrency` across participating agents rather than the max across all configs - --min-pass-rate: reject out-of-range, non-numeric, or blank values up front instead of silently disabling/inverting the CI gate - reporting: strip XML-illegal control chars from JUnit output - misc: guard userTurns against malformed dataset rows; drop the dead EvalConfig.judge field; load each eval file once; guard the --output write Co-authored-by: Isaac Signed-off-by: MarioCadenas --- packages/appkit/src/evals/dataset.ts | 15 +- packages/appkit/src/evals/http-driver.ts | 12 +- packages/appkit/src/evals/report.ts | 23 ++- packages/appkit/src/evals/run-eval.ts | 69 ++++--- packages/appkit/src/evals/run-evals.ts | 175 ++++++++++++------ .../appkit/src/evals/tests/dataset.test.ts | 12 ++ .../evals/tests/derive-concurrency.test.ts | 38 ++++ .../appkit/src/evals/tests/report.test.ts | 18 ++ .../appkit/src/evals/tests/run-eval.test.ts | 151 +++++++++++++++ .../src/evals/tests/run-with-retries.test.ts | 98 ++++++++-- packages/appkit/src/evals/types.ts | 21 ++- .../src/cli/commands/agent/eval.test.ts | 22 +++ .../shared/src/cli/commands/agent/eval.ts | 49 ++++- 13 files changed, 592 insertions(+), 111 deletions(-) create mode 100644 packages/appkit/src/evals/tests/derive-concurrency.test.ts create mode 100644 packages/shared/src/cli/commands/agent/eval.test.ts diff --git a/packages/appkit/src/evals/dataset.ts b/packages/appkit/src/evals/dataset.ts index 4edc49918..130caad3e 100644 --- a/packages/appkit/src/evals/dataset.ts +++ b/packages/appkit/src/evals/dataset.ts @@ -34,10 +34,17 @@ export interface ReadEvalDatasetOptions { * yields `[]`. */ export function userTurns(input: Record): string[] { - const messages = Array.isArray(input.messages) - ? (input.messages as Array<{ role?: string; content?: string }>) - : []; - return messages.filter((m) => m.role === "user").map((m) => m.content ?? ""); + const messages = Array.isArray(input.messages) ? input.messages : []; + // Guard each entry: a managed dataset row is external data, so a `null` or + // non-object entry must not crash the read, and non-string content coerces to + // "" rather than violate the declared `string[]` return. + return messages + .filter( + (m): m is { role?: unknown; content?: unknown } => + typeof m === "object" && m !== null, + ) + .filter((m) => m.role === "user") + .map((m) => (typeof m.content === "string" ? m.content : "")); } /** A managed eval dataset is a UC table; only 3-level names are valid. */ diff --git a/packages/appkit/src/evals/http-driver.ts b/packages/appkit/src/evals/http-driver.ts index 1664ca25c..565538890 100644 --- a/packages/appkit/src/evals/http-driver.ts +++ b/packages/appkit/src/evals/http-driver.ts @@ -160,10 +160,18 @@ export function createHttpDriver(options: HttpDriverOptions): EvalDriver { reset(): void { threadId = undefined; }, - async send(message: string): Promise { + async send( + message: string, + opts?: { signal?: AbortSignal }, + ): Promise { // Bounds connect + the entire read below. Passed to both the fetch and // the SSE reader: on expiry the reader is cancelled and the turn fails. - const signal = AbortSignal.timeout(timeoutMs); + // Composed with the caller's signal (the runner's per-eval timeout) so a + // timed-out eval aborts this turn instead of leaking a live stream. + const timeout = AbortSignal.timeout(timeoutMs); + const signal = opts?.signal + ? AbortSignal.any([timeout, opts.signal]) + : timeout; let res: Response; try { res = await fetch(`${options.baseUrl}${chatPath}`, { diff --git a/packages/appkit/src/evals/report.ts b/packages/appkit/src/evals/report.ts index 8b903f0d9..d40a02d01 100644 --- a/packages/appkit/src/evals/report.ts +++ b/packages/appkit/src/evals/report.ts @@ -88,9 +88,28 @@ export function formatResultsJson(results: EvalResult[]): string { return JSON.stringify({ summary: summarize(results), results }, null, 2); } -/** Escape a value for use in XML text/attribute content. */ +/** + * Drop the characters XML 1.0 forbids even when escaped — the C0 control chars + * except tab (9), LF (10), and CR (13). A raw NUL or ANSI escape from an agent + * reply or tool arg would otherwise make the JUnit document not well-formed and + * a strict CI parser reject it. + */ +function stripXmlControlChars(value: string): string { + // A regex char class is terser, but oxlint's `no-control-regex` rejects it + // (rule enabled repo-wide) — so filter by code point instead of suppressing. + let out = ""; + for (const ch of value) { + const code = ch.codePointAt(0) as number; + if (code >= 0x20 || code === 0x09 || code === 0x0a || code === 0x0d) { + out += ch; + } + } + return out; +} + +/** Escape a value for use in XML text/attribute content (control chars dropped). */ function escapeXml(value: string): string { - return value + return stripXmlControlChars(value) .replace(/&/g, "&") .replace(//g, ">") diff --git a/packages/appkit/src/evals/run-eval.ts b/packages/appkit/src/evals/run-eval.ts index 7129c9e40..6b58db196 100644 --- a/packages/appkit/src/evals/run-eval.ts +++ b/packages/appkit/src/evals/run-eval.ts @@ -22,23 +22,28 @@ class SkipSignal extends Error { } } -/** Rejects the test race when a per-eval timeout elapses. */ -class TimeoutSignal extends Error { - constructor(ms: number) { - super(`eval timed out after ${ms}ms`); - this.name = "TimeoutSignal"; - } -} - /** * Deep partial match: every key in `expected` is present in `actual` and equal, - * recursing into nested plain objects so extra actual keys are ignored. + * recursing into nested plain objects so extra actual keys are ignored. Arrays + * must match element-for-element (same length, deep-equal items) — an array is + * a value, not a partial shape, so reference equality would never match two + * equal arrays parsed from JSON. */ function deepContains(actual: unknown, expected: unknown): boolean { + if (Array.isArray(expected)) { + return ( + Array.isArray(actual) && + actual.length === expected.length && + expected.every((item, i) => deepContains(actual[i], item)) + ); + } if (isPlainObject(expected)) { if (!isPlainObject(actual)) return false; - return Object.keys(expected).every((key) => - deepContains(actual[key], expected[key]), + // Require the key to be present, so an expected `undefined` value does not + // silently match a key the actual args omit. + return Object.keys(expected).every( + (key) => + Object.hasOwn(actual, key) && deepContains(actual[key], expected[key]), ); } return actual === expected; @@ -81,6 +86,12 @@ export async function runEval( let sessionId: string | undefined; let lastTraceId: string | undefined; let lastSucceeded = false; + // Set when any turn fails at the transport/agent level (driver `succeeded: + // false`): surfaced as `infraFailure` so the runner can retry an infra flake. + let turnFailed = false; + // Aborted when the per-eval timeout elapses, cancelling the in-flight turn so + // a timed-out eval doesn't leak a live stream past its deadline. + const controller = new AbortController(); const record = ( label: string, @@ -128,12 +139,15 @@ export async function runEval( const t: TestContext = { async send(message) { lastInput = message; - const r = await options.driver.send(message); + const r = await options.driver.send(message, { + signal: controller.signal, + }); reply = r.reply; toolCalls = r.toolCalls; toolCallDetails = r.toolCallDetails; sessionId = r.sessionId; lastSucceeded = r.succeeded; + if (!r.succeeded) turnFailed = true; if (r.traceId) lastTraceId = r.traceId; }, reset() { @@ -175,8 +189,14 @@ export async function runEval( calledToolWith(name, expected) { const matching = toolCallDetails.filter((c) => c.name === name); const pass = matching.some((c) => deepContains(c.args, expected)); + // Report only the *keys* the agent passed, never their values — actual + // args can carry PII/secrets and this detail is persisted into the + // JSON/JUnit reports and MLflow rationales (CWE-532). `expected` is + // operator-authored, so it stays. const seen = matching.length - ? matching.map((c) => JSON.stringify(c.args)).join(", ") + ? matching + .map((c) => `{${Object.keys(c.args).sort().join(", ")}}`) + .join(", ") : "not called"; return record( `calledToolWith(${name})`, @@ -184,7 +204,7 @@ export async function runEval( undefined, `expected tool "${name}" to be called with ${JSON.stringify( expected, - )} (args seen: ${seen})`, + )} (arg keys seen: ${seen})`, ); }, check(value: string, matcher: Matcher) { @@ -230,14 +250,17 @@ export async function runEval( if (timeoutMs === undefined) { await def.test(t); } else { - // Race the test against a timeout; on elapse the sentinel rejects and we - // convert it to a non-passing result. The timer is cleared in `finally` - // so it can't keep the process alive after the test settles. + // Race the test against a timeout; on elapse we abort the in-flight driver + // turn, reject, and convert it to a non-passing result. The timer is + // cleared in `finally` so it can't keep the process alive after the test + // settles. Note: only the driver turn is cancelled — a test that hangs in + // non-driver code (a `t.judge.*` call, an in-test sleep) still runs to its + // own completion, though the eval's result is already recorded by then. const timeout = new Promise((_, reject) => { - timer = setTimeout( - () => reject(new TimeoutSignal(timeoutMs)), - timeoutMs, - ); + timer = setTimeout(() => { + controller.abort(); + reject(new Error(`eval timed out after ${timeoutMs}ms`)); + }, timeoutMs); }); await Promise.race([Promise.resolve(def.test(t)), timeout]); } @@ -274,5 +297,9 @@ export async function runEval( assertions, passed, traceId: lastTraceId, + // Only a *failing* eval whose turn broke at the transport/agent level is a + // retryable infra flake; a passing eval (or a pure assertion mismatch, where + // the turn itself succeeded) is real signal and must not be retried. + infraFailure: (turnFailed && !passed) || undefined, }; } diff --git a/packages/appkit/src/evals/run-evals.ts b/packages/appkit/src/evals/run-evals.ts index 111994a2a..378b536a7 100644 --- a/packages/appkit/src/evals/run-evals.ts +++ b/packages/appkit/src/evals/run-evals.ts @@ -1,3 +1,4 @@ +import { setTimeout as sleep } from "node:timers/promises"; import { pathToFileURL } from "node:url"; import { MlflowClient } from "../connectors/mlflow"; @@ -73,9 +74,11 @@ export interface RunEvalsOptions { timeoutMs?: number; /** * Re-run an eval up to this many extra times when it fails on an - * infrastructure error (a thrown error or timeout — `result.error` set), to - * absorb transient turn/stream flakiness. Assertion failures are NEVER - * retried (a wrong reply is real signal, not flake). Defaults to `0`. + * infrastructure failure — a thrown error or per-eval timeout (`result.error`) + * or a turn that failed at the transport/agent level (`result.infraFailure`, + * e.g. a failed fetch, 5xx, or dropped stream) — to absorb transient + * turn/stream flakiness. Assertion failures are NEVER retried (a wrong reply + * is real signal, not flake). Defaults to `0`. */ retries?: number; /** Progress callback, invoked as evals are discovered, started, and finished. */ @@ -142,16 +145,16 @@ async function loadEvalConfig(file: string): Promise { * object reached through the `default` chain is taken as the config. */ export function resolveConfigDefault(mod: unknown): EvalConfig | undefined { - const seen = new Set(); let candidate: unknown = mod; - for (let i = 0; i < 4 && candidate && !seen.has(candidate); i++) { + // The `i < 4` cap already bounds a self-referential `default` chain, so no + // visited-set is needed (mirrors resolveEvalDefault). + for (let i = 0; i < 4 && candidate; i++) { const next = (candidate as { default?: unknown }).default; if (next === undefined) { return typeof candidate === "object" ? (candidate as EvalConfig) : undefined; } - seen.add(candidate); candidate = next; } return undefined; @@ -189,10 +192,12 @@ async function runOne( options: RunEvalsOptions, ): Promise { try { - // Retry only on an infrastructure error (`result.error` — a thrown error or - // timeout), to absorb transient turn/stream flakiness; assertion failures - // are real signal and returned on the first try. Each attempt gets a fresh - // driver, so its thread never carries over the failed attempt's history. + // Retry on an infrastructure failure — a thrown error or per-eval timeout + // (`result.error`), or a turn that failed at the transport/agent level + // (`result.infraFailure`, e.g. a dropped stream or 5xx) — to absorb + // transient turn/stream flakiness; assertion failures are real signal and + // returned on the first try. Each attempt gets a fresh driver, so its thread + // never carries over the failed attempt's history. return await runWithRetries(options.retries ?? 0, () => runEval(def, { id, @@ -201,7 +206,10 @@ async function runOne( agent: def.agent ?? d.agent, headers: options.headers, mlflowRunId: runId, - timeoutMs: options.timeoutMs, + // Match the driver's per-turn cap to the eval's effective timeout so a + // `def.timeoutMs` shorter than the driver default can't leave the turn + // running past the deadline (`runEval` also aborts it via its signal). + timeoutMs: def.timeoutMs ?? options.timeoutMs, }), strict: options.strict, row, @@ -258,14 +266,17 @@ export async function resolveDatasetRows( } /** - * Load one discovered eval and run it, expanding a dataset-driven eval into one - * run per row. Appends one result per row to `results`, emitting `start`/ - * `result` around each. Never throws: a load or dataset-read failure surfaces as - * a non-passing result. `total` counts eval files, not rows — per-row detail is - * carried in the result id (`[row i/n]`). + * Run one already-loaded eval (from the `loaded` pre-pass), expanding a + * dataset-driven eval into one run per row. Appends one result per row to + * `results`, emitting `start`/`result` around each. Never throws: a load error + * (carried in `loadError`) or a dataset-read failure surfaces as a non-passing + * result. `total` counts eval files, not rows — per-row detail is carried in the + * result id (`[row i/n]`). */ async function runDiscovered( d: DiscoveredEval, + def: EvalDefinition, + loadError: string | undefined, index: number, total: number, runId: string | undefined, @@ -275,16 +286,15 @@ async function runDiscovered( ): Promise { const id = `${d.agent}/${d.id}`; - let def: EvalDefinition; - try { - def = await loadEval(d.file); - } catch (err) { + // The file failed to load in the pre-pass (its `def` is an unused + // placeholder) — surface that as one non-passing result. + if (loadError) { emit({ type: "start", id, index, total }); const result: EvalResult = { id, assertions: [], passed: false, - error: err instanceof Error ? err.message : String(err), + error: loadError, }; results.push(result); emit({ type: "result", result, index, total }); @@ -307,22 +317,44 @@ async function runDiscovered( } } +/** Base delay (ms) before the first retry; doubled per attempt, full-jittered, capped. */ +const DEFAULT_RETRY_BASE_DELAY_MS = 250; +/** Ceiling for a single retry backoff wait (ms). */ +const MAX_RETRY_DELAY_MS = 5_000; + /** * Run `attempt` up to `1 + retries` times, stopping as soon as it returns a - * result without an `error` (infra failures — thrown errors or timeouts — set - * `error`; assertion failures do not, so a failed-but-completed eval is returned - * on the first try and never retried). Returns the last result when every - * attempt errored. `retries` below 0 is treated as 0. + * result that is neither a thrown error / per-eval timeout (`error`) nor a + * transport/agent turn failure (`infraFailure`). Assertion failures set + * neither, so a failed-but-completed eval is returned on the first try and + * never retried. Returns the last result when every attempt failed on infra. + * + * Between attempts it waits a full-jittered exponential backoff — the targeted + * infra flakes (connection refused, the app's 429 stream-cap) are + * overload-correlated, so retrying instantly would amplify load. `retries` is + * coerced to a finite, non-negative integer, so a `NaN`/`Infinity` from direct + * API misuse can't loop forever. `baseDelayMs: 0` disables the wait (tests). */ export async function runWithRetries( retries: number, attempt: (attemptNumber: number) => Promise, + options: { baseDelayMs?: number } = {}, ): Promise { - const maxAttempts = 1 + Math.max(0, retries); + const baseDelayMs = options.baseDelayMs ?? DEFAULT_RETRY_BASE_DELAY_MS; + const maxRetries = Number.isFinite(retries) + ? Math.max(0, Math.floor(retries)) + : 0; + const maxAttempts = 1 + maxRetries; let result: EvalResult; for (let n = 1; ; n++) { result = await attempt(n); - if (!result.error || n >= maxAttempts) return result; + const infraFailed = result.error !== undefined || result.infraFailure; + if (!infraFailed || n >= maxAttempts) return result; + if (baseDelayMs > 0) { + // Full jitter: a random wait in [0, min(cap, base * 2^(n-1))]. + const ceiling = Math.min(baseDelayMs * 2 ** (n - 1), MAX_RETRY_DELAY_MS); + await sleep(Math.random() * ceiling); + } } } @@ -390,6 +422,31 @@ async function finalizeMlflow( */ const DEFAULT_CONCURRENCY = 4; +/** + * Resolve the work-pool width: `--concurrency` wins; otherwise the lowest + * `maxConcurrency` any *participating* agent's `evals.config.ts` requests + * (every eval drives the app as the same user and shares one per-user stream + * budget, so the most conservative ceiling governs — a `Math.max` would let one + * lax agent raise another's limit past its intent and the server's stream cap); + * an agent with no eval in this run doesn't constrain it. Falls back to + * {@link DEFAULT_CONCURRENCY}. + */ +export function deriveConcurrency( + activeAgents: Set, + configs: Map, + cliConcurrency: number | undefined, +): number { + const configMin = [...configs.entries()] + .filter(([agent]) => activeAgents.has(agent)) + .map(([, c]) => c.maxConcurrency) + .filter((n): n is number => typeof n === "number") + .reduce( + (min, n) => (min === undefined ? n : Math.min(min, n)), + undefined, + ); + return cliConcurrency ?? configMin ?? DEFAULT_CONCURRENCY; +} + /** * Discover, load, and run every eval under each agent's `evals/` dir, driving * the agents on a running app. Never throws for an individual eval — load/run @@ -450,19 +507,17 @@ export async function runEvalsInDir( loaded.push({ d, def }); } - // `evals.config.ts` `maxConcurrency` governs the single shared work pool, so - // it can't be applied per-agent without splitting the pool. The `--concurrency` - // flag wins; else the highest value any agent's config requests (the pool - // ceiling); else the built-in default. - const configMaxConcurrency = [...configs.values()] - .map((c) => c.maxConcurrency) - .filter((n): n is number => typeof n === "number") - .reduce( - (max, n) => (max === undefined ? n : Math.max(max, n)), - undefined, - ); - const concurrency = - options.concurrency ?? configMaxConcurrency ?? DEFAULT_CONCURRENCY; + // `evals.config.ts` `maxConcurrency` governs the single shared work pool (all + // evals run as the same user against one per-user stream budget). Take the + // lowest ceiling any *participating* agent requests so a filtered-out or laxer + // agent can't raise another's limit past its intent — see + // {@link deriveConcurrency}. + const activeAgents = new Set(loaded.map((l) => l.d.agent)); + const concurrency = deriveConcurrency( + activeAgents, + configs, + options.concurrency, + ); const total = loaded.length; emit({ type: "discovered", total }); @@ -498,23 +553,29 @@ export async function runEvalsInDir( // the file's options (CLI wins over `evals.config.ts`; `def.timeoutMs` still // overrides, applied inside runEval). `total` counts eval files, not dataset // rows — per-row detail is carried in the result id (`[row i/n]`). - const perFile = await mapPool(loaded, concurrency, async ({ d }, index) => { - const fileResults: EvalResult[] = []; - const fileOptions: RunEvalsOptions = { - ...options, - timeoutMs: options.timeoutMs ?? configs.get(d.agent)?.timeoutMs, - }; - await runDiscovered( - d, - index, - total, - runId, - fileOptions, - emit, - fileResults, - ); - return fileResults; - }); + const perFile = await mapPool( + loaded, + concurrency, + async ({ d, def, loadError }, index) => { + const fileResults: EvalResult[] = []; + const fileOptions: RunEvalsOptions = { + ...options, + timeoutMs: options.timeoutMs ?? configs.get(d.agent)?.timeoutMs, + }; + await runDiscovered( + d, + def, + loadError, + index, + total, + runId, + fileOptions, + emit, + fileResults, + ); + return fileResults; + }, + ); const results = perFile.flat(); const summary: EvalRunSummary = { results }; diff --git a/packages/appkit/src/evals/tests/dataset.test.ts b/packages/appkit/src/evals/tests/dataset.test.ts index 2773ebdaf..12214d1ac 100644 --- a/packages/appkit/src/evals/tests/dataset.test.ts +++ b/packages/appkit/src/evals/tests/dataset.test.ts @@ -143,4 +143,16 @@ describe("userTurns", () => { expect(userTurns({ messages: "nope" })).toEqual([]); expect(userTurns({ messages: [] })).toEqual([]); }); + + test("skips null/non-object entries instead of crashing", () => { + expect( + userTurns({ messages: [null, { role: "user", content: "hi" }, 42] }), + ).toEqual(["hi"]); + }); + + test("coerces non-string content to an empty string", () => { + expect( + userTurns({ messages: [{ role: "user", content: { nested: true } }] }), + ).toEqual([""]); + }); }); diff --git a/packages/appkit/src/evals/tests/derive-concurrency.test.ts b/packages/appkit/src/evals/tests/derive-concurrency.test.ts new file mode 100644 index 000000000..488c5c17e --- /dev/null +++ b/packages/appkit/src/evals/tests/derive-concurrency.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, test } from "vitest"; + +import { deriveConcurrency } from "../run-evals"; +import type { EvalConfig } from "../types"; + +describe("deriveConcurrency", () => { + const cfg = (maxConcurrency?: number): EvalConfig => ({ maxConcurrency }); + + test("--concurrency wins over any config", () => { + const configs = new Map([ + ["a", cfg(2)], + ["b", cfg(8)], + ]); + expect(deriveConcurrency(new Set(["a", "b"]), configs, 3)).toBe(3); + }); + + test("takes the lowest ceiling among participating agents", () => { + const configs = new Map([ + ["a", cfg(1)], + ["b", cfg(8)], + ]); + expect(deriveConcurrency(new Set(["a", "b"]), configs, undefined)).toBe(1); + }); + + test("ignores configs for agents not in this run", () => { + // Agent `a`'s limit of 1 must not throttle a run that only includes `b`. + const configs = new Map([ + ["a", cfg(1)], + ["b", cfg(8)], + ]); + expect(deriveConcurrency(new Set(["b"]), configs, undefined)).toBe(8); + }); + + test("falls back to the default when no participating agent sets a limit", () => { + const configs = new Map([["a", cfg(undefined)]]); + expect(deriveConcurrency(new Set(["a"]), configs, undefined)).toBe(4); + }); +}); diff --git a/packages/appkit/src/evals/tests/report.test.ts b/packages/appkit/src/evals/tests/report.test.ts index c752e9d2f..3001de3c8 100644 --- a/packages/appkit/src/evals/tests/report.test.ts +++ b/packages/appkit/src/evals/tests/report.test.ts @@ -133,4 +133,22 @@ describe("eval reporting", () => { // Raw unescaped special chars from the id/detail must not leak through. expect(xml).not.toContain("a/b"); }); + + test("formatResultsJUnit drops XML-illegal control chars from messages", () => { + const nul = String.fromCharCode(0); + const esc = String.fromCharCode(27); // ANSI escape + const withControl: EvalResult[] = [ + { + id: `a/ctrl${nul}`, + assertions: [], + passed: false, + error: `boom${esc}[0m bang`, + }, + ]; + const xml = formatResultsJUnit(withControl); + // A raw NUL or ESC would make the document not well-formed — they must be gone. + expect(xml).not.toContain(nul); + expect(xml).not.toContain(esc); + expect(xml).toContain("boom[0m bang"); + }); }); diff --git a/packages/appkit/src/evals/tests/run-eval.test.ts b/packages/appkit/src/evals/tests/run-eval.test.ts index fdcff6269..5b6685114 100644 --- a/packages/appkit/src/evals/tests/run-eval.test.ts +++ b/packages/appkit/src/evals/tests/run-eval.test.ts @@ -132,6 +132,82 @@ describe("runEval", () => { expect(result.passed).toBe(true); }); + test("calledToolWith matches array-valued args element-for-element", async () => { + const def = defineEval({ + async test(t) { + await t.send("filter it"); + t.calledToolWith("search", { tags: ["news", "tech"] }); + }, + }); + const result = await runEval(def, { + id: "args-array", + driver: fakeDriver({ + toolCalls: ["search"], + toolCallDetails: [ + { name: "search", args: { tags: ["news", "tech"], limit: 10 } }, + ], + }), + }); + expect(result.passed).toBe(true); + }); + + test("calledToolWith fails when an array arg differs", async () => { + const def = defineEval({ + async test(t) { + await t.send("filter it"); + t.calledToolWith("search", { tags: ["news", "tech"] }); + }, + }); + const result = await runEval(def, { + id: "args-array-mismatch", + driver: fakeDriver({ + toolCalls: ["search"], + toolCallDetails: [{ name: "search", args: { tags: ["news"] } }], + }), + }); + expect(result.passed).toBe(false); + }); + + test("calledToolWith fails when an expected key is missing, even if its value is undefined", async () => { + const def = defineEval({ + async test(t) { + await t.send("go"); + t.calledToolWith("go", { mode: undefined }); + }, + }); + const result = await runEval(def, { + id: "args-missing-key", + driver: fakeDriver({ + toolCalls: ["go"], + toolCallDetails: [{ name: "go", args: { other: 1 } }], + }), + }); + expect(result.passed).toBe(false); + }); + + test("calledToolWith detail reports arg keys, never their (possibly sensitive) values", async () => { + const def = defineEval({ + async test(t) { + await t.send("go"); + t.calledToolWith("search", { q: "weather" }); + }, + }); + const result = await runEval(def, { + id: "args-redacted", + driver: fakeDriver({ + toolCalls: ["search"], + toolCallDetails: [ + { name: "search", args: { q: "sunny", token: "s3cr3t-value" } }, + ], + }), + }); + expect(result.passed).toBe(false); + const detail = result.assertions[0].detail ?? ""; + expect(detail).toContain("token"); // the arg key name is fine to report + expect(detail).not.toContain("s3cr3t-value"); // its value must not leak + expect(detail).not.toContain("sunny"); + }); + test("soft failures don't fail the eval unless strict", async () => { const def = defineEval({ async test(t) { @@ -328,4 +404,79 @@ describe("runEval", () => { }); expect(result.error).toBe("eval timed out after 15ms"); }); + + test("a failing eval whose turn broke flags infraFailure so the runner can retry it", async () => { + const def = defineEval({ + async test(t) { + await t.send("hi"); + t.succeeded(); // gate on the turn, so a broken turn fails the eval + }, + }); + const result = await runEval(def, { + id: "infra", + driver: fakeDriver({ succeeded: false }), + }); + expect(result.passed).toBe(false); + expect(result.infraFailure).toBe(true); + }); + + test("a passing eval is not flagged for retry even if a turn reported failure", async () => { + const def = defineEval({ + async test(t) { + await t.send("hi"); // no gate on success — the eval passes vacuously + }, + }); + const result = await runEval(def, { + id: "pass-infra", + driver: fakeDriver({ succeeded: false }), + }); + expect(result.passed).toBe(true); + expect(result.infraFailure).toBeUndefined(); + }); + + test("a completed turn does not flag infraFailure (assertion misses are real signal)", async () => { + const def = defineEval({ + async test(t) { + await t.send("hi"); + t.calledTool("get_weather"); // fails, but the turn itself succeeded + }, + }); + const result = await runEval(def, { + id: "assertion-fail", + driver: fakeDriver({ succeeded: true, toolCalls: [] }), + }); + expect(result.passed).toBe(false); + expect(result.infraFailure).toBeUndefined(); + }); + + test("a per-eval timeout aborts the in-flight driver turn", async () => { + let receivedSignal: AbortSignal | undefined; + const driver: EvalDriver = { + // Resolve only once the turn is aborted — mimics a stream that ends on + // cancel rather than running to the driver's own (longer) timeout. + send: async (_message, opts) => { + receivedSignal = opts?.signal; + await new Promise((resolve) => { + opts?.signal?.addEventListener("abort", () => resolve(), { + once: true, + }); + }); + return { + reply: "", + toolCalls: [], + toolCallDetails: [], + succeeded: false, + }; + }, + }; + const def = defineEval({ + timeoutMs: 20, + async test(t) { + await t.send("hi"); + }, + }); + const result = await runEval(def, { id: "abort", driver }); + expect(result.error).toBe("eval timed out after 20ms"); + expect(receivedSignal?.aborted).toBe(true); + }); }); diff --git a/packages/appkit/src/evals/tests/run-with-retries.test.ts b/packages/appkit/src/evals/tests/run-with-retries.test.ts index f4bbd1614..1f5d3159d 100644 --- a/packages/appkit/src/evals/tests/run-with-retries.test.ts +++ b/packages/appkit/src/evals/tests/run-with-retries.test.ts @@ -4,6 +4,9 @@ import { runWithRetries } from "../run-evals"; import type { EvalResult } from "../types"; describe("runWithRetries", () => { + // Disable the inter-attempt backoff so these count-based tests stay instant. + const noBackoff = { baseDelayMs: 0 }; + const errored = (n: number): EvalResult => ({ id: `try-${n}`, assertions: [], @@ -20,43 +23,106 @@ describe("runWithRetries", () => { assertions: [{ label: "check", severity: "gate", pass: false }], passed: false, }); + const infraFailed = (n: number): EvalResult => ({ + id: `try-${n}`, + assertions: [], + passed: false, + infraFailure: true, + }); test("retries an infra error up to `retries` extra times, then returns the last", async () => { let calls = 0; - const result = await runWithRetries(2, async (n) => { - calls = n; - return errored(n); - }); + const result = await runWithRetries( + 2, + async (n) => { + calls = n; + return errored(n); + }, + noBackoff, + ); expect(calls).toBe(3); // 1 initial + 2 retries expect(result.error).toBe("turn failed"); }); test("stops as soon as an attempt succeeds", async () => { let calls = 0; - const result = await runWithRetries(5, async (n) => { - calls = n; - return n < 2 ? errored(n) : ok(n); - }); + const result = await runWithRetries( + 5, + async (n) => { + calls = n; + return n < 2 ? errored(n) : ok(n); + }, + noBackoff, + ); expect(calls).toBe(2); // errored once, then ok expect(result.passed).toBe(true); }); test("never retries an assertion failure (no error set)", async () => { let calls = 0; - const result = await runWithRetries(3, async (n) => { - calls = n; - return assertionFail(n); - }); + const result = await runWithRetries( + 3, + async (n) => { + calls = n; + return assertionFail(n); + }, + noBackoff, + ); expect(calls).toBe(1); expect(result.passed).toBe(false); }); test("retries=0 runs exactly once", async () => { let calls = 0; - await runWithRetries(0, async (n) => { - calls = n; - return errored(n); - }); + await runWithRetries( + 0, + async (n) => { + calls = n; + return errored(n); + }, + noBackoff, + ); + expect(calls).toBe(1); + }); + + test("coerces a non-finite retries to 0 (runs exactly once)", async () => { + let calls = 0; + await runWithRetries( + Number.NaN, + async (n) => { + calls = n; + return infraFailed(n); + }, + noBackoff, + ); expect(calls).toBe(1); }); + + test("retries a transport/agent turn failure (infraFailure) like an error", async () => { + let calls = 0; + const result = await runWithRetries( + 2, + async (n) => { + calls = n; + return infraFailed(n); + }, + noBackoff, + ); + expect(calls).toBe(3); // 1 initial + 2 retries + expect(result.infraFailure).toBe(true); + }); + + test("stops retrying once an infra-failed turn recovers", async () => { + let calls = 0; + const result = await runWithRetries( + 5, + async (n) => { + calls = n; + return n < 2 ? infraFailed(n) : ok(n); + }, + noBackoff, + ); + expect(calls).toBe(2); + expect(result.passed).toBe(true); + }); }); diff --git a/packages/appkit/src/evals/types.ts b/packages/appkit/src/evals/types.ts index 55b4f0286..663259292 100644 --- a/packages/appkit/src/evals/types.ts +++ b/packages/appkit/src/evals/types.ts @@ -71,7 +71,15 @@ export interface DriveResult { * app's agents endpoint; future drivers (in-process) implement the same shape. */ export interface EvalDriver { - send(message: string): Promise; + /** + * Drive one turn. `options.signal`, when provided, aborts the in-flight turn: + * the runner passes its per-eval timeout signal so a timed-out eval cancels + * the request instead of leaking a live stream. + */ + send( + message: string, + options?: { signal?: AbortSignal }, + ): Promise; /** * Drop the current conversation so the next `send` starts a fresh thread. * Optional: drivers without a session concept omit it. @@ -112,7 +120,8 @@ export interface TestContext { /** * Assert a tool was called with arguments that deep-contain `expected`: every * key in `expected` must equal the actual argument (recursively for nested - * objects), so extra arguments are ignored. Gate by default. + * objects; arrays match element-for-element), so extra arguments are ignored. + * Gate by default. */ calledToolWith( name: string, @@ -172,8 +181,6 @@ export interface EvalDefinition { /** Per-directory config from `evals.config.ts` (see {@link defineEvalConfig}). */ export interface EvalConfig { - /** LLM judge config. Defaults to the agent's own serving endpoint. */ - judge?: { model?: string }; /** Max evals to run concurrently. */ maxConcurrency?: number; /** Default per-eval timeout. */ @@ -191,6 +198,12 @@ export interface EvalResult { passed: boolean; /** Set when the eval threw before completing. */ error?: string; + /** + * A turn failed at the transport/agent level (`succeeded: false`) rather than + * on an assertion — a retryable infra flake, distinct from `error` (a thrown + * error or per-eval timeout) and from an assertion mismatch (real signal). + */ + infraFailure?: boolean; /** MLflow trace id of the eval's last turn, for attaching assessments. */ traceId?: string; } diff --git a/packages/shared/src/cli/commands/agent/eval.test.ts b/packages/shared/src/cli/commands/agent/eval.test.ts new file mode 100644 index 000000000..87d43b866 --- /dev/null +++ b/packages/shared/src/cli/commands/agent/eval.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, test } from "vitest"; + +import { parsePassRate } from "./eval"; + +describe("parsePassRate", () => { + test("returns undefined when the flag is unset", () => { + expect(parsePassRate(undefined)).toBeUndefined(); + }); + + test("accepts a value in [0, 1]", () => { + expect(parsePassRate("0")).toBe(0); + expect(parsePassRate("0.8")).toBe(0.8); + expect(parsePassRate("1")).toBe(1); + }); + + test.each(["", " ", "-1", "2", "90", "abc", "0.5junk", "NaN", "Infinity"])( + "rejects blank, out-of-range, or non-numeric %j", + (raw) => { + expect(() => parsePassRate(raw)).toThrow(/min-pass-rate/); + }, + ); +}); diff --git a/packages/shared/src/cli/commands/agent/eval.ts b/packages/shared/src/cli/commands/agent/eval.ts index a638769ad..7d259a719 100644 --- a/packages/shared/src/cli/commands/agent/eval.ts +++ b/packages/shared/src/cli/commands/agent/eval.ts @@ -91,6 +91,27 @@ function parseHeaders(values: string[]): Record { return headers; } +/** + * Parse `--min-pass-rate`: a finite number in `[0, 1]`, or `undefined` when the + * flag is unset. Rejects out-of-range and partially-numeric input (`0.5junk`, + * `-1`, `2`) by throwing — `Number` (unlike `parseFloat`) rejects trailing junk + * — so a bad gate value fails fast instead of silently disabling the CI gate + * (`-1` would pass every suite) or inverting it (`2`/`90` would fail every one). + */ +export function parsePassRate(raw: string | undefined): number | undefined { + if (raw === undefined) return undefined; + const n = Number(raw); + // `Number("")` and `Number(" ")` are 0 — a valid-looking threshold that + // would silently turn an empty/unset CI value (`--min-pass-rate "$VAR"`) into + // an always-pass gate. Reject a blank value rather than treat it as 0. + if (raw.trim() === "" || !Number.isFinite(n) || n < 0 || n > 1) { + throw new Error( + `Invalid --min-pass-rate "${raw}" — expected a number in [0, 1]`, + ); + } + return n; +} + interface EvalOptions { url: string; strict?: boolean; @@ -256,6 +277,17 @@ async function runAgentEval( const retries = parsedRetries && parsedRetries > 0 ? parsedRetries : undefined; + // Validate the pass-rate gate up front: a bad value should fail before a whole + // run, not silently disable/invert the gate at the end. + let minPassRate: number | undefined; + try { + minPassRate = parsePassRate(opts.minPassRate); + } catch (err) { + console.error(err instanceof Error ? err.message : String(err)); + process.exitCode = 1; + return; + } + // In a machine reporter (json/junit), stdout is reserved for the report (it // may be piped), so human-facing lines go to stderr and the per-eval live // streaming is suppressed. Text mode keeps its current stdout behavior. @@ -316,7 +348,17 @@ async function runAgentEval( ? runner.formatResultsJson(summary.results) : runner.formatResultsJUnit(summary.results); if (opts.output) { - fs.writeFileSync(opts.output, `${report}\n`); + try { + fs.writeFileSync(opts.output, `${report}\n`); + } catch (err) { + console.error( + `Failed to write ${reporter} report to ${opts.output}: ${ + err instanceof Error ? err.message : String(err) + }`, + ); + process.exitCode = 1; + return; + } info(`Wrote ${reporter} report to ${opts.output}`); } else { process.stdout.write(`${report}\n`); @@ -324,10 +366,7 @@ async function runAgentEval( } const stats = runner.summarize(summary.results); - const minPassRate = opts.minPassRate - ? Number.parseFloat(opts.minPassRate) - : undefined; - if (minPassRate !== undefined && !Number.isNaN(minPassRate)) { + if (minPassRate !== undefined) { // Threshold mode: gate on the aggregate pass rate rather than requiring // every eval to pass. const ok = stats.passRate >= minPassRate; From b10e8abbfb0f34af7f5990a50f5bf68dddd95db0 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Wed, 9 Sep 2026 18:23:37 +0200 Subject: [PATCH 11/11] chore(appkit): trim verbose comments in eval runner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove AI-slop from the eval-runner fixes: collapse multi-line WHY comments to one load-bearing line, drop rationale duplicated between an inline comment and its function's JSDoc, and cut fix-narration. Comments and JSDoc only — no behavior change. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- packages/appkit/src/evals/dataset.ts | 4 +- packages/appkit/src/evals/http-driver.ts | 3 +- packages/appkit/src/evals/report.ts | 9 ++-- packages/appkit/src/evals/run-eval.ts | 26 +++------- packages/appkit/src/evals/run-evals.ts | 52 ++++++------------- .../appkit/src/evals/tests/run-eval.test.ts | 3 +- packages/appkit/src/evals/types.ts | 5 +- .../shared/src/cli/commands/agent/eval.ts | 15 ++---- 8 files changed, 36 insertions(+), 81 deletions(-) diff --git a/packages/appkit/src/evals/dataset.ts b/packages/appkit/src/evals/dataset.ts index 130caad3e..b6cdc0dac 100644 --- a/packages/appkit/src/evals/dataset.ts +++ b/packages/appkit/src/evals/dataset.ts @@ -35,9 +35,7 @@ export interface ReadEvalDatasetOptions { */ export function userTurns(input: Record): string[] { const messages = Array.isArray(input.messages) ? input.messages : []; - // Guard each entry: a managed dataset row is external data, so a `null` or - // non-object entry must not crash the read, and non-string content coerces to - // "" rather than violate the declared `string[]` return. + // Dataset rows are external data: skip null/non-object entries; coerce non-string content to "". return messages .filter( (m): m is { role?: unknown; content?: unknown } => diff --git a/packages/appkit/src/evals/http-driver.ts b/packages/appkit/src/evals/http-driver.ts index 565538890..167c365c4 100644 --- a/packages/appkit/src/evals/http-driver.ts +++ b/packages/appkit/src/evals/http-driver.ts @@ -166,8 +166,7 @@ export function createHttpDriver(options: HttpDriverOptions): EvalDriver { ): Promise { // Bounds connect + the entire read below. Passed to both the fetch and // the SSE reader: on expiry the reader is cancelled and the turn fails. - // Composed with the caller's signal (the runner's per-eval timeout) so a - // timed-out eval aborts this turn instead of leaking a live stream. + // Composed with the caller's signal so a per-eval timeout also aborts this turn. const timeout = AbortSignal.timeout(timeoutMs); const signal = opts?.signal ? AbortSignal.any([timeout, opts.signal]) diff --git a/packages/appkit/src/evals/report.ts b/packages/appkit/src/evals/report.ts index d40a02d01..970550b2d 100644 --- a/packages/appkit/src/evals/report.ts +++ b/packages/appkit/src/evals/report.ts @@ -89,14 +89,11 @@ export function formatResultsJson(results: EvalResult[]): string { } /** - * Drop the characters XML 1.0 forbids even when escaped — the C0 control chars - * except tab (9), LF (10), and CR (13). A raw NUL or ANSI escape from an agent - * reply or tool arg would otherwise make the JUnit document not well-formed and - * a strict CI parser reject it. + * Drop the C0 control chars XML 1.0 forbids even when escaped (except tab, LF, + * CR) — a raw NUL or ANSI escape would otherwise make the JUnit doc reject on parse. */ function stripXmlControlChars(value: string): string { - // A regex char class is terser, but oxlint's `no-control-regex` rejects it - // (rule enabled repo-wide) — so filter by code point instead of suppressing. + // Code-point filter, not a regex: oxlint `no-control-regex` rejects the class. let out = ""; for (const ch of value) { const code = ch.codePointAt(0) as number; diff --git a/packages/appkit/src/evals/run-eval.ts b/packages/appkit/src/evals/run-eval.ts index 6b58db196..7bdbfa4a6 100644 --- a/packages/appkit/src/evals/run-eval.ts +++ b/packages/appkit/src/evals/run-eval.ts @@ -25,9 +25,7 @@ class SkipSignal extends Error { /** * Deep partial match: every key in `expected` is present in `actual` and equal, * recursing into nested plain objects so extra actual keys are ignored. Arrays - * must match element-for-element (same length, deep-equal items) — an array is - * a value, not a partial shape, so reference equality would never match two - * equal arrays parsed from JSON. + * match element-for-element (same length, deep-equal items). */ function deepContains(actual: unknown, expected: unknown): boolean { if (Array.isArray(expected)) { @@ -39,8 +37,7 @@ function deepContains(actual: unknown, expected: unknown): boolean { } if (isPlainObject(expected)) { if (!isPlainObject(actual)) return false; - // Require the key to be present, so an expected `undefined` value does not - // silently match a key the actual args omit. + // Require the key present: an expected `undefined` must not match an omitted key. return Object.keys(expected).every( (key) => Object.hasOwn(actual, key) && deepContains(actual[key], expected[key]), @@ -86,11 +83,9 @@ export async function runEval( let sessionId: string | undefined; let lastTraceId: string | undefined; let lastSucceeded = false; - // Set when any turn fails at the transport/agent level (driver `succeeded: - // false`): surfaced as `infraFailure` so the runner can retry an infra flake. + // Any transport/agent turn failure (driver `succeeded: false`) → `infraFailure`, for retry. let turnFailed = false; - // Aborted when the per-eval timeout elapses, cancelling the in-flight turn so - // a timed-out eval doesn't leak a live stream past its deadline. + // Aborted on per-eval timeout to cancel the in-flight turn (no leaked stream). const controller = new AbortController(); const record = ( @@ -189,10 +184,7 @@ export async function runEval( calledToolWith(name, expected) { const matching = toolCallDetails.filter((c) => c.name === name); const pass = matching.some((c) => deepContains(c.args, expected)); - // Report only the *keys* the agent passed, never their values — actual - // args can carry PII/secrets and this detail is persisted into the - // JSON/JUnit reports and MLflow rationales (CWE-532). `expected` is - // operator-authored, so it stays. + // Keys only, never values: actual args may hold PII/secrets and are persisted to reports (CWE-532). const seen = matching.length ? matching .map((c) => `{${Object.keys(c.args).sort().join(", ")}}`) @@ -250,12 +242,8 @@ export async function runEval( if (timeoutMs === undefined) { await def.test(t); } else { - // Race the test against a timeout; on elapse we abort the in-flight driver - // turn, reject, and convert it to a non-passing result. The timer is - // cleared in `finally` so it can't keep the process alive after the test - // settles. Note: only the driver turn is cancelled — a test that hangs in - // non-driver code (a `t.judge.*` call, an in-test sleep) still runs to its - // own completion, though the eval's result is already recorded by then. + // Race the test against the timeout; on elapse, abort the turn and settle + // non-passing. Only the driver turn cancels — non-driver hangs (judge, sleep) run on. const timeout = new Promise((_, reject) => { timer = setTimeout(() => { controller.abort(); diff --git a/packages/appkit/src/evals/run-evals.ts b/packages/appkit/src/evals/run-evals.ts index 378b536a7..13818688e 100644 --- a/packages/appkit/src/evals/run-evals.ts +++ b/packages/appkit/src/evals/run-evals.ts @@ -73,12 +73,9 @@ export interface RunEvalsOptions { */ timeoutMs?: number; /** - * Re-run an eval up to this many extra times when it fails on an - * infrastructure failure — a thrown error or per-eval timeout (`result.error`) - * or a turn that failed at the transport/agent level (`result.infraFailure`, - * e.g. a failed fetch, 5xx, or dropped stream) — to absorb transient - * turn/stream flakiness. Assertion failures are NEVER retried (a wrong reply - * is real signal, not flake). Defaults to `0`. + * Re-run an eval up to this many extra times when it fails on infrastructure — + * a thrown error/timeout (`result.error`) or a transport/agent turn failure + * (`result.infraFailure`). Assertion failures are never retried. Defaults to `0`. */ retries?: number; /** Progress callback, invoked as evals are discovered, started, and finished. */ @@ -146,8 +143,7 @@ async function loadEvalConfig(file: string): Promise { */ export function resolveConfigDefault(mod: unknown): EvalConfig | undefined { let candidate: unknown = mod; - // The `i < 4` cap already bounds a self-referential `default` chain, so no - // visited-set is needed (mirrors resolveEvalDefault). + // `i < 4` bounds the chain; no visited-set needed (cf. resolveEvalDefault). for (let i = 0; i < 4 && candidate; i++) { const next = (candidate as { default?: unknown }).default; if (next === undefined) { @@ -192,12 +188,8 @@ async function runOne( options: RunEvalsOptions, ): Promise { try { - // Retry on an infrastructure failure — a thrown error or per-eval timeout - // (`result.error`), or a turn that failed at the transport/agent level - // (`result.infraFailure`, e.g. a dropped stream or 5xx) — to absorb - // transient turn/stream flakiness; assertion failures are real signal and - // returned on the first try. Each attempt gets a fresh driver, so its thread - // never carries over the failed attempt's history. + // Each attempt builds a fresh driver, so a retry never inherits the failed + // attempt's thread. (runWithRetries defines what counts as retryable.) return await runWithRetries(options.retries ?? 0, () => runEval(def, { id, @@ -206,9 +198,7 @@ async function runOne( agent: def.agent ?? d.agent, headers: options.headers, mlflowRunId: runId, - // Match the driver's per-turn cap to the eval's effective timeout so a - // `def.timeoutMs` shorter than the driver default can't leave the turn - // running past the deadline (`runEval` also aborts it via its signal). + // Cap the driver turn at the eval's effective timeout (runEval's signal also aborts it). timeoutMs: def.timeoutMs ?? options.timeoutMs, }), strict: options.strict, @@ -286,8 +276,7 @@ async function runDiscovered( ): Promise { const id = `${d.agent}/${d.id}`; - // The file failed to load in the pre-pass (its `def` is an unused - // placeholder) — surface that as one non-passing result. + // Load failed in the pre-pass (def is a placeholder) → one non-passing result. if (loadError) { emit({ type: "start", id, index, total }); const result: EvalResult = { @@ -329,11 +318,9 @@ const MAX_RETRY_DELAY_MS = 5_000; * neither, so a failed-but-completed eval is returned on the first try and * never retried. Returns the last result when every attempt failed on infra. * - * Between attempts it waits a full-jittered exponential backoff — the targeted - * infra flakes (connection refused, the app's 429 stream-cap) are - * overload-correlated, so retrying instantly would amplify load. `retries` is - * coerced to a finite, non-negative integer, so a `NaN`/`Infinity` from direct - * API misuse can't loop forever. `baseDelayMs: 0` disables the wait (tests). + * Between attempts it waits a full-jittered exponential backoff (infra flakes + * are overload-correlated). `retries` is coerced to a finite non-negative + * integer; `baseDelayMs: 0` disables the wait (tests). */ export async function runWithRetries( retries: number, @@ -423,13 +410,10 @@ async function finalizeMlflow( const DEFAULT_CONCURRENCY = 4; /** - * Resolve the work-pool width: `--concurrency` wins; otherwise the lowest - * `maxConcurrency` any *participating* agent's `evals.config.ts` requests - * (every eval drives the app as the same user and shares one per-user stream - * budget, so the most conservative ceiling governs — a `Math.max` would let one - * lax agent raise another's limit past its intent and the server's stream cap); - * an agent with no eval in this run doesn't constrain it. Falls back to - * {@link DEFAULT_CONCURRENCY}. + * Resolve the work-pool width: `--concurrency` wins; else the lowest + * `maxConcurrency` any *participating* agent's `evals.config.ts` requests (all + * evals share one per-user stream budget, so the most conservative ceiling + * governs); else {@link DEFAULT_CONCURRENCY}. */ export function deriveConcurrency( activeAgents: Set, @@ -507,11 +491,7 @@ export async function runEvalsInDir( loaded.push({ d, def }); } - // `evals.config.ts` `maxConcurrency` governs the single shared work pool (all - // evals run as the same user against one per-user stream budget). Take the - // lowest ceiling any *participating* agent requests so a filtered-out or laxer - // agent can't raise another's limit past its intent — see - // {@link deriveConcurrency}. + // Pool width from participating agents' configs (see {@link deriveConcurrency}). const activeAgents = new Set(loaded.map((l) => l.d.agent)); const concurrency = deriveConcurrency( activeAgents, diff --git a/packages/appkit/src/evals/tests/run-eval.test.ts b/packages/appkit/src/evals/tests/run-eval.test.ts index 5b6685114..66c32ba64 100644 --- a/packages/appkit/src/evals/tests/run-eval.test.ts +++ b/packages/appkit/src/evals/tests/run-eval.test.ts @@ -452,8 +452,7 @@ describe("runEval", () => { test("a per-eval timeout aborts the in-flight driver turn", async () => { let receivedSignal: AbortSignal | undefined; const driver: EvalDriver = { - // Resolve only once the turn is aborted — mimics a stream that ends on - // cancel rather than running to the driver's own (longer) timeout. + // Resolve only when aborted — mimics a stream that ends on cancel. send: async (_message, opts) => { receivedSignal = opts?.signal; await new Promise((resolve) => { diff --git a/packages/appkit/src/evals/types.ts b/packages/appkit/src/evals/types.ts index 663259292..ee9ea96a8 100644 --- a/packages/appkit/src/evals/types.ts +++ b/packages/appkit/src/evals/types.ts @@ -199,9 +199,8 @@ export interface EvalResult { /** Set when the eval threw before completing. */ error?: string; /** - * A turn failed at the transport/agent level (`succeeded: false`) rather than - * on an assertion — a retryable infra flake, distinct from `error` (a thrown - * error or per-eval timeout) and from an assertion mismatch (real signal). + * A turn failed at the transport/agent level (`succeeded: false`), not on an + * assertion — a retryable infra flake, distinct from `error`. */ infraFailure?: boolean; /** MLflow trace id of the eval's last turn, for attaching assessments. */ diff --git a/packages/shared/src/cli/commands/agent/eval.ts b/packages/shared/src/cli/commands/agent/eval.ts index 7d259a719..50a389f41 100644 --- a/packages/shared/src/cli/commands/agent/eval.ts +++ b/packages/shared/src/cli/commands/agent/eval.ts @@ -92,18 +92,14 @@ function parseHeaders(values: string[]): Record { } /** - * Parse `--min-pass-rate`: a finite number in `[0, 1]`, or `undefined` when the - * flag is unset. Rejects out-of-range and partially-numeric input (`0.5junk`, - * `-1`, `2`) by throwing — `Number` (unlike `parseFloat`) rejects trailing junk - * — so a bad gate value fails fast instead of silently disabling the CI gate - * (`-1` would pass every suite) or inverting it (`2`/`90` would fail every one). + * Parse `--min-pass-rate`: a finite number in `[0, 1]`, or `undefined` when + * unset. Throws on blank, out-of-range, or non-numeric input so a bad gate + * value fails fast instead of silently disabling or inverting the CI gate. */ export function parsePassRate(raw: string | undefined): number | undefined { if (raw === undefined) return undefined; const n = Number(raw); - // `Number("")` and `Number(" ")` are 0 — a valid-looking threshold that - // would silently turn an empty/unset CI value (`--min-pass-rate "$VAR"`) into - // an always-pass gate. Reject a blank value rather than treat it as 0. + // Reject blank too: `Number("")` is 0, which would silently disable the gate. if (raw.trim() === "" || !Number.isFinite(n) || n < 0 || n > 1) { throw new Error( `Invalid --min-pass-rate "${raw}" — expected a number in [0, 1]`, @@ -277,8 +273,7 @@ async function runAgentEval( const retries = parsedRetries && parsedRetries > 0 ? parsedRetries : undefined; - // Validate the pass-rate gate up front: a bad value should fail before a whole - // run, not silently disable/invert the gate at the end. + // Validate up front so a bad gate value fails before the run, not after. let minPassRate: number | undefined; try { minPassRate = parsePassRate(opts.minPassRate);