-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: Support inject_schema_instructions_in_native_mode #3744
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
DouweM
left a comment
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.
@xcpky Thanks! Can you please add a test for the model profile behavior + new template field? If we update OutlinesModel/Provider to use the new model profile field, it may be sufficient to add a test to test_outlines.py.
| template: str | None | ||
| """Template for the prompt passed to the model. | ||
| The '{schema}' placeholder will be replaced with the output JSON schema. | ||
| If not specified, the default template specified on the model's profile will be used. |
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 need to clarify that this is only used if 1) explicitly provided or 2) the model requires it
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 more clarification.
| """ | ||
| ) | ||
| """The instructions template to use for prompted structured output. The '{schema}' placeholder will be replaced with the JSON schema for the output.""" | ||
| inject_schema_instructions_in_native_mode: bool = False |
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 have a mild preference for describing traits of the models/providers, rather than our behavior:
| inject_schema_instructions_in_native_mode: bool = False | |
| json_schema_output_requires_schema_in_instructions: bool = False |
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.
Should we we set this to True on any profiles already?
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 may be able to remove this:
pydantic-ai/pydantic_ai_slim/pydantic_ai/models/outlines.py
Lines 528 to 534 in c1213e4
| def customize_request_parameters(self, model_request_parameters: ModelRequestParameters) -> ModelRequestParameters: | |
| """Customize the model request parameters for the model.""" | |
| if model_request_parameters.output_mode in ('auto', 'native'): | |
| # This way the JSON schema will be included in the instructions. | |
| return replace(model_request_parameters, output_mode='prompted') | |
| else: | |
| return model_request_parameters |
if we set the new field here:
pydantic-ai/pydantic_ai_slim/pydantic_ai/providers/outlines.py
Lines 33 to 40 in c1213e4
| def model_profile(self, model_name: str) -> ModelProfile | None: | |
| """The model profile for the named model, if available.""" | |
| return ModelProfile( | |
| supports_tools=False, | |
| supports_json_schema_output=True, | |
| supports_json_object_output=True, | |
| default_structured_output_mode='native', | |
| ) |
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 have a mild preference for describing traits of the models/providers, rather than our behavior:
I think native_output_requires_schema_in_instructions is better
we may be able to remove this:
pydantic-ai/pydantic_ai_slim/pydantic_ai/models/outlines.py
Lines 528 to 534 in c1213e4
def customize_request_parameters(self, model_request_parameters: ModelRequestParameters) -> ModelRequestParameters: """Customize the model request parameters for the model.""" if model_request_parameters.output_mode in ('auto', 'native'): # This way the JSON schema will be included in the instructions. return replace(model_request_parameters, output_mode='prompted') else: return model_request_parameters if we set the new field here:
pydantic-ai/pydantic_ai_slim/pydantic_ai/providers/outlines.py
Lines 33 to 40 in c1213e4
def model_profile(self, model_name: str) -> ModelProfile | None: """The model profile for the named model, if available.""" return ModelProfile( supports_tools=False, supports_json_schema_output=True, supports_json_object_output=True, default_structured_output_mode='native', )
Done
DouweM
left a comment
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.
@xcpky Thanks! Can you please add a test for the model profile behavior + new template field? If we update OutlinesModel/Provider to use the new model profile field, it may be sufficient to add a test to test_outlines.py.
I've added the test, plz review. |
| If not specified, the default template specified on the model's profile will be used only if `native_output_requires_schema_in_instructions` on the profile is True. | ||
| If explicitly provided, it's used regardless of `native_output_requires_schema_in_instructions`. |
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.
Users mostly don't deal with profile fields directly, and should be able to assume that we've done our best to make them correct out of the box so that they don't have to worry about them, so I'd rather say:
| If not specified, the default template specified on the model's profile will be used only if `native_output_requires_schema_in_instructions` on the profile is True. | |
| If explicitly provided, it's used regardless of `native_output_requires_schema_in_instructions`. | |
| If no template is specified but the model's profile indicates that it requires the schema to be sent as a prompt, the default template specified on the profile will be used. |
closes #3490
Add a field "inject_schema_instructions_in_native_mode" in ModelProfile
The implementation is basically as per @DouweM , with the following improvement:
NativeOutputnow can accept a template and json schema instruction is injected regardless ofinject_schema_instructions_in_native_modein that case. For example: