fix(weave): Bump feedback payload size limit to 1 MiB #2926
Conversation
|
Preview this PR with FeatureBee: https://beta.wandb.ai/?betaVersion=8c2cd8dbef8e26e4bdac78c2e146b44c4a2267c7 |
| @@ -1353,7 +1353,7 @@ def feedback_create(self, req: tsi.FeedbackCreateReq) -> tsi.FeedbackCreateRes: | |||
| created_at = datetime.datetime.now(ZoneInfo("UTC")) | |||
| # TODO: Any validation on weave_ref? | |||
| payload = _dict_value_to_dump(req.payload) | |||
| MAX_PAYLOAD = 1024 | |||
There was a problem hiding this comment.
was the old size constraint arbitrary? Should we enforce this older payload size for older types of feedback?
There was a problem hiding this comment.
Yes, pretty arbitrary. I did not have time when developing feedback to analyze what the impact of choosing something larger would be, and it is much easier to increase a limit than decrease one. The larger the limit the more people are tempted to do things like base64 encode images as strings and jam them in there and we know what kind of trouble that can cause. :-) Also worth noting that I believe we don't have per-call limits on number of feedbacks.
If we change this we should also update the docs: https://github.com/wandb/weave/blob/master/docs/docs/guides/tracking/feedback.md?plain=1#L85
We should keep in mind that the grid of feedback in the UI is going to load all of this.
It would be good if a nice error message explaining what was happening was propagated to the user. I don't know if we have any information on how large the user's payloads are or an intuition on the size distribution of scorer output in general. If not, we may want to ratchet this up a little more slowly.
Description
(Most likely ;) fixes https://wandb.atlassian.net/issues/WB-21753?jql=reporter%3D%22Adrian%20Swanberg%22%20ORDER%20BY%20created%20DESC
No reason to have the limit as small as it was now that we're using Feedback for all scorers. llm-as-judge feedback in particularly can easily be larger than 1 KiB
Testing
Updated a test.