fix(python): preserve panic settlement after loop shutdown fix - #682
Conversation
github-actions
Bot
added
size:M
|
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 (2)
📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🧰 Additional context used📓 Path-based instructions (15)**/*.rs📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Files:
{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)
Files:
{crates/**/src/**/*.rs,python/**/*.py}📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.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:
**/*.{rs,py,js,jsx,ts,tsx,go,c,h,cc,cpp,md,toml,yml,yaml,sh}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,py}📄 CodeRabbit inference engine (AGENTS.md)
Files:
crates/**/*.rs📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,py,js,mjs,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{rs,py,go,js,jsx,ts,tsx,c,h,html,md,mdx,toml}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
crates/{python,ffi,node}/**/*⚙️ CodeRabbit configuration file
Files:
**/{test,tests}/**/*📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (1)📚 Learning: 2026-08-03T19:55:03.931ZApplied to files:
🔇 Additional comments (5)
WalkthroughThe PR updates ChangesSafe future bridge
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PythonEventLoop
participant TokioTask
participant RustFuture
PythonEventLoop->>TokioTask: Start safe_future_into_py
TokioTask->>RustFuture: Execute bridged future
RustFuture-->>TokioTask: Return result or panic
TokioTask-->>PythonEventLoop: Return result or RustPanic
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Signed-off-by: Will Killian <wkillian@nvidia.com>
47d1bbf to
a8dcb8c
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/python/src/py_api/mod.rs`:
- Around line 137-150: Update the async task handling around the
`tokio::select!` in the Python API so the spawned `JoinHandle` is stored and
selected by mutable reference rather than detached. When `cancel_receiver` wins,
explicitly call `handle.abort()` before returning; preserve the existing result
and panic conversion for completed tasks, and add a regression test proving
cancellation prevents the Rust-side tool work from completing.
In `@crates/python/tests/coverage/py_api_coverage_tests.rs`:
- Around line 41-63: Add a regression test alongside
safe_future_into_py_settles_rust_panics that closes the event loop before the
Rust future settles, then drive or await the task as appropriate. Verify
safe_future_into_py completes without scheduling post-closure completion and
does not produce an unraisable callback error, covering the closed-loop error
path.
🪄 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: 693737a3-e1dc-4f4d-a014-aec5b71d51ca
📒 Files selected for processing (2)
crates/python/src/py_api/mod.rscrates/python/tests/coverage/py_api_coverage_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (15)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node work
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
**/*.rs: Format Rust code with rustfmt defaults usingcargo fmt.
Runcargo clippy -- -D warnings; all Rust warnings must be treated as errors.
Use Rustsnake_casenaming conventions.
Files:
crates/python/tests/coverage/py_api_coverage_tests.rscrates/python/src/py_api/mod.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
**/*: Use release tags in raw Rust-compatible SemVer without a leadingv; tags such asv0.1.0are prohibited.
Use branch prefixesfeat/,fix/,docs/,test/, orrefactor/according to the change purpose.
Every commit in a pull request must include a DCOSigned-off-by:sign-off.
Before submitting a pull request, ensure pre-commit hooks, relevant tests, target-specific builds, documentation updates, and a rebase on the latestmainare complete.
Use commit messages in the formtype: short description, with a valid type and a first line under 72 characters.
Files:
crates/python/tests/coverage/py_api_coverage_tests.rscrates/python/src/py_api/mod.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
crates/python/tests/coverage/py_api_coverage_tests.rscrates/python/src/py_api/mod.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_api_coverage_tests.rscrates/python/src/py_api/mod.rs
**/*.{rs,py,js,jsx,ts,tsx,go,c,h,cc,cpp,md,toml,yml,yaml,sh}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, documentation, scripts, and configuration files; the project is Apache-2.0.
Files:
crates/python/tests/coverage/py_api_coverage_tests.rscrates/python/src/py_api/mod.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Use
snake_casenaming in Rust and Python.
Files:
crates/python/tests/coverage/py_api_coverage_tests.rscrates/python/src/py_api/mod.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: UseJson = serde_json::Valuein Rust-facing runtime APIs where existing code expects JSON payloads.
Treat Rust as the source of truth for runtime behavior; binding APIs should mirror Rust semantics unless a language-specific wrapper intentionally improves ergonomics.
Files:
crates/python/tests/coverage/py_api_coverage_tests.rscrates/python/src/py_api/mod.rs
**/*.{rs,py,js,mjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Preserve the existing Tokio-based asynchronous model and callback/future lifetimes; do not unexpectedly block or hide async work in bindings.
Files:
crates/python/tests/coverage/py_api_coverage_tests.rscrates/python/src/py_api/mod.rs
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}: Run tests for every language affected by a change; changes to the core Rust crate require tests across all bindings.
UseSONAR_IGNORE_START/SONAR_IGNORE_ENDonly for documented false positives, keep ignored blocks minimal, explain them with a comment, and obtain reviewer sign-off.
Preserve the layered architecture in which Rust provides the core runtime and C FFI, PyO3, and NAPI provide bindings that mirror the full API surface.
Files:
crates/python/tests/coverage/py_api_coverage_tests.rscrates/python/src/py_api/mod.rs
**/{test,tests}/**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the appropriate test files for each affected language binding.
Files:
crates/python/tests/coverage/py_api_coverage_tests.rs
**/*.{rs,py,go,js,jsx,ts,tsx,c,h,html,md,mdx,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include the appropriate SPDX copyright and Apache-2.0 license header in every source file.
Files:
crates/python/tests/coverage/py_api_coverage_tests.rscrates/python/src/py_api/mod.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_api_coverage_tests.rscrates/python/src/py_api/mod.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_api_coverage_tests.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:
crates/python/src/py_api/mod.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/python/src/py_api/mod.rs
🧠 Learnings (1)
📚 Learning: 2026-08-03T19:55:03.931Z
Learnt from: afourniernv
Repo: NVIDIA/NeMo-Relay PR: 558
File: crates/pii-redaction/src/rampart/mod.rs:265-274
Timestamp: 2026-08-03T19:55:03.931Z
Learning: In NeMo Relay first-party plugin registration helpers, treat the documented duplicate-registration `PluginError::RegistrationFailed` result from `register_plugin` as success when registration is intended to be idempotent. Do not locally reclassify this as `PluginError::Conflict`; changing the classification requires a core-wide review of the public API and FFI behavior.
Applied to files:
crates/python/tests/coverage/py_api_coverage_tests.rscrates/python/src/py_api/mod.rs
🔇 Additional comments (1)
crates/python/src/py_api/mod.rs (1)
44-44: 📐 Maintainability & Code QualityRun the required Rust validation.
Provide successful output for
cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings, andjust test-rustbefore merge.As per coding guidelines, “Any Rust change must run
just test-rust”, “cargo fmt --all”, and “cargo clippy --workspace --all-targets -- -D warnings”.Also applies to: 73-171, 819-819
Source: Coding guidelines
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/python/src/py_api/mod.rs (1)
137-150:⚠️ Potential issue | 🟠 MajorAbort the inner Tokio task when the Python future is cancelled.
Line 139 creates the
JoinHandleinsidetokio::select!. If the cancellation branch wins at Line 148, dropping the handle detaches the task. The tool future can continue after Python reports cancellation and can still perform side effects.Create the handle before
select!. Select on&mut task. Calltask.abort()in the cancellation branch. Add a regression test that verifies the Rust-side work stops after cancellation.Proposed fix
- let result = tokio::select! { - result = tokio::spawn(pyo3_async_runtimes::tokio::scope(locals, future)) => Some( + let mut task = tokio::spawn(pyo3_async_runtimes::tokio::scope(locals, future)); + let result = tokio::select! { + result = &mut task => Some( match result { Ok(result) => result, Err(error) if error.is_panic() => Err(pyo3_async_runtimes::err::RustPanic::new_err( format!("rust future panicked: {}", panic_message(error.into_panic().as_ref())), )), Err(error) => Err(pyo3::exceptions::PyRuntimeError::new_err(error.to_string())), }, ), - _ = &mut cancel_receiver => None, + _ = &mut cancel_receiver => { + task.abort(); + 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/python/src/py_api/mod.rs` around lines 137 - 150, Update the async task flow around the `tokio::select!` block to store the spawned inner task in a `JoinHandle` before entering `select!`, then select on `&mut task` and call `task.abort()` when `cancel_receiver` wins. Preserve the existing result and panic handling for completed tasks, and add a regression test verifying Rust-side work stops after Python cancellation.
🤖 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.
Duplicate comments:
In `@crates/python/src/py_api/mod.rs`:
- Around line 137-150: Update the async task flow around the `tokio::select!`
block to store the spawned inner task in a `JoinHandle` before entering
`select!`, then select on `&mut task` and call `task.abort()` when
`cancel_receiver` wins. Preserve the existing result and panic handling for
completed tasks, and add a regression test verifying Rust-side work stops after
Python cancellation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: b4c87469-b196-4913-9de5-b1f0871cf9eb
📒 Files selected for processing (2)
crates/python/src/py_api/mod.rscrates/python/tests/coverage/py_api_coverage_tests.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 (15)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node work
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
**/*.rs: Format Rust code with rustfmt defaults usingcargo fmt.
Runcargo clippy -- -D warnings; all Rust warnings must be treated as errors.
Use Rustsnake_casenaming conventions.
Files:
crates/python/tests/coverage/py_api_coverage_tests.rscrates/python/src/py_api/mod.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
**/*: Use release tags in raw Rust-compatible SemVer without a leadingv; tags such asv0.1.0are prohibited.
Use branch prefixesfeat/,fix/,docs/,test/, orrefactor/according to the change purpose.
Every commit in a pull request must include a DCOSigned-off-by:sign-off.
Before submitting a pull request, ensure pre-commit hooks, relevant tests, target-specific builds, documentation updates, and a rebase on the latestmainare complete.
Use commit messages in the formtype: short description, with a valid type and a first line under 72 characters.
Files:
crates/python/tests/coverage/py_api_coverage_tests.rscrates/python/src/py_api/mod.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
crates/python/tests/coverage/py_api_coverage_tests.rscrates/python/src/py_api/mod.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_api_coverage_tests.rscrates/python/src/py_api/mod.rs
**/*.{rs,py,js,jsx,ts,tsx,go,c,h,cc,cpp,md,toml,yml,yaml,sh}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, documentation, scripts, and configuration files; the project is Apache-2.0.
Files:
crates/python/tests/coverage/py_api_coverage_tests.rscrates/python/src/py_api/mod.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Use
snake_casenaming in Rust and Python.
Files:
crates/python/tests/coverage/py_api_coverage_tests.rscrates/python/src/py_api/mod.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: UseJson = serde_json::Valuein Rust-facing runtime APIs where existing code expects JSON payloads.
Treat Rust as the source of truth for runtime behavior; binding APIs should mirror Rust semantics unless a language-specific wrapper intentionally improves ergonomics.
Files:
crates/python/tests/coverage/py_api_coverage_tests.rscrates/python/src/py_api/mod.rs
**/*.{rs,py,js,mjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Preserve the existing Tokio-based asynchronous model and callback/future lifetimes; do not unexpectedly block or hide async work in bindings.
Files:
crates/python/tests/coverage/py_api_coverage_tests.rscrates/python/src/py_api/mod.rs
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}: Run tests for every language affected by a change; changes to the core Rust crate require tests across all bindings.
UseSONAR_IGNORE_START/SONAR_IGNORE_ENDonly for documented false positives, keep ignored blocks minimal, explain them with a comment, and obtain reviewer sign-off.
Preserve the layered architecture in which Rust provides the core runtime and C FFI, PyO3, and NAPI provide bindings that mirror the full API surface.
Files:
crates/python/tests/coverage/py_api_coverage_tests.rscrates/python/src/py_api/mod.rs
**/{test,tests}/**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the appropriate test files for each affected language binding.
Files:
crates/python/tests/coverage/py_api_coverage_tests.rs
**/*.{rs,py,go,js,jsx,ts,tsx,c,h,html,md,mdx,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include the appropriate SPDX copyright and Apache-2.0 license header in every source file.
Files:
crates/python/tests/coverage/py_api_coverage_tests.rscrates/python/src/py_api/mod.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_api_coverage_tests.rscrates/python/src/py_api/mod.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_api_coverage_tests.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:
crates/python/src/py_api/mod.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/python/src/py_api/mod.rs
🧠 Learnings (1)
📚 Learning: 2026-08-03T19:55:03.931Z
Learnt from: afourniernv
Repo: NVIDIA/NeMo-Relay PR: 558
File: crates/pii-redaction/src/rampart/mod.rs:265-274
Timestamp: 2026-08-03T19:55:03.931Z
Learning: In NeMo Relay first-party plugin registration helpers, treat the documented duplicate-registration `PluginError::RegistrationFailed` result from `register_plugin` as success when registration is intended to be idempotent. Do not locally reclassify this as `PluginError::Conflict`; changing the classification requires a core-wide review of the public API and FFI behavior.
Applied to files:
crates/python/tests/coverage/py_api_coverage_tests.rscrates/python/src/py_api/mod.rs
🔇 Additional comments (3)
crates/python/tests/coverage/py_api_coverage_tests.rs (1)
41-63: Add closed-event-loop regression coverage.This test runs
run_until_completewhile the event loop is open. The helper closes the loop only after the future settles, so this test does not exercise the closed-loop branch insafe_future_into_py. Add a case that keeps the Rust future pending, closes the event loop before it settles, and verifies that no completion callback or unraisable callback error occurs.As per path instructions, tests should cover the behavior promised by the changed API surface, including error paths.
Source: Path instructions
crates/python/src/py_api/mod.rs (2)
44-44: LGTM!Also applies to: 73-136, 151-170
819-843: Run the required validation before handoff.Run
cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,just test-rust,just test-python, anduv run pre-commit run --all-files. Verify the SPDX headers in both changed files.As per coding guidelines, Rust and binding-surface changes require formatter, strict Clippy, affected-language tests, and all-files pre-commit validation.
Source: Coding guidelines
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
/merge |
2 participants
Overview
Preserve
future_into_pypanic settlement for the closed-loop-safe tool execution bridge.Details
tool_call_execute.pyo3_async_runtimes.RustPanic, matchingfuture_into_py.Where should the reviewer start?
Start with
safe_future_into_pyincrates/python/src/py_api/mod.rsandsafe_future_into_py_settles_rust_panicsin the adjacent coverage test.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
Bug Fixes
Tests