-
Notifications
You must be signed in to change notification settings - Fork 917
Add direct
public API
#1599
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
Add direct
public API
#1599
Conversation
PR Change SummaryAdded a new public API for low-level functionality in the pydantic_ai module, with initial documentation provided.
Added Files
How can I customize these reviews?Check out the Hyperlint AI Reviewer docs for more information on how to customize the review. If you just want to ignore it on this PR, you can add the Note specifically for link checks, we only check the first 30 links in a file and we cache the results for several hours (for instance, if you just added a page, you might experience this). Our recommendation is to add |
Docs Preview
|
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 think the high level API should be using the low level API code path to make sure things stay in sync.
In the agent graph I see this:
model_request_parameters = ctx.deps.model.customize_request_parameters(model_request_parameters)
model_response, request_usage = await ctx.deps.model.request(
ctx.state.message_history, model_settings, model_request_parameters
)
That customize_request_parameters
seems important and missing from the low level code.
I don't agree, the amount of code here is tiny, and there's some (minimal) savings by caching results, but I guess we can do that later if you like. |
|
oh sorry, I'll look. |
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.
As discussed, I think we should refer to this as the 'direct' API
(or biodynamic)
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.
Pull Request Overview
This PR introduces a new low-level "direct" API for making imperative requests to language models with minimal abstraction. Key changes include:
- Adding the "direct" module with three new methods: model_request, model_request_sync, and model_request_stream.
- Adjusting default values for ModelRequestParameters and enhancing documentation and tests to support the new API.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
tests/test_examples.py | Added a test case demonstrating tool call using ToolCallPart. |
tests/test_direct.py | New tests ensuring functionality of the direct API. |
pydantic_ai_slim/pydantic_ai/models/test.py | Updated return of _WrappedTextOutput with pragma removed. |
pydantic_ai_slim/pydantic_ai/models/instrumented.py | Introduced instrument_model utility and updated all. |
pydantic_ai_slim/pydantic_ai/models/init.py | Set default field values for ModelRequestParameters. |
pydantic_ai_slim/pydantic_ai/messages.py | Added a new class method user_text_prompt to ModelRequest. |
pydantic_ai_slim/pydantic_ai/direct.py | New module providing direct request methods. |
pydantic_ai_slim/pydantic_ai/agent.py | Updated model instrumentation to use instrument_model. |
mkdocs.yml | Updated navigation to include the direct API docs. |
docs/direct.md | New documentation for the direct API with examples. |
docs/api/direct.md | API docs page for the direct API module. |
docs/direct.md
Outdated
# Make a synchronous request to the model | ||
model_response = model_request_sync( | ||
'anthropic:claude-3-5-haiku-latest', | ||
[ModelRequest.user_text_prompt('What is the capital of France?')] |
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.
[ModelRequest.user_text_prompt('What is the capital of France?')] | |
[ModelRequest.user_text_prompt('What is the capital of France?')], |
(was double checking the difference between this snippet and the next)
shouldn't this have been flagged by ruff?
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 think ruff doesn't mind adding the comma means "don't compact this on to one line"
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.
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.
because the lin length limit in docs is 88
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.
then why doesn't it add the trailing comma?
adding a trailing comma in another line also didn't force it to expand as i expected
Co-authored-by: Alex Hall <[email protected]>
This introduces a new lower level API: