chore: address Sonar code quality findings - #738
Conversation
Signed-off-by: Will Killian <wkillian@nvidia.com>
github-actions
Bot
added
size:XL
WalkthroughThis refactor extracts validation, lifecycle, callback, stream-processing, test, and reporting logic into dedicated helpers across Rust, Go, and JavaScript modules. Observable validation, cleanup, rollback, callback, rendering, and test behavior remains unchanged. ChangesCross-component helper extraction
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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/cli/src/diagnostics/mod.rs`:
- Around line 924-926: Update the check-details formatting around the probe-mode
conditional so every result, including offline results, is prefixed with
endpoints[{index}] ({endpoint_type}). Preserve the existing details text after
the prefix, and add offline coverage for indexed successful and failed endpoint
checks.
In `@crates/core/src/observability/plugin_component.rs`:
- Around line 989-1008: Update write_atif_shutdown_exports in
crates/core/src/observability/plugin_component.rs:989-1008 to record the first
export error, continue attempting every PendingAtifExport, and return the
recorded error only after the loop. In flush_atif_shutdown_work and
atif_shutdown_cleanup at
crates/core/src/observability/plugin_component.rs:952-987, preserve the prior
ordering by returning collected AtifFlushWork alongside any scope-subscriber
deregistration error, so cleanup writes all exports before reporting NotRemoved.
In `@crates/node/src/callable.rs`:
- Around line 1260-1322: Add regression tests covering duplicate completion
calls and errors during subscriber callback preparation or event value
conversion, using create_js_subscriber_completion_callback and
create_js_event_subscriber_function paths. Assert each scenario causes
flushSubscribers() to resolve and removes the pending callback ID, while
preserving the existing delivery, callback failure, registration, and
deregistration tests.
- Around line 1274-1288: The preparation path in safe_subscriber_callback must
release callback_id before propagating errors from
create_js_subscriber_completion_callback or event conversion. Invoke
complete_js_subscriber_callback(callback_id) in each failure branch, preserving
successful callback construction, and add a regression test covering both
preparation-error paths and ensuring pending callbacks can be flushed.
In `@scripts/latency_benchmark/src/report/report.js`:
- Around line 77-82: Add focused regression tests for the changed report logic:
execute and verify formatMs at magnitudes 10 and 100, strokeDasharray for series
indices 1, 2, and other values, and gateway descriptions for absolute, minimal,
and default views. Place the coverage in the existing JavaScript test setup
rather than only the embedding test, and run just test-node before handoff.
🪄 Autofix
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: 0ce7d4a7-8ac9-4c2c-b25c-0bb75e5a0ea1
📒 Files selected for processing (12)
crates/adaptive/src/response_cache/key.rscrates/cli/src/diagnostics/mod.rscrates/core/src/codec/gemini_generate_content.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/core/src/plugins/nemo_guardrails/python.rscrates/node/src/callable.rscrates/pii-redaction/src/overlay.rsgo/nemo_relay/logging_test.gogo/nemo_relay/plugin_activation_test.goscripts/latency_benchmark/src/report/report.jsscripts/latency_benchmark/src/report/template.html
💤 Files with no reviewable changes (1)
- scripts/latency_benchmark/src/report/template.html
📜 Review details
⏰ Context from checks skipped due to timeout. (44)
- GitHub Check: Node.js / Package (windows-arm64)
- GitHub Check: Rust / Package (linux-arm64)
- GitHub Check: Node.js / Package (linux-musl-amd64)
- GitHub Check: Python / Test (linux-arm64)
- GitHub Check: Python / Package (windows-arm64)
- GitHub Check: Python / Package (windows-amd64)
- GitHub Check: Python / Package (linux-musl-amd64)
- GitHub Check: Python / Package (linux-musl-arm64)
- GitHub Check: Python / Test (macos-arm64)
- GitHub Check: Node.js / Package (macos-arm64)
- GitHub Check: Python / Package (macos-arm64)
- GitHub Check: Python / Test (linux-amd64)
- GitHub Check: Node.js / Package (linux-musl-arm64)
- GitHub Check: Node.js / Package (linux-arm64)
- GitHub Check: Node.js / Package (windows-amd64)
- GitHub Check: Go / Test (macos-arm64)
- GitHub Check: Python / Package (linux-arm64)
- GitHub Check: Python / Package (linux-amd64)
- GitHub Check: Python / Test (windows-amd64)
- GitHub Check: Node.js / Package (linux-amd64)
- GitHub Check: Go / Test (windows-arm64)
- GitHub Check: Rust / Package (windows-arm64)
- GitHub Check: Go / Test (linux-amd64)
- GitHub Check: Go / Test (windows-amd64)
- GitHub Check: Node.js / Test (windows-arm64)
- GitHub Check: Python / Test (windows-arm64)
- GitHub Check: Rust / Package (linux-musl-arm64)
- GitHub Check: Rust / Package (macos-arm64)
- GitHub Check: Go / Test (linux-arm64)
- GitHub Check: Rust / Package (linux-musl-amd64)
- GitHub Check: Rust / Package (linux-amd64)
- GitHub Check: Rust / Package (windows-amd64)
- GitHub Check: Node.js / Test (linux-arm64)
- GitHub Check: Node.js / Test (windows-amd64)
- GitHub Check: Node.js / Test (linux-amd64)
- GitHub Check: Node.js / Test (macos-arm64)
- GitHub Check: Rust / Test (linux-amd64)
- GitHub Check: Rust / Test (windows-amd64)
- GitHub Check: Rust / Test (windows-arm64)
- GitHub Check: Rust / Test (macos-arm64)
- GitHub Check: Check / Run
- GitHub Check: Rust / Test (linux-arm64)
- GitHub Check: Node.js / Package OpenClaw plugin
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (33)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.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: 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.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
**/*.rs: Format Rust code with rustfmt defaults usingcargo fmt.
Runcargo clippy -- -D warnings; all Rust warnings must be treated as errors.
Use Rustsnake_casenaming conventions.
Files:
crates/adaptive/src/response_cache/key.rscrates/node/src/callable.rscrates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/cli/src/diagnostics/mod.rscrates/pii-redaction/src/overlay.rscrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.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/adaptive/src/response_cache/key.rscrates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.rs
crates/adaptive/**
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep
crates/adaptivealigned with the canonical adaptive config schema, built-in section helpers, plugin lifecycle, and validation/report behavior.
Files:
crates/adaptive/src/response_cache/key.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
**/*: Use release tags in raw Rust-compatible SemVer without a leadingv; tags such asv0.1.0are prohibited.
Use branch prefixesfeat/,fix/,docs/,test/, orrefactor/according to the change purpose.
Every commit in a pull request must include a DCOSigned-off-by:sign-off.
Before submitting a pull request, ensure pre-commit hooks, relevant tests, target-specific builds, documentation updates, and a rebase on the latestmainare complete.
Use commit messages in the formtype: short description, with a valid type and a first line under 72 characters.
Files:
crates/adaptive/src/response_cache/key.rscrates/node/src/callable.rsgo/nemo_relay/logging_test.gocrates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/cli/src/diagnostics/mod.rsscripts/latency_benchmark/src/report/report.jscrates/pii-redaction/src/overlay.rsgo/nemo_relay/plugin_activation_test.gocrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/adaptive/src/response_cache/key.rscrates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/adaptive/src/response_cache/key.rscrates/node/src/callable.rsgo/nemo_relay/logging_test.gocrates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/cli/src/diagnostics/mod.rsscripts/latency_benchmark/src/report/report.jscrates/pii-redaction/src/overlay.rsgo/nemo_relay/plugin_activation_test.gocrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/adaptive/src/response_cache/key.rscrates/node/src/callable.rsgo/nemo_relay/logging_test.gocrates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/cli/src/diagnostics/mod.rsscripts/latency_benchmark/src/report/report.jscrates/pii-redaction/src/overlay.rsgo/nemo_relay/plugin_activation_test.gocrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.rs
**/*.{rs,py,js,jsx,ts,tsx,go,c,h,cc,cpp,md,toml,yml,yaml,sh}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, documentation, scripts, and configuration files; the project is Apache-2.0.
Files:
crates/adaptive/src/response_cache/key.rscrates/node/src/callable.rsgo/nemo_relay/logging_test.gocrates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/cli/src/diagnostics/mod.rsscripts/latency_benchmark/src/report/report.jscrates/pii-redaction/src/overlay.rsgo/nemo_relay/plugin_activation_test.gocrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Use
snake_casenaming in Rust and Python.
Files:
crates/adaptive/src/response_cache/key.rscrates/node/src/callable.rscrates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/cli/src/diagnostics/mod.rscrates/pii-redaction/src/overlay.rscrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: UseJson = serde_json::Valuein Rust-facing runtime APIs where existing code expects JSON payloads.
Treat Rust as the source of truth for runtime behavior; binding APIs should mirror Rust semantics unless a language-specific wrapper intentionally improves ergonomics.
Files:
crates/adaptive/src/response_cache/key.rscrates/node/src/callable.rscrates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/cli/src/diagnostics/mod.rscrates/pii-redaction/src/overlay.rscrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.rs
**/*.{rs,py,js,mjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Preserve the existing Tokio-based asynchronous model and callback/future lifetimes; do not unexpectedly block or hide async work in bindings.
Files:
crates/adaptive/src/response_cache/key.rscrates/node/src/callable.rscrates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/cli/src/diagnostics/mod.rsscripts/latency_benchmark/src/report/report.jscrates/pii-redaction/src/overlay.rscrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.rs
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}: Run tests for every language affected by a change; changes to the core Rust crate require tests across all bindings.
UseSONAR_IGNORE_START/SONAR_IGNORE_ENDonly for documented false positives, keep ignored blocks minimal, explain them with a comment, and obtain reviewer sign-off.
Preserve the layered architecture in which Rust provides the core runtime and C FFI, PyO3, and NAPI provide bindings that mirror the full API surface.
Files:
crates/adaptive/src/response_cache/key.rscrates/node/src/callable.rsgo/nemo_relay/logging_test.gocrates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/cli/src/diagnostics/mod.rsscripts/latency_benchmark/src/report/report.jscrates/pii-redaction/src/overlay.rsgo/nemo_relay/plugin_activation_test.gocrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.rs
**/*.{rs,py,go,js,jsx,ts,tsx,c,h,html,md,mdx,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include the appropriate SPDX copyright and Apache-2.0 license header in every source file.
Files:
crates/adaptive/src/response_cache/key.rscrates/node/src/callable.rsgo/nemo_relay/logging_test.gocrates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/cli/src/diagnostics/mod.rsscripts/latency_benchmark/src/report/report.jscrates/pii-redaction/src/overlay.rsgo/nemo_relay/plugin_activation_test.gocrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.rs
crates/**/src/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees.
Files:
crates/adaptive/src/response_cache/key.rscrates/node/src/callable.rscrates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/cli/src/diagnostics/mod.rscrates/pii-redaction/src/overlay.rscrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.rs
**/*.{rs,py,go,js,ts,mdx}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
**/*.{rs,py,go,js,ts,mdx}: When changing observability event fields, exporter behavior, subscriber configuration, or binding parity, keep the core event model, ATIF, typed OpenTelemetry projections, FFI wrappers, language bindings, configuration, and documentation synchronized.
Examples and documentation must use each exporter's documented flush/deregister order before shutdown.
Files:
crates/adaptive/src/response_cache/key.rscrates/node/src/callable.rsgo/nemo_relay/logging_test.gocrates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/cli/src/diagnostics/mod.rsscripts/latency_benchmark/src/report/report.jscrates/pii-redaction/src/overlay.rsgo/nemo_relay/plugin_activation_test.gocrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.rs
crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-rust-core/SKILL.md)
crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*.rs: For changes in the Rust core, adaptive, plugin, worker, worker-proto, or types crates, runcargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings.
Use narrower crate tests only as a local debugging loop; final Rust validation must use the sharedjust test-rustworkflow.
For changes tocrates/core,crates/adaptive, or shared runtime semantics, expand validation to the full binding matrix withvalidate-changeorjust ci=true test-rust.
When public APIs, event shapes, middleware behavior, plugin semantics, or core/adaptive behavior change, also runvalidate-change.
If a change is isolated to one binding wrapper with unchanged Rust semantics, prefer that binding's build and test skill instead.
Files:
crates/adaptive/src/response_cache/key.rscrates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.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/adaptive/src/response_cache/key.rscrates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.rs
crates/{python,ffi,node}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node}/**/*: 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/node/src/callable.rs
go/nemo_relay/**/*.go
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
go/nemo_relay/**/*.go: Format changed Go packages withcd go/nemo_relay && go fmt ./...
Run Go tests withjust test-goto build and test the NeMo Relay Go binding
Usejust build-gowhen you want an explicit build-only pass or need the artifact for other work
Usejust ci=true test-gowhen you need the CI-style coverage and JUnit path
On macOS, setDYLD_LIBRARY_PATHto the../../target/releasedirectory before running the rawgo testcommand directly
Files:
go/nemo_relay/logging_test.gogo/nemo_relay/plugin_activation_test.go
{crates/python/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update the language-native bindings for every exposed surface in Python, Go, and Node.js.
Files:
go/nemo_relay/logging_test.gogo/nemo_relay/plugin_activation_test.go
{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update language wrapper helpers such as Python wrapper modules, Python type stubs, and Go shorthand packages when the new behavior belongs in those helper layers.
Files:
go/nemo_relay/logging_test.gogo/nemo_relay/plugin_activation_test.go
go/nemo_relay/**
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep shared plugin helpers in
go/nemo_relayaligned with plugin registration, composition, and lifecycle behavior.
Files:
go/nemo_relay/logging_test.gogo/nemo_relay/plugin_activation_test.go
go/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Use
PascalCasefor public Go APIs.
Files:
go/nemo_relay/logging_test.gogo/nemo_relay/plugin_activation_test.go
**/*.go
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.go: Format Go code withgofmt.
Rungo vet ./...for Go static analysis.
Use GoPascalCasenaming conventions.
Files:
go/nemo_relay/logging_test.gogo/nemo_relay/plugin_activation_test.go
**/*.{py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
**/*.{py,go,js,ts}: Ensure Python, Go, and Node.js configuration objects and subscriber/exporter methods expose the same logical knobs and semantics as the core implementation.
Runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
go/nemo_relay/logging_test.goscripts/latency_benchmark/src/report/report.jsgo/nemo_relay/plugin_activation_test.go
go/nemo_relay/**/*
⚙️ CodeRabbit configuration file
go/nemo_relay/**/*: Review Go binding changes for cgo memory ownership, race safety, callback cleanup, idiomatic exported APIs, and parity with Rust/FFI behavior.
Any API change should include focused Go tests and consider race-test behavior.
Files:
go/nemo_relay/logging_test.gogo/nemo_relay/plugin_activation_test.go
{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:
go/nemo_relay/logging_test.gogo/nemo_relay/plugin_activation_test.go
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 validationUse
Result<T>withFlowErrorin core runtime paths, keeping wrapper-layer errors explicit and binding-appropriate.Run affected Rust crate tests, and run
just test-rustwhen event fields change.
Files:
crates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.rs
crates/core/src/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
crates/core/src/**/*.rs:plugins list,plugins inspect, andplugins validatemust report lifecycle and compatibility status without leaking secret configuration.
Top-leveldoctormust report resolved dynamic-plugin and host-configuration status.
Files:
crates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.rs
{crates/plugin/**/*.rs,crates/worker-proto/**/*.rs,crates/worker/**/*.rs,crates/types/**/*.rs,crates/core/**/*.{rs,toml},python/plugin/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Run the targeted Rust, Python plugin, integration, documentation, and broader validation commands when validating dynamic-plugin changes.
Files:
crates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.rs
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use Node.js
camelCasenaming conventions.
Files:
scripts/latency_benchmark/src/report/report.js
{.github/**,.gitlab-ci.yml,.pre-commit-config.yaml,justfile,scripts/**}
⚙️ CodeRabbit configuration file
{.github/**,.gitlab-ci.yml,.pre-commit-config.yaml,justfile,scripts/**}: Review automation changes for reproducibility, pinned versions where appropriate, secret handling, and consistency with the documented validation matrix.
Pay attention to commands that need generated native artifacts, FFI libraries, or platform-specific environment variables.
Files:
scripts/latency_benchmark/src/report/report.js
crates/core/src/{api/**/*.rs,api/runtime/**/*.rs,codec/**/*.rs,json.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Implement the new or changed public runtime behavior first in the Rust core, especially under
crates/core/src/api/and related core modules such ascrates/core/src/api/runtime/,crates/core/src/codec/, andcrates/core/src/json.rs.
Files:
crates/core/src/codec/gemini_generate_content.rs
🧠 Learnings (5)
📚 Learning: 2026-08-03T19:55:03.931Z
Learnt from: afourniernv
Repo: NVIDIA/NeMo-Relay PR: 558
File: crates/pii-redaction/src/rampart/mod.rs:265-274
Timestamp: 2026-08-03T19:55:03.931Z
Learning: In NeMo Relay first-party plugin registration helpers, treat the documented duplicate-registration `PluginError::RegistrationFailed` result from `register_plugin` as success when registration is intended to be idempotent. Do not locally reclassify this as `PluginError::Conflict`; changing the classification requires a core-wide review of the public API and FFI behavior.
Applied to files:
crates/adaptive/src/response_cache/key.rscrates/node/src/callable.rscrates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/cli/src/diagnostics/mod.rscrates/pii-redaction/src/overlay.rscrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.rs
📚 Learning: 2026-07-28T20:33:25.156Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 572
File: go/nemo_relay/adaptive_runtime_test.go:214-238
Timestamp: 2026-07-28T20:33:25.156Z
Learning: When adding/adjusting Go unit tests for `BuildCacheRequestFacts` (request-ID validation and related request parsing), set `CacheRequestFactsInput.Provider` to a valid provider in all tests that are intended to isolate request-ID behavior—because `BuildCacheRequestFacts` does not validate `Provider`. Then add separate test coverage for malformed `AnnotatedRequest` JSON so JSON parsing failures are not conflated with `Provider`-related inputs.
Applied to files:
go/nemo_relay/logging_test.gogo/nemo_relay/plugin_activation_test.go
📚 Learning: 2026-07-28T20:07:29.880Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 571
File: crates/core/src/api/runtime/state.rs:996-1020
Timestamp: 2026-07-28T20:07:29.880Z
Learning: In NeMo Relay (RELAY-509), sanitizer callback failures must be treated as intentional fail-open behavior. When an event/tool (request/response) or LLM (request/response) sanitizer callback fails, the sanitizer chain should retain and publish the last valid event/payload snapshot (rather than dropping/invalidating the data) and log the failure including callback context (e.g., which sanitizer/callback failed and relevant identifiers). Apply this consistently across all sanitizer chains mentioned in the RELAY-509 documentation/migration guide.
Applied to files:
crates/core/src/plugins/nemo_guardrails/python.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/core/src/codec/gemini_generate_content.rs
📚 Learning: 2026-08-03T19:54:17.292Z
Learnt from: afourniernv
Repo: NVIDIA/NeMo-Relay PR: 558
File: crates/pii-redaction/src/rampart/mod.rs:486-492
Timestamp: 2026-08-03T19:54:17.292Z
Learning: In the Rust plugin configuration editor, use literal enum values for codec lists because `nemo_relay::editor_config!` generates static editor metadata. Do not replace these literals with a shared source such as `supported_codec_names()` unless the core editor-metadata API is first changed to support dynamic values.
Applied to files:
crates/pii-redaction/src/overlay.rs
📚 Learning: 2026-08-03T19:54:44.552Z
Learnt from: afourniernv
Repo: NVIDIA/NeMo-Relay PR: 558
File: crates/pii-redaction/src/rampart/mod.rs:670-792
Timestamp: 2026-08-03T19:54:44.552Z
Learning: In the Rust `crates/pii-redaction` crate, treat `pii_rampart` as a first-party built-in component. Preserve private `#[cfg(test)]` unit-test modules under `src`, including Rampart tests, according to the crate’s existing convention. Do not request moving these tests to `tests/` when doing so would require widening implementation visibility solely for test access.
Applied to files:
crates/pii-redaction/src/overlay.rs
🔇 Additional comments (32)
crates/cli/src/diagnostics/mod.rs (1)
1230-1297: LGTM!crates/core/src/plugin.rs (6)
1581-1602: LGTM!
1604-1620: LGTM!
1622-1647: LGTM!
1649-1686: LGTM!
1688-1712: LGTM!
1714-1749: LGTM!crates/core/src/observability/plugin_component.rs (2)
946-950: LGTM!
952-960: 📐 Maintainability & Code QualityRun the full validation matrix for this
crates/corechange.This change touches
crates/core. Runcargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings. Then expand validation across Rust, Python, Go, and Node.js.As per coding guidelines: "For changes in the Rust core, adaptive, plugin, worker, worker-proto, or types crates, run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings" and "Changes tocrates/coreorcrates/adaptivemust run the full language matrix".Source: Coding guidelines
go/nemo_relay/logging_test.go (3)
23-24: LGTM!
46-80: LGTM!
82-117: LGTM!go/nemo_relay/plugin_activation_test.go (5)
22-30: LGTM!
605-605: LGTM!Also applies to: 646-646, 891-891
656-662: LGTM!Also applies to: 804-810, 900-904
675-675: LGTM!Also applies to: 687-688, 720-724, 758-758
831-831: 📐 Maintainability & Code QualityRun the Go validation commands for this binding change.
Run
cd go/nemo_relay && go fmt ./...,go vet ./..., andjust test-go. The constant extraction changes format-string call sites, andgo vetprintf analysis confirms the argument counts still match.As per coding guidelines: "Format changed Go packages with
cd go/nemo_relay && go fmt ./..." and "Run Go tests withjust test-goto build and test the NeMo Relay Go binding".Source: Coding guidelines
crates/node/src/callable.rs (1)
1230-1231: LGTM!crates/core/src/codec/gemini_generate_content.rs (9)
578-609: LGTM!
611-706: LGTM!
708-799: LGTM!
1381-1393: LGTM!
2153-2211: LGTM!
2213-2387: LGTM!
2503-2650: LGTM!
2778-2918: LGTM!
2652-2664: 📐 Maintainability & Code QualityNo toolchain change is needed. The workspace uses Rust edition 2024 and pins Rust 1.96.1, which supports this let-chain syntax.
> Likely an incorrect or invalid review comment.crates/adaptive/src/response_cache/key.rs (1)
456-505: LGTM!crates/core/src/plugins/nemo_guardrails/python.rs (1)
1308-1369: LGTM!crates/pii-redaction/src/overlay.rs (3)
102-108: LGTM!
110-148: LGTM!
160-178: 🗄️ Data Integrity & IntegrationNo change is required for surplus tool calls.
AnnotatedLlmResponse.tool_callscomes from the raw response. Sanitization preserves or removes entries; it does not add entries. No append is required.> Likely an incorrect or invalid review comment.
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/node/src/callable.rs (1)
1283-1288: 🩺 Stability & Availability | 🟠 MajorRelease the callback ID when event conversion fails.
Json::to_napi_value(...)?exits the ThreadsafeFunction callback beforecomplete_subscriber_callback_on_errorreceives anErr. The reservedcallback_idremains inpending, soflush_js_subscriber_callbacks()can wait indefinitely.Build the
napi::Result<JsUnknown>first, then pass it tocomplete_subscriber_callback_on_error. The new test only passes synthetic errors to the helper. It does not execute the event-conversion path.Proposed fix
- let event = complete_subscriber_callback_on_error(callback_id, unsafe { - Ok(JsUnknown::from_raw_unchecked( - ctx.env.raw(), - Json::to_napi_value(ctx.env.raw(), event)?, - )) - })?; + let event = complete_subscriber_callback_on_error( + callback_id, + Json::to_napi_value(ctx.env.raw(), event).map(|value| unsafe { + JsUnknown::from_raw_unchecked(ctx.env.raw(), value) + }), + )?;Also applies to: 1619-1626
🤖 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/node/src/callable.rs` around lines 1283 - 1288, In the callback handling flow around complete_subscriber_callback_on_error, construct the napi::Result<JsUnknown> separately so Json::to_napi_value conversion errors are passed to the helper rather than returned before it runs. Apply the same change to the corresponding flow near the other reported location, ensuring callback_id is released through the helper on both success and conversion failure.
🤖 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/node/src/callable.rs`:
- Around line 1605-1628: Move the #[cfg(test)] tests from callable.rs into the
Node crate’s tests/ tree, rewriting them to exercise the public subscriber
callback path rather than private helpers such as reserve_js_subscriber_callback
and complete_subscriber_callback_on_error. If direct coverage remains necessary,
add a narrowly scoped test seam while preserving the public API and callback
error propagation behavior.
---
Duplicate comments:
In `@crates/node/src/callable.rs`:
- Around line 1283-1288: In the callback handling flow around
complete_subscriber_callback_on_error, construct the napi::Result<JsUnknown>
separately so Json::to_napi_value conversion errors are passed to the helper
rather than returned before it runs. Apply the same change to the corresponding
flow near the other reported location, ensuring callback_id is released through
the helper on both success and conversion failure.
🪄 Autofix
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: 4428a812-a300-4e40-84e1-301fff62ca26
📒 Files selected for processing (1)
crates/node/src/callable.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (34)
- GitHub Check: Python / Package (linux-musl-arm64)
- GitHub Check: Node.js / Package (macos-arm64)
- GitHub Check: Python / Package (linux-musl-amd64)
- GitHub Check: Node.js / Package (linux-musl-arm64)
- GitHub Check: Node.js / Package (linux-musl-amd64)
- GitHub Check: Node.js / Package (linux-amd64)
- GitHub Check: Python / Package (macos-arm64)
- GitHub Check: Node.js / Package (windows-arm64)
- GitHub Check: Python / Package (linux-arm64)
- GitHub Check: Node.js / Package (windows-amd64)
- GitHub Check: Python / Package (windows-amd64)
- GitHub Check: Python / Package (windows-arm64)
- GitHub Check: Go / Test (macos-arm64)
- GitHub Check: Node.js / Test (windows-amd64)
- GitHub Check: Go / Test (windows-amd64)
- GitHub Check: Go / Test (windows-arm64)
- GitHub Check: Check / Run
- GitHub Check: Node.js / Test (macos-arm64)
- GitHub Check: Node.js / Test (linux-arm64)
- GitHub Check: Rust / Package (windows-amd64)
- GitHub Check: Go / Test (linux-amd64)
- GitHub Check: Rust / Test (windows-amd64)
- GitHub Check: Rust / Test (linux-arm64)
- GitHub Check: Rust / Package (windows-arm64)
- GitHub Check: Python / Test (windows-amd64)
- GitHub Check: Python / Test (macos-arm64)
- GitHub Check: Rust / Package (linux-amd64)
- GitHub Check: Node.js / Test (windows-arm64)
- GitHub Check: Rust / Package (linux-arm64)
- GitHub Check: Rust / Test (linux-amd64)
- GitHub Check: Rust / Test (windows-arm64)
- GitHub Check: Python / Test (windows-arm64)
- GitHub Check: Node.js / Package OpenClaw plugin
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (13)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.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: 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.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
**/*.rs: Format Rust code with rustfmt defaults usingcargo fmt.
Runcargo clippy -- -D warnings; all Rust warnings must be treated as errors.
Use Rustsnake_casenaming conventions.
Files:
crates/node/src/callable.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
**/*: Use release tags in raw Rust-compatible SemVer without a leadingv; tags such asv0.1.0are prohibited.
Use branch prefixesfeat/,fix/,docs/,test/, orrefactor/according to the change purpose.
Every commit in a pull request must include a DCOSigned-off-by:sign-off.
Before submitting a pull request, ensure pre-commit hooks, relevant tests, target-specific builds, documentation updates, and a rebase on the latestmainare complete.
Use commit messages in the formtype: short description, with a valid type and a first line under 72 characters.
Files:
crates/node/src/callable.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/node/src/callable.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/node/src/callable.rs
**/*.{rs,py,js,jsx,ts,tsx,go,c,h,cc,cpp,md,toml,yml,yaml,sh}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, documentation, scripts, and configuration files; the project is Apache-2.0.
Files:
crates/node/src/callable.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Use
snake_casenaming in Rust and Python.
Files:
crates/node/src/callable.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: UseJson = serde_json::Valuein Rust-facing runtime APIs where existing code expects JSON payloads.
Treat Rust as the source of truth for runtime behavior; binding APIs should mirror Rust semantics unless a language-specific wrapper intentionally improves ergonomics.
Files:
crates/node/src/callable.rs
**/*.{rs,py,js,mjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Preserve the existing Tokio-based asynchronous model and callback/future lifetimes; do not unexpectedly block or hide async work in bindings.
Files:
crates/node/src/callable.rs
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}: Run tests for every language affected by a change; changes to the core Rust crate require tests across all bindings.
UseSONAR_IGNORE_START/SONAR_IGNORE_ENDonly for documented false positives, keep ignored blocks minimal, explain them with a comment, and obtain reviewer sign-off.
Preserve the layered architecture in which Rust provides the core runtime and C FFI, PyO3, and NAPI provide bindings that mirror the full API surface.
Files:
crates/node/src/callable.rs
**/*.{rs,py,go,js,jsx,ts,tsx,c,h,html,md,mdx,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include the appropriate SPDX copyright and Apache-2.0 license header in every source file.
Files:
crates/node/src/callable.rs
crates/**/src/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees.
Files:
crates/node/src/callable.rs
**/*.{rs,py,go,js,ts,mdx}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
**/*.{rs,py,go,js,ts,mdx}: When changing observability event fields, exporter behavior, subscriber configuration, or binding parity, keep the core event model, ATIF, typed OpenTelemetry projections, FFI wrappers, language bindings, configuration, and documentation synchronized.
Examples and documentation must use each exporter's documented flush/deregister order before shutdown.
Files:
crates/node/src/callable.rs
crates/{python,ffi,node}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node}/**/*: 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/node/src/callable.rs
🧠 Learnings (1)
📚 Learning: 2026-08-03T19:55:03.931Z
Learnt from: afourniernv
Repo: NVIDIA/NeMo-Relay PR: 558
File: crates/pii-redaction/src/rampart/mod.rs:265-274
Timestamp: 2026-08-03T19:55:03.931Z
Learning: In NeMo Relay first-party plugin registration helpers, treat the documented duplicate-registration `PluginError::RegistrationFailed` result from `register_plugin` as success when registration is intended to be idempotent. Do not locally reclassify this as `PluginError::Conflict`; changing the classification requires a core-wide review of the public API and FFI behavior.
Applied to files:
crates/node/src/callable.rs
🔇 Additional comments (4)
crates/node/src/callable.rs (4)
1230-1230: LGTM!
1274-1282: LGTM!
1294-1301: LGTM!
1303-1334: 🩺 Stability & AvailabilityNo change needed. Completion remains once-only for each
callback_id, and callback errors retain the existing recording policy.
Signed-off-by: Will Killian <wkillian@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/node/tests/scope_tests.mjs`:
- Around line 485-495: Update the flushSubscribers failure test to assert the
binding’s callback-error channel contains the expected “flush failure” after
awaiting flushSubscribers, proving the subscriber executed and recorded the
error. Reset or clear this shared callback-error channel before the test or in
its finally block, while preserving subscriber deregistration.
🪄 Autofix
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: 879c3568-d59a-434f-972e-1624d12f08a8
📒 Files selected for processing (5)
crates/cli/src/diagnostics/mod.rscrates/cli/tests/coverage/shared/doctor_tests.rscrates/core/src/observability/plugin_component.rscrates/node/src/callable.rscrates/node/tests/scope_tests.mjs
💤 Files with no reviewable changes (1)
- crates/node/src/callable.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (39)
- GitHub Check: Python / Package (linux-musl-arm64)
- GitHub Check: Python / Package (linux-arm64)
- GitHub Check: Python / Package (linux-musl-amd64)
- GitHub Check: Python / Package (linux-amd64)
- GitHub Check: Python / Package (windows-arm64)
- GitHub Check: Python / Test (linux-arm64)
- GitHub Check: Python / Test (windows-arm64)
- GitHub Check: Python / Package (windows-amd64)
- GitHub Check: Node.js / Test (macos-arm64)
- GitHub Check: Node.js / Test (windows-arm64)
- GitHub Check: Python / Test (linux-amd64)
- GitHub Check: Python / Test (windows-amd64)
- GitHub Check: Node.js / Package (linux-musl-amd64)
- GitHub Check: Node.js / Test (linux-amd64)
- GitHub Check: Rust / Package (linux-amd64)
- GitHub Check: Node.js / Test (linux-arm64)
- GitHub Check: Rust / Package (linux-musl-arm64)
- GitHub Check: Rust / Package (linux-arm64)
- GitHub Check: Node.js / Package (macos-arm64)
- GitHub Check: Node.js / Test (windows-amd64)
- GitHub Check: Rust / Package (windows-arm64)
- GitHub Check: Node.js / Package (linux-musl-arm64)
- GitHub Check: Rust / Package (linux-musl-amd64)
- GitHub Check: Rust / Package (windows-amd64)
- GitHub Check: Go / Test (windows-amd64)
- GitHub Check: Node.js / Package (linux-arm64)
- GitHub Check: Node.js / Package (linux-amd64)
- GitHub Check: Node.js / Package (windows-arm64)
- GitHub Check: Go / Test (windows-arm64)
- GitHub Check: Go / Test (linux-arm64)
- GitHub Check: Node.js / Package (windows-amd64)
- GitHub Check: Go / Test (linux-amd64)
- GitHub Check: Rust / Test (linux-arm64)
- GitHub Check: Rust / Test (windows-amd64)
- GitHub Check: Node.js / Package OpenClaw plugin
- GitHub Check: Rust / Test (windows-arm64)
- GitHub Check: Rust / Test (linux-amd64)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (23)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.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: 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.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
**/*.rs: Format Rust code with rustfmt defaults usingcargo fmt.
Runcargo clippy -- -D warnings; all Rust warnings must be treated as errors.
Use Rustsnake_casenaming conventions.
Files:
crates/cli/src/diagnostics/mod.rscrates/core/src/observability/plugin_component.rscrates/cli/tests/coverage/shared/doctor_tests.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
**/*: Use release tags in raw Rust-compatible SemVer without a leadingv; tags such asv0.1.0are prohibited.
Use branch prefixesfeat/,fix/,docs/,test/, orrefactor/according to the change purpose.
Every commit in a pull request must include a DCOSigned-off-by:sign-off.
Before submitting a pull request, ensure pre-commit hooks, relevant tests, target-specific builds, documentation updates, and a rebase on the latestmainare complete.
Use commit messages in the formtype: short description, with a valid type and a first line under 72 characters.
Files:
crates/cli/src/diagnostics/mod.rscrates/node/tests/scope_tests.mjscrates/core/src/observability/plugin_component.rscrates/cli/tests/coverage/shared/doctor_tests.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/cli/src/diagnostics/mod.rscrates/core/src/observability/plugin_component.rscrates/cli/tests/coverage/shared/doctor_tests.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/cli/src/diagnostics/mod.rscrates/core/src/observability/plugin_component.rscrates/cli/tests/coverage/shared/doctor_tests.rs
**/*.{rs,py,js,jsx,ts,tsx,go,c,h,cc,cpp,md,toml,yml,yaml,sh}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, documentation, scripts, and configuration files; the project is Apache-2.0.
Files:
crates/cli/src/diagnostics/mod.rscrates/core/src/observability/plugin_component.rscrates/cli/tests/coverage/shared/doctor_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Use
snake_casenaming in Rust and Python.
Files:
crates/cli/src/diagnostics/mod.rscrates/core/src/observability/plugin_component.rscrates/cli/tests/coverage/shared/doctor_tests.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: UseJson = serde_json::Valuein Rust-facing runtime APIs where existing code expects JSON payloads.
Treat Rust as the source of truth for runtime behavior; binding APIs should mirror Rust semantics unless a language-specific wrapper intentionally improves ergonomics.
Files:
crates/cli/src/diagnostics/mod.rscrates/core/src/observability/plugin_component.rscrates/cli/tests/coverage/shared/doctor_tests.rs
**/*.{rs,py,js,mjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Preserve the existing Tokio-based asynchronous model and callback/future lifetimes; do not unexpectedly block or hide async work in bindings.
Files:
crates/cli/src/diagnostics/mod.rscrates/node/tests/scope_tests.mjscrates/core/src/observability/plugin_component.rscrates/cli/tests/coverage/shared/doctor_tests.rs
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}: Run tests for every language affected by a change; changes to the core Rust crate require tests across all bindings.
UseSONAR_IGNORE_START/SONAR_IGNORE_ENDonly for documented false positives, keep ignored blocks minimal, explain them with a comment, and obtain reviewer sign-off.
Preserve the layered architecture in which Rust provides the core runtime and C FFI, PyO3, and NAPI provide bindings that mirror the full API surface.
Files:
crates/cli/src/diagnostics/mod.rscrates/core/src/observability/plugin_component.rscrates/cli/tests/coverage/shared/doctor_tests.rs
**/*.{rs,py,go,js,jsx,ts,tsx,c,h,html,md,mdx,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include the appropriate SPDX copyright and Apache-2.0 license header in every source file.
Files:
crates/cli/src/diagnostics/mod.rscrates/core/src/observability/plugin_component.rscrates/cli/tests/coverage/shared/doctor_tests.rs
crates/**/src/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees.
Files:
crates/cli/src/diagnostics/mod.rscrates/core/src/observability/plugin_component.rs
**/*.{rs,py,go,js,ts,mdx}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
**/*.{rs,py,go,js,ts,mdx}: When changing observability event fields, exporter behavior, subscriber configuration, or binding parity, keep the core event model, ATIF, typed OpenTelemetry projections, FFI wrappers, language bindings, configuration, and documentation synchronized.
Examples and documentation must use each exporter's documented flush/deregister order before shutdown.
Files:
crates/cli/src/diagnostics/mod.rscrates/core/src/observability/plugin_component.rscrates/cli/tests/coverage/shared/doctor_tests.rs
crates/node/**/*.{js,mjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
camelCasenaming for Node.js APIs.
Files:
crates/node/tests/scope_tests.mjs
**/{test,tests}/**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the appropriate test files for each affected language binding.
Files:
crates/node/tests/scope_tests.mjscrates/cli/tests/coverage/shared/doctor_tests.rs
crates/{python,ffi,node}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node}/**/*: 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/node/tests/scope_tests.mjs
{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/node/tests/scope_tests.mjscrates/cli/tests/coverage/shared/doctor_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/observability/plugin_component.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 validationUse
Result<T>withFlowErrorin core runtime paths, keeping wrapper-layer errors explicit and binding-appropriate.Run affected Rust crate tests, and run
just test-rustwhen event fields change.
Files:
crates/core/src/observability/plugin_component.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/src/observability/plugin_component.rs
crates/core/src/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
crates/core/src/**/*.rs:plugins list,plugins inspect, andplugins validatemust report lifecycle and compatibility status without leaking secret configuration.
Top-leveldoctormust report resolved dynamic-plugin and host-configuration status.
Files:
crates/core/src/observability/plugin_component.rs
{crates/plugin/**/*.rs,crates/worker-proto/**/*.rs,crates/worker/**/*.rs,crates/types/**/*.rs,crates/core/**/*.{rs,toml},python/plugin/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Run the targeted Rust, Python plugin, integration, documentation, and broader validation commands when validating dynamic-plugin changes.
Files:
crates/core/src/observability/plugin_component.rs
crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-rust-core/SKILL.md)
crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*.rs: For changes in the Rust core, adaptive, plugin, worker, worker-proto, or types crates, runcargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings.
Use narrower crate tests only as a local debugging loop; final Rust validation must use the sharedjust test-rustworkflow.
For changes tocrates/core,crates/adaptive, or shared runtime semantics, expand validation to the full binding matrix withvalidate-changeorjust ci=true test-rust.
When public APIs, event shapes, middleware behavior, plugin semantics, or core/adaptive behavior change, also runvalidate-change.
If a change is isolated to one binding wrapper with unchanged Rust semantics, prefer that binding's build and test skill instead.
Files:
crates/core/src/observability/plugin_component.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/observability/plugin_component.rs
🧠 Learnings (2)
📚 Learning: 2026-08-03T19:55:03.931Z
Learnt from: afourniernv
Repo: NVIDIA/NeMo-Relay PR: 558
File: crates/pii-redaction/src/rampart/mod.rs:265-274
Timestamp: 2026-08-03T19:55:03.931Z
Learning: In NeMo Relay first-party plugin registration helpers, treat the documented duplicate-registration `PluginError::RegistrationFailed` result from `register_plugin` as success when registration is intended to be idempotent. Do not locally reclassify this as `PluginError::Conflict`; changing the classification requires a core-wide review of the public API and FFI behavior.
Applied to files:
crates/cli/src/diagnostics/mod.rscrates/core/src/observability/plugin_component.rscrates/cli/tests/coverage/shared/doctor_tests.rs
📚 Learning: 2026-07-28T20:07:29.880Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 571
File: crates/core/src/api/runtime/state.rs:996-1020
Timestamp: 2026-07-28T20:07:29.880Z
Learning: In NeMo Relay (RELAY-509), sanitizer callback failures must be treated as intentional fail-open behavior. When an event/tool (request/response) or LLM (request/response) sanitizer callback fails, the sanitizer chain should retain and publish the last valid event/payload snapshot (rather than dropping/invalidating the data) and log the failure including callback context (e.g., which sanitizer/callback failed and relevant identifiers). Apply this consistently across all sanitizer chains mentioned in the RELAY-509 documentation/migration guide.
Applied to files:
crates/core/src/observability/plugin_component.rs
🔇 Additional comments (3)
crates/core/src/observability/plugin_component.rs (1)
956-965: 📐 Maintainability & Code QualityVerify required core-runtime validation.
The provided context does not confirm the required full binding-matrix validation for this
crates/corechange. Before merge, provide results forcargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,just test-rust, andvalidate-changeorjust ci=true test-rust.As per coding guidelines, "
crates/corechanges must run the full validation matrix across Rust, Python, Go, and Node.js."Source: Coding guidelines
crates/cli/src/diagnostics/mod.rs (1)
924-924: LGTM!crates/cli/tests/coverage/shared/doctor_tests.rs (1)
1204-1213: LGTM!Also applies to: 1237-1240
AjayThorve
left a comment
AjayThorve
left a comment
There was a problem hiding this comment.
seems like mostly refactor and no major changes. LGTM
|
/merge |
2 participants
Overview
Address the identified Sonar code-quality findings across Rust, Go, JavaScript, and HTML surfaces while preserving existing behavior.
Details
Where should the reviewer start?
Start with
crates/core/src/codec/gemini_generate_content.rs, then review the smaller helper extractions incrates/core/src/plugin.rsandcrates/node/src/callable.rs.Validation:
just test-rust;cargo clippy --workspace --all-targets -- -D warnings; repository pre-commit hooks including Cargo check, Go format/vet, and docs checks.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit