feat: implement nemo_guardrails remote backend#144
Conversation
|
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 implements a remote backend for the NeMo Guardrails plugin, enabling routing of LLM and tool execution to a remote HTTP endpoint. It adds feature gating, extends request configuration with continuation fields ( ChangesRemote NeMo Guardrails Backend
🎯 4 (Complex) | ⏱️ ~60 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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/core/src/plugins/nemo_guardrails/component.rs`:
- Around line 956-968: The length check uses raw thread_id.len() while the
emptiness check uses thread_id.trim().is_empty(), causing inconsistent
validation; update the logic in component.rs around request_defaults.thread_id
(the block that calls push_policy_diag with policy.unsupported_value and
NEMO_GUARDRAILS_PLUGIN_KIND) to operate on a trimmed value (e.g., compute let
thread_id_trimmed = thread_id.trim() and use thread_id_trimmed.is_empty() and
thread_id_trimmed.len() or thread_id_trimmed.chars().count() for Unicode-aware
length) so whitespace padding is handled consistently (or alternatively add
explicit whitespace validation if padded IDs should be rejected).
- Around line 20-22: Gate the remote module import and usage behind a
feature/target cfg: change the unconditional mod remote/import of
register_remote_backend to something like #[cfg(feature = "remote")] #[path =
"remote.rs"] mod remote; and #[cfg(feature = "remote")] use
remote::register_remote_backend; then add a fallback stub for
register_remote_backend or inside register_nemo_guardrails_backend (e.g.,
#[cfg(not(feature = "remote"))] pub fn register_remote_backend(...) -> Result<_,
_> or have register_nemo_guardrails_backend return a clear Err when running on
unsupported targets like wasm32) so builds without the "remote" feature or on
wasm32 do not try to pull in reqwest; ensure the stub provides a clear
compile-time or runtime message about the missing feature/unsupported target.
🪄 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: 8eb2a87e-24a6-4818-b0bf-f4bcc6337f1e
📒 Files selected for processing (7)
crates/core/Cargo.tomlcrates/core/src/plugin.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/mod.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rscrates/core/tests/unit/plugins/nemo_guardrails/plugin_component_tests.rs
💤 Files with no reviewable changes (1)
- crates/core/tests/unit/plugins/nemo_guardrails/plugin_component_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (16)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Usecargo fmtfor Rust code formatting (rustfmt defaults)
Usecargo clippy -- -D warningsfor Rust linting, treating all warnings as errors
Use Rustsnake_casenaming convention for Rust code
Runjust test-rustfor Rust test suiteUse
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
Files:
crates/core/src/plugins/nemo_guardrails/mod.rscrates/core/src/plugin.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rs
**/*.{rs,go,js,ts,jsx,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license header with format:
// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0in Rust, Go, JavaScript, and TypeScript files
Files:
crates/core/src/plugins/nemo_guardrails/mod.rscrates/core/src/plugin.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_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/mod.rscrates/core/src/plugin.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/Cargo.tomlcrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_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/mod.rscrates/core/src/plugin.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_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/mod.rscrates/core/src/plugin.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/Cargo.tomlcrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.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/mod.rscrates/core/src/plugin.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/Cargo.tomlcrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_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/mod.rscrates/core/src/plugin.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.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/mod.rscrates/core/src/plugin.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/Cargo.tomlcrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rs
**/*.{rs,py,js,ts,tsx,jsx,go,sh,bash,yml,yaml,toml,json,md,mjs,cjs}
📄 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/mod.rscrates/core/src/plugin.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/Cargo.tomlcrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_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/mod.rscrates/core/src/plugin.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rs
**/*.{py,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license header with format:
# SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0in Python and TOML files
Files:
crates/core/Cargo.toml
**/Cargo.toml
📄 CodeRabbit inference engine (.agents/skills/prepare-code-freeze/SKILL.md)
Confirm or infer the target release version from
upstream/main:Cargo.toml. Derive the release branch asrelease/<major>.<minor>.Update WebAssembly crate names and generated package names during coordinated rename operations
Files:
crates/core/Cargo.toml
**/*.{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/core/Cargo.toml
{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:
crates/core/Cargo.toml
{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.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.rs
🔇 Additional comments (29)
crates/core/src/plugin.rs (1)
765-769: LGTM!crates/core/src/plugins/nemo_guardrails/mod.rs (1)
14-14: LGTM!crates/core/src/plugins/nemo_guardrails/component.rs (8)
189-194: LGTM!
320-321: LGTM!
362-372: LGTM!
434-447: LGTM!
527-528: LGTM!
902-926: LGTM!
969-992: LGTM!
1246-1247: LGTM!crates/core/Cargo.toml (1)
17-22: ⚡ Quick winNo wasm32 build break:
remote.rsalready gatesguardrails-remotedeps
crates/core/src/plugins/nemo_guardrails/remote.rsonly pulls in/usesreqwestandrustlsunder#[cfg(all(not(target_arch = "wasm32"), feature = "guardrails-remote"))], and it provides a wasm32 (and/or non-feature)register_remote_backendfallback under#[cfg(any(target_arch = "wasm32", not(feature = "guardrails-remote")))]. Socrates/core/src/plugins/nemo_guardrails/component.rscan keep its unconditionalmod remote;/use remote::register_remote_backend;without additional gating.crates/core/src/plugins/nemo_guardrails/remote.rs (8)
1-57: LGTM!
59-99: LGTM!
101-221: LGTM!
223-403: LGTM!
405-452: LGTM!
454-520: LGTM!
530-715: LGTM!
718-941: LGTM!crates/core/tests/unit/plugins/nemo_guardrails/component_tests.rs (10)
1-50: LGTM!
52-91: LGTM!
93-222: LGTM!
224-337: LGTM!
339-430: LGTM!
432-848: LGTM!
850-1171: LGTM!
1173-1683: LGTM!
1685-2209: LGTM!
2211-2465: 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>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
afourniernv
force-pushed
the
afournier/nmf-131-remote-backend
branch
from
8f2e3ae to
3c2276c
Compare
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/core/src/plugins/nemo_guardrails/component.rs`:
- Around line 985-999: The current validation for request_defaults.state only
ensures it contains 'events' or 'state' but doesn't reject extra keys; update
the check in the request_defaults.state handling so you validate that the
object's keys are a subset of the allowed set {'events','state'} and
push_policy_diag when any other key is present. Concretely, in the same block
that currently references request_defaults.state and calls push_policy_diag with
policy.unsupported_value, add logic to iterate the object's keys (or compute the
set difference) and if there are any keys besides "events" or "state" call
push_policy_diag(diagnostics, policy.unsupported_value,
"nemo_guardrails.unsupported_value",
Some(NEMO_GUARDRAILS_PLUGIN_KIND.to_string()),
Some("request_defaults.state".to_string()), "request_defaults.state must be
empty or contain only 'events' and/or 'state'".to_string()).
In `@crates/core/src/plugins/nemo_guardrails/remote.rs`:
- Around line 173-186: The code currently includes raw remote response bodies in
observability marks and error messages (seen where self.emit_mark calls pass
payload.clone() into remote_mark_data and where FlowError::Internal interpolates
the payload); instead, sanitize or omit the body before exporting by replacing
payload.clone() with a safe representation (e.g., None, a fixed string like
"<redacted>", or a truncated/hashed summary produced by a helper like
redact_payload(payload)) and ensure the FlowError message does not embed
sensitive contents (use status and a redacted summary instead); apply the same
change to the other occurrences mentioned (the remote_mark_data calls and
FlowError::Internal usages at the other locations).
- Around line 679-708: The code builds a rails map that always sets "tool_input"
and "tool_output" to false, causing tool-only checks to run under the wrong rail
family; update the rails construction in the RemoteCheckKind branch handling
(the rails variable created when matching RemoteCheckKind::Input/Output) to set
"tool_input" and "tool_output" according to the intended check type (e.g.,
enable tool_input for tool-input checks and tool_output for tool-output checks)
instead of hardcoding them false, then keep inserting rails into options and
preserving the existing request_defaults/log/guardrails insertion logic (see
symbols: RemoteCheckKind, rails, options, request_defaults, log, guardrails).
In `@crates/core/tests/unit/plugins/nemo_guardrails/component_tests.rs`:
- Around line 1-5: The PR lacks full Rust/core validation evidence — run the
required checks for changes under crates/core (including the unit tests in
tests/unit/plugins/nemo_guardrails/component_tests.rs): execute just test-rust,
cargo fmt --all, and cargo clippy --workspace --all-targets -- -D warnings, then
run the full language matrix for crates/core (all supported Rust toolchain
versions) and attach the resulting logs/screenshots to the PR; ensure the
artifacts explicitly reference the component_tests.rs run and note any fixes
made if formatting or clippy failures are found.
- Line 946: The test currently blocks on request_rx.recv().unwrap(), which can
hang; replace the blocking recv call with a bounded wait using
request_rx.recv_timeout(Duration::from_secs(...)) and handle the Err case to
fail the test with a clear timeout message (reference the request_rx and
captured variables). If the same test or helper also awaits stream.next().await
in a loop, wrap that await with tokio::time::timeout(Duration::from_secs(...),
stream.next()).await and convert the timeout result into a test failure message
so the test fails deterministically instead of hanging.
🪄 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: ffe36d84-5071-415c-a3a7-103f8e4b9fe0
📒 Files selected for processing (7)
crates/core/Cargo.tomlcrates/core/src/plugin.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/mod.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rscrates/core/tests/unit/plugins/nemo_guardrails/plugin_component_tests.rs
💤 Files with no reviewable changes (1)
- crates/core/tests/unit/plugins/nemo_guardrails/plugin_component_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (16)
**/*.{py,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license header with format:
# SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0in Python and TOML files
Files:
crates/core/Cargo.toml
**/{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/Cargo.tomlcrates/core/src/plugin.rscrates/core/src/plugins/nemo_guardrails/mod.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rs
**/Cargo.toml
📄 CodeRabbit inference engine (.agents/skills/prepare-code-freeze/SKILL.md)
Confirm or infer the target release version from
upstream/main:Cargo.toml. Derive the release branch asrelease/<major>.<minor>.Update WebAssembly crate names and generated package names during coordinated rename operations
Files:
crates/core/Cargo.toml
{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/Cargo.tomlcrates/core/src/plugin.rscrates/core/src/plugins/nemo_guardrails/mod.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.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/Cargo.tomlcrates/core/src/plugin.rscrates/core/src/plugins/nemo_guardrails/mod.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rs
**/*.{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/core/Cargo.toml
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/Cargo.tomlcrates/core/src/plugin.rscrates/core/src/plugins/nemo_guardrails/mod.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rs
{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:
crates/core/Cargo.toml
**/*.{rs,py,js,ts,tsx,jsx,go,sh,bash,yml,yaml,toml,json,md,mjs,cjs}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.
Files:
crates/core/Cargo.tomlcrates/core/src/plugin.rscrates/core/src/plugins/nemo_guardrails/mod.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Usecargo fmtfor Rust code formatting (rustfmt defaults)
Usecargo clippy -- -D warningsfor Rust linting, treating all warnings as errors
Use Rustsnake_casenaming convention for Rust code
Runjust test-rustfor Rust test suiteUse
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
Files:
crates/core/src/plugin.rscrates/core/src/plugins/nemo_guardrails/mod.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rs
**/*.{rs,go,js,ts,jsx,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license header with format:
// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0in Rust, Go, JavaScript, and TypeScript files
Files:
crates/core/src/plugin.rscrates/core/src/plugins/nemo_guardrails/mod.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_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/plugin.rscrates/core/src/plugins/nemo_guardrails/mod.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_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/plugin.rscrates/core/src/plugins/nemo_guardrails/mod.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_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/plugin.rscrates/core/src/plugins/nemo_guardrails/mod.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.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.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.rs
🔇 Additional comments (2)
crates/core/Cargo.toml (1)
17-22: Run the required validation matrix before merging.The PR notes only mention
cargo test -p nemo-flow nemo_guardrails --lib --tests, but this change set touchescrates/coreRust code and crate features. Please verify the required Rust checks (just test-rust,cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings) and the broadercrates/corevalidation (validate-changeplus the full Rust/Python/Go/Node.js/WebAssembly matrix) were run on this branch.As per coding guidelines, "
**/*.rs: Any Rust change must runjust test-rust", "Any Rust change must runcargo fmt --all", "`Any Rust change must run `cargo clippy --workspace --all-targets -- -D warnings", "crates/core/**/*.rs: If the change touchedcrates/coreor shared runtime semantics, also usevalidate-changefor broader validation", and "crates/{core,adaptive}/**: Ifcrates/coreorcrates/adaptivechanged, run the full matrix across Rust, Python, Go, Node.js, and WebAssembly."crates/core/src/plugin.rs (1)
765-768: Guardnemo_guardrailsbuilt-in registration whenguardrails-remoteis off.
Incrates/core/src/plugin.rs(lines 765-768),register_builtinsunconditionally callscrate::plugins::nemo_guardrails::component::register_nemo_guardrails_component(). If that component (or its dependencies likereqwest/rustls) is gated behindguardrails-remotewithout a no-op/stub for the disabled case, this will break compilation. Ensure there’s either a stub/no-op implementation when the feature is off, or wrap the call (or provide a feature-gated alternative) to keep built-in initialization feature-compatible.
Signed-off-by: Alex Fournier <afournier@nvidia.com>
|
@willkill07 @dagardner-nv live tested + no cargo blast radius. Did have to add some minimal deps. ready for review. |
willkill07
left a comment
willkill07
left a comment
There was a problem hiding this comment.
First brief pass. I'll have a more complete pass later.
|
/ok to test 60999ac |
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
crates/core/src/plugins/nemo_guardrails/remote.rs (1)
183-197:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStop returning raw remote error bodies in
FlowError.The marks are redacted now, but these error paths still interpolate the full remote body into
FlowError::Internal. That can surface prompts, tool payloads, or backend diagnostics to callers and logs. Return a redacted/truncated summary here too, and update the HTTP error assertions incrates/core/tests/unit/plugins/nemo_guardrails/remote_tests.rsto match.Also applies to: 291-321, 573-588
🤖 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/core/src/plugins/nemo_guardrails/remote.rs` around lines 183 - 197, The FlowError::Internal for non-success HTTP responses currently interpolates the full remote response body (e.g., the error case inside the non-success branch where self.emit_mark(...) is called), so change those error returns to include only a redacted or truncated summary instead of the raw payload: call redact_remote_error_payload(status.as_u16(), &payload) (or use the same redaction helper used for marks) and include only the status and that redacted/truncated string in the FlowError::Internal message; apply the same change to the other non-success branches mentioned (around the other ranges in this file) and update the corresponding assertions in crates/core/tests/unit/plugins/nemo_guardrails/remote_tests.rs to expect the redacted/truncated summary rather than the full body.
🤖 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/core/src/plugins/nemo_guardrails/remote.rs`:
- Around line 87-91: The outbound llm_guardrails config currently only uses
request_defaults so top-level surface flags are ignored; modify the call to
build_llm_guardrails_config (where llm_guardrails is constructed for the remote,
using remote.config_id/remote.config_ids) to accept the top-level booleans for
input/output and, inside that builder or immediately after, set
guardrails.options.rails families to false for any disabled top-level surface
(force input rails false if input=false, force output rails false if
output=false) so the remote receives explicit disables; also add regression
tests for input-only and output-only flows to verify the disabled family
behavior remains backward-compatible and document no behavioral change unless
tests indicate migration.
- Around line 894-929: The function build_tool_check_guardrails_config currently
hardcodes "tool_input": true or "tool_output": true and thus discards any
selector configured in request_defaults; update it to merge the configured
selector instead of overwriting it: read the optional configured rails object
from request_defaults (via request_defaults -> log -> "rails"), clone it, and
when creating the synthesized rails Json use the configured value for the
relevant key ("tool_input" or "tool_output") if present (preserve whatever Json
selector is stored), otherwise default to Json::Bool(true); keep the other rail
families set to false and insert the merged rails into options as before
(reference function name build_tool_check_guardrails_config and the
"rails"/"log" keys).
---
Duplicate comments:
In `@crates/core/src/plugins/nemo_guardrails/remote.rs`:
- Around line 183-197: The FlowError::Internal for non-success HTTP responses
currently interpolates the full remote response body (e.g., the error case
inside the non-success branch where self.emit_mark(...) is called), so change
those error returns to include only a redacted or truncated summary instead of
the raw payload: call redact_remote_error_payload(status.as_u16(), &payload) (or
use the same redaction helper used for marks) and include only the status and
that redacted/truncated string in the FlowError::Internal message; apply the
same change to the other non-success branches mentioned (around the other ranges
in this file) and update the corresponding assertions in
crates/core/tests/unit/plugins/nemo_guardrails/remote_tests.rs to expect the
redacted/truncated summary rather than the full body.
🪄 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: 57ed3831-20c6-4ff7-beb3-261e5b5cda9f
📒 Files selected for processing (3)
crates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rscrates/core/tests/unit/plugins/nemo_guardrails/remote_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (12)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Usecargo fmtfor Rust code formatting (rustfmt defaults)
Usecargo clippy -- -D warningsfor Rust linting, treating all warnings as errors
Use Rustsnake_casenaming convention for Rust code
Runjust test-rustfor Rust test suiteUse
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
Files:
crates/core/tests/unit/plugins/nemo_guardrails/remote_tests.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rs
**/*.{rs,go,js,ts,jsx,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license header with format:
// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0in Rust, Go, JavaScript, and TypeScript files
Files:
crates/core/tests/unit/plugins/nemo_guardrails/remote_tests.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.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/remote_tests.rscrates/core/tests/unit/plugins/nemo_guardrails/component_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/tests/unit/plugins/nemo_guardrails/remote_tests.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_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/tests/unit/plugins/nemo_guardrails/remote_tests.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_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/tests/unit/plugins/nemo_guardrails/remote_tests.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.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/tests/unit/plugins/nemo_guardrails/remote_tests.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_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/tests/unit/plugins/nemo_guardrails/remote_tests.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.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/tests/unit/plugins/nemo_guardrails/remote_tests.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rs
**/*.{rs,py,js,ts,tsx,jsx,go,sh,bash,yml,yaml,toml,json,md,mjs,cjs}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.
Files:
crates/core/tests/unit/plugins/nemo_guardrails/remote_tests.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_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/tests/unit/plugins/nemo_guardrails/remote_tests.rscrates/core/src/plugins/nemo_guardrails/remote.rscrates/core/tests/unit/plugins/nemo_guardrails/component_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/remote_tests.rscrates/core/tests/unit/plugins/nemo_guardrails/component_tests.rs
🔇 Additional comments (1)
crates/core/tests/unit/plugins/nemo_guardrails/component_tests.rs (1)
859-860: LGTM!
Signed-off-by: Alex Fournier <afournier@nvidia.com>
|
/ok to test 38aa2e3 |
| { | ||
| let register_builtins = || { | ||
| crate::observability::plugin_component::register_observability_component()?; | ||
| crate::plugins::nemo_guardrails::component::register_nemo_guardrails_component() |
There was a problem hiding this comment.
Shouldn't this be guarded on !wasm ?
There was a problem hiding this comment.
I think “known but unavailable” is the better behavior.
but If you specifically want builtin registration to mean “fully usable on this target,” then yes, we should gate it.
| }) | ||
| } | ||
|
|
||
| async fn execute(&self, request: LlmRequest, stream: bool) -> crate::error::Result<Json> { |
There was a problem hiding this comment.
cognitive complexity of this function is really high -- can we factor out common logic? (same with execute_stream)
There was a problem hiding this comment.
Done.
I factored the common request/response plumbing out of execute / execute_stream, and also pulled the spawned SSE decode loop into a helper so the top-level methods read more as orchestration than transport detail.
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
2 participants
Overview
Adds the first real built-in
nemo_guardrailsremote backend for NeMo Relay core.This PR moves beyond the contract-only surface and implements the first shippable remote slice:
built-in plugin auto-registration in core
remote backend activation for
mode = remotenon-streaming and streaming LLM execution through the Guardrails server
managed
tool_outputexecution checks through the same remote backendrequest-default pass-through for remote Guardrails request semantics
explicit validation that stock remote mode does not currently support managed
tool_inputfocused runtime validation and coverage for transport, malformed responses, tool rewrites, and error handling
I confirm this contribution is my own work, or I have the right to submit it under this project's license.
I searched existing issues and open pull requests, and this does not duplicate existing work.
Details
nemo_guardrailsplugin throughensure_builtin_plugins_registered()guardrails-remotefor the remote backend dependency pathcrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/remote.rscodec = "openai_chat"contextthread_idstaterailsllm_paramsllm_outputoutput_varslogtool_outputexecution checks via the remote backendtool_inputin stock remote mode with a focused validation error instead of silently leaving it non-enforcingnemo_guardrails.remote.*marks for both LLM and managed tool remote checkscomponent_tests.rsandremote_tests.rsto keep contract/config tests separate from transport/runtime behaviortool_outputblocking and rewrite behaviorRail and observability boundary in this PR:
inputoutputtool_outputtool_inputremains a known NeMo Relay surface, but stock remote mode now rejects it explicitly because the Guardrails remote contract does not currently activate pre-execution tool-call rails from externally submitted chat-completions history.request_defaults.rails.inputrequest_defaults.rails.outputrequest_defaults.rails.retrievalrequest_defaults.rails.dialogrequest_defaults.rails.tool_inputrequest_defaults.rails.tool_outputretrievalanddialogare supported this way only: NeMo Relay does not currently expose separate managed retrieval or dialog execution surfaces to intercept locally.nemo_guardrails.remote.startnemo_guardrails.remote.endnemo_guardrails.remote.errortool_outputremote checks. They provide backend-level visibility into remote Guardrails activity without introducing separate NeMo Relay-native retrieval or dialog scopes in this slice.Intentional PR2 boundary:
openai_chatonlytool_outputbut not managedtool_inputValidation:
cargo fmt --allcargo test -p nemo-relay nemo_guardrails --lib --testscargo clippy --workspace --all-targets -- -D warningsuv run pre-commit run --files crates/core/src/plugins/nemo_guardrails/component.rs crates/core/tests/unit/plugins/nemo_guardrails/component_tests.rs crates/core/tests/unit/plugins/nemo_guardrails/remote_tests.rsTargeted live validation covered:
tool_outputblocked pathtool_inputNotes:
uv run pre-commit run --all-filesstill reports an unrelated repo-environment failure intyfor unresolveddeepagentsintegration importspackage-lock.jsonchurn was restored and is not part of this PRWhere should the reviewer start?
Start in
crates/core/src/plugins/nemo_guardrails/component.rs, thencrates/core/src/plugins/nemo_guardrails/remote.rs.The most important design decision in this PR is that the built-in plugin now implements a real remote execution backend while keeping the remote contract honest:
tool_outputexecution are supported through the core runtimetool_inputis explicitly rejected instead of being left as a silent best-effort pathRelated Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)