test(guardrails): Add e2e tests for config caching and various rail types#740
Open
JashG wants to merge 5 commits into
Open
test(guardrails): Add e2e tests for config caching and various rail types#740JashG wants to merge 5 commits into
JashG wants to merge 5 commits into
Conversation
…ypes Signed-off-by: Jash Gulabrai <jgulabrai@nvidia.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughGuardrails E2E utilities now use explicit request parameters and lifecycle helpers. New tests cover multiple rail types and real background refresh behavior. Middleware state is populated before blocked responses, and existing non-streaming tests use a centralized request helper. ChangesGuardrails E2E validation
Sequence Diagram(s)sequenceDiagram
participant Test
participant NeMoPlatform
participant Gateway
participant GuardrailsMiddleware
participant GuardrailConfig
participant Backend
Test->>NeMoPlatform: configure guarded VirtualModel
Test->>Gateway: submit chat completion
Gateway->>GuardrailConfig: load Guardrails configuration
Gateway->>GuardrailsMiddleware: run input rails
GuardrailsMiddleware->>GuardrailsMiddleware: store input generation response
Gateway->>Backend: request model response
Backend-->>Gateway: return response
Gateway-->>Test: return allowed or blocked result
Test->>NeMoPlatform: update or delete GuardrailConfig
Gateway->>GuardrailConfig: refresh configuration
Gateway-->>Test: reflect updated blocking or fail-closed state
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
🧹 Nitpick comments (1)
e2e/guardrails/utils.py (1)
239-252: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate blocked/activated-rails logic across files.
utils.pynow exportsis_chat_response_blocked/activated_rails_by_name, already adopted bytest_rail_types.pyandtest_config_cache_refresh.py, buttest_chat_completions.pystill keeps its own copies.
e2e/guardrails/utils.py#L239-L252: canonical implementation — no change needed here.e2e/guardrails/test_chat_completions.py#L50-L52: replace_assert_blocked'sfinish_reasoncheck withis_chat_response_blocked(response).e2e/guardrails/test_chat_completions.py#L67-L71: drop_activated_rails_by_nameand importactivated_rails_by_namefromutils.py.♻️ Proposed consolidation
-def _assert_blocked(response: dict[str, Any]) -> None: - assert response["choices"][0]["finish_reason"] == "content_filter" +def _assert_blocked(response: dict[str, Any]) -> None: + assert is_chat_response_blocked(response) assert response["choices"][0]["message"]["content"] == REFUSAL_TEXT-def _activated_rails_by_name(response: dict[str, Any]) -> dict[str, dict[str, Any]]: - guardrails_data = response.get("guardrails_data") or {} - log = guardrails_data.get("log") or {} - activated_rails = log.get("activated_rails") or [] - return {rail["name"]: rail for rail in activated_rails} +# use `activated_rails_by_name` imported from utils.py🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/guardrails/utils.py` around lines 239 - 252, The canonical implementations in e2e/guardrails/utils.py lines 239-252 require no direct change. In e2e/guardrails/test_chat_completions.py lines 50-52, update _assert_blocked to call is_chat_response_blocked(response) instead of checking finish_reason directly. In e2e/guardrails/test_chat_completions.py lines 67-71, remove _activated_rails_by_name and import and use activated_rails_by_name from utils.py.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@e2e/guardrails/utils.py`:
- Around line 239-252: The canonical implementations in e2e/guardrails/utils.py
lines 239-252 require no direct change. In
e2e/guardrails/test_chat_completions.py lines 50-52, update _assert_blocked to
call is_chat_response_blocked(response) instead of checking finish_reason
directly. In e2e/guardrails/test_chat_completions.py lines 67-71, remove
_activated_rails_by_name and import and use activated_rails_by_name from
utils.py.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 937014f6-913e-4daf-a00f-cb579082fcab
📒 Files selected for processing (4)
e2e/guardrails/test_chat_completions.pye2e/guardrails/test_config_cache_refresh.pye2e/guardrails/test_rail_types.pye2e/guardrails/utils.py
Contributor
|
Signed-off-by: Jash Gulabrai <jgulabrai@nvidia.com>
Signed-off-by: Jash Gulabrai <jgulabrai@nvidia.com>
Signed-off-by: Jash Gulabrai <jgulabrai@nvidia.com>
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
1 participant
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.
…ypes
Summary by CodeRabbit