Conversation
| input=self._apply_mask(input), | ||
| expected_output=self._apply_mask(expected_output), |
There was a problem hiding this comment.
Masking can invalidate dataset items
If a dataset requires an object-shaped input or expected output, a mask that replaces the value with a string sends an invalid item to the API. The fallback used when masking fails is also a string. An item that met the dataset schema before masking is then rejected.
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/_client/client.py
Line: 3666-3667
Comment:
**Masking can invalidate dataset items**
If a dataset requires an object-shaped input or expected output, a mask that replaces the value with a string sends an invalid item to the API. The fallback used when masking fails is also a string. An item that met the dataset schema before masking is then rejected.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Same contract as span input/output masking; a mask that changes the shape affects spans the same way.
There was a problem hiding this comment.
That’s fair. The mask callback has the same intentionally unconstrained contract for span input/output, so changing the shape—and using the string fallback when the callback raises—is expected behavior here as well. A dataset schema can therefore reject masked data, but that is an existing consequence of the masking contract rather than a defect introduced by this change. I’m withdrawing this finding.
Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.
| value=value, | ||
| dataType=data_type, # type: ignore | ||
| comment=comment, | ||
| comment=self._apply_mask(comment), |
There was a problem hiding this comment.
Structured masks discard scores
The mask contract permits JSON-serializable results, but ScoreBody.comment requires a string. If a mask returns a dict or list for a comment, model validation fails. create_score logs and swallows that error, so it never queues the score and the caller receives no failure indication.
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/_client/client.py
Line: 2046
Comment:
**Structured masks discard scores**
The mask contract permits JSON-serializable results, but `ScoreBody.comment` requires a string. If a mask returns a dict or list for a comment, model validation fails. `create_score` logs and swallows that error, so it never queues the score and the caller receives no failure indication.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Non-string mask results are now json.dumps-ed before ScoreBody (aae4ea4, test added).
| input=self._apply_mask(input), | ||
| expected_output=self._apply_mask(expected_output), | ||
| metadata=self._apply_mask(metadata), |
There was a problem hiding this comment.
Masking removes uploaded media references
When a dataset item contains LangfuseMedia, this method uploads the media before applying the mask. If the mask replaces the field, the created item no longer contains the media reference, leaving the uploaded attachment unlinked to the item's payload.
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/_client/client.py
Line: 3666-3668
Comment:
**Masking removes uploaded media references**
When a dataset item contains `LangfuseMedia`, this method uploads the media before applying the mask. If the mask replaces the field, the created item no longer contains the media reference, leaving the uploaded attachment unlinked to the item's payload.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Same order as spans: _process_media_and_apply_mask processes media first, then masks (span.py L552).
There was a problem hiding this comment.
You're right that create_dataset_item follows the established span behavior: media is processed first and the resulting payload is then passed through the mask. This ordering is consistent with the existing masking contract, so I'll withdraw the media-linking concern.
Fixes #1896. Applies the client mask to create_dataset_item input/expected_output/metadata, create_dataset metadata and create_score comment, using the same fallback as span masking; those payloads bypassed mask entirely.
The PR is not ready to merge because valid masking configurations can reject dataset items, discard scores, or leave uploaded media unlinked.
Summary
The PR adds client-side masking for dataset metadata, dataset-item payloads, and score comments, with a fallback when a mask raises. The new masking can conflict with dataset schemas, discard scores whose mask returns structured data, and leave media uploads without references.
Reviews (1) · Last reviewed commit: "test: enable tracing so create_score rea..."