Implement OpenAIResponsesGenerator#1806
Conversation
Signed-off-by: ABeltramo <beltramo.ale@gmail.com>
jmartin-tech
left a comment
jmartin-tech
left a comment
There was a problem hiding this comment.
This PR looks pretty useful, the new formats are something we are seeing more middle layers starting to mimic.
Some overall asks:
- Please rebase this on
mainand adjust the PR target. - Given the comments made here about
OpenAICompatiblethis might be functionality that has value in finding a way to integrate there.
| } | ||
|
|
||
|
|
||
| class OpenAIResponsesGenerator(Generator): |
There was a problem hiding this comment.
This should probably extend OpenAICompatible or possibly be integrated into that class.
If kept separate, I would even go as far as suggesting this should be OpenAIReponsesCompatible as the intent described in the description suggests usage with any service that supports the Responses APi.
| # response.output item types to collect into the final text. | ||
| # "message" captures standard assistant text; "reasoning" captures | ||
| # the model's reasoning summary (ResponseReasoningItem). | ||
| "output_types": ["message"], |
There was a problem hiding this comment.
This needs first class support, reasoning traces are additional data that garak needs to start gathering during the inference phase. As a first to support view I could see storing them as notes on the message but I don't think we should land them as an alternative value collected like this option suggests would be possible.
While is suspect the intent of making this a list is to gather both the option to not include messages becomes implied as valid, I am suggesting that should not be the case.
| "uri": None, | ||
| "instructions": None, | ||
| "tools": [], | ||
| "max_output_tokens": 1024, |
There was a problem hiding this comment.
This value should map to max_tokens. Since this generator is targeting the responses API this class should promote the garak standard generator option of max_tokens to be max_output_tokens during __init__ of this class instead of exposing max_output_tokens.
I understand this diverges from how OpenAIReasoningGenerator handled max_completion_tokens, however the evolution of how max_tokens is treated needs to standardize.
| super().__init__(self.name, config_root=config_root) | ||
|
|
||
| @staticmethod | ||
| def _build_input(prompt: Union[Conversation, str]): |
There was a problem hiding this comment.
No need to support string here, all prompts must be of type Conversation at this time.
| def _build_input(prompt: Union[Conversation, str]): | |
| def _build_input(prompt: Conversation): |
| if item_type == "message": | ||
| for part in item.content: | ||
| if getattr(part, "type", None) == "output_text": | ||
| text_parts.append(part.text) | ||
| elif item_type == "reasoning": | ||
| for summary in getattr(item, "summary", []): | ||
| if getattr(summary, "type", None) == "summary_text": | ||
| text_parts.append(summary.text) |
There was a problem hiding this comment.
Merging message text and reasoning text without may marker to delineate between them is not viable. Reasoning either needs to be stored separately or enclosed in something like the <think></think> tag from legacy APIs to ensure garak can properly exclude reasoning from the responses.
| instructions = self.instructions or self._extract_system_prompt(prompt) | ||
| create_args = { | ||
| "model": self.name, | ||
| "input": self._build_input(prompt), | ||
| "max_output_tokens": self.max_output_tokens, | ||
| } |
There was a problem hiding this comment.
While I understand the format is different here, _extract_system_prompt and _build_input do not take into account additional input modalities supported by the Responses API that are already incorporated in OpenAICompatible._conversation_to_list(). This can be refactored to share that ETL process and enable this generator to support the additional input modalities.
| self.key_env_var = self.ENV_VAR | ||
| self._load_unsafe() | ||
| super().__init__(self.name, config_root=config_root) |
There was a problem hiding this comment.
The Configurable class handles ENV_VAR support no need set here. Also _load_unsafe() should be called after all initialization and validation has completed.
| self.key_env_var = self.ENV_VAR | |
| self._load_unsafe() | |
| super().__init__(self.name, config_root=config_root) | |
| super().__init__(self.name, config_root=config_root) | |
| self._load_unsafe() |
2 participants
Implements a new OpenAI compatible generator
OpenAIResponsesGeneratorthat uses the responses API instead ofChatCompletion.This started as way to run Agent Breaker against agents that support the responses API, but I think it's valuable as a standalone
Generatorfor any probe.I'm currently storing the returned tool calls as
Messagenotes, mainly so that I can do manual inspection of the attempts in thereport.jsonlfile. Ideally, I think there should be a generic mechanism for storing those so that detectors can pick those up properly. Probably outside the scope for this PR, just wanted to know if you already have a direction for integrating tool calling into Garak.Verification
The new Generator can be used like others OpenAI compatible generators, ex: