fix: prevent eval trial upsert collisions (COR-84)

.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.

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 }> = [];
 

js/src/framework.ts

@@ -16,6 +16,7 @@
   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";
@@ -500,7 +501,7 @@
 
 export async function _internalInitEvaluatorExperiment(
   projectName: string,
   evaluator: Evaluator<any, any, any, any, any>,
   data: EvalData<any, any, any>,
   options: {
     disabled?: boolean;
@@ -1328,6 +1329,13 @@
         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 @@
             expected: "expected" in datum ? datum.expected : undefined,
             tags: datum.tags,
             origin,
-            ...(datum.upsert_id ? { id: datum.upsert_id } : {}),
+            ...(upsertId ? { id: upsertId } : {}),
           },
         };