-
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.
| 'grok:grok-4', | ||
| 'grok:grok-4-0709', | ||
| 'grok:grok-4-fast-non-reasoning', | ||
| 'grok:grok-4-fast-reasoning', |
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.
grok-4-fast as well?
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.
grok-4-fast is an alias for grok-4-fast-reasoning - how should we handle those, do we need all aliases defined?
| usage = self._map_usage(response) | ||
|
|
||
| # Map finish reason | ||
| finish_reason_map = { |
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.
Please move this mapping to a typed constant as we have it on other models
| if not hasattr(response, 'usage'): | ||
| return RequestUsage() | ||
|
|
||
| usage_obj = getattr(response, 'usage', 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.
All this usage of has/getattr confuses me; shouldn't the Response type have these fields so we can read them directly?
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'd rather not do any mocking and test everything against the (recorded) API.
| if model_request_parameters.builtin_tools: | ||
| tools.extend(self._get_builtin_tools(model_request_parameters)) | ||
| if model_request_parameters.tool_defs: | ||
| tools.extend(self._map_tools(model_request_parameters)) |
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.
Does the API support tool_choice? Look at the logic OpenAIChatModel uses to set it; we should do that here as well.
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.
| 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
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.