fix: recover failed ATIF remote deliveries - #665
Conversation
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughATIF runtime failures now aggregate in ChangesATIF runtime diagnostics and recovery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AtifDispatcher
participant RemoteStorage
participant LocalStorage
participant ActivePluginReport
AtifDispatcher->>RemoteStorage: Write trajectory
RemoteStorage-->>AtifDispatcher: Return delivery failure
AtifDispatcher->>LocalStorage: Write recovery copy
LocalStorage-->>AtifDispatcher: Return fallback result
AtifDispatcher->>ActivePluginReport: Record runtime diagnostic
ActivePluginReport-->>AtifDispatcher: Report diagnostics during teardown
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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 (2)
crates/core/src/observability/plugin_component.rs (1)
1333-1397: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win"atif.local_fallback_failed" is used even when Local is the only configured sink, not a fallback.
This code assigns
"atif.local_fallback_failed"to every failedSinkLabel::Localresult, without checking whether anySinkLabel::Remotetarget was also present in the sameresults. When no remote storage is configured (sink_targets()returns[SinkLabel::Local]only), a Local write failure is the primary write failure, not a fallback after a remote failure. Labeling it "local_fallback_failed" misleads anyone readingplugin.report()or the teardown error into thinking a remote delivery was attempted first.The test
atif_dispatcher_records_failed_agent_writesincrates/core/tests/unit/observability/plugin_component_tests.rs(lines 2250-2297) confirms this: it usesAtifSectionConfigwith nostorageconfigured, then asserts the failure code is"atif.local_fallback_failed"for what is actually a primary local write failure.Distinguish the two cases by checking whether any
Remotelabel is present in the sameresultsbatch.🐛 Proposed fix: distinguish primary local failure from fallback failure
fn complete_scope_write( &mut self, agent_uuid: Uuid, results: Vec<(SinkLabel, std::io::Result<()>)>, ) -> Option<(Uuid, String)> { + let is_remote_fallback = results + .iter() + .any(|(label, _)| matches!(label, SinkLabel::Remote(_))); for (label, result) in results { ... self.record_runtime_failure( - if matches!(label, SinkLabel::Local) { - "atif.local_fallback_failed" - } else { - "atif.remote_delivery_failed" - }, + match &label { + SinkLabel::Local if is_remote_fallback => "atif.local_fallback_failed", + SinkLabel::Local => "atif.local_write_failed", + SinkLabel::Remote(_) => "atif.remote_delivery_failed", + }, Some(field), message, Some(agent_uuid.to_string()), );After this fix, update the expected code in
atif_dispatcher_records_failed_agent_writes(crates/core/tests/unit/observability/plugin_component_tests.rs, line 2292) to"atif.local_write_failed", since that test configures no remote storage.🤖 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 1333 - 1397, Update complete_scope_write to detect whether the results batch contains any SinkLabel::Remote entries, then record a Local failure as "atif.local_fallback_failed" only when a remote target was present; otherwise use "atif.local_write_failed". Update atif_dispatcher_records_failed_agent_writes to expect "atif.local_write_failed" for its local-only configuration.crates/core/src/plugin.rs (1)
2033-2078: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFragile string-matching gates a safety-critical decision.
callbacks_cleareddepends on every deregistration error containing the literal substring"ATIF runtime delivery failures". This exact text is also formatted incrates/core/src/observability/plugin_component.rs'slast_error_result(). Nothing ties these two literals together at compile time. If either message wording changes later, this classification silently breaks: unsafe callback removal could get misclassified as safe, or vice versa, with no test failure pointing at the actual cause.Extract a shared constant for this marker and reference it from both sites, instead of duplicating the literal string.
♻️ Proposed fix: share one marker constant
+// crates/core/src/observability/plugin_component.rs +pub(crate) const ATIF_RUNTIME_DELIVERY_FAILURE_MARKER: &str = "ATIF runtime delivery failures";- let callbacks_cleared = deregistration_errors - .iter() - .all(|error| error.contains("ATIF runtime delivery failures")); + let callbacks_cleared = deregistration_errors.iter().all(|error| { + error.contains(crate::observability::plugin_component::ATIF_RUNTIME_DELIVERY_FAILURE_MARKER) + });Separately, verify a dedicated test exists for this exact 2054-2060 classification path, confirming that after a teardown fails solely on ATIF runtime delivery errors, a subsequent
initialize_plugins_exactcall still succeeds (i.e., future plugin configuration changes stay enabled). The line-range change details mark this segment as high complexity, and the tests shown inplugin_tests.rsfor this PR only touchConfigReport::default()fixtures, not this classification branch.🤖 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 2033 - 2078, Extract a shared public marker constant for “ATIF runtime delivery failures” in the appropriate core module, update clear_plugin_configuration_inner’s callbacks_cleared classification and plugin_component.rs’s last_error_result() formatting to reference it, and add or verify a focused test proving teardown with only ATIF delivery errors leaves a subsequent initialize_plugins_exact call successful.
🤖 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/tests/unit/observability/plugin_component_tests.rs`:
- Around line 2289-2294: Update the runtime failure code assertion in the
relevant observability plugin component test to match the corrected non-fallback
label used by complete_scope_write when no remote storage is configured. Keep
the test’s single-failure and error-result assertions unchanged.
- Around line 1835-1847: Extend the test around active_plugin_report() and
clear_plugin_configuration() to call initialize_plugins_exact again after the
expected teardown error, and assert that the subsequent initialization succeeds,
preserving the guarantee that an ATIF-only teardown failure does not disable
future plugin configuration mutations.
---
Outside diff comments:
In `@crates/core/src/observability/plugin_component.rs`:
- Around line 1333-1397: Update complete_scope_write to detect whether the
results batch contains any SinkLabel::Remote entries, then record a Local
failure as "atif.local_fallback_failed" only when a remote target was present;
otherwise use "atif.local_write_failed". Update
atif_dispatcher_records_failed_agent_writes to expect "atif.local_write_failed"
for its local-only configuration.
In `@crates/core/src/plugin.rs`:
- Around line 2033-2078: Extract a shared public marker constant for “ATIF
runtime delivery failures” in the appropriate core module, update
clear_plugin_configuration_inner’s callbacks_cleared classification and
plugin_component.rs’s last_error_result() formatting to reference it, and add or
verify a focused test proving teardown with only ATIF delivery errors leaves a
subsequent initialize_plugins_exact call successful.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 4a389eb3-4bb5-4548-9efe-244a7fa71988
📒 Files selected for processing (10)
crates/core/src/observability/plugin_component.rscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/node/plugin.d.tsdocs/configure-plugins/observability/atif.mdxgo/nemo_relay/plugin.gopython/nemo_relay/plugin.pypython/nemo_relay/plugin.pyiskills/nemo-relay-plugin-observability/references/atif.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (41)
**/*.{md,rst,html,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
**/*.{md,rst,html,txt}: Always spellNVIDIAin all caps. Do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun because the name starts with an 'en' sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company.
Use trademark symbols with product names only when the document type or legal guidance requires them.
Verify official capitalization, spacing, and hyphenation for product names.
Precede NVIDIA product names withNVIDIAon first mention when it is natural and accurate.
Do not rewrite product names for grammar or title-case rules.
Preserve third-party product names according to the owner's spelling.
Include the company name and full model qualifier on first use when it helps identify the model.
Preserve the official capitalization and punctuation of model names.
Use shorter family names only after the full name is established.
Spell out a term on first use and put the acronym in parentheses unless the acronym is widely understood by the intended audience.
Use the acronym on later mentions after it has been defined.
For long documents, reintroduce the full term if readers might lose context.
Form plurals of acronyms withs, not an apostrophe, such asGPUs.
In headings, common acronyms can remain abbreviated. Spell out the term in the first or second sentence of the body.
Common terms such asCPU,GPU,PC,API, andUIusually do not need to be spelled out for developer audiences.
Files:
skills/nemo-relay-plugin-observability/references/atif.md
**/*.{md,rst,html}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
Link the first mention of a product name when the destination helps the reader.
Files:
skills/nemo-relay-plugin-observability/references/atif.md
**/*.{md,rst,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
Spell
NVIDIAin all caps. Do not useNvidia,nvidia, orNV.
Files:
skills/nemo-relay-plugin-observability/references/atif.md
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,rst}: Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text. Avoid raw URLs and weak anchors such as "here" or "read more."
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative steps. Keep steps parallel and split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce.
Preferrefer tooverseewhen the wording points readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical docs.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values.
Use numerals for 10 or greater and include commas in thousands.
Do not add trademark symbols to learning-oriented docs unless the source, platform, or legal guidance explicitly requires them.
Files:
skills/nemo-relay-plugin-observability/references/atif.md
**/*.md
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.md: Use title case consistently in technical documentation headings
Avoid quotation marks, ampersands, and exclamation marks in headings
Keep product, event, research, and whitepaper names in their official title case
Use title case for table headers
Do not force social-media sentence case into technical docs
Format code elements, commands, parameters, package names, and expressions in monospace
Format directories, file names, and paths in monospace using backticks
Use angle brackets inside monospace for variables inside paths, such as/home/<username>/.login
Format error messages and strings in quotation marks, keeping literal code strings in code formatting when clearer
Format UI buttons, menus, fields, and labels in bold
Use angle brackets between UI labels for menu paths, such as File > Save As
Use italics for new terms on first use, sparingly and only when introducing the term
Use italics for publication titles
Format keyboard shortcuts in plain text, such as Press Ctrl+Alt+Delete
Use owner/repo link text for GitHub repositories, preferring[NVIDIA/NeMo](link)over prose references like 'the GitHub repo'
Introduce every code block with a complete sentence
Do not make a code block complete the grammar of the previous sentence
Do not continue a sentence after a code block
Use syntax highlighting when the format supports it for code blocks
Avoid the word 'snippet' unless the surrounding docs already use it as a term of art
Keep inline method, function, and class references consistent with nearby docs, omitting empty parentheses for prose readability when no call is shown
Use descriptive anchor text that matches the destination title when possible for links
Avoid raw URLs in running text
Avoid generic anchor text such as 'here,' 'this page,' and 'read more'
Include acronyms in link text when a linked term includes an acronym
Do not link long sentences or multiple sentences
Avoid links that pull readers away from a procedure unless the link is a p...
Files:
skills/nemo-relay-plugin-observability/references/atif.md
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Update
README.md,fern/, package READMEs, and binding-support notes when public behavior, package names, examples, or supported bindings change.
**/*.{md,mdx}: Prefer the documented public API, not internal shortcuts
Keep package names, repo references, and build commands current
Keep release-process and release-notes guidance in repo-maintainer docs such asRELEASING.md, not as user-facing docs pages orCHANGELOG.md
Keep stable user-facing wrappers atscripts/root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pagesIf links in documentation change, run
just docs-linkcheck.
Files:
skills/nemo-relay-plugin-observability/references/atif.mddocs/configure-plugins/observability/atif.mdx
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
skills/nemo-relay-plugin-observability/references/atif.mddocs/configure-plugins/observability/atif.mdx
**/*
📄 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.
Files:
skills/nemo-relay-plugin-observability/references/atif.mdpython/nemo_relay/plugin.pyipython/nemo_relay/plugin.pydocs/configure-plugins/observability/atif.mdxcrates/node/plugin.d.tscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rsgo/nemo_relay/plugin.go
**/*.{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:
skills/nemo-relay-plugin-observability/references/atif.mdpython/nemo_relay/plugin.pydocs/configure-plugins/observability/atif.mdxcrates/node/plugin.d.tsgo/nemo_relay/plugin.go
{crates/python/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update the language-native bindings for every exposed surface in Python, Go, and Node.js.
Files:
python/nemo_relay/plugin.pyipython/nemo_relay/plugin.pygo/nemo_relay/plugin.go
{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update language wrapper helpers such as Python wrapper modules, Python type stubs, and Go shorthand packages when the new behavior belongs in those helper layers.
Files:
python/nemo_relay/plugin.pyipython/nemo_relay/plugin.pygo/nemo_relay/plugin.go
python/nemo_relay/**/*
⚙️ CodeRabbit configuration file
python/nemo_relay/**/*: Review Python wrapper changes for typed API consistency, contextvars-based scope isolation, async behavior, and parity with the native extension.
Stubs and runtime implementations should stay aligned.
Files:
python/nemo_relay/plugin.pyipython/nemo_relay/plugin.py
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
python/nemo_relay/plugin.pycrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
python/nemo_relay/plugin.pycrates/node/plugin.d.tscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rs
python/nemo_relay/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Python wrapper modules live under
python/nemo_relay/, and the native extension is built fromcrates/pythonwithmaturin.
Files:
python/nemo_relay/plugin.py
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: When changing the Python wrapper package, tests, or docs tooling, lint with Ruff (E,F,W,I), format with Ruff formatter (120-character lines, double quotes), and passtytype checking.
Add the SPDX license header to all Python source files using the#comment form.
Files:
python/nemo_relay/plugin.py
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
python/nemo_relay/plugin.pycrates/node/plugin.d.tscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rsgo/nemo_relay/plugin.go
{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/nemo_relay/plugin.pycrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rs
python/nemo_relay/{adaptive.py,plugin.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep Python adaptive/plugin wrappers in
python/nemo_relay/adaptive.pyandpython/nemo_relay/plugin.pysynchronized with the shared adaptive/plugin boundary and lifecycle.
Files:
python/nemo_relay/plugin.py
**/*.{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/nemo_relay/plugin.pycrates/node/plugin.d.tscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rsgo/nemo_relay/plugin.go
**/*.{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/nemo_relay/plugin.pycrates/node/plugin.d.tscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rsgo/nemo_relay/plugin.go
**/*.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)
In MDX files, top-of-file comments must use JSX comment delimiters (
{/*to open and*/}to close); do not use HTML comments for MDX SPDX headers
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
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/**,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/node/**/*.{js,ts,jsx,tsx,json}
📄 CodeRabbit inference engine (.agents/skills/test-node-binding/SKILL.md)
Format changed Node files with
npm run format --workspace=nemo-relay-node
Files:
crates/node/plugin.d.ts
crates/node/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.agents/skills/test-node-binding/SKILL.md)
Use
npm run check:docstrings --workspace=nemo-relay-nodeto validate public API docstring checks when surface docs changed
Files:
crates/node/plugin.d.ts
crates/node/**/*.{js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
camelCasefor Node.js public APIs.
Files:
crates/node/plugin.d.ts
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/node/plugin.d.tscrates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rsgo/nemo_relay/plugin.go
crates/{python,ffi,node}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node}/**/*: Treat binding changes as public API changes. Check for parity with the other language bindings, FFI ownership/lifetime safety,
callback error propagation, stable type conversion, and consistent async/stream semantics.
Flag changes that update one binding without corresponding tests or documentation for the same surface elsewhere.
Files:
crates/node/plugin.d.ts
**/*.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 workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.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.
Files:
crates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_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/src/plugin.rscrates/core/tests/unit/observability/plugin_component_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 validation
Files:
crates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_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/src/plugin.rscrates/core/tests/unit/observability/plugin_component_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/src/plugin.rscrates/core/tests/unit/observability/plugin_component_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:
crates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rs
go/nemo_relay/**/*.go
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
go/nemo_relay/**/*.go: Format changed Go packages withcd go/nemo_relay && go fmt ./...
Run Go tests withjust test-goto build and test the NeMo Relay Go binding
Usejust build-gowhen you want an explicit build-only pass or need the artifact for other work
Usejust ci=true test-gowhen you need the CI-style coverage and JUnit path
On macOS, setDYLD_LIBRARY_PATHto the../../target/releasedirectory before running the rawgo testcommand directlyUse
PascalCasefor public Go APIs.
Files:
go/nemo_relay/plugin.go
**/*.go
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When changing the experimental Go binding, format Go code with
gofmtand keepgo vet ./...passing.
Files:
go/nemo_relay/plugin.go
go/nemo_relay/**
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep shared plugin helpers in
go/nemo_relayaligned with plugin registration, composition, and lifecycle behavior.
Files:
go/nemo_relay/plugin.go
go/nemo_relay/**/*
⚙️ CodeRabbit configuration file
go/nemo_relay/**/*: Review Go binding changes for cgo memory ownership, race safety, callback cleanup, idiomatic exported APIs, and parity with Rust/FFI behavior.
Any API change should include focused Go tests and consider race-test behavior.
Files:
go/nemo_relay/plugin.go
🧠 Learnings (4)
📚 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-05-07T18:04:44.387Z
Learnt from: mnajafian-nv
Repo: NVIDIA/NeMo-Flow PR: 67
File: integrations/openclaw/src/modules.ts:1-2
Timestamp: 2026-05-07T18:04:44.387Z
Learning: In NVIDIA/NeMo-Flow, TypeScript source files should use `//` line comments for SPDX headers (e.g., `// SPDX-FileCopyrightText: ...` and `// SPDX-License-Identifier: ...`) rather than C-style block comments (`/* ... */`). The repo’s copyright checker enforces this mapping, so `//` SPDX headers in `.ts` files should not be flagged as a style violation.
Applied to files:
crates/node/plugin.d.ts
📚 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
📚 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/plugin.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/src/observability/plugin_component.rs
🔇 Additional comments (14)
crates/core/src/plugin.rs (2)
189-212: LGTM!Also applies to: 1534-1537
2198-2223: 🩺 Stability & AvailabilityVerify lock-ordering safety for the new nested lock.
record_active_plugin_runtime_diagnosticlocksACTIVE_PLUGIN_CONFIGURATIONand is now called fromAtifDispatcher::record_runtime_failure, which typically runs while the caller already holds the dispatcher's ownMutex<AtifDispatcher>(for example throughcomplete_scope_writeinsidemanager.lock()...complete_scope_write(...)). Confirm no other code path acquires these two locks in the reverse order (holdingACTIVE_PLUGIN_CONFIGURATION's lock while calling into a method that locks the same dispatcher), which would create a deadlock.#!/bin/bash # Description: Look for code paths that hold ACTIVE_PLUGIN_CONFIGURATION's lock # while calling into AtifDispatcher methods, which would create reverse lock order. rg -n "ACTIVE_PLUGIN_CONFIGURATION.lock" crates/core/src -A 15 rg -n "fn store_active_plugin_configuration" crates/core/src -A 25crates/node/plugin.d.ts (1)
50-63: LGTM!go/nemo_relay/plugin.go (1)
220-233: LGTM!python/nemo_relay/plugin.py (2)
94-98: LGTM!Also applies to: 597-597
83-91: 🎯 Functional CorrectnessConfirm the native
RuntimeDiagnosticfield requirements before changing both declarations.total=Falsemakes every key optional. Split required and optional keys only if the native type guarantees thatcode,component,message, andcountare present.python/nemo_relay/plugin.pyi (1)
44-46: LGTM!crates/core/src/observability/plugin_component.rs (3)
1480-1488: 📐 Maintainability & Code Quality | ⚡ Quick winVerify docs still match the new "retry every trajectory" sink behavior.
sink_targets()no longer excludes previously failed sinks — every configured remote destination is retried for every new trajectory, per the line-range change details ("without excluding previously failed sinks").docs/configure-plugins/observability/atif.mdx(unchanged by this PR, lines 165-167 and 239-243) still states that a failed HTTP/S3 destination "records the endpoint as unhealthy and skips it for later trajectories." If a lower-level per-destination health tracker still exists in the remote-storage backend, confirm it doesn't re-introduce permanent skipping that would contradict this dispatcher-level retry-every-trajectory design. If that tracker was removed as part of this fix, update the two doc passages to describe the current retry behavior instead of "skips it for later trajectories."#!/bin/bash # Description: Check whether ATIF remote storage backends still track and skip "unhealthy" destinations. rg -n "unhealthy" crates/core/src/observability -C 6 rg -n "struct AtifRemoteStorage" crates/core/src/observability -A 30Also applies to: 1785-1820
1464-1478: 🎯 Functional Correctness | ⚡ Quick winVerify filename_template path-safety is also checked at config-activation time.
validate_atif_filename_templateruns insideprepare_destination, which is only invoked per top-level trajectory (inobserve_global). The static checks here (must contain{session_id}, must be a relative traversal-free path) don't depend on any runtime metadata, so an entirely broken template (for example, an absolute path) can passinitialize()/validate_plugin_configwith noConfigDiagnostic, and only surface once the first trajectory is silently skipped. Confirm whethervalidate_plugin_config(or the observability component's own validation step) already calls this same check at activation time; if not, consider calling it there too so a broken static template is rejected immediately instead of deferred to the first trajectory.#!/bin/bash # Description: Check whether filename_template static validation also runs at plugin activation/config-validation time. rg -n "validate_atif_filename_template" crates/core/src -B 5 -A 5 rg -n "fn validate_plugin_config" crates/core/src/plugin.rs -A 40Also applies to: 1542-1565
25-25: LGTM!Also applies to: 67-67, 1088-1099, 1189-1200, 1226-1245, 1437-1452, 1490-1517, 1549-1565, 1744-1783, 1822-1839
crates/core/tests/unit/observability/plugin_component_tests.rs (1)
709-730: LGTM!Also applies to: 2241-2241, 2328-2349, 2416-2417
crates/core/tests/unit/plugin_tests.rs (1)
747-747: LGTM!Also applies to: 1032-1038
docs/configure-plugins/observability/atif.mdx (1)
70-72: LGTM!Also applies to: 100-103
skills/nemo-relay-plugin-observability/references/atif.md (1)
71-77: LGTM!
Signed-off-by: Will Killian <wkillian@nvidia.com>
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/tests/integration/atif_storage_tests.rs`:
- Line 542: Update the teardown assertion around clear_plugin_configuration to
inspect the returned error rather than only asserting failure. Verify that the
error includes the expected ATIF runtime diagnostic code or failed destination,
preserving coverage that accumulated delivery failures are reported.
- Around line 535-540: Strengthen the assertions in the request verification
block by inspecting each request’s session or trajectory identifier, and assert
that exactly one POST belongs to handle.uuid and exactly one belongs to
second.uuid. Keep the existing POST method and /fail path checks while
validating cross-request isolation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 12c17ab7-37a6-4159-8081-a1dc6e159d29
📒 Files selected for processing (2)
crates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_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 (13)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.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.
Files:
crates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/tests/integration/atif_storage_tests.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 validation
Files:
crates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_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.
Files:
crates/core/tests/integration/atif_storage_tests.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/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:
crates/core/tests/integration/atif_storage_tests.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:
crates/core/tests/integration/atif_storage_tests.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/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:
crates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rs
🧠 Learnings (1)
📚 Learning: 2026-08-03T19:55:03.931Z
Learnt from: afourniernv
Repo: NVIDIA/NeMo-Relay PR: 558
File: crates/pii-redaction/src/rampart/mod.rs:265-274
Timestamp: 2026-08-03T19:55:03.931Z
Learning: In NeMo Relay first-party plugin registration helpers, treat the documented duplicate-registration `PluginError::RegistrationFailed` result from `register_plugin` as success when registration is intended to be idempotent. Do not locally reclassify this as `PluginError::Conflict`; changing the classification requires a core-wide review of the public API and FFI behavior.
Applied to files:
crates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rs
🔇 Additional comments (7)
crates/core/tests/unit/observability/plugin_component_tests.rs (6)
1835-1848: Cover configuration changes after the teardown error.After
clear_plugin_configuration().unwrap_err(), callinitialize_plugins_exactwith a valid configuration and assert success. Then clean up the new configuration. This verifies that the reported teardown failure does not disable future plugin configuration changes.This repeats the unresolved gap from the previous review. As per path instructions, tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Source: Path instructions
2290-2295: Verify the diagnostic code against the configured sinks.If this test configures only a local sink,
atif.local_fallback_failedmislabels the failure because no remote delivery was attempted. Use the non-fallback local-write code, or configure a failing remote sink before asserting a fallback code.This repeats the unresolved previous-review finding. As per path instructions, tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Source: Path instructions
709-729: Run the required validation before handoff.No validation results are included. Run:
cargo fmt --all just test-rust cargo clippy --workspace --all-targets -- -D warnings validate-change just test-python just test-go just test-node uv run pre-commit run --all-filesAs per coding guidelines, Rust changes require formatting, Rust tests, strict Clippy,
validate-changeforcrates/core, the full language matrix, and the final all-files pre-commit run.Source: Coding guidelines
2242-2242: LGTM!
2329-2350: LGTM!
2417-2418: LGTM!crates/core/tests/integration/atif_storage_tests.rs (1)
497-497: LGTM!
Signed-off-by: Will Killian <wkillian@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
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/plugin.rs (1)
2055-2060: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftUse a typed teardown result for callback-safety classification.
Line 2060 treats any deregistration error that contains this text as a recoverable ATIF error. A different failing callback can include the same text. The code can then set
callbacks_clearedtotrueeven when a callback may remain registered. Preserve a typed ATIF delivery-failure outcome through deregistration, and only that outcome should permit later configuration mutations.🤖 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 2055 - 2060, Replace the text-based classification in the deregistration flow around callbacks_cleared with a typed teardown result that distinguishes the ATIF runtime delivery failure from unrelated callback failures. Preserve that typed outcome through callback deregistration, and allow callbacks_cleared to be true only for the specific typed ATIF delivery-failure result; all other failures must remain unsafe and block subsequent configuration mutations.
🤖 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 `@docs/configure-plugins/observability/atif.mdx`:
- Around line 165-169: Update the documentation’s `output_directory` description
near the storage configuration to distinguish normal remote writes from recovery
behavior: state that it is ignored for successful remote storage writes but used
for the local recovery copy when all configured remote destinations fail for a
trajectory. Keep the existing remote storage configuration guidance consistent
with the behavior described near the delivery diagnostics section.
---
Outside diff comments:
In `@crates/core/src/plugin.rs`:
- Around line 2055-2060: Replace the text-based classification in the
deregistration flow around callbacks_cleared with a typed teardown result that
distinguishes the ATIF runtime delivery failure from unrelated callback
failures. Preserve that typed outcome through callback deregistration, and allow
callbacks_cleared to be true only for the specific typed ATIF delivery-failure
result; all other failures must remain unsafe and block subsequent configuration
mutations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 64fdb1fa-8347-4ebc-b9e3-96572c43bb0b
📒 Files selected for processing (7)
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.rsdocs/configure-plugins/observability/atif.mdxpython/nemo_relay/plugin.pypython/nemo_relay/plugin.pyi
📜 Review details
🧰 Additional context used
📓 Path-based instructions (28)
**/*.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 workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.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.
Files:
crates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.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/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.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 validation
Files:
crates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rscrates/core/src/observability/plugin_component.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rspython/nemo_relay/plugin.pycrates/core/src/observability/plugin_component.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rspython/nemo_relay/plugin.pycrates/core/src/observability/plugin_component.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rspython/nemo_relay/plugin.pycrates/core/src/observability/plugin_component.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/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.
Files:
crates/core/tests/integration/atif_storage_tests.rsdocs/configure-plugins/observability/atif.mdxpython/nemo_relay/plugin.pyicrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rspython/nemo_relay/plugin.pycrates/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/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.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:
crates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rspython/nemo_relay/plugin.pycrates/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:
crates/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rspython/nemo_relay/plugin.pycrates/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/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.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:
crates/core/tests/integration/atif_storage_tests.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)
In MDX files, top-of-file comments must use JSX comment delimiters (
{/*to open and*/}to close); do not use HTML comments for MDX SPDX headers
Files:
docs/configure-plugins/observability/atif.mdx
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Update
README.md,fern/, package READMEs, and binding-support notes when public behavior, package names, examples, or supported bindings change.
**/*.{md,mdx}: Prefer the documented public API, not internal shortcuts
Keep package names, repo references, and build commands current
Keep release-process and release-notes guidance in repo-maintainer docs such asRELEASING.md, not as user-facing docs pages orCHANGELOG.md
Keep stable user-facing wrappers atscripts/root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pagesIf links in documentation change, run
just docs-linkcheck.
Files:
docs/configure-plugins/observability/atif.mdx
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
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,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/nemo_relay/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/**,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/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update the language-native bindings for every exposed surface in Python, Go, and Node.js.
Files:
python/nemo_relay/plugin.pyipython/nemo_relay/plugin.py
{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update language wrapper helpers such as Python wrapper modules, Python type stubs, and Go shorthand packages when the new behavior belongs in those helper layers.
Files:
python/nemo_relay/plugin.pyipython/nemo_relay/plugin.py
python/nemo_relay/**/*
⚙️ CodeRabbit configuration file
python/nemo_relay/**/*: Review Python wrapper changes for typed API consistency, contextvars-based scope isolation, async behavior, and parity with the native extension.
Stubs and runtime implementations should stay aligned.
Files:
python/nemo_relay/plugin.pyipython/nemo_relay/plugin.py
{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.rspython/nemo_relay/plugin.pycrates/core/src/observability/plugin_component.rs
python/nemo_relay/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Python wrapper modules live under
python/nemo_relay/, and the native extension is built fromcrates/pythonwithmaturin.
Files:
python/nemo_relay/plugin.py
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: When changing the Python wrapper package, tests, or docs tooling, lint with Ruff (E,F,W,I), format with Ruff formatter (120-character lines, double quotes), and passtytype checking.
Add the SPDX license header to all Python source files using the#comment form.
Files:
python/nemo_relay/plugin.py
python/nemo_relay/{adaptive.py,plugin.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep Python adaptive/plugin wrappers in
python/nemo_relay/adaptive.pyandpython/nemo_relay/plugin.pysynchronized with the shared adaptive/plugin boundary and lifecycle.
Files:
python/nemo_relay/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/core/tests/integration/atif_storage_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.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 (4)
crates/core/src/observability/plugin_component.rs (1)
71-72: LGTM!Also applies to: 1199-1199, 1231-1236, 1340-1342, 1383-1386, 1448-1448, 1474-1475, 1487-1522
crates/core/tests/unit/observability/plugin_component_tests.rs (1)
1835-1871: LGTM!Also applies to: 2300-2304
crates/core/tests/integration/atif_storage_tests.rs (1)
541-559: LGTM!python/nemo_relay/plugin.py (1)
90-94: 🗄️ Data Integrity & IntegrationNo change is required.
RuntimeDiagnostic.fieldandRuntimeDiagnostic.session_idomit absent values during serialization.> Likely an incorrect or invalid review comment.
Signed-off-by: Will Killian <wkillian@nvidia.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/configure-plugins/observability/atif.mdx (1)
243-251: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winState that delivery diagnostics make teardown fail.
The PR objective says recorded delivery failures cause teardown to fail. “Causes plugin teardown to report the degraded delivery” can imply a non-fatal report. Replace it with “causes plugin teardown to fail,” or state the exact failure behavior. Keep this wording consistent with Lines 100-101.
As per path instructions, “Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.”
🤖 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 `@docs/configure-plugins/observability/atif.mdx` around lines 243 - 251, Update the ATIF exporter documentation sentence about the active plugin report so it explicitly states that recorded delivery diagnostics cause plugin teardown to fail, matching the failure behavior documented in lines 100-101; leave the surrounding recovery and destination behavior unchanged.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.
Outside diff comments:
In `@docs/configure-plugins/observability/atif.mdx`:
- Around line 243-251: Update the ATIF exporter documentation sentence about the
active plugin report so it explicitly states that recorded delivery diagnostics
cause plugin teardown to fail, matching the failure behavior documented in lines
100-101; leave the surrounding recovery and destination behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: d8db5322-1590-4a17-b7d1-c5bc39d94ebb
📒 Files selected for processing (1)
docs/configure-plugins/observability/atif.mdx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*.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)
In MDX files, top-of-file comments must use JSX comment delimiters (
{/*to open and*/}to close); do not use HTML comments for MDX SPDX headers
Files:
docs/configure-plugins/observability/atif.mdx
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Update
README.md,fern/, package READMEs, and binding-support notes when public behavior, package names, examples, or supported bindings change.
**/*.{md,mdx}: Prefer the documented public API, not internal shortcuts
Keep package names, repo references, and build commands current
Keep release-process and release-notes guidance in repo-maintainer docs such asRELEASING.md, not as user-facing docs pages orCHANGELOG.md
Keep stable user-facing wrappers atscripts/root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pagesIf links in documentation change, run
just docs-linkcheck.
Files:
docs/configure-plugins/observability/atif.mdx
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
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
**/*
📄 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.
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,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.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/**,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 (1)
📚 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 (3)
docs/configure-plugins/observability/atif.mdx (3)
100-103: LGTM!Also applies to: 168-171
117-119: 🎯 Functional CorrectnessDocument recovery behavior when
output_directoryis unset.
AtifConfig.output_directorydefaults toNoneinpython/nemo_relay/observability.py. This section does not state where Relay writes the recovery copy when no directory is configured. The remote-storage examples also omitoutput_directory. Verify the runtime fallback and document the actual location, or show the required setting in the examples.Source: Path instructions
70-72: 🗄️ Data Integrity & IntegrationKeep the remote recovery summary as written. It correctly states that Relay writes a local recovery copy only when every configured remote destination fails.
> Likely an incorrect or invalid review comment.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/0.7 #665 +/- ##
===============================================
- Coverage 94.39% 94.39% -0.00%
===============================================
Files 330 330
Lines 96416 96557 +141
Branches 113 113
===============================================
+ Hits 91011 91141 +130
- Misses 5404 5416 +12
+ Partials 1 0 -1
... and 8 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
mnajafian-nv
left a comment
There was a problem hiding this comment.
conditional approval upon review of the repro-backed inline suggestion on teardown-time ATIF diagnostics.
Signed-off-by: Will Killian <wkillian@nvidia.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/configure-plugins/observability/atif.mdx (1)
114-119: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winQualify recovery behavior when
output_directoryis absent.Line 117 guarantees a local recovery copy after total remote failure.
output_directoryis optional. When no local path exists,write_atifreturns ano output patherror, so ATIF cannot create that copy.State that ATIF attempts local recovery only when
output_directoryis configured. Also apply the same condition to Lines 167-171 and Lines 243-251. Document that a failed local recovery write remains a runtime diagnostic.Proposed documentation update
-`output_directory` is ignored for successful remote writes, but Relay uses it -for a local recovery copy when every configured remote destination fails for a -trajectory. +`output_directory` is ignored for successful remote writes. If every configured +remote destination fails and `output_directory` is configured, Relay attempts a +local recovery copy. If no local path exists or the recovery write fails, Relay +records a runtime diagnostic.As per path instructions, “Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.”
🤖 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 `@docs/configure-plugins/observability/atif.mdx` around lines 114 - 119, Update the ATIF documentation around the remote-write fallback and the corresponding sections near the referenced recovery guidance to state that local recovery is attempted only when output_directory is configured; when it is absent, no local recovery copy is created and write_atif may return a “no output path” error. Also document that failures writing the configured recovery copy remain runtime diagnostics.Source: Path instructions
crates/core/src/plugin.rs (2)
2076-2078: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftUse structured teardown failure status.
rollback_registrations_checkedincludes the registration name and error text in each message. A non-ATIF deregistration failure that containsATIF_RUNTIME_DELIVERY_FAILURE_MARKERthen setscallbacks_clearedtotrue.clear_plugin_configuration()releases the mutation lease although a callback can remain registered.Return structured status from the known ATIF teardown path. Do not infer callback-removal safety from an error substring. Test marker text in a registration name and in an unrelated error.
🤖 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 2076 - 2078, Update the callbacks_cleared calculation in the rollback/teardown flow to use structured status from the known ATIF teardown path rather than checking deregistration error text for ATIF_RUNTIME_DELIVERY_FAILURE_MARKER. Preserve failed-removal status for non-ATIF registrations, and add coverage where the marker appears in a registration name or unrelated error to ensure the mutation lease is not released unless callbacks are actually cleared.
192-214: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake
runtime_diagnosticsserialization match the Python contract.
ConfigReportserializesruntime_diagnosticsoptionally, butpython/nemo_relay/plugin.pydeclares it as a required key.report()only casts native JSON. A normal report with no runtime failures can omit this key, soreport["runtime_diagnostics"]can fail.Always serialize an empty list, or make the field optional in every binding. Add a cross-binding test for an empty runtime-diagnostic list.
🤖 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 192 - 214, Update ConfigReport.runtime_diagnostics to always serialize, removing the empty-vector omission so report() exposes an empty list when no failures occurred. Preserve the existing RuntimeDiagnostic representation and add a cross-binding test verifying an empty runtime-diagnostic list is present in the Python report contract.
🤖 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.
Outside diff comments:
In `@crates/core/src/plugin.rs`:
- Around line 2076-2078: Update the callbacks_cleared calculation in the
rollback/teardown flow to use structured status from the known ATIF teardown
path rather than checking deregistration error text for
ATIF_RUNTIME_DELIVERY_FAILURE_MARKER. Preserve failed-removal status for
non-ATIF registrations, and add coverage where the marker appears in a
registration name or unrelated error to ensure the mutation lease is not
released unless callbacks are actually cleared.
- Around line 192-214: Update ConfigReport.runtime_diagnostics to always
serialize, removing the empty-vector omission so report() exposes an empty list
when no failures occurred. Preserve the existing RuntimeDiagnostic
representation and add a cross-binding test verifying an empty
runtime-diagnostic list is present in the Python report contract.
In `@docs/configure-plugins/observability/atif.mdx`:
- Around line 114-119: Update the ATIF documentation around the remote-write
fallback and the corresponding sections near the referenced recovery guidance to
state that local recovery is attempted only when output_directory is configured;
when it is absent, no local recovery copy is created and write_atif may return a
“no output path” error. Also document that failures writing the configured
recovery copy remain runtime diagnostics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: c82f513e-21bb-486a-964b-22796bcdef49
📒 Files selected for processing (8)
crates/core/src/plugin.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/plugin_tests.rscrates/node/src/api/mod.rsdocs/configure-plugins/observability/atif.mdxgo/nemo_relay/plugin.gopython/nemo_relay/_native.pyipython/nemo_relay/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 (33)
{crates/python/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update the language-native bindings for every exposed surface in Python, Go, and Node.js.
Files:
python/nemo_relay/_native.pyicrates/node/src/api/mod.rsgo/nemo_relay/plugin.gopython/nemo_relay/plugin.py
{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update language wrapper helpers such as Python wrapper modules, Python type stubs, and Go shorthand packages when the new behavior belongs in those helper layers.
Files:
python/nemo_relay/_native.pyigo/nemo_relay/plugin.gopython/nemo_relay/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.
Files:
python/nemo_relay/_native.pyicrates/node/src/api/mod.rscrates/core/tests/unit/plugin_tests.rsgo/nemo_relay/plugin.gopython/nemo_relay/plugin.pydocs/configure-plugins/observability/atif.mdxcrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rs
python/nemo_relay/**/*
⚙️ CodeRabbit configuration file
python/nemo_relay/**/*: Review Python wrapper changes for typed API consistency, contextvars-based scope isolation, async behavior, and parity with the native extension.
Stubs and runtime implementations should stay aligned.
Files:
python/nemo_relay/_native.pyipython/nemo_relay/plugin.py
**/*.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 workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.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.
Files:
crates/node/src/api/mod.rscrates/core/tests/unit/plugin_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/node/src/api/mod.rscrates/core/tests/unit/plugin_tests.rspython/nemo_relay/plugin.pycrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/node/src/api/mod.rscrates/core/tests/unit/plugin_tests.rspython/nemo_relay/plugin.pycrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/node/src/api/mod.rscrates/core/tests/unit/plugin_tests.rsgo/nemo_relay/plugin.gopython/nemo_relay/plugin.pycrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/node/src/api/mod.rscrates/core/tests/unit/plugin_tests.rsgo/nemo_relay/plugin.gocrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.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/node/src/api/mod.rspython/nemo_relay/plugin.pycrates/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/node/src/api/mod.rscrates/core/tests/unit/plugin_tests.rsgo/nemo_relay/plugin.gopython/nemo_relay/plugin.pycrates/core/tests/unit/observability/plugin_component_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/node/src/api/mod.rscrates/core/tests/unit/plugin_tests.rsgo/nemo_relay/plugin.gopython/nemo_relay/plugin.pycrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/plugin.rs
crates/{python,ffi,node}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node}/**/*: Treat binding changes as public API changes. Check for parity with the other language bindings, FFI ownership/lifetime safety,
callback error propagation, stable type conversion, and consistent async/stream semantics.
Flag changes that update one binding without corresponding tests or documentation for the same surface elsewhere.
Files:
crates/node/src/api/mod.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/plugin_tests.rscrates/core/tests/unit/observability/plugin_component_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 validation
Files:
crates/core/tests/unit/plugin_tests.rscrates/core/tests/unit/observability/plugin_component_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/plugin_tests.rscrates/core/tests/unit/observability/plugin_component_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/plugin_tests.rscrates/core/tests/unit/observability/plugin_component_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/plugin_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rs
go/nemo_relay/**/*.go
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
go/nemo_relay/**/*.go: Format changed Go packages withcd go/nemo_relay && go fmt ./...
Run Go tests withjust test-goto build and test the NeMo Relay Go binding
Usejust build-gowhen you want an explicit build-only pass or need the artifact for other work
Usejust ci=true test-gowhen you need the CI-style coverage and JUnit path
On macOS, setDYLD_LIBRARY_PATHto the../../target/releasedirectory before running the rawgo testcommand directlyUse
PascalCasefor public Go APIs.
Files:
go/nemo_relay/plugin.go
**/*.go
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When changing the experimental Go binding, format Go code with
gofmtand keepgo vet ./...passing.
Files:
go/nemo_relay/plugin.go
go/nemo_relay/**
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep shared plugin helpers in
go/nemo_relayaligned with plugin registration, composition, and lifecycle behavior.
Files:
go/nemo_relay/plugin.go
**/*.{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:
go/nemo_relay/plugin.gopython/nemo_relay/plugin.pydocs/configure-plugins/observability/atif.mdx
go/nemo_relay/**/*
⚙️ CodeRabbit configuration file
go/nemo_relay/**/*: Review Go binding changes for cgo memory ownership, race safety, callback cleanup, idiomatic exported APIs, and parity with Rust/FFI behavior.
Any API change should include focused Go tests and consider race-test behavior.
Files:
go/nemo_relay/plugin.go
python/nemo_relay/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Python wrapper modules live under
python/nemo_relay/, and the native extension is built fromcrates/pythonwithmaturin.
Files:
python/nemo_relay/plugin.py
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: When changing the Python wrapper package, tests, or docs tooling, lint with Ruff (E,F,W,I), format with Ruff formatter (120-character lines, double quotes), and passtytype checking.
Add the SPDX license header to all Python source files using the#comment form.
Files:
python/nemo_relay/plugin.py
python/nemo_relay/{adaptive.py,plugin.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep Python adaptive/plugin wrappers in
python/nemo_relay/adaptive.pyandpython/nemo_relay/plugin.pysynchronized with the shared adaptive/plugin boundary and lifecycle.
Files:
python/nemo_relay/plugin.py
**/*.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)
In MDX files, top-of-file comments must use JSX comment delimiters (
{/*to open and*/}to close); do not use HTML comments for MDX SPDX headers
Files:
docs/configure-plugins/observability/atif.mdx
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Update
README.md,fern/, package READMEs, and binding-support notes when public behavior, package names, examples, or supported bindings change.
**/*.{md,mdx}: Prefer the documented public API, not internal shortcuts
Keep package names, repo references, and build commands current
Keep release-process and release-notes guidance in repo-maintainer docs such asRELEASING.md, not as user-facing docs pages orCHANGELOG.md
Keep stable user-facing wrappers atscripts/root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pagesIf links in documentation change, run
just docs-linkcheck.
Files:
docs/configure-plugins/observability/atif.mdx
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
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
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/**,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 (4)
📚 Learning: 2026-07-28T23:57:11.641Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 570
File: crates/node/src/api/mod.rs:3265-3282
Timestamp: 2026-07-28T23:57:11.641Z
Learning: In the Node.js binding, `flushSubscribers()` is Promise-based/async and must be awaited. Any session-close or teardown path (e.g., the OpenClaw live smoke session-close flow) must await `flushSubscribers()` before continuing to live ATIF export assertions and before teardown, so queued subscriber delivery fully completes and tests/assertions observe the final state.
Applied to files:
crates/node/src/api/mod.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/node/src/api/mod.rscrates/core/tests/unit/plugin_tests.rscrates/core/tests/unit/observability/plugin_component_tests.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
📚 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 (2)
crates/core/tests/unit/observability/plugin_component_tests.rs (1)
2547-2586: 📐 Maintainability & Code QualityRun the required Rust and crates/core validation.
Before merge, run
cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings, andjust test-rust. Because these changes are undercrates/core, also runvalidate-changeand the full Rust, Python, Go, and Node.js validation matrix.As per coding guidelines, “If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.”Source: Coding guidelines
docs/configure-plugins/observability/atif.mdx (1)
95-103: LGTM!
|
/merge |
Overview
Make ATIF metadata-routing and remote delivery failures visible and recoverable.
Details
Where should the reviewer start?
Start with
AtifDispatcherincrates/core/src/observability/plugin_component.rs, then the focused ATIF dispatcher tests.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
New Features
Bug Fixes
Documentation