refactor(iorails): Migrate from RailAction subclasses to CompiledRail manifest implementation - #2261
Open
tgasser-nv wants to merge 11 commits into
Open
Conversation
github-actions
Bot
added
size: XL
status: needs triage
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
tgasser-nv
added
status: triaged
tgasser-nv
marked this pull request as ready for review
August 6, 2026 16:45
Contributor
Greptile SummaryThe PR migrates supported IORails content-safety, topic-safety, and jailbreak checks from dedicated RailAction subclasses to manifest-backed CompiledRail execution.
|
| Filename | Overview |
|---|---|
| nemoguardrails/guardrails/compiled_rail.py | Adds manifest-based compilation, dependency injection, surface validation, model-call capture, and a complete fix for current-turn duplication and historical-turn reordering. |
| nemoguardrails/guardrails/rails_manager.py | Migrates supported input and output rails to CompiledRail execution and translates manifest outcomes into IORails results. |
| nemoguardrails/guardrails/iorails.py | Integrates the compiled-rail manager and updated execution, logging, lifecycle, and error-handling behavior. |
| nemoguardrails/library/topic_safety/actions.py | Reuses the manifest action for IORails, forwards configured token limits, and assembles prior history plus the checked user turn without duplication. |
| nemoguardrails/guardrails/rail_guard.py | Centralizes action failure conversion and rail outcome handling for the new execution path. |
| nemoguardrails/server/exception_handlers.py | Updates server-side exception translation and response-envelope handling for guardrail failures. |
Sequence Diagram
sequenceDiagram
participant Client
participant IORails
participant Manager as RailsManager
participant Rail as CompiledRail
participant Action as Manifest Action
participant Model
Client->>IORails: generate/check(messages)
IORails->>Manager: run configured rails
Manager->>Rail: execute(messages, bot_response)
Rail->>Rail: build context and prior-turn events
Rail->>Action: invoke injected manifest action
Action->>Model: optional safety-model/API call
Model-->>Action: verdict response
Action-->>Rail: RailOutcome
Rail-->>Manager: RailExecution
Manager-->>IORails: RailResult
IORails-->>Client: allowed, blocked, or error response
Reviews (8): Last reviewed commit: "Fix conversation-turn reordering bug" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Collaborator
Author
|
@coderabbitai Review this PR |
Contributor
|
✅ Action performedReview finished.
|
Contributor
📝 WalkthroughWalkthroughThe PR migrates rail execution to compiled rails, removes legacy rail actions and API-engine routing, centralizes surface validation, normalizes verdict display, updates HTTP error handling, and adds related tests and jailbreak fail-open documentation. ChangesCompiled rail execution
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1📝 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.
Actionable comments posted: 3
🧹 Nitpick comments (8)
nemoguardrails/guardrails/engine_registry.py (1)
44-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign all registry docstrings with the model-only contract.
Line 44 states that the registry manages model engines only. The following class and initializer text still states that it creates API engines and traces API calls. Remove those references.
Proposed documentation update
- Creates one engine per configured model or API service, keyed by name. + Creates one engine per configured model, keyed by model type. - """Build one engine per configured model and API service. + """Build one engine per configured model. - When *tracer* is provided, LLM and API calls produce OTEL spans; when + When *tracer* is provided, LLM calls produce OTEL spans; when🤖 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 `@nemoguardrails/guardrails/engine_registry.py` at line 44, Update the registry class and initializer docstrings near the model-only registry declaration to remove references to creating API engines and tracing API calls. Keep the documentation focused on managing ModelEngine instances for IORails, consistently with the module docstring and model-only contract.Source: Coding guidelines
tests/guardrails/test_compiled_rail.py (1)
373-382: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the deny-list cross-check bidirectional.
This test only verifies that each production deny-list entry exists in the catalog. It does not compare
_RETRIEVAL_CONTEXT_SURFACESwithRETRIEVAL_DEPENDENT_SURFACES. A new production refusal can therefore miss compilation coverage while this test still passes.Assert exact equality between the two sets before the catalog-membership check.
🤖 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/guardrails/test_compiled_rail.py` around lines 373 - 382, Update test_the_deny_list_names_only_real_surfaces to assert exact equality between _RETRIEVAL_CONTEXT_SURFACES and RETRIEVAL_DEPENDENT_SURFACES before checking catalog membership. Preserve the existing missing-surface validation after this bidirectional consistency check.tests/guardrails/test_iorails_streaming.py (1)
1155-1157: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
_assert_error_chunkhere too.This test indexes
error_chunks[0]directly. If no error chunk is emitted, the test fails with anIndexErrorinstead of a readable assertion. The output test at Line 1173 uses the_assert_error_chunkhelper. Use the same helper, or assert the list is non-empty first.🤖 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/guardrails/test_iorails_streaming.py` around lines 1155 - 1157, Update the test around _assert_error_chunk to use that helper instead of indexing error_chunks[0] directly, preserving the existing expected error message assertion while producing a readable failure when no error chunk is emitted.nemoguardrails/guardrails/compiled_rail.py (2)
287-295: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStale reference to the previous function name.
The comment in
_bind_parametersstill names_reject_unfillable_binding_kinds. This change renames the check to_unfillable_bindings_reason. Update that comment so the cross-reference resolves.🤖 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 `@nemoguardrails/guardrails/compiled_rail.py` around lines 287 - 295, Update the comment in _bind_parameters to reference _unfillable_bindings_reason instead of the removed _reject_unfillable_binding_kinds name, leaving the surrounding binding logic unchanged.
305-334: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftDeclare retrieval-context requirements in the manifest.
CompiledRail._request_dependenciesinjects only user and bot messages. A new action that reads retrieval context can therefore receive no evidence and still compile. Some such actions return an allow outcome when evidence is absent. Add a surface-level manifest requirement and deriveunsupported_surface_reasonfrom it instead of maintaining_RETRIEVAL_CONTEXT_SURFACES.🤖 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 `@nemoguardrails/guardrails/compiled_rail.py` around lines 305 - 334, Replace the hard-coded _RETRIEVAL_CONTEXT_SURFACES set and _retrieval_context_reason lookup with a manifest-declared retrieval-context requirement on each applicable rail surface. Update unsupported_surface_reason derivation to inspect that manifest metadata, so surfaces whose actions require retrieval evidence are rejected when CompiledRail._request_dependencies cannot provide it.nemoguardrails/guardrails/rails_manager.py (1)
77-80: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake the TRANSFORM case explicit in
_rail_result.
RailOutcomecarries three decisions.is_blockedis True only forBLOCK, so aTRANSFORMoutcome maps tois_safe=Trueandoutcome.transformsis dropped without a trace. Today_transform_target_reasonincompiled_rail.pyrefuses every transform-capable surface at compile time, so this path is unreachable. That guard is documented as temporary ("transforms in PR 5"), and when it is lifted this mapping fails open silently.Raise or log on a
TRANSFORMdecision so the gap surfaces at the moment the compile-time refusal is relaxed.🛡️ Proposed defensive mapping
def _rail_result(outcome: RailOutcome) -> RailResult: """Map an engine-neutral rail verdict onto IORails' rail result.""" + if outcome.is_transform: + # Transform-capable surfaces are refused at compile time; reaching here means + # that refusal was lifted without teaching IORails to apply the rewrite. + raise NotImplementedError("IORails cannot apply a TRANSFORM rail outcome yet") allowed = not outcome.is_blocked return RailResult(is_safe=allowed, reason=outcome.reason, return_value={"allowed": allowed, **outcome.metadata})🤖 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 `@nemoguardrails/guardrails/rails_manager.py` around lines 77 - 80, Update _rail_result to handle outcome.transforms explicitly before deriving the safe result: when a TRANSFORM decision is present, raise an error or emit a clear diagnostic instead of treating it as safe and discarding the transforms. Preserve the existing BLOCK/allow mapping for outcomes without transforms.nemoguardrails/guardrails/iorails.py (1)
618-624: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the redundant truthiness guard in the comprehension.
_get_flow_name(flow) or flowalready falls back to the raw flow string. The extraandguard only skips a flow whose normalized name and raw string are both empty, which then reachescompile_railinstead of being reported here. Use a plain assignment expression so the filter reads as one condition.♻️ Proposed simplification
out_of_scope = sorted( { name for flow in flows - if (name := _get_flow_name(flow) or flow) and (direction, name) not in cls._ENABLED_SURFACES + if ((name := _get_flow_name(flow) or flow), direction) and (direction, name) not in cls._ENABLED_SURFACES } )Or extract the normalization into a local helper and keep the comprehension condition single-purpose.
🤖 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 `@nemoguardrails/guardrails/iorails.py` around lines 618 - 624, Update the out_of_scope comprehension in the relevant class to remove the redundant truthiness check around the name assignment. Assign the normalized value from _get_flow_name(flow) or flow, then filter only on whether (direction, name) is absent from _ENABLED_SURFACES so empty names are still reported here.tests/guardrails/test_guardrails_types.py (1)
134-149: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a multi-key verdict case to cover the
"; "join.Every evidence case here carries one renderable key, so the
"; ".join(parts)separator in_verdict_evidenceis never exercised. That separator reaches client-facing violation payloads, so pin it.💚 Proposed additional case
( RailResult( is_safe=False, triggered_rail="content safety check input", return_value={"allowed": False, "policy_violations": ["S1: Violence", "S2: Sexual"]}, ), "policy_violations: S1: Violence, S2: Sexual", ), + ( + RailResult( + is_safe=False, + triggered_rail="content safety check input", + return_value={"allowed": False, "policy_violations": ["S1: Violence"], "score": 0.9}, + ), + "policy_violations: S1: Violence; score: 0.9", + ),Add a matching
"multi-key-evidence"entry toids.🤖 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/guardrails/test_guardrails_types.py` around lines 134 - 149, Add a multi-key verdict fixture in the guardrail evidence tests by adding a matching "multi-key-evidence" entry to ids and a corresponding RailResult whose return_value contains multiple renderable evidence keys. Set the expected verdict string to join those key-value parts with "; " so _verdict_evidence’s separator is exercised.
🤖 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.
Inline comments:
In `@nemoguardrails/guardrails/iorails.py`:
- Around line 1421-1425: Update display_reason() and the client-facing violation
payload paths around tool input, tool output, and other rail-block responses to
format reasons only from an explicit safe-field allowlist, excluding
user_message, bot_message, and unfiltered provider-result fields. Preserve the
complete rail metadata for internal logs and spans.
In `@nemoguardrails/guardrails/rails_manager.py`:
- Around line 180-185: The _rails map must distinguish direction-specific
compilations instead of keying only by flow. In
nemoguardrails/guardrails/rails_manager.py lines 180-185, key entries by
(direction, flow) and update the _run_rail lookup accordingly. In
tests/guardrails/test_rails_manager.py lines 185-193, update
test_rails_compiled_for_flows and test_nemoguards_rails_compiled to use tuple
keys, and add coverage for a surface listed in both input.flows and
output.flows.
In `@tests/guardrails/test_cross_engine_rail_equivalence.py`:
- Around line 281-282: Update the relevant test class docstring to state that
both engines use HTTPX for the shared jailbreak action, or remove the
transport-specific distinction; do not leave the outdated aiohttp description in
place.
---
Nitpick comments:
In `@nemoguardrails/guardrails/compiled_rail.py`:
- Around line 287-295: Update the comment in _bind_parameters to reference
_unfillable_bindings_reason instead of the removed
_reject_unfillable_binding_kinds name, leaving the surrounding binding logic
unchanged.
- Around line 305-334: Replace the hard-coded _RETRIEVAL_CONTEXT_SURFACES set
and _retrieval_context_reason lookup with a manifest-declared retrieval-context
requirement on each applicable rail surface. Update unsupported_surface_reason
derivation to inspect that manifest metadata, so surfaces whose actions require
retrieval evidence are rejected when CompiledRail._request_dependencies cannot
provide it.
In `@nemoguardrails/guardrails/engine_registry.py`:
- Line 44: Update the registry class and initializer docstrings near the
model-only registry declaration to remove references to creating API engines and
tracing API calls. Keep the documentation focused on managing ModelEngine
instances for IORails, consistently with the module docstring and model-only
contract.
In `@nemoguardrails/guardrails/iorails.py`:
- Around line 618-624: Update the out_of_scope comprehension in the relevant
class to remove the redundant truthiness check around the name assignment.
Assign the normalized value from _get_flow_name(flow) or flow, then filter only
on whether (direction, name) is absent from _ENABLED_SURFACES so empty names are
still reported here.
In `@nemoguardrails/guardrails/rails_manager.py`:
- Around line 77-80: Update _rail_result to handle outcome.transforms explicitly
before deriving the safe result: when a TRANSFORM decision is present, raise an
error or emit a clear diagnostic instead of treating it as safe and discarding
the transforms. Preserve the existing BLOCK/allow mapping for outcomes without
transforms.
In `@tests/guardrails/test_compiled_rail.py`:
- Around line 373-382: Update test_the_deny_list_names_only_real_surfaces to
assert exact equality between _RETRIEVAL_CONTEXT_SURFACES and
RETRIEVAL_DEPENDENT_SURFACES before checking catalog membership. Preserve the
existing missing-surface validation after this bidirectional consistency check.
In `@tests/guardrails/test_guardrails_types.py`:
- Around line 134-149: Add a multi-key verdict fixture in the guardrail evidence
tests by adding a matching "multi-key-evidence" entry to ids and a corresponding
RailResult whose return_value contains multiple renderable evidence keys. Set
the expected verdict string to join those key-value parts with "; " so
_verdict_evidence’s separator is exercised.
In `@tests/guardrails/test_iorails_streaming.py`:
- Around line 1155-1157: Update the test around _assert_error_chunk to use that
helper instead of indexing error_chunks[0] directly, preserving the existing
expected error message assertion while producing a readable failure when no
error chunk is emitted.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c2ca6314-fb58-4160-a167-21f8c5287add
📒 Files selected for processing (40)
docs/configure-rails/guardrail-catalog/jailbreak-protection.mdxnemoguardrails/guardrails/__init__.pynemoguardrails/guardrails/actions/content_safety_action.pynemoguardrails/guardrails/actions/jailbreak_detection_action.pynemoguardrails/guardrails/actions/topic_safety_action.pynemoguardrails/guardrails/api_engine.pynemoguardrails/guardrails/compiled_rail.pynemoguardrails/guardrails/engine_registry.pynemoguardrails/guardrails/guardrails_types.pynemoguardrails/guardrails/iorails.pynemoguardrails/guardrails/rail_action.pynemoguardrails/guardrails/rail_guard.pynemoguardrails/guardrails/rails_manager.pynemoguardrails/guardrails/telemetry.pynemoguardrails/guardrails/tool_rail_action.pynemoguardrails/library/topic_safety/actions.pynemoguardrails/server/api.pynemoguardrails/server/exception_handlers.pytests/guardrails/async_helpers.pytests/guardrails/test_api_engine.pytests/guardrails/test_compiled_rail.pytests/guardrails/test_content_safety_iorails_actions.pytests/guardrails/test_cross_engine_rail_equivalence.pytests/guardrails/test_engine_registry.pytests/guardrails/test_guardrails.pytests/guardrails/test_guardrails_types.pytests/guardrails/test_iorails_generation_log_capture.pytests/guardrails/test_iorails_streaming.pytests/guardrails/test_iorails_telemetry.pytests/guardrails/test_jailbreak_detection_iorails_actions.pytests/guardrails/test_rail_action.pytests/guardrails/test_rail_guard.pytests/guardrails/test_rails_manager.pytests/guardrails/test_telemetry_spans.pytests/guardrails/test_tool_rails_e2e.pytests/guardrails/test_topic_safety_iorails_actions.pytests/server/test_error_envelope_e2e.pytests/server/test_openai_integration.pytests/test_http_error_handling.pytests/test_topic_safety_actions.py
💤 Files with no reviewable changes (13)
- nemoguardrails/guardrails/telemetry.py
- tests/guardrails/test_content_safety_iorails_actions.py
- nemoguardrails/guardrails/actions/content_safety_action.py
- tests/guardrails/test_rail_action.py
- nemoguardrails/guardrails/actions/jailbreak_detection_action.py
- tests/guardrails/test_rail_guard.py
- tests/guardrails/test_topic_safety_iorails_actions.py
- nemoguardrails/guardrails/actions/topic_safety_action.py
- tests/guardrails/test_jailbreak_detection_iorails_actions.py
- nemoguardrails/guardrails/init.py
- nemoguardrails/guardrails/rail_action.py
- nemoguardrails/guardrails/api_engine.py
- tests/guardrails/test_api_engine.py
Comment on lines
+101
to
+113
| last_user_index = max((i for i, m in enumerate(messages) if m.get("role") == "user"), default=None) | ||
| for index, message in enumerate(messages): | ||
| content = message.get("content") | ||
| if not content: | ||
| continue | ||
| role = message.get("role") | ||
| if role == "user": | ||
| events.append({"type": _USER_MESSAGE_EVENT, "text": content}) | ||
| # The current turn is deliberately absent from the history, mirroring | ||
| # ``llmrails.py``, which emits ``UserMessage`` only for a user turn that is not the | ||
| # last. Actions append it themselves from ``context["user_message"]``, so emitting | ||
| # it here too would hand the model the same turn twice. | ||
| if index != last_user_index: | ||
| events.append({"type": _USER_MESSAGE_EVENT, "text": content}) |
Contributor
There was a problem hiding this comment.
When an explicit input-rail check receives a transcript ending with an assistant message, messages_to_events removes the latest historical user message and topic_safety_check_input appends it after that assistant response, causing topic safety to classify a reordered conversation and return an incorrect allow or block verdict.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: nemoguardrails/guardrails/compiled_rail.py
Line: 101-113
Comment:
**Historical user turn is reordered**
When an explicit input-rail check receives a transcript ending with an assistant message, `messages_to_events` removes the latest historical user message and `topic_safety_check_input` appends it after that assistant response, causing topic safety to classify a reordered conversation and return an incorrect allow or block verdict.
**Knowledge Base Used:**
- [Rails Engine](https://app.greptile.com/nvidia-public-github/-/custom-context/knowledge-base/nvidia-nemo/guardrails/-/docs/rails-engine.md)
- [Library Rails](https://app.greptile.com/nvidia-public-github/-/custom-context/knowledge-base/nvidia-nemo/guardrails/-/docs/library-rails.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.…rning a valid rejection reason)
tgasser-nv
force-pushed
the
refactor/iorails-compiledrail-migration
branch
from
August 7, 2026 04:48
28dc78d to
1367344
Compare
Contributor
|
Staged Fern docs preview: https://nvidia-preview-pr-2261.docs.buildwithfern.com/nemo/guardrails |
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.
Description
This PR is the fourth in the stack of migrating IORails to use the new Manifest-based action system. Leading up to this PR, a CompiledRail abstraction was created to take a RailSurface and make it executable by IORails. This PR migrates the nemoguards actions that IORails already supports (content-safety, topic-control, and jailbreak) from RailAction subclasses to use the new manifest + CompiledRail wrapper approach. The switchover is seamless and backwards-compatible.
PR Stack context
The PR3 mentioned in previous PRs in the stack was split into a PR3a and PR3b since it was getting too large to review. Each PR in the list is self-contained, and will be merged one-by-one to avoid a lot of rebasing work once all are improved.
Related Issue(s)
Verification
Test Plan
Pre-commit
Unit-test
Integration test with Chat
This uses all nemoguards actions affected by the migration in this PR, so this tests live end-to-end to make sure the migration was successful.
AI Assistance
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Documentation