feat(plugin)!: expose typed async middleware in Rust dynamic plugins - #730
feat(plugin)!: expose typed async middleware in Rust dynamic plugins#730afourniernv wants to merge 5 commits into
Conversation
WalkthroughThe Rust plugin SDK adds ABI-v3 typed asynchronous LLM and stream intercepts. It supports cancellation, completion settlement, downstream streaming, and backpressure. Existing synchronous registrations receive explicit ChangesTyped asynchronous middleware
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant RelayRuntime
participant PluginContext
participant NativePlugin
participant HostStream
RelayRuntime->>PluginContext: register typed async intercept
PluginContext->>NativePlugin: invoke typed callback
NativePlugin->>NativePlugin: check cancellation and process request
NativePlugin->>HostStream: push chunks or settle completion
HostStream-->>NativePlugin: report backpressure or cancellation
NativePlugin-->>RelayRuntime: resolve, reject, or terminate stream
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
willkill07
changed the title
willkill07
left a comment
willkill07
left a comment
There was a problem hiding this comment.
we should try to reuse register_llm_execution_intercept and register_llm_stream_execution_intercept
yes, i know it will be a breaking change, but it should be async first.
willkill07
changed the title
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
|
This would be a breaking change, but i agree that we should get it into 0.7.1 because it is a bug. Users with dynamic rust plugins would need to implement them as async. So we should update the PR title to "feat(plugin)!" and add a breaking change warning to the top of the description |
Signed-off-by: Alex Fournier <afournier@nvidia.com>
76153ca to
766a237
Compare
afourniernv
changed the title
|
Addressed the review in 766a237: the existing register_llm_execution_intercept and register_llm_stream_execution_intercept names are now completion-based and async-first. The synchronous typed variants remain available under explicit register_sync_* names. I also retargeted the PR to release/0.7 and added the breaking-change warning and migration notes for 0.7.1. |
|
/ok to test 766a237 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/core/tests/fixtures/native_plugin/src/lib.rs`:
- Around line 819-826: Restructure the code around output and invoke_next so the
AsyncLlmStream handle is extracted from the Mutex before calling invoke_next,
leaving only the terminal-settlement state inside the mutex. Ensure no
MutexGuard remains alive during the host invocation, while preserving callback
access through callback_output for synchronous first-chunk, error, and Done
events.
In `@crates/core/tests/integration/native_plugin_tests.rs`:
- Around line 889-908: Increase the timeout around the unrelated call in the
typed callback responsiveness test to provide more scheduling headroom, and add
a nearby comment documenting its required relationship to the fixture’s 400 ms
slow-branch delay. Keep the existing timeout assertions and call behavior
unchanged.
In `@crates/plugin/src/lib.rs`:
- Around line 3213-3220: Update async_stream_write_status so
NemoRelayStatus::Internal maps to Backpressured only when the existing crate
last-error accessor indicates the error is backpressure (contains the documented
“backpressured” marker); otherwise return an error for the genuine internal
failure. Preserve the existing mappings for Ok, InvalidArg, and other statuses.
In `@docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx`:
- Line 165: Add a complete introductory sentence immediately before the Rust
code fence in the relevant documentation section, explicitly stating what the
example registers. Keep the existing code block unchanged.
- Around line 165-180: Update the example’s nemo-relay-plugin dependency to
version 0.7.1 or another compatible release so it supports the completion-based
register_llm_execution_intercept callback, while leaving the callback
implementation unchanged.
🪄 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: d976e018-57eb-4c5f-9c07-31eefecf1353
📒 Files selected for processing (7)
crates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdxdocs/reference/migration-guides.mdxexamples/rust-native-plugin/src/lib.rs
📜 Review details
🧰 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/reference/migration-guides.mdxdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/reference/migration-guides.mdxexamples/rust-native-plugin/src/lib.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.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.
Files:
docs/reference/migration-guides.mdxexamples/rust-native-plugin/src/lib.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/plugin/src/lib.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/reference/migration-guides.mdxdocs/build-plugins/dynamic-plugins/native-dynamic/about.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/reference/migration-guides.mdxdocs/build-plugins/dynamic-plugins/native-dynamic/about.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/reference/migration-guides.mdxdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant reference documentation when public behavior or APIs change.
Files:
docs/reference/migration-guides.mdxdocs/build-plugins/dynamic-plugins/native-dynamic/about.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/reference/migration-guides.mdxexamples/rust-native-plugin/src/lib.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/plugin/src/lib.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/reference/migration-guides.mdxdocs/build-plugins/dynamic-plugins/native-dynamic/about.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/reference/migration-guides.mdxdocs/build-plugins/dynamic-plugins/native-dynamic/about.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:
examples/rust-native-plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/plugin/src/lib.rs
{crates/core/src/plugin/dynamic/**,crates/plugin/**,crates/worker/**,crates/worker-proto/**,crates/types/**,python/plugin/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Keep the stable boundary explicit: native plugins cross a C ABI, and worker plugins cross
grpc-v1.
Files:
examples/rust-native-plugin/src/lib.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rs
{crates/core/src/plugin/dynamic/**/*.rs,examples/rust-native-plugin/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not pass Rust runtime types, trait objects, futures, or allocator-owned strings across the native dynamic-library boundary.
Files:
examples/rust-native-plugin/src/lib.rs
{crates/core/src/plugin/dynamic/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Native and worker plugins are trusted extensions; document that native plugins are in-process and unsandboxed, and worker plugins provide process isolation but not a security sandbox.
Files:
examples/rust-native-plugin/src/lib.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
{docs/build-plugins/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
When detailed dynamic plugin guides exist, keep Rust native, Python worker, and
grpc-v1protocol details on separate pages.
Files:
examples/rust-native-plugin/src/lib.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
examples/rust-native-plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/plugin/src/lib.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:
examples/rust-native-plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/plugin/src/lib.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:
examples/rust-native-plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/plugin/src/lib.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Use
snake_casenaming in Rust and Python.
Files:
examples/rust-native-plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/plugin/src/lib.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:
examples/rust-native-plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/plugin/src/lib.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:
examples/rust-native-plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/plugin/src/lib.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/native_plugin_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validationUse
Result<T>withFlowErrorin core runtime paths, keeping wrapper-layer errors explicit and binding-appropriate.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.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/native_plugin_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: UseJson = serde_json::Valuein Rust-facing runtime APIs where existing code expects JSON payloads.
Treat Rust as the source of truth for runtime behavior; binding APIs should mirror Rust semantics unless a language-specific wrapper intentionally improves ergonomics.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/plugin/src/lib.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/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.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/native_plugin_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.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/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rs
{crates/core/src/plugin/dynamic/**/*.rs,crates/plugin/**/*.rs,crates/worker/**/*.rs,crates/worker-proto/**/*.rs,python/plugin/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Manifest validation must cover kind, compatibility, load contract, integrity, capability mismatch, and disabled-plugin behavior.
Files:
crates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rs
{crates/plugin/**/*.rs,python/plugin/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Rust and Python SDKs must expose every supported registration surface.
Files:
crates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/core/tests/fixtures/native_plugin/src/lib.rscrates/plugin/src/lib.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/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/plugin/src/lib.rs
🔇 Additional comments (27)
crates/plugin/tests/typed_callbacks.rs (2)
3103-3105: 📐 Maintainability & Code QualityRun the required Rust checks before handoff.
Confirm that
just test-rust,cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings, anduv run pre-commit run --all-filespass for this change.Source: Coding guidelines
3103-3105: 📐 Maintainability & Code QualityRun the required Rust checks before handoff.
crates/plugin/tests/typed_callbacks.rs#L3103-L3105: runjust test-rust,cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings, anduv run pre-commit run --all-files.examples/rust-native-plugin/src/lib.rs#L277-L285: run the same required checks.Source: Coding guidelines
examples/rust-native-plugin/src/lib.rs (1)
277-285: 📐 Maintainability & Code QualityRun the required Rust checks before handoff.
Confirm that
just test-rust,cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings, anduv run pre-commit run --all-filespass for this change.Source: Coding guidelines
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx (1)
135-163: LGTM!Also applies to: 182-191
docs/reference/migration-guides.mdx (1)
380-387: LGTM!crates/plugin/src/lib.rs (11)
1118-1183: LGTM!
1185-1304: LGTM!
1431-1462: LGTM!
2508-2569: LGTM!
3054-3062: LGTM!Also applies to: 3081-3090, 3108-3120
3205-3211: LGTM!Also applies to: 3222-3277
3279-3379: LGTM!
1386-1392: 🩺 Stability & AvailabilityDo not flag
finishfor backpressure.native_async_stream_finishcloses the sender without callingtry_send, so it cannot returnNemoRelayStatus::Internalfor a full queue.async_stream_rejectusestry_sendand has separate backpressure behavior.> Likely an incorrect or invalid review comment.
1397-1429: 🩺 Stability & AvailabilityRetain the current callback ownership handling.
The host delivers a terminal callback for completion, errors, cancellation, and settlement. The trampoline also reclaims
user_datawhen the callback returnsfalse.> Likely an incorrect or invalid review comment.
3134-3172: 🩺 Stability & AvailabilityKeep the callback reclamation logic as implemented. Each
AsyncLlmStream::invoke_nextcall has distinctuser_data. After a callback returnsfalse, the host finishes that invocation without sending a terminal callback. Concurrent calls use independent callback state.> Likely an incorrect or invalid review comment.
2600-2611: 🩺 Stability & AvailabilityKeep the current ownership handling. The guard frees
user_dataon validation failures. After transfer, the registered wrapper owns it and frees it when registration fails or the middleware is deregistered. This applies to both async LLM registration methods.> Likely an incorrect or invalid review comment.crates/core/tests/fixtures/native_plugin/src/lib.rs (6)
6-16: LGTM!Also applies to: 26-48
50-84: LGTM!
238-238: LGTM!Also applies to: 249-249, 688-688
766-810: LGTM!
844-865: LGTM!
836-842: 🎯 Functional CorrectnessNo change needed: backpressure closes the stream cleanly.
AsyncLlmStream::Dropfalls back tofinish()when rejection encounters a full queue, so the caller receives noErritem.> Likely an incorrect or invalid review comment.crates/core/tests/integration/native_plugin_tests.rs (5)
27-27: LGTM!Also applies to: 46-48, 665-665
687-726: LGTM!
821-867: LGTM!
920-955: LGTM!
956-996: LGTM!
Signed-off-by: Alex Fournier <afournier@nvidia.com>
|
/ok to test 8bdd001 |
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 (2)
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx (2)
152-156: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse “intercept callback” instead of “registration callback.”
register_llm_execution_interceptregisters the callback. Its closure runs for each managed LLM execution. Replace “registration callback” with “intercept callback” to describe the lifecycle accurately.As per path instructions, documentation must be technically accurate against the current API.
🤖 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/build-plugins/dynamic-plugins/native-dynamic/about.mdx` around lines 152 - 156, Update the documentation text describing register_llm_execution_intercept and register_llm_stream_execution_intercept to call the per-execution closure the “intercept callback” instead of the “registration callback,” while preserving the existing completion and Relay behavior.Source: Path instructions
167-179: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winShow a bounded executor handoff in the example.
The snippet creates one detached OS thread for every interception. Under load, copied code can exhaust thread resources. Use a plugin-owned bounded executor, or label this as pseudocode and avoid presenting
std::thread::spawnas the executable pattern.Based on the PR objectives, asynchronous callbacks must preserve runtime responsiveness without creating an unbounded thread-per-call path.
🤖 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/build-plugins/dynamic-plugins/native-dynamic/about.mdx` around lines 167 - 179, Update the interception example around context.register_llm_execution_intercept to avoid presenting std::thread::spawn as the production pattern: demonstrate submitting the callback to a plugin-owned bounded executor, or clearly mark the thread handoff as pseudocode and replace the executable example with bounded dispatch. Preserve the asynchronous ownership transfer and cancellation check while ensuring repeated interceptions cannot create unbounded OS threads.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/build-plugins/dynamic-plugins/native-dynamic/about.mdx`:
- Around line 152-156: Update the documentation text describing
register_llm_execution_intercept and register_llm_stream_execution_intercept to
call the per-execution closure the “intercept callback” instead of the
“registration callback,” while preserving the existing completion and Relay
behavior.
- Around line 167-179: Update the interception example around
context.register_llm_execution_intercept to avoid presenting std::thread::spawn
as the production pattern: demonstrate submitting the callback to a plugin-owned
bounded executor, or clearly mark the thread handoff as pseudocode and replace
the executable example with bounded dispatch. Preserve the asynchronous
ownership transfer and cancellation check while ensuring repeated interceptions
cannot create unbounded OS threads.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: d46be1d1-5eee-4415-b4ab-114bea3dd27b
📒 Files selected for processing (4)
crates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/plugin/src/lib.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (30)
**/*.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/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/plugin/src/lib.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/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validationUse
Result<T>withFlowErrorin core runtime paths, keeping wrapper-layer errors explicit and binding-appropriate.
Files:
crates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/core/tests/fixtures/native_plugin/src/lib.rscrates/plugin/src/lib.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
**/*: Use release tags in raw Rust-compatible SemVer without a leadingv; tags such asv0.1.0are prohibited.
Use branch prefixesfeat/,fix/,docs/,test/, orrefactor/according to the change purpose.
Every commit in a pull request must include a DCOSigned-off-by:sign-off.
Before submitting a pull request, ensure pre-commit hooks, relevant tests, target-specific builds, documentation updates, and a rebase on the latestmainare complete.
Use commit messages in the formtype: short description, with a valid type and a first line under 72 characters.
Files:
crates/core/tests/fixtures/native_plugin/src/lib.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/core/tests/integration/native_plugin_tests.rscrates/plugin/src/lib.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/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
crates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/plugin/src/lib.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/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/plugin/src/lib.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/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/plugin/src/lib.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Use
snake_casenaming in Rust and Python.
Files:
crates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/plugin/src/lib.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: UseJson = serde_json::Valuein Rust-facing runtime APIs where existing code expects JSON payloads.
Treat Rust as the source of truth for runtime behavior; binding APIs should mirror Rust semantics unless a language-specific wrapper intentionally improves ergonomics.
Files:
crates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/plugin/src/lib.rs
**/*.{rs,py,js,mjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Preserve the existing Tokio-based asynchronous model and callback/future lifetimes; do not unexpectedly block or hide async work in bindings.
Files:
crates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/plugin/src/lib.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/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/plugin/src/lib.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/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rs
**/*.{rs,py,go,js,jsx,ts,tsx,c,h,html,md,mdx,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include the appropriate SPDX copyright and Apache-2.0 license header in every source file.
Files:
crates/core/tests/fixtures/native_plugin/src/lib.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/core/tests/integration/native_plugin_tests.rscrates/plugin/src/lib.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/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rs
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
**/*.mdx: In MDX files, top-of-file comments must use JSX comment delimiters ({/*and*/}); do not use HTML comments for MDX SPDX headers.
New or regenerated MDX files must use{/* ... */}for top-of-file SPDX comments.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
{crates/core/src/plugin/dynamic/**,crates/plugin/**,crates/worker/**,crates/worker-proto/**,crates/types/**,python/plugin/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Keep the stable boundary explicit: native plugins cross a C ABI, and worker plugins cross
grpc-v1.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/plugin/src/lib.rs
{crates/core/src/plugin/dynamic/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Native and worker plugins are trusted extensions; document that native plugins are in-process and unsandboxed, and worker plugins provide process isolation but not a security sandbox.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
{docs/build-plugins/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
When detailed dynamic plugin guides exist, keep Rust native, Python worker, and
grpc-v1protocol details on separate pages.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.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/build-plugins/dynamic-plugins/native-dynamic/about.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/build-plugins/dynamic-plugins/native-dynamic/about.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/build-plugins/dynamic-plugins/native-dynamic/about.mdx
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant reference documentation when public behavior or APIs change.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.{md,mdx,rst}: Use title case consistently for technical documentation headings and table headers; avoid quotation marks, ampersands, and exclamation marks in headings, while preserving official product, event, research, and whitepaper title case.
Format code elements, commands, parameters, package names, expressions, directories, file names, and paths in monospace; represent path placeholders with angle brackets inside monospace.
Format UI buttons, menus, fields, and labels in bold, and separate consecutive UI navigation labels with>.
Use quotation marks for error messages and strings when appropriate, italics for newly introduced terms and publication titles, and plain text for keyboard shortcuts.
Represent GitHub repositories with owner/repository link text, such as[NVIDIA/NeMo](link), rather than generic repository wording.
Introduce every code block with a complete sentence; do not let a code block complete or interrupt the grammar of surrounding prose; use syntax highlighting when supported.
Keep inline method, function, and class references consistent with nearby documentation; omit empty parentheses in prose when no call is shown.
Use descriptive link text matching the destination title when possible; avoid raw URLs, generic anchors, long-sentence links, and unnecessary links that distract from procedures.
Ensure lists have a complete lead-in sentence, more than one item, no more than two levels, parallel construction, one idea or action per item, and appropriate punctuation; use bullets for unordered items and numbers for ordered tasks.
Format definition lists with a bold term followed by a complete, parallel, punctuated definition.
Use tables for reference information, decision support, compatibility matrices, and comparable choices; flag one-row tables, missing captions or lead-ins, sentence-case headers where title case is expected, unexplained empty cells, and code or links that would be clearer as prose.
Write procedure steps as imperative ...
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.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/build-plugins/dynamic-plugins/native-dynamic/about.mdx
{crates/core/src/plugin/dynamic/**/*.rs,crates/plugin/**/*.rs,crates/worker/**/*.rs,crates/worker-proto/**/*.rs,python/plugin/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Manifest validation must cover kind, compatibility, load contract, integrity, capability mismatch, and disabled-plugin behavior.
Files:
crates/plugin/src/lib.rs
{crates/plugin/**/*.rs,python/plugin/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Rust and Python SDKs must expose every supported registration surface.
Files:
crates/plugin/src/lib.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/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/plugin/src/lib.rs
🔇 Additional comments (15)
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx (5)
77-77: LGTM!
140-151: LGTM!Also applies to: 157-163
165-166: LGTM!Also applies to: 184-190
192-193: LGTM!
135-138: 🗄️ Data Integrity & IntegrationKeep the v3-to-legacy retry statement. The loader retries the entry point with the legacy table when the v3 invocation returns
NemoRelayStatus::InvalidArg.> Likely an incorrect or invalid review comment.crates/plugin/src/lib.rs (5)
1006-1008: LGTM!Also applies to: 1018-1019
2508-2651: LGTM!
3027-3031: LGTM!Also applies to: 3053-3060
3212-3221: LGTM!
2571-2651: 📐 Maintainability & Code QualityRun the required Rust validation.
Install
rustfmtandjust, then runcargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings, andjust test-rust.crates/core/tests/fixtures/native_plugin/src/lib.rs (1)
814-857: LGTM!crates/core/tests/integration/native_plugin_tests.rs (4)
687-726: LGTM!
821-867: LGTM!
868-998: LGTM!
821-998: 📐 Maintainability & Code QualityRun the full core validation matrix.
Run
just test-rust,just test-python,just test-go, andjust test-node, followed byuv run pre-commit run --all-files.
|
/ok to test 8bdd001 |
willkill07
added
breaking
| ( | ||
| NemoRelayNativeAsyncMiddlewareKind::LlmExecutionIntercept, | ||
| "fixture_async_llm_execution", | ||
| raw_async_tool_execution_callback, | ||
| ), |
There was a problem hiding this comment.
I'm not gonna lie... this original bit confuses me a bit. I'd imagine that the async callback would be similar to others here rather than be completely different.
|
Closing this |
3 participants
Warning
Breaking change: In 0.7.1,
register_llm_execution_interceptandregister_llm_stream_execution_interceptuse completion-based callback signatures. Rust dynamic plugins using the previous synchronous signatures must update their callbacks to complete asynchronously. Short, bounded synchronous callbacks can instead rename their registrations toregister_sync_llm_execution_interceptorregister_sync_llm_stream_execution_intercept. The native ABI and plugin manifest format are unchanged.Overview
Expose safe typed completion-based LLM middleware to Rust dynamic plugins and make the standard typed registration methods async-first.
Details
register_llm_execution_interceptandregister_llm_stream_execution_interceptcompletion-based.register_sync_*method names.The completion API keeps plugin-owned I/O off Relay runtime workers without introducing a Rust
Futureor executor contract across the C ABI boundary.Validation:
cargo test -p nemo-relay-plugincargo test -p nemo-relay --test native_plugin_integration -- --test-threads=1cargo check --manifest-path examples/rust-native-plugin/Cargo.tomlcargo clippy --workspace --all-targets -- -D warningsjust docsuv run pre-commit run --all-filesTwo full
just test-rustruns each reached an unrelated, order-dependent failure on therelease/0.7test matrix. The failing Switchyard and CLI tests both passed when rerun independently; all plugin and ABI coverage passed in both full runs.Where should reviewer start?
Start with the typed registration methods and completion handles in
crates/plugin/src/lib.rs, then review the ABI v3 integration coverage incrates/core/tests/fixtures/native_plugin/src/lib.rsandcrates/core/tests/integration/native_plugin_tests.rs.Related Issues or PRs
Summary by CodeRabbit