FIRE-1164 | Rogue | OpenAI API Transport#159
Merged
Conversation
Contributor
Summary by CodeRabbit
WalkthroughAdds an OpenAI API (chat completions) protocol and transport, a T-shirt store chat-completions example (ShirtifyAgent + FastAPI CLI), OpenAI-based evaluator and red-team agents using litellm, SDK/TUI enum updates, CLI ping and example-launch wiring, and VSCode debug configurations. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant EvalAgent as OpenAI API<br/>Evaluator Agent
participant ChatHistory as Chat History<br/>(per context_id)
participant litellm
participant EvaluatedAgent as Evaluated Agent<br/>(OpenAI API)
Client->>EvalAgent: _send_message_to_evaluated_agent(context_id, message)
EvalAgent->>ChatHistory: Append user message
EvalAgent->>EvalAgent: _invoke_openai_api_agent(context_id, message)
EvalAgent->>ChatHistory: Reconstruct messages for context_id
EvalAgent->>litellm: acompletion(model, messages, timeout=30s)
litellm->>EvaluatedAgent: POST /chat/completions
EvaluatedAgent-->>litellm: assistant response
litellm-->>EvalAgent: {'choices':[{'message':{'content':...}}]}
EvalAgent->>ChatHistory: Append assistant response
EvalAgent-->>Client: {'response': assistant_message}
sequenceDiagram
participant Client
participant RedTeam as OpenAI API<br/>Red Team Agent
participant MsgHistory as Session Message<br/>History
participant litellm
participant EvaluatedAgent as Evaluated Agent<br/>(OpenAI API)
Client->>RedTeam: _send_message_to_evaluated_agent(message, session_id)
RedTeam->>MsgHistory: Append user message
RedTeam->>litellm: acompletion(model='openai/evaluated-agent', messages, headers)
litellm->>EvaluatedAgent: POST with message history
EvaluatedAgent-->>litellm: Assistant response
litellm-->>RedTeam: {'choices':[{'message':{'content':...}}]}
RedTeam->>MsgHistory: Append assistant response
RedTeam-->>Client: Response content
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
rogue/evaluator_agent/red_team/openai_api_red_team_attacker_agent.py (1)
14-17: Consider explicit constructor signature instead of*args, **kwargs.Using
*args, **kwargsobscures the expected parameters and bypasses IDE/type-checker validation. The evaluator agent counterpart (OpenAIAPIEvaluatorAgent) explicitly declares all parameters. For consistency and maintainability, consider mirroring that approach here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rogue/evaluator_agent/red_team/openai_api_red_team_attacker_agent.py` around lines 14 - 17, The constructor for OpenAIAPIRedTeamAttackerAgent currently uses a generic signature (__init__(self, *args, **kwargs)) which hides expected parameters; change it to an explicit signature mirroring OpenAIAPIEvaluatorAgent (declare the same named parameters like model_name, api_key, logger, etc. used by the evaluator counterpart) and pass them to super().__init__(...) so type-checkers and IDEs can validate usage; keep the existing initialization of self._context_id_to_messages intact and ensure any default values from OpenAIAPIEvaluatorAgent are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/openai_api/chat_completions_agent/__main__.py`:
- Around line 11-18: The startup call to uvicorn.run in main lacks error
handling; wrap the uvicorn.run(...) call inside a try/except in the main
function so any exceptions during server startup are caught, logged via
logger.exception (or logger.error with exception info) and the process exits
with a non-zero status (e.g., sys.exit(1)); reference the main function and the
uvicorn.run invocation when applying this change and import sys if needed.
In `@examples/openai_api/chat_completions_agent/chat_completions_wrapper.py`:
- Around line 17-36: The endpoint chat_completions should have explicit type
hints and robust error handling: change the signature to accept request:
Dict[str, Any] and annotate the return type (e.g., Dict[str, Any] or Response),
wrap the call to get_agent().invoke(messages) in a try/except that catches
Exception, logs or records the error, and returns/raises an HTTP 500 response
(use HTTPException) with a safe error message if invocation fails; ensure the
successful response still returns the same structure (id, object, created,
model, choices) and that you extract messages = request.get("messages", [])
before invoking the agent.
In `@examples/openai_api/chat_completions_agent/shirtify_agent.py`:
- Around line 89-100: The invoke method currently calls self.graph.invoke(...)
and self.get_agent_response(...) unguarded; wrap both calls in a try/except
around the sequence in invoke (after building RunnableConfig and before
returning) to catch any Exception, log or record the error, and return a
controlled Dict[str, Any] error payload (e.g., {"error": "message", "details":
...}) that matches the method's return type; ensure session_id handling and
RunnableConfig creation remain unchanged and only exceptions from graph.invoke
and get_agent_response are caught and converted to the safe response.
- Around line 65-87: The constructor logic in ShirtifyAgent.__init__ incorrectly
requires reasoning_effort to be truthy before instantiating ChatOpenAI, which
causes a raw string to be passed as model when model_name starts with "openai:"
but reasoning_effort is None; change the conditional to check only
model_name.startswith("openai:") and always construct ChatOpenAI (passing
reasoning_effort even if None) so create_agent receives a proper model instance
(reference: model_name, reasoning_effort, ChatOpenAI, create_agent); also wrap
the agent invoke() call in a try/except around the invoke() call to catch and
log/handle exceptions per guidelines.
In `@rogue/evaluator_agent/openai_api/openai_api_evaluator_agent.py`:
- Around line 135-136: The code assumes response.choices has at least one entry
when doing assistant_message = response.choices[0].message.content; add a
defensive check before that access (in the function handling the OpenAI response
in openai_api_evaluator_agent.py) to verify response.choices is truthy/has
length > 0, log or raise a clear error via the existing logger, and set
assistant_message to a safe default (e.g., empty string) if choices is empty to
avoid IndexError; update any downstream logic that expects a non-empty message
to handle the default path consistently.
- Around line 19-46: Add explicit AWS credential parameters to the __init__
signature of OpenAIApiEvaluatorAgent (and mirror the same change in
MCPEvaluatorAgent): include judge_llm_aws_access_key_id: Optional[str] = None,
judge_llm_aws_secret_access_key: Optional[str] = None, and judge_llm_aws_region:
Optional[str] = None, and pass them into the super().__init__(...) call so the
base class receives judge_llm_aws_access_key_id=judge_llm_aws_access_key_id,
judge_llm_aws_secret_access_key=judge_llm_aws_secret_access_key, and
judge_llm_aws_region=judge_llm_aws_region (keep other args the same and preserve
existing *args/**kwargs handling).
In `@rogue/evaluator_agent/red_team/openai_api_red_team_attacker_agent.py`:
- Around line 74-75: Add a defensive check before indexing response.choices:
verify that response and response.choices exist and that len(response.choices) >
0; if not, set assistant_message = "" (or log/error-handle) instead of accessing
response.choices[0].message.content. Update the code around the
assistant_message assignment (referencing the assistant_message variable and
response object) to use this guard so an empty choices array does not raise an
IndexError.
- Around line 66-72: The acompletion call is passing authentication via
headers=headers which litellm ignores; change the parameter to extra_headers so
the auth headers are sent: update the call to acompletion(...) in
function/method using self._context_id_to_messages[context_id] and
self._evaluated_agent_address to use extra_headers=headers (or
extra_headers=self._headers if that matches this class) instead of
headers=headers so the constructed headers are applied.
---
Nitpick comments:
In `@rogue/evaluator_agent/red_team/openai_api_red_team_attacker_agent.py`:
- Around line 14-17: The constructor for OpenAIAPIRedTeamAttackerAgent currently
uses a generic signature (__init__(self, *args, **kwargs)) which hides expected
parameters; change it to an explicit signature mirroring OpenAIAPIEvaluatorAgent
(declare the same named parameters like model_name, api_key, logger, etc. used
by the evaluator counterpart) and pass them to super().__init__(...) so
type-checkers and IDEs can validate usage; keep the existing initialization of
self._context_id_to_messages intact and ensure any default values from
OpenAIAPIEvaluatorAgent are preserved.
rogue/evaluator_agent/red_team/openai_api_red_team_attacker_agent.py
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rogue/evaluator_agent/openai_api/openai_api_evaluator_agent.py`:
- Around line 157-168: The exception handler in openai_api_evaluator_agent.py
logs self._transport.value without null-safety, which can raise an
AttributeError and mask the original exception; update the logger.exception call
in the except block (around the handler that catches Exception as e) to safely
read the transport by using a null-safe access (e.g., transport_value =
self._transport.value if self._transport is not None else None or use
getattr(self._transport, "value", None)) and include that transport_value in the
extra dict instead of self._transport.value so the original error is preserved.
In `@rogue/run_cli.py`:
- Around line 471-504: The ping_openai_api_server function currently does
nothing when transport != Transport.CHAT_COMPLETIONS; add an explicit else
branch that raises a ValueError for unsupported transports (matching behavior in
ping_mcp_server and get_a2a_agent_card). Locate ping_openai_api_server and after
the Transport.CHAT_COMPLETIONS handling, raise a descriptive ValueError that
includes the unsupported transport value and the agent_url to aid debugging.
---
Duplicate comments:
In `@rogue/evaluator_agent/openai_api/openai_api_evaluator_agent.py`:
- Around line 139-140: The code currently assumes response.choices[0] exists
when computing assistant_message, which can raise IndexError if the API returns
an empty choices list; update the logic around where assistant_message is set
(in openai_api_evaluator_agent.py, near the assignment to assistant_message) to
first check that response.choices is truthy and has at least one element, then
extract response.choices[0].message.content (or fallback to an empty string); if
choices is empty, set assistant_message to "" and optionally log a warning or
include the raw response for debugging. Ensure this guard is applied before any
access to response.choices[0] to prevent exceptions.
- Around line 19-50: The __init__ of OpenAIApiEvaluatorAgent swallows AWS
credential args in **kwargs so the factory-provided judge_llm_aws_access_key_id,
judge_llm_aws_secret_access_key, and judge_llm_aws_region never reach the base
class; update the __init__ signature to explicitly accept
judge_llm_aws_access_key_id, judge_llm_aws_secret_access_key, and
judge_llm_aws_region (defaulting to Optional[str]=None) and pass them through in
the super().__init__ call (alongside the existing params) so the base class
receives those credentials.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
rogue/evaluator_agent/red_team/openai_api_red_team_attacker_agent.py (1)
99-101: Unreachable dead code can be removed.The condition at line 75 already returns early if
response.choices[0].message.contentis falsy. Sinceassistant_messageis assigned from that value at line 84, it's guaranteed to be truthy at line 99, making this check unreachable.♻️ Proposed fix
logger.debug( "Received response from agent", extra={ "has_response": bool(assistant_message), "response_length": len(assistant_message), }, ) - if not assistant_message: - logger.warning("No response from agent") - return "No response from agent" - return assistant_message🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rogue/evaluator_agent/red_team/openai_api_red_team_attacker_agent.py` around lines 99 - 101, Remove the unreachable defensive check that tests assistant_message after it was already returned-for when falsy: delete the "if not assistant_message: logger.warning(...); return ..." block (the branch that inspects assistant_message, which is assigned from response.choices[0].message.content) so the code does not contain dead code; if you prefer to keep a defensive assertion, replace it with an assert or a single-line guard earlier where response is read, but do not keep the unreachable logger.warning/return branch referencing assistant_message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rogue/evaluator_agent/openai_api/openai_api_evaluator_agent.py`:
- Around line 89-97: The logger call accesses self._transport.value but
self._transport is Optional[Transport]; update the logging to be null-safe by
resolving the transport value before use (e.g., compute transport_value =
self._transport.value if self._transport is not None else a sensible default
like None/"unknown") and pass transport_value into the logger.extra instead of
directly using self._transport.value; update the logger.info invocation in the
method containing the shown block accordingly to avoid AttributeError.
---
Duplicate comments:
In `@rogue/evaluator_agent/openai_api/openai_api_evaluator_agent.py`:
- Around line 176-187: The exception handler in the logger.exception call uses
self._transport.value without null-safety which can raise AttributeError and
hide the original exception; update that logger call (in the except block inside
the OpenAI API evaluator agent) to safely read transport by checking
self._transport (e.g. use a conditional or helper like transport_value =
self._transport.value if self._transport else None) and pass transport_value
instead of self._transport.value so the original exception isn't masked.
---
Nitpick comments:
In `@rogue/evaluator_agent/red_team/openai_api_red_team_attacker_agent.py`:
- Around line 99-101: Remove the unreachable defensive check that tests
assistant_message after it was already returned-for when falsy: delete the "if
not assistant_message: logger.warning(...); return ..." block (the branch that
inspects assistant_message, which is assigned from
response.choices[0].message.content) so the code does not contain dead code; if
you prefer to keep a defensive assertion, replace it with an assert or a
single-line guard earlier where response is read, but do not keep the
unreachable logger.warning/return branch referencing assistant_message.
Comment on lines
+89
to
+97
| logger.info( | ||
| "🔗 Making OpenAI API call to evaluated agent", | ||
| extra={ | ||
| "message": message[:100] + "..." if len(message) > 100 else message, | ||
| "context_id": context_id, | ||
| "agent_url": self._evaluated_agent_address, | ||
| "transport": self._transport.value, | ||
| }, | ||
| ) |
Contributor
There was a problem hiding this comment.
Add null-safety for transport access.
transport is declared as Optional[Transport] (line 21), so self._transport can be None. Accessing .value at line 95 will raise AttributeError if transport is None.
🛡️ Proposed fix
logger.info(
"🔗 Making OpenAI API call to evaluated agent",
extra={
"message": message[:100] + "..." if len(message) > 100 else message,
"context_id": context_id,
"agent_url": self._evaluated_agent_address,
- "transport": self._transport.value,
+ "transport": self._transport.value if self._transport else None,
},
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| logger.info( | |
| "🔗 Making OpenAI API call to evaluated agent", | |
| extra={ | |
| "message": message[:100] + "..." if len(message) > 100 else message, | |
| "context_id": context_id, | |
| "agent_url": self._evaluated_agent_address, | |
| "transport": self._transport.value, | |
| }, | |
| ) | |
| logger.info( | |
| "🔗 Making OpenAI API call to evaluated agent", | |
| extra={ | |
| "message": message[:100] + "..." if len(message) > 100 else message, | |
| "context_id": context_id, | |
| "agent_url": self._evaluated_agent_address, | |
| "transport": self._transport.value if self._transport else None, | |
| }, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rogue/evaluator_agent/openai_api/openai_api_evaluator_agent.py` around lines
89 - 97, The logger call accesses self._transport.value but self._transport is
Optional[Transport]; update the logging to be null-safe by resolving the
transport value before use (e.g., compute transport_value =
self._transport.value if self._transport is not None else a sensible default
like None/"unknown") and pass transport_value into the logger.extra instead of
directly using self._transport.value; update the logger.info invocation in the
method containing the shown block accordingly to avoid AttributeError.
drorIvry
approved these changes
Feb 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Motivation and Context
Type of Change
Changes Made
Screenshots/Examples (if applicable)
Checklist
uv run black .to format my codeuv run flake8 .and fixed all issuesuv run mypy --config-file .mypy.ini .and addressed type checking issuesuv run bandit -c .bandit.yaml -r .for security checksuv run pytestand all tests passTesting
Test Configuration:
Test Steps:
1.
2.
3.
Additional Notes
Related Issues/PRs