feat!: add async middleware across bindings - #564
Conversation
github-actions
Bot
added
size:XXL
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
🧰 Additional context used📓 Path-based instructions (11)**/*.rs📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Files:
crates/ffi/**📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
Files:
crates/ffi/**/*.rs📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
Files:
**/*.{rs,py}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,py,js,mjs,cjs,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,py,go,js,ts,c,h}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{rs,go,js,ts}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
**/*.{rs,py,go,js,ts}📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Files:
crates/{python,ffi,node}/**/*⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
WalkthroughThe PR migrates middleware callbacks and lifecycle processing to asynchronous fallible futures, adds queued sanitized event publication, introduces native ABI v3 completion-based middleware, and updates Rust, Node, Python, Go, worker, FFI, plugin, test, and documentation surfaces. ChangesAsynchronous middleware runtime
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
dadfc9a to
0da27bc
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 49
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
python/tests/test_builtin_codecs.py (1)
435-459: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest name still says "raises" but the test no longer raises.
The docstring and body now assert deferred/non-raising behavior, but the function name
test_manual_call_end_response_codec_failure_raises_after_end_eventstill implies an exception is raised. Rename for clarity.♻️ Suggested rename
- def test_manual_call_end_response_codec_failure_raises_after_end_event(self): + def test_manual_call_end_response_codec_failure_defers_without_raising(self):🤖 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 `@python/tests/test_builtin_codecs.py` around lines 435 - 459, Rename test_manual_call_end_response_codec_failure_raises_after_end_event to reflect the current deferred, non-raising behavior validated by its docstring and assertions; leave the test logic unchanged.crates/python/tests/coverage/py_callable_coverage_tests.rs (1)
156-218: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate repeated per-assertion Tokio runtime construction. Both files' async migrations construct a brand-new Tokio runtime at nearly every assertion instead of reusing one runtime for the whole test function; the shared fix is to build the runtime once and call
.block_on(...)on it repeatedly (or extract a tinyblock_onhelper).
crates/python/tests/coverage/py_callable_coverage_tests.rs#L156-L218: reuse theruntimevariable already bound at Line 113 for these four assertions instead of callingtokio::runtime::Runtime::new().unwrap()each time.crates/python/tests/coverage/coverage_tests.rs#L666-L777: hoist a singleBuilder::new_current_thread().enable_all().build().unwrap()(or a localblock_onhelper) to the top oftest_sync_wrapper_fallbacks_and_helpersand reuse it across all 8 call sites instead of rebuilding a runtime each time.🤖 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/python/tests/coverage/py_callable_coverage_tests.rs` around lines 156 - 218, Reuse the existing runtime variable in crates/python/tests/coverage/py_callable_coverage_tests.rs lines 156-218 for all four async assertions, removing per-assertion Runtime::new construction. In crates/python/tests/coverage/coverage_tests.rs lines 666-777, update test_sync_wrapper_fallbacks_and_helpers to create one current-thread Tokio runtime with enable_all at the test start (or use a local block_on helper) and reuse it across all eight call sites.docs/reference/migration-guides.mdx (1)
202-212: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDuplicated guidance in consecutive paragraphs.
Lines 205-207 and 210-212 state the same Python worker rule ("can return either an immediate value or an awaitable, but Python LLM sanitizers must still accept both the payload and directional context") almost verbatim. Drop one.
🤖 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/reference/migration-guides.mdx` around lines 202 - 212, Remove the duplicated Python worker middleware guidance from the consecutive paragraphs in the migration documentation, retaining one clear statement that Python worker sanitizers may return an immediate value or awaitable and Python LLM sanitizers must accept the payload and directional context.crates/ffi/tests/unit/api/registry_tests.rs (1)
1-429: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd async sanitizer test coverage
This module still covers only the sync sanitizer registrations. Add cases for the new async global and scope-local sanitizer registration APIs so thewrap_async_event_sanitize_fnpaths get exercised too.🤖 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/unit/api/registry_tests.rs` around lines 1 - 429, Add asynchronous sanitizer coverage to test_ffi_event_sanitizer_registries_and_error_paths, exercising both global and scope-local async registration APIs and their corresponding deregistration and callback paths. Ensure the cases invoke events/scopes so wrap_async_event_sanitize_fn is executed, and verify successful registration, sanitization behavior, invalid-argument handling, and cleanup alongside the existing synchronous tests.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 `@ATTRIBUTIONS-Rust.md`:
- Around line 24923-24924: Regenerate ATTRIBUTIONS-Rust.md by running uv run
pre-commit run --all-files, then include the hook’s resulting attribution
changes in the commit. Verify the generated file is clean and matches the
pre-commit output before handoff.
In `@crates/adaptive/tests/unit/runtime_tests.rs`:
- Around line 639-644: In the existing async test around llm_request_intercepts,
remove the nested tokio::runtime::Builder construction and block_on call. Await
llm_request_intercepts("anthropic", request.clone()) directly, preserving the
existing expect message and result handling.
In `@crates/core/src/api/llm.rs`:
- Around line 711-748: Update the event construction around
dispatch_transformed_event so its fallback snapshot never contains unsanitized
request data; seed it with a sanitized or redacted payload, and apply the same
treatment to the llm_call_end response path. Also replace the dispatcher-thread
state.read().expect in build_llm_start_event handling with non-panicking
poisoned-lock behavior so sanitizer or event processing cannot publish raw
payloads through panic fallback.
- Around line 590-636: Refactor emit_optimization_marks_with and
emit_optimization_marks_with_async to share a helper for optimization
contribution iteration and Event construction, including offset and timestamp
calculation. Keep each seam responsible only for applying its sanitizer
synchronously or asynchronously, then enqueueing, marking emitted contributions,
and preserving existing break behavior so both paths use identical accounting
logic.
In `@crates/core/src/api/runtime/subscriber_dispatcher.rs`:
- Around line 262-281: Prevent raw payload publication when queued sanitization
fails: in subscriber_dispatcher.rs, split transform execution from
event_sanitize_snapshot_chain so transform panics log an error and drop the
event, while sanitizer-chain panics retain the fail-open original fallback. In
crates/core/src/api/tool.rs lines 261-302 and crates/core/src/api/llm.rs lines
711-748, seed start and end events without raw payload data so fallback
snapshots contain no payload; update every cited raw start/end seed, including
the tool end event and LLM end event.
- Around line 242-261: Update sanitize_event_snapshot to reuse a single cached
Tokio runtime instead of building one per event, and configure that runtime with
both time and I/O drivers enabled so invoke_event_sanitize can await tonic
Channel RPCs. Preserve the existing fallback that logs runtime creation failure
and publishes the original event snapshot.
In `@crates/core/src/api/shared.rs`:
- Around line 61-70: Update snapshot_event_sanitizers and every queued caller to
preserve fail-closed behavior when the global context lock is poisoned: do not
convert its None result to an empty sanitizer list via unwrap_or_default, and
propagate the unavailable state so the event is not published unsanitized. Keep
the existing sanitize_event_with_scope_stack behavior consistent, using an
explicit Result or equivalent unavailable-state representation across the API.
In `@crates/core/src/api/tool.rs`:
- Around line 261-302: Ensure tool lifecycle events are seeded with sanitized
snapshots rather than raw arguments or results before dispatch. Update the
start-event construction around build_tool_start_event and the corresponding
tool_call_end path to sanitize raw_args and raw result first, then use those
sanitized values in the initial event; retain the transform only as needed for
subsequent processing.
In `@crates/core/src/plugin/dynamic/native.rs`:
- Around line 2399-2400: Update the legacy synchronous wrapper paths around
call_event_sanitize_callback and the other listed native callback invocations to
execute blocking plugin FFI work through Tokio’s spawn_blocking rather than
directly inside Box::pin(async move). Preserve each callback’s existing
arguments and result/error propagation while ensuring runtime worker threads are
not blocked.
In `@crates/core/src/plugin/dynamic/worker.rs`:
- Around line 1122-1131: The event sanitizer callbacks around
invoke_event_sanitize and the analogous tool/LLM and scope/mark callbacks no
longer preserve fallback behavior on worker RPC errors. Restore the previous
error handling so failures retain the original value and invoke
log_callback_fallback, or add equivalent structured logging if propagation is
intentional; apply the same policy consistently across all referenced surfaces.
In `@crates/core/src/stream.rs`:
- Around line 310-320: Update the no-runtime branch in the finalization spawning
logic near tokio::runtime::Handle::try_current so it returns a joinable
completion handle instead of spawning an untracked OS thread and returning None.
Use a runtime that supports the full async finalization work, including timers
and I/O, and ensure close() can await the resulting handle through the
finalization field before callers flush subscribers.
- Around line 304-308: Update the finalization delivery block around
sanitize_event_with_scope_stack and NemoRelayContextState::emit_event to use the
established queued event path that carries scope_stack with the message. Ensure
subscriber callbacks execute with the stream’s original scope stack, preserving
ambient context and root_uuid isolation when finalization runs in a spawned
task.
In `@crates/core/tests/fixtures/native_plugin/src/lib.rs`:
- Around line 656-658: Update every early-return branch in the native async
callback fixture, including the null-host, invalid host-string, and related
branches in the callback flow, to call async_completion_reject with the relevant
error before returning. Do not return
NemoRelayNativeAsyncCallbackState::Complete without settling the completion;
preserve the existing return state after rejection so
invoke_native_async_callback always resolves.
- Around line 293-298: Move the `NemoRelayNativeHostApiV2` cast in the V2 host
validation path until after validation. First read `abi_version` and
`struct_size` through a `NemoRelayNativeHostApiV1` pointer, return
`NemoRelayStatus::InvalidArg` when the version or size is insufficient, then
cast `host` to `NemoRelayNativeHostApiV2` only for subsequent access.
In `@crates/core/tests/integration/native_plugin_tests.rs`:
- Around line 708-717: Replace the single tokio::task::yield_now synchronization
in the async-pending native intercept test with an mpsc handshake, following
plugin_host_clear_allows_an_in_flight_native_callback_to_finish. Signal once
tool_request_intercepts has entered its pending-registration/continuation state,
await that signal before clear_plugin_configuration(), and preserve the existing
pending task completion and native_async assertion.
In `@crates/core/tests/integration/scope_local_tests.rs`:
- Line 113: Remove the redundant flush_subscribers().unwrap() call from the path
where captured_snapshot already performs the flush, while retaining the call at
the separate path around line 358 that has no captured_snapshot invocation.
In `@crates/core/tests/unit/context_tests.rs`:
- Around line 254-266: Update the enclosing tests that call
NemoRelayContextState::tool_conditional_execution_snapshot_chain to use
#[tokio::test] and await the chain directly. Remove the
tokio::runtime::Runtime::new().unwrap().block_on wrappers in both occurrences,
preserving the existing assertions and test behavior.
In `@crates/core/tests/unit/llm_api_tests.rs`:
- Around line 1381-1384: Update the sanitizer callback closure around
sanitizer_inputs so it performs the lock and push inside the returned async
future rather than when constructing the future. Clone context.codec() before
entering the async block as needed, and preserve the existing
redacted_response() result and captured input ordering based on polling.
In `@crates/core/tests/unit/plugin_tests.rs`:
- Around line 649-671: Replace the futures::executor::block_on calls around
llm_request_intercepts with the already-constructed tokio current-thread
runtime’s block_on method, including the matching calls in the later test
section. Keep the existing request setup and assertions unchanged so all async
middleware runs within the tokio runtime.
In `@crates/ffi/nemo_relay.h`:
- Around line 2570-2605: Add the missing async FFI declarations to nemo_relay.h
for every new LLM, event, and tool registration entry point exported under api,
matching each symbol’s signature, callback type, and ownership parameters. Keep
the declarations grouped with the existing async guardrail and intercept
functions so C/C++ consumers can access the complete published ABI.
In `@crates/ffi/src/api/event_registry.rs`:
- Around line 47-105: Add clear_last_error() as the first operation in
register_global_async and register_scope_async in
crates/ffi/src/api/event_registry.rs; add it before c_str_to_string in both
async registration macros in crates/ffi/src/api/llm_registry.rs; and add it to
nemo_relay_register_tool_sanitize_request_guardrail_async,
nemo_relay_register_tool_sanitize_response_guardrail_async,
nemo_relay_register_tool_conditional_execution_guardrail_async, and
nemo_relay_register_tool_execution_intercept_async in
crates/ffi/src/api/tool_registry.rs, matching the existing
async_tool_json_registration! pattern.
In `@crates/ffi/src/api/mod.rs`:
- Line 150: Replace the direct tokio_runtime().block_on call in the nemo_relay_*
synchronous FFI entrypoints with runtime-thread-safe handling: route async
callers through the completion-based ABI, or detect and reject calls made from a
Tokio runtime thread before blocking. Apply this consistently to every affected
entrypoint and add coverage invoking each one from a Tokio task.
In `@crates/ffi/src/api/scope_registry.rs`:
- Around line 69-110: Regenerate the C header for all new
scope_async_registration exports, including the tool and LLM guardrail and
request-intercept functions. Ensure the generated declarations include
break_chain before cb for both request-intercept variants, matching the
corresponding nemo_relay_register_*_async signatures.
In `@crates/ffi/src/callable.rs`:
- Around line 253-258: Capture a tokio runtime Handle when constructing
NemoRelayAsyncNext, store it in the wrapper, and use handle.spawn for both
continuation spawn sites in crates/ffi/src/callable.rs (253-258 and the related
site near line 310). Apply the same change to NativeAsyncNext in
crates/core/src/plugin/dynamic/native.rs (1570-1576), replacing bare
tokio::spawn with the stored handle.
- Around line 877-891: Async stream intercepts buffer the entire LlmJsonStream
because the ABI resolves one JSON value, preventing incremental delivery. Update
the NemoRelayAsyncInterceptCb stream registration around
crates/ffi/src/callable.rs lines 877-891 and the matching
NemoRelayNativeAsyncMiddlewareKind::LlmStreamExecutionIntercept implementation
around crates/core/src/plugin/dynamic/native.rs lines 1547-1568 to use
chunk-delivery callbacks and forward chunks as they arrive; alternatively,
explicitly document the buffering limitation on both
wrap_async_llm_stream_execution_intercept_fn and the native middleware kind.
- Around line 342-345: Replace poisoning-sensitive Mutex lock unwraps with
PoisonError::into_inner in all listed settle paths: callable.rs functions
nemo_relay_async_completion_resolve, nemo_relay_async_completion_reject, and
nemo_relay_async_next_invoke (both locks), plus native.rs functions
native_async_completion_resolve_json, native_async_completion_reject, and
native_async_next_invoke (both locks). Apply the change at
crates/ffi/src/callable.rs lines 342-345, 369, and 220, 235, and
crates/core/src/plugin/dynamic/native.rs lines 1445-1448, 1474, and 1538, 1553;
preserve the existing sender take/send behavior.
- Around line 135-145: Update the async callback completion handling in the
shown invocation path, plus invoke_async_intercept and
invoke_native_async_callback, to detect when a Complete callback returned
without consuming the completion sender. Fail immediately with the existing
internal error instead of awaiting a receiver that cannot resolve, while
preserving normal resolve/reject behavior.
In `@crates/node/src/api/mod.rs`:
- Around line 3557-3567: Update the ts_arg_type for
scope_register_llm_request_intercept to include the Promise variant of the
callback return type, mirroring the exact type shape used by the sibling API at
Line 2953. Keep the existing synchronous callback type unchanged and align it
with wrap_js_llm_request_intercept_promise_fn.
In `@crates/node/src/callable.rs`:
- Around line 210-233: Update the Promise wrappers
wrap_js_tool_conditional_promise_fn, wrap_js_tool_request_intercept_promise_fn,
the LLM conditional wrapper, and the LLM intercept outcome-deserialization
branch to call record_callback_error whenever the callback or deserialization
fails. Preserve each wrapper’s existing success and return-value behavior while
ensuring getLastCallbackError() can surface every failure path.
- Around line 497-516: Update the TSFN-backed async wrappers around the shown
Arc closure and the other wrapper implementations to avoid
recv_middleware_json_result’s blocking std::sync::mpsc::recv. Use
tokio::sync::oneshot for callback delivery and await the receiver, matching the
existing PromiseAwareFn::call_inner pattern while preserving status and error
handling.
In `@crates/node/src/callback_factory.rs`:
- Around line 78-88: Update the rejection handler in the promise chain to
preserve non-Error rejection details, matching the execution wrapper’s
String(error?.message ?? error) behavior within the existing try handling. Keep
direct string messages supported, but avoid defaulting numbers, objects without
message, or other primitives to “unknown error”; ensure reject receives the
resulting diagnostic string.
In `@crates/node/tests/tools_tests.mjs`:
- Around line 611-622: Add failure-path tests alongside the conditional
guardrail and request-intercept tests around toolCallExecute: cover a guardrail
that rejects and intercept callbacks that throw synchronously or return rejected
Promises. Assert each documented error propagates from toolCallExecute and
verify the corresponding guardrail/intercept registrations are cleaned up in
finally blocks.
In `@crates/plugin/src/lib.rs`:
- Around line 839-847: Rename the public ABI extension struct from
NemoRelayNativeHostApiV2 to NemoRelayNativeHostApiV3 and update all references
to this type, preserving its existing layout and fields.
- Around line 2386-2390: Update the ABI version check in the host validation
block to compare against the fixed version that introduced
NemoRelayNativeHostApiV2 (version 3), rather than the moving
NEMO_RELAY_NATIVE_ABI_VERSION constant. Keep the existing struct_size validation
unchanged.
In `@crates/python/src/py_api/mod.rs`:
- Around line 1273-1286: The four chain-execution functions currently return
either a value or awaitable while the Python stubs promise only Awaitable;
settle on one consistent contract. In crates/python/src/py_api/mod.rs:1273-1286,
either remove the get_running_loop synchronous fallback and always return an
awaitable, or preserve both modes while adding the TASK_SCOPE_STACK.scope(...)
wrapper to the synchronous path. In python/nemo_relay/_native.pyi:1675-1721,
update the return annotations and Returns prose for tool_request_intercepts,
tool_conditional_execution, llm_request_intercepts, and
llm_conditional_execution to match the implemented contract.
- Around line 1273-1286: Update each synchronous branch around the repeated
runtime.block_on calls to execute tool_request_intercepts within
TASK_SCOPE_STACK.scope using the captured scope_stack, matching the async path’s
scope propagation. Preserve the existing result conversion and error handling,
and apply the same change consistently at all three locations.
In `@crates/python/src/py_callable.rs`:
- Around line 1154-1165: Replace the synchronous asyncio.run handling in the
Python::attach callback with the existing split_py_object_or_future and
resolve_py_object_or_future helpers used by sibling wrappers, so awaitables
resolve on the shared runtime without holding the GIL. Update the outer
Python::attach return type to FlowResult<Result<Py<PyAny>, PyValueFuture>> and
adjust earlier return Err arms to match.
In `@crates/python/tests/coverage/coverage_tests.rs`:
- Around line 675-684: Update the assertion around wrap_py_tool_fn and tool_fail
to expect the returned future to fail rather than unwrap a successful value.
Preserve the existing runtime setup and inputs, but assert the error path
produced when tool_fail raises RuntimeError.
In `@docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx`:
- Line 119: Update the Native ABI v3 section beneath the “Native ABI v3” heading
to clarify that the entry symbol still receives the v1-prefixed table, with the
v3 extension appended afterward, before describing the v2/v3 prefix layering.
- Around line 138-142: Format the ABI completion-state names as code on both
documentation sites: in
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx lines 138-142, wrap
Complete and Pending in backticks; in docs/reference/event-sanitizers.mdx lines
191-195, wrap the state names in backticks and change their casing to Complete
and Pending to match the native ABI page.
In `@docs/reference/migration-guides.mdx`:
- Around line 276-283: Update the “For the complete ABI contract” link in the
migration guide to use descriptive “Native ABI v3” anchor text and target the
Native ABI v3 section anchor instead of the stale v2 destination. Run just
docs-linkcheck to verify the changed documentation link.
- Around line 52-61: Update the closure parameters in
register_tool_conditional_execution_guardrail so the example does not trigger
unused-parameter warnings: rename name and args to _name and _args, or reference
name in the policy logic/comment while preserving the existing guardrail
behavior.
- Around line 37-42: Update the migration table headers from “0.6 callback” and
“0.7 callback” to title case, matching the “0.6 Contract” and “0.7 Contract”
convention used by the sibling table; leave the table content unchanged.
In `@go/nemo_relay/callbacks.go`:
- Line 616: Replace the unchecked type assertions in the callback handlers
around lookupClosure at the referenced locations, including the paths at lines
653 and 664, with comma-ok assertions. When the token is unknown, freed, or
resolves to an incompatible value, reject the completion and return through the
existing failure path instead of allowing a panic across the cgo boundary.
- Around line 689-708: The callback flow invoking fn with nextFn does not
protect against delayed use after fn returns and the next handle is released.
Either enforce and document synchronous-only nextFn usage, or add lifetime
tracking with a released state so nemo_relay_async_next_invoke_callback rejects
calls after release instead of dereferencing freed memory.
- Around line 618-643: Update the async callback goroutine and the intercept
trampoline to propagate cancellation for the entire callback lifetime: use a
shared cancellable context, drive cancel from a completion-bound cancellation
poller or available FFI callback, and pass that context instead of
context.Background(). Route resolve_json and reject results through the existing
checkStatus handling so failed settlement is reported rather than ignored.
In `@go/nemo_relay/nemo_relay.go`:
- Around line 47-50: Define a Go constant representing the pending async
callback state, matching NemoRelayAsyncCallbackState::Pending, and replace the
C.uint32_t(1) returns in both async trampolines with that named constant. Keep
the existing callback behavior unchanged while removing the magic number.
In `@python/nemo_relay/_native.pyi`:
- Line 1675: Update the type annotations for tool_request_intercepts and the
three related tool interception functions to return _Json | Awaitable[_Json],
matching the native implementation’s synchronous fallback and asynchronous
behavior. Revise each corresponding Returns: docstring to describe both possible
return modes; do not remove the native synchronous path.
- Around line 60-62: Update the _LlmRequestIntercept type alias so the entire
return-type union is enclosed in a single quoted forward reference, matching the
established pattern used nearby. Preserve both LLMRequestInterceptOutcome and
Awaitable alternatives, and ensure the resulting annotation passes ty type
checking.
---
Outside diff comments:
In `@crates/ffi/tests/unit/api/registry_tests.rs`:
- Around line 1-429: Add asynchronous sanitizer coverage to
test_ffi_event_sanitizer_registries_and_error_paths, exercising both global and
scope-local async registration APIs and their corresponding deregistration and
callback paths. Ensure the cases invoke events/scopes so
wrap_async_event_sanitize_fn is executed, and verify successful registration,
sanitization behavior, invalid-argument handling, and cleanup alongside the
existing synchronous tests.
In `@crates/python/tests/coverage/py_callable_coverage_tests.rs`:
- Around line 156-218: Reuse the existing runtime variable in
crates/python/tests/coverage/py_callable_coverage_tests.rs lines 156-218 for all
four async assertions, removing per-assertion Runtime::new construction. In
crates/python/tests/coverage/coverage_tests.rs lines 666-777, update
test_sync_wrapper_fallbacks_and_helpers to create one current-thread Tokio
runtime with enable_all at the test start (or use a local block_on helper) and
reuse it across all eight call sites.
In `@docs/reference/migration-guides.mdx`:
- Around line 202-212: Remove the duplicated Python worker middleware guidance
from the consecutive paragraphs in the migration documentation, retaining one
clear statement that Python worker sanitizers may return an immediate value or
awaitable and Python LLM sanitizers must accept the payload and directional
context.
In `@python/tests/test_builtin_codecs.py`:
- Around line 435-459: Rename
test_manual_call_end_response_codec_failure_raises_after_end_event to reflect
the current deferred, non-raising behavior validated by its docstring and
assertions; leave the test logic 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: 5739b182-4979-4b86-9f7a-f107131ccd0a
📒 Files selected for processing (76)
ATTRIBUTIONS-Rust.mdcrates/adaptive/src/acg_component.rscrates/adaptive/src/adaptive_hints_intercept.rscrates/adaptive/tests/integration/runtime_integration_tests.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/adaptive/tests/unit/plugin_component_tests.rscrates/adaptive/tests/unit/runtime_features_tests.rscrates/adaptive/tests/unit/runtime_tests.rscrates/cli/src/sessions/mod.rscrates/cli/tests/coverage/shared/server_tests.rscrates/core/src/api/llm.rscrates/core/src/api/runtime/callbacks.rscrates/core/src/api/runtime/state.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/src/api/scope.rscrates/core/src/api/shared.rscrates/core/src/api/tool.rscrates/core/src/plugin/dynamic/native.rscrates/core/src/plugin/dynamic/worker.rscrates/core/src/stream.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/integration/pipeline_tests.rscrates/core/tests/integration/scope_local_tests.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/tests/unit/context_tests.rscrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/unit/llm_api_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/plugin_tests.rscrates/core/tests/unit/shared_tests.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/callable_extra_tests.rscrates/ffi/tests/unit/api/registry_tests.rscrates/ffi/tests/unit/callable_tests.rscrates/node/src/api/mod.rscrates/node/src/callable.rscrates/node/src/callback_factory.rscrates/node/src/promise_call.rscrates/node/tests/callback_error_tests.mjscrates/node/tests/event_sanitizers_tests.mjscrates/node/tests/llm_tests.mjscrates/node/tests/tools_tests.mjscrates/pii-redaction/src/builtin.rscrates/pii-redaction/tests/unit/component_tests.rscrates/plugin/README.mdcrates/plugin/src/lib.rscrates/python/src/py_api/mod.rscrates/python/src/py_callable.rscrates/python/tests/coverage/coverage_tests.rscrates/python/tests/coverage/py_api_coverage_tests.rscrates/python/tests/coverage/py_callable_coverage_tests.rsdocs/about-nemo-relay/concepts/middleware.mdxdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdxdocs/reference/event-sanitizers.mdxdocs/reference/migration-guides.mdxgo/nemo_relay/callbacks.gogo/nemo_relay/nemo_relay.gopython/nemo_relay/__init__.pypython/nemo_relay/__init__.pyipython/nemo_relay/_native.pyipython/tests/test_adaptive.pypython/tests/test_builtin_codecs.pypython/tests/test_context_isolation.pypython/tests/test_event_sanitizers.pypython/tests/test_llm.pypython/tests/test_tools.py
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/core/src/api/runtime/subscriber_dispatcher.rs (1)
217-231: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftAsync middleware now serializes behind one dispatcher thread.
Every transform and sanitizer chain is
block_on-ed on the single dispatcher thread, so one slow worker-plugin RPC sanitizer stalls publication of all unrelated events and any concurrentflush_subscriberscaller. Worth a bounded per-event timeout and a queue-depth/latency metric before this ships.🤖 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/subscriber_dispatcher.rs` around lines 217 - 231, The deliver_event path currently blocks the single dispatcher thread while running transform and sanitizer futures. Add a bounded per-event timeout around sanitize_event_snapshot (including transform and sanitizers), ensure timed-out events cleanly restore the prior scope stack and IN_DISPATCHER state, and record queue-depth or dispatch-latency metrics using the dispatcher’s existing metric facilities.crates/python/tests/coverage/coverage_tests.rs (1)
664-696: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTen separate current-thread runtimes in one test function. Build one runtime at the top of the closure and reuse it for every
block_on; the repetition obscures what each assertion checks.🤖 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/python/tests/coverage/coverage_tests.rs` around lines 664 - 696, In the test closure containing the tool_ok, tool_fail, and tool_cond assertions, construct a single current-thread Tokio runtime before the assertions and reuse it for every block_on call. Remove the repeated Builder chains while preserving each assertion’s existing result checks.go/nemo_relay/callbacks.go (1)
715-742: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
nextOpendoes not prevent use-after-free ofnext. A goroutine can pass thenextOpen.Load()check at Line 719, then be descheduled;fnreturns,nextOpen.Store(false)runs, the deferredC.nemo_relay_async_next_release(next)frees the handle, and the stalled caller then passes the freed pointer tonemo_relay_async_next_invoke_callback. The atomic flag orders nothing with respect to the release. Guard the handle with async.RWMutex(read-lock around the invoke, write-lock around the close + release) or track in-flight calls with async.WaitGroupthat the goroutine waits on before releasing.🛡️ Serialize invocation against release
- var nextOpen atomic.Bool - nextOpen.Store(true) + var nextMu sync.RWMutex + nextOpen := true + defer func() { + nextMu.Lock() + nextOpen = false + nextMu.Unlock() + }() nextFn := func(ctx context.Context, payload json.RawMessage) (json.RawMessage, error) { - if !nextOpen.Load() { - return nil, context.Canceled - } + nextMu.RLock() + defer nextMu.RUnlock() + if !nextOpen { + return nil, context.Canceled + }Note the deferred release must then run after the write-lock defer (defers run LIFO), so move
C.nemo_relay_async_next_release(next)into the same closure that flips the flag.🤖 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/callbacks.go` around lines 715 - 742, Replace the nextOpen atomic flag in the callback setup with synchronization that tracks in-flight nextFn calls, such as a sync.RWMutex or sync.WaitGroup. Ensure nextFn holds the guard through nemo_relay_async_next_invoke_callback and rejects calls after shutdown; move nemo_relay_async_next_release(next) into the shutdown closure so it executes only after the write lock or in-flight wait completes, preserving defer ordering and preventing use-after-free.Source: Path instructions
♻️ Duplicate comments (1)
crates/python/src/py_api/mod.rs (1)
1273-1289: 🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy liftScope propagation is fixed, but the sync branch still
block_ons while holding the GIL. If any middleware in the chain returns an awaitable,py_callable::split_json_or_futurehands it topyo3_async_runtimes::tokio::into_future, which resolves on the shared runtime and needs the GIL to complete — the blocked thread holds it. Wrap theblock_oninpy.detach(...)(asflush_subscribersdoes), and consider a cached runtime instead of building one per call.🤖 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/python/src/py_api/mod.rs` around lines 1273 - 1289, Update the synchronous branch around `runtime.block_on` to execute the blocking runtime work inside `py.detach(...)`, releasing the GIL while middleware awaitables resolve; preserve `TASK_SCOPE_STACK.scope` and the existing result/error conversion behavior. Reuse the established cached-runtime pattern if available instead of constructing a new runtime for every call.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/api/llm.rs`:
- Around line 896-908: Update the queued transform around state access in the
END event path to bind its event parameter and replace the panicking
context.read().expect call with graceful poisoned-lock handling. On read
failure, return the seeded end event supplied to the closure; otherwise continue
building the enriched event through build_llm_end_event with the existing
metadata and response fields.
In `@crates/core/src/api/runtime/subscriber_dispatcher.rs`:
- Around line 288-306: Update the event sanitization flow around
event_sanitize_snapshot_chain to avoid cloning transformed when
sanitizers.is_empty(); return the transformed event directly in that case. Only
create the fallback clone when sanitizers are present and catch_unwind may need
to publish the original transformed snapshot after a panic, preserving the
existing panic logging and fallback behavior.
In `@crates/core/src/api/scope.rs`:
- Around line 382-389: Preserve poisoned-lock signals instead of silently
dropping events: update snapshot_sanitizer guards to restore the
default-dispatch behavior or explicitly log an error on None. Apply this in
scope.rs:382-389 and the push_scope/pop_scope paths, tool.rs:304-307 for
skill-load and tool-call dispatches, llm.rs:709-710 for llm_call start/end
events, and stream.rs:351-359 in emit_chunk_mark; preserve scope propagation,
callback lifetimes, stream finalization, and root_uuid isolation.
In `@crates/ffi/build.rs`:
- Around line 20-26: Update the header rewriting flow around the `replacen` call
to first assert that the expected `\n#endif /* NEMO_RELAY_H */\n` marker exists
in the generated header, and fail fast with a clear message when it is absent;
only perform the replacement and subsequent write when the marker is present.
In `@crates/node/src/callable.rs`:
- Around line 753-761: Update the TSFN sanitizer handling in the request and
response paths around await_middleware_json_or_value to use
await_middleware_json_result instead, preserving the existing error propagation
behavior of wrap_js_llm_sanitize_request_promise_fn and
wrap_js_llm_sanitize_response_promise_fn. Keep successful null results mapped to
Ok(None), but propagate callback failures as Err rather than treating them as
payload omission.
- Around line 753-761: Align sanitizer failure handling with the Promise
wrappers by propagating callback errors instead of treating them as omitted
payloads: update await_middleware_json_or_value usage in
crates/node/src/callable.rs:753-761 and wrap_js_llm_sanitize_response_fn at
crates/node/src/callable.rs:799-805 to use await_middleware_json_result and
record_callback_error. Update the corresponding Python coverage assertions at
crates/python/tests/coverage/coverage_tests.rs:699-713 and 764-778 to expect
deserialization or RuntimeError failures via unwrap_err rather than Ok(None).
In `@python/nemo_relay/_native.pyi`:
- Around line 56-62: Update the return annotation of _ToolExecutionIntercept to
quote the entire union of ToolExecutionInterceptOutcome and
Awaitable[ToolExecutionInterceptOutcome], matching the established
_LlmRequestIntercept pattern while leaving its callable parameters unchanged.
---
Outside diff comments:
In `@crates/core/src/api/runtime/subscriber_dispatcher.rs`:
- Around line 217-231: The deliver_event path currently blocks the single
dispatcher thread while running transform and sanitizer futures. Add a bounded
per-event timeout around sanitize_event_snapshot (including transform and
sanitizers), ensure timed-out events cleanly restore the prior scope stack and
IN_DISPATCHER state, and record queue-depth or dispatch-latency metrics using
the dispatcher’s existing metric facilities.
In `@crates/python/tests/coverage/coverage_tests.rs`:
- Around line 664-696: In the test closure containing the tool_ok, tool_fail,
and tool_cond assertions, construct a single current-thread Tokio runtime before
the assertions and reuse it for every block_on call. Remove the repeated Builder
chains while preserving each assertion’s existing result checks.
In `@go/nemo_relay/callbacks.go`:
- Around line 715-742: Replace the nextOpen atomic flag in the callback setup
with synchronization that tracks in-flight nextFn calls, such as a sync.RWMutex
or sync.WaitGroup. Ensure nextFn holds the guard through
nemo_relay_async_next_invoke_callback and rejects calls after shutdown; move
nemo_relay_async_next_release(next) into the shutdown closure so it executes
only after the write lock or in-flight wait completes, preserving defer ordering
and preventing use-after-free.
---
Duplicate comments:
In `@crates/python/src/py_api/mod.rs`:
- Around line 1273-1289: Update the synchronous branch around `runtime.block_on`
to execute the blocking runtime work inside `py.detach(...)`, releasing the GIL
while middleware awaitables resolve; preserve `TASK_SCOPE_STACK.scope` and the
existing result/error conversion behavior. Reuse the established cached-runtime
pattern if available instead of constructing a new runtime for every call.
🪄 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: 8bed11a4-1d8f-4249-80c1-e241ae540e7c
📒 Files selected for processing (29)
ATTRIBUTIONS-Rust.mdcrates/adaptive/tests/unit/runtime_tests.rscrates/core/src/api/llm.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/src/api/scope.rscrates/core/src/api/tool.rscrates/core/src/plugin/dynamic/native.rscrates/core/src/plugin/dynamic/worker.rscrates/core/src/stream.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/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/tool_registry.rscrates/ffi/src/callable.rscrates/node/src/api/mod.rscrates/node/src/callable.rscrates/node/src/callback_factory.rscrates/plugin/src/lib.rscrates/python/src/py_api/mod.rscrates/python/src/py_callable.rscrates/python/tests/coverage/coverage_tests.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdxdocs/reference/migration-guides.mdxgo/nemo_relay/callbacks.gopython/nemo_relay/_native.pyi
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (49)
**/*.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/unit/native_plugin_tests.rscrates/ffi/build.rscrates/adaptive/tests/unit/runtime_tests.rscrates/ffi/src/api/mod.rscrates/node/src/callback_factory.rscrates/core/src/stream.rscrates/plugin/src/lib.rscrates/core/src/api/scope.rscrates/ffi/src/api/llm_registry.rscrates/python/tests/coverage/coverage_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/python/src/py_api/mod.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/tool.rscrates/ffi/src/api/event_registry.rscrates/core/src/plugin/dynamic/worker.rscrates/python/src/py_callable.rscrates/core/src/plugin/dynamic/native.rscrates/ffi/src/api/tool_registry.rscrates/core/src/api/llm.rscrates/ffi/src/callable.rscrates/node/src/api/mod.rscrates/node/src/callable.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/adaptive/tests/unit/runtime_tests.rscrates/core/src/stream.rscrates/core/src/api/scope.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/tool.rscrates/core/src/plugin/dynamic/worker.rscrates/core/src/plugin/dynamic/native.rscrates/core/src/api/llm.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/tests/unit/native_plugin_tests.rscrates/core/src/stream.rscrates/core/src/api/scope.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/tool.rscrates/core/src/plugin/dynamic/worker.rscrates/core/src/plugin/dynamic/native.rscrates/core/src/api/llm.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/tests/unit/native_plugin_tests.rscrates/ffi/build.rscrates/adaptive/tests/unit/runtime_tests.rscrates/ffi/src/api/mod.rscrates/node/src/callback_factory.rscrates/core/src/stream.rscrates/plugin/src/lib.rscrates/core/src/api/scope.rscrates/ffi/src/api/llm_registry.rscrates/python/tests/coverage/coverage_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/python/src/py_api/mod.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/tool.rscrates/ffi/src/api/event_registry.rscrates/core/src/plugin/dynamic/worker.rscrates/python/src/py_callable.rscrates/core/src/plugin/dynamic/native.rscrates/ffi/src/api/tool_registry.rscrates/core/src/api/llm.rscrates/ffi/src/callable.rscrates/node/src/api/mod.rscrates/node/src/callable.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/unit/native_plugin_tests.rscrates/ffi/build.rscrates/adaptive/tests/unit/runtime_tests.rscrates/ffi/src/api/mod.rscrates/node/src/callback_factory.rscrates/core/src/stream.rscrates/plugin/src/lib.rscrates/core/src/api/scope.rscrates/ffi/src/api/llm_registry.rscrates/python/tests/coverage/coverage_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/python/src/py_api/mod.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/tool.rscrates/ffi/src/api/event_registry.rscrates/core/src/plugin/dynamic/worker.rscrates/python/src/py_callable.rscrates/core/src/plugin/dynamic/native.rscrates/ffi/src/api/tool_registry.rscrates/core/src/api/llm.rscrates/ffi/src/callable.rscrates/node/src/api/mod.rscrates/node/src/callable.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/unit/native_plugin_tests.rscrates/ffi/build.rscrates/adaptive/tests/unit/runtime_tests.rscrates/ffi/src/api/mod.rscrates/node/src/callback_factory.rscrates/core/src/stream.rscrates/plugin/src/lib.rsgo/nemo_relay/callbacks.gocrates/core/src/api/scope.rscrates/ffi/src/api/llm_registry.rscrates/python/tests/coverage/coverage_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/python/src/py_api/mod.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/ffi/nemo_relay.hcrates/core/src/api/tool.rscrates/ffi/src/api/event_registry.rscrates/core/src/plugin/dynamic/worker.rscrates/python/src/py_callable.rscrates/core/src/plugin/dynamic/native.rscrates/ffi/src/api/tool_registry.rscrates/core/src/api/llm.rscrates/ffi/src/callable.rscrates/node/src/api/mod.rscrates/node/src/callable.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/unit/native_plugin_tests.rscrates/ffi/build.rscrates/adaptive/tests/unit/runtime_tests.rscrates/ffi/src/api/mod.rscrates/node/src/callback_factory.rscrates/core/src/stream.rscrates/plugin/src/lib.rsgo/nemo_relay/callbacks.gocrates/core/src/api/scope.rscrates/ffi/src/api/llm_registry.rscrates/python/tests/coverage/coverage_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/python/src/py_api/mod.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/tool.rscrates/ffi/src/api/event_registry.rscrates/core/src/plugin/dynamic/worker.rscrates/python/src/py_callable.rscrates/core/src/plugin/dynamic/native.rscrates/ffi/src/api/tool_registry.rscrates/core/src/api/llm.rscrates/ffi/src/callable.rscrates/node/src/api/mod.rscrates/node/src/callable.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/unit/native_plugin_tests.rscrates/ffi/build.rscrates/adaptive/tests/unit/runtime_tests.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/ffi/src/api/mod.rscrates/node/src/callback_factory.rsdocs/reference/migration-guides.mdxpython/nemo_relay/_native.pyiATTRIBUTIONS-Rust.mdcrates/core/src/stream.rscrates/plugin/src/lib.rsgo/nemo_relay/callbacks.gocrates/core/src/api/scope.rscrates/ffi/src/api/llm_registry.rscrates/python/tests/coverage/coverage_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/python/src/py_api/mod.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/ffi/nemo_relay.hcrates/core/src/api/tool.rscrates/ffi/src/api/event_registry.rscrates/core/src/plugin/dynamic/worker.rscrates/python/src/py_callable.rscrates/core/src/plugin/dynamic/native.rscrates/ffi/src/api/tool_registry.rscrates/core/src/api/llm.rscrates/ffi/src/callable.rscrates/node/src/api/mod.rscrates/node/src/callable.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/adaptive/tests/unit/runtime_tests.rscrates/core/src/stream.rscrates/core/src/api/scope.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/tool.rscrates/core/src/plugin/dynamic/worker.rscrates/core/src/plugin/dynamic/native.rscrates/core/src/api/llm.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/core/tests/unit/native_plugin_tests.rscrates/ffi/build.rscrates/adaptive/tests/unit/runtime_tests.rscrates/ffi/src/api/mod.rscrates/node/src/callback_factory.rscrates/core/src/stream.rscrates/plugin/src/lib.rsgo/nemo_relay/callbacks.gocrates/core/src/api/scope.rscrates/ffi/src/api/llm_registry.rscrates/python/tests/coverage/coverage_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/python/src/py_api/mod.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/tool.rscrates/ffi/src/api/event_registry.rscrates/core/src/plugin/dynamic/worker.rscrates/python/src/py_callable.rscrates/core/src/plugin/dynamic/native.rscrates/ffi/src/api/tool_registry.rscrates/core/src/api/llm.rscrates/ffi/src/callable.rscrates/node/src/api/mod.rscrates/node/src/callable.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/core/tests/unit/native_plugin_tests.rscrates/ffi/build.rscrates/adaptive/tests/unit/runtime_tests.rscrates/ffi/src/api/mod.rscrates/node/src/callback_factory.rscrates/core/src/stream.rscrates/plugin/src/lib.rsgo/nemo_relay/callbacks.gocrates/core/src/api/scope.rscrates/ffi/src/api/llm_registry.rscrates/python/tests/coverage/coverage_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/python/src/py_api/mod.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/tool.rscrates/ffi/src/api/event_registry.rscrates/core/src/plugin/dynamic/worker.rscrates/python/src/py_callable.rscrates/core/src/plugin/dynamic/native.rscrates/ffi/src/api/tool_registry.rscrates/core/src/api/llm.rscrates/ffi/src/callable.rscrates/node/src/api/mod.rscrates/node/src/callable.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/adaptive/tests/unit/runtime_tests.rscrates/core/src/stream.rscrates/core/src/api/scope.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/tool.rscrates/core/src/plugin/dynamic/worker.rscrates/core/src/plugin/dynamic/native.rscrates/core/src/api/llm.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/native_plugin_tests.rscrates/adaptive/tests/unit/runtime_tests.rscrates/python/tests/coverage/coverage_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.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/build.rscrates/ffi/src/api/mod.rscrates/ffi/src/api/llm_registry.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/event_registry.rscrates/ffi/src/api/tool_registry.rscrates/ffi/src/callable.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/build.rscrates/ffi/src/api/mod.rscrates/ffi/src/api/llm_registry.rscrates/ffi/src/api/event_registry.rscrates/ffi/src/api/tool_registry.rscrates/ffi/src/callable.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/build.rscrates/ffi/src/api/mod.rscrates/node/src/callback_factory.rscrates/ffi/src/api/llm_registry.rscrates/python/tests/coverage/coverage_tests.rscrates/python/src/py_api/mod.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/event_registry.rscrates/python/src/py_callable.rscrates/ffi/src/api/tool_registry.rscrates/ffi/src/callable.rscrates/node/src/api/mod.rscrates/node/src/callable.rs
crates/adaptive/**
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep
crates/adaptivealigned with the canonical adaptive config schema, built-in section helpers, plugin lifecycle, and validation/report behavior.
Files:
crates/adaptive/tests/unit/runtime_tests.rs
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
In MDX files, top-of-file comments must use JSX comment delimiters (
{/*to open and*/}to close); do not use HTML comments for MDX SPDX headers
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxdocs/reference/migration-guides.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.mdxdocs/reference/migration-guides.mdxATTRIBUTIONS-Rust.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.mdxdocs/reference/migration-guides.mdxATTRIBUTIONS-Rust.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/src/lib.rscrates/core/src/plugin/dynamic/worker.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/worker.rscrates/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.mdxdocs/reference/migration-guides.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.mdxdocs/reference/migration-guides.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.mdxdocs/reference/migration-guides.mdx
{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/llm_registry.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/event_registry.rscrates/ffi/src/api/tool_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/node/src/callback_factory.rscrates/core/src/stream.rscrates/plugin/src/lib.rscrates/core/src/api/scope.rscrates/ffi/src/api/llm_registry.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/python/src/py_api/mod.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/tool.rscrates/ffi/src/api/event_registry.rscrates/core/src/plugin/dynamic/worker.rscrates/python/src/py_callable.rscrates/core/src/plugin/dynamic/native.rscrates/ffi/src/api/tool_registry.rscrates/core/src/api/llm.rscrates/ffi/src/callable.rscrates/node/src/api/mod.rscrates/node/src/callable.rs
{crates/python/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update the language-native bindings for every exposed surface in Python, Go, and Node.js.
Files:
python/nemo_relay/_native.pyigo/nemo_relay/callbacks.gocrates/python/src/py_api/mod.rscrates/node/src/api/mod.rs
{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update language wrapper helpers such as Python wrapper modules, Python type stubs, and Go shorthand packages when the new behavior belongs in those helper layers.
Files:
python/nemo_relay/_native.pyigo/nemo_relay/callbacks.go
python/nemo_relay/**/*
⚙️ CodeRabbit configuration file
python/nemo_relay/**/*: Review Python wrapper changes for typed API consistency, contextvars-based scope isolation, async behavior, and parity with the native extension.
Stubs and runtime implementations should stay aligned.
Files:
python/nemo_relay/_native.pyi
**/*.{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:
ATTRIBUTIONS-Rust.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:
ATTRIBUTIONS-Rust.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:
ATTRIBUTIONS-Rust.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:
ATTRIBUTIONS-Rust.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:
ATTRIBUTIONS-Rust.md
{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/core/src/plugin/dynamic/worker.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.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/callbacks.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/callbacks.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/callbacks.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/callbacks.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/callbacks.go
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/scope.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/src/api/tool.rscrates/core/src/api/llm.rs
crates/core/src/api/{tool,llm,shared,scope}.rs
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Wire the new middleware chain into the appropriate lifecycle owner and pipeline stage: tool and LLM execution paths use
tool.rsorllm.rs; shared mark and scope event sanitization usesshared.rsand is called fromscope.rs.
Files:
crates/core/src/api/scope.rscrates/core/src/api/tool.rscrates/core/src/api/llm.rs
crates/core/src/api/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Preserve the documented pipeline order: conditional guardrails, request intercepts, request sanitization, execution intercepts, and response sanitization for tool and LLM execution; specialized sanitization, event creation, and dispatch for mark and scope events.
Files:
crates/core/src/api/scope.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/src/api/tool.rscrates/core/src/api/llm.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
{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/worker.rscrates/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/worker.rscrates/core/src/plugin/dynamic/native.rs
🪛 markdownlint-cli2 (0.23.0)
ATTRIBUTIONS-Rust.md
[warning] 24924-24924: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
[warning] 24924-24924: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 24925-24925: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
[warning] 24925-24925: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 25131-25131: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 25132-25132: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 25132-25132: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (35)
crates/core/src/plugin/dynamic/native.rs (3)
2442-2443: Legacy sync wrappers still execute plugin FFI directly on runtime worker threads; previously flagged.
796-799: LGTM!Also applies to: 866-880
1377-1454: LGTM!Also applies to: 1539-1620
crates/core/src/api/runtime/subscriber_dispatcher.rs (1)
254-287: LGTM!crates/core/src/api/scope.rs (1)
248-255: LGTM!Also applies to: 314-321
crates/core/src/api/tool.rs (1)
284-303: LGTM!Also applies to: 459-489
crates/plugin/src/lib.rs (1)
43-44: LGTM!Also applies to: 847-888, 2388-2393
crates/core/src/plugin/dynamic/worker.rs (2)
1122-1131: LGTM!Also applies to: 1161-1212, 1250-1304
1495-1545: LGTM!Also applies to: 1872-1888
crates/core/tests/fixtures/native_plugin/src/lib.rs (1)
293-309: LGTM!Also applies to: 651-727, 729-795
crates/core/tests/unit/native_plugin_tests.rs (1)
260-260: LGTM!crates/ffi/build.rs (1)
30-56: LGTM!crates/adaptive/tests/unit/runtime_tests.rs (1)
639-641: LGTM!crates/ffi/src/api/mod.rs (1)
108-115: LGTM!Also applies to: 159-159, 197-197, 251-251, 414-414
crates/node/src/callback_factory.rs (1)
69-88: LGTM!python/nemo_relay/_native.pyi (1)
1675-1740: LGTM!crates/ffi/src/api/llm_registry.rs (1)
5-15: LGTM!Also applies to: 17-105
crates/ffi/nemo_relay.h (1)
135-149: LGTM!Also applies to: 258-267, 467-493, 2570-2605, 2668-2709, 3213-3239
crates/ffi/src/api/event_registry.rs (1)
5-7: LGTM!Also applies to: 47-75, 77-106, 166-199, 201-233
crates/ffi/src/api/tool_registry.rs (1)
5-11: LGTM!Also applies to: 13-38, 40-63, 65-88, 90-113, 115-118, 120-144
crates/ffi/src/callable.rs (6)
261-286:LlmStreamcontinuation still buffers the whole generation into aJson::Array. Incremental delivery is lost for async stream intercepts; same shape asnemo_relay_async_next_invoke_callback(Lines 332-346). Already raised on this file.
136-156: LGTM!
158-204: LGTM!
218-233: LGTM!Also applies to: 287-299
304-363: LGTM!
368-423: LGTM!docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx (2)
139-143:CompleteandPendingare ABI return values and still need monospace. Same wording appears indocs/reference/event-sanitizers.mdxwith different casing. As per coding guidelines, "Format code elements, commands, parameters, package names, and expressions in monospace".Source: Coding guidelines
119-137: LGTM!Also applies to: 145-152
crates/node/src/api/mod.rs (1)
3544-3570: LGTM!crates/node/src/callable.rs (2)
210-245: LGTM!Also applies to: 247-281, 374-442
548-626: LGTM!Also applies to: 663-722, 956-981, 1092-1172
crates/python/src/py_callable.rs (1)
410-426: LGTM!Also applies to: 1109-1177
go/nemo_relay/callbacks.go (1)
192-213: LGTM!Also applies to: 639-672, 674-697
docs/reference/migration-guides.mdx (1)
29-72: LGTM!Also applies to: 276-300
ATTRIBUTIONS-Rust.md (1)
24923-24925: Regenerate the attribution file with the pre-commit hook.These blocks still trigger markdownlint for missing blank lines and an unspecified fenced-code language. Because
ATTRIBUTIONS-Rust.mdis generated, run the hook rather than editing this file manually, then commit its output and verify a clean result. This duplicates the previously reported stale-generated-file issue.As per coding guidelines, run
uv run pre-commit run --all-filesbefore handoff.Also applies to: 25131-25132
Sources: Coding guidelines, Linters/SAST tools
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/node/src/callable.rs (1)
753-766: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRecord request-deserialization failures.
Invalid non-null sanitizer output returns
FlowErrorwithout updatinggetLastCallbackError(), unlike the Promise path. Record theserde_json::from_valueerror too.Proposed fix
- serde_json::from_value(value) - .map(Some) - .map_err(|error| FlowError::Internal(format!( - "JS LLM sanitize request callback failed: failed to deserialize LlmRequest: {error}" - ))) + serde_json::from_value(value) + .map(Some) + .map_err(|error| FlowError::Internal(format!( + "JS LLM sanitize request callback failed: failed to deserialize LlmRequest: {error}" + ))) + .inspect_err(|error| record_callback_error(error.to_string()))🤖 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/node/src/callable.rs` around lines 753 - 766, Update the non-null deserialization path after the null check in the sanitizer callback flow to record the serde_json::from_value error via record_callback_error, matching the existing Promise path behavior, before returning the FlowError. Preserve the current successful Some(value) result and error message.crates/core/src/api/shared.rs (1)
67-84: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPoisoned-lock handling now graceful for the context lock, but the scope-stack lock still panics.
Lines 76-83 added graceful logging + fail-closed
Nonewhencontext.read()fails, addressing the earlier fail-open concern (raw unsanitized event no longer gets published — every caller shown here treatsNoneas "drop the event"). However, line 72'sscope_stack.read().expect("scope stack lock poisoned")will still panic the entire call path if that lock is poisoned, defeating the resilience just added for the sibling lock in the same function.♻️ Proposed fix to fail closed on both locks
- let scope_guard = scope_stack.read().expect("scope stack lock poisoned"); - let context = global_context(); - let state = match context.read() { - Ok(state) => state, - Err(error) => { + let scope_guard = match scope_stack.read() { + Ok(guard) => guard, + Err(error) => { + log::error!( + target: "nemo_relay.runtime", + event = "event_sanitizer_snapshot_failed"; + "Event was dropped because the scope stack lock is poisoned: {error}" + ); + return None; + } + }; + let context = global_context(); + let state = match context.read() { + Ok(state) => state, + Err(error) => { log::error!( target: "nemo_relay.runtime", event = "event_sanitizer_snapshot_failed"; "Event was dropped because the runtime context lock is poisoned: {error}" ); return None; } };🤖 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/shared.rs` around lines 67 - 84, Update snapshot_event_sanitizers to handle scope_stack.read() failure without panicking: log the poisoned-lock error with the same event-sanitizer snapshot context and return None, matching the existing fail-closed handling for context.read(). Remove the expect call while preserving normal scope-stack access behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/reference/event-sanitizers.mdx`:
- Around line 191-195: Update the async sanitizer registration documentation
around the source-first C API description to state that every callback returning
Pending must settle its completion handle exactly once by resolving or
rejecting; otherwise serial publication remains blocked indefinitely. Also
document how cancellation is handled and whether late settlement after
cancellation is ignored or otherwise constrained, without changing the existing
callback or registration API descriptions.
---
Outside diff comments:
In `@crates/core/src/api/shared.rs`:
- Around line 67-84: Update snapshot_event_sanitizers to handle
scope_stack.read() failure without panicking: log the poisoned-lock error with
the same event-sanitizer snapshot context and return None, matching the existing
fail-closed handling for context.read(). Remove the expect call while preserving
normal scope-stack access behavior.
In `@crates/node/src/callable.rs`:
- Around line 753-766: Update the non-null deserialization path after the null
check in the sanitizer callback flow to record the serde_json::from_value error
via record_callback_error, matching the existing Promise path behavior, before
returning the FlowError. Preserve the current successful Some(value) result and
error message.
🪄 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: 2ab99778-f2c3-4400-9762-db9719324f57
📒 Files selected for processing (15)
ATTRIBUTIONS-Rust.mdcrates/core/src/api/llm.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/src/api/shared.rscrates/core/src/stream.rscrates/ffi/build.rscrates/node/src/callable.rscrates/node/src/callback_factory.rscrates/node/tests/tools_tests.mjscrates/python/tests/coverage/coverage_tests.rscrates/python/tests/coverage/py_callable_coverage_tests.rsdocs/reference/event-sanitizers.mdxdocs/reference/migration-guides.mdxpython/nemo_relay/_native.pyipython/tests/test_builtin_codecs.py
💤 Files with no reviewable changes (1)
- ATTRIBUTIONS-Rust.md
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Preview docs
- GitHub Check: Check / Run
🧰 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/node/src/callback_factory.rscrates/ffi/build.rscrates/core/src/stream.rscrates/python/tests/coverage/py_callable_coverage_tests.rscrates/core/src/api/shared.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/node/src/callable.rscrates/core/src/api/llm.rscrates/python/tests/coverage/coverage_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/node/src/callback_factory.rspython/tests/test_builtin_codecs.pycrates/ffi/build.rscrates/core/src/stream.rscrates/python/tests/coverage/py_callable_coverage_tests.rscrates/core/src/api/shared.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/node/src/callable.rscrates/core/src/api/llm.rscrates/python/tests/coverage/coverage_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/node/src/callback_factory.rspython/tests/test_builtin_codecs.pycrates/node/tests/tools_tests.mjscrates/ffi/build.rscrates/core/src/stream.rscrates/python/tests/coverage/py_callable_coverage_tests.rscrates/core/src/api/shared.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/node/src/callable.rscrates/core/src/api/llm.rscrates/python/tests/coverage/coverage_tests.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/node/src/callback_factory.rspython/tests/test_builtin_codecs.pycrates/ffi/build.rscrates/core/src/stream.rscrates/python/tests/coverage/py_callable_coverage_tests.rscrates/core/src/api/shared.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/node/src/callable.rscrates/core/src/api/llm.rscrates/python/tests/coverage/coverage_tests.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/node/src/callback_factory.rscrates/ffi/build.rscrates/core/src/stream.rscrates/python/tests/coverage/py_callable_coverage_tests.rscrates/core/src/api/shared.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/node/src/callable.rscrates/core/src/api/llm.rscrates/python/tests/coverage/coverage_tests.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/node/src/callback_factory.rspython/tests/test_builtin_codecs.pycrates/core/src/stream.rscrates/core/src/api/shared.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/node/src/callable.rscrates/core/src/api/llm.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/node/src/callback_factory.rsdocs/reference/event-sanitizers.mdxpython/tests/test_builtin_codecs.pycrates/node/tests/tools_tests.mjscrates/ffi/build.rscrates/core/src/stream.rscrates/python/tests/coverage/py_callable_coverage_tests.rscrates/core/src/api/shared.rspython/nemo_relay/_native.pyidocs/reference/migration-guides.mdxcrates/core/src/api/runtime/subscriber_dispatcher.rscrates/node/src/callable.rscrates/core/src/api/llm.rscrates/python/tests/coverage/coverage_tests.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/node/src/callback_factory.rspython/tests/test_builtin_codecs.pycrates/ffi/build.rscrates/core/src/stream.rscrates/python/tests/coverage/py_callable_coverage_tests.rscrates/core/src/api/shared.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/node/src/callable.rscrates/core/src/api/llm.rscrates/python/tests/coverage/coverage_tests.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/node/src/callback_factory.rspython/tests/test_builtin_codecs.pycrates/ffi/build.rscrates/core/src/stream.rscrates/python/tests/coverage/py_callable_coverage_tests.rscrates/core/src/api/shared.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/node/src/callable.rscrates/core/src/api/llm.rscrates/python/tests/coverage/coverage_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/node/src/callback_factory.rscrates/node/tests/tools_tests.mjscrates/ffi/build.rscrates/python/tests/coverage/py_callable_coverage_tests.rscrates/node/src/callable.rscrates/python/tests/coverage/coverage_tests.rs
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
In MDX files, top-of-file comments must use JSX comment delimiters (
{/*to open and*/}to close); do not use HTML comments for MDX SPDX headers
Files:
docs/reference/event-sanitizers.mdxdocs/reference/migration-guides.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/reference/event-sanitizers.mdxdocs/reference/migration-guides.mdx
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
docs/reference/event-sanitizers.mdxdocs/reference/migration-guides.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/reference/event-sanitizers.mdxdocs/reference/migration-guides.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/reference/event-sanitizers.mdxdocs/reference/migration-guides.mdx
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.
Flag stale examples, missing SPDX headers where required, and instructions that no longer match CI or pre-commit behavior.
Files:
docs/reference/event-sanitizers.mdxdocs/reference/migration-guides.mdx
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: When changing the Python wrapper package, tests, or docs tooling, lint with Ruff (E,F,W,I), format with Ruff formatter (120-character lines, double quotes), and passtytype checking.
Add the SPDX license header to all Python source files using the#comment form.
Files:
python/tests/test_builtin_codecs.py
**/*.{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:
python/tests/test_builtin_codecs.py
python/tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
python/tests/**/*.py: Pytest is used to run tests.
Do not add@pytest.mark.asyncioto any test; async tests are automatically detected and run by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, with thespecconstructor argument when necessary.
Name mocked classes with themockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; if a fixture is needed in multiple test files, place it in aconftest.pyfile.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the fixture function asdef <fixture_name>_fixture() -> <return_type>:; only specifyscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating individual tests for different input types.
Files:
python/tests/test_builtin_codecs.py
{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:
python/tests/test_builtin_codecs.pycrates/node/tests/tools_tests.mjscrates/python/tests/coverage/py_callable_coverage_tests.rscrates/python/tests/coverage/coverage_tests.rs
crates/node/**/*.{js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
camelCasefor Node.js public APIs.
Files:
crates/node/tests/tools_tests.mjs
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/build.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/build.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/stream.rscrates/core/src/api/shared.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/src/api/llm.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/src/stream.rscrates/core/src/api/shared.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/src/api/llm.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/src/stream.rscrates/core/src/api/shared.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/src/api/llm.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/src/stream.rscrates/core/src/api/shared.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/src/api/llm.rs
crates/core/src/{api/**/*.rs,api/runtime/**/*.rs,codec/**/*.rs,json.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Implement the new or changed public runtime behavior first in the Rust core, especially under
crates/core/src/api/and related core modules such ascrates/core/src/api/runtime/,crates/core/src/codec/, andcrates/core/src/json.rs.
Files:
crates/core/src/api/shared.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/src/api/llm.rs
crates/core/src/api/{tool,llm,shared,scope}.rs
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Wire the new middleware chain into the appropriate lifecycle owner and pipeline stage: tool and LLM execution paths use
tool.rsorllm.rs; shared mark and scope event sanitization usesshared.rsand is called fromscope.rs.
Files:
crates/core/src/api/shared.rscrates/core/src/api/llm.rs
crates/core/src/api/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Preserve the documented pipeline order: conditional guardrails, request intercepts, request sanitization, execution intercepts, and response sanitization for tool and LLM execution; specialized sanitization, event creation, and dispatch for mark and scope events.
Files:
crates/core/src/api/shared.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/src/api/llm.rs
{crates/python/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update the language-native bindings for every exposed surface in Python, Go, and Node.js.
Files:
python/nemo_relay/_native.pyi
{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update language wrapper helpers such as Python wrapper modules, Python type stubs, and Go shorthand packages when the new behavior belongs in those helper layers.
Files:
python/nemo_relay/_native.pyi
python/nemo_relay/**/*
⚙️ CodeRabbit configuration file
python/nemo_relay/**/*: Review Python wrapper changes for typed API consistency, contextvars-based scope isolation, async behavior, and parity with the native extension.
Stubs and runtime implementations should stay aligned.
Files:
python/nemo_relay/_native.pyi
🔇 Additional comments (19)
crates/core/src/api/llm.rs (1)
839-911: LGTM!crates/core/src/api/runtime/subscriber_dispatcher.rs (1)
288-290: LGTM!crates/core/src/stream.rs (1)
351-359: LGTM!crates/ffi/build.rs (1)
21-30: LGTM!crates/node/src/callable.rs (1)
799-805: LGTM!python/tests/test_builtin_codecs.py (1)
435-456: LGTM!crates/node/tests/tools_tests.mjs (2)
611-623: Async success tests still omit rejection coverage.The existing finding remains applicable: add cases where the conditional guardrail rejects and the request intercept throws or returns a rejected Promise, including propagation and cleanup assertions. As per path instructions, tests should cover promised API error paths.
Also applies to: 798-810
Source: Path instructions
963-979: LGTM!crates/node/src/callback_factory.rs (1)
69-77: LGTM!Also applies to: 78-84
docs/reference/event-sanitizers.mdx (2)
56-69: LGTM!
197-202: LGTM!docs/reference/migration-guides.mdx (4)
3-3: 🎯 Functional CorrectnessVerify the omitted migration examples against the current API contracts.
The supplied excerpt does not include these ranges. Confirm that the async callback table, migration text, and pinned
Box::pin(async move { ... })example match the current Rust and worker APIs before publishing them.Also applies to: 16-73, 99-103
Source: Path instructions
202-210: LGTM!Also applies to: 219-227
274-281: 🗄️ Data Integrity & IntegrationVerify the ABI v3 fallback wording against the loader contract.
This text makes a compatibility guarantee about preserving the v2 table prefix and retrying a legacy v2 table after v3 rejection. Confirm it matches the native loader and header behavior exactly, since incorrect migration guidance can strand existing plugins.
Source: Path instructions
298-298: 📐 Maintainability & Code QualityRun
just docs-linkcheckfor the updated ABI link.As per coding guidelines, changed documentation links require
just docs-linkcheck.Source: Coding guidelines
crates/python/tests/coverage/py_callable_coverage_tests.rs (1)
113-215: LGTM!crates/core/src/api/shared.rs (1)
48-59: LGTM!Also applies to: 221-296
python/nemo_relay/_native.pyi (1)
40-62: LGTM!Also applies to: 1675-1734
crates/python/tests/coverage/coverage_tests.rs (1)
664-749: LGTM!
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/node/src/callable.rs (1)
735-735: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPreserve serialization failures instead of converting them into omission values.
Both adapters turn serialization errors into
null/None, allowing sanitizer processing to continue with incomplete input and potentially clear fields. Return aFlowErrorand record it at each site.
crates/node/src/callable.rs#L735-L735: replaceunwrap_or(Json::Null)with fallible serialization and error propagation.crates/node/src/callable.rs#L461-L466: replaceand_then(...ok())with fallibletranspose()handling forcategory_profile.As per path instructions, binding changes must preserve stable async and type-conversion semantics.
🤖 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/node/src/callable.rs` at line 735, The request serialization at crates/node/src/callable.rs#L735 must propagate serialization failures as FlowError instead of converting them to Json::Null; record the error while preserving the existing async and type-conversion behavior. At crates/node/src/callable.rs#L461-L466, update category_profile serialization to use fallible transpose handling and propagate/record errors rather than dropping them through and_then(...).ok().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/api/shared.rs`:
- Around line 72-80: Apply the same fail-closed poisoned-lock handling to the
scope_stack reads in the public mark emission path at scope.rs methods around
the existing expect calls near lines 360 and 365. Replace those panicking expect
calls with error handling that logs the poisoned lock and drops the event,
matching the None-return behavior already used by the shared snapshot helper and
preserving normal successful reads.
In `@docs/reference/event-sanitizers.mdx`:
- Around line 191-197: Update the migration guidance near the source-first C API
discussion to state that callbacks returning Pending must call
nemo_relay_async_completion_release after resolving or rejecting the completion
handle, ensuring the callback-owned reference is released while preserving the
existing exactly-once settlement requirement.
---
Outside diff comments:
In `@crates/node/src/callable.rs`:
- Line 735: The request serialization at crates/node/src/callable.rs#L735 must
propagate serialization failures as FlowError instead of converting them to
Json::Null; record the error while preserving the existing async and
type-conversion behavior. At crates/node/src/callable.rs#L461-L466, update
category_profile serialization to use fallible transpose handling and
propagate/record errors rather than dropping them through and_then(...).ok().
🪄 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: 5167972f-c0da-480d-90e8-8fc856e6bad7
📒 Files selected for processing (4)
crates/core/src/api/shared.rscrates/node/src/callable.rscrates/node/tests/tools_tests.mjsdocs/reference/event-sanitizers.mdx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (25)
crates/node/**/*.{js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
camelCasefor Node.js public APIs.
Files:
crates/node/tests/tools_tests.mjs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/node/tests/tools_tests.mjscrates/core/src/api/shared.rscrates/node/src/callable.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/node/tests/tools_tests.mjsdocs/reference/event-sanitizers.mdxcrates/core/src/api/shared.rscrates/node/src/callable.rs
crates/{python,ffi,node}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node}/**/*: Treat binding changes as public API changes. Check for parity with the other language bindings, FFI ownership/lifetime safety,
callback error propagation, stable type conversion, and consistent async/stream semantics.
Flag changes that update one binding without corresponding tests or documentation for the same surface elsewhere.
Files:
crates/node/tests/tools_tests.mjscrates/node/src/callable.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/node/tests/tools_tests.mjs
**/*.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/reference/event-sanitizers.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/reference/event-sanitizers.mdx
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
docs/reference/event-sanitizers.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/reference/event-sanitizers.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/reference/event-sanitizers.mdx
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.
Flag stale examples, missing SPDX headers where required, and instructions that no longer match CI or pre-commit behavior.
Files:
docs/reference/event-sanitizers.mdx
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node 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/shared.rscrates/node/src/callable.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/shared.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/shared.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/src/api/shared.rscrates/node/src/callable.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/shared.rscrates/node/src/callable.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/shared.rscrates/node/src/callable.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/shared.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/shared.rscrates/node/src/callable.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/shared.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/core/src/api/shared.rscrates/node/src/callable.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/shared.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/shared.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/shared.rscrates/node/src/callable.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/shared.rs
🔇 Additional comments (4)
crates/node/src/callable.rs (1)
210-246: LGTM!Also applies to: 247-269, 271-281, 283-295, 297-341, 343-371, 373-395, 397-442, 444-460, 467-509, 552-570, 579-597, 606-624, 668-718, 732-734, 736-767, 773-785, 794-807, 961-980, 1097-1171
docs/reference/event-sanitizers.mdx (1)
56-69: LGTM!Also applies to: 200-205
crates/node/tests/tools_tests.mjs (1)
624-637: LGTM!Also applies to: 818-846
crates/core/src/api/shared.rs (1)
72-80: 📐 Maintainability & Code QualityRun the required core-runtime validation matrix.
Provide successful
cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,just test-rust,cargo deny check,validate-change, and the configured Rust/Python/Go/Node matrix before merge.As per coding guidelines, “Any Rust change must run
just test-rust,” “Any Rust change must runcargo fmt --all,” and “Changes tocrates/coreorcrates/adaptivemust run the full language matrix.”Source: Coding guidelines
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
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)
go/nemo_relay/callbacks.go (1)
194-216: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winWait for the cancellation poller before releasing
completion.Line 213 stops the poller but does not join it. It can still be inside
nemo_relay_async_completion_is_cancelledwhen the caller’s next defer releasescompletion, causing a use-after-free across cgo.Proposed fix
func contextForCompletion(completion *C.NemoRelayAsyncCompletion) (context.Context, func()) { ctx, cancel := context.WithCancel(context.Background()) done := make(chan struct{}) + stopped := make(chan struct{}) var doneOnce sync.Once go func() { + defer close(stopped) ticker := time.NewTicker(10 * time.Millisecond) defer ticker.Stop() for { @@ return ctx, func() { - doneOnce.Do(func() { close(done) }) - cancel() + doneOnce.Do(func() { + close(done) + cancel() + }) + <-stopped } }🤖 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/callbacks.go` around lines 194 - 216, Update contextForCompletion so its returned cleanup function stops and joins the polling goroutine before returning, ensuring no call to nemo_relay_async_completion_is_cancelled remains in progress before completion can be released. Preserve the existing once-only cleanup and context cancellation behavior.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@go/nemo_relay/callbacks.go`:
- Around line 194-216: Update contextForCompletion so its returned cleanup
function stops and joins the polling goroutine before returning, ensuring no
call to nemo_relay_async_completion_is_cancelled remains in progress before
completion can be released. Preserve the existing once-only cleanup and context
cancellation behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 6fee23b3-4b8c-4f00-8b6e-b5bcd7d0ee46
📒 Files selected for processing (17)
crates/core/src/api/llm.rscrates/core/src/api/runtime/state.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/tests/unit/llm_api_tests.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/scope_registry.rscrates/ffi/src/callable.rscrates/node/src/callable.rscrates/plugin/src/lib.rscrates/python/src/py_api/mod.rscrates/python/src/py_callable.rscrates/python/tests/coverage/py_callable_coverage_tests.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdxdocs/reference/event-sanitizers.mdxdocs/reference/migration-guides.mdxgo/nemo_relay/callbacks.gopython/tests/test_llm.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (43)
**/*.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.mdxdocs/reference/event-sanitizers.mdxdocs/reference/migration-guides.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.mdxdocs/reference/event-sanitizers.mdxdocs/reference/migration-guides.mdx
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxdocs/reference/event-sanitizers.mdxdocs/reference/migration-guides.mdx
{crates/core/src/plugin/dynamic/**,crates/plugin/**,crates/worker/**,crates/worker-proto/**,crates/types/**,python/plugin/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Keep the stable boundary explicit: native plugins cross a C ABI, and worker plugins cross
grpc-v1.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/plugin/src/lib.rs
{crates/core/src/plugin/dynamic/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Native and worker plugins are trusted extensions; document that native plugins are in-process and unsandboxed, and worker plugins provide process isolation but not a security sandbox.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
{docs/build-plugins/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
When detailed dynamic plugin guides exist, keep Rust native, Python worker, and
grpc-v1protocol details on separate pages.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxdocs/reference/event-sanitizers.mdxdocs/reference/migration-guides.mdx
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxdocs/reference/event-sanitizers.mdxcrates/python/tests/coverage/py_callable_coverage_tests.rscrates/plugin/src/lib.rscrates/core/tests/unit/llm_api_tests.rsgo/nemo_relay/callbacks.godocs/reference/migration-guides.mdxcrates/python/src/py_api/mod.rscrates/ffi/src/api/scope_registry.rscrates/ffi/nemo_relay.hcrates/core/src/api/runtime/subscriber_dispatcher.rscrates/ffi/src/callable.rscrates/core/src/api/runtime/state.rscrates/core/src/api/llm.rspython/tests/test_llm.pycrates/node/src/callable.rscrates/python/src/py_callable.rs
docs/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If documentation examples or commands under
docs/change, run the targeted docs checks appropriate to the change.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxdocs/reference/event-sanitizers.mdxdocs/reference/migration-guides.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.mdxdocs/reference/event-sanitizers.mdxdocs/reference/migration-guides.mdx
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node 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/python/tests/coverage/py_callable_coverage_tests.rscrates/plugin/src/lib.rscrates/core/tests/unit/llm_api_tests.rscrates/python/src/py_api/mod.rscrates/ffi/src/api/scope_registry.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/ffi/src/callable.rscrates/core/src/api/runtime/state.rscrates/core/src/api/llm.rscrates/node/src/callable.rscrates/python/src/py_callable.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/python/tests/coverage/py_callable_coverage_tests.rscrates/plugin/src/lib.rscrates/core/tests/unit/llm_api_tests.rscrates/python/src/py_api/mod.rscrates/ffi/src/api/scope_registry.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/ffi/src/callable.rscrates/core/src/api/runtime/state.rscrates/core/src/api/llm.rspython/tests/test_llm.pycrates/node/src/callable.rscrates/python/src/py_callable.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/python/tests/coverage/py_callable_coverage_tests.rscrates/plugin/src/lib.rscrates/core/tests/unit/llm_api_tests.rscrates/python/src/py_api/mod.rscrates/ffi/src/api/scope_registry.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/ffi/src/callable.rscrates/core/src/api/runtime/state.rscrates/core/src/api/llm.rspython/tests/test_llm.pycrates/node/src/callable.rscrates/python/src/py_callable.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/python/tests/coverage/py_callable_coverage_tests.rscrates/plugin/src/lib.rscrates/core/tests/unit/llm_api_tests.rsgo/nemo_relay/callbacks.gocrates/python/src/py_api/mod.rscrates/ffi/src/api/scope_registry.rscrates/ffi/nemo_relay.hcrates/core/src/api/runtime/subscriber_dispatcher.rscrates/ffi/src/callable.rscrates/core/src/api/runtime/state.rscrates/core/src/api/llm.rspython/tests/test_llm.pycrates/node/src/callable.rscrates/python/src/py_callable.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/python/tests/coverage/py_callable_coverage_tests.rscrates/plugin/src/lib.rscrates/core/tests/unit/llm_api_tests.rsgo/nemo_relay/callbacks.gocrates/python/src/py_api/mod.rscrates/ffi/src/api/scope_registry.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/ffi/src/callable.rscrates/core/src/api/runtime/state.rscrates/core/src/api/llm.rscrates/node/src/callable.rscrates/python/src/py_callable.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/python/tests/coverage/py_callable_coverage_tests.rscrates/plugin/src/lib.rscrates/core/tests/unit/llm_api_tests.rsgo/nemo_relay/callbacks.gocrates/python/src/py_api/mod.rscrates/ffi/src/api/scope_registry.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/ffi/src/callable.rscrates/core/src/api/runtime/state.rscrates/core/src/api/llm.rspython/tests/test_llm.pycrates/node/src/callable.rscrates/python/src/py_callable.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/python/tests/coverage/py_callable_coverage_tests.rscrates/plugin/src/lib.rscrates/core/tests/unit/llm_api_tests.rsgo/nemo_relay/callbacks.gocrates/python/src/py_api/mod.rscrates/ffi/src/api/scope_registry.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/ffi/src/callable.rscrates/core/src/api/runtime/state.rscrates/core/src/api/llm.rspython/tests/test_llm.pycrates/node/src/callable.rscrates/python/src/py_callable.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/python/tests/coverage/py_callable_coverage_tests.rscrates/python/src/py_api/mod.rscrates/ffi/src/api/scope_registry.rscrates/ffi/nemo_relay.hcrates/ffi/src/callable.rscrates/node/src/callable.rscrates/python/src/py_callable.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/python/tests/coverage/py_callable_coverage_tests.rscrates/core/tests/unit/llm_api_tests.rspython/tests/test_llm.py
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/plugin/src/lib.rscrates/python/src/py_api/mod.rscrates/ffi/src/api/scope_registry.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/ffi/src/callable.rscrates/core/src/api/runtime/state.rscrates/core/src/api/llm.rspython/tests/test_llm.pycrates/node/src/callable.rscrates/python/src/py_callable.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.rs
{crates/plugin/**/*.rs,python/plugin/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Rust and Python SDKs must expose every supported registration surface.
Files:
crates/plugin/src/lib.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/tests/unit/llm_api_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/src/api/runtime/state.rscrates/core/src/api/llm.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/tests/unit/llm_api_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/src/api/runtime/state.rscrates/core/src/api/llm.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/tests/unit/llm_api_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/src/api/runtime/state.rscrates/core/src/api/llm.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/tests/unit/llm_api_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/src/api/runtime/state.rscrates/core/src/api/llm.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/callbacks.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/callbacks.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/callbacks.gocrates/python/src/py_api/mod.rs
{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/callbacks.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/callbacks.gopython/tests/test_llm.py
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/callbacks.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/callbacks.go
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/scope_registry.rscrates/ffi/nemo_relay.hcrates/ffi/src/callable.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/scope_registry.rscrates/ffi/src/callable.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/scope_registry.rscrates/ffi/nemo_relay.h
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
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/subscriber_dispatcher.rscrates/core/src/api/runtime/state.rscrates/core/src/api/llm.rs
crates/core/src/api/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Preserve the documented pipeline order: conditional guardrails, request intercepts, request sanitization, execution intercepts, and response sanitization for tool and LLM execution; specialized sanitization, event creation, and dispatch for mark and scope events.
Files:
crates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/src/api/runtime/state.rscrates/core/src/api/llm.rs
crates/core/src/api/runtime/state.rs
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
crates/core/src/api/runtime/state.rs: Add aSortedRegistry<GuardrailEntry<...>>orSortedRegistry<Intercept<...>>field toNemoRelayContextStatefor the new middleware type.
Add chain-execution helpers toNemoRelayContextState, following existing helpers such astool_sanitize_request_chainortool_request_intercepts_chain.
Files:
crates/core/src/api/runtime/state.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
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: When changing the Python wrapper package, tests, or docs tooling, lint with Ruff (E,F,W,I), format with Ruff formatter (120-character lines, double quotes), and passtytype checking.
Add the SPDX license header to all Python source files using the#comment form.
Files:
python/tests/test_llm.py
python/tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
python/tests/**/*.py: Pytest is used to run tests.
Do not add@pytest.mark.asyncioto any test; async tests are automatically detected and run by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, with thespecconstructor argument when necessary.
Name mocked classes with themockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; if a fixture is needed in multiple test files, place it in aconftest.pyfile.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the fixture function asdef <fixture_name>_fixture() -> <return_type>:; only specifyscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating individual tests for different input types.
Files:
python/tests/test_llm.py
🔇 Additional comments (9)
python/tests/test_llm.py (1)
170-170: LGTM!Also applies to: 279-302, 303-326, 327-348, 349-369, 404-404, 416-416
crates/core/src/api/runtime/state.rs (1)
647-648: 📐 Maintainability & Code QualityAttach the required cross-layer validation results.
crates/core/src/api/runtime/state.rs#L647-L648: run Rust formatting, strict Clippy,just test-rust,validate-change, and the core full language matrix.crates/core/src/api/llm.rs#L710-L712: include lifecycle and credential-redaction coverage in that matrix.crates/core/src/api/runtime/subscriber_dispatcher.rs#L168-L190: include async dispatch, ordering, and failure-path coverage.crates/python/src/py_api/mod.rs#L1373-L1381: run the Python binding test target.crates/python/src/py_callable.rs#L1037-L1041: run Python async-callback coverage.crates/core/tests/unit/llm_api_tests.rs#L298-L298: include this updated async sanitizer test.docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx#L121-L145: run full pre-commit validation.docs/reference/migration-guides.mdx#L209-L210: runjust docs-linkcheckfor the changed documentation links.As per coding guidelines, “Any Rust change must run
just test-rust,” “Ifcrates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js,” and “Before review or handoff, runuv run pre-commit run --all-files.”Source: Coding guidelines
crates/ffi/src/callable.rs (1)
105-109: LGTM!crates/plugin/src/lib.rs (1)
832-834: LGTM!Also applies to: 2390-2407
crates/ffi/nemo_relay.h (1)
489-493: LGTM!crates/ffi/src/api/scope_registry.rs (1)
33-67: LGTM!Also applies to: 113-141
crates/node/src/callable.rs (1)
454-512: LGTM!crates/python/tests/coverage/py_callable_coverage_tests.rs (1)
703-728: LGTM!docs/reference/event-sanitizers.mdx (1)
52-54: 📐 Maintainability & Code QualityNo action needed.
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
/coderabbit review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/adaptive/src/test_support.rs`:
- Around line 6-12: Move the test-only block_on helper from the production
test_support module into the existing test-support module under
crates/adaptive/tests, preserving its current implementation and imports. Remove
the production src definition and update test references to use the relocated
helper.
In `@crates/ffi/build.rs`:
- Around line 6-8: Update the Rust validation workflow associated with main and
validate_async_registration_parity to run and report cargo fmt --all, cargo
clippy --workspace --all-targets -- -D warnings, just test-rust, the complete
core/adaptive language matrix, and FFI surface/header validation; include the
results for each required check.
- Around line 39-97: Update validate_async_registration_parity to validate
complete async ABI prototypes rather than only function names and callback
typedefs. Parse or derive structured declarations containing each exported
function’s full return type, parameter types and order, including scope_uuid,
break_chain, and ownership parameters, then compare them against
ASYNC_REGISTRATIONS so any nemo_relay.h prototype drift fails validation.
🪄 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: acced103-fd3d-4e5f-9a7b-bacd4cbeaa82
📒 Files selected for processing (13)
crates/adaptive/src/lib.rscrates/adaptive/src/test_support.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/core/src/api/llm.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/tests/integration/pipeline_tests.rscrates/core/tests/integration/test_support.rscrates/ffi/build.rscrates/ffi/nemo_relay.hdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (32)
**/*.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/adaptive/src/test_support.rscrates/core/tests/integration/test_support.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/ffi/build.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/core/tests/integration/pipeline_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/adaptive/src/test_support.rscrates/core/tests/integration/test_support.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/core/tests/integration/pipeline_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/adaptive/src/test_support.rscrates/core/tests/integration/test_support.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/ffi/build.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/core/tests/integration/pipeline_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.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/adaptive/src/test_support.rscrates/core/tests/integration/test_support.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/ffi/build.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/core/tests/integration/pipeline_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.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/adaptive/src/test_support.rscrates/core/tests/integration/test_support.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/ffi/build.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/ffi/nemo_relay.hcrates/core/tests/integration/pipeline_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.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/adaptive/src/test_support.rscrates/core/tests/integration/test_support.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/ffi/build.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/core/tests/integration/pipeline_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.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/adaptive/src/test_support.rscrates/adaptive/src/lib.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/src/api/llm.rs
crates/adaptive/**
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep
crates/adaptivealigned with the canonical adaptive config schema, built-in section helpers, plugin lifecycle, and validation/report behavior.
Files:
crates/adaptive/src/test_support.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/adaptive/src/test_support.rscrates/core/tests/integration/test_support.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/ffi/build.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/ffi/nemo_relay.hcrates/core/tests/integration/pipeline_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/adaptive/src/test_support.rscrates/core/tests/integration/test_support.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/core/tests/integration/pipeline_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/adaptive/src/test_support.rscrates/core/tests/integration/test_support.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/ffi/build.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/core/tests/integration/pipeline_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/adaptive/src/test_support.rscrates/core/tests/integration/test_support.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/ffi/build.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/core/tests/integration/pipeline_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/adaptive/src/test_support.rscrates/core/tests/integration/test_support.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/core/tests/integration/pipeline_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/tests/integration/test_support.rscrates/core/tests/integration/pipeline_tests.rscrates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/src/api/llm.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/test_support.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/core/tests/integration/pipeline_tests.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/integration/middleware_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/build.rscrates/ffi/nemo_relay.h
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/build.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/build.rscrates/ffi/nemo_relay.h
**/*.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.mdx
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
{crates/core/src/plugin/dynamic/**,crates/plugin/**,crates/worker/**,crates/worker-proto/**,crates/types/**,python/plugin/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Keep the stable boundary explicit: native plugins cross a C ABI, and worker plugins cross
grpc-v1.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.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.mdx
{docs/build-plugins/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
When detailed dynamic plugin guides exist, keep Rust native, Python worker, and
grpc-v1protocol details on separate pages.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
docs/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If documentation examples or commands under
docs/change, run the targeted docs checks appropriate to the change.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
{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/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
{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/nemo_relay.h
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/subscriber_dispatcher.rscrates/core/src/api/llm.rs
crates/core/src/api/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Preserve the documented pipeline order: conditional guardrails, request intercepts, request sanitization, execution intercepts, and response sanitization for tool and LLM execution; specialized sanitization, event creation, and dispatch for mark and scope events.
Files:
crates/core/src/api/runtime/subscriber_dispatcher.rscrates/core/src/api/llm.rs
crates/core/src/api/{tool,llm,shared,scope}.rs
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Wire the new middleware chain into the appropriate lifecycle owner and pipeline stage: tool and LLM execution paths use
tool.rsorllm.rs; shared mark and scope event sanitization usesshared.rsand is called fromscope.rs.
Files:
crates/core/src/api/llm.rs
🧠 Learnings (1)
📚 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/subscriber_dispatcher.rs
🔇 Additional comments (11)
crates/adaptive/src/lib.rs (1)
17-18: 🎯 Functional CorrectnessDrop this concern The
adaptive_hints_intercept_tests.rsfile is included as a unit test fromcrates/adaptive/src/adaptive_hints_intercept.rs, socrate::test_support::block_onis available in the crate test context.> Likely an incorrect or invalid review comment.crates/core/src/api/runtime/subscriber_dispatcher.rs (1)
54-54: LGTM!Also applies to: 98-355
crates/core/src/api/llm.rs (1)
23-24: LGTM!Also applies to: 549-571, 860-860
crates/ffi/nemo_relay.h (1)
3221-3225: LGTM!docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx (1)
151-156: LGTM!crates/adaptive/tests/unit/acg_component_tests.rs (1)
1090-1095: LGTM!Also applies to: 1338-1343
crates/adaptive/tests/unit/adaptive_hints_intercept_tests.rs (1)
199-207: LGTM!Also applies to: 269-277, 308-316
crates/core/tests/integration/api_surface_tests.rs (1)
10-11: LGTM!Also applies to: 71-71, 710-758
crates/core/tests/integration/middleware_tests.rs (1)
16-17: LGTM!crates/core/tests/integration/pipeline_tests.rs (1)
12-13: LGTM!crates/core/tests/integration/test_support.rs (1)
7-18: LGTM!
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
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/ffi/build.rs`:
- Around line 69-74: In the async declaration collection in build.rs, replace
the deduplication performed after sorting with validation that rejects any
adjacent declarations sharing the same name. Perform this uniqueness check
before signature validation, and preserve the declarations for subsequent
processing instead of silently discarding duplicates.
🪄 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: c545ce61-ccee-4efc-b9f4-5332952e8070
📒 Files selected for processing (4)
crates/adaptive/src/lib.rscrates/adaptive/tests/support/mod.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/ffi/build.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 (17)
**/*.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/adaptive/tests/support/mod.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/ffi/build.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/adaptive/tests/support/mod.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/adaptive/tests/support/mod.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/ffi/build.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/adaptive/tests/support/mod.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/ffi/build.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/adaptive/tests/support/mod.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/ffi/build.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/adaptive/tests/support/mod.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/ffi/build.rs
crates/adaptive/**
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep
crates/adaptivealigned with the canonical adaptive config schema, built-in section helpers, plugin lifecycle, and validation/report behavior.
Files:
crates/adaptive/tests/support/mod.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/adaptive/tests/support/mod.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/ffi/build.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/adaptive/tests/support/mod.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/adaptive/tests/support/mod.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/ffi/build.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/adaptive/tests/support/mod.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rscrates/ffi/build.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/adaptive/tests/support/mod.rscrates/adaptive/src/lib.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/adaptive/tests/support/mod.rscrates/adaptive/tests/unit/adaptive_hints_intercept_tests.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/adaptive/src/lib.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/build.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/build.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/build.rs
🔇 Additional comments (4)
crates/adaptive/src/lib.rs (1)
18-18: LGTM!crates/adaptive/tests/support/mod.rs (1)
1-12: LGTM!crates/adaptive/tests/unit/adaptive_hints_intercept_tests.rs (1)
7-7: LGTM!Also applies to: 200-207, 270-277, 309-316
crates/ffi/build.rs (1)
36-68: LGTM!Also applies to: 84-160
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Overview
Add async middleware support across the Rust runtime and all supported bindings while preserving synchronous scope, mark, and manual lifecycle event-emission APIs.
Warning
BREAKING CHANGE: Rust middleware callbacks now return futures. Update guardrails, request/execution intercepts, tool/LLM sanitizers, and event sanitizers to return
Box::pin(async move { ... })(or an equivalent future). Rebuild native plugins against ABI v3 and migrate raw C/Go consumers that need asynchronous callbacks to the completion-based APIs. Python and Node.js support awaitables/Promises additively; scope, mark, and manual lifecycle event-emission APIs remain synchronous.Details
Where should the reviewer start?
Start with
crates/core/src/api/runtime/callbacks.rsandcrates/core/src/api/runtime/subscriber_dispatcher.rsfor the asynchronous middleware contract and delayed event publication. Then reviewcrates/ffi/src/callable.rsand the Python/Node callback bridges.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit