feat(codec): add OCI Generative AI typed variants and response codec (series 1/4)#554
feat(codec): add OCI Generative AI typed variants and response codec (series 1/4)#554fede-kamel wants to merge 1 commit into
Conversation
github-actions
Bot
added
size:L
WalkthroughAdds OCI GenAI request and response variants, exports a codec supporting GENERIC and COHERE response formats, normalizes content, tool calls, finish reasons, and usage, and adds unit tests for supported response shapes and edge cases. ChangesOCI GenAI codec support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant OCI as OCI GenAI response
participant Codec as OCIGenAIChatCodec
participant Decoder as GENERIC or COHERE decoder
participant Result as AnnotatedLlmResponse
OCI->>Codec: submit JSON response
Codec->>Decoder: select decoder from apiFormat
Decoder-->>Codec: return decoded message and tool calls
Codec->>Result: populate normalized fields and metadata
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ 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.
| fn decode_response_tool_call(value: &Json) -> Option<ResponseToolCall> { | ||
| let obj = value.as_object()?; | ||
| let name = get_first(obj, "name")?.as_str()?.to_string(); | ||
| let arguments = match get_first(obj, "arguments") { | ||
| Some(Json::String(text)) => { | ||
| // CRITICAL: GENERIC arguments arrive JSON-encoded; parse for the | ||
| // normalized shape, preserving the raw string when unparseable. | ||
| serde_json::from_str::<Json>(text).unwrap_or_else(|_| Json::String(text.clone())) | ||
| } | ||
| Some(other) => other.clone(), | ||
| None => get_first(obj, "parameters").cloned().unwrap_or(Json::Null), | ||
| }; | ||
| Some(ResponseToolCall { | ||
| id: get_first(obj, "id") | ||
| .and_then(Json::as_str) | ||
| .unwrap_or_default() | ||
| .to_string(), | ||
| name, | ||
| arguments, | ||
| }) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Cohere tool calls need a unique fallback id
When OCI omits id, every Cohere tool call gets "", so parallel calls become indistinguishable. Synthesize a stable per-call fallback instead of defaulting to an empty string.
🤖 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 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.
1 participant
Overview
First PR of a four-part stacked series adding Oracle Cloud Infrastructure (OCI) Generative AI as a built-in provider in the Rust core — the rework requested in #549 review, split into foundation-ordered units of ~≤1k lines for reviewability (per the plan posted on #552 before closing it).
Series map (each stacked on the previous, opened as its predecessor merges):
ApiSpecificvariants + response codec (~560 lines)Details
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.rsintroducingOCIGenAIChatCodecwith itsLlmResponseCodecimplementation: decodes OCIChatResultpayloads — bothGENERIC(choices-based; Meta Llama, Google, xAI, OpenAI, and imported open-weights models such as NVIDIA Nemotron on dedicated AI clusters) andCOHERE(text-based) — normalizing model, message text, tool calls, finish reasons, and usage counters. Key lookup tolerates the three conventions OCI emits (SDK camelCase, CLI kebab-case, snake_case).Why: OCI GenAI responses are currently opaque to Relay observability. This layer alone enables normalized
LLMEndannotations (model, finish reason, token usage) for OCI-backed calls when the codec is supplied explicitly; subsequent PRs add request editing, automatic detection, and binding exposure.How: Follows the built-in provider pattern (
anthropic.rsas template): unit-struct codec, in-module tests,FinishReasonmapping for both formats' vocabularies (stop/length/tool_calls,COMPLETE/MAX_TOKENS). Response decoding is manual-extraction rather than serde intermediates because of the multi-convention keys.Testing: 8 unit tests with fixtures taken from real OCI wire traffic (GENERIC and COHERE
ChatResult, kebab-case CLI shape, non-dict fallback, finish-reason and usage mapping, invalid-content error paths, ProviderNative part preservation, tool-call-only empty content). Fullcargo test --workspacegreen (3,369 tests),cargo clippy --workspace --all-targetszero warnings,cargo fmtclean,missing_docssatisfied. The complete series content has additionally been validated end-to-end: the encoded payload of the full codec was posted verbatim to the signed OCI REST endpoint and answered by a dedicated AI cluster serving an imported NVIDIA Nemotron 3 model.Breaking changes: None — additive enum variants and a new module only.
Where should the reviewer start?
crates/types/src/codec/{request,response}.rsfor the variant shapes, thendecode_responseand the fixtures incrates/core/tests/unit/codec/oci_genai_tests.rs.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit