Skip to content

[AIC-3363] Add support for evaluation data to be defined in code - #127

Open
aknight-ld wants to merge 1 commit into
mainfrom
AIC-3363-support-inline-datasets
Open

aknight-ld wants to merge 1 commit into
mainfrom
AIC-3363-support-inline-datasets

Conversation

@aknight-ld

@aknight-ld aknight-ld commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Adjusted approach for inline dataset definition. For users who wish to define their evaluation data directly from code, we now offer an alternate path where they provide the data directly by dropping the dataset key and instead providing rows.

Sample shape:

result = await evals.run(
    project_key="my-project",
    key="support-qa-2026-08-20",
    rows=[
        {"input": "How do I reset my password?", "expectedOutput": "Use the reset link."},
        {"input": "Where is order {{order_id}}?", "variables": {"order_id": "A-17"}},
    ],
    handler=create_openai_messages_handler(),
    generation={"provider": "OpenAI", "model": "gpt-4o"},
)

Note

Overview
Adds code-defined evaluation datasets via a new rows argument on evals.run(), mutually exclusive with the existing dataset key. Rows can be DatasetRow instances or wire-shaped mappings (input, expectedOutput, variables, metadata); list position defines row index, with validation failing before any LaunchDarkly records are created.

For inline runs, the harness creates a run without a hosted datasetId, uploads unrendered rows to …/runs/{id}/dataset-rows in batches of up to 500 (before generation/events, to avoid premature run completion), then renders {{…}} templates the same way as hosted datasets via shared render_row. Generation and criterion events omit datasetId / datasetKey when no stored dataset is attached.

The management API client gains an idempotent POST option so dataset-row uploads retry safely on 5xx or transport errors. InlineDatasetRow is exported; README documents the inline path. Hosted-dataset behavior and event identities are covered by new tests.

Reviewed by Cursor Bugbot for commit 196d0b1. Bugbot is set up for automated code reviews on this repo. Configure here.

@jeffdupont jeffdupont left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed with the 1.0 freeze in mind. Inline rows are a useful addition, and since evals are additive and Python-only for 1.0 (the GA plan defers JS evals to 1.1), the lack of a JS counterpart isn't a blocker. JS has no eval code on main to add this to. make test passes at 196d0b1 (1418 passed, 11 skipped, exit 0) and make typecheck is clean.

Three things I'd like settled before merge:

  1. No spec, and the PR goes against the one on main. TESTING.md §8.5 says the run create body is "exactly {source: \"api\", datasetId: <dataset id>}", and §8.6 lists datasetId as a run identity field on every event that an event type "must never drop". This PR drops it from the run body, the identity set, the eventId hash and the payloads, and adds a new request (POST …/runs/{id}/dataset-rows). The spec for the earlier approach, launchdarkly/ai-sdks-monorepo#15, was closed with #95, and I didn't find a replacement. Event payloads are a data contract with ingest, and JS will build against the spec in 1.1. So I think this needs a spec PR covering the upload request, batch size, retry rule, event identity without a dataset, and row validation before it merges.
  2. A bad row can still fail after the records exist. The README says "a malformed row fails the run before any records are created", but values inside variables / metadata are never checked for JSON. I reproduced both cases at 196d0b1. A datetime in variables raises a bare TypeError from json.dumps, after POST evaluations and POST runs have both gone out. A math.nan in metadata goes out as "metadata": {"score": NaN}, which isn't valid JSON. §8.4 already requires this check, done up front, for inline tool schemas, for the same reason. #95 had it.
  3. A failed upload leaves a run that never finishes. The upload happens after the run is created, one batch at a time. With 600 rows and a 400 on the second batch (reproduced), rows 0-499 are stored, the call raises, and the run stays open with a placeholder row count, so nothing will ever complete it. Should the SDK mark the run failed, or should the server time it out? Whichever it is should go in the spec. Retrying the batch POST on 5xx relies on the server applying each rowIdx at most once. I haven't checked that in the server, and the spec should say it too, because §8.3 currently says POSTs are never retried on 5xx.

Smaller notes:

  • InlineDatasetRow is a new root export. It's only a type alias (DatasetRow | Mapping[str, Any]), so callers don't need it to use rows=. If evals move to experimental.evaluations as the lifecycle draft proposes, it should go with them. Either way I'd keep it out of the root __all__.
  • Passing a DatasetRow means writing row_index= by hand, and it has to equal the row's position or the call raises. DatasetRow is also documented as "a rendered dataset row" for the handler. Reusing it as the input type freezes both roles together. A mapping-only input, or row_index made optional for inline rows, would be easier to change later.
  • #95 also capped the row count (10k) and the rendered byte size. Does the server enforce limits on dataset-rows? If it does, checking them up front avoids the partial-upload case above. I haven't checked the server.
  • The DatasetRef docstring says "the server drops any that name one as a mismatch". I haven't verified that. If it's true, it belongs in the spec, because it's the reason events can't carry a dataset id.

raise EvaluationsError(
f"Inline dataset row {position} {field_name} must be a string"
)
for field_name in ("variables", "metadata"):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This checks that variables / metadata are mappings, but not what's inside them. Reproduced at 196d0b1: variables={"when": datetime(2026, 1, 1)} raises a bare TypeError from json.dumps in api.py:154, after POST evaluations and POST runs. metadata={"score": math.nan} is sent as NaN, which isn't valid JSON. A strict json.dumps(..., allow_nan=False) per row here would keep the README's "fails before any records are created" promise true. §8.4 asks for the same check on inline tool schemas.

# placeholder row count of 1, so a result counted before the rows
# land would mark the run complete.
await asyncio.to_thread(
self._runner._upload_dataset_rows,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If any batch fails here, the run already exists and has some rows. Reproduced with 600 rows and a 400 on the second batch: 500 rows stored, EvaluationsError raised, and nothing ever completes the run. Should this mark the run failed before re-raising, or does the server expire runs that never get events? Either way the spec should say which.

"datasetId": dataset_id,
}
body: dict[str, Any] = {"source": "api"}
if dataset_id is not None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TESTING.md §8.5 on main says this body is "exactly {source: \"api\", datasetId: <dataset id>}". The inline-dataset spec, ai-sdks-monorepo#15, was closed along with #95, so nothing describes the no-dataset run, the dataset-rows request or its batch size. That needs a spec PR before this freezes, so JS has something to build against in 1.1.

]
}
try:
self._api.post(path, body=body, idempotent=True)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retrying this POST on 5xx and on timeouts is safe only if the server applies each rowIdx at most once and a replay doesn't add rows or fail with a conflict. I haven't checked that in the server. §8.3 says POSTs are never retried on 5xx, so idempotent=True needs a spec line saying which endpoints it covers.

"rowIndex": result["row_index"],
}
if dataset.id is None:
del identity["datasetId"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes the event identity set, and so the eventId hash, for inline runs. §8.6 says an event type may extend the identity set but "must never drop a field from it". I think the change is reasonable if ingest really rejects a dataset id it doesn't know, but it's a data contract change and should be in the spec. The same applies to the criterion identity at line 1114 and to datasetKey at 807.

"EvaluationsError",
"EvaluationsModule",
"GenerationConfig",
"InlineDatasetRow",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new root name for a type alias that callers don't need, since rows= accepts a DatasetRow or a plain dict either way. The lifecycle draft moves evals to launchdarkly_ai_server.experimental.evaluations. I'd leave it in launchdarkly_ai_server.evaluations only and keep it out of the root __all__, so the 1.0 surface trim doesn't have to remove it.

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.

2 participants