fix: surface ATIF delivery failures consistently - #687
Conversation
github-actions
Bot
added
size:M
|
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 (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (13)**/*.rs📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Files:
**/*📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
**/*.{rs,py,go,js,ts}📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Files:
**/*.{rs,py,js,jsx,ts,tsx,go,c,h,cc,cpp,md,toml,yml,yaml,sh}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,py}📄 CodeRabbit inference engine (AGENTS.md)
Files:
crates/**/*.rs📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,py,js,mjs,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/{test,tests}/**/*📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{rs,py,go,js,jsx,ts,tsx,c,h,html,md,mdx,toml}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
crates/{python,ffi,node}/**/*⚙️ CodeRabbit configuration file
Files:
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (1)📚 Learning: 2026-08-03T19:55:03.931ZApplied to files:
🔇 Additional comments (1)
WalkthroughATIF and OpenTelemetry cleanup now report whether registrations were removed, removed with errors, or retained. Plugin rollback preserves runtime diagnostics and tracks callback safety separately. Metadata rendering rejects invalid values. Python asynchronous clearing protects newer clear operations from older completions. ChangesATIF validation and teardown
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PythonBinding
participant PluginTeardown
participant PluginConfigurationClearState
PythonBinding->>PluginTeardown: start clear_async()
PluginTeardown->>PluginConfigurationClearState: finish completed operation
PluginConfigurationClearState->>PluginConfigurationClearState: reset only if identity matches
PluginConfigurationClearState-->>PythonBinding: publish clear result
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
lang:python
![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/core/src/observability/plugin_component.rs`:
- Around line 1613-1628: Update the metadata traversal around the resolved
selector so it distinguishes a missing/null value from an intermediate
non-object value. Reject traversal when any intermediate segment encounters a
scalar or other non-object JSON value, while retaining fallback handling for
genuinely missing or null final values; add a regression test covering a nested
selector with a scalar intermediate segment.
In `@crates/core/src/plugin.rs`:
- Around line 2343-2350: Replace the substring-based classification in the
deregistration flow around registration.deregister and callbacks_cleared with a
typed outcome that independently reports delivery diagnostics and whether
callback removal completed. Ensure callbacks_cleared becomes true only when the
callback explicitly reports successful removal, regardless of error text, and
add a regression test covering a non-ATIF deregistration failure whose message
contains ATIF_RUNTIME_DELIVERY_FAILURE_MARKER.
In `@crates/core/tests/integration/atif_storage_tests.rs`:
- Around line 538-546: The test currently verifies the diagnostic with code
"atif.remote_delivery_failed" exists before teardown but does not verify it
persists after the clear_plugin_configuration() call fails. Add a second
assertion block after line 570 that reads active_plugin_report() again and
asserts the same diagnostic code, field property value, and count to ensure the
retained-report contract is upheld on the production ATIF path when teardown
fails.
In `@python/tests/test_observability_plugin.py`:
- Line 471: The match parameter in the pytest.raises call uses an unescaped dot
in the regex pattern, which matches any character instead of just a literal dot.
Update the match string to use a raw string with an escaped dot so that the
pattern r"atif\.destination_render_failed" only matches the exact error message
and rejects unintended matches like atifXdestination_render_failed.
- Around line 442-477: Wrap the test logic in
test_atif_non_string_metadata_is_reported_and_failed_clear_is_drainable within a
try/finally block to ensure cleanup executes even if assertions fail before the
final clear_async calls. In the finally block, tolerate the expected
RuntimeError from the first plugin.clear_async() invocation using a try/except,
then call plugin.clear_async() again to complete teardown. Additionally, update
the pytest.raises match parameter to use a raw string with escaped dots to match
the exact diagnostic code (use r"atif\.destination_render_failed" instead of the
current pattern).
🪄 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: 969ae74d-7011-44cf-82e7-c3829cac493d
📒 Files selected for processing (9)
crates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rsdocs/configure-plugins/observability/atif.mdxpython/tests/test_observability_plugin.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (29)
**/*.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/python/src/py_plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/python/src/py_plugin.rspython/tests/test_observability_plugin.pycrates/core/src/plugin.rscrates/core/src/observability/plugin_component.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/python/src/py_plugin.rsdocs/configure-plugins/observability/atif.mdxcrates/core/tests/integration/atif_storage_tests.rspython/tests/test_observability_plugin.pycrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_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.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
crates/python/src/py_plugin.rscrates/core/tests/integration/atif_storage_tests.rspython/tests/test_observability_plugin.pycrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_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/python/src/py_plugin.rscrates/core/tests/integration/atif_storage_tests.rspython/tests/test_observability_plugin.pycrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_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/python/src/py_plugin.rscrates/core/tests/integration/atif_storage_tests.rspython/tests/test_observability_plugin.pycrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Use
snake_casenaming in Rust and Python.
Files:
crates/python/src/py_plugin.rscrates/core/tests/integration/atif_storage_tests.rspython/tests/test_observability_plugin.pycrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_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/python/src/py_plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_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/python/src/py_plugin.rscrates/core/tests/integration/atif_storage_tests.rspython/tests/test_observability_plugin.pycrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_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/python/src/py_plugin.rscrates/core/tests/integration/atif_storage_tests.rspython/tests/test_observability_plugin.pycrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_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/python/src/py_plugin.rsdocs/configure-plugins/observability/atif.mdxcrates/core/tests/integration/atif_storage_tests.rspython/tests/test_observability_plugin.pycrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_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/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rs
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
**/*.mdx: In MDX files, top-of-file comments must use JSX comment delimiters ({/*and*/}); do not use HTML comments for MDX SPDX headers.
New or regenerated MDX files must use{/* ... */}for top-of-file SPDX comments.
Files:
docs/configure-plugins/observability/atif.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/configure-plugins/observability/atif.mdx
docs/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If documentation examples or commands under
docs/change, run the targeted docs checks appropriate to the change.
Files:
docs/configure-plugins/observability/atif.mdx
**/*.{md,mdx}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If links in documentation change, run
just docs-linkcheck.Use documented public APIs and stable wrapper commands in examples and user-facing documentation; do not rely on internal helpers.
**/*.{md,mdx}: Prefer the documented public API over internal shortcuts in documentation and examples.
Keep package names, repository references, and build commands current.
Contribution workflow documentation must require an issue before external contribution pull requests and note that NVIDIA contributors may use a GitHub or Linear issue.
Update entry-point documentation when examples or reading paths change.
Keep release-process and release-notes guidance in maintainer documentation such asRELEASING.md, rather than user-facing documentation pages orCHANGELOG.md.
Use stable user-facing wrappers at thescripts/root in documentation and examples; reference namespaced helper paths only for internal maintenance documentation.
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pages.
Dynamic plugin manifests in documentation and examples should usecompat.relay = ">=0.5,<1.0"unless deliberately narrower.
Render images, diagrams, tables, and other visual content at representative page widths, ensuring legibility and complete access without clipping; use responsive scaling, reflow, or overflow as appropriate and scope visual styling narrowly.
Dynamic plugin entry pages should link to native, worker, Rust example, Python example, and protocol pages when those pages exist.
Images, diagrams, tables, and custom visual content must remain legible and fully accessible at representative desktop and narrow page widths.
Release-policy documentation must point to GitHub Releases as the only release-history source of truth.
Runjust docswhen the documentation site changes; retain./scripts/build-docs.sh htmlas the compatibility wrapper.
Files:
docs/configure-plugins/observability/atif.mdx
**/*.{md,mdx,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Examples and documentation must use each exporter's documented flush/deregister order before shutdown.
Files:
docs/configure-plugins/observability/atif.mdxpython/tests/test_observability_plugin.py
docs/{about-nemo-relay/concepts/subscribers.mdx,configure-plugins/observability/**/*.mdx}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Update observability documentation and examples alongside implementation changes, including configuration version 3 with one
opentelemetrysection containing typed endpoints and no standalone public OpenInference surface.
Files:
docs/configure-plugins/observability/atif.mdx
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant reference documentation when public behavior or APIs change.
Files:
docs/configure-plugins/observability/atif.mdx
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.{md,mdx,rst}: Use title case consistently for technical documentation headings and table headers; avoid quotation marks, ampersands, and exclamation marks in headings, while preserving official product, event, research, and whitepaper title case.
Format code elements, commands, parameters, package names, expressions, directories, file names, and paths in monospace; represent path placeholders with angle brackets inside monospace.
Format UI buttons, menus, fields, and labels in bold, and separate consecutive UI navigation labels with>.
Use quotation marks for error messages and strings when appropriate, italics for newly introduced terms and publication titles, and plain text for keyboard shortcuts.
Represent GitHub repositories with owner/repository link text, such as[NVIDIA/NeMo](link), rather than generic repository wording.
Introduce every code block with a complete sentence; do not let a code block complete or interrupt the grammar of surrounding prose; use syntax highlighting when supported.
Keep inline method, function, and class references consistent with nearby documentation; omit empty parentheses in prose when no call is shown.
Use descriptive link text matching the destination title when possible; avoid raw URLs, generic anchors, long-sentence links, and unnecessary links that distract from procedures.
Ensure lists have a complete lead-in sentence, more than one item, no more than two levels, parallel construction, one idea or action per item, and appropriate punctuation; use bullets for unordered items and numbers for ordered tasks.
Format definition lists with a bold term followed by a complete, parallel, punctuated definition.
Use tables for reference information, decision support, compatibility matrices, and comparable choices; flag one-row tables, missing captions or lead-ins, sentence-case headers where title case is expected, unexplained empty cells, and code or links that would be clearer as prose.
Write procedure steps as imperative ...
Files:
docs/configure-plugins/observability/atif.mdx
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.
Flag stale examples, missing SPDX headers where required, and instructions that no longer match CI or pre-commit behavior.
Files:
docs/configure-plugins/observability/atif.mdx
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/tests/integration/atif_storage_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_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 validationUse
Result<T>withFlowErrorin core runtime paths, keeping wrapper-layer errors explicit and binding-appropriate.
Files:
crates/core/tests/integration/atif_storage_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.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/tests/integration/atif_storage_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rs
**/{test,tests}/**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the appropriate test files for each affected language binding.
Files:
crates/core/tests/integration/atif_storage_tests.rspython/tests/test_observability_plugin.pycrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/tests/integration/atif_storage_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_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/integration/atif_storage_tests.rspython/tests/test_observability_plugin.pycrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rs
python/tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
python/tests/**/*.py: Pytest is used to run tests.
Do not add@pytest.mark.asyncioto any test; async tests are automatically detected and run by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, with thespecconstructor argument when necessary.
Name mocked classes with themockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; if a fixture is needed in multiple test files, place it in aconftest.pyfile.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the fixture function asdef <fixture_name>_fixture() -> <return_type>:; only specifyscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating individual tests for different input types.Maintain test coverage for Python binding and wrapper changes with the Python test suite.
Files:
python/tests/test_observability_plugin.py
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: Lint Python with Ruff using rule setsE,F,W, andI.
Format Python with the Ruff formatter, using a 120-character line length and double quotes.
Runtyfor Python type checking.
Use Pythonsnake_casenaming conventions.
Files:
python/tests/test_observability_plugin.py
🧠 Learnings (3)
📚 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/python/src/py_plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rs
📚 Learning: 2026-07-14T02:53:59.997Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 415
File: docs/configure-plugins/observability/opentelemetry.mdx:98-113
Timestamp: 2026-07-14T02:53:59.997Z
Learning: In NeMo-Relay’s OpenTelemetry/OpenInference observability projection docs under docs/configure-plugins/observability/, document the projected-attribute contract as follows: (1) emit scalar top-level `data`/`metadata` fields as typed dotted OTLP attributes (for example, `nemo_relay.start.metadata.tenant`); (2) keep nested objects/arrays as JSON strings at their top-level OTLP attribute (rather than expanding them into nested OTLP attributes); and (3) do not reference the legacy `*_json` payload attributes (e.g., `data_json`, `metadata_json`, `input_json`) because they were intentionally removed as a breaking change.
Applied to files:
docs/configure-plugins/observability/atif.mdx
📚 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/plugin.rscrates/core/src/observability/plugin_component.rs
🪛 Ruff (0.16.0)
python/tests/test_observability_plugin.py
[warning] 471-471: Pattern passed to match= contains metacharacters but is neither escaped nor raw
(RUF043)
🔇 Additional comments (5)
crates/python/src/py_plugin.rs (1)
936-949: LGTM!Also applies to: 972-979
crates/python/tests/coverage/py_plugin_coverage_tests.rs (1)
168-189: LGTM!crates/core/tests/unit/observability/plugin_component_tests.rs (1)
2425-2433: LGTM!docs/configure-plugins/observability/atif.mdx (1)
92-104: LGTM!Also applies to: 250-256
crates/core/tests/unit/plugin_tests.rs (1)
1538-1540: LGTM!
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/observability/plugin_component.rs`:
- Around line 1628-1639: Update the metadata selector traversal in the
surrounding placeholder-resolution function so Some(Json::Null) is treated like
missing metadata: stop traversal and allow the existing null fallback to produce
the default value. Preserve the non-object traversal error for other JSON value
types, and add a regression test covering a nested null such as metadata.routing
being null with a -unassigned fallback.
In `@crates/core/src/plugin.rs`:
- Around line 1576-1581: Preserve the previous plugin configuration’s report
while replacement teardown callbacks execute, rather than removing it before
rollback. In the replacement path around rollback_registrations_checked, when
teardown.errors is non-empty, retain the updated runtime diagnostics report in
LAST_FAILED_RUNTIME_DIAGNOSTICS_REPORT before returning the registration error.
Add a regression test covering failed remote upload during replacement and
verify the diagnostics remain available through plugin.report().
🪄 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: 22fae4cf-f325-45e8-9d8f-3b06191258c0
📒 Files selected for processing (6)
crates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rspython/tests/test_observability_plugin.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (20)
**/*.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/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/tests/integration/atif_storage_tests.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/tests/integration/atif_storage_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 validationUse
Result<T>withFlowErrorin core runtime paths, keeping wrapper-layer errors explicit and binding-appropriate.
Files:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/tests/integration/atif_storage_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/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rspython/tests/test_observability_plugin.pycrates/core/tests/integration/atif_storage_tests.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/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/tests/integration/atif_storage_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.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rspython/tests/test_observability_plugin.pycrates/core/tests/integration/atif_storage_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/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rspython/tests/test_observability_plugin.pycrates/core/tests/integration/atif_storage_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/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rspython/tests/test_observability_plugin.pycrates/core/tests/integration/atif_storage_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Use
snake_casenaming in Rust and Python.
Files:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rspython/tests/test_observability_plugin.pycrates/core/tests/integration/atif_storage_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/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/tests/integration/atif_storage_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/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rspython/tests/test_observability_plugin.pycrates/core/tests/integration/atif_storage_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/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rspython/tests/test_observability_plugin.pycrates/core/tests/integration/atif_storage_tests.rs
**/{test,tests}/**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the appropriate test files for each affected language binding.
Files:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rspython/tests/test_observability_plugin.pycrates/core/tests/integration/atif_storage_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/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rspython/tests/test_observability_plugin.pycrates/core/tests/integration/atif_storage_tests.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/tests/integration/atif_storage_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/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rspython/tests/test_observability_plugin.pycrates/core/tests/integration/atif_storage_tests.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/core/src/plugin.rscrates/core/src/observability/plugin_component.rspython/tests/test_observability_plugin.py
python/tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
python/tests/**/*.py: Pytest is used to run tests.
Do not add@pytest.mark.asyncioto any test; async tests are automatically detected and run by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, with thespecconstructor argument when necessary.
Name mocked classes with themockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; if a fixture is needed in multiple test files, place it in aconftest.pyfile.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the fixture function asdef <fixture_name>_fixture() -> <return_type>:; only specifyscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating individual tests for different input types.Maintain test coverage for Python binding and wrapper changes with the Python test suite.
Files:
python/tests/test_observability_plugin.py
**/*.{md,mdx,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Examples and documentation must use each exporter's documented flush/deregister order before shutdown.
Files:
python/tests/test_observability_plugin.py
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: Lint Python with Ruff using rule setsE,F,W, andI.
Format Python with the Ruff formatter, using a 120-character line length and double quotes.
Runtyfor Python type checking.
Use Pythonsnake_casenaming conventions.
Files:
python/tests/test_observability_plugin.py
🧠 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/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/tests/integration/atif_storage_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/plugin.rscrates/core/src/observability/plugin_component.rs
🔇 Additional comments (7)
crates/core/tests/unit/observability/plugin_component_tests.rs (1)
2434-2443: LGTM!crates/core/src/plugin.rs (2)
334-372: LGTM!Also applies to: 2141-2182, 2345-2388, 2471-2497
1576-1581: 🩺 Stability & AvailabilityNo change required.
NotRemovedadds an error, and the initialization wrapper retainsLegacyPluginMutationLeaseto block later mutations. Existing tests cover incomplete teardown and owner retention.crates/core/src/observability/plugin_component.rs (1)
64-66: LGTM!Also applies to: 897-945
crates/core/tests/integration/atif_storage_tests.rs (1)
26-27: LGTM!Also applies to: 501-512, 538-546, 579-587
crates/core/tests/unit/plugin_tests.rs (1)
1509-1581: LGTM!Also applies to: 2188-2199
python/tests/test_observability_plugin.py (1)
17-17: LGTM!Also applies to: 442-481
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/core/src/plugin.rs (1)
1588-1602: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not discard an unremoved callback during replacement teardown.
NotRemovedmeans that the callback remains registered. The rollback at Line 1588 runs againstprevious_state.registrations. Lines 1589-1595 then remove the temporary active state. This path does not checkcallbacks_cleared. The remaining callback loses its deregistration record, and a later activation can register another callback unless another layer rejects it.
crates/core/src/plugin.rs#L1588-L1602: Whencallbacks_clearedis false, retain cleanup state or set a terminal state that blocks later activation.crates/core/tests/unit/plugin_tests.rs#L1583-L1637: Add a replacement test withPluginRegistrationCleanupOutcome::NotRemoved(...). Assert that retrying initialization cannot create a duplicate callback.Verify that the mutation-owner layer rejects a repeated
initialize_plugins_exactcall after this outcome. Based on PR context, rollback tracks callback removal separately from delivery errors.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/core/src/plugin.rs` around lines 1588 - 1602, The replacement teardown in rollback_registrations_checked must preserve cleanup state when callbacks_cleared is false, or transition to a terminal state that blocks later initialize_plugins_exact calls instead of discarding the active state. Update crates/core/src/plugin.rs lines 1588-1602 accordingly; add a replacement test in crates/core/tests/unit/plugin_tests.rs lines 1583-1637 using PluginRegistrationCleanupOutcome::NotRemoved(...) and assert that retrying initialization cannot register a duplicate callback.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@crates/core/src/plugin.rs`:
- Around line 1588-1602: The replacement teardown in
rollback_registrations_checked must preserve cleanup state when
callbacks_cleared is false, or transition to a terminal state that blocks later
initialize_plugins_exact calls instead of discarding the active state. Update
crates/core/src/plugin.rs lines 1588-1602 accordingly; add a replacement test in
crates/core/tests/unit/plugin_tests.rs lines 1583-1637 using
PluginRegistrationCleanupOutcome::NotRemoved(...) and assert that retrying
initialization cannot register a duplicate callback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: ce58a2ce-ad4c-496f-b1d6-241ac9de86ea
📒 Files selected for processing (4)
crates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (17)
**/*.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/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.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.
Files:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.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/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.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/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.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.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.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/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.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/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Use
snake_casenaming in Rust and Python.
Files:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.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/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.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/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.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/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rs
**/{test,tests}/**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the appropriate test files for each affected language binding.
Files:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_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/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.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/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/core/src/observability/plugin_component.rscrates/core/src/plugin.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/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.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.rscrates/core/src/plugin.rs
🔇 Additional comments (2)
crates/core/src/observability/plugin_component.rs (1)
1632-1632: LGTM!crates/core/tests/unit/observability/plugin_component_tests.rs (1)
2444-2448: LGTM!
411a2c4 to
4a6d1b6
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/python/tests/coverage/py_plugin_coverage_tests.rs (1)
168-189: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the conditional branch of the reset, not only the replacement.
The assertion at Line 189 proves that a completed clear replaces the global state. It does not exercise the new condition in
reset_plugin_configuration_clear_state_if, which must leave the current state untouched when the finishing operation is no longer current. That branch is the actual behavior change in this layer.Add a case that finishes a stale state after a reset and asserts the current state pointer is unchanged.
As per path instructions, "Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/python/tests/coverage/py_plugin_coverage_tests.rs` around lines 168 - 189, Extend the async clear coverage around plugin_configuration_clear_state and reset_plugin_configuration_clear_state_if to create a stale in-progress state, reset it, then finish the stale operation and assert the current state pointer remains unchanged. Keep the existing assertion for completed current-state replacement, while explicitly exercising the conditional branch that ignores stale finishes.Source: Path instructions
crates/python/src/py_plugin.rs (1)
1061-1069: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMake state selection and teardown start atomic.
If another clear finishes after
plugin_configuration_clear_state()returns but beforebegin_clear()locksstarted, this call can wait on the completed state.PluginTeardownCompletion::waitreturns its retained result immediately, so the call can report success without clearing the current configuration. Resolve the state and setstartedunder one critical section, while preserving sharing for in-flight clears. Add a regression test for this interleaving.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/python/src/py_plugin.rs` around lines 1061 - 1069, Make plugin state selection and teardown initiation atomic around plugin_configuration_clear_state and PluginTeardownCompletion::begin_clear, so a new caller cannot select a completed state before marking it started. Preserve sharing of the same completion object for clears already in flight, while ensuring calls after completion obtain and start a fresh state. Add a regression test covering the interleaving where one clear completes between state selection and begin_clear.crates/core/tests/unit/plugin_tests.rs (1)
1583-1638: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTwo test names promise mutation behavior that the assertions do not check.
test_opentelemetry_delivery_failure_allows_later_plugin_configurationasserts onlyoutcome.callbacks_clearedandoutcome.result.is_err().test_mixed_opentelemetry_shutdown_failure_blocks_later_configurationasserts only!outcome.callbacks_clearedand the error text. Neither test attempts a later configuration mutation, so neither proves the allow or block behavior in its name.Add the missing step to each test: call the public
clear_plugin_configurationorinitialize_plugins_exactafterwards and assert success in theRemovedWithErrorcase and the disabled-mutation error in theNotRemovedcase.As per path instructions, "Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant."
🤖 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/plugin_tests.rs` around lines 1583 - 1638, Update test_opentelemetry_delivery_failure_allows_later_plugin_configuration and test_mixed_opentelemetry_shutdown_failure_blocks_later_configuration to perform a subsequent public configuration mutation after clear_plugin_configuration_inner. Assert that initialize_plugins_exact or clear_plugin_configuration succeeds after the RemovedWithError case, and returns the disabled-mutation error after the NotRemoved case, while preserving the existing cleanup and error assertions.Source: Path instructions
🤖 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/observability/plugin_component.rs`:
- Around line 1019-1031: Update shutdown_opentelemetry_subscribers and its
callback to return the delivery-failure classification alongside the
PluginError, such as Option<(PluginError, bool)> or a dedicated enum, instead of
encoding it with OTEL_RUNTIME_DELIVERY_FAILURE_MARKER. In the cleanup closure,
match directly on that structured result so delivery failures produce
RemovedWithError and other errors produce NotRemoved; remove the
to_string().contains(...) classification.
- Around line 898-929: Update the shutdown cleanup callback so failures from
deregister_atif_shutdown_subscriber remain classified as NotRemoved, while
failures from prepare_atif_shutdown_file, sink-target locking, and post-write
completion are classified as RemovedWithError after subscriber removal has
completed. Split the cleanup flow around the subscriber-removal loop, preserving
existing error propagation and ensuring export/write failures do not leave the
registration marked as active.
In `@crates/core/src/plugin.rs`:
- Around line 1582-1613: Update the ATIF documentation to state that replacement
teardown failures with cleared callbacks, including recoverable ATIF or
OpenTelemetry delivery errors, abort activation of the new configuration and
leave no active plugin configuration. Reference the behavior covered by
test_replacement_teardown_runtime_diagnostics_remain_in_the_plugin_report, while
preserving the existing clear-path documentation.
In `@docs/configure-plugins/observability/atif.mdx`:
- Around line 250-256: The documentation at
docs/configure-plugins/observability/atif.mdx lines 250-256 must state that a
pending remote-delivery failure causes the next initialize replacing the
configuration to fail, leaving no active configuration. In
crates/core/src/plugin.rs lines 1582-1613, preserve the current behavior covered
by test_replacement_teardown_runtime_diagnostics_remain_in_the_plugin_report and
confirm it remains intentional; no direct code change is requested there.
In `@python/tests/test_observability_plugin.py`:
- Around line 472-481: Strengthen the assertion in the clear_async failure test
by verifying that plugin.report() retains the atif.destination_render_failed
diagnostic code, not merely that a report object exists. Keep the existing
failed-clear and cleanup flow unchanged, and assert against the report
structure’s established diagnostic-code field.
---
Outside diff comments:
In `@crates/core/tests/unit/plugin_tests.rs`:
- Around line 1583-1638: Update
test_opentelemetry_delivery_failure_allows_later_plugin_configuration and
test_mixed_opentelemetry_shutdown_failure_blocks_later_configuration to perform
a subsequent public configuration mutation after
clear_plugin_configuration_inner. Assert that initialize_plugins_exact or
clear_plugin_configuration succeeds after the RemovedWithError case, and returns
the disabled-mutation error after the NotRemoved case, while preserving the
existing cleanup and error assertions.
In `@crates/python/src/py_plugin.rs`:
- Around line 1061-1069: Make plugin state selection and teardown initiation
atomic around plugin_configuration_clear_state and
PluginTeardownCompletion::begin_clear, so a new caller cannot select a completed
state before marking it started. Preserve sharing of the same completion object
for clears already in flight, while ensuring calls after completion obtain and
start a fresh state. Add a regression test covering the interleaving where one
clear completes between state selection and begin_clear.
In `@crates/python/tests/coverage/py_plugin_coverage_tests.rs`:
- Around line 168-189: Extend the async clear coverage around
plugin_configuration_clear_state and reset_plugin_configuration_clear_state_if
to create a stale in-progress state, reset it, then finish the stale operation
and assert the current state pointer remains unchanged. Keep the existing
assertion for completed current-state replacement, while explicitly exercising
the conditional branch that ignores stale finishes.
🪄 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: 23b7a0b3-4128-459b-b78a-55efacc53b2e
📒 Files selected for processing (9)
crates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rsdocs/configure-plugins/observability/atif.mdxpython/tests/test_observability_plugin.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (29)
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
python/tests/test_observability_plugin.pycrates/core/src/plugin.rscrates/python/src/py_plugin.rscrates/core/src/observability/plugin_component.rs
python/tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
python/tests/**/*.py: Pytest is used to run tests.
Do not add@pytest.mark.asyncioto any test; async tests are automatically detected and run by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, with thespecconstructor argument when necessary.
Name mocked classes with themockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; if a fixture is needed in multiple test files, place it in aconftest.pyfile.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the fixture function asdef <fixture_name>_fixture() -> <return_type>:; only specifyscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating individual tests for different input types.Maintain test coverage for Python binding and wrapper changes with the Python test suite.
Files:
python/tests/test_observability_plugin.py
**/*
📄 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:
python/tests/test_observability_plugin.pycrates/core/tests/unit/observability/plugin_component_tests.rsdocs/configure-plugins/observability/atif.mdxcrates/core/src/plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.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.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
python/tests/test_observability_plugin.pycrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.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:
python/tests/test_observability_plugin.pycrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rs
**/*.{md,mdx,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Examples and documentation must use each exporter's documented flush/deregister order before shutdown.
Files:
python/tests/test_observability_plugin.pydocs/configure-plugins/observability/atif.mdx
**/*.{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:
python/tests/test_observability_plugin.pycrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Use
snake_casenaming in Rust and Python.
Files:
python/tests/test_observability_plugin.pycrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.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:
python/tests/test_observability_plugin.pycrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rs
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: Lint Python with Ruff using rule setsE,F,W, andI.
Format Python with the Ruff formatter, using a 120-character line length and double quotes.
Runtyfor Python type checking.
Use Pythonsnake_casenaming conventions.
Files:
python/tests/test_observability_plugin.py
**/*.{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:
python/tests/test_observability_plugin.pycrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rs
**/{test,tests}/**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the appropriate test files for each affected language binding.
Files:
python/tests/test_observability_plugin.pycrates/core/tests/unit/observability/plugin_component_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/plugin_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:
python/tests/test_observability_plugin.pycrates/core/tests/unit/observability/plugin_component_tests.rsdocs/configure-plugins/observability/atif.mdxcrates/core/src/plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.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:
python/tests/test_observability_plugin.pycrates/core/tests/unit/observability/plugin_component_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/plugin_tests.rs
**/*.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/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/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.
Files:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/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/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.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/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/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/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rs
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
**/*.mdx: In MDX files, top-of-file comments must use JSX comment delimiters ({/*and*/}); do not use HTML comments for MDX SPDX headers.
New or regenerated MDX files must use{/* ... */}for top-of-file SPDX comments.
Files:
docs/configure-plugins/observability/atif.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/configure-plugins/observability/atif.mdx
docs/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If documentation examples or commands under
docs/change, run the targeted docs checks appropriate to the change.
Files:
docs/configure-plugins/observability/atif.mdx
**/*.{md,mdx}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If links in documentation change, run
just docs-linkcheck.Use documented public APIs and stable wrapper commands in examples and user-facing documentation; do not rely on internal helpers.
**/*.{md,mdx}: Prefer the documented public API over internal shortcuts in documentation and examples.
Keep package names, repository references, and build commands current.
Contribution workflow documentation must require an issue before external contribution pull requests and note that NVIDIA contributors may use a GitHub or Linear issue.
Update entry-point documentation when examples or reading paths change.
Keep release-process and release-notes guidance in maintainer documentation such asRELEASING.md, rather than user-facing documentation pages orCHANGELOG.md.
Use stable user-facing wrappers at thescripts/root in documentation and examples; reference namespaced helper paths only for internal maintenance documentation.
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pages.
Dynamic plugin manifests in documentation and examples should usecompat.relay = ">=0.5,<1.0"unless deliberately narrower.
Render images, diagrams, tables, and other visual content at representative page widths, ensuring legibility and complete access without clipping; use responsive scaling, reflow, or overflow as appropriate and scope visual styling narrowly.
Dynamic plugin entry pages should link to native, worker, Rust example, Python example, and protocol pages when those pages exist.
Images, diagrams, tables, and custom visual content must remain legible and fully accessible at representative desktop and narrow page widths.
Release-policy documentation must point to GitHub Releases as the only release-history source of truth.
Runjust docswhen the documentation site changes; retain./scripts/build-docs.sh htmlas the compatibility wrapper.
Files:
docs/configure-plugins/observability/atif.mdx
docs/{about-nemo-relay/concepts/subscribers.mdx,configure-plugins/observability/**/*.mdx}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Update observability documentation and examples alongside implementation changes, including configuration version 3 with one
opentelemetrysection containing typed endpoints and no standalone public OpenInference surface.
Files:
docs/configure-plugins/observability/atif.mdx
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant reference documentation when public behavior or APIs change.
Files:
docs/configure-plugins/observability/atif.mdx
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.{md,mdx,rst}: Use title case consistently for technical documentation headings and table headers; avoid quotation marks, ampersands, and exclamation marks in headings, while preserving official product, event, research, and whitepaper title case.
Format code elements, commands, parameters, package names, expressions, directories, file names, and paths in monospace; represent path placeholders with angle brackets inside monospace.
Format UI buttons, menus, fields, and labels in bold, and separate consecutive UI navigation labels with>.
Use quotation marks for error messages and strings when appropriate, italics for newly introduced terms and publication titles, and plain text for keyboard shortcuts.
Represent GitHub repositories with owner/repository link text, such as[NVIDIA/NeMo](link), rather than generic repository wording.
Introduce every code block with a complete sentence; do not let a code block complete or interrupt the grammar of surrounding prose; use syntax highlighting when supported.
Keep inline method, function, and class references consistent with nearby documentation; omit empty parentheses in prose when no call is shown.
Use descriptive link text matching the destination title when possible; avoid raw URLs, generic anchors, long-sentence links, and unnecessary links that distract from procedures.
Ensure lists have a complete lead-in sentence, more than one item, no more than two levels, parallel construction, one idea or action per item, and appropriate punctuation; use bullets for unordered items and numbers for ordered tasks.
Format definition lists with a bold term followed by a complete, parallel, punctuated definition.
Use tables for reference information, decision support, compatibility matrices, and comparable choices; flag one-row tables, missing captions or lead-ins, sentence-case headers where title case is expected, unexplained empty cells, and code or links that would be clearer as prose.
Write procedure steps as imperative ...
Files:
docs/configure-plugins/observability/atif.mdx
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.
Flag stale examples, missing SPDX headers where required, and instructions that no longer match CI or pre-commit behavior.
Files:
docs/configure-plugins/observability/atif.mdx
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/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_plugin.rs
🧠 Learnings (3)
📚 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/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rs
📚 Learning: 2026-07-14T02:53:59.997Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 415
File: docs/configure-plugins/observability/opentelemetry.mdx:98-113
Timestamp: 2026-07-14T02:53:59.997Z
Learning: In NeMo-Relay’s OpenTelemetry/OpenInference observability projection docs under docs/configure-plugins/observability/, document the projected-attribute contract as follows: (1) emit scalar top-level `data`/`metadata` fields as typed dotted OTLP attributes (for example, `nemo_relay.start.metadata.tenant`); (2) keep nested objects/arrays as JSON strings at their top-level OTLP attribute (rather than expanding them into nested OTLP attributes); and (3) do not reference the legacy `*_json` payload attributes (e.g., `data_json`, `metadata_json`, `input_json`) because they were intentionally removed as a breaking change.
Applied to files:
docs/configure-plugins/observability/atif.mdx
📚 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/plugin.rscrates/core/src/observability/plugin_component.rs
🔇 Additional comments (11)
crates/core/src/plugin.rs (2)
340-346: LGTM!Also applies to: 363-385, 2376-2419
2500-2529: 🩺 Stability & AvailabilityKeep the existing rollback error propagation. Both unsafe rollback branches append an error, and activation callers already reject the operation and disable later mutations when those errors are recorded.
> Likely an incorrect or invalid review comment.crates/core/src/observability/plugin_component.rs (1)
1653-1677: LGTM!crates/core/tests/unit/plugin_tests.rs (1)
1509-1581: LGTM!Also applies to: 1640-1694, 2299-2313
crates/core/tests/integration/atif_storage_tests.rs (1)
26-27: LGTM!Also applies to: 501-512, 538-545, 575-587
docs/configure-plugins/observability/atif.mdx (1)
92-105: LGTM!crates/core/tests/unit/observability/plugin_component_tests.rs (3)
2450-2473: LGTM!
2898-2948: LGTM!
30-51: 📐 Maintainability & Code QualityNo change needed.
shutdownandset_resourcehave default implementations inopentelemetry_sdk0.32.1.Arc,AtomicUsize, andOrderingare used by unconditionally compiled test code.> Likely an incorrect or invalid review comment.python/tests/test_observability_plugin.py (1)
17-17: LGTM!Also applies to: 442-470
crates/python/src/py_plugin.rs (1)
945-948: LGTM!Also applies to: 972-979
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
4a6d1b6 to
d4adb5d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/python/tests/coverage/py_plugin_coverage_tests.rs (1)
168-190: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover failed async clear in the Python binding.
Add a test that forces
clear_plugin_configuration_asyncto return a Python error, then performs a later clear or initialization. Core teardown tests cover this behavior, but the Python binding has no equivalent regression test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/python/tests/coverage/py_plugin_coverage_tests.rs` around lines 168 - 190, Add a regression case alongside the existing async clear test using the Python binding’s `clear_plugin_configuration_async`: force its first invocation to return a Python error, assert that failure, then perform a subsequent clear or initialization and verify it succeeds and resets state as expected. Reuse the existing `with_event_loop`, `load_module`, and `plugin_configuration_clear_state` helpers to mirror the current successful path.Source: Path instructions
🤖 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/plugin.rs`:
- Around line 2399-2401: Update the RemovedWithError branch in the plugin
registration cleanup aggregation to prefix or wrap error.to_string() with the
registration kind and name, matching the identity format used by NotRemoved.
Preserve the underlying error text, including atif.remote_delivery_failed, and
do not introduce the “could not be removed” phrase.
In `@crates/core/tests/unit/plugin_tests.rs`:
- Around line 1674-1692: Extend the aborted replacement test around
initialize_plugins_exact to assert that no active plugin configuration exists
after the delivery failure. Use the active-configuration accessor, not
active_plugin_report(), because the report is intentionally retained for
diagnostics; keep the existing diagnostics and subsequent
successful-initialization assertions unchanged.
---
Outside diff comments:
In `@crates/python/tests/coverage/py_plugin_coverage_tests.rs`:
- Around line 168-190: Add a regression case alongside the existing async clear
test using the Python binding’s `clear_plugin_configuration_async`: force its
first invocation to return a Python error, assert that failure, then perform a
subsequent clear or initialization and verify it succeeds and resets state as
expected. Reuse the existing `with_event_loop`, `load_module`, and
`plugin_configuration_clear_state` helpers to mirror the current successful
path.
🪄 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: 2c0f8811-4742-4bf0-b7a7-680c2b34eadb
📒 Files selected for processing (9)
crates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rsdocs/configure-plugins/observability/atif.mdxpython/tests/test_observability_plugin.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (29)
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
python/tests/test_observability_plugin.pycrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/python/src/py_plugin.rs
python/tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
python/tests/**/*.py: Pytest is used to run tests.
Do not add@pytest.mark.asyncioto any test; async tests are automatically detected and run by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, with thespecconstructor argument when necessary.
Name mocked classes with themockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; if a fixture is needed in multiple test files, place it in aconftest.pyfile.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the fixture function asdef <fixture_name>_fixture() -> <return_type>:; only specifyscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating individual tests for different input types.Maintain test coverage for Python binding and wrapper changes with the Python test suite.
Files:
python/tests/test_observability_plugin.py
**/*
📄 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:
python/tests/test_observability_plugin.pycrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rsdocs/configure-plugins/observability/atif.mdx
**/*.{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.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
python/tests/test_observability_plugin.pycrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_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:
python/tests/test_observability_plugin.pycrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rs
**/*.{md,mdx,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Examples and documentation must use each exporter's documented flush/deregister order before shutdown.
Files:
python/tests/test_observability_plugin.pydocs/configure-plugins/observability/atif.mdx
**/*.{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:
python/tests/test_observability_plugin.pycrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Use
snake_casenaming in Rust and Python.
Files:
python/tests/test_observability_plugin.pycrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_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:
python/tests/test_observability_plugin.pycrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rs
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: Lint Python with Ruff using rule setsE,F,W, andI.
Format Python with the Ruff formatter, using a 120-character line length and double quotes.
Runtyfor Python type checking.
Use Pythonsnake_casenaming conventions.
Files:
python/tests/test_observability_plugin.py
**/*.{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:
python/tests/test_observability_plugin.pycrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rs
**/{test,tests}/**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the appropriate test files for each affected language binding.
Files:
python/tests/test_observability_plugin.pycrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_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:
python/tests/test_observability_plugin.pycrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rsdocs/configure-plugins/observability/atif.mdx
{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:
python/tests/test_observability_plugin.pycrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rs
**/*.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/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_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.rscrates/core/src/plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_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 validationUse
Result<T>withFlowErrorin core runtime paths, keeping wrapper-layer errors explicit and binding-appropriate.
Files:
crates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.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.rscrates/core/src/plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_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/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_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/observability/plugin_component.rscrates/core/src/plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_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/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rs
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
**/*.mdx: In MDX files, top-of-file comments must use JSX comment delimiters ({/*and*/}); do not use HTML comments for MDX SPDX headers.
New or regenerated MDX files must use{/* ... */}for top-of-file SPDX comments.
Files:
docs/configure-plugins/observability/atif.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/configure-plugins/observability/atif.mdx
docs/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If documentation examples or commands under
docs/change, run the targeted docs checks appropriate to the change.
Files:
docs/configure-plugins/observability/atif.mdx
**/*.{md,mdx}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If links in documentation change, run
just docs-linkcheck.Use documented public APIs and stable wrapper commands in examples and user-facing documentation; do not rely on internal helpers.
**/*.{md,mdx}: Prefer the documented public API over internal shortcuts in documentation and examples.
Keep package names, repository references, and build commands current.
Contribution workflow documentation must require an issue before external contribution pull requests and note that NVIDIA contributors may use a GitHub or Linear issue.
Update entry-point documentation when examples or reading paths change.
Keep release-process and release-notes guidance in maintainer documentation such asRELEASING.md, rather than user-facing documentation pages orCHANGELOG.md.
Use stable user-facing wrappers at thescripts/root in documentation and examples; reference namespaced helper paths only for internal maintenance documentation.
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pages.
Dynamic plugin manifests in documentation and examples should usecompat.relay = ">=0.5,<1.0"unless deliberately narrower.
Render images, diagrams, tables, and other visual content at representative page widths, ensuring legibility and complete access without clipping; use responsive scaling, reflow, or overflow as appropriate and scope visual styling narrowly.
Dynamic plugin entry pages should link to native, worker, Rust example, Python example, and protocol pages when those pages exist.
Images, diagrams, tables, and custom visual content must remain legible and fully accessible at representative desktop and narrow page widths.
Release-policy documentation must point to GitHub Releases as the only release-history source of truth.
Runjust docswhen the documentation site changes; retain./scripts/build-docs.sh htmlas the compatibility wrapper.
Files:
docs/configure-plugins/observability/atif.mdx
docs/{about-nemo-relay/concepts/subscribers.mdx,configure-plugins/observability/**/*.mdx}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Update observability documentation and examples alongside implementation changes, including configuration version 3 with one
opentelemetrysection containing typed endpoints and no standalone public OpenInference surface.
Files:
docs/configure-plugins/observability/atif.mdx
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant reference documentation when public behavior or APIs change.
Files:
docs/configure-plugins/observability/atif.mdx
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.{md,mdx,rst}: Use title case consistently for technical documentation headings and table headers; avoid quotation marks, ampersands, and exclamation marks in headings, while preserving official product, event, research, and whitepaper title case.
Format code elements, commands, parameters, package names, expressions, directories, file names, and paths in monospace; represent path placeholders with angle brackets inside monospace.
Format UI buttons, menus, fields, and labels in bold, and separate consecutive UI navigation labels with>.
Use quotation marks for error messages and strings when appropriate, italics for newly introduced terms and publication titles, and plain text for keyboard shortcuts.
Represent GitHub repositories with owner/repository link text, such as[NVIDIA/NeMo](link), rather than generic repository wording.
Introduce every code block with a complete sentence; do not let a code block complete or interrupt the grammar of surrounding prose; use syntax highlighting when supported.
Keep inline method, function, and class references consistent with nearby documentation; omit empty parentheses in prose when no call is shown.
Use descriptive link text matching the destination title when possible; avoid raw URLs, generic anchors, long-sentence links, and unnecessary links that distract from procedures.
Ensure lists have a complete lead-in sentence, more than one item, no more than two levels, parallel construction, one idea or action per item, and appropriate punctuation; use bullets for unordered items and numbers for ordered tasks.
Format definition lists with a bold term followed by a complete, parallel, punctuated definition.
Use tables for reference information, decision support, compatibility matrices, and comparable choices; flag one-row tables, missing captions or lead-ins, sentence-case headers where title case is expected, unexplained empty cells, and code or links that would be clearer as prose.
Write procedure steps as imperative ...
Files:
docs/configure-plugins/observability/atif.mdx
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.
Flag stale examples, missing SPDX headers where required, and instructions that no longer match CI or pre-commit behavior.
Files:
docs/configure-plugins/observability/atif.mdx
🧠 Learnings (3)
📚 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.rscrates/core/src/plugin.rs
📚 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/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rs
📚 Learning: 2026-07-14T02:53:59.997Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 415
File: docs/configure-plugins/observability/opentelemetry.mdx:98-113
Timestamp: 2026-07-14T02:53:59.997Z
Learning: In NeMo-Relay’s OpenTelemetry/OpenInference observability projection docs under docs/configure-plugins/observability/, document the projected-attribute contract as follows: (1) emit scalar top-level `data`/`metadata` fields as typed dotted OTLP attributes (for example, `nemo_relay.start.metadata.tenant`); (2) keep nested objects/arrays as JSON strings at their top-level OTLP attribute (rather than expanding them into nested OTLP attributes); and (3) do not reference the legacy `*_json` payload attributes (e.g., `data_json`, `metadata_json`, `input_json`) because they were intentionally removed as a breaking change.
Applied to files:
docs/configure-plugins/observability/atif.mdx
🔇 Additional comments (9)
crates/core/src/plugin.rs (1)
340-346: LGTM!Also applies to: 363-378, 1582-1615, 2189-2192, 2215-2215, 2378-2397, 2402-2421, 2504-2505, 2529-2530
crates/core/src/observability/plugin_component.rs (2)
66-68: LGTM!Also applies to: 896-906, 910-951, 1024-1037, 1094-1131, 1669-1693, 2438-2453, 2495-2583
907-909: 🎯 Functional CorrectnessNo change needed. The helper accepts
&Uuidand&str, and Rust deref coercion accepts the current&&Uuidand&&Stringarguments.> Likely an incorrect or invalid review comment.crates/core/tests/unit/plugin_tests.rs (1)
1509-1534: LGTM!Also applies to: 1536-1569, 1590-1599, 1618-1627, 1640-1673, 2301-2312
crates/core/tests/integration/atif_storage_tests.rs (1)
26-27: LGTM!Also applies to: 501-512, 538-546, 575-587
crates/core/tests/unit/observability/plugin_component_tests.rs (1)
2450-2473: LGTM!Also applies to: 2759-2824, 2826-2889, 2891-2915, 2933-2934, 3064-3070
docs/configure-plugins/observability/atif.mdx (1)
92-105: LGTM!Also applies to: 250-258
python/tests/test_observability_plugin.py (1)
17-17: LGTM!Also applies to: 442-486
crates/python/src/py_plugin.rs (1)
936-947: LGTM!Also applies to: 972-979
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/core/src/observability/plugin_component.rs (1)
2541-2563: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider including the configured endpoint text in the collision message.
canonical_opentelemetry_hostreplaces every loopback form with the literal<loopback>. The collision message then reportshttp://<loopback>:80/v1/traces, which does not appear in any config file. The message names both endpoint indices, so the user can still locate the entry, but the canonical form alone is hard to map back to127.0.0.1versuslocalhost.Keep
displayfor the canonical key comparison, and add the raw configured endpoint of the colliding entry to the diagnostic message.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/core/src/observability/plugin_component.rs` around lines 2541 - 2563, Update the collision diagnostic that uses canonicalized OpenTelemetry destinations to also include the colliding entry’s raw configured endpoint text. Preserve display for canonical key comparison and retain both endpoint indices, while using the raw value from OpenTelemetryDestinationKey::Raw or the corresponding destination data to distinguish forms such as 127.0.0.1 and localhost.
🤖 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/plugin.rs`:
- Around line 2505-2510: Guard the record_rollback_failures call in both
PendingPluginRegistrations and the other pending-rollback Drop implementation so
it runs only when !outcome.callbacks_cleared; apply this change at
crates/core/src/plugin.rs lines 2505-2510 and 2530-2535, preserving rollback
error details without treating successfully removed callbacks as leaked.
In `@crates/core/tests/unit/observability/plugin_component_tests.rs`:
- Around line 2459-2473: Add a positive assertion in the nested selector test
block around AtifDispatcher::prepare_destination that supplies metadata with a
multi-segment path resolving to a string, then verify the generated destination
contains that resolved value. Keep the existing non-object and null fallback
cases unchanged, and specifically exercise the successful traversal path of the
nested selector.
In `@crates/python/tests/coverage/py_plugin_coverage_tests.rs`:
- Line 168: Extend the coverage tests around plugin_configuration_clear_state to
simulate stale completion: install a newer state before completing the older
clear operation, then verify the newer Arc remains current rather than being
overwritten. Preserve deterministic synchronization and add cross-request
isolation coverage where applicable, targeting
reset_plugin_configuration_clear_state_if rather than changing unrelated
behavior.
---
Outside diff comments:
In `@crates/core/src/observability/plugin_component.rs`:
- Around line 2541-2563: Update the collision diagnostic that uses canonicalized
OpenTelemetry destinations to also include the colliding entry’s raw configured
endpoint text. Preserve display for canonical key comparison and retain both
endpoint indices, while using the raw value from
OpenTelemetryDestinationKey::Raw or the corresponding destination data to
distinguish forms such as 127.0.0.1 and localhost.
🪄 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: 625dfc48-2284-4ec1-b51c-bd73f20fc6f7
📒 Files selected for processing (9)
crates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rsdocs/configure-plugins/observability/atif.mdxpython/tests/test_observability_plugin.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (29)
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
python/tests/test_observability_plugin.pycrates/python/src/py_plugin.rscrates/core/src/observability/plugin_component.rscrates/core/src/plugin.rs
python/tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
python/tests/**/*.py: Pytest is used to run tests.
Do not add@pytest.mark.asyncioto any test; async tests are automatically detected and run by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, with thespecconstructor argument when necessary.
Name mocked classes with themockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; if a fixture is needed in multiple test files, place it in aconftest.pyfile.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the fixture function asdef <fixture_name>_fixture() -> <return_type>:; only specifyscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating individual tests for different input types.Maintain test coverage for Python binding and wrapper changes with the Python test suite.
Files:
python/tests/test_observability_plugin.py
**/*
📄 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:
python/tests/test_observability_plugin.pycrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rsdocs/configure-plugins/observability/atif.mdxcrates/core/tests/unit/observability/plugin_component_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.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
python/tests/test_observability_plugin.pycrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_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:
python/tests/test_observability_plugin.pycrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_tests.rs
**/*.{md,mdx,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Examples and documentation must use each exporter's documented flush/deregister order before shutdown.
Files:
python/tests/test_observability_plugin.pydocs/configure-plugins/observability/atif.mdx
**/*.{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:
python/tests/test_observability_plugin.pycrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Use
snake_casenaming in Rust and Python.
Files:
python/tests/test_observability_plugin.pycrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_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:
python/tests/test_observability_plugin.pycrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_tests.rs
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: Lint Python with Ruff using rule setsE,F,W, andI.
Format Python with the Ruff formatter, using a 120-character line length and double quotes.
Runtyfor Python type checking.
Use Pythonsnake_casenaming conventions.
Files:
python/tests/test_observability_plugin.py
**/*.{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:
python/tests/test_observability_plugin.pycrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_tests.rs
**/{test,tests}/**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the appropriate test files for each affected language binding.
Files:
python/tests/test_observability_plugin.pycrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/tests/unit/observability/plugin_component_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:
python/tests/test_observability_plugin.pycrates/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rsdocs/configure-plugins/observability/atif.mdxcrates/core/tests/unit/observability/plugin_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:
python/tests/test_observability_plugin.pycrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rs
**/*.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/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_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/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_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/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/tests/integration/atif_storage_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_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 validationUse
Result<T>withFlowErrorin core runtime paths, keeping wrapper-layer errors explicit and binding-appropriate.
Files:
crates/core/tests/integration/atif_storage_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_tests.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/tests/integration/atif_storage_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_tests.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/tests/integration/atif_storage_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_tests.rs
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
**/*.mdx: In MDX files, top-of-file comments must use JSX comment delimiters ({/*and*/}); do not use HTML comments for MDX SPDX headers.
New or regenerated MDX files must use{/* ... */}for top-of-file SPDX comments.
Files:
docs/configure-plugins/observability/atif.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/configure-plugins/observability/atif.mdx
docs/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If documentation examples or commands under
docs/change, run the targeted docs checks appropriate to the change.
Files:
docs/configure-plugins/observability/atif.mdx
**/*.{md,mdx}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If links in documentation change, run
just docs-linkcheck.Use documented public APIs and stable wrapper commands in examples and user-facing documentation; do not rely on internal helpers.
**/*.{md,mdx}: Prefer the documented public API over internal shortcuts in documentation and examples.
Keep package names, repository references, and build commands current.
Contribution workflow documentation must require an issue before external contribution pull requests and note that NVIDIA contributors may use a GitHub or Linear issue.
Update entry-point documentation when examples or reading paths change.
Keep release-process and release-notes guidance in maintainer documentation such asRELEASING.md, rather than user-facing documentation pages orCHANGELOG.md.
Use stable user-facing wrappers at thescripts/root in documentation and examples; reference namespaced helper paths only for internal maintenance documentation.
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pages.
Dynamic plugin manifests in documentation and examples should usecompat.relay = ">=0.5,<1.0"unless deliberately narrower.
Render images, diagrams, tables, and other visual content at representative page widths, ensuring legibility and complete access without clipping; use responsive scaling, reflow, or overflow as appropriate and scope visual styling narrowly.
Dynamic plugin entry pages should link to native, worker, Rust example, Python example, and protocol pages when those pages exist.
Images, diagrams, tables, and custom visual content must remain legible and fully accessible at representative desktop and narrow page widths.
Release-policy documentation must point to GitHub Releases as the only release-history source of truth.
Runjust docswhen the documentation site changes; retain./scripts/build-docs.sh htmlas the compatibility wrapper.
Files:
docs/configure-plugins/observability/atif.mdx
docs/{about-nemo-relay/concepts/subscribers.mdx,configure-plugins/observability/**/*.mdx}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Update observability documentation and examples alongside implementation changes, including configuration version 3 with one
opentelemetrysection containing typed endpoints and no standalone public OpenInference surface.
Files:
docs/configure-plugins/observability/atif.mdx
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant reference documentation when public behavior or APIs change.
Files:
docs/configure-plugins/observability/atif.mdx
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.{md,mdx,rst}: Use title case consistently for technical documentation headings and table headers; avoid quotation marks, ampersands, and exclamation marks in headings, while preserving official product, event, research, and whitepaper title case.
Format code elements, commands, parameters, package names, expressions, directories, file names, and paths in monospace; represent path placeholders with angle brackets inside monospace.
Format UI buttons, menus, fields, and labels in bold, and separate consecutive UI navigation labels with>.
Use quotation marks for error messages and strings when appropriate, italics for newly introduced terms and publication titles, and plain text for keyboard shortcuts.
Represent GitHub repositories with owner/repository link text, such as[NVIDIA/NeMo](link), rather than generic repository wording.
Introduce every code block with a complete sentence; do not let a code block complete or interrupt the grammar of surrounding prose; use syntax highlighting when supported.
Keep inline method, function, and class references consistent with nearby documentation; omit empty parentheses in prose when no call is shown.
Use descriptive link text matching the destination title when possible; avoid raw URLs, generic anchors, long-sentence links, and unnecessary links that distract from procedures.
Ensure lists have a complete lead-in sentence, more than one item, no more than two levels, parallel construction, one idea or action per item, and appropriate punctuation; use bullets for unordered items and numbers for ordered tasks.
Format definition lists with a bold term followed by a complete, parallel, punctuated definition.
Use tables for reference information, decision support, compatibility matrices, and comparable choices; flag one-row tables, missing captions or lead-ins, sentence-case headers where title case is expected, unexplained empty cells, and code or links that would be clearer as prose.
Write procedure steps as imperative ...
Files:
docs/configure-plugins/observability/atif.mdx
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.
Flag stale examples, missing SPDX headers where required, and instructions that no longer match CI or pre-commit behavior.
Files:
docs/configure-plugins/observability/atif.mdx
🧠 Learnings (3)
📚 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/python/src/py_plugin.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/integration/atif_storage_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_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.rscrates/core/src/plugin.rs
📚 Learning: 2026-07-14T02:53:59.997Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 415
File: docs/configure-plugins/observability/opentelemetry.mdx:98-113
Timestamp: 2026-07-14T02:53:59.997Z
Learning: In NeMo-Relay’s OpenTelemetry/OpenInference observability projection docs under docs/configure-plugins/observability/, document the projected-attribute contract as follows: (1) emit scalar top-level `data`/`metadata` fields as typed dotted OTLP attributes (for example, `nemo_relay.start.metadata.tenant`); (2) keep nested objects/arrays as JSON strings at their top-level OTLP attribute (rather than expanding them into nested OTLP attributes); and (3) do not reference the legacy `*_json` payload attributes (e.g., `data_json`, `metadata_json`, `input_json`) because they were intentionally removed as a breaking change.
Applied to files:
docs/configure-plugins/observability/atif.mdx
🔇 Additional comments (10)
crates/core/src/observability/plugin_component.rs (1)
896-951: LGTM!Also applies to: 1024-1037, 1094-1131, 1669-1693
crates/core/src/plugin.rs (2)
340-347: LGTM!Also applies to: 358-386, 2378-2425
1582-1615: LGTM!Also applies to: 2189-2192, 2215-2215
crates/core/tests/unit/plugin_tests.rs (1)
1509-1534: LGTM!Also applies to: 1536-1581, 1590-1599, 1618-1627, 1640-1704, 2311-2322
crates/core/tests/integration/atif_storage_tests.rs (1)
26-27: LGTM!Also applies to: 501-512, 538-546, 575-587
crates/core/tests/unit/observability/plugin_component_tests.rs (1)
2450-2458: LGTM!Also applies to: 2759-2824, 2827-2889, 2892-2925, 3064-3070
docs/configure-plugins/observability/atif.mdx (1)
92-105: LGTM!Also applies to: 250-258
python/tests/test_observability_plugin.py (1)
17-17: LGTM!Also applies to: 442-485
crates/python/src/py_plugin.rs (2)
936-948: LGTM!
972-979: LGTM!
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/python/tests/coverage/py_plugin_coverage_tests.rs`:
- Around line 212-214: Update the cleanup in the test using
lock_plugin_test_state_for_tests so the Arc comparison result is captured before
calling reset_plugin_configuration_clear_state, then assert the captured result
afterward; alternatively, add a Drop guard that always resets
PLUGIN_CONFIGURATION_CLEAR_STATE if the assertion panics.
🪄 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: f0766c4b-d4b0-46b4-8186-a39b92a46c67
📒 Files selected for processing (4)
crates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (18)
**/*.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/core/tests/unit/observability/plugin_component_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.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.
Files:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.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/core/tests/unit/observability/plugin_component_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.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/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.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.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.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/core/tests/unit/observability/plugin_component_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.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/core/tests/unit/observability/plugin_component_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Use
snake_casenaming in Rust and Python.
Files:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.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/core/tests/unit/observability/plugin_component_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.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/core/tests/unit/observability/plugin_component_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.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/core/tests/unit/observability/plugin_component_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rs
**/{test,tests}/**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the appropriate test files for each affected language binding.
Files:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/unit/plugin_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/core/tests/unit/observability/plugin_component_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.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/observability/plugin_component_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/unit/plugin_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/python/tests/coverage/py_plugin_coverage_tests.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/core/src/plugin.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/core/tests/unit/observability/plugin_component_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.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/plugin.rs
🔇 Additional comments (4)
crates/core/src/plugin.rs (1)
360-378: LGTM!Also applies to: 2508-2510, 2535-2537
crates/core/tests/unit/plugin_tests.rs (1)
1480-1509: LGTM!crates/core/tests/unit/observability/plugin_component_tests.rs (1)
2474-2478: LGTM!crates/python/tests/coverage/py_plugin_coverage_tests.rs (1)
201-211: LGTM!
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
/merge |
3 participants
Overview
Reject non-string ATIF filename metadata instead of silently routing trajectories through a fallback, and make remote-delivery teardown failures consistently observable and drainable.
Details
plugin.report()after subscriber delivery is flushed.Validation:
release/0.7.cargo fmt, workspace clippy, cargo check, changed-file pre-commit checks, Go tests, and documentation checks passed./Users/wkillian/.nemo-relay/plugins.toml, followed by global-state cascades where those tests aborted before cleanup.Where should the reviewer start?
Start with
render_atif_filenameincrates/core/src/observability/plugin_component.rs, then the end-to-end regression inpython/tests/test_observability_plugin.py.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
Bug Fixes
Documentation
Tests