feat(datasets): support multimodal dataset items#1710
Conversation
Adds dataset item media upload and optional get_dataset media hydration.
|
@claude review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd791b64ee
ℹ️ 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".
…hon-sdk-changes # Conflicts: # uv.lock
There was a problem hiding this comment.
actual (non generated changes)
| # Avoid jsonpath-ng here: dataset writes should keep working | ||
| # under python -OO where parser docstrings may be stripped. |
There was a problem hiding this comment.
in my opinion an okay compromise as the write path doesn't become a hard blocker for users and easy to maintain
Avoid hand rolling jsonpath parsing for the read path -> users can also opt out here so in my opinion fine
There was a problem hiding this comment.
actual (non generated changes)
There was a problem hiding this comment.
actual (non generated changes)
There was a problem hiding this comment.
actual (non generated changes)
| "opentelemetry-api>=1.33.1,<2", | ||
| "opentelemetry-sdk>=1.33.1,<2", | ||
| "opentelemetry-exporter-otlp-proto-http>=1.33.1,<2", | ||
| "jsonpath-ng>=1.8.0,<2", |
There was a problem hiding this comment.
Apache 2.0 license, 731 stars, actively maintained: https://github.com/h2non/jsonpath-ng
Annoying thing is the issues about running optimized python but I think we addressed them enough: https://github.com/h2non/jsonpath-ng#special-note-about-ply-and-docstrings
Next runner up would be jsonpath-python (https://pypi.org/project/python-jsonpath/)
- MIT license
- only 69 stars
I'd avoid hand rolling the parsing. Especially since we have find references via the TS jsonpath library we don't know exactly which syntax might get in there and which cases of the spec we need to handle.
Resolved LangfuseMediaReference items from get_dataset(resolve_media_ references=True) discarded the original @@@langfuseMedia:...@@@ string, so feeding them back into create_dataset_item or run_experiment serialized them as opaque dicts and orphaned the media. Persist the reference string and emit it from both _process_dataset_item_media and EventSerializer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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>
_process_dataset_item_media only recursed into list/dict, so a LangfuseMedia held in a tuple, set, or frozenset slipped through to fern's encoder and got silently base64-inlined instead of uploaded. Widen the walker to those containers and rebuild them in place. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…name The DatasetItemMediaReferenceField enum value changed from expected_output to expectedOutput. Decouple hydration from the wire value by mapping the enum member to the model attribute, so the rename (and any future one) no longer silently skips expected-output media references. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rebuilding tuples via type(data)(iterable) breaks namedtuple/NamedTuple subclasses, which take positional field args rather than an iterable. Keep list/set/frozenset handling and leave tuples untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
get_singleton_httpx_client silently returned the first-inserted instance, so a LangfuseMediaReference fetched from one client could go out through another client's transport (proxy/CA/mTLS). Mirror get_client: warn and fall back to default httpx when multiple clients exist, and let fetch_bytes/fetch_base64/fetch_data_uri take an explicit httpx client to deterministically honor per-client settings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Dataset item media uploads now send datasetId + datasetItemId + field instead of a trace context. create_dataset_item settles the item id up front (generating one when absent) so media can be linked before the item exists, threads the field and resolves the dataset id for each upload, and reuses the id for the create call. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hon-sdk-changes # Conflicts: # uv.lock
This comment has been minimized.
This comment has been minimized.
Resolving the dataset id inside the per-media upload path fired one datasets.get per distinct media and could orphan already-uploaded media if a later lookup failed. Resolve it once up front, before any upload, and thread dataset_id through the media processing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
URL-encode the dataset name in the create_dataset_item media lookup to match the other datasets.* call sites, and add the new dataset_id / dataset_item_id keys to the _upload_job test fixture so _process_upload_media_job no longer KeyErrors. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace each LangfuseMedia with its reference string and collect the media to upload in a single traversal per field, then resolve the dataset id once and upload the collected set. A plain item no longer does any datasets.get; media items do exactly one, before any upload. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
LangfuseMediaininput,expected_output, andmetadata.get_dataset(..., resolve_media_references=True)hydration to replace backend-reported media reference JSONPaths withLangfuseMediaReferenceobjects.Linear
Major Decisions
get_dataset(...)as the only high-level SDK read surface for resolved dataset media references.jsonpath-ngrecursive discovery for create-path media replacement.Review Focus
LangfuseMediaReferenceexport andget_dataset(..., resolve_media_references=True).Greptile Summary
This PR adds multimodal support to Langfuse dataset items, allowing
LangfuseMediaobjects to be embedded ininput,expected_output, andmetadatafields and uploaded trace-lessly, and adding aresolve_media_references=Trueoption toget_dataset()that hydrates backend-provided JSONPath references intoLangfuseMediaReferenceobjects.create_dataset_itemnow recursively discoversLangfuseMediavalues via$..this`` (jsonpath-ng), uploads each one synchronously through the updated_upload_media_sync(which accepts `trace_id=None`), deduplicates by `media_id` within a single call, and replaces each value with its reference string before sending to the API.get_dataset(resolve_media_references=True)passesinclude_media_referencesto the list endpoint, then walks backend-provided JSONPath annotations to replace reference strings with frozenLangfuseMediaReferencedataclass instances that exposefetch_bytes(),fetch_base64(), andfetch_data_uri()helpers.DatasetItemMediaReference,DatasetItemMediaReferenceField,DatasetItemMediaReferenceMedia) and theGetMediaUploadUrlRequestschema are updated to maketrace_idandfieldoptional, enabling trace-less uploads.Confidence Score: 4/5
Safe to merge; all changed paths work correctly for the expected JSON-like data shapes, and the two main concerns are style/fragility rather than broken behavior.
The media-upload and hydration flows are logically sound and covered by both unit and E2E tests. The two notable findings are:
_replace_json_path_valuediscards the return value ofupdate()and relies implicitly on in-place mutation (works today, diverges from how_process_dataset_item_mediahandles the same operation); andfetch_bytes()makes a blocking HTTP call with no timeout. Neither produces wrong output in the current code paths, but both leave the door open for subtle failures on refactoring or slow network conditions.langfuse/_client/client.py(_replace_json_path_value) andlangfuse/media.py(LangfuseMediaReference.fetch_bytes) are the two spots that would benefit from a second look before shipping to customers at scale.Sequence Diagram
sequenceDiagram participant User participant Langfuse SDK participant MediaManager participant Langfuse API participant Object Storage Note over User,Object Storage: create_dataset_item with LangfuseMedia User->>Langfuse SDK: create_dataset_item(input={"img": LangfuseMedia(...)}) Langfuse SDK->>Langfuse SDK: _process_dataset_item_media()<br/>jsonpath-ng discovers LangfuseMedia nodes Langfuse SDK->>MediaManager: _upload_media_sync(media) MediaManager->>Langfuse API: media.get_upload_url(trace_id=None, field=None) Langfuse API-->>MediaManager: upload_url + media_id MediaManager->>Object Storage: PUT bytes Object Storage-->>MediaManager: 200 OK MediaManager->>Langfuse API: media.patch(media_id, upload_http_status=200) MediaManager-->>Langfuse SDK: done Langfuse SDK->>Langfuse API: dataset_items.create(input={"img": "@@@langfuseMedia:..."}) Langfuse API-->>Langfuse SDK: DatasetItem Langfuse SDK-->>User: DatasetItem (reference strings) Note over User,Object Storage: get_dataset with resolve_media_references=True User->>Langfuse SDK: get_dataset(name, resolve_media_references=True) Langfuse SDK->>Langfuse API: dataset_items.list(includeMediaReferences=true) Langfuse API-->>Langfuse SDK: items with mediaReferences[] (JSONPaths + signed URLs) loop For each item Langfuse SDK->>Langfuse SDK: _hydrate_dataset_item_media_references()<br/>jsonpath-ng update per JSONPath to LangfuseMediaReference end Langfuse SDK-->>User: DatasetClient (LangfuseMediaReference in fields) User->>Langfuse SDK: item.input["img"].fetch_bytes() Langfuse SDK->>Object Storage: httpx.get(signed_url) Object Storage-->>Langfuse SDK: bytes Langfuse SDK-->>User: bytesPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat(datasets): support media references" | Re-trigger Greptile