diff --git a/.changeset/remote-eval-trial-upsert-ids.md b/.changeset/remote-eval-trial-upsert-ids.md new file mode 100644 index 000000000..eb1126221 --- /dev/null +++ b/.changeset/remote-eval-trial-upsert-ids.md @@ -0,0 +1,5 @@ +--- +"braintrust": patch +--- + +Prevent evaluation trials from overwriting each other when an upsert ID is supplied, while preserving stable per-trial IDs across reruns. diff --git a/js/src/framework.test.ts b/js/src/framework.test.ts index 095a2d28b..58624e4ea 100644 --- a/js/src/framework.test.ts +++ b/js/src/framework.test.ts @@ -7,6 +7,7 @@ import { afterEach, vi, } from "vitest"; +import { type ExperimentEvent } from "../util"; import { defaultErrorScoreHandler, Eval, @@ -21,6 +22,7 @@ import { initLogger, injectTraceContext, TestBackgroundLogger, + withParent, } from "./logger"; import { parseBaggage } from "./propagation"; import { configureNode } from "./node/config"; @@ -795,6 +797,95 @@ test("trialIndex is passed to task", async () => { }); }); +describe.each([false, true])( + "trial upsert IDs with parent context: %s", + (useParent) => { + test.each( + ["eval-row", undefined, ""].flatMap((upsertId) => + [ + { trialCount: 1, rowTrialCount: undefined }, + { trialCount: 3, rowTrialCount: undefined }, + { trialCount: 1, rowTrialCount: 3 }, + { trialCount: 3, rowTrialCount: 1 }, + ].map((counts) => ({ upsertId, ...counts })), + ), + )( + "preserves separate trial rows across reruns: %j", + async ({ upsertId, trialCount, rowTrialCount }) => { + await _exportsForTestingOnly.simulateLoginForTests(); + const memoryLogger = _exportsForTestingOnly.useTestBackgroundLogger(); + const experiment = + _exportsForTestingOnly.initTestExperiment("trial-upsert"); + const parent = await experiment.export(); + const count = rowTrialCount ?? trialCount; + let previousIds: string[] = []; + + for (const input of [1, 2]) { + await withParent(parent, () => + runEvaluator( + useParent ? null : experiment, + { + projectName: "proj", + evalName: "trial-upsert", + state: experiment.loggingState, + data: [ + { input, upsert_id: upsertId, trialCount: rowTrialCount }, + ], + task: (value, { trialIndex }) => value * 10 + trialIndex, + scores: [], + trialCount, + summarizeScores: false, + }, + new NoopProgressReporter(), + [], + undefined, + undefined, + true, + ), + ); + + await memoryLogger.flush(); + const spans = (await memoryLogger.drain()).filter( + (log): log is ExperimentEvent => + "experiment_id" in log && "span_id" in log, + ); + const roots = spans + .filter((span) => !span.span_parents?.length) + .sort((a, b) => Number(a.output) - Number(b.output)); + expect(roots).toHaveLength(count); + expect(spans).toHaveLength(count * 2); + for (const [trialIndex, root] of roots.entries()) { + expect(root.output).toBe(input * 10 + trialIndex); + const children = spans.filter( + (span) => span.span_parents?.[0] === root.span_id, + ); + expect(children).toHaveLength(1); + expect(children[0].output).toEqual(root.output); + } + + const ids = roots.map((root) => root.id); + expect(new Set(ids).size).toBe(count); + if (upsertId) { + expect(ids).toEqual( + [ + upsertId, + "0a452074-8534-5d0a-a418-8dc075187dd6", + "1df30387-03bd-5d6a-a651-d80f0b576e6e", + ].slice(0, count), + ); + if (previousIds.length) { + expect(ids).toEqual(previousIds); + } + } else { + expect(ids.some((id) => previousIds.includes(id))).toBe(false); + } + previousIds = ids; + } + }, + ); + }, +); + test("trialIndex with multiple inputs", async () => { const trialData: Array<{ input: number; trialIndex: number }> = []; diff --git a/js/src/framework.ts b/js/src/framework.ts index bb59b02dd..fa18da27c 100644 --- a/js/src/framework.ts +++ b/js/src/framework.ts @@ -16,6 +16,7 @@ import { type SSEProgressEventDataType as SSEProgressEventData, } from "./generated_types"; import { queue } from "async"; +import { v5 as uuidv5 } from "uuid"; import iso from "./isomorph"; import { debugLogger } from "./debug-logger"; @@ -1328,6 +1329,13 @@ async function runEvaluatorInternal( const origin = inlineDatasetOrigin ?? (parsedDatumOrigin?.success ? parsedDatumOrigin.data : undefined); + const upsertId = + datum.upsert_id && trialIndex > 0 + ? uuidv5( + `braintrust:eval:${datum.upsert_id}:trial:${trialIndex}`, + uuidv5.URL, + ) + : datum.upsert_id; const baseEvent: StartSpanArgs = { name: "eval", @@ -1339,7 +1347,7 @@ async function runEvaluatorInternal( expected: "expected" in datum ? datum.expected : undefined, tags: datum.tags, origin, - ...(datum.upsert_id ? { id: datum.upsert_id } : {}), + ...(upsertId ? { id: upsertId } : {}), }, };