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>
| or f"<Upload handling failed for LangfuseMedia of type {obj._content_type}>" | ||
| ) | ||
|
|
||
| if ( | ||
| isinstance(obj, LangfuseMediaReference) | ||
| and obj.reference_string is not None | ||
| ): | ||
| return obj.reference_string | ||
|
|
||
| # Check if numpy is available and if the object is a numpy scalar | ||
| # If so, convert it to a Python scalar using the item() method | ||
| if np is not None and isinstance(obj, np.generic): |
There was a problem hiding this comment.
🔴 The new LangfuseMediaReference opacity branch at serializer.py:65-69 only fires when an LMR is dispatched directly to default() (top-level or as a dict/list element). When an LMR is nested inside a user @dataclass or pydantic.BaseModel, the existing is_dataclass: return asdict(obj) branch at line 130 (and the model_dump() branch at 132-139) does CPython's recursive conversion that never re-enters default(), so the nested LMR (itself a frozen dataclass) is emitted as a flat dict exposing url, url_expiry, content_length, and reference_string as raw JSON fields — defeating the opacity this PR was explicitly added to provide. Fix: in the dataclass and BaseModel branches, iterate fields manually and route each value through self.default() so nested LMRs (and LangfuseMedia) hit the opacity branches.
Extended reasoning...
What the bug is
This PR adds a new opacity branch in EventSerializer._default_inner for LangfuseMediaReference (serializer.py:65-69), mirroring the existing branch for LangfuseMedia at 59-63. Both are meant to collapse the rich media object back to its @@@langfuseMedia:...@@@ reference string instead of letting CPython's default object serialization leak the signed URL and other transport-only fields.
That branch only fires when the LMR is dispatched directly to default() — i.e., top-level, or as a dict/list element (since the dict/list branches at lines 148-152 explicitly call self.default(v) on each value). But the dataclass branch (return asdict(obj), line 130) and the Pydantic BaseModel branch (return obj.model_dump(), line 139) do CPython-native recursive conversion that never calls back into default(). LangfuseMediaReference is itself a @dataclass(frozen=True), so when wrapped in a parent dataclass asdict walks into it and emits its fields as a flat dict.
Why existing safeguards don't catch it
- The new unit test
test_langfuse_media_reference_serializes_to_reference_stringonly verifies the top-level case. - The existing
LangfuseMediabranch has the exact same blind spot but is partially masked by the trace-side flow:MediaManager._find_and_process_mediatraverses dicts/lists and replaces nestedLangfuseMediainstances with reference-string equivalents beforeEventSerializerever sees them. There's no equivalent traversal forLangfuseMediaReference. - Same shape exists on the write path via fern's
jsonable_encoder(langfuse/api/core/jsonable_encoder.py), which also usesasdict/model_dump— so the same nested-LMR-in-user-dataclass leak reachescreate_dataset_itemAPI bodies and persists in stored dataset items, breaking the read-side round-trip guarantee that bug Bump langchain from 0.0.237 to 0.0.262 #5'sreference_stringfield added for direct-LMR-in-dict.
Step-by-step proof
Verified empirically against current HEAD:
@dataclass
class Container:
image_ref: LangfuseMediaReference
note: str = 'hello'
lmr = LangfuseMediaReference(
media_id='abc', content_type='image/png',
url='https://signed.example.com/SECRET_TOKEN',
url_expiry='2026-06-15T12:00:00Z', content_length=1234,
reference_string='@@@langfuseMedia:type=image/png|id=abc|source=bytes@@@',
)
s = EventSerializer()
print(s.encode(lmr))
# "@@@langfuseMedia:type=image/png|id=abc|source=bytes@@@" (opacity preserved)
print(s.encode({'image_ref': lmr}))
# {"image_ref": "@@@langfuseMedia:..."} (opacity preserved)
print(s.encode(Container(image_ref=lmr)))
# {"image_ref": {"media_id": "abc", "content_type": "image/png",
# "url": "https://signed.example.com/SECRET_TOKEN",
# "url_expiry": "...", "content_length": 1234,
# "reference_string": "@@@..."}, "note": "hello"} (URL LEAKED)Same shape for pydantic.BaseModel: s.encode(PydanticContainer(image_ref=lmr)) emits the identical flat-dict structure with the signed URL inlined.
Impact
The signed url and url_expiry end up serialized into trace data and (via fern's encoder on the write path) into create_dataset_item API bodies / stored dataset items, even though this PR explicitly added the opacity branch to prevent that.
The round-trip case from bug #5's fix breaks specifically when an LMR is wrapped in a user dataclass/Pydantic model: the server's media-reference scanner still finds the @@@langfuseMedia:...@@@ pattern inside the persisted reference_string field on hydration, so get_dataset(resolve_media_references=True) emits an LMR at $['wrap']['image_ref']['reference_string'] but leaves the sibling url/url_expiry/content_length strings stale — the user observes a mixed/confusing shape where item.input['wrap']['image_ref'] is a dict (not an LMR) and ['url'] is a soon-to-expire string from the original write.
The trigger is plausible: users with @observe-decorated functions returning typed results (dataclasses or Pydantic models) wrapping LMRs obtained from get_dataset(resolve_media_references=True) — a natural pattern in ML/LLM application code that builds domain types around SDK outputs.
How to fix
In the dataclass branch (and analogously for the Pydantic branch), iterate fields manually and route each value back through self.default() so nested LMRs / LangfuseMedia hit the opacity branches:
if is_dataclass(obj):
from dataclasses import fields
return {f.name: self.default(getattr(obj, f.name)) for f in fields(obj)}
if isinstance(obj, BaseModel):
obj.model_rebuild()
if isinstance(raw := getattr(obj, "raw", None), BaseModel):
raw.model_rebuild()
return {k: self.default(v) for k, v in obj.model_dump().items()}The Pydantic case needs a small pre-pass or field iteration since model_dump() itself recursively converts; alternatively the LMR substitution can be done before model_dump() is called. Either approach preserves opacity for nested LMRs without regressing the LMR-in-dict round-trip the PR already restored.
(The write path — fern's jsonable_encoder — has the same structural issue and is worth tracking separately; the serializer fix alone closes the trace-side leak.)
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