Improved support for roundtrip AG_UI message conversion #3474
Conversation
|
|
||
| id_index = 0 | ||
|
|
||
| def get_next_id() -> str: |
There was a problem hiding this comment.
This is unfortunately more awkward than it would ideally be because metadata isn't universally available on all "MessageParts" and a ModelRequst/Response may contain multiple AG_UI Messages
@DouweM feels like metadata should maybe just be a base property on MessagePart?
There was a problem hiding this comment.
@ChuckJonas Yeah I think we'll need to have some dict for arbitrary values on each part. There's some related discussion in #3453 (if you go down a few comments) as well, although that focuses on developer-provided fields to be sent to provider APIs, whereas here the purpose is to store some information we receive from outside of Pydantic AI (whether that's a provider API or frontend) so that we can use it to losslessly send data back to that same provider/frontend. So we may need 2 dicts 😅
In any case, we're not going to be able to implement that in this PR, but I think we'll need it sooner rather than later.
| __all__ = ['messages_from_ag_ui', 'messages_to_ag_ui'] | ||
|
|
||
|
|
||
| def messages_from_ag_ui(messages: Sequence[Message]) -> list[ModelMessage]: |
There was a problem hiding this comment.
Feels like there could be a more "standardized" ModelMessage translator interface with to & from functions. This would be useful for converting between all the different model formats in general.
For now, I just kept it simple for now with the messages_from_ag_ui & messages_to_ag_ui functions.
There was a problem hiding this comment.
My comment above could've been here -- that's pretty much what I was thinking with load/dump methods on the adapter.
ChuckJonas
force-pushed
the
messages_from_ag_ui
branch
from
055a098 to
1080d91
Compare
| 'AGUIAdapter', | ||
| 'AGUIEventStream', | ||
| 'messages_from_ag_ui', | ||
| 'messages_to_ag_ui', |
There was a problem hiding this comment.
Quoting from Slack:
The way I imagined it, we'd have
load_messagesanddump_messagesclassmethods on the adapters, which is being implemented for Vercel in #3392. Some of the comments on that PR about lossless roundtrip may be relevant to AG-UI as well, although I'm not sure they support arbitrary metadata, and I'd be OK with an initial version that's best-effort
| __all__ = ['messages_from_ag_ui', 'messages_to_ag_ui'] | ||
|
|
||
|
|
||
| def messages_from_ag_ui(messages: Sequence[Message]) -> list[ModelMessage]: |
There was a problem hiding this comment.
My comment above could've been here -- that's pretty much what I was thinking with load/dump methods on the adapter.
|
|
||
| id_index = 0 | ||
|
|
||
| def get_next_id() -> str: |
There was a problem hiding this comment.
@ChuckJonas Yeah I think we'll need to have some dict for arbitrary values on each part. There's some related discussion in #3453 (if you go down a few comments) as well, although that focuses on developer-provided fields to be sent to provider APIs, whereas here the purpose is to store some information we receive from outside of Pydantic AI (whether that's a provider API or frontend) so that we can use it to losslessly send data back to that same provider/frontend. So we may need 2 dicts 😅
In any case, we're not going to be able to implement that in this PR, but I think we'll need it sooner rather than later.
|
@ChuckJonas FYI there's an older PR at #3068; I recommend checking out the code and comments to see if they apply here |
ChuckJonas
force-pushed
the
messages_from_ag_ui
branch
from
1080d91 to
257e448
Compare
2 participants
Adds
dump_messagesfunction for AG-UI (introduced in https://github.com/pydantic/pydantic-ai/pull/3392/files).Assuming that PR goes out soon, I can rebase this branch after and resolve conflicts.
NOTE: I attempted to implement preservation of the AG_UI id's but the complexity and lack of
metadata(as discussed in the outdated comment below), made it more trouble than it was worth.Sounds like this is consistent with the behavior in the "Vercel AI" implementation, although I didn't add a
_id_generator()for testing purposes.