feat: add local backend for built-in nemo guardrails#197
Conversation
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
|
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:
WalkthroughThis PR adds a complete in-process NeMo Guardrails local backend: a Rust provider registry and component routing, a Python bridge that serializes configs and invokes Python registration, and a full Python implementation of rail enforcement for LLM and tool execution with streaming support. Tests and documentation are included. ChangesLocal Backend Provider & Core Integration
Python Bridge & Local Guardrails Module
Integration Testing
CLI Testing & Documentation Updates
🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
size:XL
Signed-off-by: Alex Fournier <afournier@nvidia.com>
|
/ok to test 98d4915 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@crates/python/src/lib.rs`:
- Around line 112-121: The current retry unconditionally swallows any py.import
error and prepends source_python_dir; change load_guardrails_local_register_fn
to only attempt the fallback when the original PyErr is specifically a
ModuleNotFoundError for "nemo_relay" or "nemo_relay._guardrails_local": capture
the Err(err) from py.import, inspect its Python exception type (e.g. compare
against pyo3::exceptions::PyModuleNotFoundError) and/or the exception message to
ensure it indicates the module is missing, and only then check
source_python_dir, call prepend_python_path_if_missing(py, &source_python_dir)
and retry the import; for any other error type, immediately return Err(err)
unchanged (preserving the original error).
In `@crates/python/src/py_plugin.rs`:
- Around line 177-181: The call to register_fn.call1((plugin_config_py,
py_ctx.clone_ref(py))) can raise after partial registrations were made, but the
current code only drains registrations on the success path; to fix, call
register_fn.call1 and capture its Result, and if it Errs then immediately bind
the PyPluginContext (py_ctx.bind(py).borrow()) and call drain_registrations() to
rollback any partial registrations before returning the Err; keep the
successful-path behavior unchanged. Use the existing symbols register_fn.call1,
py_ctx, PyPluginContext (py_ctx.bind(py).borrow()), and drain_registrations() to
locate and implement the error-path cleanup.
In `@crates/python/tests/coverage/coverage_tests.rs`:
- Around line 344-357: The test currently only asserts check_calls length, which
misses verifying the actual guardrail kinds; update the assertions to explicitly
assert the captured guardrail sequence from result_json["check_calls"] matches
the expected rail types in order (input, output, tool_input, tool_output) and
also validate each call's input payload where relevant (use the recorded
(messages, rail_types) entries from the fake backend), replacing the len() check
with explicit equality checks against the expected sequence so order and types
are enforced (refer to check_calls, seen_request_messages, seen_tool_args, and
llm_result in the diff to locate the assertions to update).
- Around line 232-236: The tests mutate Python's sys.modules for the nemo_relay
namespace without isolating changes; add a small helper (e.g.,
snapshot_and_swap_nemo_relay or with_nemo_relay_modules) that, given a Python
GIL/python interpreter handle, snapshots all sys.modules keys starting with
"nemo_relay", clears or removes those keys, yields to a closure where you
perform the modules.set_item("nemo_relay._native", native_module.clone()) swap,
and finally restores the original snapshot even on panic; replace the direct
modules.set_item calls in
test_guardrails_local_helper_registers_and_enforces_llm_and_tool_checks,
test_guardrails_local_helper_enforces_streamed_output_rails, and
test_local_guardrails_provider_initializes_and_enforces_managed_core_calls with
this helper so each test always restores the full nemo_relay.* entries.
In `@python/nemo_relay/_guardrails_local.py`:
- Around line 455-486: The monitor task is created eagerly which leaks when the
returned async generator (guarded_provider_stream) is never iterated; move
creation of text_queue, blocked and the asyncio.create_task(...) that starts
_monitor_streaming_output_rails into the start of guarded_provider_stream so the
monitor is only created when the generator is actually consumed, keep the
existing finally block logic (await text_queue.put(None); await monitor;
re-check blocked["message"] and call _raise_streaming_output_blocked if needed),
and ensure you still use _extract_stream_text, stream and blocked inside the
generator as before.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Enterprise
Run ID: 37c4df6f-db71-420c-b283-495a31160158
📒 Files selected for processing (11)
crates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/local.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rscrates/python/src/lib.rscrates/python/src/py_plugin.rscrates/python/tests/coverage/coverage_tests.rsdocs/about-nemo-relay/concepts/plugins.mdxdocs/build-plugins/nemoguardrails.mdxdocs/nemo-guardrails-plugin/about.mdxdocs/nemo-guardrails-plugin/configuration.mdxpython/nemo_relay/_guardrails_local.py
💤 Files with no reviewable changes (1)
- docs/build-plugins/nemoguardrails.mdx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (32)
{docs/**,README.md,CONTRIBUTING.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
{docs/**,README.md,CONTRIBUTING.md}: For docs-only changes, run targeted checks only if commands, package names, or examples changed. Usejust docsfor docs-site builds andjust docs-linkcheckwhen links changed
Run docs site build withjust docs
Files:
docs/about-nemo-relay/concepts/plugins.mdxdocs/nemo-guardrails-plugin/about.mdxdocs/nemo-guardrails-plugin/configuration.mdx
{docs/**,README.md,CONTRIBUTING.md,**/*.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Run docs link validation with
just docs-linkcheckwhen links change
Files:
docs/about-nemo-relay/concepts/plugins.mdxdocs/nemo-guardrails-plugin/about.mdxdocs/nemo-guardrails-plugin/configuration.mdx
{docs/**,README.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Verify README and docs entry points still match current package names and paths for large or public-facing changes
Files:
docs/about-nemo-relay/concepts/plugins.mdxdocs/nemo-guardrails-plugin/about.mdxdocs/nemo-guardrails-plugin/configuration.mdx
{docs/**,examples/**,README.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Verify examples still run with documented commands for large or public-facing changes
Files:
docs/about-nemo-relay/concepts/plugins.mdxdocs/nemo-guardrails-plugin/about.mdxdocs/nemo-guardrails-plugin/configuration.mdx
{docs/**,README.md,**/Cargo.toml,**/package.json,**/*.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Ensure renamed public surfaces are reflected consistently in manifests and docs for large or public-facing changes
Files:
docs/about-nemo-relay/concepts/plugins.mdxdocs/nemo-guardrails-plugin/about.mdxdocs/nemo-guardrails-plugin/configuration.mdx
**/*.{md,mdx,py,sh,yaml,yml,toml,json}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repo references, and build commands current
Files:
docs/about-nemo-relay/concepts/plugins.mdxdocs/nemo-guardrails-plugin/about.mdxpython/nemo_relay/_guardrails_local.pydocs/nemo-guardrails-plugin/configuration.mdx
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
In MDX files, top-of-file comments must use JSX comment delimiters: {/* to open and */} to close. Do not use HTML comments for MDX SPDX headers.
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
Files:
docs/about-nemo-relay/concepts/plugins.mdxdocs/nemo-guardrails-plugin/about.mdxdocs/nemo-guardrails-plugin/configuration.mdx
**/*.{html,md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license header in HTML and Markdown files using HTML comment syntax
Files:
docs/about-nemo-relay/concepts/plugins.mdxdocs/nemo-guardrails-plugin/about.mdxdocs/nemo-guardrails-plugin/configuration.mdx
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update embedded documentation snippets, patch docs, and binding-support notes if examples or supported bindings changed
Files:
docs/about-nemo-relay/concepts/plugins.mdxdocs/nemo-guardrails-plugin/about.mdxdocs/nemo-guardrails-plugin/configuration.mdx
docs/**
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run
just docsor./scripts/build-docs.sh htmlto regenerate ignored Fern API reference pages before validation for documentation site changes
Files:
docs/about-nemo-relay/concepts/plugins.mdxdocs/nemo-guardrails-plugin/about.mdxdocs/nemo-guardrails-plugin/configuration.mdx
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.
Flag stale examples, missing SPDX headers where required, and instructions that no longer match CI or pre-commit behavior.
Files:
docs/about-nemo-relay/concepts/plugins.mdxdocs/nemo-guardrails-plugin/about.mdxdocs/nemo-guardrails-plugin/configuration.mdx
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Use
snake_casenaming convention for Rust identifiers (e.g.,nemo_relay_tool_call)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node work
**/*.rs: Runcargo fmt --allto format all Rust code
Runcargo clippy --workspace --all-targets -- -D warningsto enforce all clippy lints as errors
**/*.rs: Runcargo fmt --allwhen Rust files changed as part of WebAssembly work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files changed as part of WebAssembly work
**/*.rs: If any Rust code changed, always runjust test-rust
If any Rust code changed, also runcargo fmt --all
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings
Run Rust formatting withcargo fmt --all
Run Rust linting withcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Usecargo fmtfor Rust code formatting
Runcargo clippy -- -D warningsto lint Rust code and treat all warnings as errors
Use Rust snake_case naming convention for Rust identifiers
Include SPDX license header in all Rust source files using double-slash comment syntax
Validate Rust code withuv run pre-commit run --all-filesto enforce cargo fmt formatting check, cargo clippy lints, and cargo deny aud...
Files:
crates/core/src/plugins/nemo_guardrails/local.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rscrates/python/src/py_plugin.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/python/src/lib.rscrates/python/tests/coverage/coverage_tests.rs
**/{Cargo.toml,**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Maintain consistency between Rust package names in
Cargo.tomland their actual usage across the codebase
Files:
crates/core/src/plugins/nemo_guardrails/local.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rscrates/python/src/py_plugin.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/python/src/lib.rscrates/python/tests/coverage/coverage_tests.rs
**/*.{h,hpp,c,cpp,rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Ensure FFI header and library naming follows consistent conventions across platform-specific builds
Files:
crates/core/src/plugins/nemo_guardrails/local.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rscrates/python/src/py_plugin.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/python/src/lib.rscrates/python/tests/coverage/coverage_tests.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/src/plugins/nemo_guardrails/local.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rscrates/core/src/plugins/nemo_guardrails/component.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Rust crate names and module prefixes during coordinated rename operations
Files:
crates/core/src/plugins/nemo_guardrails/local.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rscrates/python/src/py_plugin.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/python/src/lib.rscrates/python/tests/coverage/coverage_tests.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
crates/core/**/*.rs: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths. Keep errors explicit and binding-appropriate at the wrapper layer.
Files:
crates/core/src/plugins/nemo_guardrails/local.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rscrates/core/src/plugins/nemo_guardrails/component.rs
crates/{core,adaptive}/**
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full matrix across Rust, Python, Go, Node.js, and WebAssembly
Files:
crates/core/src/plugins/nemo_guardrails/local.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rscrates/core/src/plugins/nemo_guardrails/component.rs
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.
Files:
crates/core/src/plugins/nemo_guardrails/local.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rscrates/python/src/py_plugin.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/python/src/lib.rscrates/python/tests/coverage/coverage_tests.rspython/nemo_relay/_guardrails_local.py
**/*.{rs,py,go,js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions: Rust and Python use
snake_case, C FFI exports prefixednemo_relay_, Go usesPascalCasefor public APIs, Node.js usescamelCase.
Files:
crates/core/src/plugins/nemo_guardrails/local.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rscrates/python/src/py_plugin.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/python/src/lib.rscrates/python/tests/coverage/coverage_tests.rspython/nemo_relay/_guardrails_local.py
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: Keep async behavior on the existing tokio-based model. Bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
UseJson = serde_json::Valuein Rust-facing runtime APIs for JSON payload handling.
Files:
crates/core/src/plugins/nemo_guardrails/local.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rscrates/python/src/py_plugin.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/python/src/lib.rscrates/python/tests/coverage/coverage_tests.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/src/plugins/nemo_guardrails/local.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rscrates/core/src/plugins/nemo_guardrails/component.rs
{crates/adaptive/**/*.rs,**/*test*.{rs,py,go,ts,js},**/*adaptive*test*.{rs,py,go,ts,js},docs/plugins/adaptive/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Maintain documented and tested validation and report behavior for adaptive surfaces
Files:
crates/core/tests/unit/plugins/nemo_guardrails/component_tests.rscrates/python/tests/coverage/coverage_tests.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/core/tests/unit/plugins/nemo_guardrails/component_tests.rscrates/python/tests/coverage/coverage_tests.rs
crates/python/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
If the native Rust bridge changed, add the Rust crate tests for
nemo-relay-python
Files:
crates/python/src/py_plugin.rscrates/python/src/lib.rscrates/python/tests/coverage/coverage_tests.rs
crates/{python,ffi,node,wasm}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node,wasm}/**/*: Treat binding changes as public API changes. Check for parity with the other language bindings, FFI ownership/lifetime safety,
callback error propagation, stable type conversion, and consistent async/stream semantics.
Flag changes that update one binding without corresponding tests or documentation for the same surface elsewhere.
Files:
crates/python/src/py_plugin.rscrates/python/src/lib.rscrates/python/tests/coverage/coverage_tests.rs
{crates/python/src/py_api/**/*.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update Python native binding in
crates/python/src/py_api/mod.rswith Python wrapper docstring inpython/nemo_relay/<module>.pyand type stubs inpython/nemo_relay/*.pyimodules
Files:
python/nemo_relay/_guardrails_local.py
python/nemo_relay/**/*.py
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Use
snake_casenaming convention for Python identifiers (e.g.,nemo_relay.tools.call)Format changed Python wrapper and test files with
uv run ruff format pythonPython wrapper modules live under
python/nemo_relay/; the native extension is built fromcrates/pythonwithmaturin.
Files:
python/nemo_relay/_guardrails_local.py
{pyproject.toml,**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Maintain consistency between Python package names in
pyproject.tomland import paths used throughout the codebase
Files:
python/nemo_relay/_guardrails_local.py
**/*.{py,txt,toml,cfg,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Python package names and top-level module imports during coordinated rename operations
Files:
python/nemo_relay/_guardrails_local.py
**/*.py
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*.py: Run Python formatting withuv run ruff format python
Run Python testing withuv run pytest -k "<pattern>"
**/*.py: Use Ruff with rule sets E, F, W, I for Python linting
Use Ruff formatter with line length 120 and double quotes for Python code formatting
Runtyfor Python type checking
Use Python snake_case naming convention for Python identifiers
Include SPDX license header in all Python source files using hash comment syntax
Validate Python code withuv run pre-commit run --all-filesto enforce Ruff linting and formatting, and ty type checking
Files:
python/nemo_relay/_guardrails_local.py
python/nemo_relay/**/*
⚙️ CodeRabbit configuration file
python/nemo_relay/**/*: Review Python wrapper changes for typed API consistency, contextvars-based scope isolation, async behavior, and parity with the native extension.
Stubs and runtime implementations should stay aligned.
Files:
python/nemo_relay/_guardrails_local.py
🪛 LanguageTool
docs/nemo-guardrails-plugin/configuration.mdx
[style] ~294-~294: To form a complete sentence, be sure to include a subject.
Context: ...g_yamlis required. -colang_contentcan only be used withconfig_yaml. - rem...
(MISSING_IT_THERE)
🪛 Ruff (0.15.15)
python/nemo_relay/_guardrails_local.py
[warning] 59-59: Missing return type annotation for private function _load_nemoguardrails
(ANN202)
[warning] 66-69: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 70-73: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 83-83: Dynamically typed expressions (typing.Any) are disallowed in status
(ANN401)
[warning] 87-87: Dynamically typed expressions (typing.Any) are disallowed in annotated
(ANN401)
[warning] 92-92: Dynamically typed expressions (typing.Any) are disallowed in rails
(ANN401)
[warning] 93-93: Dynamically typed expressions (typing.Any) are disallowed in rail_type
(ANN401)
[warning] 94-94: Dynamically typed expressions (typing.Any) are disallowed in rail_status
(ANN401)
[warning] 122-126: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 156-160: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 163-167: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 171-171: Dynamically typed expressions (typing.Any) are disallowed in result
(ANN401)
[warning] 174-180: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 184-184: Dynamically typed expressions (typing.Any) are disallowed in rails
(ANN401)
[warning] 185-185: Dynamically typed expressions (typing.Any) are disallowed in rail_type
(ANN401)
[warning] 186-186: Dynamically typed expressions (typing.Any) are disallowed in rail_status
(ANN401)
[warning] 202-202: Dynamically typed expressions (typing.Any) are disallowed in rails
(ANN401)
[warning] 206-206: Dynamically typed expressions (typing.Any) are disallowed in rails
(ANN401)
[warning] 206-206: Dynamically typed expressions (typing.Any) are disallowed in _output_streaming_config
(ANN401)
[warning] 210-210: Dynamically typed expressions (typing.Any) are disallowed in rails
(ANN401)
[warning] 215-215: Too many return statements (10 > 6)
(PLR0911)
[warning] 215-215: Too many branches (13 > 12)
(PLR0912)
[warning] 271-271: Missing return type annotation for private function _queue_string_stream
(ANN202)
[warning] 271-271: Remove quotes from type annotation
Remove quotes
(UP037)
[warning] 281-281: Dynamically typed expressions (typing.Any) are disallowed in rails
(ANN401)
[warning] 283-283: Remove quotes from type annotation
Remove quotes
(UP037)
[warning] 300-304: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 307-307: Dynamically typed expressions (typing.Any) are disallowed in rails_config_cls
(ANN401)
[warning] 307-307: Dynamically typed expressions (typing.Any) are disallowed in _build_guardrails_config
(ANN401)
[warning] 319-319: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 324-324: Dynamically typed expressions (typing.Any) are disallowed in rails
(ANN401)
[warning] 325-325: Dynamically typed expressions (typing.Any) are disallowed in rail_type
(ANN401)
[warning] 326-326: Dynamically typed expressions (typing.Any) are disallowed in rail_status
(ANN401)
[warning] 347-347: Dynamically typed expressions (typing.Any) are disallowed in rails
(ANN401)
[warning] 348-348: Dynamically typed expressions (typing.Any) are disallowed in rail_type
(ANN401)
[warning] 349-349: Dynamically typed expressions (typing.Any) are disallowed in rail_status
(ANN401)
[warning] 376-376: Missing return type annotation for private function _make_llm_intercept
(ANN202)
[warning] 378-378: Dynamically typed expressions (typing.Any) are disallowed in rails
(ANN401)
[warning] 379-379: Dynamically typed expressions (typing.Any) are disallowed in rail_type
(ANN401)
[warning] 380-380: Dynamically typed expressions (typing.Any) are disallowed in rail_status
(ANN401)
[warning] 385-385: Missing return type annotation for private function intercept
(ANN202)
[warning] 415-415: Missing return type annotation for private function _make_llm_stream_intercept
(ANN202)
[warning] 417-417: Dynamically typed expressions (typing.Any) are disallowed in rails
(ANN401)
[warning] 418-418: Dynamically typed expressions (typing.Any) are disallowed in rail_type
(ANN401)
[warning] 419-419: Dynamically typed expressions (typing.Any) are disallowed in rail_status
(ANN401)
[warning] 425-425: Missing return type annotation for private function stream_intercept
(ANN202)
[warning] 443-446: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 450-453: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 466-466: Missing return type annotation for private function guarded_provider_stream
(ANN202)
[warning] 491-491: Missing return type annotation for private function _make_tool_intercept
(ANN202)
[warning] 493-493: Dynamically typed expressions (typing.Any) are disallowed in rails
(ANN401)
[warning] 494-494: Dynamically typed expressions (typing.Any) are disallowed in rail_type
(ANN401)
[warning] 495-495: Dynamically typed expressions (typing.Any) are disallowed in rail_status
(ANN401)
[warning] 499-499: Missing return type annotation for private function tool_intercept
(ANN202)
[warning] 527-527: Dynamically typed expressions (typing.Any) are disallowed in result
(ANN401)
[warning] 533-538: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (12)
crates/core/src/plugins/nemo_guardrails/local.rs (2)
17-37: LGTM!
39-51: Lock-free invocation is the right call. Cloning the provider handle and dropping theMutexGuardbefore invoking avoids re-entrancy deadlock if the provider touches the registry, and keeps the lock off the async registration path.crates/core/src/plugins/nemo_guardrails/component.rs (2)
20-28: LGTM!Also applies to: 454-454
960-970: Local-moderequest_defaultsrejection is consistent. Early-return correctly short-circuits the remote-only field checks. Note the diagnostic honorspolicy.unsupported_value, so anIgnorepolicy will pass validation and forwardrequest_defaultsto the provider — that matches policy semantics, just confirm the Python backend tolerates/ignores the field rather than erroring.crates/core/tests/unit/plugins/nemo_guardrails/component_tests.rs (1)
43-49: Coverage and isolation look solid.reset_runtimeclearing the provider at each test entry (undertest_mutex) keeps the new dispatch test from leaking registration state into siblings, and the assertions inside the provider closure verify both invocation and config propagation.Also applies to: 793-807, 995-995, 1027-1054
docs/about-nemo-relay/concepts/plugins.mdx (1)
174-177: LGTM!docs/nemo-guardrails-plugin/about.mdx (3)
23-55: LGTM!
59-116: LGTM!
121-121: LGTM!docs/nemo-guardrails-plugin/configuration.mdx (3)
37-59: LGTM!
61-197: LGTM!
211-345: LGTM!
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
|
/ok to test 67fd1b9 |
willkill07
left a comment
willkill07
left a comment
There was a problem hiding this comment.
Overall the design looks good. Let's propagate the logic as embedded rust/python code rather than needing nemo-relay installed.
Also, we need to fix CI (which is failing)
Signed-off-by: Alex Fournier <afournier@nvidia.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/python/src/lib.rs (1)
155-166:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOnly fall back to
../../pythonon a true missing-module error.This still treats any
py.import("nemo_relay")failure as recoverable, so an installed-but-broken package can be silently shadowed by the source tree. Preserve the originalPyErr, retry only forModuleNotFoundErroronnemo_relay, and return that original error unchanged when the source tree is absent.🤖 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 `@crates/python/src/lib.rs` around lines 155 - 166, In ensure_nemo_relay_package_importable, preserve and inspect the original PyErr from py.import("nemo_relay") instead of treating any failure as recoverable: call py.import and if it returns Err, capture that error, check whether it is a ModuleNotFoundError specifically for the "nemo_relay" name; only then proceed to check embedded_guardrails_source_python_dir and call prepend_python_path_if_missing and retry import via py.import; if the source tree is absent return the original PyErr unchanged, and if the original error was not ModuleNotFoundError for "nemo_relay" return it immediately (do not shadow a broken installed package).
🤖 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.
Duplicate comments:
In `@crates/python/src/lib.rs`:
- Around line 155-166: In ensure_nemo_relay_package_importable, preserve and
inspect the original PyErr from py.import("nemo_relay") instead of treating any
failure as recoverable: call py.import and if it returns Err, capture that
error, check whether it is a ModuleNotFoundError specifically for the
"nemo_relay" name; only then proceed to check
embedded_guardrails_source_python_dir and call prepend_python_path_if_missing
and retry import via py.import; if the source tree is absent return the original
PyErr unchanged, and if the original error was not ModuleNotFoundError for
"nemo_relay" return it immediately (do not shadow a broken installed package).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 505b1e7e-510f-4922-89e9-b887e63991ad
📒 Files selected for processing (4)
crates/python/embedded_python/_guardrails_local.pycrates/python/src/lib.rscrates/python/tests/coverage/coverage_tests.rsdocs/nemo-guardrails-plugin/configuration.mdx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (25)
{pyproject.toml,**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Maintain consistency between Python package names in
pyproject.tomland import paths used throughout the codebase
Files:
crates/python/embedded_python/_guardrails_local.py
**/*.{py,txt,toml,cfg,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Python package names and top-level module imports during coordinated rename operations
Files:
crates/python/embedded_python/_guardrails_local.py
**/*.py
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*.py: Run Python formatting withuv run ruff format python
Run Python testing withuv run pytest -k "<pattern>"
**/*.py: Use Ruff with rule sets E, F, W, I for Python linting
Use Ruff formatter with line length 120 and double quotes for Python code formatting
Runtyfor Python type checking
Use Python snake_case naming convention for Python identifiers
Include SPDX license header in all Python source files using hash comment syntax
Validate Python code withuv run pre-commit run --all-filesto enforce Ruff linting and formatting, and ty type checking
Files:
crates/python/embedded_python/_guardrails_local.py
**/*.{md,mdx,py,sh,yaml,yml,toml,json}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repo references, and build commands current
Files:
crates/python/embedded_python/_guardrails_local.pydocs/nemo-guardrails-plugin/configuration.mdx
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.
Files:
crates/python/embedded_python/_guardrails_local.pycrates/python/tests/coverage/coverage_tests.rscrates/python/src/lib.rs
**/*.{rs,py,go,js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions: Rust and Python use
snake_case, C FFI exports prefixednemo_relay_, Go usesPascalCasefor public APIs, Node.js usescamelCase.
Files:
crates/python/embedded_python/_guardrails_local.pycrates/python/tests/coverage/coverage_tests.rscrates/python/src/lib.rs
crates/{python,ffi,node,wasm}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node,wasm}/**/*: Treat binding changes as public API changes. Check for parity with the other language bindings, FFI ownership/lifetime safety,
callback error propagation, stable type conversion, and consistent async/stream semantics.
Flag changes that update one binding without corresponding tests or documentation for the same surface elsewhere.
Files:
crates/python/embedded_python/_guardrails_local.pycrates/python/tests/coverage/coverage_tests.rscrates/python/src/lib.rs
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Use
snake_casenaming convention for Rust identifiers (e.g.,nemo_relay_tool_call)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node work
**/*.rs: Runcargo fmt --allto format all Rust code
Runcargo clippy --workspace --all-targets -- -D warningsto enforce all clippy lints as errors
**/*.rs: Runcargo fmt --allwhen Rust files changed as part of WebAssembly work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files changed as part of WebAssembly work
**/*.rs: If any Rust code changed, always runjust test-rust
If any Rust code changed, also runcargo fmt --all
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings
Run Rust formatting withcargo fmt --all
Run Rust linting withcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Usecargo fmtfor Rust code formatting
Runcargo clippy -- -D warningsto lint Rust code and treat all warnings as errors
Use Rust snake_case naming convention for Rust identifiers
Include SPDX license header in all Rust source files using double-slash comment syntax
Validate Rust code withuv run pre-commit run --all-filesto enforce cargo fmt formatting check, cargo clippy lints, and cargo deny aud...
Files:
crates/python/tests/coverage/coverage_tests.rscrates/python/src/lib.rs
{crates/adaptive/**/*.rs,**/*test*.{rs,py,go,ts,js},**/*adaptive*test*.{rs,py,go,ts,js},docs/plugins/adaptive/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Maintain documented and tested validation and report behavior for adaptive surfaces
Files:
crates/python/tests/coverage/coverage_tests.rs
**/{Cargo.toml,**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Maintain consistency between Rust package names in
Cargo.tomland their actual usage across the codebase
Files:
crates/python/tests/coverage/coverage_tests.rscrates/python/src/lib.rs
**/*.{h,hpp,c,cpp,rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Ensure FFI header and library naming follows consistent conventions across platform-specific builds
Files:
crates/python/tests/coverage/coverage_tests.rscrates/python/src/lib.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Rust crate names and module prefixes during coordinated rename operations
Files:
crates/python/tests/coverage/coverage_tests.rscrates/python/src/lib.rs
crates/python/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
If the native Rust bridge changed, add the Rust crate tests for
nemo-relay-python
Files:
crates/python/tests/coverage/coverage_tests.rscrates/python/src/lib.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: Keep async behavior on the existing tokio-based model. Bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
UseJson = serde_json::Valuein Rust-facing runtime APIs for JSON payload handling.
Files:
crates/python/tests/coverage/coverage_tests.rscrates/python/src/lib.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/python/tests/coverage/coverage_tests.rs
{docs/**,README.md,CONTRIBUTING.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
{docs/**,README.md,CONTRIBUTING.md}: For docs-only changes, run targeted checks only if commands, package names, or examples changed. Usejust docsfor docs-site builds andjust docs-linkcheckwhen links changed
Run docs site build withjust docs
Files:
docs/nemo-guardrails-plugin/configuration.mdx
{docs/**,README.md,CONTRIBUTING.md,**/*.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Run docs link validation with
just docs-linkcheckwhen links change
Files:
docs/nemo-guardrails-plugin/configuration.mdx
{docs/**,README.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Verify README and docs entry points still match current package names and paths for large or public-facing changes
Files:
docs/nemo-guardrails-plugin/configuration.mdx
{docs/**,examples/**,README.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Verify examples still run with documented commands for large or public-facing changes
Files:
docs/nemo-guardrails-plugin/configuration.mdx
{docs/**,README.md,**/Cargo.toml,**/package.json,**/*.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Ensure renamed public surfaces are reflected consistently in manifests and docs for large or public-facing changes
Files:
docs/nemo-guardrails-plugin/configuration.mdx
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
In MDX files, top-of-file comments must use JSX comment delimiters: {/* to open and */} to close. Do not use HTML comments for MDX SPDX headers.
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
Files:
docs/nemo-guardrails-plugin/configuration.mdx
**/*.{html,md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license header in HTML and Markdown files using HTML comment syntax
Files:
docs/nemo-guardrails-plugin/configuration.mdx
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update embedded documentation snippets, patch docs, and binding-support notes if examples or supported bindings changed
Files:
docs/nemo-guardrails-plugin/configuration.mdx
docs/**
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run
just docsor./scripts/build-docs.sh htmlto regenerate ignored Fern API reference pages before validation for documentation site changes
Files:
docs/nemo-guardrails-plugin/configuration.mdx
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.
Flag stale examples, missing SPDX headers where required, and instructions that no longer match CI or pre-commit behavior.
Files:
docs/nemo-guardrails-plugin/configuration.mdx
🪛 Ruff (0.15.15)
crates/python/embedded_python/_guardrails_local.py
[warning] 80-83: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 87-91: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (7)
docs/nemo-guardrails-plugin/configuration.mdx (1)
228-234: LGTM!crates/python/src/lib.rs (1)
30-54: LGTM!Also applies to: 80-91, 131-152, 170-172
crates/python/embedded_python/_guardrails_local.py (1)
25-25: LGTM!Also applies to: 78-91
crates/python/tests/coverage/coverage_tests.rs (4)
48-50: LGTM!Also applies to: 122-148
326-330: LGTM!Also applies to: 350-350, 423-423
479-557: LGTM!
574-578: LGTM!Also applies to: 615-615, 713-713
willkill07
left a comment
willkill07
left a comment
There was a problem hiding this comment.
The new shape looks great!
I just realized a core issue that we have:
The plugin config and enablement is under core but the logic for the guardrails plugin is all under Python. We can't have this split.
I'm okay with core getting an added PyO3 dependency (add a new feature named python and default to enabled).
Then we can gate the "local" config on the python feature.
I know this is extra churn, but I'm not sure how else we could get it to work in a deployed scenario with the core Relay library (or just the CLI)
Signed-off-by: Alex Fournier <afournier@nvidia.com>
|
/ok to test 3c7eecd |
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
…acked-backend-for-built-in-nemo
|
/ok to test 31584ef |
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
…nt-local-python-backed-backend-for-built-in-nemo' into guardrails-cli-local-test # Conflicts: # crates/core/src/plugins/nemo_guardrails/local.rs # crates/python/tests/coverage/coverage_tests.rs
|
/ok to test 0260c7e |
Signed-off-by: Alex Fournier <afournier@nvidia.com>
|
/ok to test 4aee554 |
|
/ok to test 0260c7e |
@willkill07, there was an error processing your request: See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/2/ |
|
/ok to test 4aee554 |
…acked-backend-for-built-in-nemo Signed-off-by: Will Killian <2007799+willkill07@users.noreply.github.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
/ok to test aba629c |
2 participants
Overview
Details
This PR adds the built-in
localbackend for the first-partynemo_guardrailsplugin and updates the shippedremote/localmode boundaries in docs and coverage.What changed:
crates/coreownership instead of a Python-binding-installed provider seampythonfeature tocrates/coreand gatemode = "local"on that featurenemoguardrails==0.22.0input/output, local managedtool_input/tool_output, and local streaming output through Guardrails-nativestream_first = truesemanticsrequest_defaultsandrails.output.streaming.stream_first = falseImportant local-mode boundaries in this PR:
pythonfeature in corenemoguardrails==0.22.0stream_first = truesemantics, so a later output block can happen after some chunks were already emittedrequest_defaultsrails.output.streaming.stream_first = falsenemo-relayplus upstreamnemoguardrails==0.22.0Where should the reviewer start?
Start with:
crates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/local.rscrates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/plugins/nemo_guardrails/embedded_python/_guardrails_local.pydocs/nemo-guardrails-plugin/configuration.mdxRelated Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)