Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughThis PR makes Qualifire configuration environment-driven: hardcoded Qualifire URLs are replaced with Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.
> [!CAUTION]
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)rogue/server/api/red_team.py (1)
313-316:⚠️ Potential issue | 🟠 MajorAlign report URL env var with
QUALIFIRE_BASE_URLto avoid mismatched links.
The default now pullsQUALIFIRE_BASE_URL, but the report URL generation still usesQUALIFIRE_URLat Line 313-316, so the returned link can point to a different host than the one used for reporting.🔧 Suggested fix
- base_url = os.getenv( - "QUALIFIRE_URL", - "https://app.qualifire.ai", - ) + base_url = os.getenv( + "QUALIFIRE_BASE_URL", + "https://app.qualifire.ai", + )Also applies to: 346-349
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rogue/server/api/red_team.py` around lines 313 - 316, The report URL generation currently reads os.getenv("QUALIFIRE_URL", "https://app.qualifire.ai") which mismatches the rest of the code that uses QUALIFIRE_BASE_URL; update both occurrences that set base_url (the assignments that use os.getenv("QUALIFIRE_URL", ...), around the base_url variable in red_team.py) to read os.getenv("QUALIFIRE_BASE_URL", "https://app.qualifire.ai") so the generated report links use the same QUALIFIRE_BASE_URL as reporting and avoid host mismatches.rogue/server/services/qualifire_service.py (1)
46-51:⚠️ Potential issue | 🟡 MinorAdd type hints for
report_red_team_scanparameters and return type.
This keeps the method compliant with the repository typing rule.🧩 Suggested update
-from typing import Optional +from typing import Any, Optional @@ -from rogue_sdk.types import EvaluationResults, ReportSummaryRequest +from rogue_sdk.types import EvaluationResults, ReportSummaryRequest, RedTeamJob @@ - def report_red_team_scan( - job, - report, - qualifire_api_key: str, - qualifire_url: Optional[str] = None, - ): + def report_red_team_scan( + job: RedTeamJob, + report: Any, + qualifire_api_key: str, + qualifire_url: Optional[str] = None, + ) -> dict[str, Any]:As per coding guidelines: Use type hints for all function signatures.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rogue/server/services/qualifire_service.py` around lines 46 - 51, Annotate the report_red_team_scan signature with explicit types and a return type (e.g. add "from typing import Any, Dict, Optional" if not present) and change the signature to something like: def report_red_team_scan(job: Any, report: Dict[str, Any], qualifire_api_key: str, qualifire_url: Optional[str] = None) -> Optional[Dict[str, Any]]; ensure you update any internal uses to satisfy the chosen types and adjust imports at the top of the module if needed.
🧹 Nitpick comments (2)
rogue/server/api/red_team.py (1)
346-349: Avoid import-time env capture forqualifire_urldefaults.
Usingos.getenvat class definition time freezes the value at import;default_factoryreads it when the model is instantiated.♻️ Recommended refactor
-from pydantic import BaseModel +from pydantic import BaseModel, Field @@ - qualifire_url: str = os.getenv( - "QUALIFIRE_BASE_URL", - "https://app.qualifire.ai", - ) + qualifire_url: str = Field( + default_factory=lambda: os.getenv( + "QUALIFIRE_BASE_URL", + "https://app.qualifire.ai", + ), + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rogue/server/api/red_team.py` around lines 346 - 349, The qualifire_url default is being captured at import time via os.getenv which freezes its value; change the Pydantic/Dataclass field qualifire_url to use a default_factory that calls os.getenv("QUALIFIRE_BASE_URL", "https://app.qualifire.ai") so the environment is read when the model is instantiated, and remove the module-level os.getenv usage to avoid import-time capture.sdks/python/rogue_sdk/types.py (1)
1012-1015: Avoid import-time env capture forqualifire_urldefaults.
Preferdefault_factoryso the env value is read when the model instance is created.♻️ Recommended refactor
- qualifire_url: Optional[str] = os.getenv( - "QUALIFIRE_BASE_URL", - "https://app.qualifire.ai", - ) + qualifire_url: Optional[str] = Field( + default_factory=lambda: os.getenv( + "QUALIFIRE_BASE_URL", + "https://app.qualifire.ai", + ), + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/python/rogue_sdk/types.py` around lines 1012 - 1015, The current code captures QUALIFIRE_BASE_URL at import time by assigning qualifire_url = os.getenv(...); change this to use a runtime default_factory so the environment is read when a model instance is created: replace the top-level assignment with a model field that uses default_factory (e.g., Field(default_factory=lambda: os.getenv("QUALIFIRE_BASE_URL", "https://app.qualifire.ai")) or dataclasses.field(default_factory=...) depending on the model type) for the qualifire_url field so the env var is resolved at instantiation instead of import.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@rogue/server/api/red_team.py`:
- Around line 313-316: The report URL generation currently reads
os.getenv("QUALIFIRE_URL", "https://app.qualifire.ai") which mismatches the rest
of the code that uses QUALIFIRE_BASE_URL; update both occurrences that set
base_url (the assignments that use os.getenv("QUALIFIRE_URL", ...), around the
base_url variable in red_team.py) to read os.getenv("QUALIFIRE_BASE_URL",
"https://app.qualifire.ai") so the generated report links use the same
QUALIFIRE_BASE_URL as reporting and avoid host mismatches.
In `@rogue/server/services/qualifire_service.py`:
- Around line 46-51: Annotate the report_red_team_scan signature with explicit
types and a return type (e.g. add "from typing import Any, Dict, Optional" if
not present) and change the signature to something like: def
report_red_team_scan(job: Any, report: Dict[str, Any], qualifire_api_key: str,
qualifire_url: Optional[str] = None) -> Optional[Dict[str, Any]]; ensure you
update any internal uses to satisfy the chosen types and adjust imports at the
top of the module if needed.
---
Nitpick comments:
In `@rogue/server/api/red_team.py`:
- Around line 346-349: The qualifire_url default is being captured at import
time via os.getenv which freezes its value; change the Pydantic/Dataclass field
qualifire_url to use a default_factory that calls
os.getenv("QUALIFIRE_BASE_URL", "https://app.qualifire.ai") so the environment
is read when the model is instantiated, and remove the module-level os.getenv
usage to avoid import-time capture.
In `@sdks/python/rogue_sdk/types.py`:
- Around line 1012-1015: The current code captures QUALIFIRE_BASE_URL at import
time by assigning qualifire_url = os.getenv(...); change this to use a runtime
default_factory so the environment is read when a model instance is created:
replace the top-level assignment with a model field that uses default_factory
(e.g., Field(default_factory=lambda: os.getenv("QUALIFIRE_BASE_URL",
"https://app.qualifire.ai")) or dataclasses.field(default_factory=...) depending
on the model type) for the qualifire_url field so the env var is resolved at
instantiation instead of import.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.vscode/launch.jsonVERSIONrogue/server/api/red_team.pyrogue/server/services/qualifire_service.pysdks/python/rogue_sdk/types.py
drorIvry
approved these changes
Feb 24, 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