feat: expose event sanitizers in Python [3/6]#373
Conversation
github-actions
Bot
added
size:L
|
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 (4)
📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
🧰 Additional context used📓 Path-based instructions (17)**/*.{rs,py}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,py,js,mjs,cjs,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.py📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{rs,py,go,js,ts,c,h}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
{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)
Files:
{crates/**/src/**/*.rs,python/**/*.py}📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Files:
{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)
Files:
{crates/plugin/**/*.rs,python/plugin/**/*.py}📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Files:
**/*.{py,go,js,ts}📄 CodeRabbit inference engine (.agents/skills/maintain-observability/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:
**⚙️ CodeRabbit configuration file
Files:
python/tests/**/*.py📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
Files:
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}⚙️ CodeRabbit configuration file
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:
{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Files:
python/nemo_relay/**/*⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (6)
WalkthroughAdds event-sanitize guardrail APIs for mark and scope start/end events across Rust and Python bindings, extends the plugin worker dispatch and coverage, and adds a ChangesEvent sanitize guardrail feature
PII redaction mark flag
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant PluginContext
participant _WorkerService
participant EventSanitizeCallback
participant Client
PluginContext->>_WorkerService: register sanitize guardrail surface
Client->>_WorkerService: invoke sanitize surface with event payload
_WorkerService->>EventSanitizeCallback: call(event, fields)
EventSanitizeCallback-->>_WorkerService: sanitized fields
_WorkerService-->>Client: JSON response with sanitized fields
Possibly related PRs
Suggested labels: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
willkill07
changed the title
github-actions
Bot
added
size:XL
9e3ab05 to
5d4a20b
Compare
github-actions
Bot
added
size:L
Signed-off-by: Will Killian <wkillian@nvidia.com>
5d4a20b to
c37d47f
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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
python/plugin/src/nemo_relay_plugin/_api.py (1)
432-465: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winShared
event_sanitizersdict causes cross-surface name collisions and misrouted callbacks.
event_sanitizersis a singledict[str, EventSanitizeCallback]shared across three distinct surfaces (MARK_SANITIZE_GUARDRAIL,SCOPE_SANITIZE_START_GUARDRAIL,SCOPE_SANITIZE_END_GUARDRAIL)._register_event_sanitizer(Line 518-526) stores callbacks keyed only byname, and_invoke_result(Line 1469-1484) dispatches viaself._handler(self._handlers.event_sanitizers, request.registration_name)— also keyed only byname, ignoringsurface.The class docstring explicitly states: "Registration names need to be unique only within a registration surface" (Line 477-479), and this exact pattern (same name across two surfaces) is demonstrated for tool sanitizers, which correctly use separate dicts (
tool_sanitize_requests/tool_sanitize_responses) to avoid collision. If a plugin registers, e.g.,"sanitize"for bothregister_mark_sanitize_guardrailandregister_scope_sanitize_start_guardrail(which_push_registration's per-surface uniqueness check permits), the second call silently overwrites the first entry in the shared dict, and the wrong callback runs for the other surface — misrouting observability sanitization, with real risk of unsanitized/leaked data reaching exporters since this is a redaction path.🐛 Proposed fix: split into per-surface dicts
class _Handlers: registrations: list[Any] subscribers: dict[str, SubscriberCallback] - event_sanitizers: dict[str, EventSanitizeCallback] + mark_sanitizers: dict[str, EventSanitizeCallback] + scope_start_sanitizers: dict[str, EventSanitizeCallback] + scope_end_sanitizers: dict[str, EventSanitizeCallback] tool_sanitize_requests: dict[str, ToolSanitizeCallback] ... `@classmethod` def empty(cls) -> _Handlers: return cls( registrations=[], subscribers={}, - event_sanitizers={}, + mark_sanitizers={}, + scope_start_sanitizers={}, + scope_end_sanitizers={}, ... )- def _register_event_sanitizer( - self, - name: str, - callback: EventSanitizeCallback, - surface: int, - priority: int, - ) -> None: - self._push_registration(name, surface, priority, False) - self._handlers.event_sanitizers[name] = callback - def register_mark_sanitize_guardrail( self, name: str, callback: EventSanitizeCallback, *, priority: int = 0 ) -> None: """Register a sanitizer for mark event observability fields.""" - self._register_event_sanitizer(name, callback, pb.MARK_SANITIZE_GUARDRAIL, priority) + self._push_registration(name, pb.MARK_SANITIZE_GUARDRAIL, priority, False) + self._handlers.mark_sanitizers[name] = callback def register_scope_sanitize_start_guardrail( self, name: str, callback: EventSanitizeCallback, *, priority: int = 0 ) -> None: """Register a sanitizer for scope start event observability fields.""" - self._register_event_sanitizer(name, callback, pb.SCOPE_SANITIZE_START_GUARDRAIL, priority) + self._push_registration(name, pb.SCOPE_SANITIZE_START_GUARDRAIL, priority, False) + self._handlers.scope_start_sanitizers[name] = callback def register_scope_sanitize_end_guardrail( self, name: str, callback: EventSanitizeCallback, *, priority: int = 0 ) -> None: """Register a sanitizer for scope end event observability fields.""" - self._register_event_sanitizer(name, callback, pb.SCOPE_SANITIZE_END_GUARDRAIL, priority) + self._push_registration(name, pb.SCOPE_SANITIZE_END_GUARDRAIL, priority, False) + self._handlers.scope_end_sanitizers[name] = callback- if request.surface in { - pb.MARK_SANITIZE_GUARDRAIL, - pb.SCOPE_SANITIZE_START_GUARDRAIL, - pb.SCOPE_SANITIZE_END_GUARDRAIL, - }: + event_sanitizer_dicts = { + pb.MARK_SANITIZE_GUARDRAIL: self._handlers.mark_sanitizers, + pb.SCOPE_SANITIZE_START_GUARDRAIL: self._handlers.scope_start_sanitizers, + pb.SCOPE_SANITIZE_END_GUARDRAIL: self._handlers.scope_end_sanitizers, + } + if request.surface in event_sanitizer_dicts: event = _decode_required_envelope(request.event, "event", EVENT_SCHEMA) fields: EventSanitizeFields = { "data": event.get("data"), "category_profile": event.get("category_profile"), "metadata": event.get("metadata"), } - return _json_response( - await _maybe_await( - self._handler(self._handlers.event_sanitizers, request.registration_name)(event, fields) - ) - ) + handler = self._handler(event_sanitizer_dicts[request.surface], request.registration_name) + return _json_response(await _maybe_await(handler(event, fields)))Also applies to: 518-545, 1469-1484
🤖 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/plugin/src/nemo_relay_plugin/_api.py` around lines 432 - 465, The shared event sanitizer registry in _Handlers is causing callbacks to collide across MARK_SANITIZE_GUARDRAIL, SCOPE_SANITIZE_START_GUARDRAIL, and SCOPE_SANITIZE_END_GUARDRAIL because _register_event_sanitizer and _invoke_result key only by registration name. Split event_sanitizers into separate per-surface maps, update _Handlers.empty and the related registration/dispatch paths in _register_event_sanitizer and _invoke_result to use the correct surface-specific dict, and keep the lookup aligned with the class’s per-surface uniqueness rule.
🤖 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_plugin.rs`:
- Around line 214-301: The new guardrail helper in PyPluginContext only covers
the three sanitize-event registrations, but the same
qualify/register/track/deregister-closure pattern is still duplicated across the
older registration methods like register_tool_sanitize_request_guardrail and
register_llm_sanitize_response_guardrail. Extract a more generic helper from
register_event_sanitizer that also accepts the wrap function and the underlying
register/deregister hooks, then refactor the remaining methods to use it so the
repeated registrations all share the same logic.
In `@python/plugin/src/nemo_relay_plugin/_api.py`:
- Around line 528-544: The three new sanitizer registration methods in
PluginContext are missing the same docstring structure used by the other
register_* APIs. Update register_mark_sanitize_guardrail,
register_scope_sanitize_start_guardrail, and
register_scope_sanitize_end_guardrail to include the standard Args section for
name/callback/priority and the Callback errors section, matching the style of
register_subscriber and register_tool_sanitize_request_guardrail. Keep the
method behavior unchanged and make the documentation consistent across these
registration helpers.
In `@python/tests/plugin/test_worker_sdk.py`:
- Around line 235-237: Add a test that reuses the same sanitizer registration
name across two different sanitizer surfaces with different callbacks, similar
to the existing tool sanitizer coverage. Update the test around
ctx.register_mark_sanitize_guardrail,
ctx.register_scope_sanitize_start_guardrail, and
ctx.register_scope_sanitize_end_guardrail to include an analogous same-name case
so it exercises per-surface uniqueness and would catch the shared-dict collision
in event_sanitizers. Use the relevant guardrail registration methods in
test_worker_sdk.py to locate the spot and assert both callbacks remain distinct.
In `@python/tests/test_event_sanitizers.py`:
- Line 21: The test functions in this module are annotated with forbidden `->
None` return types. Remove the return annotation from all affected test
definitions, including
`test_global_mark_sanitizers_order_convert_fields_and_remove_values` and the
other test functions in this file, while keeping the test names and bodies
unchanged.
- Around line 106-131: This test leaves scope and subscriber state behind if an
assertion or event call fails; wrap the scope.push/scope.event/scope.pop
sequence and the subscribers.deregister(capture_name) cleanup in try/finally.
Use the existing test helpers and symbols like scope.push, scope.pop,
scope_local.register_mark_sanitize, and subscribers.deregister so owner/child
scopes are always closed and the capture subscriber is always removed even on
failure.
- Around line 14-18: Convert the duplicated _capture_events helper into a pytest
fixture that uses yield for setup/teardown, and update the tests to depend on
that fixture instead of calling the helper directly. Move the
subscribers.register(name, events.append) logic into the fixture setup and
ensure subscribers.deregister(capture_name) is always performed in the fixture
cleanup so teardown is centralized and consistent across the file.
---
Outside diff comments:
In `@python/plugin/src/nemo_relay_plugin/_api.py`:
- Around line 432-465: The shared event sanitizer registry in _Handlers is
causing callbacks to collide across MARK_SANITIZE_GUARDRAIL,
SCOPE_SANITIZE_START_GUARDRAIL, and SCOPE_SANITIZE_END_GUARDRAIL because
_register_event_sanitizer and _invoke_result key only by registration name.
Split event_sanitizers into separate per-surface maps, update _Handlers.empty
and the related registration/dispatch paths in _register_event_sanitizer and
_invoke_result to use the correct surface-specific dict, and keep the lookup
aligned with the class’s per-surface uniqueness rule.
🪄 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: cc515614-9a4d-404a-a841-ef5b938e68a7
📒 Files selected for processing (21)
crates/python/src/py_api/mod.rscrates/python/src/py_callable.rscrates/python/src/py_plugin.rscrates/python/tests/coverage/coverage_tests.rscrates/python/tests/coverage/py_callable_coverage_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rspython/nemo_relay/__init__.pypython/nemo_relay/__init__.pyipython/nemo_relay/_native.pyipython/nemo_relay/guardrails.pypython/nemo_relay/pii_redaction.pypython/nemo_relay/pii_redaction.pyipython/nemo_relay/plugin.pypython/nemo_relay/plugin.pyipython/nemo_relay/scope_local.pypython/plugin/src/nemo_relay_plugin/__init__.pypython/plugin/src/nemo_relay_plugin/_api.pypython/tests/plugin/test_public_api_docstrings.pypython/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.pypython/tests/test_pii_redaction_plugin.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (22)
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
python/tests/test_pii_redaction_plugin.pypython/nemo_relay/pii_redaction.pycrates/python/tests/coverage/py_callable_coverage_tests.rscrates/python/tests/coverage/coverage_tests.rspython/plugin/src/nemo_relay_plugin/__init__.pypython/tests/plugin/test_public_api_docstrings.pypython/nemo_relay/scope_local.pycrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_callable.rspython/nemo_relay/plugin.pypython/nemo_relay/__init__.pypython/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.pypython/nemo_relay/guardrails.pycrates/python/src/py_plugin.rscrates/python/src/py_api/mod.rspython/plugin/src/nemo_relay_plugin/_api.py
**/*.{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:
python/tests/test_pii_redaction_plugin.pypython/nemo_relay/pii_redaction.pycrates/python/tests/coverage/py_callable_coverage_tests.rscrates/python/tests/coverage/coverage_tests.rspython/plugin/src/nemo_relay_plugin/__init__.pypython/tests/plugin/test_public_api_docstrings.pypython/nemo_relay/scope_local.pycrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_callable.rspython/nemo_relay/plugin.pypython/nemo_relay/__init__.pypython/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.pypython/nemo_relay/guardrails.pycrates/python/src/py_plugin.rscrates/python/src/py_api/mod.rspython/plugin/src/nemo_relay_plugin/_api.py
**/*.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_pii_redaction_plugin.pypython/nemo_relay/pii_redaction.pypython/plugin/src/nemo_relay_plugin/__init__.pypython/tests/plugin/test_public_api_docstrings.pypython/nemo_relay/scope_local.pypython/nemo_relay/plugin.pypython/nemo_relay/__init__.pypython/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.pypython/nemo_relay/guardrails.pypython/plugin/src/nemo_relay_plugin/_api.py
**/*.{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:
python/tests/test_pii_redaction_plugin.pypython/nemo_relay/pii_redaction.pycrates/python/tests/coverage/py_callable_coverage_tests.rscrates/python/tests/coverage/coverage_tests.rspython/plugin/src/nemo_relay_plugin/__init__.pypython/tests/plugin/test_public_api_docstrings.pypython/nemo_relay/scope_local.pycrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_callable.rspython/nemo_relay/plugin.pypython/nemo_relay/__init__.pypython/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.pypython/nemo_relay/guardrails.pycrates/python/src/py_plugin.rscrates/python/src/py_api/mod.rspython/plugin/src/nemo_relay_plugin/_api.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:
python/tests/test_pii_redaction_plugin.pypython/nemo_relay/pii_redaction.pypython/plugin/src/nemo_relay_plugin/__init__.pypython/tests/plugin/test_public_api_docstrings.pypython/nemo_relay/scope_local.pycrates/python/src/py_callable.rspython/nemo_relay/plugin.pypython/nemo_relay/__init__.pypython/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.pypython/nemo_relay/guardrails.pycrates/python/src/py_plugin.rscrates/python/src/py_api/mod.rspython/plugin/src/nemo_relay_plugin/_api.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_pii_redaction_plugin.pypython/nemo_relay/pii_redaction.pypython/plugin/src/nemo_relay_plugin/__init__.pypython/tests/plugin/test_public_api_docstrings.pypython/nemo_relay/scope_local.pypython/nemo_relay/plugin.pypython/nemo_relay/__init__.pypython/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.pypython/nemo_relay/guardrails.pypython/plugin/src/nemo_relay_plugin/_api.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_pii_redaction_plugin.pypython/tests/plugin/test_public_api_docstrings.pypython/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.py
**/*
📄 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:
python/tests/test_pii_redaction_plugin.pypython/nemo_relay/pii_redaction.pyipython/nemo_relay/pii_redaction.pycrates/python/tests/coverage/py_callable_coverage_tests.rscrates/python/tests/coverage/coverage_tests.rspython/plugin/src/nemo_relay_plugin/__init__.pypython/tests/plugin/test_public_api_docstrings.pypython/nemo_relay/scope_local.pycrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_callable.rspython/nemo_relay/plugin.pypython/nemo_relay/__init__.pypython/nemo_relay/__init__.pyipython/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.pypython/nemo_relay/guardrails.pypython/nemo_relay/plugin.pyicrates/python/src/py_plugin.rspython/nemo_relay/_native.pyicrates/python/src/py_api/mod.rspython/plugin/src/nemo_relay_plugin/_api.py
**/*.{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:
python/tests/test_pii_redaction_plugin.pypython/nemo_relay/pii_redaction.pycrates/python/tests/coverage/py_callable_coverage_tests.rscrates/python/tests/coverage/coverage_tests.rspython/plugin/src/nemo_relay_plugin/__init__.pypython/tests/plugin/test_public_api_docstrings.pypython/nemo_relay/scope_local.pycrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_callable.rspython/nemo_relay/plugin.pypython/nemo_relay/__init__.pypython/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.pypython/nemo_relay/guardrails.pycrates/python/src/py_plugin.rscrates/python/src/py_api/mod.rspython/plugin/src/nemo_relay_plugin/_api.py
**
⚙️ CodeRabbit configuration file
**:AGENTS.md
This file provides guidance to agents, including Claude Code and OpenAI Codex, when working in this repository.
Project Overview
NeMo Relay is a multi-language agent runtime framework for execution scopes, lifecycle events, middleware, plugins, and observability around tool and LLM calls. The core runtime is Rust. Primary supported bindings are Rust, Python, and Node.js. Go and the raw C FFI are experimental and source-first.
The shared runtime model is:
- Scope stacks decide where work belongs and which scope-local behavior is visible.
- Middleware registries decide what guardrails and intercepts run around managed calls.
- Plugins install reusable runtime behavior from configuration.
- Events record runtime behavior in ATOF form.
- Subscribers and exporters consume events in-process or export them to ATIF, OpenTelemetry, OpenInference, or other backends.
Repository Structure
The repository layout separates the Rust runtime, language bindings,
documentation, integrations, and agent-facing skills.crates/ core/ # Rust core runtime crate, published as nemo-relay adaptive/ # Adaptive runtime primitives and plugin components python/ # PyO3 native extension for the Python package ffi/ # Raw C ABI layer used by downstream bindings such as Go node/ # NAPI Node.js binding and JavaScript/TypeScript entry points python/ nemo_relay/ # Python wrapper package: scopes, tools, LLM, middleware, typed helpers, plugins, adaptive helpers tests/ # Python tests go/ nemo_relay/ # Experimental Go CGo binding and tests fern/ # Fern documentation site scripts/ # Stable wrappers and helper scripts; build/test/docs entry points live in justfile skills/ # Published Codex/agent skills for NeMo Relay usage patternsPrerequisites
Insta...
Files:
python/tests/test_pii_redaction_plugin.pypython/nemo_relay/pii_redaction.pyipython/nemo_relay/pii_redaction.pycrates/python/tests/coverage/py_callable_coverage_tests.rscrates/python/tests/coverage/coverage_tests.rspython/plugin/src/nemo_relay_plugin/__init__.pypython/tests/plugin/test_public_api_docstrings.pypython/nemo_relay/scope_local.pycrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_callable.rspython/nemo_relay/plugin.pypython/nemo_relay/__init__.pypython/nemo_relay/__init__.pyipython/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.pypython/nemo_relay/guardrails.pypython/nemo_relay/plugin.pyicrates/python/src/py_plugin.rspython/nemo_relay/_native.pyicrates/python/src/py_api/mod.rspython/plugin/src/nemo_relay_plugin/_api.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_pii_redaction_plugin.pycrates/python/tests/coverage/py_callable_coverage_tests.rscrates/python/tests/coverage/coverage_tests.rspython/tests/plugin/test_public_api_docstrings.pycrates/python/tests/coverage/py_plugin_coverage_tests.rspython/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.py
{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/pii_redaction.pyipython/nemo_relay/pii_redaction.pypython/nemo_relay/scope_local.pypython/nemo_relay/plugin.pypython/nemo_relay/__init__.pypython/nemo_relay/__init__.pyipython/nemo_relay/guardrails.pypython/nemo_relay/plugin.pyipython/nemo_relay/_native.pyicrates/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:
python/nemo_relay/pii_redaction.pyipython/nemo_relay/pii_redaction.pypython/nemo_relay/scope_local.pypython/nemo_relay/plugin.pypython/nemo_relay/__init__.pypython/nemo_relay/__init__.pyipython/nemo_relay/guardrails.pypython/nemo_relay/plugin.pyipython/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/pii_redaction.pyipython/nemo_relay/pii_redaction.pypython/nemo_relay/scope_local.pypython/nemo_relay/plugin.pypython/nemo_relay/__init__.pypython/nemo_relay/__init__.pyipython/nemo_relay/guardrails.pypython/nemo_relay/plugin.pyipython/nemo_relay/_native.pyi
python/nemo_relay/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Python wrapper modules live under
python/nemo_relay/, and the native extension is built fromcrates/pythonwithmaturin.
Files:
python/nemo_relay/pii_redaction.pypython/nemo_relay/scope_local.pypython/nemo_relay/plugin.pypython/nemo_relay/__init__.pypython/nemo_relay/guardrails.py
**/*.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/python/tests/coverage/coverage_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_callable.rscrates/python/src/py_plugin.rscrates/python/src/py_api/mod.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/python/tests/coverage/coverage_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_callable.rscrates/python/src/py_plugin.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_callable_coverage_tests.rscrates/python/tests/coverage/coverage_tests.rscrates/python/tests/coverage/py_plugin_coverage_tests.rscrates/python/src/py_callable.rscrates/python/src/py_plugin.rscrates/python/src/py_api/mod.rs
{crates/core/src/plugin/dynamic/**,crates/plugin/**,crates/worker/**,crates/worker-proto/**,crates/types/**,python/plugin/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Keep the stable boundary explicit: native plugins cross a C ABI, and worker plugins cross
grpc-v1.
Files:
python/plugin/src/nemo_relay_plugin/__init__.pypython/plugin/src/nemo_relay_plugin/_api.py
{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:
python/plugin/src/nemo_relay_plugin/__init__.pypython/plugin/src/nemo_relay_plugin/_api.py
{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:
python/plugin/src/nemo_relay_plugin/__init__.pypython/plugin/src/nemo_relay_plugin/_api.py
python/nemo_relay/{adaptive.py,plugin.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep Python adaptive/plugin wrappers in
python/nemo_relay/adaptive.pyandpython/nemo_relay/plugin.pysynchronized with the shared adaptive/plugin boundary and lifecycle.
Files:
python/nemo_relay/plugin.py
🪛 Ruff (0.15.20)
python/nemo_relay/__init__.py
[warning] 82-82: Import from collections.abc instead: AsyncIterator, Awaitable, Callable
Import from collections.abc
(UP035)
python/tests/test_event_sanitizers.py
[warning] 14-14: Missing return type annotation for private function _capture_events
(ANN202)
[warning] 135-135: Missing return type annotation for private function validate
Add return type annotation: None
(ANN202)
[warning] 138-138: Missing return type annotation for private function register
Add return type annotation: None
(ANN202)
[warning] 170-170: Missing return type annotation for private function validate
Add return type annotation: None
(ANN202)
[warning] 173-173: Missing return type annotation for private function register
Add return type annotation: Never
(ANN202)
[warning] 182-182: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (22)
python/nemo_relay/pii_redaction.py (1)
137-137: LGTM!Also applies to: 154-154
python/nemo_relay/pii_redaction.pyi (1)
59-59: LGTM!python/tests/test_pii_redaction_plugin.py (1)
40-43: LGTM!python/nemo_relay/__init__.py (1)
82-82: LGTM!Also applies to: 141-155, 494-495
python/nemo_relay/__init__.pyi (1)
26-26: LGTM!Also applies to: 145-153
python/nemo_relay/_native.pyi (1)
27-43: LGTM!Also applies to: 1190-1215
python/nemo_relay/plugin.py (1)
18-18: LGTM!Also applies to: 86-99
python/nemo_relay/plugin.pyi (1)
9-9: LGTM!Also applies to: 39-45
python/nemo_relay/guardrails.py (1)
24-24: LGTM!Also applies to: 40-46, 65-71, 82-116, 365-370
python/nemo_relay/scope_local.py (2)
40-48: LGTM!Also applies to: 85-93, 663-669
113-147: 📐 Maintainability & Code QualityNo change needed for these wrappers
These functions match the untyped wrapper style used throughout
python/nemo_relay/scope_local.py, so adding annotations here would be inconsistent with the rest of the module.> Likely an incorrect or invalid review comment.python/plugin/src/nemo_relay_plugin/__init__.py (1)
21-21: LGTM!Also applies to: 35-35, 64-65, 98-99
python/plugin/src/nemo_relay_plugin/_api.py (1)
72-72: LGTM!Also applies to: 83-92, 406-409, 1647-1649
python/tests/plugin/test_public_api_docstrings.py (1)
24-24: LGTM!python/tests/plugin/test_worker_sdk.py (1)
187-189: LGTM!Also applies to: 279-281, 320-322, 605-645, 2379-2381
crates/python/src/py_callable.rs (2)
28-37: LGTM!
857-917: 🔒 Security & PrivacyNo change needed — event sanitizer callbacks intentionally fall back to the original fields on conversion, call, and shape errors, matching the existing sanitizer contract.
> Likely an incorrect or invalid review comment.crates/python/tests/coverage/py_callable_coverage_tests.rs (1)
598-651: LGTM!crates/python/src/py_api/mod.rs (1)
878-915: LGTM!Also applies to: 1313-1358, 1738-1754, 1831-1851
crates/python/tests/coverage/coverage_tests.rs (1)
251-256: LGTM!Also applies to: 287-292
crates/python/src/py_plugin.rs (1)
19-33: LGTM!Also applies to: 43-47
crates/python/tests/coverage/py_plugin_coverage_tests.rs (1)
123-125: LGTM!Also applies to: 169-189, 277-287
|
Worth a unit test to confirm/enforce: |
Signed-off-by: Will Killian <wkillian@nvidia.com>
willkill07
changed the title
coderabbitai
Bot
added
the
DO NOT MERGE
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 `@python/plugin/src/nemo_relay_plugin/_api.py`:
- Around line 1522-1535: Centralize the sanitizer surface-to-handler association
so registration and dispatch use the same source of truth, instead of
duplicating the mapping in `_register_event_sanitizer` call sites and
`_invoke_result`. Add a single shared class-level mapping (for example, from
`pb.MARK_SANITIZE_GUARDRAIL`, `pb.SCOPE_SANITIZE_START_GUARDRAIL`, and
`pb.SCOPE_SANITIZE_END_GUARDRAIL` to the corresponding `self._handlers`
attribute names) and have both `_register_event_sanitizer` and `_invoke_result`
look up handlers through it. This keeps the `*_sanitizers` storage selection
consistent and prevents future surface additions from reintroducing the
collision bug.
🪄 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: d5db4376-046e-42b9-bdfd-f749e8ce6b34
📒 Files selected for processing (4)
crates/python/src/py_plugin.rspython/plugin/src/nemo_relay_plugin/_api.pypython/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (17)
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
python/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.pypython/plugin/src/nemo_relay_plugin/_api.pycrates/python/src/py_plugin.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:
python/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.pypython/plugin/src/nemo_relay_plugin/_api.pycrates/python/src/py_plugin.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/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.pypython/plugin/src/nemo_relay_plugin/_api.py
**/*.{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:
python/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.pypython/plugin/src/nemo_relay_plugin/_api.pycrates/python/src/py_plugin.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:
python/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.pypython/plugin/src/nemo_relay_plugin/_api.pycrates/python/src/py_plugin.rs
**/*.{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/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.pypython/plugin/src/nemo_relay_plugin/_api.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/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.py
**/*
📄 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:
python/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.pypython/plugin/src/nemo_relay_plugin/_api.pycrates/python/src/py_plugin.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:
python/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.pypython/plugin/src/nemo_relay_plugin/_api.pycrates/python/src/py_plugin.rs
**
⚙️ CodeRabbit configuration file
**:AGENTS.md
This file provides guidance to agents, including Claude Code and OpenAI Codex, when working in this repository.
Project Overview
NeMo Relay is a multi-language agent runtime framework for execution scopes, lifecycle events, middleware, plugins, and observability around tool and LLM calls. The core runtime is Rust. Primary supported bindings are Rust, Python, and Node.js. Go and the raw C FFI are experimental and source-first.
The shared runtime model is:
- Scope stacks decide where work belongs and which scope-local behavior is visible.
- Middleware registries decide what guardrails and intercepts run around managed calls.
- Plugins install reusable runtime behavior from configuration.
- Events record runtime behavior in ATOF form.
- Subscribers and exporters consume events in-process or export them to ATIF, OpenTelemetry, OpenInference, or other backends.
Repository Structure
The repository layout separates the Rust runtime, language bindings,
documentation, integrations, and agent-facing skills.crates/ core/ # Rust core runtime crate, published as nemo-relay adaptive/ # Adaptive runtime primitives and plugin components python/ # PyO3 native extension for the Python package ffi/ # Raw C ABI layer used by downstream bindings such as Go node/ # NAPI Node.js binding and JavaScript/TypeScript entry points python/ nemo_relay/ # Python wrapper package: scopes, tools, LLM, middleware, typed helpers, plugins, adaptive helpers tests/ # Python tests go/ nemo_relay/ # Experimental Go CGo binding and tests fern/ # Fern documentation site scripts/ # Stable wrappers and helper scripts; build/test/docs entry points live in justfile skills/ # Published Codex/agent skills for NeMo Relay usage patternsPrerequisites
Insta...
Files:
python/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.pypython/plugin/src/nemo_relay_plugin/_api.pycrates/python/src/py_plugin.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:
python/tests/plugin/test_worker_sdk.pypython/tests/test_event_sanitizers.py
{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:
python/plugin/src/nemo_relay_plugin/_api.py
{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:
python/plugin/src/nemo_relay_plugin/_api.py
{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:
python/plugin/src/nemo_relay_plugin/_api.py
**/*.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/src/py_plugin.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/src/py_plugin.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/src/py_plugin.rs
🔇 Additional comments (9)
python/plugin/src/nemo_relay_plugin/_api.py (2)
435-437: 🎯 Functional CorrectnessCollision fix confirmed: separate dicts per sanitizer surface.
Splitting
mark_sanitizers/scope_start_sanitizers/scope_end_sanitizersinto distinct maps resolves the earlier shared-dict collision (same name across mark/scope-start/scope-end no longer overwrites callbacks).Also applies to: 455-457
522-598: LGTM! Registration methods correctly route each surface into its own dict, and docstrings now include Args/Callback errors sections matching sibling methods.crates/python/src/py_plugin.rs (2)
244-563: LGTM! DRY refactor now applied consistently to all registration methods (subscriber, tool, LLM, sanitize guardrails, intercepts), fully addressing the earlier request to extend the pattern beyond just the sanitize guardrails.
214-238: LGTM!register_callbackcleanly consolidates the qualify/register/track/deregister boilerplate.python/tests/plugin/test_worker_sdk.py (2)
241-243: 🎯 Functional CorrectnessLGTM! Reusing
"event_sanitize"across mark and scope-start surfaces with distinct callbacks, and parametrizing the invoke test over all three surfaces, directly exercises the cross-surface name-collision fix requested previously.Also applies to: 624-632
285-287: LGTM! Proto ID assertions and expected-registrations list correctly reflect the three new sanitizer surfaces.Also applies to: 324-340
python/tests/test_event_sanitizers.py (3)
15-22: LGTM! Fixture follows the required@pytest.fixture(name=...)/<fixture_name>_fixtureconvention with properyield-based teardown.
24-56: LGTM!-> Noneannotations removed from test functions per path instructions.Also applies to: 59-72, 75-104, 136-197
106-134: LGTM! try/finally now correctly guards scope push/pop so owner/child scopes can't leak on assertion failure.
willkill07
changed the title
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
/merge |
#### Overview Documents the event sanitizer and PII redaction behavior implemented by the merged RELAY-409 stack (#371–#375). This docs-only PR is the final part of that six-PR stack. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Adds a dedicated event sanitizer reference covering callback replacement semantics, ordering, registration lifetimes, cross-language APIs, and dynamic-plugin protocol values. - Explains which mark and scope event fields sanitizers can replace and which lifecycle and identity fields remain immutable. - Documents PII `mark = true` defaults, opt-out behavior, generic scope `input` and `output` coverage, and subscriber/exporter privacy guarantees. - Updates related middleware, event, scope/mark, plugin-authoring, Python, Node.js, Go, and observability-skill guidance. - Contains no implementation changes. - Breaking changes: none. Validation: - `just docs` - `just docs-linkcheck` - `uv run pre-commit run --all-files` #### Where should the reviewer start? Start with `docs/reference/event-sanitizers.mdx`, then review `docs/configure-plugins/pii-redaction/configuration.mdx` for the privacy contract. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to: RELAY-409 - Relates to: #371, #372, #373, #374, and #375 ## Summary by CodeRabbit - **New Features** - Added observability sanitization for **mark** and **scope** events (including scope-start and scope-end) before delivery to subscribers/exporters. - Sanitizers can rewrite observability fields (`data`, `category_profile`, `metadata`) while keeping core identity/context fields immutable. - Expanded privacy redaction support with a configurable `mark` option (default enabled) for mark and generic scope events. - **Documentation** - Added a new Event Sanitizers reference covering registration levels, ordering, callback contracts, and examples. - Updated middleware and language-binding guidance plus Node.js/Go/Python overviews to reflect mark/scope sanitizer capabilities. Authors: - Will Killian (https://github.com/willkill07) Approvers: - https://github.com/lvojtku - Bryan Bednarski (https://github.com/bbednarski9) URL: #376
2 participants
Overview
Exposes the mark and scope event sanitizer registries from PRs #371 and #372 through the Python binding and worker SDK.
Details
PluginContextand Python gRPC worker SDK.markto typed PII configuration helpers.Validation:
just test-python,just test-python-plugin, focused native tests and coverage, Ruff, ty, and pre-commit.Where should the reviewer start?
Start with
python/nemo_relay/guardrails.py,python/nemo_relay/scope_local.py, andpython/tests/test_event_sanitizers.py.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
EventSanitizeFieldsandEventSanitizeGuardrailtyping for sanitizer callbacks, plus new registration/deregistration helpers.markoption to PII redaction configuration.