feat(datasets): multimodal dataset support (LFE-10288)#840
Merged
Conversation
Hand-applied ahead of the upstream Fern spec sync so the SDK can build
against the new endpoints. These files live under the generated
packages/core/src/api tree and will be reproduced verbatim by the API
spec bot once the langfuse/langfuse spec change merges:
- DatasetItem.mediaReferences + DatasetItemMediaReference{,Field,Media}
- GetDatasetItemsRequest.includeMediaReferences (+ list query wiring)
- GetMediaUploadUrlRequest: traceId and field are now optional, enabling
media uploads that are not associated with a trace (dataset items)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@claude review |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a490e1446
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Mirror langfuse/langfuse-python#1710 to add media support to datasets: - LangfuseMediaReference (core): a signed-URL media handle returned when resolving dataset media, with urlIsExpired / fetchBytes / fetchBase64 / fetchDataUri helpers for feeding media to LLM providers - uploadMedia (core): reusable upload routine that works without a trace context; MediaService now delegates to it instead of duplicating the upload + backoff logic - DatasetManager.createItem: uploads any LangfuseMedia found in input, expectedOutput, or metadata (deduped) and replaces it with a reference string before creating the item; createDatasetItem now routes here - DatasetManager.get(resolveMediaReferences): requests includeMediaReferences and hydrates reference strings into LangfuseMediaReference objects using jsonpath-plus (eval-free, so it is safe under strict CSP / edge runtimes) - re-export LangfuseMedia and LangfuseMediaReference from @langfuse/client Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
wochinge
force-pushed
the
feature/lfe-10288-js-sdk-changes
branch
from
6a490e1 to
af0256d
Compare
The Fern-generated resource methods dispatch to a private `this.__<name>` on their first line, so the unbound aliases (fetchTrace, fetchTraces, fetchObservation, fetchObservations, fetchSessions, getDatasetRun, getDatasetRuns, createDataset, getDatasetItem, fetchMedia, resolveMediaReferences) threw a TypeError when called off the client. Bind each to its owning resource and add regression tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Covers create with LangfuseMedia (stored as reference strings, deduped), get with resolveMediaReferences (hydration + fetch helpers), and re-using a resolved item through dataset.createItem (lossless round-trip).
The API spec renamed the DatasetItemMediaReferenceField enum value from expected_output to expectedOutput. Update the generated enum and the hydration field-allowlist (now an identity map) to match.
jsonpath-plus returns undefined (not []) when the root is null/undefined, so reading matches.length threw a TypeError (caught, surfaced as a misleading warning) when a media reference pointed at a null field.
The media upload endpoint now requires the (dataset, item) a media belongs to instead of a trace context. createItem settles the item id up front (the item need not exist yet), resolves the dataset id, and uploads each LangfuseMedia with its field (input/expectedOutput/metadata). Mirrors langfuse/langfuse-python#1710.
createItem now walks each field collecting media (reference strings are content-derived, no upload needed), then resolves the dataset id and uploads only if media was found — media-free items make no extra request. The walk is extracted to a module-level replaceDatasetItemMedia function for readability.
dataset.get now always returns LangfuseMediaReference objects instead of an opt-in resolveMediaReferences flag — the backend resolves them unconditionally, so the includeMediaReferences param is dropped from the SDK. Use api.datasetItems.list for the raw reference strings. Also rename LangfuseMediaReference.urlIsExpired to isUrlExpired.
hassiebp
reviewed
Jun 22, 2026
hassiebp
reviewed
Jun 22, 2026
hassiebp
reviewed
Jun 22, 2026
hassiebp
requested changes
Jun 22, 2026
hassiebp
left a comment
hassiebp
left a comment
Collaborator
There was a problem hiding this comment.
thanks Tobi - same questions as with Python PR on the dependency
Unit tests were co-located under packages/*/src and not executed by any CI job (the workspace only defined integration + e2e projects). Move the dataset media unit tests to tests/unit, add a src-aliased `unit` vitest project + `test:unit` script + a CI job, and DRY the per-project @langfuse/* alias blocks into one helper.
Relocate the pre-existing otel (span-filter, span-processor) and experiment (RunnerContext) unit tests from packages/*/src to tests/unit, rewriting their relative imports to public @langfuse/* package imports. All unit tests now live in tests/unit and run via the unit project in CI.
Drop the jsonpath-plus dependency in favor of a tiny parse/set helper (jsonPath.ts) that handles exactly the bracket-normalized paths the backend emits ($, ['key'], [int]) — ported from the Python SDK. Add unit tests mirroring the Python json_path table and extend the e2e to cover key/list-index/nested-list path shapes, verifying each resolves to its own media via fetchBytes.
Drop the thin MediaManager.uploadMedia wrapper (an unintended public method on langfuse.media); createItem now calls the core uploadMedia helper directly.
…sdk-changes # Conflicts: # vitest.workspace.ts
…sdk-changes # Conflicts: # packages/core/src/api/api/resources/commons/types/DatasetItem.ts # packages/core/src/api/api/resources/commons/types/DatasetItemMediaReference.ts # packages/core/src/api/api/resources/media/types/GetMediaUploadUrlRequest.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
dataset.createItem): uploads anyLangfuseMediaininput/expectedOutput/metadata(nested, deduped) against its(dataset, item, field), and stores@@@langfuseMedia:…@@@reference strings. The dataset id is resolved only when the item actually carries media.dataset.get): always hydrates the reference strings intoLangfuseMediaReferenceobjects (signed URL +fetchBytes/fetchBase64/fetchDataUri+isUrlExpired). Useapi.datasetItems.listfor the raw strings.uploadMedia;LangfuseMediaReference(withtoJSON→ reference string, so resolved items round-trip losslessly). Re-exported from@langfuse/client.thiscrash).Linear
Major Decisions
jsonPath.ts, ported from the Python SDK) instead ofjsonpath-plus. It handles exactly the bracket-normalized paths the backend emits ($,['key'],[int]).dataset.getresolves media unconditionally (no opt-in flag); the backend resolves them by default.packages/core/src/apichanges are hand-applied ahead of the spec bot — drop/rebase that part once the bot regenerates.Review Focus
dataset.createItem,dataset.get(now always-resolving),LangfuseMedia/LangfuseMediaReference.MediaManager.uploadMediawas removed to avoid an unintended public method; the deprecatedcreateDatasetItemalias changed signature (no.withRawResponse()/requestOptions) — documented on the field.