-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add messages_to_ag_ui helper method
#3068
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| messages.append( | ||
| AssistantMessage( | ||
| id=str(uuid.uuid4()), | ||
| content=' '.join(content_parts) if content_parts else None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't quite right: consecutive text parts should be concatenated without a space in between, and if there are parts in between (like built-in tool calls), the text to either side should be joined by \n\n. See the example here: https://github.com/pydantic/pydantic-ai/pull/2970/files#diff-2eb561c8eaa8a723f1017556cce8006c42e504997c187b8b394b5e8634f91283R1148
| # Create separate ToolMessages for builtin tool returns | ||
| for builtin_return in builtin_returns: | ||
| prefixed_id = ( | ||
| f'{_BUILTIN_TOOL_CALL_ID_PREFIX}|{builtin_return.provider_name or ""}|{builtin_return.tool_call_id}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a helper method as I suggested above
| tool_call_id=prefixed_id, | ||
| content=builtin_return.content | ||
| if isinstance(builtin_return.content, str) | ||
| else str(builtin_return.content), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use builtin_return.model_response_str()
| result = messages_to_ag_ui(messages) | ||
|
|
||
| # Check structure and count | ||
| assert len(result) == 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use assert result == snapshot() here and below so we can see the entire thing in the test. The first time you run the test it'll be filled in.
| return messages, builtin_returns | ||
|
|
||
|
|
||
| def messages_to_ag_ui(messages: list[ModelMessage]) -> list[Message]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make messages_from_ag_ui public as well
|
@DouweM Great, thanks for input and nice that you pointed out to model_response_str. Will get to them shortly. One thing I noticed while doing this was that Logfire don't parse the chat history as expected. The tool output is a json string which seem to not work with how logfire handles the events. Just noticed how the UI cannot parse these events properly This is a separate issue and PR not related to this, but adding the option to pass history highlights this issue |
@jhammarstedt I don't quite understand, would you mind filing a new issue with an example? |
|
This PR is stale, and will be closed in 3 days if no reply is received. |
|
I'm refactoring the AG-UI integration in #2923. This can become a static method on |
|
This PR is stale, and will be closed in 3 days if no reply is received. |
|
This PR is stale, and will be closed in 3 days if no reply is received. |
|
@jhammarstedt Now that #2923 has landed, this could become a new Are you still interested in working on this? |
|
Ok nice! Yea just haven't gotten time to get to it. I'll see if I can carve out some next week otherwise I'll let you know! |
|
This PR is stale, and will be closed in 3 days if no reply is received. |
|
This PR is stale, and will be closed in 3 days if no reply is received. |
Add reverse conversion function
messages_to_ag_ui()Motivation
The AG-UI integration currently supports converting AG-UI messages to Pydantic AI's internal format via
_messages_from_ag_ui(), but there's no public function to perform the reverse conversion. This creates an asymmetry in the API and makes it difficult for users to:Changes
New Function:
messages_to_ag_ui()Added a public function that converts Pydantic AI's
ModelMessageformat to AG-UI'sMessageformat:Implementation details:
UserPromptPart,SystemPromptPart,ToolReturnPart,RetryPromptPart,TextPart,ToolCallPart,BuiltinToolCallPart, andBuiltinToolReturnPartpyd_ai_builtin|{provider_name}|{tool_call_id}to match the format expected by_messages_from_ag_ui()TextPartand tool calls from a singleModelResponseinto oneAssistantMessageThinkingPartas it's not part of the conversational message historyComplexity reduction:
To maintain code quality (cyclomatic complexity < 15), the implementation is split into helper functions:
_convert_request_part(): Converts individualModelRequestparts to AG-UI messages_convert_response_parts(): ProcessesModelResponseparts and collects builtin tool returnsComprehensive Test Coverage
Added
test_messages_to_ag_ui()that validates:The test covers the same complex scenario as
test_messages_from_ag_ui()to ensure bidirectional conversion compatibility.Checklist