feat: Allow for multiple self-check rails#1874
Conversation
7117521 to
eaa87eb
Compare
📝 WalkthroughWalkthroughThis PR extends self-check rails to support multiple sequential checks with parameterized task selection. Input and output check actions now accept string task parameters with placeholder mapping to defaults; flows pass task parameters from configuration; and tests validate multi-rail execution and backward compatibility. ChangesMultiple Self-Check Rails Feature
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_multiple_self_check_rails.py (1)
75-75: ⚡ Quick winStrengthen the happy-path input test assertion.
test_multiple_input_rails_both_passshould assert the returned content too, not just role, to catch regressions where the wrong assistant output is produced.✅ Suggested test hardening
def test_multiple_input_rails_both_pass(): @@ new_message = rails.generate(messages=[{"role": "user", "content": "hello"}]) assert new_message["role"] == "assistant" + assert new_message["content"] == "Hey!"🤖 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 `@tests/test_multiple_self_check_rails.py` at line 75, test_multiple_input_rails_both_pass currently only asserts new_message["role"]; add a content assertion to lock the happy-path output by comparing new_message["content"] (or new_message["content"]["parts"][0] if your message payload uses parts) to an expected string defined in the test (e.g., expected_response) and keep the existing role check; update the test function test_multiple_input_rails_both_pass to include that additional assertion so regressions in assistant output are caught.
🤖 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 `@tests/test_multiple_self_check_rails.py`:
- Line 75: test_multiple_input_rails_both_pass currently only asserts
new_message["role"]; add a content assertion to lock the happy-path output by
comparing new_message["content"] (or new_message["content"]["parts"][0] if your
message payload uses parts) to an expected string defined in the test (e.g.,
expected_response) and keep the existing role check; update the test function
test_multiple_input_rails_both_pass to include that additional assertion so
regressions in assistant output are caught.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 62074a50-d213-4cbd-acf8-fe8fc813fd7d
📒 Files selected for processing (8)
docs/configure-rails/guardrail-catalog/self-check.mdnemoguardrails/library/self_check/input_check/actions.pynemoguardrails/library/self_check/input_check/flows.conemoguardrails/library/self_check/input_check/flows.v1.conemoguardrails/library/self_check/output_check/actions.pynemoguardrails/library/self_check/output_check/flows.conemoguardrails/library/self_check/output_check/flows.v1.cotests/test_multiple_self_check_rails.py
Greptile SummaryThis PR consolidates the
|
| Filename | Overview |
|---|---|
| nemoguardrails/library/self_check/message_check/actions.py | New unified self_check module; minor issues: wrong default task in internal self_check() and missing space in ValueError message. |
| nemoguardrails/library/self_check/message_check/flows.co | Colang v2 flows for both input and output rails with correct default parameter values; symmetric abort behavior. |
| nemoguardrails/library/self_check/message_check/flows.v1.co | Colang v1 flows correctly handle unset context variables via explicit null-guard before using default task name. |
| tests/test_multiple_self_check_rails.py | Thorough test coverage: sequential blocking, early-abort, per-task LLM dispatch, fallback chain ordering, and backward-compat default task. |
| docs/configure-rails/guardrail-catalog/self-check.md | Documentation added for multiple input/output rail configuration with Colang v1 global-variable warning and example configs. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User Message] --> B[self check input flow 1\n$input_task=check_harmful]
B --> C{SelfCheckInputAction\ntask=check_harmful}
C -->|allowed=True| D[self check input flow 2\n$input_task=check_off_topic]
C -->|allowed=False| E{enable_rails_exceptions?}
E -->|Yes| F[send InputRailException]
E -->|No| G[bot refuse to respond]
F --> H[abort]
G --> H
D --> I{SelfCheckInputAction\ntask=check_off_topic}
I -->|allowed=True| J[Continue to LLM]
I -->|allowed=False| E2{enable_rails_exceptions?}
E2 -->|Yes| F2[send InputRailException]
E2 -->|No| G2[bot refuse to respond]
F2 --> H2[abort]
G2 --> H2
subgraph _get_llm fallback
K[task in llms?] -->|Yes| L[use task model]
K -->|No| M[default_task in llms?]
M -->|Yes| N[use default_task model]
M -->|No| O[default_llm provided?]
O -->|Yes| P[use main llm]
O -->|No| Q[raise ValueError]
end
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
nemoguardrails/library/self_check/message_check/actions.py:56-65
The internal `self_check` helper defaults `task` to `DEFAULT_OUTPUT_TASK` even though it handles both input and output paths. This default is never reached in practice (both public callers always forward their own `task` argument), but if `self_check` were ever invoked directly without an explicit `task` in a `USER_MESSAGE` context, the wrong default would be used to look up the prompt and LLM, silently producing incorrect behaviour.
```suggestion
async def self_check(
llms: Dict[str, LLMModel],
message_type: str,
llm_task_manager: LLMTaskManager,
default_llm: Optional[LLMModel] = None,
context: Optional[dict] = None,
config: Optional[RailsConfig] = None,
task: Optional[str] = None,
**kwargs,
):
```
### Issue 2 of 2
nemoguardrails/library/self_check/message_check/actions.py:49-52
Two adjacent f-string literals are concatenated without a separating space, so the rendered message reads `"…found.Please configure…"` with no space after the period.
```suggestion
error_msg = (
f"No matching model for task={task} found. "
f"Please configure a model with type={task}, type={default_task}, or type=main"
)
```
Reviews (6): Last reviewed commit: "Overhaul self-check flows: unify action ..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
aea2848 to
46bc7f6
Compare
Documentation preview |
46bc7f6 to
75a5219
Compare
…guration, allow for multiple self check tasks
75a5219 to
4fabed6
Compare
Description
Adds an optional
$input_taskand$output_taskvariable to the self_check rails, that allows for specification of multiple sequential self-check prompts in a configuration, e.g.:This PR also:
content_safetyrailRelated Issue(s)
Addresses #1873
Checklist
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests