Skip to content
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

Port pydantic_ai.Agent to using pydantic_graph #725

Merged
merged 19 commits into from
Jan 30, 2025
Merged

Conversation

dmontagu
Copy link
Contributor

@dmontagu dmontagu commented Jan 20, 2025

This is an attempt to move the pydantic_ai.Agent implementation to be based on pydantic_graph, so we can get all the various graph utility features "for free".

While I think we might want to change some of the behaviors, I have attempted to port the existing implementation to use pydantic_graph with the minimal set of external-facing changes possible.

Copy link

cloudflare-workers-and-pages bot commented Jan 26, 2025

Deploying pydantic-ai with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2e37316
Status: ✅  Deploy successful!
Preview URL: https://b595f6c9.pydantic-ai.pages.dev
Branch Preview URL: https://dmontagu-graph-agent.pydantic-ai.pages.dev

View logs

@dmontagu dmontagu changed the base branch from main to dmontagu/graph-agent-prep January 26, 2025 17:23
@dmontagu dmontagu force-pushed the dmontagu/graph-agent-prep branch from a064ead to fb95046 Compare January 26, 2025 18:04
@dmontagu dmontagu force-pushed the dmontagu/graph-agent branch from 64a7e6d to 4fa40c9 Compare January 26, 2025 18:05
@dmontagu dmontagu marked this pull request as ready for review January 26, 2025 23:22
@Finndersen
Copy link

Nice one, do the normal and stream variants of each node need to be seperate classes? Could a stream: bool attribute used to decide behaviour work? (Could factor out this behaviour into a StreamableNode or something)

@dmontagu
Copy link
Contributor Author

Not in principle, but in practice we’re using different vendor APIs for streaming and non-streaming so it doesn’t get as much benefit from unifying the types as you might think. I’m planning to implement a stream_events API using the graph nodes next, which will better demonstrate how to have streaming with an iterator coexist with the usual run in a graph context.

@Finndersen
Copy link

Also, it would be nice if nodes could define & return abstractions/signatures of other nodes instead of concrete implementations/instances. Because currently this limits their flexibility and re-usability as building blocks.

Like for example each "node type" could have a signature (effectively an abstract base class kind of thing), and this is referenced by other nodes, but the actual implementations could be defined/overridden at runtime. So people could plug in their own implementations of these different nodes within the "agent" to customise it's behaviour as required

@Finndersen
Copy link

@dmontagu ok, I'm just on my phone so haven't looked in detail, but seems like there's still a bit of overlap in functionality, and to me I think it makes more sense to have a single node with a stream switch, since conceptually it is/does the same thing, just with a different approach.

I would've thought that streaming would require a different method API/signature?

@dmontagu
Copy link
Contributor Author

It sounds to me like you want a way to override a node’s behavior; why can’t you just do that by subclassing the node and overriding the run method? If you intend for the node to be extensible in some way, you can include the ways you want it to be extended as methods called inside the run method, and subclasses can override just those methods.

@Finndersen
Copy link

Finndersen commented Jan 27, 2025

But then the subclassed node won't be used in place of the original parent node by other nodes that already reference it.
Currently once a graph is made from a collection of nodes (like this agent implementation), it's effectively immutable and you can't extend/customise it by swapping out/overriding individual nodes in the graph.

This is because each node implementation contains both the node logic as well as the edge / next node configuration coupled together.
Sorry, this discussion isn't really relevant to this PR

@dmontagu dmontagu force-pushed the dmontagu/graph-agent branch 3 times, most recently from cf39d81 to 5c94b48 Compare January 27, 2025 18:17
@dmontagu dmontagu force-pushed the dmontagu/graph-agent-prep branch from 31bee5d to fbff76c Compare January 27, 2025 18:18
@dmontagu dmontagu force-pushed the dmontagu/graph-agent branch from 5c94b48 to 0f0434e Compare January 27, 2025 18:19
.github/workflows/ci.yml Outdated Show resolved Hide resolved
pydantic_ai_slim/pydantic_ai/agent.py Outdated Show resolved Hide resolved
pydantic_ai_slim/pydantic_ai/agent.py Outdated Show resolved Hide resolved
pydantic_ai_slim/pydantic_ai/agent.py Outdated Show resolved Hide resolved
Base automatically changed from dmontagu/graph-agent-prep to main January 28, 2025 16:44
@samuelcolvin samuelcolvin enabled auto-merge (squash) January 30, 2025 10:03
@samuelcolvin samuelcolvin merged commit 2398e50 into main Jan 30, 2025
13 checks passed
@samuelcolvin samuelcolvin deleted the dmontagu/graph-agent branch January 30, 2025 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants