feat: implement nemo_guardrails remote backend#144
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (3)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (12)**/*.rs📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{rs,go,js,ts,jsx,tsx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/{Cargo.toml,**/*.rs}📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Files:
**/*.{h,hpp,c,cpp,rs}📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Files:
{crates/core,crates/adaptive}/**/*📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Files:
**/*.{rs,toml}📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Files:
crates/core/**/*.rs📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
Files:
crates/{core,adaptive}/**📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
**/*.{rs,py,js,ts,tsx,jsx,go,sh,bash,yml,yaml,toml,json,md,mjs,cjs}📄 CodeRabbit inference engine (AGENTS.md)
Files:
crates/{core,adaptive}/**/*.rs⚙️ CodeRabbit configuration file
Files:
{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)
Files:
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (11)
WalkthroughAdds a feature-gated remote backend for NeMo Guardrails: new ChangesRemote NeMo Guardrails Backend
Sequence DiagramsequenceDiagram
participant Plugin as NeMo Guardrails Plugin
participant Runtime as RemoteBackendRuntime
participant Remote as Remote Endpoint
participant Tool as Tool Execution
participant Events as EventEmitter
Plugin->>Runtime: register (remote mode config)
Runtime->>Runtime: build reqwest client
Plugin->>Runtime: execute LLM request
Runtime->>Events: emit nemo_guardrails.remote.start
Runtime->>Runtime: build request body with guardrails + context/thread/state
Runtime->>Remote: POST /v1/chat/completions (stream=false|true)
alt HTTP Success (non-stream)
Remote-->>Runtime: JSON response
Runtime->>Events: emit nemo_guardrails.remote.end
Runtime-->>Plugin: return parsed JSON
else Streaming SSE
Remote-->>Runtime: SSE chunks
Runtime->>Plugin: forward LlmJsonStream chunks
Runtime->>Events: emit nemo_guardrails.remote.end (on completion)
else HTTP/transport error
Remote-->>Runtime: error/timeout/status
Runtime->>Events: emit nemo_guardrails.remote.error
Runtime-->>Plugin: return error
end
Plugin->>Runtime: intercept tool execution
Runtime->>Remote: POST tool_input check (stream=false)
Remote-->>Runtime: guardrails.log.activated_rails
alt blocking rail found
Runtime->>Events: emit tool_error
Runtime-->>Tool: reject execution
else pass or rewrite
Runtime-->>Tool: execute with original/rewritten arguments
Tool-->>Runtime: result
Runtime->>Remote: POST tool_output check
Remote-->>Runtime: guardrails.log
Runtime-->>Plugin: return original/rewritten result or reject if blocked
end
Estimated code review effort🎯 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.
| // SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| //! Unit tests for the planned NeMo Guardrails plugin component contract. | ||
| #![allow(clippy::await_holding_lock)] |
There was a problem hiding this comment.
Run the required Rust/core validation matrix before merge.
The provided validation evidence only shows a targeted cargo test invocation. For this crates/core Rust change, the required formatting/lint/test commands and full language matrix are not evidenced.
As per coding guidelines, “If any Rust code changed, always run just test-rust… also run cargo fmt --all… also run cargo clippy --workspace --all-targets -- -D warnings” and “Changes to crates/core or crates/adaptive must run the full language matrix.”
🤖 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/tests/unit/plugins/nemo_guardrails/component_tests.rs` around
lines 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.
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.
|
|
||
| #[cfg(all(not(target_arch = "wasm32"), feature = "guardrails-remote"))] |
There was a problem hiding this comment.
If you have this at the top of the file, I don't believe you need it on every single include:
#![cfg(all(not(target_arch = "wasm32"), feature = "guardrails-remote"))]| #[cfg(all(feature = "guardrails-remote", not(target_arch = "wasm32")))] | ||
| #[path = "remote.rs"] | ||
| mod remote; |
There was a problem hiding this comment.
Actually, since you have this, I don't think you need all of the cfg entries in remote.rs
|
|
||
| #[cfg(all(not(target_arch = "wasm32"), feature = "guardrails-remote"))] | ||
| impl RemoteBackendRuntime { | ||
| fn new(config: &NeMoGuardrailsConfig, remote: &RemoteBackendConfig) -> PluginResult<Self> { |
There was a problem hiding this comment.
Doesn't NeMoGuardrailsConfig already hold the optional RemoteBackendConfig ?
Shouldn't we just access that?
| FlowError::Internal("LLM request content is not a JSON object".to_string()) | ||
| })?; | ||
| body.insert("stream".to_string(), Json::Bool(stream)); | ||
| if let Some(guardrails) = self.build_guardrails_config() { |
There was a problem hiding this comment.
We should not have to build the guardrails config with each outgoing request. Recommendation: hoist all logic as part of Runtime initialization
There was a problem hiding this comment.
This file appears to be doing more than just the component contract. Maybe split remote tests into their own file.
|
/ok to test 60999ac |
2 participants
Overview
Adds the first real built-in
nemo_guardrailsremote backend for NeMo Flow 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_inputandtool_outputexecution checks through the same remote backendrequest-default pass-through for remote Guardrails request semantics
focused 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/plugin_component.rscodec = "openai_chat"contextthread_idstaterailsllm_paramsllm_outputoutput_varslogtool_inputandtool_outputexecution checks via the remote backendnemo_guardrails.remote.*marks for both LLM and managed tool remote checksIntentional PR2 boundary:
openai_chatonlyValidation:
cargo test -p nemo-flow nemo_guardrails --lib --testsTargeted live smoke against a local Guardrails server covered:
tool_inputblocked pathtool_outputblocked pathNotes:
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/plugin_component.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:
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
New Features
Validation
Tests