From fdfa921bbb4f87b3d9b23f30ee8bda38a9e250b1 Mon Sep 17 00:00:00 2001 From: Koosha Paridehpour <42529354+KooshaPari@users.noreply.github.com> Date: Mon, 14 Sep 2026 19:18:20 -0700 Subject: [PATCH] fix(backend): eval runner: failed calls no longer score as passed + dotAll regex (#13413) Eval runner: a case whose upstream call failed can no longer score as passed when the grading regex happens to match the error text (#13137), and regex grading compiles with dotAll so `.` spans newlines in multi-line answers, for both string and `RegExp` patterns (#13138). Chosen over #13542 / #13530, which each covered half. Validated in one consolidated batch of this series (37 PRs boarded together on `release/v3.8.51`): `typecheck:core`, `check:open-sse-typecheck` and `check:dashboard-typecheck` clean; ESLint clean on every changed file; file-size, complexity, cognitive-complexity, changelog-integrity, docs-counts, docs-sync and migration-numbering gates green (only the pre-existing `open-sse/utils/stream.ts` file-size red remains, inherited from the base); 3,743 focused `node:test` cases plus 34 vitest cases green. Thanks @KooshaPari! --- src/lib/evals/evalRunner.ts | 22 ++- .../unit/eval-runner-bugs-13137-13138.test.ts | 129 ++++++++++++++++++ 2 files changed, 149 insertions(+), 2 deletions(-) create mode 100644 tests/unit/eval-runner-bugs-13137-13138.test.ts diff --git a/src/lib/evals/evalRunner.ts b/src/lib/evals/evalRunner.ts index d0b9280013..c53adc8e6a 100644 --- a/src/lib/evals/evalRunner.ts +++ b/src/lib/evals/evalRunner.ts @@ -155,8 +155,18 @@ export function evaluateCase(evalCase: any, actualOutput: string) { } const regex = expectedValue instanceof RegExp - ? new RegExp(expectedValue.source, expectedValue.flags.replace(/[gy]/g, "")) - : new RegExp(expectedValue); + ? new RegExp( + expectedValue.source, + // #13138 — Preserve the dotAll (s) flag so "." matches newlines + // in multi-line LLM answers. Strip only g (global) and y (sticky) + // which are inappropriate for a test() call. + expectedValue.flags.includes("s") + ? expectedValue.flags.replace(/[gy]/g, "") + : `s${expectedValue.flags.replace(/[gy]/g, "")}` + ) + : // #13138 — Compile string patterns with dotAll so "." matches + // newlines in multi-line LLM answers. + new RegExp(expectedValue, "s"); if (regex.source.length > 512) { passed = false; details.error = "Regex pattern too large for safe evaluation."; @@ -243,6 +253,14 @@ export function runSuite( result.error = metrics.error; } + // #13137 — A failed upstream call must never score as passed. + // executeEvalCase() returns the error text as output, and the grading + // regex can accidentally match it (e.g. /error/i on "[ERROR] ..."). + // Force the result to failed so the pass-rate dashboard stays accurate. + if (metrics?.error) { + result.passed = false; + } + return result; }); diff --git a/tests/unit/eval-runner-bugs-13137-13138.test.ts b/tests/unit/eval-runner-bugs-13137-13138.test.ts new file mode 100644 index 0000000000..4d4b4e6d6b --- /dev/null +++ b/tests/unit/eval-runner-bugs-13137-13138.test.ts @@ -0,0 +1,129 @@ +// #13137 — Eval cases whose upstream call failed must never score as passed. +// #13138 — Regex grading must compile patterns with dotAll so "." matches +// newlines in multi-line LLM answers. + +import test from "node:test"; +import assert from "node:assert/strict"; + +import { evaluateCase, runSuite } from "../../src/lib/evals/evalRunner.ts"; + +const serial = { concurrency: false }; + +// ── #13138: dotAll regex tests ────────────────────────────────────────── + +test("#13138 — regex from string matches across newlines", serial, () => { + const evalCase = { + id: "dotall-1", + name: "Multi-line regex match", + expected: { strategy: "regex", value: "1.*2.*3.*4.*5" }, + }; + const output = "1\n2\n3\n4\n5"; + const result = evaluateCase(evalCase, output); + assert.equal(result.passed, true, "dotAll regex should match across newlines"); +}); + +test("#13138 — regex from string with SQL pattern matches multiline", serial, () => { + const evalCase = { + id: "dotall-2", + name: "SQL regex", + expected: { strategy: "regex", value: "SELECT.*FROM.*WHERE" }, + }; + const output = "```sql\nSELECT *\nFROM users\nWHERE age > 25```"; + const result = evaluateCase(evalCase, output); + assert.equal(result.passed, true, "SQL regex should match multiline SQL"); +}); + +test("#13138 — regex from string with numbered list format", serial, () => { + const evalCase = { + id: "dotall-3", + name: "Numbered list", + expected: { strategy: "regex", value: "1\\..*2\\..*3\\..*4\\..*5\\." }, + }; + const output = "1. Mercury\n2. Venus\n3. Earth\n4. Mars\n5. Jupiter"; + const result = evaluateCase(evalCase, output); + assert.equal(result.passed, true, "numbered list regex should match across lines"); +}); + +test("#13138 — RegExp object preserves dotAll flag", serial, () => { + const evalCase = { + id: "dotall-4", + name: "RegExp with dotAll", + expected: { strategy: "regex", value: /1.*2.*3/s }, + }; + const output = "1\n2\n3"; + const result = evaluateCase(evalCase, output); + assert.equal(result.passed, true, "RegExp with dotAll should match across newlines"); +}); + +test("#13138 — RegExp object without dotAll gets it added", serial, () => { + const evalCase = { + id: "dotall-5", + name: "RegExp without dotAll", + expected: { strategy: "regex", value: /hello.*world/ }, + }; + const output = "hello\nworld"; + const result = evaluateCase(evalCase, output); + assert.equal(result.passed, true, "RegExp without dotAll should still get it added"); +}); + +test("#13138 — dotAll does not break single-line matching", serial, () => { + const evalCase = { + id: "dotall-6", + name: "Single-line still works", + expected: { strategy: "regex", value: "foo.*bar" }, + }; + const output = "foo bar baz"; + const result = evaluateCase(evalCase, output); + assert.equal(result.passed, true, "single-line regex should still work"); +}); + +// ── #13137: failed-upstream-must-not-pass tests ───────────────────────── + +test("#13137 — runSuite forces passed=false when caseMetrics has an error", serial, () => { + const suiteId = "golden-set"; + const outputs: Record = { + "gs-01": "[ERROR] 503 Service Unavailable", + }; + const caseMetrics = { + "gs-01": { error: "503 Service Unavailable", durationMs: 1000 }, + }; + + const result = runSuite(suiteId, outputs, caseMetrics); + const gs01 = result.results.find((r) => r.caseId === "gs-01"); + assert.ok(gs01, "gs-01 result should exist"); + assert.equal(gs01.passed, false, "case with upstream error must not pass"); + assert.equal(gs01.error, "503 Service Unavailable"); +}); + +test("#13137 — runSuite passes when no error in metrics", serial, () => { + const suiteId = "golden-set"; + const outputs: Record = { + "gs-01": "Hello, world!", + }; + const caseMetrics = { + "gs-01": { durationMs: 500 }, + }; + + const result = runSuite(suiteId, outputs, caseMetrics); + const gs01 = result.results.find((r) => r.caseId === "gs-01"); + assert.ok(gs01, "gs-01 result should exist"); + // gs-01 in golden-set uses "exact" strategy — "Hello, world!" should match + assert.equal(gs01.passed, true, "case without error should pass if output matches"); +}); + +test("#13137 — runSuite fails even if error text matches the expected pattern", serial, () => { + const suiteId = "golden-set"; + // gs-01 exact strategy expects "Hello, world!" — an error message that + // coincidentally contains this string should still be forced to fail. + const outputs: Record = { + "gs-01": "[ERROR] Hello, world! (from upstream)", + }; + const caseMetrics = { + "gs-01": { error: "Hello, world! (from upstream)", durationMs: 500 }, + }; + + const result = runSuite(suiteId, outputs, caseMetrics); + const gs01 = result.results.find((r) => r.caseId === "gs-01"); + assert.ok(gs01, "gs-01 result should exist"); + assert.equal(gs01.passed, false, "error text matching expected pattern must still fail"); +});