perf(observability): queue managed observability sanitizers - #750
Conversation
github-actions
Bot
added
size:L
|
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:
WalkthroughLLM and tool lifecycle sanitization now uses queued transformed publication. Stream finalization uses the shared publication executor. Tests and documentation define asynchronous execution, ordering, subscriber flushing, and timestamped stream END events. ChangesAsynchronous publication
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ManagedExecution
participant LifecycleAPI
participant PublicationExecutor
participant Sanitizers
participant Subscribers
ManagedExecution->>LifecycleAPI: queue provisional start or end event
LifecycleAPI->>PublicationExecutor: dispatch transformed event
PublicationExecutor->>Sanitizers: sanitize payload and finalize event
Sanitizers->>Subscribers: publish lifecycle event
ManagedExecution->>ManagedExecution: complete without awaiting publication
sequenceDiagram
participant Stream
participant PublicationExecutor
participant EndEventBuilder
participant Subscribers
Stream->>PublicationExecutor: schedule finalization
PublicationExecutor->>EndEventBuilder: construct timestamped END event
EndEventBuilder->>Subscribers: publish after response sanitization
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
9c2c348 to
a1af795
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
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/stream.rs (1)
244-254: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the now-unreachable finalization plumbing.
emit_end_eventalways returnsNoneand ignores_background_thread. Three consequences follow:
self.finalizationis alwaysNone, so thepoll_nextbranch at Lines 443-460 is unreachable, including the"stream finalization task failed"error path.- The
close()await at Lines 508-512 never runs, soclose()no longer waits for END publication. The doc comment at Line 67 already states this, but the code still implies otherwise.- The unused
_background_threadparameter and theOption<tokio::task::JoinHandle<()>>return type carry no information.Delete the parameter, change the return type to
(), drop thefinalizationfield with itspoll_nextandclose()handling, or keep the field and document why it is retained. The doc comment at Lines 242-243 also still claims this function "runs sanitize response guardrails"; the guardrails now run on the queued path.Also applies to: 368-373
🤖 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/stream.rs` around lines 244 - 254, Remove the obsolete finalization plumbing around Stream::emit_end_event: drop the unused _background_thread parameter and change the method to return unit, then update its callers. Remove self.finalization and the unreachable poll_next join-handle error branch and close() await, while preserving END publication behavior; also update the emit_end_event documentation to remove the outdated sanitize-response guardrails claim.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/core/src/api/llm.rs`:
- Around line 1298-1360: The failed-execution END flow must preserve one
timestamp through queued event rebuilding. In crates/core/src/api/llm.rs lines
1298-1360, capture Utc::now() before queue_llm_end_event and pass it via
EndLlmHandleParams::timestamp both when building the provisional event and
inside the transform calling end_llm_handle, matching llm_call_end_with_behavior
and the streaming path. In crates/core/tests/integration/middleware_tests.rs
lines 4526-4612, add coverage for a failing managed llm_call_execute that blocks
response sanitization, records the terminal instant, and verifies the published
END event timestamp is not later than that instant.
- Around line 489-521: Narrow the scope-stack write-lock section in
queue_llm_start_with_subscribers: collect scope registries and read global state
without holding the scope-stack writer lock, then acquire the write lock only
around take_agent_freshness(handle.parent_uuid). Preserve the existing entries,
agent freshness, and full_payloads_enabled results while avoiding nested
global-lock acquisition under the writer.
In `@crates/core/src/api/tool.rs`:
- Around line 392-393: Reuse a single current_scope_stack() result within both
tool_call_with_subscriber_snapshot and tool_call_end_with_pending_marks: hoist
the capture before the registry/subscriber or pending-mark snapshots, then pass
that same scope_stack to snapshot_event_sanitizers and the dispatch target
instead of calling current_scope_stack() again.
In `@crates/core/tests/integration/middleware_tests.rs`:
- Around line 4380-4467: Update
test_managed_llm_payload_sanitizers_are_queued_off_execution_path to register a
recording subscriber instead of the no-op managed_queued_llm_observer. After
flush completes, inspect the recorded START and END events and assert that they
contain the sanitized request input and sanitized response data, respectively,
while preserving the existing queueing and cleanup assertions.
In `@docs/about-nemo-relay/architecture.mdx`:
- Around line 118-124: Update the prose reference to flush_subscribers in the
architecture documentation to omit the empty parentheses, keeping the
surrounding explanation unchanged.
---
Outside diff comments:
In `@crates/core/src/stream.rs`:
- Around line 244-254: Remove the obsolete finalization plumbing around
Stream::emit_end_event: drop the unused _background_thread parameter and change
the method to return unit, then update its callers. Remove self.finalization and
the unreachable poll_next join-handle error branch and close() await, while
preserving END publication behavior; also update the emit_end_event
documentation to remove the outdated sanitize-response guardrails claim.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: ef291bbf-8fa6-4c5c-878d-5855daf24ef6
📒 Files selected for processing (6)
crates/core/src/api/llm.rscrates/core/src/api/tool.rscrates/core/src/stream.rscrates/core/tests/integration/middleware_tests.rsdocs/about-nemo-relay/architecture.mdxdocs/about-nemo-relay/concepts/middleware.mdx
📜 Review details
⏰ Context from checks skipped due to timeout. (44)
- GitHub Check: Node.js / Package (linux-amd64)
- GitHub Check: Python / Package (linux-amd64)
- GitHub Check: Python / Package (windows-arm64)
- GitHub Check: Python / Package (linux-musl-arm64)
- GitHub Check: Rust / Package (linux-amd64)
- GitHub Check: Python / Package (windows-amd64)
- GitHub Check: Python / Test (windows-arm64)
- GitHub Check: Node.js / Package (linux-arm64)
- GitHub Check: Rust / Package (macos-arm64)
- GitHub Check: Python / Package (linux-arm64)
- GitHub Check: Rust / Package (windows-arm64)
- GitHub Check: Python / Package (linux-musl-amd64)
- GitHub Check: Node.js / Package (linux-musl-arm64)
- GitHub Check: Rust / Package (linux-musl-amd64)
- GitHub Check: Rust / Package (linux-arm64)
- GitHub Check: Python / Package (macos-arm64)
- GitHub Check: Rust / Package (linux-musl-arm64)
- GitHub Check: Node.js / Package (windows-amd64)
- GitHub Check: Node.js / Package (windows-arm64)
- GitHub Check: Python / Test (linux-amd64)
- GitHub Check: Rust / Package (windows-amd64)
- GitHub Check: Node.js / Package (macos-arm64)
- GitHub Check: Node.js / Package (linux-musl-amd64)
- GitHub Check: Python / Test (windows-amd64)
- GitHub Check: Python / Test (linux-arm64)
- GitHub Check: Node.js / Test (linux-amd64)
- GitHub Check: Node.js / Test (linux-arm64)
- GitHub Check: Python / Test (macos-arm64)
- GitHub Check: Node.js / Test (windows-amd64)
- GitHub Check: Node.js / Test (macos-arm64)
- GitHub Check: Node.js / Test (windows-arm64)
- GitHub Check: Go / Test (linux-amd64)
- GitHub Check: Go / Test (windows-amd64)
- GitHub Check: Go / Test (linux-arm64)
- GitHub Check: Go / Test (macos-arm64)
- GitHub Check: Go / Test (windows-arm64)
- GitHub Check: Rust / Test (windows-arm64)
- GitHub Check: Check / Run
- GitHub Check: Rust / Test (windows-amd64)
- GitHub Check: Rust / Test (linux-arm64)
- GitHub Check: Rust / Test (macos-arm64)
- GitHub Check: Rust / Test (linux-amd64)
- GitHub Check: Node.js / Package OpenClaw plugin
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (31)
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
**/*.mdx: In MDX files, top-of-file comments must use JSX comment delimiters ({/*and*/}); do not use HTML comments for MDX SPDX headers.
New or regenerated MDX files must use{/* ... */}for top-of-file SPDX comments.
Files:
docs/about-nemo-relay/concepts/middleware.mdxdocs/about-nemo-relay/architecture.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/about-nemo-relay/concepts/middleware.mdxdocs/about-nemo-relay/architecture.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.
**/*: Use release tags in raw Rust-compatible SemVer without a leadingv; tags such asv0.1.0are prohibited.
Use branch prefixesfeat/,fix/,docs/,test/, orrefactor/according to the change purpose.
Every commit in a pull request must include a DCOSigned-off-by:sign-off.
Before submitting a pull request, ensure pre-commit hooks, relevant tests, target-specific builds, documentation updates, and a rebase on the latestmainare complete.
Use commit messages in the formtype: short description, with a valid type and a first line under 72 characters.Run the prescribed plugin validation commands, including fixture building, focused Rust and Python package tests, integration tests, documentation checks, and the broader
validate-changematrix for broad runtime or public API changes.
**/*: Keep observability changes scoped, surface assumptions, and define focused validation before editing.
Run affected Rust tests and just test-rust for event-field changes; run just test-python, just test-go, and just test-node when binding-native configuration or lifecycle changes; update docs and examples in the same branch.
Files:
docs/about-nemo-relay/concepts/middleware.mdxdocs/about-nemo-relay/architecture.mdxcrates/core/src/api/tool.rscrates/core/src/stream.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
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/about-nemo-relay/concepts/middleware.mdxdocs/about-nemo-relay/architecture.mdx
**/*.{md,mdx}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If links in documentation change, run
just docs-linkcheck.Use documented public APIs and stable wrapper commands in examples and user-facing documentation; do not rely on internal helpers.
**/*.{md,mdx}: Prefer the documented public API over internal shortcuts in documentation and examples.
Keep package names, repository references, and build commands current.
Contribution workflow documentation must require an issue before external contribution pull requests and note that NVIDIA contributors may use a GitHub or Linear issue.
Update entry-point documentation when examples or reading paths change.
Keep release-process and release-notes guidance in maintainer documentation such asRELEASING.md, rather than user-facing documentation pages orCHANGELOG.md.
Use stable user-facing wrappers at thescripts/root in documentation and examples; reference namespaced helper paths only for internal maintenance documentation.
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pages.
Dynamic plugin manifests in documentation and examples should usecompat.relay = ">=0.5,<1.0"unless deliberately narrower.
Render images, diagrams, tables, and other visual content at representative page widths, ensuring legibility and complete access without clipping; use responsive scaling, reflow, or overflow as appropriate and scope visual styling narrowly.
Dynamic plugin entry pages should link to native, worker, Rust example, Python example, and protocol pages when those pages exist.
Images, diagrams, tables, and custom visual content must remain legible and fully accessible at representative desktop and narrow page widths.
Release-policy documentation must point to GitHub Releases as the only release-history source of truth.
Runjust docswhen the documentation site changes; retain./scripts/build-docs.sh htmlas the compatibility wrapper.
Files:
docs/about-nemo-relay/concepts/middleware.mdxdocs/about-nemo-relay/architecture.mdx
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant reference documentation when public behavior or APIs change.
Files:
docs/about-nemo-relay/concepts/middleware.mdxdocs/about-nemo-relay/architecture.mdx
**/*.{rs,py,go,js,jsx,ts,tsx,c,h,html,md,mdx,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include the appropriate SPDX copyright and Apache-2.0 license header in every source file.
Files:
docs/about-nemo-relay/concepts/middleware.mdxdocs/about-nemo-relay/architecture.mdxcrates/core/src/api/tool.rscrates/core/src/stream.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.{md,mdx,rst}: Use title case consistently for technical documentation headings and table headers; avoid quotation marks, ampersands, and exclamation marks in headings, while preserving official product, event, research, and whitepaper title case.
Format code elements, commands, parameters, package names, expressions, directories, file names, and paths in monospace; represent path placeholders with angle brackets inside monospace.
Format UI buttons, menus, fields, and labels in bold, and separate consecutive UI navigation labels with>.
Use quotation marks for error messages and strings when appropriate, italics for newly introduced terms and publication titles, and plain text for keyboard shortcuts.
Represent GitHub repositories with owner/repository link text, such as[NVIDIA/NeMo](link), rather than generic repository wording.
Introduce every code block with a complete sentence; do not let a code block complete or interrupt the grammar of surrounding prose; use syntax highlighting when supported.
Keep inline method, function, and class references consistent with nearby documentation; omit empty parentheses in prose when no call is shown.
Use descriptive link text matching the destination title when possible; avoid raw URLs, generic anchors, long-sentence links, and unnecessary links that distract from procedures.
Ensure lists have a complete lead-in sentence, more than one item, no more than two levels, parallel construction, one idea or action per item, and appropriate punctuation; use bullets for unordered items and numbers for ordered tasks.
Format definition lists with a bold term followed by a complete, parallel, punctuated definition.
Use tables for reference information, decision support, compatibility matrices, and comparable choices; flag one-row tables, missing captions or lead-ins, sentence-case headers where title case is expected, unexplained empty cells, and code or links that would be clearer as prose.
Write procedure steps as imperative ...
Files:
docs/about-nemo-relay/concepts/middleware.mdxdocs/about-nemo-relay/architecture.mdx
docs/**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
For documentation-only changes, prefer
contribute-docsplus targeted command checks.
Files:
docs/about-nemo-relay/concepts/middleware.mdxdocs/about-nemo-relay/architecture.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/about-nemo-relay/concepts/middleware.mdxdocs/about-nemo-relay/architecture.mdx
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node work
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
**/*.rs: Format Rust code with rustfmt defaults usingcargo fmt.
Runcargo clippy -- -D warnings; all Rust warnings must be treated as errors.
Use Rustsnake_casenaming conventions.
Files:
crates/core/src/api/tool.rscrates/core/src/stream.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.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/api/tool.rscrates/core/src/stream.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validationUse
Result<T>withFlowErrorin core runtime paths, keeping wrapper-layer errors explicit and binding-appropriate.
Files:
crates/core/src/api/tool.rscrates/core/src/stream.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
crates/core/src/{api/**/*.rs,api/runtime/**/*.rs,codec/**/*.rs,json.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Implement the new or changed public runtime behavior first in the Rust core, especially under
crates/core/src/api/and related core modules such ascrates/core/src/api/runtime/,crates/core/src/codec/, andcrates/core/src/json.rs.
Files:
crates/core/src/api/tool.rscrates/core/src/api/llm.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.For shared-semantics or broad runtime changes in the core or adaptive crates, run
just ci=true test-rust.
Files:
crates/core/src/api/tool.rscrates/core/src/stream.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.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}: Keep FFI and Python, Go, and Node.js binding configuration objects and subscriber/exporter methods aligned with the core observability configuration and lifecycle semantics.
Preserve complete sanitized LLM request input and annotations when enable_full_payloads is enabled, while retaining credential removal and sanitizers.
Use each exporter's documented flush and deregister order before shutdown in observability examples and implementations.
Files:
crates/core/src/api/tool.rscrates/core/src/stream.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
crates/core/src/api/{tool,llm,shared,scope}.rs
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Wire the new middleware chain into the appropriate lifecycle owner and pipeline stage: tool and LLM execution paths use
tool.rsorllm.rs; shared mark and scope event sanitization usesshared.rsand is called fromscope.rs.
Files:
crates/core/src/api/tool.rscrates/core/src/api/llm.rs
crates/core/src/api/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Preserve the documented pipeline order: conditional guardrails, request intercepts, request sanitization, execution intercepts, and response sanitization for tool and LLM execution; specialized sanitization, event creation, and dispatch for mark and scope events.
Files:
crates/core/src/api/tool.rscrates/core/src/api/llm.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/src/api/tool.rscrates/core/src/stream.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
**/*.{rs,py,js,jsx,ts,tsx,go,c,h,cc,cpp,md,toml,yml,yaml,sh}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, documentation, scripts, and configuration files; the project is Apache-2.0.
Files:
crates/core/src/api/tool.rscrates/core/src/stream.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Use
snake_casenaming in Rust and Python.
Files:
crates/core/src/api/tool.rscrates/core/src/stream.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: UseJson = serde_json::Valuein Rust-facing runtime APIs where existing code expects JSON payloads.
Treat Rust as the source of truth for runtime behavior; binding APIs should mirror Rust semantics unless a language-specific wrapper intentionally improves ergonomics.
crates/**/*.rs: Runtime helpers must cover marks, scopes, continuations, and isolated scope stacks.
plugins list,plugins inspect, andplugins validatemust report lifecycle and compatibility status without leaking secret configuration.
Top-leveldoctormust report resolved dynamic-plugin and host-configuration status.
Files:
crates/core/src/api/tool.rscrates/core/src/stream.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
**/*.{rs,py,js,mjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Preserve the existing Tokio-based asynchronous model and callback/future lifetimes; do not unexpectedly block or hide async work in bindings.
Files:
crates/core/src/api/tool.rscrates/core/src/stream.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}: Run tests for every language affected by a change; changes to the core Rust crate require tests across all bindings.
UseSONAR_IGNORE_START/SONAR_IGNORE_ENDonly for documented false positives, keep ignored blocks minimal, explain them with a comment, and obtain reviewer sign-off.
Preserve the layered architecture in which Rust provides the core runtime and C FFI, PyO3, and NAPI provide bindings that mirror the full API surface.
Files:
crates/core/src/api/tool.rscrates/core/src/stream.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
**/*.{toml,md,rs,py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Treat plugin Relay compatibility as normal SemVer; use
>=0.5,<1.0in examples unless a plugin intentionally declares a narrower range.
Files:
crates/core/src/api/tool.rscrates/core/src/stream.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
crates/**/src/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees.
Files:
crates/core/src/api/tool.rscrates/core/src/stream.rscrates/core/src/api/llm.rs
crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/test-rust-core/SKILL.md)
For changes in the Rust core, adaptive, dynamic plugin, worker, worker-proto, or types crates, run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warningsas the default validation sequence.
Files:
crates/core/src/api/tool.rscrates/core/src/stream.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*
📄 CodeRabbit inference engine (.agents/skills/test-rust-core/SKILL.md)
crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*: For changes affectingcrates/core,crates/adaptive, or shared Rust runtime semantics, expand validation to the full binding matrix withvalidate-change.
Use narrower crate-specific tests only as a local debug loop, not as the final validation for a Rust change.
If a public API, event shape, middleware behavior, plugin semantics, orcrates/core/crates/adaptivebehavior changes, also runvalidate-change.
If the change is isolated to one binding wrapper while Rust semantics remain unchanged, prefer that binding's build/test skill instead.
Files:
crates/core/src/api/tool.rscrates/core/src/stream.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.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/api/tool.rscrates/core/src/stream.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
**/{test,tests}/**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the appropriate test files for each affected language binding.
Files:
crates/core/tests/integration/middleware_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/middleware_tests.rs
🧠 Learnings (2)
📚 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/api/tool.rscrates/core/src/stream.rscrates/core/src/api/llm.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/api/tool.rscrates/core/src/stream.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
🔇 Additional comments (10)
crates/core/src/api/llm.rs (2)
522-568: LGTM!
935-945: LGTM!Also applies to: 1171-1229, 1613-1619, 1820-1826
crates/core/src/api/tool.rs (2)
354-354: LGTM!Also applies to: 370-370, 392-412
575-604: LGTM!crates/core/src/stream.rs (2)
16-27: LGTM!Also applies to: 66-68, 441-442
337-347: LGTM!crates/core/tests/integration/middleware_tests.rs (3)
19-19: LGTM!Also applies to: 3523-3523, 3765-3765
3908-3920: LGTM!
4469-4524: LGTM!docs/about-nemo-relay/concepts/middleware.mdx (1)
28-31: LGTM!Also applies to: 43-49, 222-227
github-actions
Bot
added
lang:go
afourniernv
left a comment
afourniernv
left a comment
There was a problem hiding this comment.
The queued observability direction makes sense. I’m requesting changes for two narrower lifecycle regressions that I reproduced against the PR base and head. Both should be fixable without changing the overall design.
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
b18848e to
d52229f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/core/src/api/llm.rs`:
- Around line 1641-1648: Update the managed start-event assertions in
crates/core/src/api/llm.rs:1641-1648 and
crates/core/tests/unit/llm_api_tests.rs:88-98 to expect the lowercase
runtime-owned traceparent injected by inject_traceparent before sanitization for
both buffered and streaming managed calls; retain the caller-provided
traceparent assertion for manual llm_call.
In `@crates/core/tests/integration/middleware_tests.rs`:
- Around line 4544-4599: Strengthen
test_managed_tool_payload_sanitizers_are_queued_off_execution_path by recording
published subscriber events instead of using a no-op callback. After
flush_subscribers(), locate the START event for managed-queued-tool and assert
its payload contains the sanitized arguments, mirroring the sibling LLM test’s
event-recording and assertion pattern.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 05793b76-ff2e-4d48-8291-18997295a9de
📒 Files selected for processing (6)
crates/core/src/api/llm.rscrates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/llm_api_tests.rscrates/node/tests/callback_error_tests.mjscrates/node/tests/llm_tests.mjspython/tests/test_llm.py
📜 Review details
⏰ Context from checks skipped due to timeout. (37)
- GitHub Check: Python / Package (linux-musl-amd64)
- GitHub Check: Node.js / Package (linux-musl-arm64)
- GitHub Check: Python / Package (windows-amd64)
- GitHub Check: Python / Package (windows-arm64)
- GitHub Check: Python / Test (windows-arm64)
- GitHub Check: Node.js / Package (windows-arm64)
- GitHub Check: Node.js / Package (windows-amd64)
- GitHub Check: Python / Test (linux-amd64)
- GitHub Check: Python / Test (linux-arm64)
- GitHub Check: Node.js / Package (linux-musl-amd64)
- GitHub Check: Node.js / Package (linux-arm64)
- GitHub Check: Python / Package (linux-amd64)
- GitHub Check: Python / Test (windows-amd64)
- GitHub Check: Check / Run
- GitHub Check: Python / Package (linux-arm64)
- GitHub Check: Python / Package (linux-musl-arm64)
- GitHub Check: Node.js / Package (linux-amd64)
- GitHub Check: Rust / Test (windows-arm64)
- GitHub Check: Rust / Package (windows-amd64)
- GitHub Check: Rust / Package (windows-arm64)
- GitHub Check: Rust / Package (linux-musl-arm64)
- GitHub Check: Rust / Package (linux-arm64)
- GitHub Check: Node.js / Test (windows-amd64)
- GitHub Check: Node.js / Test (linux-arm64)
- GitHub Check: Node.js / Test (linux-amd64)
- GitHub Check: Node.js / Test (windows-arm64)
- GitHub Check: Rust / Package (linux-amd64)
- GitHub Check: Rust / Package (linux-musl-amd64)
- GitHub Check: Rust / Test (linux-arm64)
- GitHub Check: Go / Test (windows-arm64)
- GitHub Check: Rust / Test (linux-amd64)
- GitHub Check: Go / Test (linux-arm64)
- GitHub Check: Rust / Test (windows-amd64)
- GitHub Check: Go / Test (windows-amd64)
- GitHub Check: Go / Test (linux-amd64)
- GitHub Check: Node.js / Package OpenClaw plugin
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (28)
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
**/*: Use release tags in raw Rust-compatible SemVer without a leadingv; tags such asv0.1.0are prohibited.
Use branch prefixesfeat/,fix/,docs/,test/, orrefactor/according to the change purpose.
Every commit in a pull request must include a DCOSigned-off-by:sign-off.
Before submitting a pull request, ensure pre-commit hooks, relevant tests, target-specific builds, documentation updates, and a rebase on the latestmainare complete.
Use commit messages in the formtype: short description, with a valid type and a first line under 72 characters.Run the prescribed plugin validation commands, including fixture building, focused Rust and Python package tests, integration tests, documentation checks, and the broader
validate-changematrix for broad runtime or public API changes.
**/*: Keep observability changes scoped, surface assumptions, and define focused validation before editing.
Run affected Rust tests and just test-rust for event-field changes; run just test-python, just test-go, and just test-node when binding-native configuration or lifecycle changes; update docs and examples in the same branch.
Files:
crates/node/tests/callback_error_tests.mjscrates/core/tests/unit/llm_api_tests.rspython/tests/test_llm.pycrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rscrates/node/tests/llm_tests.mjs
crates/node/**/*.{js,mjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
camelCasenaming for Node.js APIs.
Files:
crates/node/tests/callback_error_tests.mjscrates/node/tests/llm_tests.mjs
**/*.{rs,py,js,mjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Preserve the existing Tokio-based asynchronous model and callback/future lifetimes; do not unexpectedly block or hide async work in bindings.
Files:
crates/node/tests/callback_error_tests.mjscrates/core/tests/unit/llm_api_tests.rspython/tests/test_llm.pycrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rscrates/node/tests/llm_tests.mjs
**/{test,tests}/**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the appropriate test files for each affected language binding.
Files:
crates/node/tests/callback_error_tests.mjscrates/core/tests/unit/llm_api_tests.rspython/tests/test_llm.pycrates/core/tests/integration/middleware_tests.rscrates/node/tests/llm_tests.mjs
crates/{python,ffi,node}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node}/**/*: Treat binding changes as public API changes. Check for parity with the other language bindings, FFI ownership/lifetime safety,
callback error propagation, stable type conversion, and consistent async/stream semantics.
Flag changes that update one binding without corresponding tests or documentation for the same surface elsewhere.
Files:
crates/node/tests/callback_error_tests.mjscrates/node/tests/llm_tests.mjs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/node/tests/callback_error_tests.mjscrates/core/tests/unit/llm_api_tests.rspython/tests/test_llm.pycrates/core/tests/integration/middleware_tests.rscrates/node/tests/llm_tests.mjs
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node work
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
**/*.rs: Format Rust code with rustfmt defaults usingcargo fmt.
Runcargo clippy -- -D warnings; all Rust warnings must be treated as errors.
Use Rustsnake_casenaming conventions.
Files:
crates/core/tests/unit/llm_api_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.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/llm_api_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validationUse
Result<T>withFlowErrorin core runtime paths, keeping wrapper-layer errors explicit and binding-appropriate.
Files:
crates/core/tests/unit/llm_api_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.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.For shared-semantics or broad runtime changes in the core or adaptive crates, run
just ci=true test-rust.
Files:
crates/core/tests/unit/llm_api_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.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}: Keep FFI and Python, Go, and Node.js binding configuration objects and subscriber/exporter methods aligned with the core observability configuration and lifecycle semantics.
Preserve complete sanitized LLM request input and annotations when enable_full_payloads is enabled, while retaining credential removal and sanitizers.
Use each exporter's documented flush and deregister order before shutdown in observability examples and implementations.
Files:
crates/core/tests/unit/llm_api_tests.rspython/tests/test_llm.pycrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/core/tests/unit/llm_api_tests.rspython/tests/test_llm.pycrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
**/*.{rs,py,js,jsx,ts,tsx,go,c,h,cc,cpp,md,toml,yml,yaml,sh}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, documentation, scripts, and configuration files; the project is Apache-2.0.
Files:
crates/core/tests/unit/llm_api_tests.rspython/tests/test_llm.pycrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Use
snake_casenaming in Rust and Python.
Files:
crates/core/tests/unit/llm_api_tests.rspython/tests/test_llm.pycrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: UseJson = serde_json::Valuein Rust-facing runtime APIs where existing code expects JSON payloads.
Treat Rust as the source of truth for runtime behavior; binding APIs should mirror Rust semantics unless a language-specific wrapper intentionally improves ergonomics.
crates/**/*.rs: Runtime helpers must cover marks, scopes, continuations, and isolated scope stacks.
plugins list,plugins inspect, andplugins validatemust report lifecycle and compatibility status without leaking secret configuration.
Top-leveldoctormust report resolved dynamic-plugin and host-configuration status.
Files:
crates/core/tests/unit/llm_api_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}: Run tests for every language affected by a change; changes to the core Rust crate require tests across all bindings.
UseSONAR_IGNORE_START/SONAR_IGNORE_ENDonly for documented false positives, keep ignored blocks minimal, explain them with a comment, and obtain reviewer sign-off.
Preserve the layered architecture in which Rust provides the core runtime and C FFI, PyO3, and NAPI provide bindings that mirror the full API surface.
Files:
crates/core/tests/unit/llm_api_tests.rspython/tests/test_llm.pycrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
**/*.{rs,py,go,js,jsx,ts,tsx,c,h,html,md,mdx,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include the appropriate SPDX copyright and Apache-2.0 license header in every source file.
Files:
crates/core/tests/unit/llm_api_tests.rspython/tests/test_llm.pycrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
**/*.{toml,md,rs,py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Treat plugin Relay compatibility as normal SemVer; use
>=0.5,<1.0in examples unless a plugin intentionally declares a narrower range.
Files:
crates/core/tests/unit/llm_api_tests.rspython/tests/test_llm.pycrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/test-rust-core/SKILL.md)
For changes in the Rust core, adaptive, dynamic plugin, worker, worker-proto, or types crates, run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warningsas the default validation sequence.
Files:
crates/core/tests/unit/llm_api_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*
📄 CodeRabbit inference engine (.agents/skills/test-rust-core/SKILL.md)
crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*: For changes affectingcrates/core,crates/adaptive, or shared Rust runtime semantics, expand validation to the full binding matrix withvalidate-change.
Use narrower crate-specific tests only as a local debug loop, not as the final validation for a Rust change.
If a public API, event shape, middleware behavior, plugin semantics, orcrates/core/crates/adaptivebehavior changes, also runvalidate-change.
If the change is isolated to one binding wrapper while Rust semantics remain unchanged, prefer that binding's build/test skill instead.
Files:
crates/core/tests/unit/llm_api_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.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/llm_api_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
python/tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain test coverage for Python binding and wrapper changes with the Python test suite.
python/tests/**/*.py: Use pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, useunittest.mock.MagicMockorunittest.mock.AsyncMock, usingspecwhen necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
If a fixture is needed in multiple test files, define it in aconftest.pyfile instead of repeating it.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a<fixture_name>_fixturefunction; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Run focused pytest tests first when the affected area is known, and run the full suite withjust test-pythonbefore review.
Files:
python/tests/test_llm.py
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: Lint Python with Ruff using rule setsE,F,W, andI.
Format Python with the Ruff formatter, using a 120-character line length and double quotes.
Runtyfor Python type checking.
Use Pythonsnake_casenaming conventions.
Files:
python/tests/test_llm.py
python/**/*.py
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Python SDK tests belong under
python/tests, not under source directories.
python/**/*.py: Format changed Python wrapper and test files withuv run ruff format python python/plugin.
Runuv run ruff format python python/pluginafter changing Python wrapper or test files.
Files:
python/tests/test_llm.py
crates/core/src/{api/**/*.rs,api/runtime/**/*.rs,codec/**/*.rs,json.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Implement the new or changed public runtime behavior first in the Rust core, especially under
crates/core/src/api/and related core modules such ascrates/core/src/api/runtime/,crates/core/src/codec/, andcrates/core/src/json.rs.
Files:
crates/core/src/api/llm.rs
crates/core/src/api/{tool,llm,shared,scope}.rs
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Wire the new middleware chain into the appropriate lifecycle owner and pipeline stage: tool and LLM execution paths use
tool.rsorllm.rs; shared mark and scope event sanitization usesshared.rsand is called fromscope.rs.
Files:
crates/core/src/api/llm.rs
crates/core/src/api/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Preserve the documented pipeline order: conditional guardrails, request intercepts, request sanitization, execution intercepts, and response sanitization for tool and LLM execution; specialized sanitization, event creation, and dispatch for mark and scope events.
Files:
crates/core/src/api/llm.rs
crates/**/src/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees.
Files:
crates/core/src/api/llm.rs
🧠 Learnings (2)
📚 Learning: 2026-08-03T19:55:03.931Z
Learnt from: afourniernv
Repo: NVIDIA/NeMo-Relay PR: 558
File: crates/pii-redaction/src/rampart/mod.rs:265-274
Timestamp: 2026-08-03T19:55:03.931Z
Learning: In NeMo Relay first-party plugin registration helpers, treat the documented duplicate-registration `PluginError::RegistrationFailed` result from `register_plugin` as success when registration is intended to be idempotent. Do not locally reclassify this as `PluginError::Conflict`; changing the classification requires a core-wide review of the public API and FFI behavior.
Applied to files:
crates/core/tests/unit/llm_api_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
📚 Learning: 2026-07-28T20:07:29.880Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 571
File: crates/core/src/api/runtime/state.rs:996-1020
Timestamp: 2026-07-28T20:07:29.880Z
Learning: In NeMo Relay (RELAY-509), sanitizer callback failures must be treated as intentional fail-open behavior. When an event/tool (request/response) or LLM (request/response) sanitizer callback fails, the sanitizer chain should retain and publish the last valid event/payload snapshot (rather than dropping/invalidating the data) and log the failure including callback context (e.g., which sanitizer/callback failed and relevant identifiers). Apply this consistently across all sanitizer chains mentioned in the RELAY-509 documentation/migration guide.
Applied to files:
crates/core/src/api/llm.rs
🪛 Ruff (0.16.1)
python/tests/test_llm.py
[warning] 619-619: Missing return type annotation for private function execution_intercept
(ANN202)
[warning] 623-623: Missing return type annotation for private function provider
(ANN202)
[warning] 646-646: Missing return type annotation for private function execution_intercept
(ANN202)
[warning] 650-650: Missing return type annotation for private function provider
(ANN202)
🔇 Additional comments (13)
crates/core/src/api/llm.rs (2)
494-583: LGTM!
1166-1166: LGTM!Also applies to: 1187-1245, 1295-1295, 1315-1386
crates/core/tests/integration/middleware_tests.rs (4)
19-19: LGTM!Also applies to: 173-189, 3447-3459, 3493-3505, 3527-3527, 3662-3674, 3731-3743, 3808-3808
3558-3595: LGTM!
3951-3963: LGTM!
4423-4542: LGTM!Also applies to: 4601-4687
crates/core/tests/unit/llm_api_tests.rs (2)
330-436: LGTM!
1290-1290: LGTM!crates/node/tests/callback_error_tests.mjs (1)
20-20: LGTM!Also applies to: 55-58
crates/node/tests/llm_tests.mjs (2)
832-834: LGTM!Also applies to: 1093-1173
403-403: 🩺 Stability & AvailabilityKeep the direct
flushSubscribers()calls. It waits for managed terminal publications, andPromiseAwareFnresolves after JS promise fulfillment. Lines 403 and 508 use synchronous callbacks; line 619 is covered by the flush barrier.> Likely an incorrect or invalid review comment.python/tests/test_llm.py (2)
21-32: LGTM!Also applies to: 309-309
614-665: LGTM!
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 (4)
crates/core/src/api/llm.rs (4)
494-584: 📐 Maintainability & Code Quality | 🔵 TrivialRun the required full validation matrix before handoff.
This changes
crates/coreRust lifecycle and observability semantics. Runcargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,just test-rust,just ci=true test-rust,validate-change,just test-python,just test-go,just test-node, anduv run pre-commit run --all-files.As per coding guidelines, core Rust changes require strict Rust validation, binding tests, broader change validation, and the final pre-commit pass.
🤖 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/api/llm.rs` around lines 494 - 584, Run the required validation matrix for the queue_llm_start_with_subscribers lifecycle and observability changes: cargo fmt --all, cargo clippy --workspace --all-targets -- -D warnings, just test-rust, just ci=true test-rust, validate-change, just test-python, just test-go, just test-node, and uv run pre-commit run --all-files.Source: Coding guidelines
494-584: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve the last valid payload when a sanitizer callback fails.
The queued request and response sanitizer results flow directly into event construction. A failed chain can produce
input = Noneordata = None, which removes the payload and related annotations from the published event.Apply the fail-open sanitizer contract to the queued start, normal END, and no-output END paths. Retain the last valid snapshot, stop the remaining callbacks, and log the failed callback with its context.
Based on learnings, sanitizer callback failures must retain and publish the last valid event or payload snapshot and log callback context.
Also applies to: 1187-1245, 1314-1386
🤖 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/api/llm.rs` around lines 494 - 584, Update queue_llm_start_with_subscribers and the normal and no-output END event flows to apply fail-open sanitizer handling: preserve the last valid request, response, or event payload and related annotations when a callback fails, stop executing remaining callbacks, and publish that snapshot instead of converting it to None. Log each failed sanitizer callback with sufficient event/path context, while preserving successful chained sanitization behavior.Source: Learnings
494-584: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRestore agent freshness when dispatch fails.
take_agent_freshnessruns beforedispatch_transformed_event, whosefalseresult is ignored. A scope-snapshot or queue failure can consume freshness without publishing the START event. Restore freshness when dispatch returnsfalse, or consume it only after successful dispatch.🤖 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/api/llm.rs` around lines 494 - 584, Update queue_llm_start_with_subscribers to handle the boolean result from dispatch_transformed_event: if dispatch returns false, restore the agent freshness for handle.parent_uuid through the corresponding scope-stack API. Preserve the current freshness consumption behavior when dispatch succeeds, and ensure failures do not permanently consume freshness.
423-423: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftTest the queued LLM start transformation.
emit_llm_start_with_subscribersduplicates request sanitization, annotation decoding, freshness handling, projection, and event construction fromqueue_llm_start_with_subscribers. The current test covers only the duplicate path. Test the queued path withflush_subscribers(), or extract a shared transformation helper.🤖 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/api/llm.rs` at line 423, Add coverage for the queued transformation implemented by queue_llm_start_with_subscribers, ensuring the test invokes flush_subscribers() and validates the resulting sanitized, decoded, freshness-aware, projected LLM start event. Prefer reusing a shared transformation helper with emit_llm_start_with_subscribers if practical, so both paths exercise the same logic.
🤖 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/api/llm.rs`:
- Around line 494-584: Run the required validation matrix for the
queue_llm_start_with_subscribers lifecycle and observability changes: cargo fmt
--all, cargo clippy --workspace --all-targets -- -D warnings, just test-rust,
just ci=true test-rust, validate-change, just test-python, just test-go, just
test-node, and uv run pre-commit run --all-files.
- Around line 494-584: Update queue_llm_start_with_subscribers and the normal
and no-output END event flows to apply fail-open sanitizer handling: preserve
the last valid request, response, or event payload and related annotations when
a callback fails, stop executing remaining callbacks, and publish that snapshot
instead of converting it to None. Log each failed sanitizer callback with
sufficient event/path context, while preserving successful chained sanitization
behavior.
- Around line 494-584: Update queue_llm_start_with_subscribers to handle the
boolean result from dispatch_transformed_event: if dispatch returns false,
restore the agent freshness for handle.parent_uuid through the corresponding
scope-stack API. Preserve the current freshness consumption behavior when
dispatch succeeds, and ensure failures do not permanently consume freshness.
- Line 423: Add coverage for the queued transformation implemented by
queue_llm_start_with_subscribers, ensuring the test invokes flush_subscribers()
and validates the resulting sanitized, decoded, freshness-aware, projected LLM
start event. Prefer reusing a shared transformation helper with
emit_llm_start_with_subscribers if practical, so both paths exercise the same
logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: b0d12d12-ea16-4e92-939e-1485a585c38c
📒 Files selected for processing (1)
crates/core/src/api/llm.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (44)
- GitHub Check: Python / Package (linux-musl-arm64)
- GitHub Check: Python / Package (linux-amd64)
- GitHub Check: Rust / Package (windows-amd64)
- GitHub Check: Python / Package (macos-arm64)
- GitHub Check: Python / Package (windows-arm64)
- GitHub Check: Python / Test (windows-arm64)
- GitHub Check: Python / Package (windows-amd64)
- GitHub Check: Python / Package (linux-arm64)
- GitHub Check: Python / Test (linux-amd64)
- GitHub Check: Python / Package (linux-musl-amd64)
- GitHub Check: Python / Test (macos-arm64)
- GitHub Check: Node.js / Package (windows-arm64)
- GitHub Check: Go / Test (macos-arm64)
- GitHub Check: Python / Test (windows-amd64)
- GitHub Check: Go / Test (linux-amd64)
- GitHub Check: Rust / Package (windows-arm64)
- GitHub Check: Python / Test (linux-arm64)
- GitHub Check: Rust / Package (linux-musl-amd64)
- GitHub Check: Rust / Test (macos-arm64)
- GitHub Check: Rust / Package (linux-arm64)
- GitHub Check: Go / Test (windows-amd64)
- GitHub Check: Node.js / Package (windows-amd64)
- GitHub Check: Go / Test (windows-arm64)
- GitHub Check: Rust / Test (linux-arm64)
- GitHub Check: Rust / Package (macos-arm64)
- GitHub Check: Go / Test (linux-arm64)
- GitHub Check: Rust / Package (linux-amd64)
- GitHub Check: Node.js / Package (linux-musl-amd64)
- GitHub Check: Rust / Package (linux-musl-arm64)
- GitHub Check: Node.js / Package (linux-arm64)
- GitHub Check: Node.js / Package (linux-amd64)
- GitHub Check: Node.js / Package (macos-arm64)
- GitHub Check: Node.js / Package (linux-musl-arm64)
- GitHub Check: Node.js / Test (linux-arm64)
- GitHub Check: Rust / Test (windows-arm64)
- GitHub Check: Rust / Test (windows-amd64)
- GitHub Check: Rust / Test (linux-amd64)
- GitHub Check: Node.js / Test (windows-arm64)
- GitHub Check: Node.js / Test (macos-arm64)
- GitHub Check: Node.js / Test (linux-amd64)
- GitHub Check: Node.js / Test (windows-amd64)
- GitHub Check: Check / Run
- GitHub Check: Node.js / Package OpenClaw plugin
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (21)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node work
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
**/*.rs: Format Rust code with rustfmt defaults usingcargo fmt.
Runcargo clippy -- -D warnings; all Rust warnings must be treated as errors.
Use Rustsnake_casenaming conventions.
Files:
crates/core/src/api/llm.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/api/llm.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validationUse
Result<T>withFlowErrorin core runtime paths, keeping wrapper-layer errors explicit and binding-appropriate.
Files:
crates/core/src/api/llm.rs
crates/core/src/{api/**/*.rs,api/runtime/**/*.rs,codec/**/*.rs,json.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Implement the new or changed public runtime behavior first in the Rust core, especially under
crates/core/src/api/and related core modules such ascrates/core/src/api/runtime/,crates/core/src/codec/, andcrates/core/src/json.rs.
Files:
crates/core/src/api/llm.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
**/*: Use release tags in raw Rust-compatible SemVer without a leadingv; tags such asv0.1.0are prohibited.
Use branch prefixesfeat/,fix/,docs/,test/, orrefactor/according to the change purpose.
Every commit in a pull request must include a DCOSigned-off-by:sign-off.
Before submitting a pull request, ensure pre-commit hooks, relevant tests, target-specific builds, documentation updates, and a rebase on the latestmainare complete.
Use commit messages in the formtype: short description, with a valid type and a first line under 72 characters.Run the prescribed plugin validation commands, including fixture building, focused Rust and Python package tests, integration tests, documentation checks, and the broader
validate-changematrix for broad runtime or public API changes.
**/*: Keep observability changes scoped, surface assumptions, and define focused validation before editing.
Run affected Rust tests and just test-rust for event-field changes; run just test-python, just test-go, and just test-node when binding-native configuration or lifecycle changes; update docs and examples in the same branch.
Files:
crates/core/src/api/llm.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.For shared-semantics or broad runtime changes in the core or adaptive crates, run
just ci=true test-rust.
Files:
crates/core/src/api/llm.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}: Keep FFI and Python, Go, and Node.js binding configuration objects and subscriber/exporter methods aligned with the core observability configuration and lifecycle semantics.
Preserve complete sanitized LLM request input and annotations when enable_full_payloads is enabled, while retaining credential removal and sanitizers.
Use each exporter's documented flush and deregister order before shutdown in observability examples and implementations.
Files:
crates/core/src/api/llm.rs
crates/core/src/api/{tool,llm,shared,scope}.rs
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Wire the new middleware chain into the appropriate lifecycle owner and pipeline stage: tool and LLM execution paths use
tool.rsorllm.rs; shared mark and scope event sanitization usesshared.rsand is called fromscope.rs.
Files:
crates/core/src/api/llm.rs
crates/core/src/api/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Preserve the documented pipeline order: conditional guardrails, request intercepts, request sanitization, execution intercepts, and response sanitization for tool and LLM execution; specialized sanitization, event creation, and dispatch for mark and scope events.
Files:
crates/core/src/api/llm.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/src/api/llm.rs
**/*.{rs,py,js,jsx,ts,tsx,go,c,h,cc,cpp,md,toml,yml,yaml,sh}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, documentation, scripts, and configuration files; the project is Apache-2.0.
Files:
crates/core/src/api/llm.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Use
snake_casenaming in Rust and Python.
Files:
crates/core/src/api/llm.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: UseJson = serde_json::Valuein Rust-facing runtime APIs where existing code expects JSON payloads.
Treat Rust as the source of truth for runtime behavior; binding APIs should mirror Rust semantics unless a language-specific wrapper intentionally improves ergonomics.
crates/**/*.rs: Runtime helpers must cover marks, scopes, continuations, and isolated scope stacks.
plugins list,plugins inspect, andplugins validatemust report lifecycle and compatibility status without leaking secret configuration.
Top-leveldoctormust report resolved dynamic-plugin and host-configuration status.
Files:
crates/core/src/api/llm.rs
**/*.{rs,py,js,mjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Preserve the existing Tokio-based asynchronous model and callback/future lifetimes; do not unexpectedly block or hide async work in bindings.
Files:
crates/core/src/api/llm.rs
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}: Run tests for every language affected by a change; changes to the core Rust crate require tests across all bindings.
UseSONAR_IGNORE_START/SONAR_IGNORE_ENDonly for documented false positives, keep ignored blocks minimal, explain them with a comment, and obtain reviewer sign-off.
Preserve the layered architecture in which Rust provides the core runtime and C FFI, PyO3, and NAPI provide bindings that mirror the full API surface.
Files:
crates/core/src/api/llm.rs
**/*.{rs,py,go,js,jsx,ts,tsx,c,h,html,md,mdx,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include the appropriate SPDX copyright and Apache-2.0 license header in every source file.
Files:
crates/core/src/api/llm.rs
**/*.{toml,md,rs,py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Treat plugin Relay compatibility as normal SemVer; use
>=0.5,<1.0in examples unless a plugin intentionally declares a narrower range.
Files:
crates/core/src/api/llm.rs
crates/**/src/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees.
Files:
crates/core/src/api/llm.rs
crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/test-rust-core/SKILL.md)
For changes in the Rust core, adaptive, dynamic plugin, worker, worker-proto, or types crates, run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warningsas the default validation sequence.
Files:
crates/core/src/api/llm.rs
crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*
📄 CodeRabbit inference engine (.agents/skills/test-rust-core/SKILL.md)
crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*: For changes affectingcrates/core,crates/adaptive, or shared Rust runtime semantics, expand validation to the full binding matrix withvalidate-change.
Use narrower crate-specific tests only as a local debug loop, not as the final validation for a Rust change.
If a public API, event shape, middleware behavior, plugin semantics, orcrates/core/crates/adaptivebehavior changes, also runvalidate-change.
If the change is isolated to one binding wrapper while Rust semantics remain unchanged, prefer that binding's build/test skill instead.
Files:
crates/core/src/api/llm.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/api/llm.rs
🧠 Learnings (2)
📚 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/api/llm.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/api/llm.rs
🔇 Additional comments (1)
crates/core/src/api/llm.rs (1)
25-26: LGTM!Also applies to: 940-941, 950-961, 1117-1118, 1166-1166, 1295-1295, 1641-1649, 1694-1695, 1852-1860
willkill07
changed the title
github-actions
Bot
added
Improvement
afourniernv
left a comment
afourniernv
left a comment
There was a problem hiding this comment.
Verified the two previously blocking regressions on current head ca5acf5: stream accounting closes before queued finalization even without an explicit flush, and delayed sanitizer publication preserves the managed call completion timestamp. Both focused tests pass locally, and the full CI matrix is green.
|
/merge |
2 participants
Overview
Queue managed payload and event sanitization so observability work does not delay tool, buffered LLM, or stream execution.
Details
flush_subscribers()completion barrier.enable_full_payloadsbehavior.Where should the reviewer start?
Start with
crates/core/src/api/llm.rsandcrates/core/src/stream.rs;crates/core/tests/integration/middleware_tests.rsdemonstrates the non-blocking and flush-barrier contract.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit