Skip to content

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

Open
paultancre-bt wants to merge 1 commit into
mainfrom
fix/remote-eval-trial-upsert-ids
Open

fix: prevent eval trial upsert collisions (COR-84)#2476
paultancre-bt wants to merge 1 commit into
mainfrom
fix/remote-eval-trial-upsert-ids

Conversation

@paultancre-bt

@paultancre-bt paultancre-bt commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Why

Related to COR-84. Addresses the JS counterpart of the P1 raised on the Python SDK fix.

The intent is to keep one independently updatable result per evaluation trial. The JS SDK already preserves upsert_id, but currently reuses that exact record ID for every trial. With multiple trials, their root records overwrite each other, losing trial results and potentially leaving task spans attached to the wrong surviving root. Generating fresh random IDs would avoid collisions but break rerun upserts.

This fixes an existing JS multi-trial bug, not Python's original dropped-upsert_id bug. It matches the per-trial ID scheme in braintrust-sdk-python#763.

Repro

  1. Run an evaluator with a row containing upsert_id: "eval-row" and trialCount: 3, with a task returning a different output for each trial index.
  2. Inspect the logged records after row merging: only one root record remains instead of three because all trials use id: "eval-row".
  3. Rerun with the same upsert ID: the trials collide again instead of each updating their own result.

The regression reproduces this for both experiment-backed and remote-parent execution, and with global or per-row trial counts. Before the fix, four multi-trial cases fail with one logged root instead of three; the other 20 cases pass. Two remote eval columns are not required to trigger this P1.

Fix

  • Keep the original upsert_id for trial zero, preserving single-trial behavior.
  • For later trials, derive a deterministic UUID v5 in the URL namespace from braintrust:eval:<upsert_id>:trial:<trial_index>. Each trial has a distinct ID that stays stable on rerun, matching Python.
  • Leave missing/empty upsert IDs on the existing fresh-ID path. Do not change dataset origins, trial scheduling, or public APIs; reuse the existing UUID dependency.
  • Include the required patch changeset for braintrust.

The behavior change is limited to additional trials with a non-empty upsert ID. This prevents future trial collisions after upgrading; it does not restore overwritten results or clean up historical records.

Test

  • pnpm test src/framework.test.ts --reporter=dot: all 104 tests pass, including 24 regression cases covering supplied/missing/empty IDs, global/per-row trial counts, and experiment/parent-context execution.
  • Regression assertions check separate roots, matching root/task outputs, stable IDs across reruns, and UUID values matching Python's implementation.
  • pnpm run check:typings: passes for production and test code.
  • Targeted ESLint: no errors; existing warnings remain and the repository config excludes test files.
  • pnpm run fix:formatting and git diff --check: pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant