From 36493a6270d393f541ea7dbffe11a97513b532dc Mon Sep 17 00:00:00 2001 From: Wu Shuwen Date: Fri, 18 Sep 2026 22:30:35 +0800 Subject: [PATCH] fix(evals): fail a case whose model call errored instead of scoring it passed (#13201) * fix(evals): fail a case whose model call errored instead of scoring it passed runSuite() attached caseMetrics[id].error to the graded result but never forced `passed` to false. executeEvalCase() returns a failed call as an ordinary output string ("[ERROR] "), so any expected pattern that happened to match that text was recorded as a pass. That inflates the reported pass rate, and reports a non-zero score for a run in which no model was ever reached. Built-in codex-comparison case codex-07 reproduces it: its pattern is "try|catch|throw|error|Error" and the provider-resolution failure text ends with "...added as a combo entry.", so the `try` alternative matches and the case is scored as passed while carrying a non-empty error. A case that never reached a model has no measured behaviour to grade, so a failure is the only honest score. Refs #13137 * docs(changelog): add fragment for the errored-eval-case fix (#13201) --- .../13201-eval-errored-case-scored-passed.md | 1 + src/lib/evals/evalRunner.ts | 11 ++- ...-runner-errored-case-scored-passed.test.ts | 87 +++++++++++++++++++ 3 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 changelog.d/fixes/13201-eval-errored-case-scored-passed.md create mode 100644 tests/unit/eval-runner-errored-case-scored-passed.test.ts diff --git a/changelog.d/fixes/13201-eval-errored-case-scored-passed.md b/changelog.d/fixes/13201-eval-errored-case-scored-passed.md new file mode 100644 index 0000000000..2263a47e3b --- /dev/null +++ b/changelog.d/fixes/13201-eval-errored-case-scored-passed.md @@ -0,0 +1 @@ +- **fix(evals):** an eval case whose model call errored is no longer scored as passed — a case that never reached a model has no measured behaviour to grade ([#13201](https://github.com/diegosouzapw/OmniRoute/pull/13201)) — thanks @aaustinhuang diff --git a/src/lib/evals/evalRunner.ts b/src/lib/evals/evalRunner.ts index c53adc8e6a..5d06c100e9 100644 --- a/src/lib/evals/evalRunner.ts +++ b/src/lib/evals/evalRunner.ts @@ -249,8 +249,15 @@ export function runSuite( result.durationMs = Math.max(0, Math.round(Number(metrics.durationMs))); } - if (metrics?.error && !result.error) { - result.error = metrics.error; + if (metrics?.error) { + // A case whose call errored never reached a model, so there is no measured + // behaviour to grade. The runner surfaces failures as an ordinary output string + // ("[ERROR] …" — see executeEvalCase), so leaving `passed` alone lets a pattern + // that happens to match that text score a pass and inflate the reported rate. + result.passed = false; + if (!result.error) { + result.error = metrics.error; + } } // #13137 — A failed upstream call must never score as passed. diff --git a/tests/unit/eval-runner-errored-case-scored-passed.test.ts b/tests/unit/eval-runner-errored-case-scored-passed.test.ts new file mode 100644 index 0000000000..683046922f --- /dev/null +++ b/tests/unit/eval-runner-errored-case-scored-passed.test.ts @@ -0,0 +1,87 @@ +/** + * Regression: an eval case whose upstream call failed must never be scored as passed. + * + * Issue #13137 — `runSuite()` attaches `caseMetrics[id].error` to the graded result but + * never forces `passed` to `false`. Because `executeEvalCase()` (src/lib/evals/runtime.ts) + * returns the failure as an ordinary output string (`[ERROR] ${error}`), any expected + * pattern that happens to match that text scores a pass — so a run in which no model was + * ever reached still reports a non-zero pass rate. + */ + +import { describe, it, after } from "node:test"; +import assert from "node:assert/strict"; + +import { registerSuite, runSuite, resetSuites } from "../../src/lib/evals/evalRunner.ts"; + +describe("evalRunner — errored case scoring (#13137)", () => { + after(() => { + // Remove test-registered suites while preserving built-ins. + resetSuites(); + }); + + it("fails a case whose call errored even when the error text matches the pattern", () => { + registerSuite({ + id: "test-errored-case", + name: "Errored case", + cases: [ + { + id: "errored-01", + name: "Error handling pattern", + model: "codex", + input: { messages: [{ role: "user", content: "Add error handling" }] }, + // Mirrors built-in `codex-comparison` case codex-07, whose pattern matches the + // runner's own "[ERROR] ..." string when the upstream call never resolves. + expected: { strategy: "regex", value: "try|catch|throw|error|Error" }, + }, + ], + }); + + // Exact production text (open-sse/services/model.ts:851), wrapped by + // executeEvalCase as `[ERROR] ${error}` (src/lib/evals/runtime.ts:232). + // It contains "entry", so the pattern's `try` alternative matches it. + const failure = + "Unable to determine provider for model 'codex'. Use a provider/model prefix " + + "(e.g. openai/codex) or ensure the model is added as a combo entry."; + const run = runSuite( + "test-errored-case", + { "errored-01": `[ERROR] ${failure}` }, + { "errored-01": { durationMs: 3, error: failure } } + ); + + assert.equal(run.results[0].error, failure); + assert.equal( + run.results[0].passed, + false, + "a case that never reached a model has no measured behaviour to grade" + ); + assert.equal(run.summary.passed, 0); + assert.equal(run.summary.failed, 1); + assert.equal(run.summary.passRate, 0); + }); + + it("still scores a matching case as passed when the call succeeded", () => { + registerSuite({ + id: "test-healthy-case", + name: "Healthy case", + cases: [ + { + id: "healthy-01", + name: "Error handling pattern", + model: "codex", + input: { messages: [{ role: "user", content: "Add error handling" }] }, + expected: { strategy: "regex", value: "try|catch|throw|error|Error" }, + }, + ], + }); + + const run = runSuite( + "test-healthy-case", + { "healthy-01": "Wrap the read in a try/catch block." }, + { "healthy-01": { durationMs: 120 } } + ); + + assert.equal(run.results[0].passed, true); + assert.equal(run.results[0].error, undefined); + assert.equal(run.summary.passRate, 100); + }); +});