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

Change: Internal Discriminator Modification #247

Merged
merged 2 commits into from
Dec 14, 2024
Merged

Conversation

sydney-runkle
Copy link
Member

@sydney-runkle sydney-runkle commented Dec 14, 2024

Following up on #232 (comment), where we discussed that this change should be added to a separate PR.

This PR changes the Messages structures such that each one has:

  • message_kind: a discriminator used to identify the message type
  • role: either 'model' or 'user', like Gemini has, and similar to Anthropic's 'assistant' or 'user'

Message parts, which are used to construct a ModelResponse now have a part_kind discriminator instead of just a plain kind.

Copy link

cloudflare-workers-and-pages bot commented Dec 14, 2024

Deploying pydantic-ai with  Cloudflare Pages  Cloudflare Pages

Latest commit: 84e9d09
Status: ✅  Deploy successful!
Preview URL: https://627b5456.pydantic-ai.pages.dev
Branch Preview URL: https://kind-vs-role.pydantic-ai.pages.dev

View logs

Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one drive by comment otherwise LGTM.

Feel free to switch to isinstance in another PR.

@@ -196,7 +196,7 @@ async def _messages_create(
anthropic_messages: list[MessageParam] = []

for m in messages:
if m.role == 'system':
if m.message_kind == 'system-prompt':
Copy link
Member

@samuelcolvin samuelcolvin Dec 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably better to use isinstance. @dmontagu already changed it in most places.

Copy link
Member Author

@sydney-runkle sydney-runkle Dec 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, will do!

@sydney-runkle
Copy link
Member Author

Verifying that the chat app still works locally, then I'll merge.

@sydney-runkle
Copy link
Member Author

The chat app is actually broken on main, so I'll fix that in a different PR.

@sydney-runkle sydney-runkle merged commit b08874b into main Dec 14, 2024
16 checks passed
@sydney-runkle sydney-runkle deleted the kind-vs-role branch December 14, 2024 13:55
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.

2 participants