feat(types): add shared relay DTO crate - #305
Conversation
|
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 ignored due to path filters (1)
📒 Files selected for processing (4)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (13)**/*.rs📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Files:
crates/core/src/observability/{atif,otel,openinference}.rs📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Files:
**/{Cargo.toml,**/*.rs}📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Files:
**/*.{h,hpp,c,cpp,rs}📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Files:
{crates/core,crates/adaptive}/**/*📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Files:
**/*.{rs,toml}📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Files:
crates/core/**/*.rs📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
Files:
crates/{core,adaptive}/**📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,py,go,js,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
crates/**/*.rs📄 CodeRabbit inference engine (AGENTS.md)
Files:
**⚙️ CodeRabbit configuration file
Files:
crates/{core,adaptive}/**/*.rs⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (4)
WalkthroughAdds a shared ChangesShared DTO extraction and core migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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>
59afada to
8ad7582
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/core/src/api/event.rs (1)
19-51: 🗄️ Data Integrity & Integration | 🟠 MajorKeep
normalized_llm_*onEvent
crates/core/src/api/event.rs:19-51movesnormalized_llm_request()/normalized_llm_response()behindEventNormalizationExt, which breaks downstream callers that only importEvent. Keep the inherent methods, or add thin wrappers, so the publicEventAPI stays source-compatible.🤖 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/event.rs` around lines 19 - 51, Keep the normalized_llm_request and normalized_llm_response API available directly on Event so downstream callers that only use Event remain source-compatible. The current EventNormalizationExt split changes the public surface; either restore these as inherent methods on Event or add thin forwarding wrappers on Event that delegate to EventNormalizationExt::normalized_llm_request and EventNormalizationExt::normalized_llm_response. Ensure the existing behavior for annotated values and fallback serde_json/resolve decoding stays unchanged.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/types/src/api/event.rs`:
- Around line 714-717: Lowercase attributes during canonicalization so
ScopeEvent::new() actually produces canonical wire values. Update
canonicalize_attributes() to normalize each attribute to lowercase before
sorting and deduping, and keep the existing sort/dedup behavior so inputs like
Remote and remote collapse to one value.
In `@crates/types/src/api/tool.rs`:
- Around line 9-16: ToolAttributes currently derives serde in a human-readable
bitflag form, which makes the wire format Rust-specific and unstable across
languages. Update the ToolAttributes definition to use an explicit numeric serde
representation for the raw bits (for example via a custom serde helper or
equivalent bitflags serde support) so serialization/deserialization stays stable
and future unknown flags can round-trip cleanly.
In `@crates/types/src/codec/request.rs`:
- Around line 4-8: The rustdoc in request.rs contains a stale link to
crate::codec::traits::LlmCodec, which does not exist in this crate. Update the
module docs near AnnotatedLlmRequest so the reference is removed or rewritten to
the actual codec API exposed by codec, keeping the comment accurate and linkable
without referencing a missing trait.
---
Outside diff comments:
In `@crates/core/src/api/event.rs`:
- Around line 19-51: Keep the normalized_llm_request and normalized_llm_response
API available directly on Event so downstream callers that only use Event remain
source-compatible. The current EventNormalizationExt split changes the public
surface; either restore these as inherent methods on Event or add thin
forwarding wrappers on Event that delegate to
EventNormalizationExt::normalized_llm_request and
EventNormalizationExt::normalized_llm_response. Ensure the existing behavior for
annotated values and fallback serde_json/resolve decoding stays 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: dfb565c3-e1d7-4ddd-837b-99becc959851
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
Cargo.tomlcrates/core/Cargo.tomlcrates/core/src/api/event.rscrates/core/src/api/llm.rscrates/core/src/api/scope.rscrates/core/src/api/tool.rscrates/core/src/codec/request.rscrates/core/src/codec/response.rscrates/core/src/json.rscrates/core/src/observability/openinference.rscrates/core/src/observability/otel.rscrates/core/src/plugin.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/unit/codec/response_tests.rscrates/core/tests/unit/types_tests.rscrates/types/Cargo.tomlcrates/types/src/api/event.rscrates/types/src/api/llm.rscrates/types/src/api/mod.rscrates/types/src/api/scope.rscrates/types/src/api/tool.rscrates/types/src/codec/mod.rscrates/types/src/codec/request.rscrates/types/src/codec/response.rscrates/types/src/lib.rscrates/types/src/plugin.rscrates/types/tests/serialization_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (22)
**/{Cargo.toml,**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Maintain consistency between Rust package names in
Cargo.tomland their actual usage across the codebase
Files:
crates/types/Cargo.tomlcrates/types/src/api/tool.rscrates/types/src/plugin.rscrates/types/src/codec/mod.rsCargo.tomlcrates/types/tests/serialization_tests.rscrates/types/src/api/scope.rscrates/types/src/api/mod.rscrates/core/src/observability/openinference.rscrates/core/tests/unit/types_tests.rscrates/types/src/lib.rscrates/core/Cargo.tomlcrates/core/src/observability/otel.rscrates/core/tests/integration/api_surface_tests.rscrates/types/src/api/llm.rscrates/core/tests/unit/codec/response_tests.rscrates/core/src/plugin.rscrates/core/src/api/tool.rscrates/core/src/json.rscrates/core/src/codec/request.rscrates/core/src/api/scope.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/types/src/api/event.rscrates/core/src/api/llm.rscrates/core/src/codec/response.rscrates/core/src/api/event.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Rust crate names and module prefixes during coordinated rename operations
Files:
crates/types/Cargo.tomlcrates/types/src/api/tool.rscrates/types/src/plugin.rscrates/types/src/codec/mod.rsCargo.tomlcrates/types/tests/serialization_tests.rscrates/types/src/api/scope.rscrates/types/src/api/mod.rscrates/core/src/observability/openinference.rscrates/core/tests/unit/types_tests.rscrates/types/src/lib.rscrates/core/Cargo.tomlcrates/core/src/observability/otel.rscrates/core/tests/integration/api_surface_tests.rscrates/types/src/api/llm.rscrates/core/tests/unit/codec/response_tests.rscrates/core/src/plugin.rscrates/core/src/api/tool.rscrates/core/src/json.rscrates/core/src/codec/request.rscrates/core/src/api/scope.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/types/src/api/event.rscrates/core/src/api/llm.rscrates/core/src/codec/response.rscrates/core/src/api/event.rs
**/*.{py,txt,toml,cfg,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Python package names and top-level module imports during coordinated rename operations
Files:
crates/types/Cargo.tomlCargo.tomlcrates/core/Cargo.toml
**/Cargo.toml
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update WebAssembly crate names and generated package names during coordinated rename operations
Confirm or infer the target release version from
upstream/main:Cargo.toml. Derive the release branch asrelease/<major>.<minor>.
**/Cargo.toml: MaintainCargo.toml[workspace.package].versionas the source of truth for the Rust workspace and Python build versioning
KeepCargo.toml[workspace.dependencies]self-references aligned with the workspace version when the workspace version changes
After updating workspace package entries, runcargo check --workspaceto refreshCargo.lock
Files:
crates/types/Cargo.tomlCargo.tomlcrates/core/Cargo.toml
{docs/**,README.md,**/Cargo.toml,**/package.json,**/*.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Ensure renamed public surfaces are reflected consistently in manifests and docs for large or public-facing changes
Files:
crates/types/Cargo.tomlCargo.tomlcrates/core/Cargo.toml
**/*.{md,mdx,py,sh,yaml,yml,toml,json}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repo references, and build commands current
Files:
crates/types/Cargo.tomlCargo.tomlcrates/core/Cargo.toml
**/*.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license header in TOML configuration files using hash comment syntax
Files:
crates/types/Cargo.tomlCargo.tomlcrates/core/Cargo.toml
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.
Files:
crates/types/Cargo.tomlcrates/types/src/api/tool.rscrates/types/src/plugin.rscrates/types/src/codec/mod.rsCargo.tomlcrates/types/tests/serialization_tests.rscrates/types/src/api/scope.rscrates/types/src/api/mod.rscrates/core/src/observability/openinference.rscrates/core/tests/unit/types_tests.rscrates/types/src/lib.rscrates/core/Cargo.tomlcrates/core/src/observability/otel.rscrates/core/tests/integration/api_surface_tests.rscrates/types/src/api/llm.rscrates/core/tests/unit/codec/response_tests.rscrates/core/src/plugin.rscrates/core/src/api/tool.rscrates/core/src/json.rscrates/core/src/codec/request.rscrates/core/src/api/scope.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/types/src/api/event.rscrates/core/src/api/llm.rscrates/core/src/codec/response.rscrates/core/src/api/event.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, WebAssembly, 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,
integration patches, 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 wasm/ # wasm-bindgen WebAssembly binding and JS wrappers 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 third_party/ # P...
Files:
crates/types/Cargo.tomlcrates/types/src/api/tool.rscrates/types/src/plugin.rscrates/types/src/codec/mod.rsCargo.tomlcrates/types/tests/serialization_tests.rscrates/types/src/api/scope.rscrates/types/src/api/mod.rscrates/core/src/observability/openinference.rscrates/core/tests/unit/types_tests.rscrates/types/src/lib.rscrates/core/Cargo.tomlcrates/core/src/observability/otel.rscrates/core/tests/integration/api_surface_tests.rscrates/types/src/api/llm.rscrates/core/tests/unit/codec/response_tests.rscrates/core/src/plugin.rscrates/core/src/api/tool.rscrates/core/src/json.rscrates/core/src/codec/request.rscrates/core/src/api/scope.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/types/src/api/event.rscrates/core/src/api/llm.rscrates/core/src/codec/response.rscrates/core/src/api/event.rs
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Use
snake_casenaming convention for Rust identifiers (e.g.,nemo_relay_tool_call)
**/*.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: Runcargo fmt --allto format all Rust code
Runcargo clippy --workspace --all-targets -- -D warningsto enforce all clippy lints as errors
**/*.rs: Runcargo fmt --allwhen Rust files changed as part of WebAssembly work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files changed as part of WebAssembly 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
Run Rust formatting withcargo fmt --all
Run Rust linting withcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Usecargo fmtfor Rust code formatting
Runcargo clippy -- -D warningsto lint Rust code and treat all warnings as errors
Use Rust snake_case naming convention for Rust identifiers
Include SPDX license header in all Rust source files using double-slash comment syntax
Validate Rust code withuv run pre-commit run --all-filesto enforce cargo fmt formatting check, cargo clippy lints, and cargo deny aud...
Files:
crates/types/src/api/tool.rscrates/types/src/plugin.rscrates/types/src/codec/mod.rscrates/types/tests/serialization_tests.rscrates/types/src/api/scope.rscrates/types/src/api/mod.rscrates/core/src/observability/openinference.rscrates/core/tests/unit/types_tests.rscrates/types/src/lib.rscrates/core/src/observability/otel.rscrates/core/tests/integration/api_surface_tests.rscrates/types/src/api/llm.rscrates/core/tests/unit/codec/response_tests.rscrates/core/src/plugin.rscrates/core/src/api/tool.rscrates/core/src/json.rscrates/core/src/codec/request.rscrates/core/src/api/scope.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/types/src/api/event.rscrates/core/src/api/llm.rscrates/core/src/codec/response.rscrates/core/src/api/event.rs
**/*.{h,hpp,c,cpp,rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Ensure FFI header and library naming follows consistent conventions across platform-specific builds
Files:
crates/types/src/api/tool.rscrates/types/src/plugin.rscrates/types/src/codec/mod.rscrates/types/tests/serialization_tests.rscrates/types/src/api/scope.rscrates/types/src/api/mod.rscrates/core/src/observability/openinference.rscrates/core/tests/unit/types_tests.rscrates/types/src/lib.rscrates/core/src/observability/otel.rscrates/core/tests/integration/api_surface_tests.rscrates/types/src/api/llm.rscrates/core/tests/unit/codec/response_tests.rscrates/core/src/plugin.rscrates/core/src/api/tool.rscrates/core/src/json.rscrates/core/src/codec/request.rscrates/core/src/api/scope.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/types/src/api/event.rscrates/core/src/api/llm.rscrates/core/src/codec/response.rscrates/core/src/api/event.rs
**/*.{rs,py,go,js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions: Rust and Python use
snake_case, C FFI exports prefixednemo_relay_, Go usesPascalCasefor public APIs, Node.js usescamelCase.
Files:
crates/types/src/api/tool.rscrates/types/src/plugin.rscrates/types/src/codec/mod.rscrates/types/tests/serialization_tests.rscrates/types/src/api/scope.rscrates/types/src/api/mod.rscrates/core/src/observability/openinference.rscrates/core/tests/unit/types_tests.rscrates/types/src/lib.rscrates/core/src/observability/otel.rscrates/core/tests/integration/api_surface_tests.rscrates/types/src/api/llm.rscrates/core/tests/unit/codec/response_tests.rscrates/core/src/plugin.rscrates/core/src/api/tool.rscrates/core/src/json.rscrates/core/src/codec/request.rscrates/core/src/api/scope.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/types/src/api/event.rscrates/core/src/api/llm.rscrates/core/src/codec/response.rscrates/core/src/api/event.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: Keep async behavior on the existing tokio-based model. Bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
UseJson = serde_json::Valuein Rust-facing runtime APIs for JSON payload handling.
Files:
crates/types/src/api/tool.rscrates/types/src/plugin.rscrates/types/src/codec/mod.rscrates/types/tests/serialization_tests.rscrates/types/src/api/scope.rscrates/types/src/api/mod.rscrates/core/src/observability/openinference.rscrates/core/tests/unit/types_tests.rscrates/types/src/lib.rscrates/core/src/observability/otel.rscrates/core/tests/integration/api_surface_tests.rscrates/types/src/api/llm.rscrates/core/tests/unit/codec/response_tests.rscrates/core/src/plugin.rscrates/core/src/api/tool.rscrates/core/src/json.rscrates/core/src/codec/request.rscrates/core/src/api/scope.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/types/src/api/event.rscrates/core/src/api/llm.rscrates/core/src/codec/response.rscrates/core/src/api/event.rs
{crates/adaptive/**/*.rs,**/*test*.{rs,py,go,ts,js},**/*adaptive*test*.{rs,py,go,ts,js},docs/plugins/adaptive/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Maintain documented and tested validation and report behavior for adaptive surfaces
Files:
crates/types/tests/serialization_tests.rscrates/core/tests/unit/types_tests.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/unit/codec/response_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/types/tests/serialization_tests.rscrates/core/tests/unit/types_tests.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/unit/codec/response_tests.rs
crates/core/src/observability/{atif,otel,openinference}.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
When changing event fields in ATIF, OpenTelemetry, or OpenInference observability surfaces, keep the core event model in
crates/core/src/observability/atif.rs,crates/core/src/observability/otel.rs, andcrates/core/src/observability/openinference.rsin sync
Files:
crates/core/src/observability/openinference.rscrates/core/src/observability/otel.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/observability/openinference.rscrates/core/tests/unit/types_tests.rscrates/core/Cargo.tomlcrates/core/src/observability/otel.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/unit/codec/response_tests.rscrates/core/src/plugin.rscrates/core/src/api/tool.rscrates/core/src/json.rscrates/core/src/codec/request.rscrates/core/src/api/scope.rscrates/core/src/api/llm.rscrates/core/src/codec/response.rscrates/core/src/api/event.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
crates/core/**/*.rs: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths. Keep errors explicit and binding-appropriate at the wrapper layer.
Files:
crates/core/src/observability/openinference.rscrates/core/tests/unit/types_tests.rscrates/core/src/observability/otel.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/unit/codec/response_tests.rscrates/core/src/plugin.rscrates/core/src/api/tool.rscrates/core/src/json.rscrates/core/src/codec/request.rscrates/core/src/api/scope.rscrates/core/src/api/llm.rscrates/core/src/codec/response.rscrates/core/src/api/event.rs
crates/{core,adaptive}/**
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full matrix across Rust, Python, Go, Node.js, and WebAssembly
Files:
crates/core/src/observability/openinference.rscrates/core/tests/unit/types_tests.rscrates/core/Cargo.tomlcrates/core/src/observability/otel.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/unit/codec/response_tests.rscrates/core/src/plugin.rscrates/core/src/api/tool.rscrates/core/src/json.rscrates/core/src/codec/request.rscrates/core/src/api/scope.rscrates/core/src/api/llm.rscrates/core/src/codec/response.rscrates/core/src/api/event.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/observability/openinference.rscrates/core/tests/unit/types_tests.rscrates/core/src/observability/otel.rscrates/core/tests/integration/api_surface_tests.rscrates/core/tests/unit/codec/response_tests.rscrates/core/src/plugin.rscrates/core/src/api/tool.rscrates/core/src/json.rscrates/core/src/codec/request.rscrates/core/src/api/scope.rscrates/core/src/api/llm.rscrates/core/src/codec/response.rscrates/core/src/api/event.rs
crates/core/src/api/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Implement behavior first in Rust core API modules:
crates/core/src/api/and related core modules such ascrates/core/src/api/runtime/,crates/core/src/codec/, orcrates/core/src/json.rs
Files:
crates/core/src/api/tool.rscrates/core/src/api/scope.rscrates/core/src/api/llm.rscrates/core/src/api/event.rs
crates/core/src/api/{tool,llm}.rs
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Wire the new middleware chain into the execute path in
crates/core/src/api/tool.rsorcrates/core/src/api/llm.rsat the appropriate pipeline stage
Files:
crates/core/src/api/tool.rscrates/core/src/api/llm.rs
🔇 Additional comments (19)
crates/core/Cargo.toml (1)
33-33: 📐 Maintainability & Code QualityPlease confirm the required Rust validation suite ran.
This cohort changes Rust core surfaces, but the provided validation summary only lists downstream package tests. Please confirm
just test-rust,cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings, anduv run pre-commit run --all-fileswere run for this branch. As per coding guidelines, any Rust change must run those validation commands.Also applies to: 78-78
Source: Coding guidelines
crates/core/src/api/llm.rs (1)
31-31: 📐 Maintainability & Code QualityConfirm the required Rust/core validation for this public DTO re-export.
This touches
crates/coreand public API type locations; please include evidence forcargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,just test-rust, and the affected binding matrix or note why a binding surface is unaffected.As per coding guidelines, "
**/*.rs: Any Rust change must runjust test-rust", "cargo fmt --all", and "cargo clippy --workspace --all-targets -- -D warnings"; also, "{crates/core,crates/adaptive}/**/*: Changes tocrates/coreorcrates/adaptivemust run the full language matrix".Source: Coding guidelines
crates/core/src/api/scope.rs (1)
18-18: LGTM!crates/core/src/api/tool.rs (1)
23-23: LGTM!crates/core/src/observability/openinference.rs (1)
24-24: LGTM!crates/core/src/codec/request.rs (1)
4-6: LGTM!crates/core/src/json.rs (1)
10-10: LGTM!crates/core/src/codec/response.rs (1)
8-8: 🎯 Functional CorrectnessNo API break from the CostEstimate move — the
CostEstimatehelper methods now live onnemo_relay_types::codec::response::CostEstimate, so the re-export incrates/core/src/codec/response.rspreserves the public API.> Likely an incorrect or invalid review comment.Cargo.toml (1)
7-7: LGTM!Also applies to: 28-28
crates/types/Cargo.toml (1)
1-27: LGTM!crates/types/src/lib.rs (1)
1-21: LGTM!crates/types/src/api/llm.rs (2)
22-29: LGTM!
11-19: 🗄️ Data Integrity & IntegrationNo change needed for
LlmAttributes/ToolAttributesserde. bitflags’ serde support already preserves unknown bits, so this is not a forward-compatibility bug. The current derive is acceptable here.> Likely an incorrect or invalid review comment.crates/types/src/plugin.rs (1)
1-35: LGTM!crates/types/src/api/mod.rs (1)
1-13: LGTM!crates/types/src/codec/mod.rs (1)
1-9: LGTM!crates/types/src/api/scope.rs (1)
1-79: LGTM!crates/types/src/codec/response.rs (1)
1-365: LGTM!crates/types/tests/serialization_tests.rs (1)
17-80: LGTM!
Signed-off-by: Alex Fournier <afournier@nvidia.com> # Conflicts: # crates/core/src/observability/openinference.rs # crates/core/src/observability/otel.rs
Signed-off-by: Alex Fournier <afournier@nvidia.com>
|
/ok to test cfdfb30 |
|
/ok to test f34b734 |
License DiffCompared against Lockfile license changesLockfile License ChangesRustAdded
Removed
Updated/Changed
NodeAdded
Removed
Updated/Changed
PythonAdded
Removed
Updated/Changed
Status output |
|
/merge |
#### Overview Add the Rust native plugin authoring SDK on top of the shared DTO crate. Stack dependency: this PR cannot be merged until parent stacked PR #305 is merged: #305 - [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 - Add `nemo-relay-plugin` with the stable native C ABI v1. - Add the high-level typed Rust authoring layer and callback trampoline tests. - Add the standalone native Rust plugin example beside the SDK. Validation run across the completed stack: - `cargo test -p nemo-relay-types` - `cargo test -p nemo-relay-plugin` - `cargo test -p nemo-relay-worker-proto` - `cargo test -p nemo-relay-worker` - `cargo check -p nemo-relay-cli` #### Where should the reviewer start? Start with `crates/plugin/src/lib.rs`, then check `crates/plugin/tests/typed_callbacks.rs`. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to: #302 - Relates to: #230 ## Summary by CodeRabbit * **New Features** * Added a Rust native plugin SDK with a stable ABI and typed callback interfaces for plugin lifecycle, tool/LLM guardrails, and intercepts (including LLM streaming). * Added a dynamic Rust native plugin example with configurable JSON validation, request/execution/stream interception, and optional isolated-scope event emission. * **Tests** * Added a comprehensive typed-callback test suite covering ABI checks, JSON/error handling, registration cleanup, stream behavior, and panic safety. * **Documentation** * Added example README plus plugin build/config manifests. * **Chores** * Updated the workspace to include the plugin crate and extended local clean rules for example artifacts. Authors: - Will Killian (https://github.com/willkill07) Approvers: - Alex Fournier (https://github.com/afourniernv) URL: #306
Bring in NVIDIA#304 (prefer normalized exporter fallbacks) and NVIDIA#305 (shared relay DTO crate) so the token/cost docs and tests reflect current behavior. Clean auto-merge; my two contract tests pass on the merged tree. Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
…ss review (RELAY-243) After merging main, the docs predated NVIDIA#304 (prefer normalized exporter fallbacks) and NVIDIA#305 (shared DTO crate). Corrected: exporter cost precedence (normalized-first, umbrella wording); ATIF now consumes the codec annotation (not raw-only) in the exporter table, atif.mdx, and Stability; marked the ATIF raw-only divergence Fixed in 0.5 (known-issues), with USD-only cost kept as an active 0.5 limitation; qualified Granularity (user/agent steps; final_metrics sums present step metrics on this trajectory, excludes subagents); corrected the total_tokens projection note; scoped the additive-compat promise to serialized JSON/ATOF (Rust structs/enums are exhaustive); qualified 'optional/unset'. Tests: rewrote the OTel cost-only test to drive OtelEventProcessor and assert the finished span's keys (rejecting any token-ish or gen_ai key, closing the nemo_relay.prompt_tokens gap); added a codec-level test that OpenAIChatCodec drops completion_tokens_details. Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
#### Overview Document and stabilize NeMo Relay's LLM token and cost field semantics. This freezes the current behavior as a documented contract and locks it with characterization tests. There is no runtime behavior change. The branch is merged with current `main`, so the docs and tests reflect #304 (prefer normalized exporter fallbacks) and #305 (shared `nemo_relay_types` DTO crate). - [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 canonical **Token and Cost Field Semantics** section to `docs/integrate-into-frameworks/provider-response-codecs.mdx`: - `Usage` and `CostEstimate` field reference (names, units, optionality; optional fields can also be codec-computed or pricing-synthesized). - Per-provider token normalization table (OpenAI Chat / OpenAI Responses / Anthropic → `Usage`). - **Granularity**: per-call values; an exported call yields a `user` start step (no metrics) and an `agent` end step that carries metrics; `final_metrics` is the only aggregate (a per-trajectory sum of present step metrics, excluding embedded subagent trajectories). - Exporter field-mapping table across ATOF / ATIF / OpenInference / OpenTelemetry: OpenTelemetry is cost-only and currency-aware; ATIF and OpenInference are USD-only; ATIF sources metrics from the codec-normalized usage with raw-payload fallback. - A **Stability** subsection (stable as of ATOF `0.1` / `ATIF-v1.7` / pricing catalog `version: 1`). The additive-compatibility promise is scoped to the serialized JSON/ATOF shapes; the Rust `Usage`/`CostEstimate` structs and `CostSource` enum are exhaustive, so adding a field/variant is source-breaking for Rust consumers. Short field pointers + back-links were added to the OpenTelemetry, OpenInference, and ATIF exporter pages. Cost policy is stated once on the canonical page, per the runtime-contract docs convention. Two characterization tests lock the freeze: - The OpenTelemetry LLM span emits cost only — driven end-to-end through `OtelEventProcessor`, asserting the finished span carries `nemo_relay.llm.cost.{total,currency}` and no token-count attribute in any namespace (`token`-containing or `gen_ai.*` keys fail). - `OpenAIChatCodec` drops `completion_tokens_details`, and `Usage` ignores unmodeled provider subfields (forward-compat: no serde catch-all). Existing tests already cover the remaining projections, per-provider mapping, reasoning-tokens-in-`api_specific`, and the USD-only/currency-aware cost behavior. **Testing:** targeted `cargo test` on the merged tree (the new/changed tests pass; perturbing the OTel exporter with a `nemo_relay.prompt_tokens` attribute makes the span test fail as intended, then reverted), `just docs-linkcheck` (0 errors), and `pre-commit` (SPDX, markdown linkcheck, cargo fmt/clippy/check) all pass. #### Where should the reviewer start? `docs/integrate-into-frameworks/provider-response-codecs.mdx` — the **Token and Cost Field Semantics** section (the exporter field-mapping table and the **Stability** subsection are the core contract). Then `crates/core/tests/unit/observability/otel_tests.rs::llm_end_emits_cost_only_no_token_or_gen_ai_attributes`. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - N/A ## Summary by CodeRabbit * **Bug Fixes** * Response decoding now preserves supported `usage` token totals while omitting provider-only completion token detail fields. * `Usage` serialization now ignores unknown/unmodeled provider subfields. * Observability “LLM end” spans now emit only total cost attributes (no token-related or `gen_ai*` attributes). * **Tests** * Added unit tests covering decoding behavior, handling of unmodeled `usage` fields, and cost-only observability attributes. * **Documentation** * Added/expanded stable “Token and Cost Field Semantics” mappings, including updates for ATIF, OpenInference, and OpenTelemetry expected-output fields. Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
Overview
Add the shared serializable Relay DTO crate that the rest of the dynamic plugin stack builds on.
Stack dependency: this is the root PR in the dynamic plugin stack. It has no parent stacked PR. Later PRs in the stack cannot be merged until this PR is merged.
Details
nemo-relay-typesas the shared DTO source of truth.nemo_relaypaths.Validation run across the completed stack:
cargo test -p nemo-relay-typescargo test -p nemo-relay-plugincargo test -p nemo-relay-worker-protocargo test -p nemo-relay-workercargo check -p nemo-relay-cliWhere should the reviewer start?
Start with
crates/types/src/lib.rsand the core re-export changes incrates/core/src/api/event.rs.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
New Features
Bug Fixes
Tests