fix: emit OpenInference system prompts as input messages#542
Conversation
Signed-off-by: mnajafian-nv <mnajafian@nvidia.com>
|
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:
WalkthroughOpenInference replay and annotated LLM projections now represent system prompts as indexed input messages rather than ChangesOpenInference projection
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ReplayOrAnnotatedRequest
participant OpenInferenceProjection
participant FlattenedAttributes
ReplayOrAnnotatedRequest->>OpenInferenceProjection: provide system prompt and messages
OpenInferenceProjection->>OpenInferenceProjection: validate entries and assign indices
OpenInferenceProjection->>FlattenedAttributes: emit llm.input_messages.* attributes
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
size:S
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/core/src/observability/openinference.rs (1)
1252-1267: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject messages missing either required field before emitting attributes.
The
&&condition only rejects objects missing both fields.{"role":"user"}emits a role-only message, while{"content":"x"}is accepted with a default role. Validate both the role and displayable content before writing attributes or returningtrue.As stated in the PR objective, incomplete replay messages must be skipped.
Proposed fix
fn push_replay_input_message(attributes: &mut Vec<KeyValue>, index: usize, message: &Json) -> bool { let Some(object) = message.as_object() else { return false; }; - if !object.contains_key("role") && !object.contains_key("content") { - return false; - } - let role = object.get("role").and_then(Json::as_str).unwrap_or("user"); + let Some(role) = object.get("role").and_then(Json::as_str) else { + return false; + }; + let Some(text) = object.get("content").and_then(display_text_from_json) else { + return false; + }; push_message_role(attributes, "llm.input_messages", index, role); - if let Some(text) = object.get("content").and_then(display_text_from_json) { - attributes.push(KeyValue::new( - format!("llm.input_messages.{index}.message.content"), - text, - )); - } + attributes.push(KeyValue::new( + format!("llm.input_messages.{index}.message.content"), + text, + )); true }🤖 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/observability/openinference.rs` around lines 1252 - 1267, Update push_replay_input_message to reject messages unless both role and displayable content are present. Change the missing-field check from accepting either field to requiring both, validate content via display_text_from_json before emitting any attributes, and return false for incomplete messages; only then call push_message_role, append content, and return true.
🤖 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/observability/openinference.rs`:
- Around line 1225-1250: Update push_replay_input_messages so the messages
branch returns only when at least one replay message was emitted. Track whether
push_replay_input_message incremented next_index; for empty arrays or arrays
whose entries are all ignored, continue to the existing prompt fallback, while
preserving the early return when a message was projected.
In `@crates/core/tests/unit/observability/openinference_tests.rs`:
- Around line 4202-4204: Extend the assertions in the relevant OpenInference
test to validate llm.input_messages.0.message.content against the expected
system prompt returned by sample_openinference_annotated_request(). Keep the
existing llm.system removal and role assertions, ensuring the migrated
system-prompt content is explicitly preserved.
- Around line 1708-1718: Add regression cases in the OpenInference tests for
empty messages and incomplete replay message objects, exercising the new
bool-return path and prompt fallback. Assert that prompt attributes are emitted
and their indices remain contiguous without gaps, while preserving the existing
valid-message assertions.
---
Outside diff comments:
In `@crates/core/src/observability/openinference.rs`:
- Around line 1252-1267: Update push_replay_input_message to reject messages
unless both role and displayable content are present. Change the missing-field
check from accepting either field to requiring both, validate content via
display_text_from_json before emitting any attributes, and return false for
incomplete messages; only then call push_message_role, append content, and
return true.
🪄 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: 84dc0fef-54a0-4583-b9bc-5049d18dec73
📒 Files selected for processing (2)
crates/core/src/observability/openinference.rscrates/core/tests/unit/observability/openinference_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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.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/observability/openinference.rs
crates/core/src/observability/{atif,otel,openinference}.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
When changing the core event model, emitted fields, exporter behavior, subscriber config, or lifecycle, keep the ATIF, OpenTelemetry, and OpenInference implementations in sync.
Files:
crates/core/src/observability/openinference.rs
🔇 Additional comments (2)
crates/core/src/observability/openinference.rs (2)
999-1005: LGTM!Also applies to: 1034-1040
1225-1267: 📐 Maintainability & Code QualityRun the required core/Rust validation before handoff.
As per coding guidelines, Rust changes in
crates/corerequirecargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,just test-rust,validate-change, the full Rust/Python/Go/Node matrix, anduv run pre-commit run --all-files.Source: Coding guidelines
Signed-off-by: mnajafian-nv <mnajafian@nvidia.com>
Signed-off-by: mnajafian-nv <mnajafian@nvidia.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/core/src/observability/openinference.rs (1)
1263-1278: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject incomplete replay message objects.
Line 1267 uses
&&, so{ "role": "user" }and{ "content": "..." }are projected despite being incomplete. This violates the replay contract to skip incomplete entries and can suppress prompt fallback. Require both fields before emitting.Proposed fix
- if !object.contains_key("role") && !object.contains_key("content") { + if !object.contains_key("role") || !object.contains_key("content") { return false; }🤖 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/observability/openinference.rs` around lines 1263 - 1278, The push_replay_input_message function must reject incomplete message objects by requiring both “role” and “content” keys. Change the current presence check so objects missing either field return false, while complete objects continue through role and content projection unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/core/src/observability/openinference.rs`:
- Around line 1263-1278: The push_replay_input_message function must reject
incomplete message objects by requiring both “role” and “content” keys. Change
the current presence check so objects missing either field return false, while
complete objects continue through role and content projection unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: ccf67bd1-eba7-4865-bd33-5acb817f80af
📒 Files selected for processing (2)
crates/core/src/observability/openinference.rscrates/core/tests/unit/observability/openinference_tests.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (13)
- GitHub Check: Rust / Test (linux-amd64)
- GitHub Check: Node.js / Test (windows-arm64)
- GitHub Check: Go / Test (windows-amd64)
- GitHub Check: Rust / Test (linux-arm64)
- GitHub Check: Rust / Test (windows-amd64)
- GitHub Check: Node.js / Test (windows-amd64)
- GitHub Check: Python / Test (windows-arm64)
- GitHub Check: Rust / Test (windows-arm64)
- GitHub Check: Rust / Test (macos-arm64)
- GitHub Check: Go / Test (windows-arm64)
- GitHub Check: Python / Test (linux-amd64)
- GitHub Check: Python / Test (macos-arm64)
- GitHub Check: Python / Test (windows-amd64)
🧰 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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.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/observability/openinference.rs
crates/core/src/observability/{atif,otel,openinference}.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
When changing the core event model, emitted fields, exporter behavior, subscriber config, or lifecycle, keep the ATIF, OpenTelemetry, and OpenInference implementations in sync.
Files:
crates/core/src/observability/openinference.rs
🔇 Additional comments (5)
crates/core/src/observability/openinference.rs (3)
1246-1252: Restore prompt fallback when no replay message is emitted.Line 1252 returns merely because
messagesis an array. Empty arrays and arrays whose entries are all skipped suppressprompt, including after a valid system prompt. Return only if at least one array message was projected.
1000-1004: LGTM!Also applies to: 1096-1102
1041-1047: 📐 Maintainability & Code QualityConfirm the required core-Rust validation matrix.
Please provide results 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, “If
crates/core... changed, run the full validation matrix across Rust, Python, Go, and Node.js,” and core runtime changes must satisfycargo deny checkand strict Rust validation.Source: Coding guidelines
crates/core/tests/unit/observability/openinference_tests.rs (2)
1740-1752: Add empty and malformed replay-message fallback coverage.The valid-message fixture does not exercise the skipped-entry path. Cover
messages: []and incomplete objects, assertingpromptis emitted at the next contiguous index.
408-438: LGTM!Also applies to: 4234-4240, 4321-4321, 4374-4428
Signed-off-by: mnajafian-nv <mnajafian@nvidia.com>
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/observability/openinference.rs`:
- Around line 1270-1274: Update the replay-message handling around
push_message_role to require both a string role and valid content before
emitting attributes; remove the unwrap_or("user") fallback so content-only
objects return false and are skipped. Add a test covering a content-only replay
message.
🪄 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: 0ebadba2-bca2-48eb-92bd-393034999a91
📒 Files selected for processing (2)
crates/core/src/observability/openinference.rscrates/core/tests/unit/observability/openinference_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/src/observability/openinference.rscrates/core/tests/unit/observability/openinference_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/observability/openinference.rscrates/core/tests/unit/observability/openinference_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/observability/openinference.rscrates/core/tests/unit/observability/openinference_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/src/observability/openinference.rscrates/core/tests/unit/observability/openinference_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/observability/openinference.rscrates/core/tests/unit/observability/openinference_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/observability/openinference.rscrates/core/tests/unit/observability/openinference_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/observability/openinference.rscrates/core/tests/unit/observability/openinference_tests.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/observability/openinference.rs
crates/core/src/observability/{atif,otel,openinference}.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
When changing the core event model, emitted fields, exporter behavior, subscriber config, or lifecycle, keep the ATIF, OpenTelemetry, and OpenInference implementations in sync.
Files:
crates/core/src/observability/openinference.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/observability/openinference.rscrates/core/tests/unit/observability/openinference_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/observability/openinference.rscrates/core/tests/unit/observability/openinference_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/observability/openinference.rscrates/core/tests/unit/observability/openinference_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/observability/openinference.rscrates/core/tests/unit/observability/openinference_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/observability/openinference.rscrates/core/tests/unit/observability/openinference_tests.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/core/tests/unit/observability/openinference_tests.rs
🔇 Additional comments (2)
crates/core/src/observability/openinference.rs (2)
1246-1252: Preserve prompt fallback when every replay message is skipped.An empty
messagesarray—or one containing only rejected entries—returns before projectingprompt. Return only ifnext_indexadvanced due to a replay message, then add regression cases withpromptfor empty and malformed arrays.
991-1005: 📐 Maintainability & Code QualityAttach the required core-runtime validation results.
This
crates/coreRust change needs evidence forcargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,just test-rust,cargo deny check,validate-change, the full Rust/Python/Go/Node matrix, and finaluv run pre-commit run --all-files.As per coding guidelines, “If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation” and “Ifcrates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.”Source: Coding guidelines
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/core/src/observability/openinference.rs (1)
1008-1013: 📐 Maintainability & Code Quality | 🔵 TrivialRun the Rust/core validation matrix before handoff —
cargo fmtisn’t wired in this environment, so userustfmt/the repo formatter setup, then runjust test-rust, strict Clippy,validate-change,just test-all, anduv run pre-commit run --all-files.🤖 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/observability/openinference.rs` around lines 1008 - 1013, Before handoff, validate the replay input handling around replay_llm_payload and push_replay_input_messages using the repository’s rustfmt/formatter setup rather than cargo fmt. Run just test-rust, strict Clippy, validate-change, just test-all, and uv run pre-commit run --all-files; resolve any failures before completing the change.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/core/src/observability/openinference.rs`:
- Around line 1008-1013: Before handoff, validate the replay input handling
around replay_llm_payload and push_replay_input_messages using the repository’s
rustfmt/formatter setup rather than cargo fmt. Run just test-rust, strict
Clippy, validate-change, just test-all, and uv run pre-commit run --all-files;
resolve any failures before completing the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 53f7c71a-9405-4d19-a33f-a4197eaa8d24
📒 Files selected for processing (2)
crates/core/src/observability/openinference.rscrates/core/tests/unit/observability/openinference_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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.rscrates/core/src/observability/openinference.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/observability/openinference_tests.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/observability/openinference.rs
crates/core/src/observability/{atif,otel,openinference}.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
When changing the core event model, emitted fields, exporter behavior, subscriber config, or lifecycle, keep the ATIF, OpenTelemetry, and OpenInference implementations in sync.
Files:
crates/core/src/observability/openinference.rs
🔇 Additional comments (6)
crates/core/src/observability/openinference.rs (4)
1255-1261: Preserve prompt fallback when replay messages emit nothing.The unconditional
returnskipspromptwhenmessagesis empty or every entry is invalid. Return early only after at least one replay message was emitted; otherwise continue to the existing prompt fallback. This is the same unresolved finding from the prior review.
1276-1283: Require a valid replay role before projection.Content-only or non-string-role objects still pass the guard and are exported as
userbecause ofunwrap_or("user"). Require both a stringroleand valid content, otherwise returnfalse. This is the same unresolved finding from the prior review.
565-565: LGTM!Also applies to: 660-672, 684-699
1043-1058: LGTM!Also applies to: 1105-1144
crates/core/tests/unit/observability/openinference_tests.rs (2)
1785-1838: Cover empty-array and malformed replay fallback cases.This test covers only one invalid content value. Add cases for
messages: [], all-invalid messages with aprompt, and content-only or non-string-role objects so the projection’s fallback and error paths are verified.As per path instructions, tests should cover the behavior promised by the changed API surface, including error paths.
Source: Path instructions
408-439: LGTM!Also applies to: 1740-1752, 3440-3465, 4316-4322, 4403-4403, 4457-4510
Signed-off-by: mnajafian-nv <mnajafian@nvidia.com>
|
/merge |
2 participants
Overview
Align OpenInference export behavior with the expected attribute contract by stopping Relay from writing prompt text into
llm.system. System content is now exported as orderedllm.input_messagesentries instead.Details
llm.systememission from both replay-payload projection and annotated request projection in the OpenInference exporter.systemPromptcontent by exporting it as the leadingllm.input_messages.<index>entry with rolesystem.AnnotatedLlmRequest.instructionsas the leading system input message when present.Message::Systemcontent inllm.input_messagesinstead of the legacyllm.systemfield.promptstill falls back when replaymessagesis empty or every replay message is skipped, and skip incomplete replay message objects unless they provide both a stringroleand displayablecontent.llm.systemis absent, system and user messages are exported in the expected order, replay fallback remains contiguous, and migrated system-message content is preserved.Validation:
cargo fmt --alljust test-rustcargo clippy --workspace --all-targets -- -D warningsjust test-pythonjust test-gojust test-nodeuv run pre-commit run --files crates/core/src/observability/openinference.rs crates/core/tests/unit/observability/openinference_tests.rsWhere should the reviewer start?
Start in
crates/core/src/observability/openinference.rs, especially the request projection path aroundpush_llm_request_attributes()andpush_replay_input_messages(). The most relevant regression coverage is incrates/core/tests/unit/observability/openinference_tests.rs.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
Bug Fixes
llm.input_messagesentry withrole: system, instead of emitting a separatellm.systemscalar.llm.input_messagesindexing/ordering for both annotated requests (from instructions) and replay inputs, with correct shifting of subsequent messages.Tests