-
Notifications
You must be signed in to change notification settings - Fork 1.4k
XaiModel with tests and stock analysis agent #3400
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
|
@brightsparc Thanks Julian! Let me know when this is ready for review or if you have any questions. |
|
Due to using otel in our own SDK, I did notice an error when running the pydantic ai code in async.io loop: We could disable our tracing with |
@brightsparc Looks like grok-4-fast is not on https://github.com/pydantic/genai-prices/blob/main/prices/providers/x_ai.yml yet. Contribution welcome! (See contrib docs) Failing tests: I believe we just need to add the xAI env var to the list here: pydantic-ai/tests/test_examples.py Lines 179 to 182 in c096d99
This one should be fixed by running Looks like we need the new
Do you have example code for me that triggers it? |
I will make a separate PR here
Added this:
Not sure I follow this
You can run the following command to repo that error: |
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.
If we're going to include this example, we should have a doc under docs/examples as well. I'm also OK with not including this example.
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.
I've added a page in the docs for this example. I can remove it if you don't think it's worthwhile.
| xai_settings['frequency_penalty'] = model_settings['frequency_penalty'] | ||
|
|
||
| # Create chat instance | ||
| chat = client.chat.create(model=self._model_name, messages=xai_messages, tools=tools_param, **xai_settings) |
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.
Just to confirm that the API does not support structured JSON output? If it does, we need to implement support for that as well: https://ai.pydantic.dev/output/#native-output
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.
Yes it does, I can add support for that.
…d XaiProvider and move api_key and client to this
…common _create_chat method
docs/api/models/grok.md
Outdated
| @@ -0,0 +1,7 @@ | |||
| # `pydantic_ai.models.grok` | |||
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.
We should add both new docs pages to mkdocs.yml so they show up in the navbar
tests/models/test_xai.py
Outdated
| response_message = messages[-1] | ||
| assert isinstance(response_message, ModelResponse) | ||
|
|
||
| # Should have at least one BuiltinToolCallPart for MCP tools (prefixed with server_label, e.g. "linear.list_issues") |
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.
Instead of these types of tests with multiple assertions, I'd much rather test against the entire message history snapshot, so that we can see immediately whether it looks like what we expect -- not just that the subfields we test for have the right values
This PR includes support for
GrokModelwhich uses the xai-sdk-python library, which supports chat completion, tool calling, and built-in server side tool calls.I have included a series of tests, as well as an example for a stock analysis agent that uses search and code execution, and returns results in the form of a local tool call.
Below is what the output looks like in logfire. It includes custom spans emmited from the xai-sdk.
Cost is coming back as Unknown so it would be good to understand how we can populate this.