feature/FIRE 779 get the api key from env#46
Conversation
WalkthroughIntroduces job_id propagation through the CLI evaluation and reporting flow, adds an optional qualifire_api_key to CLI input, and adds environment-variable fallback for the Qualifire API key in server handlers. Also updates a doc note about a parameter type in evaluator_agent. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant CLI as run_cli
participant Eval as run_scenarios/_run_scenarios_with_sdk
participant S as Rogue Server
participant SDK as SDK.generate_summary
participant R as create_report
U->>CLI: Invoke evaluation
CLI->>Eval: run_scenarios(...)
Eval->>S: Start evaluation (scenarios, config)
S-->>Eval: results + job_id
Eval-->>CLI: (results, job_id)
alt results present
CLI->>R: create_report(..., job_id)
R->>SDK: generate_summary(..., job_id)
SDK-->>R: report string
R-->>CLI: report path
else no results
CLI-->>U: No results (None, None)
end
sequenceDiagram
autonumber
participant C as Client
participant API as server/api/llm.generate_summary
participant ENV as Process Env
participant Q as QualifireService
C->>API: Request (qualifire_api_key? , job_id?)
alt qualifire_api_key missing
API->>ENV: Read QUALIFIRE_API_KEY
ENV-->>API: key (or null)
API->>API: request.qualifire_api_key = env key if available
end
API->>Q: report_summary(..., qualifire_api_key, job_id)
Q-->>API: summary
API-->>C: summary
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| rogue_server_url=args.rogue_server_url, | ||
| judge_llm=cli_input.judge_llm, | ||
| results=results, | ||
| job_id=job_id, |
There was a problem hiding this comment.
Bug: API Key Ignored in Report Creation
The qualifire_api_key from CLIInput isn't passed to the create_report function. This causes any Qualifire API key provided via the CLI to be ignored, preventing its use by the server.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rogue/run_cli.py (1)
364-374: Thread Qualifire API key from CLI/config into create_report.
CLIInputnow hasqualifire_api_key, but it isn’t passed here. Without server env, remote servers won’t get the key.Apply this diff to pass it through:
report_summary = await create_report( rogue_server_url=args.rogue_server_url, judge_llm=cli_input.judge_llm, results=results, job_id=job_id, output_report_file=cli_input.output_report_file, judge_llm_api_key_secret=cli_input.judge_llm_api_key, + qualifire_api_key_secret=cli_input.qualifire_api_key, deep_test_mode=cli_input.deep_test_mode, judge_model=cli_input.judge_llm, )
🧹 Nitpick comments (3)
rogue/evaluator_agent/evaluator_agent.py (1)
113-119: Align _send_message_to_evaluated_agent return contract with docs.Docs say
"response"is the other agent's text, but the function returns the full JSON (model_dump_json()). Recommend returningagent_response_textand, if needed, include raw JSON under a different key.Apply this diff:
- return {"response": response.model_dump_json()} + return {"response": agent_response_text, "raw": response.model_dump_json()}Also applies to: 588-589
rogue/server/api/llm.py (1)
100-104: Good: env fallback added. Also guard /report_summary when key missing.Env fallback looks right and avoids logging the secret. In
/report_summary, if the key is still absent after fallback, return 400 instead of failing inside the service.Apply this diff within
report_summary_handlerafter the fallback:if not request.qualifire_api_key: - env_api_key = os.getenv("QUALIFIRE_API_KEY") - if env_api_key: - request.qualifire_api_key = env_api_key + env_api_key = os.getenv("QUALIFIRE_API_KEY") + if env_api_key: + request.qualifire_api_key = env_api_key + if not request.qualifire_api_key: + raise HTTPException( + status_code=400, + detail="Missing Qualifire API key. Provide it in the request or set QUALIFIRE_API_KEY on the server.", + )Also applies to: 180-184
rogue/run_cli.py (1)
16-86: Add a CLI flag for the Qualifire API key (with env-based guidance).Expose
--qualifire-api-keyto allow passing the key when the server env isn’t available.Add this argument in
set_cli_args:parser.add_argument( "--qualifire-api-key", required=False, help="Qualifire API key. If omitted, the server will use its QUALIFIRE_API_KEY environment variable when available.", )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
rogue/evaluator_agent/evaluator_agent.py(1 hunks)rogue/models/cli_input.py(1 hunks)rogue/run_cli.py(7 hunks)rogue/server/api/llm.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{rogue,tests,examples}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
{rogue,tests,examples}/**/*.py: Format Python code with Black
Organize imports according to isort conventions
Use type hints for all function signatures
Follow PEP 8 naming (snake_case for variables and functions, PascalCase for classes)
Use try/except blocks for code that may raise exceptions
Ensure code passes flake8 linting
Ensure code passes mypy type checks per .mypy.ini
Address Bandit-reported security issues in Python code
Files:
rogue/evaluator_agent/evaluator_agent.pyrogue/models/cli_input.pyrogue/server/api/llm.pyrogue/run_cli.py
rogue/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Place application/package code under the rogue/ directory
Files:
rogue/evaluator_agent/evaluator_agent.pyrogue/models/cli_input.pyrogue/server/api/llm.pyrogue/run_cli.py
🧬 Code graph analysis (2)
rogue/server/api/llm.py (2)
sdks/python/rogue_sdk/client.py (1)
request(79-83)packages/sdk/src/client.ts (1)
request(31-79)
rogue/run_cli.py (1)
sdks/python/rogue_sdk/types.py (1)
EvaluationResults(192-208)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: codestyle
- GitHub Check: Cursor Bugbot
🔇 Additional comments (2)
rogue/run_cli.py (1)
99-100: LGTM: job_id propagation end-to-end.Tuple returns, wiring, and SDK call updates are coherent and backwards-safe for callers that now unpack
(results, job_id).Also applies to: 126-137, 176-180, 190-191, 221-221, 347-358, 369-369
rogue/models/cli_input.py (1)
18-18: Incorrect — no change required: qualifire_api_key already present on CLIInput.
Field is defined in rogue/models/cli_input.py (line ~18) and is propagated/used across run_cli.py, server/api/llm.py and the SDK; the suggested diff is unnecessary.Likely an incorrect or invalid review comment.
| - "response": A string containing the other agent's response. If there is no response from the other agent, the string is empty. | ||
|
|
||
| 3. `_log_evaluation(scenario: dict, context_id: str, evaluation_passed: bool, reason: str)` | ||
| 3. `_log_evaluation(scenario: dict, context_id: str, evaluation_passed: bool, reason: str)` NOTE: THE SCENARIO IS A DICTIONARY NOT A STRING |
There was a problem hiding this comment.
Fix tool doc/signature mismatch for _log_evaluation (scenario_type).
The tool doc omits scenario_type, but the function requires it. This will cause tool-call validation failures. Either document the param as optional or make it optional in code.
Apply this doc tweak:
-3. `_log_evaluation(scenario: dict, context_id: str, evaluation_passed: bool, reason: str)` NOTE: THE SCENARIO IS A DICTIONARY NOT A STRING
+3. `_log_evaluation(scenario: dict, context_id: str, evaluation_passed: bool, reason: str, scenario_type: Optional[str] = None)` NOTE: THE SCENARIO IS A DICTIONARY NOT A STRINGAnd make the code accept it as optional (outside this hunk):
# Change signature to:
def _log_evaluation(
self,
scenario: dict[str, str],
context_id: str,
evaluation_passed: bool,
reason: str,
scenario_type: Optional[str] = None,
) -> None:
...🤖 Prompt for AI Agents
In rogue/evaluator_agent/evaluator_agent.py around line 120, the tool doc and
function signature for _log_evaluation are out of sync: the doc omits
scenario_type while the function requires it, and the doc incorrectly states
scenario is a string (it’s a dict). Update the docstring to document scenario as
dict[str, str] and add an optional scenario_type parameter, and change the
function signature to accept scenario_type: Optional[str] = None so tool-call
validation passes; ensure all internal uses handle scenario_type possibly being
None.
3 participants
Summary by CodeRabbit