-
Notifications
You must be signed in to change notification settings - Fork 633
[FEAT] Support JSON Schema in Responses #1177
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?
Changes from all commits
073cf62
244f2cf
9fccc5b
dd36ea2
170b16b
dd56600
8df25b9
5405dec
fa4ca37
7390271
320d58c
842cd03
80e8cd4
2003466
45cb825
ec4efaa
1eb4395
29fdb2f
2c8e919
9009edf
d899af4
c78f819
45f73a6
becb214
f37d070
6072ae3
4262cd7
970c4f2
55502ff
388f138
b6182e9
630842a
4da3b9d
fd03bba
21fe8a0
48c61cc
cf71d2d
d231d79
c8b4d0d
03c59a6
5275b14
9dfdcf6
5cef55d
bea538c
e5bbea0
f325976
271ea14
e90e409
b1a7fdb
a272af9
eee44be
41f576c
955ce14
5c53356
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,48 @@ | ||||||
| # Copyright (c) Microsoft Corporation. | ||||||
| # Licensed under the MIT license. | ||||||
|
|
||||||
| from __future__ import annotations | ||||||
|
|
||||||
| import json | ||||||
| from dataclasses import dataclass | ||||||
| from typing import Any, Dict, Optional | ||||||
|
|
||||||
|
|
||||||
| @dataclass | ||||||
| class JsonResponseConfig: | ||||||
| """ | ||||||
| Configuration for JSON responses (with OpenAI). | ||||||
| """ | ||||||
|
|
||||||
| enabled: bool = False | ||||||
| schema: Optional[Dict[str, Any]] = None | ||||||
| schema_name: str = "CustomSchema" | ||||||
| strict: bool = True | ||||||
|
|
||||||
| @classmethod | ||||||
| def from_metadata(cls, *, metadata: Optional[Dict[str, Any]]) -> "JsonResponseConfig": | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: should be possible with the imported annotations.
Suggested change
|
||||||
| if not metadata: | ||||||
| return cls(enabled=False) | ||||||
|
|
||||||
| response_format = metadata.get("response_format") | ||||||
| if response_format != "json": | ||||||
| return cls(enabled=False) | ||||||
|
|
||||||
| schema_val = metadata.get("json_schema") | ||||||
| if schema_val: | ||||||
| if isinstance(schema_val, str): | ||||||
| try: | ||||||
| schema = json.loads(schema_val) if schema_val else None | ||||||
| except json.JSONDecodeError: | ||||||
| raise ValueError(f"Invalid JSON schema provided: {schema_val}") | ||||||
| else: | ||||||
| schema = schema_val | ||||||
|
|
||||||
| return cls( | ||||||
| enabled=True, | ||||||
| schema=schema, | ||||||
| schema_name=metadata.get("schema_name", "CustomSchema"), | ||||||
| strict=metadata.get("strict", True), | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have to say this worries me. You also pointed that out in a comment earlier @riedgar-ms . The keys used here are basically now reserved for JSON schema. I would be less concerned if it was prefixed with |
||||||
| ) | ||||||
|
|
||||||
| return cls(enabled=True) | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ | |
| import abc | ||
| from typing import Optional | ||
|
|
||
| from pyrit.models import MessagePiece | ||
| from pyrit.models import JsonResponseConfig, MessagePiece | ||
| from pyrit.prompt_target import PromptTarget | ||
|
|
||
|
|
||
|
|
@@ -86,16 +86,32 @@ def is_response_format_json(self, message_piece: MessagePiece) -> bool: | |
| include a "response_format" key. | ||
|
|
||
| Returns: | ||
| bool: True if the response format is JSON and supported, False otherwise. | ||
| bool: True if the response format is JSON, False otherwise. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what prompted the extra indentation? |
||
|
|
||
| Raises: | ||
| ValueError: If "json" response format is requested but unsupported. | ||
| """ | ||
| if message_piece.prompt_metadata: | ||
| response_format = message_piece.prompt_metadata.get("response_format") | ||
| if response_format == "json": | ||
| if not self.is_json_response_supported(): | ||
| target_name = self.get_identifier()["__type__"] | ||
| raise ValueError(f"This target {target_name} does not support JSON response format.") | ||
| return True | ||
| return False | ||
| config = self.get_json_response_config(message_piece=message_piece) | ||
| return config.enabled | ||
|
|
||
| def get_json_response_config(self, *, message_piece: MessagePiece) -> JsonResponseConfig: | ||
| """ | ||
| Get the JSON response configuration from the message piece metadata. | ||
|
|
||
| Args: | ||
| message_piece: A MessagePiece object with a `prompt_metadata` dictionary that may | ||
| include JSON response configuration. | ||
|
|
||
| Returns: | ||
| JsonResponseConfig: The JSON response configuration. | ||
|
|
||
| Raises: | ||
| ValueError: If JSON response format is requested but unsupported. | ||
| """ | ||
| config = JsonResponseConfig.from_metadata(metadata=message_piece.prompt_metadata) | ||
|
|
||
| if config.enabled and not self.is_json_response_supported(): | ||
| target_name = self.get_identifier()["__type__"] | ||
| raise ValueError(f"This target {target_name} does not support JSON response format.") | ||
|
|
||
| return config | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ | |
| pyrit_target_retry, | ||
| ) | ||
| from pyrit.models import ( | ||
| JsonResponseConfig, | ||
| Message, | ||
| MessagePiece, | ||
| PromptDataType, | ||
|
|
@@ -315,7 +316,9 @@ async def _build_input_for_multi_modal_async(self, conversation: MutableSequence | |
|
|
||
| return input_items | ||
|
|
||
| async def _construct_request_body(self, conversation: MutableSequence[Message], is_json_response: bool) -> dict: | ||
| async def _construct_request_body( | ||
| self, *, conversation: MutableSequence[Message], json_config: JsonResponseConfig | ||
| ) -> dict: | ||
| """ | ||
| Construct the request body to send to the Responses API. | ||
|
|
||
|
|
@@ -324,13 +327,15 @@ async def _construct_request_body(self, conversation: MutableSequence[Message], | |
|
|
||
| Args: | ||
| conversation: The full conversation history. | ||
| is_json_response: Whether the response should be formatted as JSON. | ||
| json_config: Specification for JSON formatting. | ||
|
|
||
| Returns: | ||
| dict: The request body to send to the Responses API. | ||
| """ | ||
| input_items = await self._build_input_for_multi_modal_async(conversation) | ||
|
|
||
| text_format = self._build_text_format(json_config=json_config) | ||
|
|
||
| body_parameters = { | ||
| "model": self._model_name, | ||
| "max_output_tokens": self._max_output_tokens, | ||
|
|
@@ -339,7 +344,7 @@ async def _construct_request_body(self, conversation: MutableSequence[Message], | |
| "stream": False, | ||
| "input": input_items, | ||
| # Correct JSON response format per Responses API | ||
| "response_format": {"type": "json_object"} if is_json_response else None, | ||
| "text": text_format, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the 'bug fix' part; https://platform.openai.com/docs/api-reference/responses/create#responses_create-text |
||
| } | ||
|
|
||
| if self._extra_body_parameters: | ||
|
|
@@ -348,6 +353,23 @@ async def _construct_request_body(self, conversation: MutableSequence[Message], | |
| # Filter out None values | ||
| return {k: v for k, v in body_parameters.items() if v is not None} | ||
|
|
||
| def _build_text_format(self, json_config: JsonResponseConfig) -> Optional[Dict[str, Any]]: | ||
| if not json_config.enabled: | ||
| return None | ||
|
|
||
| if json_config.schema: | ||
| return { | ||
| "format": { | ||
| "type": "json_schema", | ||
| "name": json_config.schema_name, | ||
| "schema": json_config.schema, | ||
| "strict": json_config.strict, | ||
| } | ||
| } | ||
|
|
||
| logger.info("Using json_object format without schema - consider providing a schema for better results") | ||
| return {"format": {"type": "json_object"}} | ||
|
|
||
| def _check_content_filter(self, response: Any) -> bool: | ||
| """ | ||
| Check if a Response API response has a content filter error. | ||
|
|
@@ -445,7 +467,10 @@ async def send_prompt_async(self, *, message: Message) -> list[Message]: | |
| self._validate_request(message=message) | ||
|
|
||
| message_piece: MessagePiece = message.message_pieces[0] | ||
| is_json_response = self.is_response_format_json(message_piece) | ||
| json_config = JsonResponseConfig(enabled=False) | ||
| if message.message_pieces: | ||
| last_piece = message.message_pieces[-1] | ||
| json_config = self.get_json_response_config(message_piece=last_piece) | ||
|
|
||
| # Get full conversation history from memory and append the current message | ||
| conversation: MutableSequence[Message] = self._memory.get_conversation( | ||
|
|
@@ -462,7 +487,7 @@ async def send_prompt_async(self, *, message: Message) -> list[Message]: | |
| while True: | ||
| logger.info(f"Sending conversation with {len(conversation)} messages to the prompt target") | ||
|
|
||
| body = await self._construct_request_body(conversation=conversation, is_json_response=is_json_response) | ||
| body = await self._construct_request_body(conversation=conversation, json_config=json_config) | ||
|
|
||
| # Use unified error handling - automatically detects Response and validates | ||
| result = await self._handle_openai_request( | ||
|
|
||
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 did this because one of the unit tests whinged at me, but I'm not sure that this really ought to be public @romanlutz ?
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.
Ah yes, test_api_documentation.py 😆 You can definitely add exclusions there. It wouldn't be the first.