Repository navigation
Conversation
Use the same public LLMRails constructor as the server so the existing NEMO_GUARDRAILS_IORAILS_ENGINE switch selects the checking backend. Preserve Colang 1.0 admission, invoke lifecycle hooks only when available, and log the actual engine after initialization, including fallback. Exercise real engines for buffered and streaming allow/block behavior, unchanged request and response bytes, lifecycle ownership, fresh-process environment selection, and legacy fallback. Document comparison commands and the new server-aligned LLMRails default. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
|
Staged Fern docs preview: https://nvidia-preview-pr-2509.docs.buildwithfern.com/nemo/guardrails |
[Medium impact] The PR appears safe to merge. SummaryThe proxy now honors
No actionable issues were found. Tests were inspected but not run.
|
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Engine selection and repository startup nemoguardrails/server/experimental/application.md, nemoguardrails/server/experimental/configuration-resolution.md, nemoguardrails/server/experimental/rails/_repository.py, tests/server/experimental/test_application_manifest.py, tests/server/experimental/test_guardrails_repository.py, tests/server/experimental/test_proxy_engine_selection.py |
The repository rejects configurations outside Colang 1.0 and constructs LLMRails. Documentation and tests cover engine selection, buffered and streaming checks, and proxy responses. |
Optional lifecycle hooks nemoguardrails/server/experimental/configuration-resolution.md, nemoguardrails/server/experimental/rails/_adapter.py, tests/server/experimental/test_guardrails_repository.py, tests/server/experimental/test_guardrails_startup_cleanup.py |
The adapter calls shutdown only when the hook exists. Documentation and tests cover lifecycle handling and cleanup behavior. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Possibly related PRs
- NVIDIA-NeMo/Guardrails#2468: Adds the YAML launcher that uses the Rails repository changed here.
- NVIDIA-NeMo/Guardrails#2467: Adds proxy application configuration and assembly that use this Rails repository.
- NVIDIA-NeMo/Guardrails#2465: Adds an OpenAI Responses integration that reuses the proxy application and Rails repository.
Suggested reviewers: tanushriya910, christinaexyou
Merge Risk: 
Minimal · up to eff66
-
The proxy follows the server’s engine switch. No merge-blocking issue was established.
Pre-merge checks: 
5 · 
1
-
Failed checks: 1 warning Passed checks: 5 passed-
Full details: Test Results For Major Changes
-
Explanation
The PR contains a major engine-selection change, and its description includes testing information. It lists comparison commands and tests for environment selection, buffered and streaming allow/block behavior, byte preservation, LLMRails fallback, and cleanup/rollback. The diff also adds
tests/server/experimental/test_proxy_engine_selection.pyand updates related tests.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 27 functions across 6 files. (2 skipped: 2 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
PR merge guidance@Pouyanpi thanks for the PR. GitHub is currently blocking merge for one or more repository requirements:
Relevant guide: |
tgasser-nv
left a comment
There was a problem hiding this comment.
Can you take a look at the comments?
There are some high-level items:
- #2508 Updated docs to say "the proxy currently constructs Guardrails with
require_iorails=True", that's no longer the case. - The adapter assumes BLOCKED results have a non-empty string. There are two cases where this isn't true and the client gets a 502 response. The 502 response can lead to clients retrying.
a) LLMRails emits aRailExceptionevent ifenable_rails_exceptionsis set. This will raise a ValidationError from pydantic.
b) A custom flow that stops without a bot_message (content="").
| _SHUTDOWN_TIMEOUT_SECONDS = 5.0 | ||
|
|
||
|
|
||
| class GuardrailsRepository: |
There was a problem hiding this comment.
There's an issue with an unbounded in-memory cache not in scope of this PR, but this code will expose it on the new proxy.
If state isn't provided to generate_async(), it caches events in-memory here. Inside check_async(), it uses generate_async() here with only messages and options (i.e. no state). This means memory usage will grow unbounded for a long-running proxy.
Recommend fixing in a separate PR and merging that prior to this one
| rails = Guardrails(config=config, require_iorails=True) | ||
| if config.colang_version != "1.0": | ||
| raise UnsupportedContentCheckerConfiguration("Proxy content checks require Colang 1.0.") | ||
| rails = LLMRails(config=config) |
There was a problem hiding this comment.
The require_iorails flag defaults to False so this will fall back to LLMRails if there's an unsupported config with a warning log message rather than raising.
There are seven output rails which check $relevant_chunks which can't be passed in a checks_async() call to LLMRails, they have to be generated from generate_async(). But the LLMRails fallback logic doesn't use this fact. This is a silent fail-open for the following rails:
self check facts
alignscore check facts
fiddler bot faithfulness
patronus api check output
patronus lynx check output hallucination
autoalign groundedness output
self check hallucination
|
Thanks @tgasser-nv, I agree with your points! Shall we close this PR then? The original intention was to enable benchmarking comparisons, but I don’t think it’s worth the effort. We can compare IORails on the server and proxy separately if needed. what do you think? |
Make the experimental proxy honor
NEMO_GUARDRAILS_IORAILS_ENGINE, using the same engine selection as the Guardrails server. This lets deployments compare input/output checking with LLMRails and IORails using the same manifest, upstream and streaming settings.An unset variable,
0, orfalseselects LLMRails, replacing the proxy's previous forced IORails selection. Values1,true, oryesselect the Guardrails wrapper, which prefers IORails for compatible configurations and falls back to LLMRails otherwise. Selection happens when Python imports the library, so changing engines requires a process restart.Startup logs the actual engine for each checker, including fallback. The proxy invokes lifecycle hooks when the selected backend provides them and continues to reject Colang 2.x checking configurations before constructing an engine.
Adds comparison commands and tests covering environment selection, real-engine buffered and streaming allow/block behavior, byte preservation, LLMRails fallback, and existing IORails cleanup and rollback behavior.
Summary by CodeRabbit
New Features
Documentation