feat(codec): add OCI Generative AI provider support - #554
Conversation
github-actions
Bot
added
size:L
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds OCI GenAI request and response variants, codecs for GENERIC, COHERE, and COHEREV2 formats, provider registration, middleware integrations, adaptive cache handling, language bindings, observability mapping, and validation coverage. ChangesOCI GenAI provider support
Estimated code review effort: 5 (Critical) | ~100 minutes Sequence Diagram(s)sequenceDiagram
participant Client as Client request
participant Resolver as Provider resolver
participant Codec as OCIGenAIChatCodec
participant Middleware as Guardrails or PII redaction
participant Pipeline as LLM pipeline
participant Binding as Python or Node binding
Client->>Resolver: submit OCI payload
Resolver->>Codec: select OCI GenAI provider surface
Codec->>Middleware: normalize request or response
Middleware->>Pipeline: apply policy and annotations
Binding->>Codec: invoke decode, encode, or decode_response
Codec-->>Binding: return normalized JSON or error
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
How this series fits togetherThis PR is layer 1 of 4. Each layer adds one capability, each is independently buildable and testable, and each later branch is a pure superset of the previous — so review feedback here rebases cleanly through the rest of the stack.
Branch links and the full plan of record are on #548. Lineage: #549 (Python approach, closed per review) → #552 (this same content as a single Rust PR, closed in favor of this reviewable split). Verification provenanceEvery branch in the stack passes the full local gate on its own: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/core/src/codec/oci_genai.rs`:
- Around line 124-162: Update flatten_all_text_parts to return None for an empty
parts slice, while preserving Text concatenation for non-empty arrays containing
only valid TEXT parts. This ensures decode_generic_content returns None for
content: [] and does not mark tool-call-only messages as containing text.
In `@crates/core/tests/unit/codec/oci_genai_tests.rs`:
- Around line 131-167: Add error-path coverage alongside the existing OCI GenAI
codec tests for GENERIC responses: create a test such as
test_invalid_generic_content_shape_errors that passes a numeric or object
message.content to OCIGenAIChatCodec.decode_response and asserts it returns an
error. Also cover an array containing a non-object part if practical, ensuring
decode_generic_content rejects each invalid shape with
FlowError::InvalidArgument.
🪄 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: a9645a77-4c88-4c9e-8a1c-90f1492e291a
📒 Files selected for processing (5)
crates/core/src/codec/mod.rscrates/core/src/codec/oci_genai.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/types/src/codec/request.rscrates/types/src/codec/response.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (17)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/core/src/codec/oci_genai.rscrates/core/tests/unit/codec/oci_genai_tests.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/codec/mod.rscrates/core/src/codec/oci_genai.rscrates/core/tests/unit/codec/oci_genai_tests.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/mod.rscrates/core/src/codec/oci_genai.rscrates/core/tests/unit/codec/oci_genai_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/core/src/codec/oci_genai.rscrates/core/tests/unit/codec/oci_genai_tests.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/core/src/codec/oci_genai.rscrates/core/tests/unit/codec/oci_genai_tests.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/core/src/codec/oci_genai.rscrates/core/tests/unit/codec/oci_genai_tests.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/core/src/codec/oci_genai.rscrates/core/tests/unit/codec/oci_genai_tests.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/mod.rscrates/core/src/codec/oci_genai.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/mod.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/core/src/codec/oci_genai.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/core/src/codec/oci_genai.rscrates/core/tests/unit/codec/oci_genai_tests.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/src/codec/mod.rscrates/core/src/codec/oci_genai.rscrates/core/tests/unit/codec/oci_genai_tests.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/core/src/codec/oci_genai.rscrates/core/tests/unit/codec/oci_genai_tests.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/core/src/codec/oci_genai.rscrates/core/tests/unit/codec/oci_genai_tests.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/mod.rscrates/core/src/codec/oci_genai.rscrates/core/tests/unit/codec/oci_genai_tests.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:
crates/types/src/codec/response.rscrates/types/src/codec/request.rs
crates/types/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Runtime helpers must cover marks, scopes, continuations, and isolated scope stacks.
Files:
crates/types/src/codec/response.rscrates/types/src/codec/request.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/core/tests/unit/codec/oci_genai_tests.rs
🔇 Additional comments (8)
crates/types/src/codec/request.rs (1)
537-549: LGTM!crates/types/src/codec/response.rs (1)
329-339: LGTM!crates/core/src/codec/mod.rs (1)
19-19: LGTM!crates/core/src/codec/oci_genai.rs (3)
197-254: LGTM!
262-329: LGTM!
90-101: LGTM!Also applies to: 332-341
crates/core/tests/unit/codec/oci_genai_tests.rs (2)
16-129: LGTM!
169-188: LGTM!
ccb3281 to
21d6e18
Compare
|
Both review findings addressed in the updated commit:
Full workspace build and test suite re-run green after the change; the fix is propagated through the staged follow-up branches of the series. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/core/src/codec/oci_genai.rs`:
- Around line 313-333: The decode_response_tool_call path silently discards
malformed tool calls when the name is missing or non-string. Add an appropriate
debug or warning log before returning None for this validation failure,
including enough context to identify the malformed OCI payload while preserving
the existing successful decoding behavior.
In `@crates/core/tests/unit/codec/oci_genai_tests.rs`:
- Around line 1-210: Add a test near test_invalid_generic_content_shape_errors
that decodes a GENERIC response with mixed TEXT and non-TEXT content through
OCIGenAIChatCodec::decode_response. Assert the message is MessageContent::Parts,
contains both parts, and specifically verify the non-TEXT part is represented as
ContentPart::ProviderNative with its original payload preserved.
🪄 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: 09237b3d-efcb-4116-9173-2272097f9a8c
📒 Files selected for processing (5)
crates/core/src/codec/mod.rscrates/core/src/codec/oci_genai.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/types/src/codec/request.rscrates/types/src/codec/response.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (17)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.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/codec/mod.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.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/mod.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.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/mod.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.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/mod.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.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/mod.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.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/mod.rscrates/core/src/codec/oci_genai.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/mod.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/core/src/codec/oci_genai.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/src/codec/mod.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.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/mod.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/types/src/codec/request.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.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/mod.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.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:
crates/types/src/codec/response.rscrates/types/src/codec/request.rs
crates/types/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Runtime helpers must cover marks, scopes, continuations, and isolated scope stacks.
Files:
crates/types/src/codec/response.rscrates/types/src/codec/request.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/core/tests/unit/codec/oci_genai_tests.rs
🔇 Additional comments (4)
crates/types/src/codec/request.rs (1)
537-549: LGTM!crates/types/src/codec/response.rs (1)
329-339: LGTM!crates/core/src/codec/oci_genai.rs (1)
1-113: LGTM!Also applies to: 201-258, 266-345
crates/core/src/codec/mod.rs (1)
19-19: LGTM!
|
End-to-end validation re-run on the updated series content (including the empty-content fix above), via the Python binding built from the stack tip:
Unit suite on the same tip: 33 OCI codec tests green, full workspace build and tests passing, clippy zero warnings. |
Introduce the first layer of the OCI Generative AI provider codec:
typed ApiSpecificRequest::OCIGenAI and ApiSpecificResponse::OCIGenAI
variants, plus an OCIGenAIChatCodec implementing LlmResponseCodec.
Response decode covers ChatResult ({modelId, chatResponse}) and bare
chat responses in both apiFormat variants (GENERIC choices-based and
COHERE text-based), tolerates camelCase, kebab-case, and snake_case
key conventions (SDK vs CLI shapes), parses GENERIC string-encoded
tool-call arguments, maps promptTokens/completionTokens/totalTokens
into Usage, and maps finish reasons (stop/COMPLETE -> complete,
length/MAX_TOKENS -> length, tool_calls -> tool_use, else unknown).
Request encode/decode, streaming, and provider-surface registration
follow in subsequent changes.
Signed-off-by: Federico Kamelhar <federico.kamelhar@oracle.com>
21d6e18 to
1b8a31f
Compare
|
Second CodeRabbit pass triaged:
Suite on the updated tip: 34 OCI codec tests green, clippy zero warnings, full stack rebased. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/core/src/codec/oci_genai.rs`:
- Around line 313-333: The decode_response_tool_call function currently assigns
an empty string when the OCI response omits id, causing parallel Cohere tool
calls to collide. Replace the unwrap_or_default fallback with a stable, unique
per-call identifier derived from the available tool-call data, while preserving
the provided id unchanged when present.
🪄 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: bb97e075-db42-4f75-be55-471c3f8be155
📒 Files selected for processing (5)
crates/core/src/codec/mod.rscrates/core/src/codec/oci_genai.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/types/src/codec/request.rscrates/types/src/codec/response.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (17)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/types/src/codec/request.rscrates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/types/src/codec/request.rscrates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.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/types/src/codec/request.rscrates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.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/types/src/codec/request.rscrates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.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/types/src/codec/request.rscrates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.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:
crates/types/src/codec/request.rscrates/types/src/codec/response.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/types/src/codec/request.rscrates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/core/src/codec/oci_genai.rs
crates/types/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Runtime helpers must cover marks, scopes, continuations, and isolated scope stacks.
Files:
crates/types/src/codec/request.rscrates/types/src/codec/response.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/types/src/codec/request.rscrates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.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/types/src/codec/request.rscrates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/types/src/codec/request.rscrates/core/src/codec/mod.rscrates/types/src/codec/response.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.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/codec/mod.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.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/mod.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.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/mod.rscrates/core/src/codec/oci_genai.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/src/codec/mod.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.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/mod.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/core/tests/unit/codec/oci_genai_tests.rs
🔇 Additional comments (5)
crates/types/src/codec/request.rs (1)
537-549: LGTM!crates/types/src/codec/response.rs (1)
329-339: LGTM!crates/core/src/codec/oci_genai.rs (1)
1-354: Rest of the file (key-convention helpers, GENERIC content decoding, envelope/chat_response split, usage mapping, finish-reason mapping) is correct and the previously-flagged empty-content and error-path issues are now addressed.crates/core/src/codec/mod.rs (1)
19-19: LGTM!crates/core/tests/unit/codec/oci_genai_tests.rs (1)
1-247: LGTM! Both previously-flagged gaps (invalid-content error path, mixed-partsProviderNativecoverage) are now covered.
|
@fede-kamel A few questions:
If Oracle strictly adheres to Cohere, then I'd expect separate CohereV1 and CohereV2 Codecs rather than an Oracle-specific codec.
This seems generally problematic to me. Schema specifications should be concrete and well-defined. If Oracle is extending the Cohere schema to be case/format insensitive, I really need good justification as to why a vendor-specific schema should live in Relay itself. Cohere's API is clearly defined with |
Validated the codec against live OCI Generative AI responses across
model families (Meta, OpenAI, Google, xAI, Cohere) and the three
renderings Oracle tooling emits for the documented camelCase schema.
Changes from that sweep:
- Unwrap the OCI CLI data envelope so captured CLI output decodes like
wire and SDK payloads; document the transport matrix in the module
doc (REST wire/SDKs camelCase, oci.util.to_dict() snake_case, CLI
kebab-case in a data envelope)
- Map the lowercase max_tokens finish reason emitted by Gemini models
to FinishReason::Length
- Map promptTokensDetails.cachedTokens (OpenAI/xAI models) to
cache_read_tokens instead of dropping it
- Synthesize positional call_{index} ids for COHERE tool calls, which
carry no id on the wire, so parallel calls stay distinguishable
- Add unit tests for each live-observed shape and a pipeline
integration test decoding an OCI GENERIC tool-call response through
llm_call_execute
Signed-off-by: Federico Kamelhar <federico.kamelhar@oracle.com>
|
Thanks for the careful review @willkill07 — these are exactly the right questions. Rather than answer from the docs alone, I captured live responses from the service (us-chicago-1, one model from each family: Meta, OpenAI, Google, xAI, Cohere) and checked every claim below against them. 1. Cohere v1 or v2? Neither, and that's the crux: Oracle does not strictly adhere to Cohere. The COHERE format borrows Cohere v1 semantics (single top-level {
"modelId": "cohere.command-r-08-2024",
"modelVersion": "2.0",
"chatResponse": {
"apiFormat": "COHERE",
"text": "...",
"chatHistory": [{"role": "USER", "message": "..."}, {"role": "CHATBOT", "message": "..."}],
"finishReason": "COMPLETE",
"toolCalls": [{"name": "get_weather", "parameters": {"city": "Paris"}}],
"usage": {"promptTokens": 29, "completionTokens": 37, "totalTokens": 66}
}
}A native CohereV1 codec (snake_case, no envelope) would decode none of this. If Relay later adds Cohere's own API as a provider surface, CohereV1/V2 codecs make sense there — but they'd be decoding a different wire format than this one. I also checked the newest Cohere model on OCI ( 2. Streaming Intentional for this layer — streaming is layer 3 of the series ( 3. GENERIC spec It's Oracle's own documented schema:
So routing GENERIC through the OpenAI codec isn't an option; it hard-errors on the tool-call shape and misses the envelope and usage fields. I also verified GENERIC is what every non-Cohere family returns on OCI ( 4. Key conventions Agreed that a case/format-insensitive schema would be a smell, and the module doc described this poorly — I've reworded it in the latest push. To state it precisely: Oracle's schema is concrete, and the REST wire is strictly camelCase, always. The three spellings exist because Oracle's own tooling re-keys the same schema deterministically at different capture points:
I verified all three by running the same call through each path; the test fixtures now mirror those captures. The lookup is not fuzzy matching — the codec derives the kebab/snake spelling mechanically from the documented camelCase key, so there's exactly one accepted rendering per convention and no ambiguity. The snake_case path matters most in practice: Python is Relay's biggest binding, and That said, if you'd rather the built-in codec accept only the wire format and leave CLI/SDK-dict renderings to the caller, I'm happy to trim to camelCase-only — small diff, and I'd rather match the project's conventions than defend a preference. The live sweep also surfaced a few real-world shapes the first cut missed, all fixed with tests in the latest commit: Gemini models emit a lowercase |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/core/src/codec/oci_genai.rs (1)
245-249: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHandle
COHEREV2as a Cohere response
COHEREV2also carries its generated text inmessage, but this branch sends it to the generic parser, which expectschoicesand drops valid V2 responses. Add an explicitCOHEREV2case here, or reject unsupported formats instead of treating them as generic.🤖 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/codec/oci_genai.rs` around lines 245 - 249, Update the api_format branch in the response decoding flow to route both "COHERE" and "COHEREV2" through decode_cohere_response_body; ensure COHEREV2 no longer reaches decode_generic_response_body, which expects choices.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/core/tests/integration/pipeline_tests.rs`:
- Around line 1442-1515: Make subscriber cleanup in the OCI response codec test
panic-safe by introducing an RAII guard immediately after registering the
subscriber, using its drop behavior to call deregister_subscriber for
"oci_resp_codec_sub". Remove the explicit cleanup dependency at the end of the
test while preserving deregistration on successful completion and during
assertion unwinding.
---
Outside diff comments:
In `@crates/core/src/codec/oci_genai.rs`:
- Around line 245-249: Update the api_format branch in the response decoding
flow to route both "COHERE" and "COHEREV2" through decode_cohere_response_body;
ensure COHEREV2 no longer reaches decode_generic_response_body, which expects
choices.
🪄 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: d86502fa-3656-483c-99da-0672bf8768be
📒 Files selected for processing (3)
crates/core/src/codec/oci_genai.rscrates/core/tests/integration/pipeline_tests.rscrates/core/tests/unit/codec/oci_genai_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (15)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/core/tests/integration/pipeline_tests.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/tests/integration/pipeline_tests.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/tests/integration/pipeline_tests.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/tests/integration/pipeline_tests.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/core/tests/integration/pipeline_tests.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/core/tests/integration/pipeline_tests.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/core/tests/integration/pipeline_tests.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/core/tests/integration/pipeline_tests.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/tests/integration/pipeline_tests.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/core/tests/integration/pipeline_tests.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/core/tests/integration/pipeline_tests.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/tests/integration/pipeline_tests.rscrates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/core/tests/integration/pipeline_tests.rscrates/core/tests/unit/codec/oci_genai_tests.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/oci_genai.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/oci_genai.rs
🔇 Additional comments (4)
crates/core/src/codec/oci_genai.rs (2)
13-18: 📐 Maintainability & Code QualityRecord the required core-runtime validation matrix.
Please attach CI evidence for
cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,just test-rust,cargo deny check,validate-change, the Rust/Python/Go/Node matrix, anduv run pre-commit run --all-files.As per coding guidelines, changed
crates/corecode requires these Rust checks,validate-change, dependency validation, and the full language matrix.Source: Coding guidelines
219-224: LGTM!Also applies to: 297-371
crates/core/tests/unit/codec/oci_genai_tests.rs (1)
131-206: LGTM!Also applies to: 305-358, 368-369
crates/core/tests/integration/pipeline_tests.rs (1)
33-33: LGTM!
Relay only interfaces with the wire format, so yes. |
Per review, Relay only interfaces with the wire format, so the codec now accepts the documented camelCase schema only. Removes the kebab-case/snake_case key derivation and the CLI data-envelope unwrapping; converting alternate renderings produced by Oracle tooling is the caller's responsibility. Adds a contract test asserting non-wire renderings are not decoded. Signed-off-by: Federico Kamelhar <federico.kamelhar@oracle.com>
|
Done in 7282cdd — the codec now accepts the camelCase wire format only. Dropped the kebab/snake key derivation and the CLI |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/core/tests/unit/codec/oci_genai_tests.rs`:
- Around line 106-124: Extend test_non_wire_renderings_are_not_decoded with a
kebab-case CLI-shaped response wrapped in {"data": {...}}, using the documented
non-REST field names. Decode it with OCIGenAIChatCodec and assert model,
message, and finish_reason are all None, matching the existing snake_case case.
🪄 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: 6bd01613-ee5b-40e4-9587-2f9cfce7b63d
📒 Files selected for processing (2)
crates/core/src/codec/oci_genai.rscrates/core/tests/unit/codec/oci_genai_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (15)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/tests/unit/codec/oci_genai_tests.rscrates/core/src/codec/oci_genai.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/core/tests/unit/codec/oci_genai_tests.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/oci_genai.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/oci_genai.rs
🔇 Additional comments (1)
crates/core/src/codec/oci_genai.rs (1)
168-196: 📐 Maintainability & Code QualityProvide the required core validation evidence.
The summary reports formatting, workspace tests, and Clippy, but not
just test-rust,cargo deny check,validate-change,uv run pre-commit run --all-files, or the required Rust/Python/Go/Node matrix forcrates/corechanges. Attach the corresponding CI or command logs before handoff.As per coding guidelines, “If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js,” and core runtime changes must “satisfycargo deny checkperdeny.toml.”Source: Coding guidelines
Extends the non-wire-rendering contract test with a kebab-case data-enveloped CLI-shaped response, guarding the REST-only boundary against regression on both alternate renderings. Signed-off-by: Federico Kamelhar <federico.kamelhar@oracle.com>
willkill07
left a comment
willkill07
left a comment
There was a problem hiding this comment.
Thanks for splitting this into a focused first step. I found two issues that need addressing before this response codec is ready to merge.
Per review on the OCI codec: - Add a COHEREV2 decoder matching the OCI CohereChatResponseV2 schema: single assistant message with typed content parts (TEXT flattened, THINKING/IMAGE_URL/DOCUMENT preserved as provider-native), nested OpenAI-style function tool calls, response id, and the V2 finish reasons TOOL_CALL and STOP_SEQUENCE. The live service does not accept the format yet (probed us-chicago-1: HTTP 400), so the fixture mirrors the published SDK schema. - Preserve envelope and chat-response fields the normalized shape does not model (timeCreated, serviceTier, chatHistory, grounding metadata, future fields) in extra instead of discarding them, consistent with the other response codecs. Signed-off-by: Federico Kamelhar <federico.kamelhar@oracle.com>
|
/ok to test 3573bc2 |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
willkill07
left a comment
willkill07
left a comment
There was a problem hiding this comment.
I think this is my last round of changes
- Use set-or-remove semantics in the OCI patch_params so an intercept clearing temperature, topP, maxTokens, or stop removes the provider field, matching the other provider codecs; cover clears in tests - Suppress the terminal COHERE full-text snapshot in the local guardrails stream extractor so output rails do not evaluate doubled text, mirroring the OCI streaming collector deduplication - Add oci_genai to the Node LlmCodecIdentity builtin union - Refresh stale codec-support lists: pii-redaction config table and README, NeMo Guardrails support table and codec boundary, middleware concepts, and native dynamic plugin docs Signed-off-by: Federico Kamelhar <federico.kamelhar@oracle.com>
|
/ok to test 895842a |
|
@fede-kamel can you please update: |
|
/ok to test 98b4c75 |
|
@fede-kamel I also notice this PR doesn't update other areas of the codebase related to plugins: https://github.com/search?q=repo%3ANVIDIA%2FNeMo-Relay+BuiltinLlmCodec&type=code
|
- Add the OCIGenAI variant to the core BuiltinLlmCodec enum and implement codec_identity on OCIGenAIChatCodec so sanitize contexts report the builtin oci_genai identity instead of opaque - Map oci_genai in the worker SDK proto decoding and in the native plugin SDK enum and async invocation context, matching the other built-ins - Route the BuiltIn(OCIGenAI) identity to the OCI provider surface in the pii-redaction normalized sanitize path - Document the OCI GenAI codec in the provider codec guides: built-in codec lists, per-binding import table, usage normalization table, and the gateway route note Signed-off-by: Federico Kamelhar <federico.kamelhar@oracle.com>
|
@willkill07 both follow-ups are covered in c2048f2: Docs —
New tests cover each mapping: codec-identity tests for both trait impls, OCI rows in the request/response sanitizer context state tests, worker proto decoding, a plugin async-SDK context test (fails with Validated locally: full test suites for core, worker, plugin, and pii-redaction, clippy with |
|
/ok to test a9492c8 |
The sanitize context's retained codec capability is released on the SDK executor thread after the result completion is delivered, so the immediate live_host_strings() == 0 assertion raced the release under nextest on CI. Poll with a bounded deadline instead, matching the wait_for_release idiom used elsewhere in this suite. Signed-off-by: Federico Kamelhar <federico.kamelhar@oracle.com>
|
The |
|
/ok to test e88e4e4 |
|
/merge |
2 participants
Overview
Adds Oracle Cloud Infrastructure (OCI) Generative AI as a built-in provider across the Rust core and the Python/Node bindings. Originally opened as layer 1 of a four-part stacked series; per reviewer request (comment), the remaining three layers are now folded into this single PR, so it carries the complete feature.
The four capability layers, now all in this PR (one commit each, reviewable in sequence):
ApiSpecificOCI variants andLlmResponseCodecforGENERIC,COHERE, andCOHEREV2ChatResultpayloadsLlmCodecdecode plus merge-not-replace encode withencode(decode(x), x) == xidentity and unmodeled-field preservationProviderSurface::OCIGenAIregistration and detection,OCIGenAIStreamingCodec(SSE for both formats), and OCI awareness in guardrails, the PII-redaction overlay, and adaptive request surfacesOCIGenAIChatCodecexposed to Python (pyo3,.pyi,codecs.py) and Node (napi,.d.ts), with binding testsDetails
What:
ApiSpecificRequest::OCIGenAI { compartment_id, serving_mode, api_format }andApiSpecificResponse::OCIGenAI { api_format, model_version }(serde tag"oci_genai") incrates/types, pluscrates/core/src/codec/oci_genai.rsintroducingOCIGenAIChatCodec:apiFormats —GENERIC(choices-based; Meta Llama, Google, xAI, OpenAI, and imported open-weights models such as NVIDIA Nemotron on dedicated AI clusters),COHERE(text-based), andCOHEREV2(single assistantmessagewith typed content parts and nested-function tool calls) — normalizing model, response id, message content, tool calls, finish reasons, and usage counters (includingpromptTokensDetails.cachedTokens). Envelope and chat-response fields outside the normalized shape (timeCreated,serviceTier,chatHistory, grounding metadata) are preserved inextra.ChatDetailsfor both formats and re-encodes by merging into the original payload rather than replacing it, so unmodeled fields survive intercept round-trips; the encode/decode identity is asserted in tests.ProviderSurface::OCIGenAIwith detection keyed on the strongest-signal envelope fields (placed first in the resolver), so OCI payloads resolve without an explicit codec. Streaming is handled byOCIGenAIStreamingCodec(SSE lifecycle for GENERIC and COHERE). Guardrails, the PII-redaction overlay, and adaptive request surfaces gain OCI-format awareness with parity cases mirroring the existing providers.OCIGenAIChatCodecclasses in Python and Node with the same construction and resolver-integration semantics as the other built-in codecs.BuiltinLlmCodec::OCIGenAIsurfaced end to end —codec_identity()on both codec trait impls, worker SDK proto decoding (crates/worker), native plugin SDK enum and async invocation context (crates/plugin), the NodeLlmCodecIdentitytyping, and PII-redaction surface routing — so sanitize callbacks in every SDK receive{ kind: "builtin", id: "oci_genai" }instead ofopaque.The codec accepts the REST wire format only (camelCase, as documented); converting alternate renderings produced by Oracle tooling is the caller's responsibility, per review discussion.
Why: OCI GenAI calls are currently opaque to Relay. This PR provides normalized
LLMEndannotations (model, finish reason, token usage), safe request editing for intercepts (redaction, policy), automatic provider detection including streaming, and access from every primary binding.How: Follows the built-in provider pattern (
anthropic.rsas template): unit-struct codec,FinishReasonmapping for the three formats' vocabularies (stop/length/max_tokens/tool_calls;COMPLETE/MAX_TOKENS;TOOL_CALL/STOP_SEQUENCE), positionalcall_{index}fallback ids for COHERE tool calls (noidon the wire). Binding classes mirrorAnthropicMessagesCodec/OpenAIChatCodecexposure.Testing: Full workspace suite green,
cargo clippy --workspace --all-targetszero warnings,cargo fmt --checkclean,missing_docssatisfied;just test-python(567 passed) andjust test-node(294 passed) green on the built bindings. OCI-specific coverage includes: GENERIC/COHERE fixtures from live OCI wire captures (simple, tool-call, mixed-content, tool-call-only, model-family sweep across Meta, OpenAI, Google, xAI, Cohere), COHEREV2 fixtures per the publishedCohereChatResponseV2schema with the wire shape confirmed against the live service, request round-trip identity and unmodeled-field preservation, wire-format-only contract tests, streaming lifecycle tests for both formats, resolver detection and parity cases, PII-redaction overlay coverage, invalid-content error paths, codec-identity tests across core, worker, plugin, and pii-redaction, and binding tests in both languages. The combined content was also validated end-to-end: the codec-encoded request was posted verbatim to the signed OCI REST chat endpoint (dedicated AI cluster serving an imported NVIDIA Nemotron 3 model) and the live reply decoded with correct finish reason and token usage. Known unrelated flake:install_registrations_covers_registry_error_edges(dynamic worker plugin) is parallelism-sensitive onmain; the worker code here is byte-identical tomainand the test passes serially and in isolation.Breaking changes: None — additive enum variants, a new codec module, a new provider surface, and new binding classes.
Where should the reviewer start?
crates/core/src/codec/oci_genai.rstop-to-bottom (module docs explain the three formats and the wire-only contract), then the fixtures and round-trip tests incrates/core/tests/unit/codec/oci_genai_tests.rs, then the resolver registration incrates/core/src/codec/resolve.rs. The four commits after themainmerge apply the layers in order if commit-by-commit review is preferred.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
New Features
Bug Fixes