feat(plugin): add targeted LLM continuations to native API v2 - #594
feat(plugin): add targeted LLM continuations to native API v2#594bbednarski9 wants to merge 32 commits into
Conversation
|
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:
WalkthroughNative plugin loading now supports API v1 and v2. API v2 adds ABI-v4 typed unary and streaming LLM continuations, targeted provider dispatch, cooperative tasks, cancellation, backpressure, failure sanitization, and library lifetime handling. ChangesNative API v2 dispatch
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
size:XL
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/core/src/plugin/dynamic/native.rs`:
- Around line 1773-1781: Update the blocking callback error path in the
`spawn_blocking(invoke).await` handling to reclaim `completion_ref`, `next_ref`,
and the owned host string in `invocation` before propagating the `JoinError`.
Mirror the cleanup performed by the inline panic branch, while preserving the
existing `FlowError::Internal` message for the join failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 65fd9b9c-c534-4ca1-b8ec-3e7d5a62ca43
📒 Files selected for processing (8)
crates/core/src/plugin/dynamic/native.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/README.mdcrates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Python / Test (windows-amd64)
🧰 Additional context used
📓 Path-based instructions (33)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.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.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/plugin/README.mddocs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.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.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.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.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.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.rscrates/core/tests/unit/native_plugin_tests.rs
**/*.{md,rst,html,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
**/*.{md,rst,html,txt}: Always spellNVIDIAin all caps. Do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun because the name starts with an 'en' sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company.
Use trademark symbols with product names only when the document type or legal guidance requires them.
Verify official capitalization, spacing, and hyphenation for product names.
Precede NVIDIA product names withNVIDIAon first mention when it is natural and accurate.
Do not rewrite product names for grammar or title-case rules.
Preserve third-party product names according to the owner's spelling.
Include the company name and full model qualifier on first use when it helps identify the model.
Preserve the official capitalization and punctuation of model names.
Use shorter family names only after the full name is established.
Spell out a term on first use and put the acronym in parentheses unless the acronym is widely understood by the intended audience.
Use the acronym on later mentions after it has been defined.
For long documents, reintroduce the full term if readers might lose context.
Form plurals of acronyms withs, not an apostrophe, such asGPUs.
In headings, common acronyms can remain abbreviated. Spell out the term in the first or second sentence of the body.
Common terms such asCPU,GPU,PC,API, andUIusually do not need to be spelled out for developer audiences.
Files:
crates/plugin/README.md
**/*.{md,rst,html}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
Link the first mention of a product name when the destination helps the reader.
Files:
crates/plugin/README.md
**/*.{md,rst,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
Spell
NVIDIAin all caps. Do not useNvidia,nvidia, orNV.
Files:
crates/plugin/README.md
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,rst}: Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text. Avoid raw URLs and weak anchors such as "here" or "read more."
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative steps. Keep steps parallel and split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce.
Preferrefer tooverseewhen the wording points readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical docs.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values.
Use numerals for 10 or greater and include commas in thousands.
Do not add trademark symbols to learning-oriented docs unless the source, platform, or legal guidance explicitly requires them.
Files:
crates/plugin/README.md
**/*.md
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.md: Use title case consistently in technical documentation headings
Avoid quotation marks, ampersands, and exclamation marks in headings
Keep product, event, research, and whitepaper names in their official title case
Use title case for table headers
Do not force social-media sentence case into technical docs
Format code elements, commands, parameters, package names, and expressions in monospace
Format directories, file names, and paths in monospace using backticks
Use angle brackets inside monospace for variables inside paths, such as/home/<username>/.login
Format error messages and strings in quotation marks, keeping literal code strings in code formatting when clearer
Format UI buttons, menus, fields, and labels in bold
Use angle brackets between UI labels for menu paths, such as File > Save As
Use italics for new terms on first use, sparingly and only when introducing the term
Use italics for publication titles
Format keyboard shortcuts in plain text, such as Press Ctrl+Alt+Delete
Use owner/repo link text for GitHub repositories, preferring[NVIDIA/NeMo](link)over prose references like 'the GitHub repo'
Introduce every code block with a complete sentence
Do not make a code block complete the grammar of the previous sentence
Do not continue a sentence after a code block
Use syntax highlighting when the format supports it for code blocks
Avoid the word 'snippet' unless the surrounding docs already use it as a term of art
Keep inline method, function, and class references consistent with nearby docs, omitting empty parentheses for prose readability when no call is shown
Use descriptive anchor text that matches the destination title when possible for links
Avoid raw URLs in running text
Avoid generic anchor text such as 'here,' 'this page,' and 'read more'
Include acronyms in link text when a linked term includes an acronym
Do not link long sentences or multiple sentences
Avoid links that pull readers away from a procedure unless the link is a p...
Files:
crates/plugin/README.md
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Update
README.md,fern/, package READMEs, and binding-support notes when public behavior, package names, examples, or supported bindings change.
**/*.{md,mdx}: Prefer the documented public API, not internal shortcuts
Keep package names, repo references, and build commands current
Keep release-process and release-notes guidance in repo-maintainer docs such asRELEASING.md, not as user-facing docs pages orCHANGELOG.md
Keep stable user-facing wrappers atscripts/root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pagesIf links in documentation change, run
just docs-linkcheck.
Files:
crates/plugin/README.mddocs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
crates/plugin/README.mddocs/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:
crates/plugin/README.mddocs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.rs
**/*.{md,mdx,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Examples and documentation must use each exporter's documented flush/deregister order before shutdown.
Files:
crates/plugin/README.mddocs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
In MDX files, top-of-file comments must use JSX comment delimiters (
{/*to open and*/}to close); do not use HTML comments for MDX SPDX headers
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
{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.mdxcrates/core/src/plugin/dynamic/native.rs
{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
{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/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.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.rscrates/core/src/plugin/dynamic/native.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:
crates/core/src/plugin/dynamic/native.rs
crates/core/src/plugin/dynamic/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
The native loader must keep libraries alive until registered callbacks are cleared and must deregister plugin kinds before unload.
Files:
crates/core/src/plugin/dynamic/native.rs
🧠 Learnings (1)
📚 Learning: 2026-07-28T20:07:29.880Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 571
File: crates/core/src/api/runtime/state.rs:996-1020
Timestamp: 2026-07-28T20:07:29.880Z
Learning: In NeMo Relay (RELAY-509), sanitizer callback failures must be treated as intentional fail-open behavior. When an event/tool (request/response) or LLM (request/response) sanitizer callback fails, the sanitizer chain should retain and publish the last valid event/payload snapshot (rather than dropping/invalidating the data) and log the failure including callback context (e.g., which sanitizer/callback failed and relevant identifiers). Apply this consistently across all sanitizer chains mentioned in the RELAY-509 documentation/migration guide.
Applied to files:
crates/core/src/plugin/dynamic/native.rs
🔇 Additional comments (29)
crates/plugin/src/lib.rs (9)
12-12: LGTM!Also applies to: 39-47
877-888: LGTM!
915-1087: LGTM!
1253-1324: LGTM!
2052-2059: LGTM!
2888-2935: LGTM!
3607-3629: LGTM!
3907-4013: LGTM!
4069-4099: LGTM!crates/plugin/tests/typed_callbacks.rs (1)
19-33: LGTM!Also applies to: 339-339, 374-379, 408-413, 423-441
crates/plugin/README.md (1)
34-34: LGTM!Also applies to: 52-54, 100-125
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx (1)
3-3: LGTM!Also applies to: 119-167, 169-175, 185-185
crates/core/src/plugin/dynamic/native.rs (11)
11-11: LGTM!Also applies to: 35-36, 45-72
335-342: LGTM!
389-405: LGTM!
814-829: LGTM!Also applies to: 890-890, 914-931
1433-1433: LGTM!
1535-1601: LGTM!
2295-2412: LGTM!
2415-2470: LGTM!
2626-2804: LGTM!
2806-2880: LGTM!
3136-3157: LGTM!Also applies to: 3246-3365, 3397-3458
crates/core/tests/fixtures/native_plugin/src/lib.rs (1)
300-328: LGTM!crates/core/tests/integration/native_plugin_tests.rs (1)
1201-1201: LGTM!Also applies to: 1249-1302
crates/core/tests/unit/native_plugin_tests.rs (4)
8-8: LGTM!Also applies to: 112-164
534-541: LGTM!
797-968: LGTM!
1030-1448: LGTM!
bbednarski9
changed the title
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)
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx (1)
171-178: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse a complete sentence before the code block.
Line 171 relies on the code block to complete the phrase ending in
with:. Replace it with a standalone sentence, such as “Use the following macro to export a v2-only plugin.”As per coding guidelines, “Introduce every code block with a complete sentence.”
🤖 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 171 - 178, Update the prose immediately before the nemo_relay_plugin_v2! code block to be a complete standalone sentence introducing the example, such as stating that the macro exports a v2-only plugin; leave the code block unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/cli/tests/coverage/shared/gateway_tests.rs`:
- Around line 692-756: Update the test
typed_dispatch_target_overrides_legacy_headers_without_exposing_transport_data
to assert effective.credential_policy equals
TargetCredentialPolicy::ExplicitTarget, matching
explicit_keyless_target_drops_source_credentials. Keep the existing header and
routing assertions unchanged.
In `@crates/core/src/api/runtime/continuation_context.rs`:
- Around line 109-121: Update invoke_with_llm_dispatch_target to delegate
through the existing invoke method instead of duplicating the callback future
wrapper. Preserve the current scoping order by keeping scope_llm_dispatch_target
as the outer operation while invoke handles Relay context restoration
internally.
In `@crates/core/src/api/runtime/llm_dispatch_context.rs`:
- Around line 18-25: Replace the derived Debug implementation for
LlmDispatchTargetContext with a manual one that prints method, route, and a URL
with its query string redacted, while representing headers by names only and
never values. Preserve the existing Clone, PartialEq, and Eq derives and ensure
formatting remains available to callers of current_llm_dispatch_target.
In `@crates/core/tests/unit/native_plugin_tests.rs`:
- Around line 946-993: Strengthen the assertions in
native_api_v2_rejects_prohibited_target_methods_and_headers and
native_api_v2_rejects_target_url_credentials by checking
assert_last_error_contains with the specific validation reason after each
prepare_typed_llm_dispatch failure, not just NemoRelayStatus::InvalidArg. Use
the existing assertion pattern near line 937 and match each case to its intended
method, header, or URL-credentials error message.
In `@crates/plugin/src/lib.rs`:
- Around line 963-1037: Update LlmNonHttpFailureKindV2 with an Unknown variant
and custom Deserialize implementation that maps unrecognized serialized strings
to Unknown, while preserving existing snake_case names and known-variant
behavior. Do not rely on #[serde(other)] or add #[non_exhaustive] to the public
structs LlmHttpFailureV2 and LlmNonHttpFailureV2.
---
Outside diff comments:
In `@docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx`:
- Around line 171-178: Update the prose immediately before the
nemo_relay_plugin_v2! code block to be a complete standalone sentence
introducing the example, such as stating that the macro exports a v2-only
plugin; leave the code block unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 17d827ab-ebb7-4966-a303-9febc999ecdd
📒 Files selected for processing (12)
crates/cli/src/gateway/mod.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/README.mdcrates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (35)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/cli/src/gateway/mod.rscrates/core/src/plugin/dynamic/native.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/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/cli/src/gateway/mod.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/cli/src/gateway/mod.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/cli/src/gateway/mod.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/cli/src/gateway/mod.rscrates/core/src/plugin/dynamic/native.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/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/cli/src/server/mod.rscrates/plugin/src/lib.rscrates/cli/src/gateway/mod.rscrates/core/src/plugin/dynamic/native.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/core/src/api/runtime.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/cli/src/server/mod.rscrates/plugin/README.mdcrates/cli/tests/coverage/shared/gateway_tests.rscrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/cli/src/gateway/mod.rscrates/core/src/plugin/dynamic/native.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.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/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/cli/src/gateway/mod.rscrates/core/src/plugin/dynamic/native.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/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.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/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/cli/src/gateway/mod.rscrates/core/src/plugin/dynamic/native.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/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
In MDX files, top-of-file comments must use JSX comment delimiters (
{/*to open and*/}to close); do not use HTML comments for MDX SPDX headers
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Update
README.md,fern/, package READMEs, and binding-support notes when public behavior, package names, examples, or supported bindings change.
**/*.{md,mdx}: Prefer the documented public API, not internal shortcuts
Keep package names, repo references, and build commands current
Keep release-process and release-notes guidance in repo-maintainer docs such asRELEASING.md, not as user-facing docs pages orCHANGELOG.md
Keep stable user-facing wrappers atscripts/root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pagesIf links in documentation change, run
just docs-linkcheck.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/plugin/README.md
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/plugin/README.md
{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/README.mdcrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.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.mdxcrates/core/src/plugin/dynamic/native.rs
{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,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.mdxcrates/plugin/README.md
{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
**/*.{md,rst,html,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
**/*.{md,rst,html,txt}: Always spellNVIDIAin all caps. Do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun because the name starts with an 'en' sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company.
Use trademark symbols with product names only when the document type or legal guidance requires them.
Verify official capitalization, spacing, and hyphenation for product names.
Precede NVIDIA product names withNVIDIAon first mention when it is natural and accurate.
Do not rewrite product names for grammar or title-case rules.
Preserve third-party product names according to the owner's spelling.
Include the company name and full model qualifier on first use when it helps identify the model.
Preserve the official capitalization and punctuation of model names.
Use shorter family names only after the full name is established.
Spell out a term on first use and put the acronym in parentheses unless the acronym is widely understood by the intended audience.
Use the acronym on later mentions after it has been defined.
For long documents, reintroduce the full term if readers might lose context.
Form plurals of acronyms withs, not an apostrophe, such asGPUs.
In headings, common acronyms can remain abbreviated. Spell out the term in the first or second sentence of the body.
Common terms such asCPU,GPU,PC,API, andUIusually do not need to be spelled out for developer audiences.
Files:
crates/plugin/README.md
**/*.{md,rst,html}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
Link the first mention of a product name when the destination helps the reader.
Files:
crates/plugin/README.md
**/*.{md,rst,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
Spell
NVIDIAin all caps. Do not useNvidia,nvidia, orNV.
Files:
crates/plugin/README.md
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,rst}: Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text. Avoid raw URLs and weak anchors such as "here" or "read more."
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative steps. Keep steps parallel and split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce.
Preferrefer tooverseewhen the wording points readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical docs.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values.
Use numerals for 10 or greater and include commas in thousands.
Do not add trademark symbols to learning-oriented docs unless the source, platform, or legal guidance explicitly requires them.
Files:
crates/plugin/README.md
**/*.md
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.md: Use title case consistently in technical documentation headings
Avoid quotation marks, ampersands, and exclamation marks in headings
Keep product, event, research, and whitepaper names in their official title case
Use title case for table headers
Do not force social-media sentence case into technical docs
Format code elements, commands, parameters, package names, and expressions in monospace
Format directories, file names, and paths in monospace using backticks
Use angle brackets inside monospace for variables inside paths, such as/home/<username>/.login
Format error messages and strings in quotation marks, keeping literal code strings in code formatting when clearer
Format UI buttons, menus, fields, and labels in bold
Use angle brackets between UI labels for menu paths, such as File > Save As
Use italics for new terms on first use, sparingly and only when introducing the term
Use italics for publication titles
Format keyboard shortcuts in plain text, such as Press Ctrl+Alt+Delete
Use owner/repo link text for GitHub repositories, preferring[NVIDIA/NeMo](link)over prose references like 'the GitHub repo'
Introduce every code block with a complete sentence
Do not make a code block complete the grammar of the previous sentence
Do not continue a sentence after a code block
Use syntax highlighting when the format supports it for code blocks
Avoid the word 'snippet' unless the surrounding docs already use it as a term of art
Keep inline method, function, and class references consistent with nearby docs, omitting empty parentheses for prose readability when no call is shown
Use descriptive anchor text that matches the destination title when possible for links
Avoid raw URLs in running text
Avoid generic anchor text such as 'here,' 'this page,' and 'read more'
Include acronyms in link text when a linked term includes an acronym
Do not link long sentences or multiple sentences
Avoid links that pull readers away from a procedure unless the link is a p...
Files:
crates/plugin/README.md
{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/cli/tests/coverage/shared/gateway_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/unit/native_plugin_tests.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.rscrates/core/src/plugin/dynamic/native.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/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:
crates/core/src/plugin/dynamic/native.rs
crates/core/src/plugin/dynamic/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
The native loader must keep libraries alive until registered callbacks are cleared and must deregister plugin kinds before unload.
Files:
crates/core/src/plugin/dynamic/native.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/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
📚 Learning: 2026-07-28T03:31:05.964Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 564
File: crates/core/src/api/runtime/subscriber_dispatcher.rs:297-314
Timestamp: 2026-07-28T03:31:05.964Z
Learning: In this codebase’s runtime API, do not implement incremental native LLM stream forwarding via the native ABI v3 asynchronous middleware protocol (it can only settle a single JSON value via a one-shot completion handle and cannot forward stream chunks incrementally). If a latency-sensitive plugin needs streaming behavior, review for use of synchronous native stream intercepts or worker plugins instead of trying to chunk-deliver or incrementally forward over the ABI v3 async path.
Applied to files:
crates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rs
🔇 Additional comments (17)
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx (1)
174-182: 🎯 Functional CorrectnessThe documentation example is correct as written.
The
nemo_relay_plugin_v2!macro accepts any symbol name and generates a function with that name. Both native API v1 and v2 plugins can use the same symbol name (nemo_relay_register_plugin) because the safety mechanism depends on the manifest'scompat.native_apideclaration, not the symbol name. Whencompat.native_api = "2", the loader passes only the v4 host API; the generated plugin code validates the API version it receives. Symbol name reuse is safe because each plugin exists in a separate library with its own manifest configuration.> Likely an incorrect or invalid review comment.crates/plugin/src/lib.rs (1)
940-951: LGTM!Also applies to: 1039-1071
crates/plugin/README.md (1)
117-133: LGTM!crates/plugin/tests/typed_callbacks.rs (1)
38-98: LGTM!crates/core/src/api/runtime.rs (1)
9-9: LGTM!Also applies to: 27-28
crates/core/src/api/runtime/llm_dispatch_context.rs (1)
74-87: LGTM!crates/core/src/plugin/dynamic/native.rs (4)
2302-2372: LGTM!
2684-2762: LGTM!Also applies to: 2865-2913
2775-2801: 🗄️ Data Integrity & IntegrationNo target rebinding is needed in the producer task.
upstream.send().awaitcompletes beforesse_json_stream(response)constructs the returned stream. Producer polling only consumes the established response body.> Likely an incorrect or invalid review comment.
2383-2410: 🩺 Stability & AvailabilityNo cancellation arm is required
FlowErrorhas no cancellation, abort, or shutdown variant.LlmNonHttpFailureKindV2::Cancelledis emitted directly by stream lifecycle paths, not throughtyped_llm_failure.> Likely an incorrect or invalid review comment.crates/core/tests/unit/native_plugin_tests.rs (1)
131-168: LGTM!Also applies to: 815-944, 1015-1044, 1108-1320, 1323-1612
crates/cli/src/gateway/mod.rs (3)
29-30: LGTM!Also applies to: 145-145, 308-308, 317-317, 802-802, 812-823, 847-847, 875-893, 953-953, 963-970, 1169-1169
1-1: 🩺 Stability & Availability | 🏗️ Heavy liftPanic risk on malformed typed-target data is unverified and untested.
effective_dispatch_request_with_targetuses four.expect()calls that assume native-plugin-supplied header names, header values, HTTP method, and route strings were already validated. That validation is not visible in this review batch, and no test exercises the failure case.
crates/cli/src/gateway/mod.rs#L894-935: confirm (with the linked verification script) thatcrates/core/src/api/runtime/llm_dispatch_context.rsand the plugin SDK's route enum incrates/plugin/src/lib.rsfully guarantee these invariants; otherwise replace the.expect()calls with a gracefulFlowError/CliErrorpath.crates/cli/tests/coverage/shared/gateway_tests.rs#L692-756: add a test that constructs anLlmDispatchTargetContextwith an invalid header name/value, invalid method, or unrecognized route to prove the intended behavior (safe rejection, not a panic).
894-935: 🩺 Stability & AvailabilityRoute enum mismatch between plugin SDK and gateway is a latent maintenance risk; header and method validation is already enforced upstream.
LlmDispatchTargetContextcarries only untyped strings for method, route, and headers. The comments at lines 915, 917, and 926 state these values "were validated"—this is correct. Validation occurs incrates/core/src/plugin/dynamic/native.rsinprepare_typed_llm_dispatch()(lines 2302–2355) beforeLlmDispatchTargetContextis constructed:
- HTTP method (lines 2321–2328): validated via
reqwest::Method::from_bytes(), rejects CONNECT and TRACE.- Header names (lines 2330–2334): validated via
HeaderName::from_bytes().- Header values (lines 2341–2345): validated via
HeaderValue::from_str().The genuine risk is the route enum mismatch.
LlmDispatchRouteV2in the plugin SDK defines exactly three variants:OpenaiChat,OpenaiResponses,AnthropicMessages. All three map viaas_str()to strings thatProviderRoute::from_dispatch_override()accepts (lines 67, 71, 77). If the plugin SDK adds or renames a route variant without a corresponding update tofrom_dispatch_override(), the string will not match any arm, and the.expect()at line 924 panics.Enforcing sync between the plugin SDK's route enum and gateway route acceptance is the practical value. Header and method
.expect()calls are safe because validation is exhaustive upstream; the route.expect()is a maintenance invariant that requires code review discipline.crates/cli/src/server/mod.rs (1)
63-63: LGTM!Also applies to: 546-560
crates/cli/tests/coverage/shared/gateway_tests.rs (2)
16-16: LGTM!Also applies to: 2068-2068, 2107-2107
692-756: 🩺 Stability & Availability | 🏗️ Heavy liftMissing negative-path coverage for malformed dispatch targets.
This test only covers a well-formed
LlmDispatchTargetContext.effective_dispatch_request_with_targetincrates/cli/src/gateway/mod.rspanics via.expect()on invalid header names/values, an invalid method, or an unrecognized route string. Add a test constructing a target with one such invalid value to confirm the intended behavior (either a documented panic-is-unreachable invariant, backed by upstream validation, or a graceful error). This note is tied to the panic-risk finding raised oncrates/cli/src/gateway/mod.rs(lines 894-935); see the consolidated comment.As per path instructions, tests in this path should cover "error paths" for the changed API surface.
Source: Path instructions
bbednarski9
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/plugin/README.md (1)
124-126: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSeparate normal completion from cancellation.
Document the normal path as requesting events through
DoneorFailure, then releasing the provider stream exactly once. If the caller stops before a terminal event, document calling cancellation once before releasing the stream exactly once.🤖 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/plugin/README.md` around lines 124 - 126, Update the streaming dispatch documentation to distinguish normal completion from early cancellation: request events until receiving Done or Failure, then release the provider stream exactly once; if the caller stops before a terminal event, cancel once and then release the stream exactly once.
♻️ Duplicate comments (1)
crates/core/tests/unit/native_plugin_tests.rs (1)
949-996: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPin the specific rejection reason in these validation tests.
native_api_v2_rejects_prohibited_target_methods_and_headers(Lines 949-996) andnative_api_v2_rejects_target_url_credentials(Lines 999-1015) each assert onlyNemoRelayStatus::InvalidArg.prepare_llm_continuation_invocationreturnsInvalidArgfor every validation failure: bad method, prohibited header, invalid header value, and non-absolute or credentialed URL. A regression that breaks one specific branch (for example, the header check silently passing through) still passes these tests because a different branch also producesInvalidArg.
native_api_v2_rejects_non_absolute_dispatch_targets_before_continuation(Line 939) already pins its reason withassert_last_error_contains("absolute HTTP(S) URL"). Apply the same pattern here.💚 Proposed test tightening
target.method = "CONNECT".into(); assert_eq!( prepare_llm_continuation_invocation(LlmContinuationInvocationV2 { request: request.clone(), target, }) .unwrap_err(), NemoRelayStatus::InvalidArg ); + assert_last_error_contains("method was invalid or prohibited"); let mut target = test_dispatch_target( "https://provider.example/v1/chat/completions", nemo_relay_plugin::LlmContinuationRouteV2::OpenaiChat, ); target .headers .insert("x-nemo-relay-internal-dispatch-url".into(), "secret".into()); assert_eq!( prepare_llm_continuation_invocation(LlmContinuationInvocationV2 { request: request.clone(), target, }) .unwrap_err(), NemoRelayStatus::InvalidArg ); + assert_last_error_contains("host-owned or prohibited"); let mut target = test_dispatch_target( "https://provider.example/v1/chat/completions", nemo_relay_plugin::LlmContinuationRouteV2::OpenaiChat, ); target .headers .insert("host".into(), "attacker.invalid".into()); assert_eq!( prepare_llm_continuation_invocation(LlmContinuationInvocationV2 { request, target }) .unwrap_err(), NemoRelayStatus::InvalidArg ); + assert_last_error_contains("host-owned or prohibited"); } #[test] fn native_api_v2_rejects_target_url_credentials() { let target = test_dispatch_target( "https://user:secret@provider.example/v1/chat/completions", nemo_relay_plugin::LlmContinuationRouteV2::OpenaiChat, ); assert_eq!( prepare_llm_continuation_invocation(LlmContinuationInvocationV2 { request: LlmRequest { headers: Map::new(), content: json!({}), }, target, }) .unwrap_err(), NemoRelayStatus::InvalidArg ); + assert_last_error_contains("absolute HTTP(S) URL without user info"); }Also applies to: 999-1015
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/core/tests/unit/native_plugin_tests.rs` around lines 949 - 996, Strengthen the validation tests by asserting each rejection’s specific error message, not only NemoRelayStatus::InvalidArg. In native_api_v2_rejects_prohibited_target_methods_and_headers, call the existing assert_last_error_contains pattern after each failed prepare_llm_continuation_invocation, using distinct expected text for the CONNECT method, prohibited internal-dispatch header, and host header; apply the same reason-specific assertion in native_api_v2_rejects_target_url_credentials, following native_api_v2_rejects_non_absolute_dispatch_targets_before_continuation.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/core/src/plugin/dynamic/native.rs`:
- Around line 2330-2353: Update the target-header validation loop before
constructing LlmDispatchTargetContext to track normalized HeaderName values and
reject any case-insensitive duplicate with InvalidArg and an appropriate native
error. Keep the existing prohibited-name and invalid-value validation unchanged,
and ensure duplicates are detected before invocation.target.headers is consumed.
In `@crates/plugin/README.md`:
- Around line 113-115: Update the registration documentation in the plugin
README to present the typed v2 registration methods first, and move the *_raw
helper references into a clearly labeled advanced ABI section. Keep the existing
compat.native_api and PluginContext::host_api_v4 details accurate while
prioritizing the documented public API over raw registration helpers.
---
Outside diff comments:
In `@crates/plugin/README.md`:
- Around line 124-126: Update the streaming dispatch documentation to
distinguish normal completion from early cancellation: request events until
receiving Done or Failure, then release the provider stream exactly once; if the
caller stops before a terminal event, cancel once and then release the stream
exactly once.
---
Duplicate comments:
In `@crates/core/tests/unit/native_plugin_tests.rs`:
- Around line 949-996: Strengthen the validation tests by asserting each
rejection’s specific error message, not only NemoRelayStatus::InvalidArg. In
native_api_v2_rejects_prohibited_target_methods_and_headers, call the existing
assert_last_error_contains pattern after each failed
prepare_llm_continuation_invocation, using distinct expected text for the
CONNECT method, prohibited internal-dispatch header, and host header; apply the
same reason-specific assertion in native_api_v2_rejects_target_url_credentials,
following
native_api_v2_rejects_non_absolute_dispatch_targets_before_continuation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 93f2d5b2-b3d9-4bd7-9deb-53a01cc78cb3
📒 Files selected for processing (5)
crates/core/src/plugin/dynamic/native.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/README.mdcrates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (28)
**/*.{md,rst,html,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
**/*.{md,rst,html,txt}: Always spellNVIDIAin all caps. Do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun because the name starts with an 'en' sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company.
Use trademark symbols with product names only when the document type or legal guidance requires them.
Verify official capitalization, spacing, and hyphenation for product names.
Precede NVIDIA product names withNVIDIAon first mention when it is natural and accurate.
Do not rewrite product names for grammar or title-case rules.
Preserve third-party product names according to the owner's spelling.
Include the company name and full model qualifier on first use when it helps identify the model.
Preserve the official capitalization and punctuation of model names.
Use shorter family names only after the full name is established.
Spell out a term on first use and put the acronym in parentheses unless the acronym is widely understood by the intended audience.
Use the acronym on later mentions after it has been defined.
For long documents, reintroduce the full term if readers might lose context.
Form plurals of acronyms withs, not an apostrophe, such asGPUs.
In headings, common acronyms can remain abbreviated. Spell out the term in the first or second sentence of the body.
Common terms such asCPU,GPU,PC,API, andUIusually do not need to be spelled out for developer audiences.
Files:
crates/plugin/README.md
**/*.{md,rst,html}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
Link the first mention of a product name when the destination helps the reader.
Files:
crates/plugin/README.md
**/*.{md,rst,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
Spell
NVIDIAin all caps. Do not useNvidia,nvidia, orNV.
Files:
crates/plugin/README.md
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,rst}: Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text. Avoid raw URLs and weak anchors such as "here" or "read more."
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative steps. Keep steps parallel and split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce.
Preferrefer tooverseewhen the wording points readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical docs.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values.
Use numerals for 10 or greater and include commas in thousands.
Do not add trademark symbols to learning-oriented docs unless the source, platform, or legal guidance explicitly requires them.
Files:
crates/plugin/README.md
**/*.md
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.md: Use title case consistently in technical documentation headings
Avoid quotation marks, ampersands, and exclamation marks in headings
Keep product, event, research, and whitepaper names in their official title case
Use title case for table headers
Do not force social-media sentence case into technical docs
Format code elements, commands, parameters, package names, and expressions in monospace
Format directories, file names, and paths in monospace using backticks
Use angle brackets inside monospace for variables inside paths, such as/home/<username>/.login
Format error messages and strings in quotation marks, keeping literal code strings in code formatting when clearer
Format UI buttons, menus, fields, and labels in bold
Use angle brackets between UI labels for menu paths, such as File > Save As
Use italics for new terms on first use, sparingly and only when introducing the term
Use italics for publication titles
Format keyboard shortcuts in plain text, such as Press Ctrl+Alt+Delete
Use owner/repo link text for GitHub repositories, preferring[NVIDIA/NeMo](link)over prose references like 'the GitHub repo'
Introduce every code block with a complete sentence
Do not make a code block complete the grammar of the previous sentence
Do not continue a sentence after a code block
Use syntax highlighting when the format supports it for code blocks
Avoid the word 'snippet' unless the surrounding docs already use it as a term of art
Keep inline method, function, and class references consistent with nearby docs, omitting empty parentheses for prose readability when no call is shown
Use descriptive anchor text that matches the destination title when possible for links
Avoid raw URLs in running text
Avoid generic anchor text such as 'here,' 'this page,' and 'read more'
Include acronyms in link text when a linked term includes an acronym
Do not link long sentences or multiple sentences
Avoid links that pull readers away from a procedure unless the link is a p...
Files:
crates/plugin/README.md
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Update
README.md,fern/, package READMEs, and binding-support notes when public behavior, package names, examples, or supported bindings change.
**/*.{md,mdx}: Prefer the documented public API, not internal shortcuts
Keep package names, repo references, and build commands current
Keep release-process and release-notes guidance in repo-maintainer docs such asRELEASING.md, not as user-facing docs pages orCHANGELOG.md
Keep stable user-facing wrappers atscripts/root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pagesIf links in documentation change, run
just docs-linkcheck.
Files:
crates/plugin/README.md
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
crates/plugin/README.md
{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:
crates/plugin/README.mdcrates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rscrates/core/src/plugin/dynamic/native.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/plugin/README.mdcrates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
**/*.{md,mdx,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Examples and documentation must use each exporter's documented flush/deregister order before shutdown.
Files:
crates/plugin/README.md
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.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/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.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/src/lib.rscrates/plugin/tests/typed_callbacks.rscrates/core/src/plugin/dynamic/native.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.rscrates/plugin/tests/typed_callbacks.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/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.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/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.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/plugin/tests/typed_callbacks.rscrates/core/tests/unit/native_plugin_tests.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.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/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.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:
crates/core/src/plugin/dynamic/native.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:
crates/core/src/plugin/dynamic/native.rs
crates/core/src/plugin/dynamic/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
The native loader must keep libraries alive until registered callbacks are cleared and must deregister plugin kinds before unload.
Files:
crates/core/src/plugin/dynamic/native.rs
🧠 Learnings (1)
📚 Learning: 2026-07-28T20:07:29.880Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 571
File: crates/core/src/api/runtime/state.rs:996-1020
Timestamp: 2026-07-28T20:07:29.880Z
Learning: In NeMo Relay (RELAY-509), sanitizer callback failures must be treated as intentional fail-open behavior. When an event/tool (request/response) or LLM (request/response) sanitizer callback fails, the sanitizer chain should retain and publish the last valid event/payload snapshot (rather than dropping/invalidating the data) and log the failure including callback context (e.g., which sanitizer/callback failed and relevant identifiers). Apply this consistently across all sanitizer chains mentioned in the RELAY-509 documentation/migration guide.
Applied to files:
crates/core/src/plugin/dynamic/native.rs
🔇 Additional comments (8)
crates/plugin/src/lib.rs (2)
977-990: Unknown non-HTTP failure kinds still fail deserialization.
LlmNonHttpFailureKindV2has no catch-all variant. A future host that adds a kind breaks older plugins that deserializeLlmContinuationFailureV2. This was raised on the previous commit; only the doc comment on Line 984 changed.
46-47: LGTM!Also applies to: 915-961, 1004-1031, 1042-1071, 1262-1329, 2061-2067, 2897-2943, 3619-3640, 3931-3947, 3985-4005, 4082-4082
crates/plugin/tests/typed_callbacks.rs (1)
17-21: LGTM!Also applies to: 39-98, 400-484
crates/core/src/plugin/dynamic/native.rs (2)
54-61: LGTM!Also applies to: 918-918, 1538-1538, 1577-1591, 2374-2413, 2456-2470, 2473-2531, 2688-2866, 2869-2917
916-932: 📐 Maintainability & Code QualityRun the full validation matrix before handoff.
This change touches
crates/core, so the repository rules requirecargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,just test-rust, and the Python, Go, and Node.js test targets. The PR description lists workspace Clippy, the documentation build, pre-commit, and full cross-language validation as still pending.As per coding guidelines: "If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js."Source: Coding guidelines
crates/plugin/README.md (2)
34-34: LGTM!Also applies to: 52-54, 100-112, 117-123, 127-133
113-115: 🗄️ Data Integrity & IntegrationKeep the Native API v2 documentation as written.
compat.native_api = "2"selects the ABI v4 host table, and the loader test covers this contract.host_api_v4()is optional only for hosts that do not provide ABI v4.> Likely an incorrect or invalid review comment.crates/core/tests/unit/native_plugin_tests.rs (1)
8-8: LGTM!Also applies to: 29-30, 112-183, 553-559, 816-893, 896-946, 1018-1047, 1110-1248, 1250-1324, 1326-1403, 1405-1486, 1488-1617
bbednarski9
changed the title
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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 1485-1489: The core-owned dispatch path around
targeted_llm_execution and targeted_llm_stream_execution must preserve upstream
status, headers, and response body metadata instead of leaving gateway
upstream_info and response_bytes empty. Propagate the execution response
metadata through both buffered and streaming paths into the gateway response,
then add tests covering status, headers, and body metadata for each path.
In `@crates/core/src/api/runtime/llm_dispatch_context.rs`:
- Around line 212-222: Bound the successful response body in dispatch_buffered
instead of calling response.bytes() without a limit. Reuse bounded_response_body
with a dedicated success-size limit, or validate Content-Length before reading,
while preserving JSON parsing and existing transport-error handling.
- Around line 305-321: Update transport_error and its call sites to log
structured diagnostics including the target host and reqwest error while
preserving the existing redacted plugin-visible bodies. Extend timeout detection
to recognize streamed read timeouts wrapped in decode errors by checking the
decode error source chain, and classify those failures as
UpstreamFailureClass::Timeout rather than Connection.
- Around line 278-289: Remove the global Client timeout from
targeted_http_client(), leaving connect_timeout and read_timeout intact for
shared transport behavior. Apply HTTP_REQUEST_TIMEOUT via
RequestBuilder::timeout only in dispatch_buffered(), while keeping
dispatch_stream() governed by HTTP_READ_TIMEOUT for idle-read protection.
In `@crates/core/tests/fixtures/native_plugin/src/lib.rs`:
- Around line 332-347: Remove the manual Box::from_raw cleanup in the
failed-registration branch after register_async_llm_execution_v2_raw; rely on
the registration failure path to invoke drop_targeted_fixture_state for state,
while preserving the existing error return.
In `@crates/core/tests/integration/native_plugin_tests.rs`:
- Around line 2160-2221: The EmbeddedFakeProvider::spawn worker can block
indefinitely in listener.accept(), causing Drop’s thread join to hang when
dispatch never reaches the provider. Configure an accept timeout or otherwise
bound the initial connection wait before the existing read-timeout logic, and
propagate the resulting failure so the test reports normally instead of hanging.
- Around line 1332-1358: Add the existing NativePluginTestCleanup guard at the
start of this test, before the assertions and provider interaction, so its Drop
implementation clears the plugin configuration and activation even when an
assertion panics. Remove the reliance on the trailing clear_plugin_configuration
and activation.clear calls, while preserving the current success-path assertions
and cleanup behavior.
In `@crates/core/tests/unit/llm_dispatch_context_tests.rs`:
- Around line 16-60: The duplicated fake HTTP providers can hang indefinitely in
accept and duplicate request parsing. In
crates/core/tests/unit/llm_dispatch_context_tests.rs#L16-L60, extract
FakeProvider, read_http_request, and response handling into shared test support
and bound the listener’s wait so the worker exits when no dispatch connects; in
crates/core/tests/integration/native_plugin_tests.rs#L2160-L2221, remove
EmbeddedFakeProvider and use that shared helper, with no separate accept or
Content-Length parsing logic.
- Around line 16-60: Update FakeProvider and the corresponding
EmbeddedFakeProvider test helper to use a shared implementation instead of
duplicating the fake-server setup. Ensure the listener’s accept path has a
bounded timeout or otherwise cannot block indefinitely, and preserve request
capture, response writing, and thread cleanup behavior for both unit and
integration tests.
- Around line 173-202: Update the provider thread’s response-writing logic in
the helper used by buffered_http_failure_is_bounded_and_filters_headers to
tolerate a client disconnect during write_all, treating a short
write/broken-pipe result as expected instead of panicking through expect.
Preserve panic behavior for unrelated write failures and keep the test’s
existing assertions unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 6b988f20-41c0-4b49-850d-e384d67b27dd
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
crates/cli/src/gateway/mod.rscrates/core/Cargo.tomlcrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/error.rscrates/core/src/plugin/dynamic/native.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/README.mddocs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (37)
{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/Cargo.tomlcrates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/Cargo.toml
📄 CodeRabbit inference engine (.agents/skills/prepare-code-freeze/SKILL.md)
Confirm or infer the target release version from
upstream/main:Cargo.toml. Derive the release branch asrelease/<major>.<minor>.Keep Rust package names and workspace metadata in
Cargo.tomlinternally consistent across the project.OpenTelemetry and OpenInference dependencies must be unconditional rather than Cargo feature-gated.
Files:
crates/core/Cargo.toml
**/*.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all TOML files using the
#comment form.
Files:
crates/core/Cargo.toml
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/core/Cargo.tomlcrates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/cli/src/gateway/mod.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/core/tests/integration/native_plugin_tests.rscrates/plugin/README.mdcrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.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/Cargo.tomlcrates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/cli/src/gateway/mod.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/cli/src/gateway/mod.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/cli/src/gateway/mod.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/cli/src/gateway/mod.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/cli/src/gateway/mod.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/cli/src/gateway/mod.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.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/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/cli/src/gateway/mod.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.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/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/cli/src/gateway/mod.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.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/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.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.rscrates/core/src/api/runtime.rscrates/core/src/api/runtime/llm_dispatch_context.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.rscrates/core/src/api/runtime.rscrates/core/src/api/runtime/llm_dispatch_context.rs
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
In MDX files, top-of-file comments must use JSX comment delimiters (
{/*to open and*/}to close); do not use HTML comments for MDX SPDX headers
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Update
README.md,fern/, package READMEs, and binding-support notes when public behavior, package names, examples, or supported bindings change.
**/*.{md,mdx}: Prefer the documented public API, not internal shortcuts
Keep package names, repo references, and build commands current
Keep release-process and release-notes guidance in repo-maintainer docs such asRELEASING.md, not as user-facing docs pages orCHANGELOG.md
Keep stable user-facing wrappers atscripts/root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pagesIf links in documentation change, run
just docs-linkcheck.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/plugin/README.md
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/plugin/README.md
{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/README.mdcrates/core/src/plugin/dynamic/native.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.mdxcrates/core/src/plugin/dynamic/native.rs
{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,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.mdxcrates/plugin/README.md
{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/**/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/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rs
**/*.{md,rst,html,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
**/*.{md,rst,html,txt}: Always spellNVIDIAin all caps. Do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun because the name starts with an 'en' sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company.
Use trademark symbols with product names only when the document type or legal guidance requires them.
Verify official capitalization, spacing, and hyphenation for product names.
Precede NVIDIA product names withNVIDIAon first mention when it is natural and accurate.
Do not rewrite product names for grammar or title-case rules.
Preserve third-party product names according to the owner's spelling.
Include the company name and full model qualifier on first use when it helps identify the model.
Preserve the official capitalization and punctuation of model names.
Use shorter family names only after the full name is established.
Spell out a term on first use and put the acronym in parentheses unless the acronym is widely understood by the intended audience.
Use the acronym on later mentions after it has been defined.
For long documents, reintroduce the full term if readers might lose context.
Form plurals of acronyms withs, not an apostrophe, such asGPUs.
In headings, common acronyms can remain abbreviated. Spell out the term in the first or second sentence of the body.
Common terms such asCPU,GPU,PC,API, andUIusually do not need to be spelled out for developer audiences.
Files:
crates/plugin/README.md
**/*.{md,rst,html}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
Link the first mention of a product name when the destination helps the reader.
Files:
crates/plugin/README.md
**/*.{md,rst,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
Spell
NVIDIAin all caps. Do not useNvidia,nvidia, orNV.
Files:
crates/plugin/README.md
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,rst}: Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text. Avoid raw URLs and weak anchors such as "here" or "read more."
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative steps. Keep steps parallel and split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce.
Preferrefer tooverseewhen the wording points readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical docs.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values.
Use numerals for 10 or greater and include commas in thousands.
Do not add trademark symbols to learning-oriented docs unless the source, platform, or legal guidance explicitly requires them.
Files:
crates/plugin/README.md
**/*.md
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.md: Use title case consistently in technical documentation headings
Avoid quotation marks, ampersands, and exclamation marks in headings
Keep product, event, research, and whitepaper names in their official title case
Use title case for table headers
Do not force social-media sentence case into technical docs
Format code elements, commands, parameters, package names, and expressions in monospace
Format directories, file names, and paths in monospace using backticks
Use angle brackets inside monospace for variables inside paths, such as/home/<username>/.login
Format error messages and strings in quotation marks, keeping literal code strings in code formatting when clearer
Format UI buttons, menus, fields, and labels in bold
Use angle brackets between UI labels for menu paths, such as File > Save As
Use italics for new terms on first use, sparingly and only when introducing the term
Use italics for publication titles
Format keyboard shortcuts in plain text, such as Press Ctrl+Alt+Delete
Use owner/repo link text for GitHub repositories, preferring[NVIDIA/NeMo](link)over prose references like 'the GitHub repo'
Introduce every code block with a complete sentence
Do not make a code block complete the grammar of the previous sentence
Do not continue a sentence after a code block
Use syntax highlighting when the format supports it for code blocks
Avoid the word 'snippet' unless the surrounding docs already use it as a term of art
Keep inline method, function, and class references consistent with nearby docs, omitting empty parentheses for prose readability when no call is shown
Use descriptive anchor text that matches the destination title when possible for links
Avoid raw URLs in running text
Avoid generic anchor text such as 'here,' 'this page,' and 'read more'
Include acronyms in link text when a linked term includes an acronym
Do not link long sentences or multiple sentences
Avoid links that pull readers away from a procedure unless the link is a p...
Files:
crates/plugin/README.md
{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:
crates/core/src/plugin/dynamic/native.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/core/src/plugin/dynamic/native.rs
crates/core/src/plugin/dynamic/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
The native loader must keep libraries alive until registered callbacks are cleared and must deregister plugin kinds before unload.
Files:
crates/core/src/plugin/dynamic/native.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/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
📚 Learning: 2026-07-28T03:31:05.964Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 564
File: crates/core/src/api/runtime/subscriber_dispatcher.rs:297-314
Timestamp: 2026-07-28T03:31:05.964Z
Learning: In this codebase’s runtime API, do not implement incremental native LLM stream forwarding via the native ABI v3 asynchronous middleware protocol (it can only settle a single JSON value via a one-shot completion handle and cannot forward stream chunks incrementally). If a latency-sensitive plugin needs streaming behavior, review for use of synchronous native stream intercepts or worker plugins instead of trying to chunk-deliver or incrementally forward over the ABI v3 async path.
Applied to files:
crates/core/src/api/runtime/llm_dispatch_context.rs
🔇 Additional comments (29)
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx (6)
186-188: Keep typed registration methods first.This section still points authors to raw v2 registration helpers. Present the typed
PluginContextregistration methods first. Move*_rawhelpers to an advanced ABI section.Source: Path instructions
3-3: LGTM!Also applies to: 16-16
119-131: LGTM!
133-170: LGTM!
172-185: LGTM!
190-196: LGTM!Also applies to: 206-206
crates/plugin/README.md (4)
113-115: Keep typed registration methods first.This section still points authors to raw v2 registration helpers. Present the typed
PluginContextregistration methods first. Move*_rawhelpers to an advanced ABI section.Source: Path instructions
34-34: LGTM!Also applies to: 52-54
100-112: LGTM!
117-139: LGTM!crates/core/src/api/runtime/llm_dispatch_context.rs (4)
70-88: 🗄️ Data Integrity & Integration | ⚡ Quick winCase-insensitive duplicate target headers are silently collapsed.
headersis aBTreeMap<String, String>, so"Authorization"and"authorization"are distinct keys.HeaderName::from_bytesnormalizes both toauthorization, andHeaderMap::insertreplaces the earlier value. The plugin gets no error, and which credential reaches the provider depends onBTreeMapordering. Reject case-insensitive duplicates instead of relying on replacement.🛡️ Proposed fix
let mut validated_headers = HeaderMap::new(); for (name, value) in headers { let name = HeaderName::from_bytes(name.as_bytes()).map_err(|_| { FlowError::InvalidArgument( "LLM continuation contained an invalid target header name".into(), ) })?; if prohibited_target_header(&name) { return Err(FlowError::InvalidArgument(format!( "LLM continuation target header {name} is host-owned or prohibited" ))); } + if validated_headers.contains_key(&name) { + return Err(FlowError::InvalidArgument(format!( + "LLM continuation target header {name} was specified more than once" + ))); + } let value = HeaderValue::from_str(&value).map_err(|_| {
4-31: LGTM!
127-147: LGTM!
337-381: LGTM!crates/core/src/api/runtime.rs (1)
27-31: LGTM!crates/core/Cargo.toml (1)
66-66: LGTM!crates/core/src/api/llm.rs (1)
30-31: LGTM!crates/core/src/plugin/dynamic/native.rs (1)
2305-2314: LGTM!crates/core/src/error.rs (1)
15-15: LGTM!Also applies to: 37-37, 51-51, 125-125
crates/cli/src/gateway/mod.rs (3)
323-333: LGTM!Also applies to: 527-537
799-810: LGTM!Also applies to: 912-912, 922-929
862-877: 🗄️ Data Integrity & IntegrationRemove this review comment.
effective_dispatch_requestis test-only, and production forwarding passes the real method tobuild_effective_dispatch_request. The wrapper cannot affect observability or ATIF capture.> Likely an incorrect or invalid review comment.crates/core/tests/fixtures/native_plugin/src/lib.rs (2)
10-15: LGTM!
351-371: LGTM!Also applies to: 373-463, 465-468
crates/core/tests/integration/native_plugin_tests.rs (1)
8-8: LGTM!crates/core/tests/unit/llm_dispatch_context_tests.rs (3)
1-14: LGTM!Also applies to: 93-123
125-171: LGTM!
204-221: LGTM!Also applies to: 223-283, 285-319, 321-347
crates/core/tests/unit/native_plugin_tests.rs (1)
828-839: LGTM!Also applies to: 1426-1430
License DiffCompared against Lockfile license changesLockfile License ChangesRustAdded
Removed
Updated/Changed
NodeAdded
Removed
Updated/Changed
PythonAdded
Removed
Updated/Changed
Status output |
coderabbitai
Bot
added
the
DO NOT MERGE
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/core/src/api/runtime/llm_dispatch_context.rs (1)
326-350: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRead-timeout misclassification during streaming remains open.
The new
log::warn!call fixes the missing-diagnostics half of the earlier concern by recordingprovider_hostandfailure_kind. The classification half is still open:error.is_timeout()does not detect a read timeout that fires whiledispatch_stream's body is being consumed —reqwestwraps that as a body/decode error, not a timeout error. Line 262 routes any stream-body error through this same function, so a stalled provider during streaming getsUpstreamFailureClass::Connectioninstead ofUpstreamFailureClass::Timeout.UpstreamFailure::is_retryabletreats both the same, so retries are unaffected, but the class is wrong for observability and policy.Check the decode-error source chain (or
error.is_decode()plus the wrapped source) to classify streamed read timeouts asTimeout.🤖 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/runtime/llm_dispatch_context.rs` around lines 326 - 350, Update transport_error to detect read timeouts wrapped inside streaming body/decode errors, using the reqwest decode status and its source error chain in addition to error.is_timeout(). Set timeout, failure_kind, and the resulting UpstreamFailureClass/response body consistently to Timeout for these cases, while preserving transport classification for non-timeout errors.
♻️ Duplicate comments (1)
crates/core/src/api/runtime/llm_dispatch_context.rs (1)
217-230: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound the buffered success-body read.
Line 222 bounds the failure body through
bounded_response_body, but line 225-229 still callsresponse.bytes()with no size limit on the success path.reqwesthas no default response-size cap, so a target that streams an unbounded body forces Relay to buffer the whole thing in memory. The plugin selects this target, so a single continuation can exhaust process memory.Reuse a bounded read (e.g.,
bounded_response_bodywith a dedicated success-size limit) or reject responses whoseContent-Lengthexceeds that limit before reading.🛡️ Proposed fix
- let bytes = response - .bytes() - .await - .map_err(|error| transport_error(target, error))?; + let bytes = bounded_response_body(target, response).await?; serde_json::from_slice(&bytes).map_err(|_| http_error(status, headers, &bytes))🤖 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/runtime/llm_dispatch_context.rs` around lines 217 - 230, Update dispatch_buffered so successful responses also use a dedicated maximum body-size limit instead of response.bytes(). Reuse bounded_response_body or an equivalent bounded read, preserving the existing transport-error mapping and JSON parsing while rejecting or reporting bodies that exceed the limit.
🤖 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/runtime/llm_dispatch_context.rs`:
- Around line 326-350: Update transport_error to detect read timeouts wrapped
inside streaming body/decode errors, using the reqwest decode status and its
source error chain in addition to error.is_timeout(). Set timeout, failure_kind,
and the resulting UpstreamFailureClass/response body consistently to Timeout for
these cases, while preserving transport classification for non-timeout errors.
---
Duplicate comments:
In `@crates/core/src/api/runtime/llm_dispatch_context.rs`:
- Around line 217-230: Update dispatch_buffered so successful responses also use
a dedicated maximum body-size limit instead of response.bytes(). Reuse
bounded_response_body or an equivalent bounded read, preserving the existing
transport-error mapping and JSON parsing while rejecting or reporting bodies
that exceed the limit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: af91c6ba-c4bf-4302-9f79-e7aaaa4219e0
📒 Files selected for processing (7)
crates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/unit/native_plugin_tests.rs
💤 Files with no reviewable changes (1)
- crates/core/tests/fixtures/native_plugin/src/lib.rs
📜 Review details
🧰 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 workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.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/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.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/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.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/src/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.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/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.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/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.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/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.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:
crates/core/src/plugin/dynamic/native.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:
crates/core/src/plugin/dynamic/native.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:
crates/core/src/plugin/dynamic/native.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/core/src/plugin/dynamic/native.rs
crates/core/src/plugin/dynamic/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
The native loader must keep libraries alive until registered callbacks are cleared and must deregister plugin kinds before unload.
Files:
crates/core/src/plugin/dynamic/native.rs
🧠 Learnings (2)
📚 Learning: 2026-07-28T03:31:05.964Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 564
File: crates/core/src/api/runtime/subscriber_dispatcher.rs:297-314
Timestamp: 2026-07-28T03:31:05.964Z
Learning: In this codebase’s runtime API, do not implement incremental native LLM stream forwarding via the native ABI v3 asynchronous middleware protocol (it can only settle a single JSON value via a one-shot completion handle and cannot forward stream chunks incrementally). If a latency-sensitive plugin needs streaming behavior, review for use of synchronous native stream intercepts or worker plugins instead of trying to chunk-deliver or incrementally forward over the ABI v3 async path.
Applied to files:
crates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.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/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
🔇 Additional comments (8)
crates/core/tests/integration/native_plugin_tests.rs (2)
2169-2190: LGTM!
1331-1332: 📐 Maintainability & Code QualityComplete the required
crates/corevalidation matrix before merge.The PR states that the cross-language matrix remains pending. Run
just test-rust,just test-python,just test-go,just test-node,cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,cargo deny check,validate-change, anduv run pre-commit run --all-files.
crates/core/tests/integration/native_plugin_tests.rs#L1331-L1332: include this integration-test change in the validation results.crates/core/tests/unit/llm_dispatch_context_tests.rs#L25-L64: include this provider-lifecycle change in the validation results.crates/core/tests/unit/native_plugin_tests.rs#L973-L1024: include this native API v2 validation coverage in the validation results.As per coding guidelines: “Changes to
crates/coreorcrates/adaptivemust run the full language matrix.”Source: Coding guidelines
crates/core/tests/unit/native_plugin_tests.rs (1)
973-973: LGTM!Also applies to: 990-990, 1004-1004, 1024-1024
crates/core/src/api/runtime/llm_dispatch_context.rs (2)
82-86: LGTM!The case-insensitive duplicate-header check is correct here:
HeaderNamelookup inHeaderMapis case-insensitive, sovalidated_headers.contains_key(&name)catches variants likeAuthorization/authorization. This mirrors the same fix already applied incrates/core/src/plugin/dynamic/native.rs.
329-335: 🩺 Stability & AvailabilityNo change needed:
logenables thekvfeature incrates/core/Cargo.toml.> Likely an incorrect or invalid review comment.crates/core/src/api/runtime/continuation_context.rs (1)
11-13: LGTM!The prior review comment requesting delegation to
invokeinstead of duplicating its body is now applied at line 120. Scoping order is preserved: the dispatch target remains the outer task-local, andinvoke/runrestores the Relay context inside it.Also applies to: 109-121
crates/core/src/plugin/dynamic/native.rs (2)
1776-1793: LGTM!The
spawn_blockingjoin-error path now reclaimscompletion_ref,next_ref, and freesinvocationbefore returning, matching the previously requested fix.
1797-1822: 🩺 Stability & AvailabilityKeep
next_refownership with the callback.The callback owns
nextafter invocation and must callasync_next_releaseexactly once. The host must not reclaim it in the panic or invalid-state branches, because the callback may already have released it. The join-error branch is different because the callback did not execute.> Likely an incorrect or invalid review comment.
a3e78ac to
a11ff5e
Compare
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
070b6d3 to
3a8f8f0
Compare
1 participant
Overview
Important
Introduces native API v2
This PR adds the
compat.native_api = "2"manifest contract andNemoRelayNativeHostApiV4for the first time (seeprevious native API v1 and
previous ABI V3). Existing native API v1 plugins continue
to use ABI V3 unchanged. Development builds compiled against earlier commits
of this PR must be rebuilt, but no published native-plugin contract is broken.
Native LLM routing policies need to invoke a captured Relay continuation
against an explicit provider target, inspect the buffered or streaming result,
and leave calls they do not target on Relay's ordinary path. That capability
must work in CLI and SDK-embedded hosts without private request headers, CLI
gateway state, request-long blocking callbacks, or plugin-authored unsafe FFI
glue.
This PR provides that general contract:
LLM middleware.
targeted requests with HTTP
POST.completion-based and
incremental-stream middleware registration.
hooks on top of the previous ABI V3.
nemo-relay-pluginwraps the C ABI with safe Rust continuations, providerstreams, asynchronous registration, and host-owned stream pass-through.
Switchyard is the first consumer, but no Switchyard algorithm, protocol codec,
retry policy, or service behavior is encoded in Relay.
Details
Core-managed targeted LLM continuations
LlmContinuationInvocationV2contains a replacementLlmRequestand anLlmContinuationTargetV2with:Relay validates the target, rejects redirects and unsafe URL/header forms, and
performs the terminal request in core. The target is typed invocation context,
not a private field in
LlmRequest.headers. Relay captures and restores thatcontext across continuation, task, and thread hops, so a targeted call cannot
silently fall back to the host's original provider.
Remaining LLM execution middleware runs before terminal dispatch. Successful
buffered provider JSON is bounded to 16 MiB. Provider rejections return HTTP
status, a bounded body, and safe response headers. Failures without an HTTP
response use a small transport-oriented classification. Retry, reselection,
and fallback remain plugin policy.
Target URLs, credential values, and transport headers are not written into
provider JSON, marks, spans, logs, or inspection output.
Cooperative native API v2 execution
Safe v2 callbacks register through ABI V3's existing
completion-based and
incremental-stream asynchronous middleware
functions and return
Pending. V4 (see previous ABI V3)provides general owner-bound task scheduling plus the targeted continuation
operations.
Relay:
cancellation;
versions; and
Native API v2 does not add a second registration lane, callback execution
policy, blocking executor, or version-specific output-capacity contract.
Targeted provider streams are pull-based and allow one outstanding pull.
Streaming endpoints return SSE with JSON
dataframes; Relay removes the SSEframing and exposes JSON events to the plugin.
Native callback futures must remain executor-neutral. A plugin shared library
may link a different async-runtime instance, so host polling does not enter
plugin-local Tokio state across the dynamic-library boundary.
Safe Rust SDK
Rust plugins use:
PluginContext::register_async_llm_execution_v2PluginContext::register_async_llm_stream_execution_v2LlmContinuationV2::callLlmContinuationV2::call_passthroughLlmStreamContinuationV2::open_streamLlmStreamExecutionOutcomeV2::PassthroughThe SDK owns host strings, JSON conversion, callback-to-future adaptation,
panic isolation, completion settlement, stream polling, cancellation, and
opaque-handle release. Continuations are cloneable and support repeated or
concurrent calls. The raw C table remains available for advanced and non-Rust
consumers.
For an untargeted stream,
async_llm_next_forward_stream_v2connects theordinary downstream continuation directly to Relay's caller-facing output
stream. Provider events do not cross into the plugin merely to be forwarded.
flowchart TD A["Caller or embedded host"] --> B["Relay LLM pipeline"] B --> C["V3 asynchronous middleware callback"] C --> D["Safe SDK policy future"] D --> E["V4 cooperative host task"] E -->|"Pending"| F["Park without blocking a worker"] F -->|"Wake or cancellation"| E E -->|"Targeted call or stream"| G["Captured LLM continuation"] G --> H["Remaining LLM middleware"] H --> I["Relay core HTTP POST JSON/SSE transport"] I --> J["Selected provider"] J --> G E -->|"Plugin output"| K["V3 bounded output stream"] K --> A E -->|"Untargeted stream"| L["V4 direct-forward hook"] L --> H H --> M["Relay-owned bounded forwarding"] M --> ACompatibility and scope
compat.native_apiis the operator-facing native-plugin API selector inrelay-plugin.toml. It is not the Relay release version, plugin version,nemo-relay-plugincrate version, or C host-table revision. Relay validates itbefore loading the native library and selects the required contract:
"1"nemo_relay_plugin!NemoRelayNativeHostApiV3"2"nemo_relay_plugin_v2!NemoRelayNativeHostApiV4Native API v1 layouts, symbols, loading, and behavior
remain unchanged. A host that supports only native API v1 rejects a v2-only
plugin instead of loading it through an older table.
compat.relayseparatelydeclares compatible Relay release versions using a SemVer requirement.
serde_json::Value, allocator-owned Ruststring, or
FlowErrorcrosses the C boundary.continuation redesign, provider codec, or routing-policy implementation.
Alternative design: one manifest generation with table probing
A new manifest value is not required merely because V4 appends fields to the C
host table. Relay could keep
compat.native_api = "1"for every native pluginand probe V3, legacy V2, and V4 until the plugin entry point accepts one. The
existing native API v1 loader already uses
InvalidArgto negotiate between V3and legacy V2, so extending that sequence is technically possible.
This PR instead treats native API v2 as a capability and behavior generation:
operations that do not exist in V3.
append. Its inherited
async_stream_push_jsonandasync_stream_rejectoperations report a full queue as
WouldBlock; native API v1 reports thesame condition as
Internal. The distinction is documented in thestream backpressure contract. Giving V4 to a
permissive v1 plugin could therefore change its control flow even though the
struct prefix is readable.
unsupported capability before invoking plugin code. It also lets
plugins validate,plugins inspect, anddoctorexplain the requirementwithout inferring it from a failed entry call.
on
InvalidArg. That status could mean either "this table is unsupported"or a genuine plugin entry failure, and the outcome would depend on probe
order. An older host would also rely entirely on
compat.relaybeing narrowenough to reject a V4-only bundle before attempting to load it.
native_api = "2"native_api = "1"InvalidArgThe single-generation alternative becomes robust if the team prefers it and
first makes V4 a pure append:
WouldBlockbackpressure;InvalidArgas a version mismatch;compat.relayas the authoritative older-host gate; andor add a separate capability declaration.
The current PR chooses the explicit v2 manifest contract because it isolates
the behavioral difference and produces clearer compatibility diagnostics. The
alternative remains viable, but it is a broader ABI-negotiation redesign rather
than deletion of the
"2"value alone.Relationship to Switchyard
libsy.run_streamrouting plugin using this general contract.Validation
cargo test -p nemo-relay-plugin: 91 typed callback tests and one optimization-contract test passed.cargo test -p nemo-relay --test native_plugin_integration: 37/37 passed, including native API v1 compatibility and embedded-core v2 execution without CLI state.Focused continuation-context, targeted-dispatch, cooperative-task, ownership, cancellation, and malformed-ABI suites passed.
Switchyard test: validate OpenClaw hook-only fallback exports #220 rebuilt against exact Relay revision
3a8f8f0745fc6545a9162bd585da2273fa052785and passed its real-process fake-provider E2E for random and LLM-classifier routing, all three supported protocols, buffering, streaming, retry/fallback, provider-event preservation, untargeted pass-through, and credential isolation.A live Ollama smoke routed nonempty buffered and streaming responses across both
qwen3:4bandllama3.2:latest, with genuine Switchyard requested/decision marks for every attempt.just test-rustpassed, including core, adaptive, CLI, native-plugin,worker, raw-FFI, integration, and doctest suites.
just test-pythonpassed with 610 tests; the focused plugin suite passed125 tests at 97.07% coverage and its worker E2E passed.
just test-gopassed andjust test-nodepassed with 344 tests.Workspace formatting, Clippy with warnings denied,
cargo deny,just docs,and
uv run pre-commit run --all-filespassed.The repository's documentation link checker passed as part of full
pre-commit validation.
Where should the reviewer start?
crates/core/src/api/runtime/llm_dispatch_context.rsfor target validationand core-owned HTTP dispatch.
crates/core/src/api/runtime/continuation_context.rsfor target capture andrestoration across continuation hops.
crates/plugin/src/native_v2.rsfor the safe Rust facade.crates/core/src/plugin/dynamic/native.rsfor cooperative task polling,pass-through, backpressure, cancellation, and lifetime handling.
crates/core/tests/integration/native_plugin_tests.rsfor embedded-coreexecution without CLI state.
The central design decision is that ABI V3 continues to own
generic asynchronous middleware registration. V4 adds only targeted LLM
continuation operations and general cooperative task facilities.
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)