feat: add async middleware C and Go APIs - #572
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:
WalkthroughAdds completion-based asynchronous middleware across the Rust FFI, generated C header, Go bindings, global and scope-local registration APIs, streaming continuations, cancellation handling, parity validation, documentation, and comprehensive tests. ChangesCompletion-based Rust ABI
Go async bridge
Validation and documentation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant GoMiddleware
participant GoTrampoline
participant RustAsyncWrapper
participant AsyncNext
participant AsyncCompletion
GoMiddleware->>GoTrampoline: process invocation
GoTrampoline->>RustAsyncWrapper: resolve or reject JSON
RustAsyncWrapper->>AsyncNext: invoke continuation when requested
AsyncNext-->>RustAsyncWrapper: return result or stream chunks
RustAsyncWrapper->>AsyncCompletion: settle middleware outcome
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
github-actions
Bot
added
size:XL
|
@coderabbitai review |
✅ Action performedReview finished.
|
github-actions
Bot
added
size:XXL
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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ffi/build.rs (1)
12-33: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winHeader generation failure is still swallowed, and the async block now depends on an exact guard string.
Two build-integrity issues in the same block:
if let Ok(bindings)discards the error. When cbindgen fails, the injection step is skipped entirely and the stale committednemo_relay.hships — now with the added risk that its async section no longer matches the Rust exports, even thoughvalidate_async_registration_paritypassed. Make this fatal (.expect(...)).marker = "\n#endif /* NEMO_RELAY_H */\n"hard-codes cbindgen's current two-space comment spacing and assumesinclude_guardis set incbindgen.toml. Any formatting change or a switch topragma_onceturns this into a build panic. Derive the guard line from the config value, or append the block viabindings.write+ a documented trailer instead of string surgery.🔧 Fail loudly on generation errors
- if let Ok(bindings) = cbindgen::Builder::new() + let bindings = cbindgen::Builder::new() .with_crate(&crate_dir) .with_config(config) .generate() - { + .expect("generate FFI bindings"); + {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ffi/build.rs` around lines 12 - 33, Update the cbindgen generation block to make `.generate()` failure fatal with `.expect(...)` instead of silently skipping header injection. Replace the hard-coded `marker` in the header rewrite with a trailer derived from the configured include guard (or use a documented `bindings.write` trailer approach), preserving correct behavior if formatting changes or `pragma_once` is enabled.
🤖 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/ffi/src/api/event_registry.rs`:
- Around line 47-60: Ensure failed async registrations invoke free_fn exactly
once for every validation failure. In crates/ffi/src/api/event_registry.rs:47-60
and register_scope_async at 166-184, plus async registration macros/functions in
crates/ffi/src/api/llm_registry.rs:17-46,
crates/ffi/src/api/scope_registry.rs:33-68, and
crates/ffi/src/api/tool_registry.rs:13-38, 74-77, update Err(status) paths for
name and UUID parsing to release user_data before returning. In
go/nemo_relay/nemo_relay.go:1243-1259 and withScopeAsyncMiddleware at 2496-2503,
call unregisterClosure(id) whenever checkStatus reports registration failure
across all global Register*Async APIs; document and consistently preserve the
chosen ownership ordering.
In `@crates/ffi/src/api/tool_registry.rs`:
- Around line 13-38: Consolidate the duplicated registration bodies into one
global macro and one scope-local macro in api/mod.rs, parameterized by callback
type and optional break_chain support. Replace async_tool_json_registration!,
async_llm_registration!, async_llm_execution_registration!,
scope_async_registration!, and scope_async_execution_registration! with these
shared macros while preserving their generated signatures and registration
behavior. Generate nemo_relay_register_tool_execution_intercept_async through
the execution macro instead of maintaining its hand-written implementation, so
cbindgen emits only one declaration.
In `@crates/ffi/src/callable.rs`:
- Around line 228-279: The nemo_relay_async_next_invoke error handling has an
inconsistent completion-settlement contract. Choose one consistent policy for
all failure paths, apply it to null next/completion, invalid invocation JSON,
and typed deserialization failures, and document the selected settle-on-error
behavior in the function’s header documentation so callers do not hang or
receive duplicate signals.
- Around line 920-952: Update wrap_async_llm_stream_execution_intercept_fn and
its invoke_async_intercept/next_invoke collection path to enforce a finite
maximum on buffered stream chunks or serialized bytes, failing fast with a clear
FlowError when the cap is exceeded. Define and reuse an explicit limit rather
than allowing unbounded Json::Array growth, and adjust the stream-support
documentation to state the cap and that this remains completion-based buffering
until a chunk-oriented async ABI is available.
- Around line 799-848: Replace the Debug-string codec construction in
wrap_async_llm_sanitize_request_fn and wrap_async_llm_sanitize_response_fn with
the structured codec identity produced by ffi_codec_identity, preserving the
sync FFI envelope shape of {codec_kind, codec_id} for both async callbacks.
In `@crates/ffi/tests/unit/api/coverage_sweeps_tests.rs`:
- Line 151: Update the test using fresh_scope_stack so the owned FfiScopeStack
is explicitly released with nemo_relay_scope_stack_free at the end of the test,
matching the cleanup performed by sibling tests and preserving the thread
binding.
In `@go/nemo_relay/async_middleware_test.go`:
- Around line 22-65: Extend TestAsyncMiddlewareGlobalRegistrationParity to
assert duplicate registration errors and missing-name deregistration errors for
every registration family, while retaining successful register/deregister
coverage. Add focused async middleware tests covering priority ordering with
priorities 0 and 10, and cancellation of a pending completion through
context.Context; use the existing async registration and deregistration APIs and
callbacks, and verify the documented error and execution behavior.
In `@go/nemo_relay/callbacks.go`:
- Around line 194-217: Replace the per-invocation goroutine and fixed 10 ms
ticker in contextForCompletion with push-based cancellation via a registered
cancel callback exposed by the C ABI. Ensure the callback cancels the returned
context and is unregistered or safely released by the cleanup function,
including normal completion, while preserving idempotent cleanup through
doneOnce.
- Around line 683-701: Fix closure-token cleanup in goAsyncNextResultTrampoline
and the async result flow around the registration at lines 733-744. Unregister
userData before returning when the type assertion fails, and explicitly
unregister the registered token after the result-waiting select completes so
tokens are released even when Rust never invokes the callback. Preserve callback
handling while ensuring cleanup is safe if the trampoline already unregistered
the token.
- Around line 715-751: In the goroutine cleanup around contextForCompletion,
call cancel() before acquiring nextMu to set nextOpen=false and release next.
Preserve the existing deferred cleanup structure otherwise, ensuring in-flight
nextFn calls waiting on ctx.Done() can exit before the write lock is taken.
In `@go/nemo_relay/nemo_relay.go`:
- Around line 1243-1259: Make withScopeAsyncMiddleware generic over
AsyncMiddlewareFunc and AsyncExecutionInterceptFunc so the supplied closure type
and corresponding C callback are selected together, preventing mismatched
registrations. Add a withGlobalAsyncMiddleware helper with the same typed
behavior, then update all fourteen global Register*Async functions to delegate
to it instead of duplicating registration bodies.
---
Outside diff comments:
In `@crates/ffi/build.rs`:
- Around line 12-33: Update the cbindgen generation block to make `.generate()`
failure fatal with `.expect(...)` instead of silently skipping header injection.
Replace the hard-coded `marker` in the header rewrite with a trailer derived
from the configured include guard (or use a documented `bindings.write` trailer
approach), preserving correct behavior if formatting changes or `pragma_once` is
enabled.
🪄 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: 03fd7496-905c-48e5-b960-d7ed120fb0ca
📒 Files selected for processing (18)
crates/ffi/build.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/event_registry.rscrates/ffi/src/api/llm_registry.rscrates/ffi/src/api/mod.rscrates/ffi/src/api/scope_registry.rscrates/ffi/src/api/tool_registry.rscrates/ffi/src/callable.rscrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/tests/unit/callable_tests.rsgo/nemo_relay/adaptive_runtime_test.gogo/nemo_relay/async_middleware_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.gogo/nemo_relay/optimization_test.go
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
⚠️ CI failures not shown inline (1)
Commit Status: Branch Checker: Branch Checker
Conclusion: failure
Base branch is not under active development
🧰 Additional context used
📓 Path-based instructions (22)
**/*.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/ffi/src/api/mod.rscrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/callable_tests.rscrates/ffi/build.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/src/api/tool_registry.rscrates/ffi/src/api/llm_registry.rscrates/ffi/src/callable.rscrates/ffi/src/api/scope_registry.rs
crates/ffi/**
📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
Rebuild the FFI crate in release mode so the shared library and header stay in sync when making changes to crates/ffi
Files:
crates/ffi/src/api/mod.rscrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/callable_tests.rscrates/ffi/build.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/src/api/tool_registry.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/llm_registry.rscrates/ffi/src/callable.rscrates/ffi/src/api/scope_registry.rs
crates/ffi/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/ffi, also usetest-ffi-surfacefor validationUse C FFI export names prefixed with
nemo_relay_in the raw C FFI layer.
Files:
crates/ffi/src/api/mod.rscrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/callable_tests.rscrates/ffi/build.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/src/api/tool_registry.rscrates/ffi/src/api/llm_registry.rscrates/ffi/src/callable.rscrates/ffi/src/api/scope_registry.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/ffi/src/api/mod.rscrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/callable_tests.rscrates/ffi/build.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/src/api/tool_registry.rscrates/ffi/src/api/llm_registry.rscrates/ffi/src/callable.rscrates/ffi/src/api/scope_registry.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/ffi/src/api/mod.rscrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/callable_tests.rscrates/ffi/build.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/src/api/tool_registry.rscrates/ffi/src/api/llm_registry.rscrates/ffi/src/callable.rscrates/ffi/src/api/scope_registry.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/ffi/src/api/mod.rsgo/nemo_relay/optimization_test.gogo/nemo_relay/adaptive_runtime_test.gocrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/callable_tests.rsgo/nemo_relay/async_middleware_test.gocrates/ffi/build.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/src/api/tool_registry.rscrates/ffi/nemo_relay.hgo/nemo_relay/callbacks.gocrates/ffi/src/api/llm_registry.rscrates/ffi/src/callable.rscrates/ffi/src/api/scope_registry.rsgo/nemo_relay/nemo_relay.go
**/*.{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/ffi/src/api/mod.rsgo/nemo_relay/optimization_test.gogo/nemo_relay/adaptive_runtime_test.gocrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/callable_tests.rsgo/nemo_relay/async_middleware_test.gocrates/ffi/build.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/src/api/tool_registry.rsgo/nemo_relay/callbacks.gocrates/ffi/src/api/llm_registry.rscrates/ffi/src/callable.rscrates/ffi/src/api/scope_registry.rsgo/nemo_relay/nemo_relay.go
{crates/ffi/src/api/*.rs,crates/ffi/nemo_relay.h}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Add or update the shared C/FFI surface in the relevant
crates/ffi/src/api/*.rsmodule, re-export it throughcrates/ffi/src/api/mod.rs, and keep the generatedcrates/ffi/nemo_relay.hheader correct.
Files:
crates/ffi/src/api/mod.rscrates/ffi/src/api/event_registry.rscrates/ffi/src/api/tool_registry.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/llm_registry.rscrates/ffi/src/api/scope_registry.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/ffi/src/api/mod.rscrates/ffi/src/api/event_registry.rscrates/ffi/src/api/tool_registry.rscrates/ffi/src/api/llm_registry.rscrates/ffi/src/callable.rscrates/ffi/src/api/scope_registry.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/ffi/src/api/mod.rsgo/nemo_relay/optimization_test.gogo/nemo_relay/adaptive_runtime_test.gocrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/callable_tests.rsgo/nemo_relay/async_middleware_test.gocrates/ffi/build.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/src/api/tool_registry.rscrates/ffi/nemo_relay.hgo/nemo_relay/callbacks.gocrates/ffi/src/api/llm_registry.rscrates/ffi/src/callable.rscrates/ffi/src/api/scope_registry.rsgo/nemo_relay/nemo_relay.go
**/*.{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.
Files:
crates/ffi/src/api/mod.rsgo/nemo_relay/optimization_test.gogo/nemo_relay/adaptive_runtime_test.gocrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/callable_tests.rsgo/nemo_relay/async_middleware_test.gocrates/ffi/build.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/src/api/tool_registry.rsgo/nemo_relay/callbacks.gocrates/ffi/src/api/llm_registry.rscrates/ffi/src/callable.rscrates/ffi/src/api/scope_registry.rsgo/nemo_relay/nemo_relay.go
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/ffi/src/api/mod.rsgo/nemo_relay/optimization_test.gogo/nemo_relay/adaptive_runtime_test.gocrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/callable_tests.rsgo/nemo_relay/async_middleware_test.gocrates/ffi/build.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/src/api/tool_registry.rsgo/nemo_relay/callbacks.gocrates/ffi/src/api/llm_registry.rscrates/ffi/src/callable.rscrates/ffi/src/api/scope_registry.rsgo/nemo_relay/nemo_relay.go
crates/{python,ffi,node}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node}/**/*: Treat binding changes as public API changes. Check for parity with the other language bindings, FFI ownership/lifetime safety,
callback error propagation, stable type conversion, and consistent async/stream semantics.
Flag changes that update one binding without corresponding tests or documentation for the same surface elsewhere.
Files:
crates/ffi/src/api/mod.rscrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/callable_tests.rscrates/ffi/build.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/src/api/tool_registry.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/llm_registry.rscrates/ffi/src/callable.rscrates/ffi/src/api/scope_registry.rs
go/nemo_relay/**/*.go
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
go/nemo_relay/**/*.go: Format changed Go packages withcd go/nemo_relay && go fmt ./...
Run Go tests withjust test-goto build and test the NeMo Relay Go binding
Usejust build-gowhen you want an explicit build-only pass or need the artifact for other work
Usejust ci=true test-gowhen you need the CI-style coverage and JUnit path
On macOS, setDYLD_LIBRARY_PATHto the../../target/releasedirectory before running the rawgo testcommand directlyUse
PascalCasefor public Go APIs.
Files:
go/nemo_relay/optimization_test.gogo/nemo_relay/adaptive_runtime_test.gogo/nemo_relay/async_middleware_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
**/*.go
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When changing the experimental Go binding, format Go code with
gofmtand keepgo vet ./...passing.
Files:
go/nemo_relay/optimization_test.gogo/nemo_relay/adaptive_runtime_test.gogo/nemo_relay/async_middleware_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
{crates/python/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update the language-native bindings for every exposed surface in Python, Go, and Node.js.
Files:
go/nemo_relay/optimization_test.gogo/nemo_relay/adaptive_runtime_test.gogo/nemo_relay/async_middleware_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update language wrapper helpers such as Python wrapper modules, Python type stubs, and Go shorthand packages when the new behavior belongs in those helper layers.
Files:
go/nemo_relay/optimization_test.gogo/nemo_relay/adaptive_runtime_test.gogo/nemo_relay/async_middleware_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
**/*.{py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Keep Python, Go, and Node.js config objects and subscriber/exporter methods aligned so all bindings expose the same logical knobs and semantics.
Files:
go/nemo_relay/optimization_test.gogo/nemo_relay/adaptive_runtime_test.gogo/nemo_relay/async_middleware_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
go/nemo_relay/**
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep shared plugin helpers in
go/nemo_relayaligned with plugin registration, composition, and lifecycle behavior.
Files:
go/nemo_relay/optimization_test.gogo/nemo_relay/adaptive_runtime_test.gogo/nemo_relay/async_middleware_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
go/nemo_relay/**/*
⚙️ CodeRabbit configuration file
go/nemo_relay/**/*: Review Go binding changes for cgo memory ownership, race safety, callback cleanup, idiomatic exported APIs, and parity with Rust/FFI behavior.
Any API change should include focused Go tests and consider race-test behavior.
Files:
go/nemo_relay/optimization_test.gogo/nemo_relay/adaptive_runtime_test.gogo/nemo_relay/async_middleware_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
{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:
go/nemo_relay/optimization_test.gogo/nemo_relay/adaptive_runtime_test.gocrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/callable_tests.rsgo/nemo_relay/async_middleware_test.gocrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/tests/unit/api/registry_tests.rs
crates/ffi/nemo_relay.h
📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
Check the generated header diff when any exported symbol or type changed in the FFI surface
Update generated or generated-from-build surfaces such as
crates/ffi/nemo_relay.hthrough the proper build step.
Files:
crates/ffi/nemo_relay.h
🔇 Additional comments (19)
crates/ffi/tests/integration/api_tests.rs (1)
664-774: LGTM!Also applies to: 776-824
crates/ffi/tests/unit/api/coverage_sweeps_tests.rs (1)
16-31: LGTM!Also applies to: 38-149, 162-286
crates/ffi/tests/unit/api/registry_tests.rs (1)
172-174: LGTM!Also applies to: 334-334
crates/ffi/tests/unit/callable_private_tests.rs (1)
8-35: LGTM!Also applies to: 50-115, 117-139, 141-211, 213-266
crates/ffi/tests/unit/callable_tests.rs (1)
20-60: LGTM!Also applies to: 62-111, 113-203, 205-228, 230-258
go/nemo_relay/adaptive_runtime_test.go (1)
173-210: LGTM!Also applies to: 212-237
go/nemo_relay/async_middleware_test.go (1)
1-20: LGTM!Also applies to: 141-177, 179-224
go/nemo_relay/optimization_test.go (1)
101-119: LGTM!crates/ffi/nemo_relay.h (1)
135-148: LGTM!Also applies to: 258-267, 467-486, 2667-2708, 3212-3243
crates/ffi/src/callable.rs (2)
56-161: LGTM!
1347-1352: 🎯 Functional CorrectnessBehavior change on existing sync sanitizers: codec identity failure now hard-errors.
ffi_codec_identityfailure previously yieldedOk(None)(skip sanitization); it now aborts the LLM request withFlowError. Only reachable case today is a runtime codec ID containing an embedded NUL, but this is a semantic change to a shipped surface and is unrelated to the async feature. Confirm it is intentional and covered by a test.#!/bin/bash rg -nP -C4 'embedded NUL|ffi_codec_identity' crates/ffi fd . crates/ffi/tests -e rs --exec rg -nP -C3 'codec.*NUL|InvalidArgument'Also applies to: 1391-1396
crates/ffi/src/api/event_registry.rs (1)
77-105: LGTM!Also applies to: 201-232
crates/ffi/src/api/mod.rs (1)
17-33: LGTM!crates/ffi/src/api/scope_registry.rs (1)
70-112: LGTM!Also applies to: 143-157
crates/ffi/build.rs (1)
36-105: LGTM!Also applies to: 107-133, 135-166
go/nemo_relay/callbacks.go (2)
53-62: LGTM!Also applies to: 182-192
642-676: LGTM!Also applies to: 678-681
go/nemo_relay/nemo_relay.go (2)
47-50: LGTM! Callback return type (uint32_t) and pointer constness match the Rust#[repr(u32)] NemoRelayAsyncCallbackStateand the generated header typedefs.Also applies to: 128-244, 311-312
1266-1269: 📐 Maintainability & Code QualityConfirm Python/Node parity and docs for the async surface.
This adds ~28 new exported async entry points to the Go binding and the C ABI with no counterpart in
crates/python,crates/node, or the Python stubs. Per the repo's binding rules, an exposed surface should land in every language binding or be explicitly documented as C/Go-only with a follow-up.As per coding guidelines, "Update the language-native bindings for every exposed surface in Python, Go, and Node.js."
#!/bin/bash rg -nP --iglob '!go/**' -C2 '_async\b|Async\(' crates/python crates/node python/nemo_relay | head -50 fd . docs -e md --exec rg -ln 'async middleware|Async middleware' 2>/dev/nullAlso applies to: 2505-2510
7934489 to
9be032b
Compare
c3a940f to
74c253c
Compare
Signed-off-by: Will Killian <wkillian@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
go/nemo_relay/adaptive_runtime_test.go (1)
231-231: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
defer runtime.Shutdown()drops its error, unlike Lines 46, 119-121, and 161.This is the same unchecked-error pattern that was fixed elsewhere in this file; it will trip
errcheckunder most golangci-lint configs.♻️ Match the file convention
- defer runtime.Shutdown() + defer func() { _ = runtime.Shutdown() }()🤖 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 `@go/nemo_relay/adaptive_runtime_test.go` at line 231, Update the deferred cleanup around runtime.Shutdown in the affected test to handle or explicitly propagate its returned error, matching the existing error-handling convention at the other runtime.Shutdown call sites in this file and satisfying errcheck.
🤖 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/ffi/nemo_relay.h`:
- Around line 2684-2687: Update the Rust doc comment for
nemo_relay_async_completion_is_cancelled in callable.rs to explicitly state that
a null completion is reported as cancelled, then regenerate the FFI header so
the generated declaration documents this behavior.
In `@crates/ffi/tests/integration/api_tests.rs`:
- Line 666: Update the TEST_MUTEX acquisition in both affected tests to use the
poison-tolerant unwrap_or_else(|e| e.into_inner()) convention, matching sibling
tests and allowing execution to continue after a prior test panic.
---
Duplicate comments:
In `@go/nemo_relay/adaptive_runtime_test.go`:
- Line 231: Update the deferred cleanup around runtime.Shutdown in the affected
test to handle or explicitly propagate its returned error, matching the existing
error-handling convention at the other runtime.Shutdown call sites in this file
and satisfying errcheck.
🪄 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: 505be7aa-8ccc-4e6d-9fe1-33caf462cf1e
📒 Files selected for processing (18)
crates/ffi/build.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/event_registry.rscrates/ffi/src/api/llm_registry.rscrates/ffi/src/api/mod.rscrates/ffi/src/api/scope_registry.rscrates/ffi/src/api/tool_registry.rscrates/ffi/src/callable.rscrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/tests/unit/callable_tests.rsgo/nemo_relay/adaptive_runtime_test.gogo/nemo_relay/async_middleware_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.gogo/nemo_relay/optimization_test.go
📜 Review details
⏰ Context from checks skipped due to timeout. (8)
- GitHub Check: Rust / Test (windows-amd64)
- GitHub Check: Rust / Test (linux-arm64)
- GitHub Check: Rust / Test (macos-arm64)
- GitHub Check: Go / Test (windows-amd64)
- GitHub Check: Go / Test (windows-arm64)
- GitHub Check: Rust / Test (linux-amd64)
- GitHub Check: Go / Test (macos-arm64)
- GitHub Check: Rust / Test (windows-arm64)
⚠️ CI failures not shown inline (1)
Commit Status: Branch Checker: Branch Checker
Conclusion: failure
Base branch is not under active development
🧰 Additional context used
📓 Path-based instructions (22)
go/nemo_relay/**/*.go
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
go/nemo_relay/**/*.go: Format changed Go packages withcd go/nemo_relay && go fmt ./...
Run Go tests withjust test-goto build and test the NeMo Relay Go binding
Usejust build-gowhen you want an explicit build-only pass or need the artifact for other work
Usejust ci=true test-gowhen you need the CI-style coverage and JUnit path
On macOS, setDYLD_LIBRARY_PATHto the../../target/releasedirectory before running the rawgo testcommand directlyUse
PascalCasefor public Go APIs.
Files:
go/nemo_relay/optimization_test.gogo/nemo_relay/async_middleware_test.gogo/nemo_relay/adaptive_runtime_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
**/*.go
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When changing the experimental Go binding, format Go code with
gofmtand keepgo vet ./...passing.
Files:
go/nemo_relay/optimization_test.gogo/nemo_relay/async_middleware_test.gogo/nemo_relay/adaptive_runtime_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
**/*.{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:
go/nemo_relay/optimization_test.gocrates/ffi/tests/integration/api_tests.rscrates/ffi/src/api/mod.rscrates/ffi/src/api/tool_registry.rscrates/ffi/tests/unit/api/registry_tests.rsgo/nemo_relay/async_middleware_test.gocrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/llm_registry.rscrates/ffi/nemo_relay.hgo/nemo_relay/adaptive_runtime_test.gocrates/ffi/src/api/scope_registry.rscrates/ffi/build.rscrates/ffi/src/callable.rscrates/ffi/tests/unit/callable_tests.rsgo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
**/*.{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:
go/nemo_relay/optimization_test.gocrates/ffi/tests/integration/api_tests.rscrates/ffi/src/api/mod.rscrates/ffi/src/api/tool_registry.rscrates/ffi/tests/unit/api/registry_tests.rsgo/nemo_relay/async_middleware_test.gocrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/llm_registry.rsgo/nemo_relay/adaptive_runtime_test.gocrates/ffi/src/api/scope_registry.rscrates/ffi/build.rscrates/ffi/src/callable.rscrates/ffi/tests/unit/callable_tests.rsgo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
{crates/python/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update the language-native bindings for every exposed surface in Python, Go, and Node.js.
Files:
go/nemo_relay/optimization_test.gogo/nemo_relay/async_middleware_test.gogo/nemo_relay/adaptive_runtime_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update language wrapper helpers such as Python wrapper modules, Python type stubs, and Go shorthand packages when the new behavior belongs in those helper layers.
Files:
go/nemo_relay/optimization_test.gogo/nemo_relay/async_middleware_test.gogo/nemo_relay/adaptive_runtime_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
**/*.{py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Keep Python, Go, and Node.js config objects and subscriber/exporter methods aligned so all bindings expose the same logical knobs and semantics.
Files:
go/nemo_relay/optimization_test.gogo/nemo_relay/async_middleware_test.gogo/nemo_relay/adaptive_runtime_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
go/nemo_relay/**
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep shared plugin helpers in
go/nemo_relayaligned with plugin registration, composition, and lifecycle behavior.
Files:
go/nemo_relay/optimization_test.gogo/nemo_relay/async_middleware_test.gogo/nemo_relay/adaptive_runtime_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
**/*
📄 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:
go/nemo_relay/optimization_test.gocrates/ffi/tests/integration/api_tests.rscrates/ffi/src/api/mod.rscrates/ffi/src/api/tool_registry.rscrates/ffi/tests/unit/api/registry_tests.rsgo/nemo_relay/async_middleware_test.gocrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/llm_registry.rscrates/ffi/nemo_relay.hgo/nemo_relay/adaptive_runtime_test.gocrates/ffi/src/api/scope_registry.rscrates/ffi/build.rscrates/ffi/src/callable.rscrates/ffi/tests/unit/callable_tests.rsgo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
**/*.{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.
Files:
go/nemo_relay/optimization_test.gocrates/ffi/tests/integration/api_tests.rscrates/ffi/src/api/mod.rscrates/ffi/src/api/tool_registry.rscrates/ffi/tests/unit/api/registry_tests.rsgo/nemo_relay/async_middleware_test.gocrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/llm_registry.rsgo/nemo_relay/adaptive_runtime_test.gocrates/ffi/src/api/scope_registry.rscrates/ffi/build.rscrates/ffi/src/callable.rscrates/ffi/tests/unit/callable_tests.rsgo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
go/nemo_relay/optimization_test.gocrates/ffi/tests/integration/api_tests.rscrates/ffi/src/api/mod.rscrates/ffi/src/api/tool_registry.rscrates/ffi/tests/unit/api/registry_tests.rsgo/nemo_relay/async_middleware_test.gocrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/llm_registry.rsgo/nemo_relay/adaptive_runtime_test.gocrates/ffi/src/api/scope_registry.rscrates/ffi/build.rscrates/ffi/src/callable.rscrates/ffi/tests/unit/callable_tests.rsgo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
go/nemo_relay/**/*
⚙️ CodeRabbit configuration file
go/nemo_relay/**/*: Review Go binding changes for cgo memory ownership, race safety, callback cleanup, idiomatic exported APIs, and parity with Rust/FFI behavior.
Any API change should include focused Go tests and consider race-test behavior.
Files:
go/nemo_relay/optimization_test.gogo/nemo_relay/async_middleware_test.gogo/nemo_relay/adaptive_runtime_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
{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:
go/nemo_relay/optimization_test.gocrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/api/registry_tests.rsgo/nemo_relay/async_middleware_test.gocrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rsgo/nemo_relay/adaptive_runtime_test.gocrates/ffi/tests/unit/callable_tests.rs
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node 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/ffi/tests/integration/api_tests.rscrates/ffi/src/api/mod.rscrates/ffi/src/api/tool_registry.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/llm_registry.rscrates/ffi/src/api/scope_registry.rscrates/ffi/build.rscrates/ffi/src/callable.rscrates/ffi/tests/unit/callable_tests.rs
crates/ffi/**
📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
Rebuild the FFI crate in release mode so the shared library and header stay in sync when making changes to crates/ffi
Files:
crates/ffi/tests/integration/api_tests.rscrates/ffi/src/api/mod.rscrates/ffi/src/api/tool_registry.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/llm_registry.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/scope_registry.rscrates/ffi/build.rscrates/ffi/src/callable.rscrates/ffi/tests/unit/callable_tests.rs
crates/ffi/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/ffi, also usetest-ffi-surfacefor validationUse C FFI export names prefixed with
nemo_relay_in the raw C FFI layer.
Files:
crates/ffi/tests/integration/api_tests.rscrates/ffi/src/api/mod.rscrates/ffi/src/api/tool_registry.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/llm_registry.rscrates/ffi/src/api/scope_registry.rscrates/ffi/build.rscrates/ffi/src/callable.rscrates/ffi/tests/unit/callable_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/ffi/tests/integration/api_tests.rscrates/ffi/src/api/mod.rscrates/ffi/src/api/tool_registry.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/llm_registry.rscrates/ffi/src/api/scope_registry.rscrates/ffi/build.rscrates/ffi/src/callable.rscrates/ffi/tests/unit/callable_tests.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/ffi/tests/integration/api_tests.rscrates/ffi/src/api/mod.rscrates/ffi/src/api/tool_registry.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/llm_registry.rscrates/ffi/src/api/scope_registry.rscrates/ffi/build.rscrates/ffi/src/callable.rscrates/ffi/tests/unit/callable_tests.rs
crates/{python,ffi,node}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node}/**/*: Treat binding changes as public API changes. Check for parity with the other language bindings, FFI ownership/lifetime safety,
callback error propagation, stable type conversion, and consistent async/stream semantics.
Flag changes that update one binding without corresponding tests or documentation for the same surface elsewhere.
Files:
crates/ffi/tests/integration/api_tests.rscrates/ffi/src/api/mod.rscrates/ffi/src/api/tool_registry.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/event_registry.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/llm_registry.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/scope_registry.rscrates/ffi/build.rscrates/ffi/src/callable.rscrates/ffi/tests/unit/callable_tests.rs
{crates/ffi/src/api/*.rs,crates/ffi/nemo_relay.h}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Add or update the shared C/FFI surface in the relevant
crates/ffi/src/api/*.rsmodule, re-export it throughcrates/ffi/src/api/mod.rs, and keep the generatedcrates/ffi/nemo_relay.hheader correct.
Files:
crates/ffi/src/api/mod.rscrates/ffi/src/api/tool_registry.rscrates/ffi/src/api/event_registry.rscrates/ffi/src/api/llm_registry.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/scope_registry.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/ffi/src/api/mod.rscrates/ffi/src/api/tool_registry.rscrates/ffi/src/api/event_registry.rscrates/ffi/src/api/llm_registry.rscrates/ffi/src/api/scope_registry.rscrates/ffi/src/callable.rs
crates/ffi/nemo_relay.h
📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
Check the generated header diff when any exported symbol or type changed in the FFI surface
Update generated or generated-from-build surfaces such as
crates/ffi/nemo_relay.hthrough the proper build step.
Files:
crates/ffi/nemo_relay.h
🧠 Learnings (1)
📚 Learning: 2026-07-28T20:33:25.156Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 572
File: go/nemo_relay/adaptive_runtime_test.go:214-238
Timestamp: 2026-07-28T20:33:25.156Z
Learning: When adding/adjusting Go unit tests for `BuildCacheRequestFacts` (request-ID validation and related request parsing), set `CacheRequestFactsInput.Provider` to a valid provider in all tests that are intended to isolate request-ID behavior—because `BuildCacheRequestFacts` does not validate `Provider`. Then add separate test coverage for malformed `AnnotatedRequest` JSON so JSON parsing failures are not conflated with `Provider`-related inputs.
Applied to files:
go/nemo_relay/optimization_test.gogo/nemo_relay/async_middleware_test.gogo/nemo_relay/adaptive_runtime_test.go
🔇 Additional comments (19)
crates/ffi/nemo_relay.h (1)
243-251: LGTM!Also applies to: 463-473, 2646-2682, 3197-3233
crates/ffi/tests/integration/api_tests.rs (1)
668-796: LGTM!Also applies to: 802-860
crates/ffi/tests/unit/api/coverage_sweeps_tests.rs (1)
16-31: LGTM!Also applies to: 33-150, 151-288
crates/ffi/tests/unit/api/registry_tests.rs (1)
172-173: LGTM! The7at Line 338 correctly asserts that popping the owning scope frees nothing new, guarding against a double destructor invocation.Also applies to: 338-338
crates/ffi/tests/unit/callable_private_tests.rs (1)
8-88: LGTM!Also applies to: 101-210, 212-235, 237-281, 283-314, 316-420, 422-506
crates/ffi/tests/unit/callable_tests.rs (1)
21-62: LGTM! Kind 7 assertingInternalfromnemo_relay_flush_subscribersinside the sanitizer is a good cross-layer check of the would-block contract documented incrates/ffi/nemo_relay.hLine 1299.Also applies to: 64-84, 86-137, 139-229, 231-294, 296-319, 321-350, 806-816, 818-860
go/nemo_relay/adaptive_runtime_test.go (1)
8-8: LGTM! The provider/request-ID isolation matches the documentedBuildCacheRequestFactsbehavior, with malformed JSON covered separately.Based on learnings:
BuildCacheRequestFactsdoes not validateCacheRequestFactsInput.Provider, so tests isolating request-ID behavior should use a valid provider and cover malformedAnnotatedRequestJSON separately.Also applies to: 46-46, 161-161, 173-211, 213-230, 232-244
Source: Learnings
go/nemo_relay/async_middleware_test.go (1)
1-22: LGTM! The 14-family global and scope-local tables mirror the async exports declared incrates/ffi/nemo_relay.hLines 3206-3233, and the detached-nexttest correctly joins the continuation and unblocks the tool implementation on every exit path viasync.Once.As per coding guidelines, this covers "registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, ... and parity across affected bindings."
Also applies to: 24-74, 76-154, 156-189, 191-227, 229-274, 276-315, 317-374
Source: Coding guidelines
go/nemo_relay/optimization_test.go (1)
101-119: LGTM! The name now matches the coverage, and unknown top-level fields are correctly left out of the rejection contract.Based on learnings: unknown top-level fields in
LLMOptimizationContributionare intentionally preserved inExtrafor forward-compatible round trips.Source: Learnings
crates/ffi/build.rs (2)
45-67:REGISTRATION_SOURCESremains a hardcoded allowlist, so a futuresrc/api/*_registry.rsthat uses these macros is silently absent from bothexpectedandASYNC_REGISTRATIONS, and the parity assert still passes. Same concern as the earlier review; enumeratingsrc/apiwould close it.
8-34: LGTM!Also applies to: 88-230, 232-269
crates/ffi/src/callable.rs (1)
60-155: LGTM!Also applies to: 194-542, 811-1063, 1582-1597
crates/ffi/src/api/mod.rs (1)
74-135: LGTM!crates/ffi/src/api/event_registry.rs (1)
47-64: LGTM!Also applies to: 125-142
crates/ffi/src/api/llm_registry.rs (1)
17-54: LGTM!Also applies to: 437-448
crates/ffi/src/api/scope_registry.rs (1)
33-101: LGTM!crates/ffi/src/api/tool_registry.rs (1)
13-45: LGTM!go/nemo_relay/nemo_relay.go (1)
47-50: LGTM!Also applies to: 128-244, 311-312, 1243-1267, 1327-1654, 2444-2523, 2722-2762
go/nemo_relay/callbacks.go (1)
53-62: LGTM!Also applies to: 118-176, 192-287, 712-865
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
crates/ffi/tests/integration/api_tests.rs (1)
666-666: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the poison-tolerant lock convention for
TEST_MUTEX.Still
.unwrap()here (and Line 800), while sibling suites use.unwrap_or_else(|e| e.into_inner())(crates/ffi/tests/unit/api/registry_tests.rsLine 111,crates/ffi/tests/unit/api/coverage_sweeps_tests.rsLine 44). One earlier panicking test turns these into unrelatedPoisonErrorfailures.♻️ Proposed change
- let _guard = TEST_MUTEX.lock().unwrap(); + let _guard = TEST_MUTEX.lock().unwrap_or_else(|error| error.into_inner());Also applies to: 800-800
🤖 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/ffi/tests/integration/api_tests.rs` at line 666, Update both TEST_MUTEX lock acquisitions in the integration tests to use the poison-tolerant unwrap_or_else(|e| e.into_inner()) convention instead of unwrap(), preserving the existing _guard bindings.crates/ffi/build.rs (1)
46-51: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winHardcoded source allowlist keeps a silent-drift hole open.
A future
src/api/*_registry.rsthat usesglobal_async_registration!/scope_async_registration!contributes nothing toexpected, so its Rust export ships with no C declaration andassert_eq!(declared, expected)still passes — the exact failure this validator exists to catch. Enumeratesrc/api/*_registry.rsfrom the directory instead.♻️ Enumerate the registry modules
- const REGISTRATION_SOURCES: &[&str] = &[ - "src/api/event_registry.rs", - "src/api/llm_registry.rs", - "src/api/scope_registry.rs", - "src/api/tool_registry.rs", - ]; - println!("cargo:rerun-if-changed=cbindgen.toml"); println!("cargo:rerun-if-changed=src"); + + let api_dir = format!("{crate_dir}/src/api"); + let registration_sources = std::fs::read_dir(&api_dir) + .unwrap_or_else(|error| panic!("read {api_dir}: {error}")) + .map(|entry| entry.expect("read src/api entry").path()) + .filter(|path| { + path.file_name() + .and_then(|name| name.to_str()) + .is_some_and(|name| name.ends_with("_registry.rs")) + }) + .collect::<Vec<_>>();Then iterate
registration_sourcesin place of the string list.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ffi/build.rs` around lines 46 - 51, Replace the hardcoded REGISTRATION_SOURCES allowlist with directory-based enumeration of every src/api/*_registry.rs file, and build registration_sources from those discovered paths. Update the validator loop to iterate registration_sources so newly added registry modules contribute to expected declarations and cannot silently bypass validation.
🤖 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/ffi/src/callable.rs`:
- Around line 1872-1886: The sanitizer JSON produced by ffi_codec_identity_json
does not match the documented LlmCodecIdentity context contract because
codec_kind is emitted as an undocumented integer. Update ffi_codec_identity_json
to serialize codec_kind using the established string representations none,
builtin, runtime, and opaque, or alternatively revise the ASYNC_REGISTRATIONS
contract comment to explicitly document the discriminant values; keep codec_id
unchanged.
In `@go/nemo_relay/async_middleware_test.go`:
- Around line 276-289: Strengthen the assertions in the stream-reading test
around stream.Next and chunks so they verify the exact expected incremental
payloads in emitted order, rather than only checking len(chunks) == 2. Preserve
the existing EOF and read-error handling, and compare each collected
json.RawMessage against the corresponding emitted value.
In `@go/nemo_relay/callbacks.go`:
- Around line 1034-1086: Add async middleware tests covering scope-local
inheritance and cleanup around the async tool execution intercept: register a
guardrail on a parent scope, verify it runs for a child-scope invocation, then
pop the owning scope and verify subsequent invocations no longer run it. Reuse
the existing async registration, scope, and invocation helpers in
async_middleware_test.go, preserving coverage for duplicate names and idempotent
deregistration.
---
Duplicate comments:
In `@crates/ffi/build.rs`:
- Around line 46-51: Replace the hardcoded REGISTRATION_SOURCES allowlist with
directory-based enumeration of every src/api/*_registry.rs file, and build
registration_sources from those discovered paths. Update the validator loop to
iterate registration_sources so newly added registry modules contribute to
expected declarations and cannot silently bypass validation.
In `@crates/ffi/tests/integration/api_tests.rs`:
- Line 666: Update both TEST_MUTEX lock acquisitions in the integration tests to
use the poison-tolerant unwrap_or_else(|e| e.into_inner()) convention instead of
unwrap(), preserving the existing _guard bindings.
🪄 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: 27f79d97-3fb6-4a68-aa4b-ef4274326cf5
📒 Files selected for processing (17)
crates/ffi/build.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/event_registry.rscrates/ffi/src/api/llm_registry.rscrates/ffi/src/api/mod.rscrates/ffi/src/api/scope_registry.rscrates/ffi/src/api/tool_registry.rscrates/ffi/src/callable.rscrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/tests/unit/callable_tests.rsgo/nemo_relay/README.mdgo/nemo_relay/async_middleware_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
⚠️ CI failures not shown inline (1)
Commit Status: Branch Checker: Branch Checker
Conclusion: failure
Base branch is not under active development
🧰 Additional context used
📓 Path-based instructions (29)
**/*.{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:
go/nemo_relay/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:
go/nemo_relay/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:
go/nemo_relay/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:
go/nemo_relay/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:
go/nemo_relay/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:
go/nemo_relay/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:
go/nemo_relay/README.md
go/nemo_relay/**
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep shared plugin helpers in
go/nemo_relayaligned with plugin registration, composition, and lifecycle behavior.
Files:
go/nemo_relay/README.mdgo/nemo_relay/async_middleware_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
**/*
📄 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:
go/nemo_relay/README.mdcrates/ffi/src/api/event_registry.rscrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/scope_registry.rscrates/ffi/src/api/tool_registry.rsgo/nemo_relay/async_middleware_test.gocrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/mod.rscrates/ffi/nemo_relay.hcrates/ffi/tests/unit/callable_tests.rscrates/ffi/build.rsgo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.gocrates/ffi/src/callable.rscrates/ffi/src/api/llm_registry.rs
go/nemo_relay/**/*
⚙️ CodeRabbit configuration file
go/nemo_relay/**/*: Review Go binding changes for cgo memory ownership, race safety, callback cleanup, idiomatic exported APIs, and parity with Rust/FFI behavior.
Any API change should include focused Go tests and consider race-test behavior.
Files:
go/nemo_relay/README.mdgo/nemo_relay/async_middleware_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
**/*.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/ffi/src/api/event_registry.rscrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/scope_registry.rscrates/ffi/src/api/tool_registry.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/mod.rscrates/ffi/tests/unit/callable_tests.rscrates/ffi/build.rscrates/ffi/src/callable.rscrates/ffi/src/api/llm_registry.rs
crates/ffi/**
📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
Rebuild the FFI crate in release mode so the shared library and header stay in sync when making changes to crates/ffi
Files:
crates/ffi/src/api/event_registry.rscrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/scope_registry.rscrates/ffi/src/api/tool_registry.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/mod.rscrates/ffi/nemo_relay.hcrates/ffi/tests/unit/callable_tests.rscrates/ffi/build.rscrates/ffi/src/callable.rscrates/ffi/src/api/llm_registry.rs
crates/ffi/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/ffi, also usetest-ffi-surfacefor validationUse C FFI export names prefixed with
nemo_relay_in the raw C FFI layer.
Files:
crates/ffi/src/api/event_registry.rscrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/scope_registry.rscrates/ffi/src/api/tool_registry.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/mod.rscrates/ffi/tests/unit/callable_tests.rscrates/ffi/build.rscrates/ffi/src/callable.rscrates/ffi/src/api/llm_registry.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/ffi/src/api/event_registry.rscrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/scope_registry.rscrates/ffi/src/api/tool_registry.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/mod.rscrates/ffi/tests/unit/callable_tests.rscrates/ffi/build.rscrates/ffi/src/callable.rscrates/ffi/src/api/llm_registry.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/ffi/src/api/event_registry.rscrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/scope_registry.rscrates/ffi/src/api/tool_registry.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/mod.rscrates/ffi/tests/unit/callable_tests.rscrates/ffi/build.rscrates/ffi/src/callable.rscrates/ffi/src/api/llm_registry.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/ffi/src/api/event_registry.rscrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/scope_registry.rscrates/ffi/src/api/tool_registry.rsgo/nemo_relay/async_middleware_test.gocrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/mod.rscrates/ffi/nemo_relay.hcrates/ffi/tests/unit/callable_tests.rscrates/ffi/build.rsgo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.gocrates/ffi/src/callable.rscrates/ffi/src/api/llm_registry.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/ffi/src/api/event_registry.rscrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/scope_registry.rscrates/ffi/src/api/tool_registry.rsgo/nemo_relay/async_middleware_test.gocrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/mod.rscrates/ffi/tests/unit/callable_tests.rscrates/ffi/build.rsgo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.gocrates/ffi/src/callable.rscrates/ffi/src/api/llm_registry.rs
{crates/ffi/src/api/*.rs,crates/ffi/nemo_relay.h}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Add or update the shared C/FFI surface in the relevant
crates/ffi/src/api/*.rsmodule, re-export it throughcrates/ffi/src/api/mod.rs, and keep the generatedcrates/ffi/nemo_relay.hheader correct.
Files:
crates/ffi/src/api/event_registry.rscrates/ffi/src/api/scope_registry.rscrates/ffi/src/api/tool_registry.rscrates/ffi/src/api/mod.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/llm_registry.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/ffi/src/api/event_registry.rscrates/ffi/src/api/scope_registry.rscrates/ffi/src/api/tool_registry.rscrates/ffi/src/api/mod.rscrates/ffi/src/callable.rscrates/ffi/src/api/llm_registry.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.
Files:
crates/ffi/src/api/event_registry.rscrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/scope_registry.rscrates/ffi/src/api/tool_registry.rsgo/nemo_relay/async_middleware_test.gocrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/mod.rscrates/ffi/tests/unit/callable_tests.rscrates/ffi/build.rsgo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.gocrates/ffi/src/callable.rscrates/ffi/src/api/llm_registry.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/ffi/src/api/event_registry.rscrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/scope_registry.rscrates/ffi/src/api/tool_registry.rsgo/nemo_relay/async_middleware_test.gocrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/mod.rscrates/ffi/tests/unit/callable_tests.rscrates/ffi/build.rsgo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.gocrates/ffi/src/callable.rscrates/ffi/src/api/llm_registry.rs
crates/{python,ffi,node}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node}/**/*: Treat binding changes as public API changes. Check for parity with the other language bindings, FFI ownership/lifetime safety,
callback error propagation, stable type conversion, and consistent async/stream semantics.
Flag changes that update one binding without corresponding tests or documentation for the same surface elsewhere.
Files:
crates/ffi/src/api/event_registry.rscrates/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rscrates/ffi/src/api/scope_registry.rscrates/ffi/src/api/tool_registry.rscrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/src/api/mod.rscrates/ffi/nemo_relay.hcrates/ffi/tests/unit/callable_tests.rscrates/ffi/build.rscrates/ffi/src/callable.rscrates/ffi/src/api/llm_registry.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/ffi/tests/integration/api_tests.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/api/coverage_sweeps_tests.rsgo/nemo_relay/async_middleware_test.gocrates/ffi/tests/unit/callable_private_tests.rscrates/ffi/tests/unit/callable_tests.rs
go/nemo_relay/**/*.go
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
go/nemo_relay/**/*.go: Format changed Go packages withcd go/nemo_relay && go fmt ./...
Run Go tests withjust test-goto build and test the NeMo Relay Go binding
Usejust build-gowhen you want an explicit build-only pass or need the artifact for other work
Usejust ci=true test-gowhen you need the CI-style coverage and JUnit path
On macOS, setDYLD_LIBRARY_PATHto the../../target/releasedirectory before running the rawgo testcommand directlyUse
PascalCasefor public Go APIs.
Files:
go/nemo_relay/async_middleware_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
**/*.go
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When changing the experimental Go binding, format Go code with
gofmtand keepgo vet ./...passing.
Files:
go/nemo_relay/async_middleware_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
{crates/python/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update the language-native bindings for every exposed surface in Python, Go, and Node.js.
Files:
go/nemo_relay/async_middleware_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update language wrapper helpers such as Python wrapper modules, Python type stubs, and Go shorthand packages when the new behavior belongs in those helper layers.
Files:
go/nemo_relay/async_middleware_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
**/*.{py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Keep Python, Go, and Node.js config objects and subscriber/exporter methods aligned so all bindings expose the same logical knobs and semantics.
Files:
go/nemo_relay/async_middleware_test.gogo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.go
crates/ffi/nemo_relay.h
📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
Check the generated header diff when any exported symbol or type changed in the FFI surface
Update generated or generated-from-build surfaces such as
crates/ffi/nemo_relay.hthrough the proper build step.
Files:
crates/ffi/nemo_relay.h
🧠 Learnings (1)
📚 Learning: 2026-07-28T20:33:25.156Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 572
File: go/nemo_relay/adaptive_runtime_test.go:214-238
Timestamp: 2026-07-28T20:33:25.156Z
Learning: When adding/adjusting Go unit tests for `BuildCacheRequestFacts` (request-ID validation and related request parsing), set `CacheRequestFactsInput.Provider` to a valid provider in all tests that are intended to isolate request-ID behavior—because `BuildCacheRequestFacts` does not validate `Provider`. Then add separate test coverage for malformed `AnnotatedRequest` JSON so JSON parsing failures are not conflated with `Provider`-related inputs.
Applied to files:
go/nemo_relay/async_middleware_test.go
🔇 Additional comments (24)
crates/ffi/tests/integration/api_tests.rs (1)
668-796: LGTM!Also applies to: 802-859
crates/ffi/tests/unit/api/coverage_sweeps_tests.rs (1)
16-40: LGTM!Also applies to: 42-177, 178-337
crates/ffi/tests/unit/api/registry_tests.rs (1)
246-247: LGTM!Also applies to: 412-412
crates/ffi/tests/unit/callable_private_tests.rs (2)
7-124: LGTM!Also applies to: 138-231, 233-302, 304-422, 424-553, 555-639, 641-729
730-733: 🩺 Stability & AvailabilityNo callback-state UAF here.
nemo_relay_async_stream_invocation_cancelwaits oncallback_gate, andinvoke_async_next_stream_callbackrecheckscancelledunder the same gate before touchinguser_data, so droppingcallback_stateaftercancel_done_rxis safe.> Likely an incorrect or invalid review comment.crates/ffi/tests/unit/callable_tests.rs (1)
21-152: LGTM!Also applies to: 154-244, 246-309, 311-365, 846-846, 864-864
go/nemo_relay/README.md (1)
64-93: LGTM!go/nemo_relay/async_middleware_test.go (1)
17-83: LGTM!Also applies to: 85-163, 165-198, 200-236, 293-331, 333-403, 405-450, 452-491, 493-550
crates/ffi/src/callable.rs (1)
206-310: LGTM!Also applies to: 327-427, 459-565, 602-680, 1265-1353
crates/ffi/src/api/mod.rs (1)
74-135: LGTM!crates/ffi/src/api/event_registry.rs (1)
47-64: LGTM!Also applies to: 125-142
crates/ffi/src/api/llm_registry.rs (2)
17-54: LGTM!
437-439: 📐 Maintainability & Code QualityNo change needed for the reentrant flush docs. Reentrant
flush_subscribers()is a no-op that returnsOk(()), so the wording about returning before later queued callbacks finish matches the implementation.> Likely an incorrect or invalid review comment.crates/ffi/src/api/scope_registry.rs (1)
34-102: LGTM!crates/ffi/src/api/tool_registry.rs (1)
13-45: LGTM!crates/ffi/build.rs (1)
6-34: LGTM!Also applies to: 104-243
crates/ffi/nemo_relay.h (2)
468-490: LGTM!Also applies to: 2593-2695, 3189-3266
238-257: 🩺 Stability & AvailabilityNo duplicate typedefs in the generated header. The repeated async names only appear in callback prototypes, not as repeated declarations.
> Likely an incorrect or invalid review comment.go/nemo_relay/nemo_relay.go (3)
47-52: LGTM!Also applies to: 130-246, 304-306
2314-2336: LGTM!
1238-1251: 📐 Maintainability & Code QualityDrop the doc-gap request. The async surface is already covered in
go/nemo_relay/README.md,docs/reference/migration-guides.mdx, and the plugin READMEs, so this change doesn’t leave the new*AsyncAPIs undocumented.> Likely an incorrect or invalid review comment.go/nemo_relay/callbacks.go (3)
53-74: LGTM!Also applies to: 146-188, 204-321
746-899: LGTM!
901-1130: LGTM!
|
Closing this stack layer intentionally: completion-based middleware APIs for the raw C FFI and Go binding are out of scope. Those bindings retain their existing synchronous callbacks; async middleware remains in Rust, Python, Node.js, workers, and native plugins. The retained contract and native-thread blocking behavior are documented in PR #571. |
2 participants
Overview
Add completion-based asynchronous middleware registration to the raw C ABI and Go binding while preserving their existing synchronous APIs.
Note
Async C callbacks use one-shot completion handles with explicit resolve, reject, cancellation, duplicate-settlement, and late-settlement behavior. No implicit middleware timeout is introduced.
Details
nextcontinuations for execution and stream intercepts.Asyncvariants backed bycontext.Context, goroutines, and safe continuation helpers.This is PR 3 of 3 in GitHub stack #574. The stack is rooted on upstream
main, and this PR is based on #571:Where should the reviewer start?
Start with
crates/ffi/src/callable.rs, the public declarations incrates/ffi/nemo_relay.h, andgo/nemo_relay/callbacks.go.Validation:
cargo test -p nemo-relay-ffi --all-features -- --test-threads=1— 173 passedjust test-goRelated Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit