refactor(iorails): Refactor Guardrails and IORails for top-level import and clean separation#1893
Conversation
Greptile SummaryThis PR refactors the
|
| Filename | Overview |
|---|---|
| nemoguardrails/init.py | Adds top-level Guardrails import with carefully ordered imports to avoid circular-import; aliases LLMRails to Guardrails when env var is set. |
| nemoguardrails/guardrails/guardrails.py | Adds require_iorails kwarg; delegates IORails eligibility check to IORails.unsupported_reason() and logs/raises on fallback. Removes duplicated detection logic. |
| nemoguardrails/guardrails/iorails.py | Moves IORails eligibility logic into unsupported_reason() and can_handle() class methods with the supported-flow constants; clean ownership separation. |
| tests/guardrails/test_guardrails.py | Migrates all _has_only_iorails_flows tests to IORails.can_handle; adds TestIORailsUnsupportedReason and TestRequireIORails suites for the new API surface. |
| docs/troubleshooting.md | Adds require_iorails=True guidance; uses LLMRails(config, require_iorails=True) as an env-var-conditional snippet that silently breaks with a TypeError when the env var is absent. |
| tests/test_imports.py | Adds TestIORailsEngineEnvVar which reloads nemoguardrails with/without the env var to verify the LLMRails-to-Guardrails alias swap. |
| docs/observability/metrics/enable-metrics.md | Replaces LLMRails(config) with Guardrails(config, use_iorails=True, require_iorails=True); removes the env-var requirement from the run command. |
| docs/observability/metrics/opentelemetry-integration.md | Updates import and construction to top-level Guardrails with require_iorails=True, ensuring loud failure when the config is incompatible with IORails. |
| docs/configure-rails/yaml-schema/guardrails-configuration/parallel-rails.md | Documents the new silent-fallback behaviour and require_iorails=True escape hatch; import updated to top-level Guardrails. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Guardrails(config, llm, use_iorails, require_iorails)"] --> B{use_iorails?}
B -- No --> E[LLMRails engine]
B -- Yes --> C["IORails.unsupported_reason(config, llm)"]
C -- None --> D[IORails engine]
C -- reason string --> F{require_iorails?}
F -- True --> G["raise ValueError(reason)"]
F -- False --> H["log.warning(reason)"]
H --> E
style D fill:#d4edda
style E fill:#d1ecf1
style G fill:#f8d7da
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
docs/troubleshooting.md:123-125
**`LLMRails(config, require_iorails=True)` throws TypeError without the env var**
The snippet on line 123 (repeated on line 133) uses `LLMRails(config, require_iorails=True)` under the condition "when running with `NEMO_GUARDRAILS_IORAILS_ENGINE=1`". When that env var is set `LLMRails` is aliased to `Guardrails`, so `require_iorails` is accepted. However, if a reader copies the call without setting the env var, the real `LLMRails.__init__` has no `require_iorails` parameter and raises `TypeError` immediately. The unconditional `Guardrails(config, use_iorails=True, require_iorails=True)` form used in the other docs pages avoids this confusion entirely; consider replacing the `LLMRails(...)` alternative here with that form too.
### Issue 2 of 2
nemoguardrails/guardrails/guardrails.py:395-414
**`require_iorails` is silently dropped on pickle/unpickle**
`__getstate__` saves `use_iorails=self.use_iorails_engine` but omits `require_iorails`. After unpickling, `__setstate__` reconstructs with `require_iorails` defaulting to `False`. For a compatible config this is harmless, but if the config later becomes incompatible (e.g., config files were updated between pickling and unpickling), the reconstructed instance will silently fall back to LLMRails rather than raising as the original caller intended. Adding `require_iorails` to the state dict would preserve the contract across serialization.
Reviews (8): Last reviewed commit: "Address Greptile doc feedback" | Re-trigger Greptile
📝 WalkthroughWalkthroughThis PR promotes ChangesGuardrails Top-Level Export and Related Updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Documentation preview |
…s import Guardrails' to 'from nemoguardrails import Guardrails'
…of Guardrails-level
97b55a6 to
aeac517
Compare
Pouyanpi
left a comment
There was a problem hiding this comment.
Approved, but I’d like us to follow up on the long-term Guardrails/IORails boundary after this lands.
| log.warning(message) | ||
| self._rails_engine = LLMRails(config, llm, verbose) | ||
| self.use_iorails_engine = False | ||
| else: |
There was a problem hiding this comment.
it still silently uses LLMRails. I think that is semantically wrong because require_iorails sounds fail-closed. if it is intended then a docstring update could be good.
Description
Stacked PR on top of #1892 .
This PR cleans up the Guardrails and IORails/LLMRails entry points:
from nemoguardrails.guardrails.guardrails import Guardrailstofrom nemoguardrails import Guardrailsin the spirit of feat(api): canonical top-level imports for LLM types and registry functions #1882 .require_iorailswhich will raise if the user setsuse_iorailsand we can't because they provided an LLM reference tollmor their config isn't supported by IORails.Related Issue(s)
#1638
Test Plan
Pre-commit
Unit-test
Integration test with Chat (IORails)
Integration test with Chat (LLMRails)
Checklist
Summary by CodeRabbit
New Features
Guardrailsis now available as a top-level import from thenemoguardrailspackage.Documentation
Guardrailsimport.