feat(codec): add a provider codec factory and route built-in consumers through it#396
feat(codec): add a provider codec factory and route built-in consumers through it#396zhongxuanwang-nv wants to merge 5 commits into
Conversation
Built-in provider codec dispatch is currently hand-rolled in several private spots across crates (response-cache decode/streaming, NeMo Guardrails, PII redaction, the CLI gateway, and ACG). Each one re-encodes the set of built-in providers and their canonical string names behind a `_ => None/Err` arm that silently fails to gain a new provider. Add a single source of truth in `nemo_relay::codec::resolve`: - `ProviderSurface::codec_name` / `ProviderSurface::from_codec_name` map a surface to and from its canonical name (`openai_chat`, `openai_responses`, `anthropic_messages`), round-tripping through the built-in registry. - `request_codec`, `response_codec`, and `streaming_codec` construct the matching built-in `LlmCodec`, `LlmResponseCodec`, and `StreamingCodec` for a surface. - `supported_codec_names` lists the accepted names for config validation so the advertised set never drifts from the codecs that can actually be constructed. Construction and the canonical name live on `ProviderSurfaceDescriptor` next to each codec, matching the existing detect/decode descriptor pattern, so adding a provider is one descriptor edit. This is additive and changes no existing behavior. Downstream call sites can adopt it in follow-ups; the response-cache migration is staged separately because that feature is not yet on main. Tested: cargo fmt; cargo test -p nemo-relay (codec suite incl. new factory tests); cargo clippy -p nemo-relay --all-targets -D warnings; cargo build --workspace. Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
Route built-in provider codec dispatch through the new `nemo_relay::codec::resolve` factory instead of re-encoding the provider set and canonical names in each consumer: - CLI gateway `codecs_for_route` maps `ProviderRoute` to a `ProviderSurface` and builds the streaming/response codecs via the factory. - Adaptive ACG `decode_request_for_surface` builds the request codec via the factory (error messages unchanged). - NeMo Guardrails `resolve_codec`/`LocalGuardrailsCodec` resolve the codec name and construct codecs through the factory; the enum stays only for provider-specific streaming-text extraction. - PII redaction resolves the codec name once via the factory and stores the request/response codecs, dropping the local `BuiltinRequestResponseCodec` trait and `instantiate_builtin_codec`; `BuiltinCodecName` stays only for response overlay. No behavior change and all internal (private fns / pub(crate) enums), so not breaking. Removes the `_ => None/Err` name arms that silently failed to gain a new provider. Tested: cargo fmt; cargo clippy --workspace --all-targets -D warnings; cargo build --workspace; cargo test -p nemo-relay -p nemo-relay-adaptive -p nemo-relay-pii-redaction -p nemo-relay-cli. Two CLI plugin_shim tests and one server test fail on the base commit too (environment-dependent, unrelated to this change). Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
…m the factory Address audit feedback on the provider-codec factory: - Drop non-essential narrating comments added with the factory and its consumers; keep concise public-API docstrings. - Validate the NeMo Guardrails and PII redaction `codec` config through `supported_codec_names()` instead of a hardcoded name list, so the accepted set is single-sourced with the factory (diagnostics unchanged). No behavior change. Tested: cargo fmt; cargo clippy --workspace --all-targets -D warnings; cargo test -p nemo-relay-pii-redaction and -p nemo-relay (guardrails + codec suites). Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
WalkthroughThis change centralizes provider codec registration and resolution, then updates adaptive decoding, gateway dispatch, Guardrails, and PII redaction to use provider-surface codec factories with dynamic supported-codec validation. ChangesProvider codec resolution
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RequestRoute
participant ProviderSurface
participant CodecResolver
participant RequestOrResponseCodec
RequestRoute->>ProviderSurface: map route or request surface
ProviderSurface->>CodecResolver: resolve descriptor
CodecResolver->>RequestOrResponseCodec: construct codec
RequestRoute->>RequestOrResponseCodec: decode or encode payload
RequestOrResponseCodec-->>RequestRoute: serialized or decoded content
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 |
github-actions
Bot
added
size:M
…face labels Address review feedback: the factory-constructed request/response codecs are stateless, so the descriptor constructors and public factory functions return Box instead of Arc; the CLI gateway and PII redaction convert via Arc::from where shared ownership is genuinely required. Replace the hand-rolled ACG surface_label match with a strum AsRefStr derive on RequestSurface (error labels are now the snake_case variant names). Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
License DiffCompared against Lockfile license changesLockfile License ChangesRustAdded
Removed
Updated/Changed
NodeAdded
Removed
Updated/Changed
PythonAdded
Removed
Updated/Changed
Status output |
…odec-factory Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/pii-redaction/src/component.rs (1)
711-719: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the diagnostic message from the registry to finish centralizing codec names.
The check now validates against
supported_codec_names(), but the failure message on Line 718 still hard-codes'openai_chat', 'openai_responses', or 'anthropic_messages'. If the registry gains/renames a surface, validation and the user-facing message will silently diverge, defeating the centralization goal of this change.♻️ Build the message from the canonical list
- "codec must be 'openai_chat', 'openai_responses', or 'anthropic_messages'".to_string(), + format!( + "codec must be one of: {}", + supported_codec_names().join(", ") + ),🤖 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/pii-redaction/src/component.rs` around lines 711 - 719, Update the codec validation diagnostic in the surrounding validation function to derive its message from the canonical list returned by supported_codec_names(), rather than hard-coding codec names. Format the registry values into the existing “codec must be …” message while preserving the current push_policy_diag arguments.crates/core/src/plugins/nemo_guardrails/component.rs (1)
905-914: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded error text will drift from the dynamic
supported_codec_names()check.The validation condition now derives from
supported_codec_names(), but the diagnostic message at Line 912 still hardcodes the three current names. IfBUILTIN_PROVIDER_SURFACESgains a new provider surface, the check will accept it while the error message (shown for other unsupported names) keeps advertising only the stale three — silently drifting from the actual supported set, defeating the purpose of centralizing codec names.♻️ Generate the message from the canonical list
if !supported_codec_names().contains(&codec) { + let supported = supported_codec_names().join(", "); push_policy_diag( diagnostics, policy.unsupported_value, "nemo_guardrails.unsupported_value", Some(NEMO_GUARDRAILS_PLUGIN_KIND.to_string()), Some("codec".to_string()), - "codec must be 'openai_chat', 'openai_responses', or 'anthropic_messages'".to_string(), + format!("codec must be one of: {supported}"), ); }🤖 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/plugins/nemo_guardrails/component.rs` around lines 905 - 914, Update the unsupported-codec diagnostic in the validation logic using supported_codec_names() so its message is generated from the canonical supported list rather than hardcoded names; preserve the existing policy diagnostic structure and format the list clearly for users.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/core/src/plugins/nemo_guardrails/component.rs`:
- Around line 905-914: Update the unsupported-codec diagnostic in the validation
logic using supported_codec_names() so its message is generated from the
canonical supported list rather than hardcoded names; preserve the existing
policy diagnostic structure and format the list clearly for users.
In `@crates/pii-redaction/src/component.rs`:
- Around line 711-719: Update the codec validation diagnostic in the surrounding
validation function to derive its message from the canonical list returned by
supported_codec_names(), rather than hard-coding codec names. Format the
registry values into the existing “codec must be …” message while preserving the
current push_policy_diag arguments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: b6e48a3b-cd65-49e1-b7fd-1e101734d0a3
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
crates/adaptive/Cargo.tomlcrates/adaptive/src/acg/request_surfaces/mod.rscrates/adaptive/src/acg_component.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/cli/src/gateway.rscrates/core/src/codec/anthropic.rscrates/core/src/codec/openai_chat.rscrates/core/src/codec/openai_responses.rscrates/core/src/codec/resolve.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/python.rscrates/core/tests/unit/codec/resolve_tests.rscrates/pii-redaction/src/builtin.rscrates/pii-redaction/src/component.rscrates/pii-redaction/src/overlay.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: CodeRabbit / Review
- GitHub Check: CodeRabbit / Review
🧰 Additional context used
📓 Path-based instructions (18)
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/adaptive/Cargo.tomlcrates/core/src/codec/openai_chat.rscrates/adaptive/src/acg/request_surfaces/mod.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/core/src/codec/anthropic.rscrates/core/src/codec/openai_responses.rscrates/core/tests/unit/codec/resolve_tests.rscrates/adaptive/src/acg_component.rscrates/core/src/codec/resolve.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/python.rs
**/Cargo.toml
📄 CodeRabbit inference engine (.agents/skills/prepare-code-freeze/SKILL.md)
Confirm or infer the target release version from
upstream/main:Cargo.toml. Derive the release branch asrelease/<major>.<minor>.Keep Rust package names and workspace metadata in
Cargo.tomlinternally consistent across the project.
Files:
crates/adaptive/Cargo.toml
**/*.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all TOML files using the
#comment form.
Files:
crates/adaptive/Cargo.toml
crates/adaptive/**
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep
crates/adaptivealigned with the canonical adaptive config schema, built-in section helpers, plugin lifecycle, and validation/report behavior.
Files:
crates/adaptive/Cargo.tomlcrates/adaptive/src/acg/request_surfaces/mod.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/adaptive/src/acg_component.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/adaptive/Cargo.tomlcrates/core/src/codec/openai_chat.rscrates/adaptive/src/acg/request_surfaces/mod.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/pii-redaction/src/overlay.rscrates/core/src/codec/anthropic.rscrates/pii-redaction/src/component.rscrates/core/src/codec/openai_responses.rscrates/core/tests/unit/codec/resolve_tests.rscrates/adaptive/src/acg_component.rscrates/cli/src/gateway.rscrates/core/src/codec/resolve.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/pii-redaction/src/builtin.rscrates/core/src/plugins/nemo_guardrails/python.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/adaptive/Cargo.tomlcrates/core/src/codec/openai_chat.rscrates/adaptive/src/acg/request_surfaces/mod.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/core/src/codec/anthropic.rscrates/core/src/codec/openai_responses.rscrates/core/tests/unit/codec/resolve_tests.rscrates/adaptive/src/acg_component.rscrates/core/src/codec/resolve.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/python.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:
crates/adaptive/Cargo.tomlcrates/core/src/codec/openai_chat.rscrates/adaptive/src/acg/request_surfaces/mod.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/pii-redaction/src/overlay.rscrates/core/src/codec/anthropic.rscrates/pii-redaction/src/component.rscrates/core/src/codec/openai_responses.rscrates/core/tests/unit/codec/resolve_tests.rscrates/adaptive/src/acg_component.rscrates/cli/src/gateway.rscrates/core/src/codec/resolve.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/pii-redaction/src/builtin.rscrates/core/src/plugins/nemo_guardrails/python.rs
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/core/src/codec/openai_chat.rscrates/adaptive/src/acg/request_surfaces/mod.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/pii-redaction/src/overlay.rscrates/core/src/codec/anthropic.rscrates/pii-redaction/src/component.rscrates/core/src/codec/openai_responses.rscrates/core/tests/unit/codec/resolve_tests.rscrates/adaptive/src/acg_component.rscrates/cli/src/gateway.rscrates/core/src/codec/resolve.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/pii-redaction/src/builtin.rscrates/core/src/plugins/nemo_guardrails/python.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/src/codec/openai_chat.rscrates/core/src/codec/anthropic.rscrates/core/src/codec/openai_responses.rscrates/core/tests/unit/codec/resolve_tests.rscrates/core/src/codec/resolve.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/python.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/src/codec/openai_chat.rscrates/adaptive/src/acg/request_surfaces/mod.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/pii-redaction/src/overlay.rscrates/core/src/codec/anthropic.rscrates/pii-redaction/src/component.rscrates/core/src/codec/openai_responses.rscrates/core/tests/unit/codec/resolve_tests.rscrates/adaptive/src/acg_component.rscrates/cli/src/gateway.rscrates/core/src/codec/resolve.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/pii-redaction/src/builtin.rscrates/core/src/plugins/nemo_guardrails/python.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/core/src/codec/openai_chat.rscrates/adaptive/src/acg/request_surfaces/mod.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/pii-redaction/src/overlay.rscrates/core/src/codec/anthropic.rscrates/pii-redaction/src/component.rscrates/core/src/codec/openai_responses.rscrates/core/tests/unit/codec/resolve_tests.rscrates/adaptive/src/acg_component.rscrates/cli/src/gateway.rscrates/core/src/codec/resolve.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/pii-redaction/src/builtin.rscrates/core/src/plugins/nemo_guardrails/python.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/core/src/codec/openai_chat.rscrates/adaptive/src/acg/request_surfaces/mod.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/pii-redaction/src/overlay.rscrates/core/src/codec/anthropic.rscrates/pii-redaction/src/component.rscrates/core/src/codec/openai_responses.rscrates/core/tests/unit/codec/resolve_tests.rscrates/adaptive/src/acg_component.rscrates/cli/src/gateway.rscrates/core/src/codec/resolve.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/pii-redaction/src/builtin.rscrates/core/src/plugins/nemo_guardrails/python.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/core/src/codec/openai_chat.rscrates/adaptive/src/acg/request_surfaces/mod.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/pii-redaction/src/overlay.rscrates/core/src/codec/anthropic.rscrates/pii-redaction/src/component.rscrates/core/src/codec/openai_responses.rscrates/core/tests/unit/codec/resolve_tests.rscrates/adaptive/src/acg_component.rscrates/cli/src/gateway.rscrates/core/src/codec/resolve.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/pii-redaction/src/builtin.rscrates/core/src/plugins/nemo_guardrails/python.rs
crates/core/src/{api/**/*.rs,api/runtime/**/*.rs,codec/**/*.rs,json.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Implement the new or changed public runtime behavior first in the Rust core, especially under
crates/core/src/api/and related core modules such ascrates/core/src/api/runtime/,crates/core/src/codec/, andcrates/core/src/json.rs.
Files:
crates/core/src/codec/openai_chat.rscrates/core/src/codec/anthropic.rscrates/core/src/codec/openai_responses.rscrates/core/src/codec/resolve.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/core/src/codec/openai_chat.rscrates/adaptive/src/acg/request_surfaces/mod.rscrates/pii-redaction/src/overlay.rscrates/core/src/codec/anthropic.rscrates/pii-redaction/src/component.rscrates/core/src/codec/openai_responses.rscrates/adaptive/src/acg_component.rscrates/cli/src/gateway.rscrates/core/src/codec/resolve.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/pii-redaction/src/builtin.rscrates/core/src/plugins/nemo_guardrails/python.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/core/src/codec/openai_chat.rscrates/adaptive/src/acg/request_surfaces/mod.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/pii-redaction/src/overlay.rscrates/core/src/codec/anthropic.rscrates/pii-redaction/src/component.rscrates/core/src/codec/openai_responses.rscrates/core/tests/unit/codec/resolve_tests.rscrates/adaptive/src/acg_component.rscrates/cli/src/gateway.rscrates/core/src/codec/resolve.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/pii-redaction/src/builtin.rscrates/core/src/plugins/nemo_guardrails/python.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/codec/openai_chat.rscrates/adaptive/src/acg/request_surfaces/mod.rscrates/adaptive/tests/unit/acg_component_tests.rscrates/core/src/codec/anthropic.rscrates/core/src/codec/openai_responses.rscrates/core/tests/unit/codec/resolve_tests.rscrates/adaptive/src/acg_component.rscrates/core/src/codec/resolve.rscrates/core/src/plugins/nemo_guardrails/component.rscrates/core/src/plugins/nemo_guardrails/python.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/adaptive/tests/unit/acg_component_tests.rscrates/core/tests/unit/codec/resolve_tests.rs
🔇 Additional comments (18)
crates/core/src/codec/resolve.rs (1)
46-56: LGTM!Also applies to: 148-205
crates/core/src/codec/anthropic.rs (1)
59-62: LGTM!crates/core/src/codec/openai_chat.rs (1)
36-39: LGTM!crates/core/src/codec/openai_responses.rs (1)
51-54: LGTM!crates/core/tests/unit/codec/resolve_tests.rs (1)
393-562: LGTM!crates/pii-redaction/src/builtin.rs (1)
17-19: LGTM!Also applies to: 31-32, 117-131, 244-257
crates/pii-redaction/src/overlay.rs (1)
7-7: LGTM!Also applies to: 18-24
crates/core/src/plugins/nemo_guardrails/component.rs (1)
14-14: LGTM!crates/core/src/plugins/nemo_guardrails/python.rs (3)
24-24: LGTM!
815-849: LGTM!Delegation to
request_codec/response_codecfactories correctly mirrors the surfaces exercised inresolve_tests.rs(request_codec_decodes_each_surface,response_codec_decodes_each_surface), and the exhaustivefrom_provider_surfacematch will force a compile error if new provider surfaces are added, which is a good safety net.
851-867: LGTM!crates/adaptive/Cargo.toml (2)
28-28: 📐 Maintainability & Code QualityConfirm required Rust validation was run for this change.
Adding
strumand consumingAsRefStrinrequest_surfaces/mod.rsis a Rust source change across the workspace. As per coding guidelines, "Any Rust change must runjust test-rust", "Runcargo fmt --all", and "runcargo clippy --workspace --all-targets -- -D warnings" before this is merged.Source: Coding guidelines
28-28: LGTM!crates/adaptive/src/acg/request_surfaces/mod.rs (1)
21-28: LGTM!
provider_surface()correctly mirrorsfrom_provider_surface(), and theAsRefStr/snake_casederive addresses the earlier review feedback requesting a strum-based label instead of a hand-rolled one.Also applies to: 54-61
crates/adaptive/src/acg_component.rs (1)
31-32: LGTM!
decode_request_for_surfacenow delegates to the centralizedrequest_codec(provider_surface)factory and surfaces the request-surface label viaAsRefStr, matchingcrates/core/src/codec/resolve.rs'srequest_codeccontract.Also applies to: 87-97
crates/adaptive/tests/unit/acg_component_tests.rs (1)
1119-1137: LGTM!Assertions correctly track the new error text produced by
decode_request_for_surfaceafter the factory refactor.crates/cli/src/gateway.rs (2)
20-23: LGTM!
ProviderRoute::provider_surface()mapping is consistent with the variant coverage used elsewhere in thisimpl(alignment_route,upstream_url), and correctly returnsNonefor the token-count/model-listing routes that don't need schema codecs.Also applies to: 951-959
251-262: 🩺 Stability & AvailabilityRouteCodecs stays request-scoped
codecs_for_route(prepared.provider)is called inside the managed request path and the resultingRouteCodecsis passed by value into each handler, so the single-use streaming codec is not shared across requests.
2 participants
Overview
Add a single provider-codec factory in
nemo_relay::codec::resolveand route the previously hand-rolled surface/name → codec dispatch across the built-in consumers through it, so the set of built-in provider codecs and their canonical names live in one place instead of being re-encoded per crate (several used_ => None/Errarms that silently failed to gain a new provider).Details
Factory in
crates/core/src/codec/resolve.rs, hung offProviderSurfaceDescriptornext to each codec:ProviderSurface::codec_name/from_codec_name— canonical name ↔ surface.request_codec/response_codec/streaming_codec— construct the built-inLlmCodec/LlmResponseCodec/StreamingCodec(returned asBox; the codecs are stateless, so callers that share them convert withArc::fromat their own boundary).supported_codec_names— accepted names for config validation.Consumers routed through the factory (all internal — private fns /
pub(crate)enums — so not breaking):codecs_for_routevia newProviderRoute::provider_surface.decode_request_for_surfacevia newRequestSurface::provider_surface(error labels are now strum-derived from the surface variant names, per review).resolve_codec/LocalGuardrailsCodec(enum retained for provider-specific streaming-text extraction).BuiltinRequestResponseCodec+instantiate_builtin_codec; split the codec into request/response codecs;BuiltinCodecNameretained for response overlay.supported_codec_names()instead of a hardcoded list.Additive
pub/pub(crate)only; no serialized, wire, config, event, or cache-key schema change.Follow-ups (out of scope here): fully single-source the JSON-schema/editor display lists (currently order-fixed consts) with drift-guard tests; adopt the factory in the opt-in response-cache feature once it lands.
Where should the reviewer start?
crates/core/src/codec/resolve.rs— the newProviderSurfacemethods andrequest_codec/response_codec/streaming_codec/supported_codec_names, and how they hang offProviderSurfaceDescriptor. Thencrates/core/tests/unit/codec/resolve_tests.rs. The four consumer migrations all follow the same pattern.Validation:
cargo fmt,cargo clippy --workspace --all-targets -- -D warnings,cargo build --workspace, andcargo testfornemo-relay(codec + guardrails),nemo-relay-adaptive, andnemo-relay-pii-redaction.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
New Features
Bug Fixes